[BUG][GUI] Fix double conf dialog for RPC commands that expose privkeys #2122
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently the confirmation pop-up is shown only when the RPC command string is exactly "dumpwallet"/"dumpprivkey" (with no arguments). Such command strings are not valid, as both RPC functions require one argument (the filename for "dumpwallet" and the PIVX address for "dumpprivkey"). So, when the user confirms the dialog, the RPC console returns an error (as the argument is missing), showing the RPC help. If the user, instead, enters the correct RPC strings (`dumpwallet <filename>`/`dumpprivkey <pivxaddress>`), then **the warning dialog is NOT presented** and the command is executed immediately. Fix it by doing a pre-validation for the potentially dangerous commands.
random-zebra
added
GUI
Bug
Needs Backport
Fuzzbawls
approved these changes
Jan 4, 2021
furszy
approved these changes
Jan 4, 2021
furszy
referenced
this pull request
in furszy/bitcoin-core
Jan 5, 2021
Currently the confirmation pop-up is shown only when the RPC command string is exactly "dumpwallet"/"dumpprivkey" (with no arguments). Such command strings are not valid, as both RPC functions require one argument (the filename for "dumpwallet" and the PIVX address for "dumpprivkey"). So, when the user confirms the dialog, the RPC console returns an error (as the argument is missing), showing the RPC help. If the user, instead, enters the correct RPC strings (`dumpwallet <filename>`/`dumpprivkey <pivxaddress>`), then **the warning dialog is NOT presented** and the command is executed immediately. Fix it by doing a pre-validation for the potentially dangerous commands. Github-Pull: bitcoin#2122 Rebased-From: 1d63d1a
furszy
referenced
this pull request
in furszy/bitcoin-core
Jan 5, 2021
Github-Pull: bitcoin#2122 Rebased-From: 81e4d15
Fuzzbawls
added a commit
that referenced
this pull request
Jan 6, 2021
41fb982 Add PR list and authors to release notes for v5.0.0 (Fuzzbawls) 528912b [Doc] release-notes: final enforcement block height plus timeline recommendation. (furszy) a778833 [Doc] Add "Mandatory Upgrade" section (ActiveProtocol upgrade time TBD) Github-Pull: #2114 Rebased-From: 8f9e1d9 (random-zebra) 0c2dc8d [Doc] Document HD wallet upgrade to SAPLING_VERSION Github-Pull: #2114 Rebased-From: c94c19e (random-zebra) ff59122 [Doc] Document SwiftX and Zerocoin removal Github-Pull: #2114 Rebased-From: d36f37f (random-zebra) 0e37c67 [Doc] Document consensus and policy changes related to Shield Github-Pull: #2114 Rebased-From: cc3c83d (random-zebra) 07d9db7 [Doc] Document more RPC changes Github-Pull: #2114 Rebased-From: 08b50bb (random-zebra) 95992ac [Doc] minor changes to the "Shield Protocol" section of the relnotes Github-Pull: #2114 Rebased-From: 0f3f77d (random-zebra) 75a7597 [Doc] Document new dependencies (sodium/rust) Github-Pull: #2114 Rebased-From: 39ee780 (random-zebra) 033e8b3 [Doc] Document sapling parameters for installation/upgrade Github-Pull: #2114 Rebased-From: 1d18ebf (random-zebra) a3d2b52 Add RPC changes to the release notes (Fuzzbawls) b2b113c [Doc] v5 release notes (furszy) 3fd22bf [GUI] Update Translations from Transifex (Fuzzbawls) 78d0b01 Test case: StoreAndLoadSaplingZkeys, added a loop to try to generate a diversified address different than the default one. (furszy) 090b901 [RPC][Wallet] Update also Sapling HD seed with sethdseed Github-Pull: #2120 Rebased-From: 092f825 (random-zebra) 36336ec [Tests] Verify Sapling wallet HD chain in wallet_hd.py Github-Pull: #2120 Rebased-From: e14fe3d (random-zebra) d8d347e [RPC] Add shield address support to getaddressinfo Github-Pull: #2120 Rebased-From: 8ec1875 (random-zebra) 9eb84ce [Cleanup][RPC] Remove not-returned fields from getaddressinfo help Github-Pull: #2120 Rebased-From: 501782a (random-zebra) 0d8b6c0 Cleaning up not used `nWalletDBUpdated`. (furszy) 6219dfe [GUI] fixing tooltip balance amount cropped when the value surpass the 1kk range. (furszy) 0e158b0 RPC: Document the `services` return entry in `getinfo` (Fuzzbawls) 576efe8 RPC: Properly document all example return fields in getchaintips (Fuzzbawls) 7100c33 RPC: Fix help output for getblockchaininfo (Fuzzbawls) 3254dc8 RPC: Fix help output for waitfor* commands (Fuzzbawls) 46f47c5 RPC: document results for dumpwallet (Fuzzbawls) 3bfb513 RPC: drop "ed" suffix from input arguments for shield export commands (Fuzzbawls) 9bc65af RPC: document per-network MN count for getmasternodecount (Fuzzbawls) aaf9216 RPC: Remove unused mnEntries vector (Fuzzbawls) 90b1d6e [GUI][RPC] Add exportsaplingkey to the list of pot. dangerous cmds Github-Pull: #2122 Rebased-From: 81e4d15 (random-zebra) 55ff95e [GUI][BUG] Fix confirmation dialog for dumpwallet/dumpprivkey (random-zebra) 8863d76 [GUI][Trivial] Save cold-staking address in contacts (random-zebra) 51e1c97 Test: print error if shield diversified address is equal to the default address. (furszy) 351b2ce [Refactor] CoinControl: getTotals returns struct Github-Pull: #2111 Rebased-From: efa1331 (random-zebra) 6506d71 [BUG] Fix isDust label in coincontrol Github-Pull: #2111 Rebased-From: 0e08e40 (random-zebra) 805552c [Refactor] CControl: decouple getTotals from updateLabels Github-Pull: #2111 Rebased-From: 8e79aa9 (random-zebra) 391eff6 [GUI] Fix size computation in coin-control (random-zebra) 1656246 [Doc] RPC getrawtransaction and decoderawtransaction, sapling transaction data (furszy) 1ed9970 Set v5 enforcement time for mainnet (furszy) 0cf5703 [GUI] Support for proposal outputs (op_return). (furszy) Pull request description: Back porting PRs from master: #2100 #2111 #2112 #2113 #2114 #2115 #2116 #2119 #2120 #2121 #2122 #2123 #2125 ACKs for top commit: random-zebra: utACK 41fb982 Fuzzbawls: ACK 41fb982 Tree-SHA512: da8706da289ea7f0b09fc6fa39bd55579d25cb6b0761ac9999ccf6609cd68fe54bd5b325d9c686758b78b234397eb1e1d0091e69f663110f695899f8fa7c3c7a
random-zebra
removed
the
Needs Backport
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix an issue with the double confirmation dialog introduced in #1928
Currently the confirmation pop-up is shown only when the RPC command string is exactly "dumpwallet"/"dumpprivkey" (with no arguments).
Such command strings are not valid, as both RPC functions require one argument (the filename for "dumpwallet" and the PIVX address for "dumpprivkey").
So, when the user confirms the dialog, the RPC console returns an error (as the argument is missing), showing the RPC help.
When, instead, the user enters the correct RPC strings (
dumpwallet <filename>/dumpprivkey <pivxaddress>), then the warning dialog is NOT presented and the command is executed immediately.Fix it by doing a pre-validation for the potentially dangerous commands.
Bonus: include
exportsaplingkeyto the list.Note: for
dumpprivkey/exportsaplingkeythis means a double parsing of the command string and decoding of the address argument (one during the pre-validation, and one during the actual execution of the RPC). But I think we can live with that for now.