Make display of private key in beaconstatus OPT-IN only #1275
Conversation
|
I wholeheartedly agree with this!
…Sent from my iPhone
On Aug 21, 2018, at 12:44 PM, Paul Jensen ***@***.***> wrote:
As we all know this has happened numerous times where a user seeking help unknowingly displays their own private keys while displaying beaconstatus for help from the community (ex today from yet another user). This private key is not required for a user to receive help from any community member and should be default "eyes-only". I've done an implementation to do as such;
Allow the cpid string option to be empty or null allowing use of the 2nd paramater argument without user having to enter their cpid
Add default bool of false for display of private key
Update the help to reflect that
removed not needed .c_str()'s for push backs
I used the same method for the bool of int or true/false as @jamescowens had implemented in another PR thou I think we should come up with a static or inline function of some sort that does this determination for us or perhaps modify univalue bool to accept an int as well as text true/false for a bool in future.
You can view, comment on, or merge this pull request online at:
#1275
Commit Summary
Make display of private key in beaconstatus OPT-IN only
File Changes
M src/rpcblockchain.cpp (25)
M src/rpcclient.cpp (1)
Patch Links:
https://github.com/gridcoin-community/Gridcoin-Research/pull/1275.patch
https://github.com/gridcoin-community/Gridcoin-Research/pull/1275.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Conflicts #1088 |
|
Advertise and import of my PR needs some testing. I tried to write tests, but only managed to write some smoke tests and none for the major import and advertise functions. |
|
Why don’t we first implement it “alongside” the traditional approach, and also have a setting in conf that says “look in app cache” or look in conf file for the beacon keys. That way if something goes wrong with the wallet based beacon key storage, someone can easily revert to the conf based version. This can eventually be removed once well tested. This allows us to test this in testnet without doing impossible to write unit tests... |
|
@tomasbrod yes i see the conflict. how about this general idea on top of where u mention the todo about displaying it. this idea can wait till after yours is implemented or before it whichever works best for u. I agree we should show it only if the use opts to see it and ur comment generally does that. If you choose to add this idea just directly to ur pr i'll close this one. |
|
#1257 is good idea (please change the title), but this pull request implementation is kind of duplicate to what I wrote. |
|
i'll close this and you can do ur implementation as you see fit since it is a change in your area of coding with regards to beacon keys in wallet |
Added: - Add `rainbymagnitude` RPC command #1235 (@Foggyx420). - Add stake splitting and side staking #1265 (@jamescowens). - Detect and block Windows shutdown so wallet can exit cleanly #1309 (@jamescowens). - Add message support to sendfrom and sendtoaddress #1400 (@denravonska). Changed: - Configuration options are now case insensitive #294 (@Foggyx420). - Update command in beaconstatus help message #1312 (@chrstphrchvz). - Improve synchronization speeds: - Refactor superblock pack/unpack #1194 (@denravonska). - Optimize neuralsecurity calculations #1255 (@denravonska). - Reduce hash calculations when checking blocks #1206 (@denravonska). - Make display of private key in beaconstatus OPT-IN only #1275 (@Foggyx420). - Store Beacon keys in Wallet #1088 (@tomasbrod). - Use default colors for pie chart #1333 (@chrstphrchvz). - Show hand cursor when hovering clickable labels #1332 (@chrstphrchvz). - Update README.md #1337 (@Peppernrino). - Fix integer overflow with displayed nonce #1297 (@personthingman2). - Improve application cache performance #1317 (@denravonska). - Improve reorg speeds #1263 (@denravonska). - Update Polish translation #1375 (@michalkania). Fixed: - Remove expired polls from overview page #1250 (@personthingman2). - Fix plural text on block age #1304 (@scribblemaniac). - Fix researcher staking issue if your chain head was staked by you, #1299 (@denravonska). - Fix incorrect address to grcpool node #1314 (@wilkart). - Do not replace underscores by spaces in Qt Poll URLs #1327 (@tomasbrod). - Fix scraper SSL issues #1330 (@Foggyx420). Removed: - Remove or merged several RPC commands #1228 (@Foggyx420): - `newburnaddress`, removed. - `burn2`: Removed. - `cpid`: Merged into `projects`. - `mymagnitude`: Merged into `magnitude`. - `rsa`: Removed, use `magnitude`. - `rsaweight`: Removed, use `magnitude`. - `proveownership`: Removed. - `encrypt`: Removed. - Remove obsolete POW fields from RPC responses #1358 (@jamescowens). - Remove obsolete netsoft fields for slight RAM requirement reduction #1336 (@denravonska). - Remove unused attachment functionality #1345 (@denravonska).
As we all know this has happened numerous times where a user seeking help unknowingly displays their own private keys while displaying
beaconstatusfor help from the community (ex today from yet another user). This private key is not required for a user to receive help from any community member and should be default "eyes-only". I've done an implementation to do as such;Allow the cpid string option to be empty or null allowing use of the 2nd paramater argument without user having to enter their cpid
Add default bool of false for display of private key
Update the help to reflect that
removed not needed .c_str()'s for push backs
I used the same method for the bool of int or true/false as @jamescowens had implemented in another PR thou I think we should come up with a static or inline function of some sort that does this determination for us or perhaps modify univalue bool to accept an int as well as text true/false for a bool in future.