add* RPCs require backups #11243

issue TheBlueMatt openend this issue on September 5, 2017

TheBlueMatt commented at 6:48 pm on September 5, 2017: member

At a minimum, help text should note this. This is true for things like addwitnessaddress especially.
TheBlueMatt commented at 6:48 pm on September 5, 2017: member

This needs an 0.15.1 tag.
MarcoFalke added the label Docs and Output on Sep 5, 2017
MarcoFalke added the label RPC/REST/ZMQ on Sep 5, 2017
sipa added this to the milestone 0.15.1 on Sep 5, 2017
MarcoFalke added the label Easy to implement on Sep 5, 2017
sipa commented at 7:01 pm on September 5, 2017: member

Also, importprivkey, importmulti, importaddress, importpubkey, addmultisigaddress, … and perhaps more
laanwj commented at 5:29 pm on September 6, 2017: member

Also the watch-only ones?
sipa commented at 5:31 pm on September 6, 2017: member

Also the watch-only ones?

Yes, I’ve included those in the list above.
MarcoFalke commented at 9:11 pm on September 9, 2017: member

From #11294: The dumpwallet command currently writes out all keys, including the hd seed. In case there are keys in the wallet that are not derivable from that seed, a warning should be included to explain that that seed on itself is not enough to backup the wallet.
PavelNiedoba commented at 10:13 pm on September 24, 2017: none

From business perspective it’s very important to have possibility to backup wallet using string, which can be stored on paper or hardware password manager. Files are much harder to handle safely. Think about people running business, storing tens of millions in this wallet.
sipa commented at 10:16 pm on September 24, 2017: member

@PavelNiedoba Yes, of course. But by definition, importing something in the wallet will always require a backup. That’s why it’s important to make this clear.
JohnVonNeumann commented at 10:37 pm on November 7, 2017: none
Excuse the noob question here, I saw this had the good first issue tag applied to it so just looking to get further clarification. This issue is in regards to help output surrounding the following RPC’s:
- addwitnessaddress
- importprivkey
- importmulti
- importaddress
- importpubkey
- addmultisigaddress
The basic premise of the issue to add extra help output for users to indicate that before performing any of these actions, they should be performing a backup of the item they intend to use as input? IE: non-segwit address, multikey wallets, addresses, etc

Let me know if I’m not on the right path here.

EDIT: Apologies, I didn’t see that MeshCollider had already written a fix via #11289.
MarcoFalke commented at 10:41 pm on November 7, 2017: member

EDIT: Apologies, I didn’t see that MeshCollider had already written a fix via #11289. @JohnVonNeumann You can still help by reviewing :)
JohnVonNeumann commented at 10:57 pm on November 7, 2017: none

I kinda assumed my review would be pointless being a noob? Wouldn’t it require more experienced people to pass it? @MarcoFalke
MarcoFalke commented at 11:36 pm on November 7, 2017: member

Obviously, if you just run around and spam “ACK” or “Concept ACK”, it will be ignored. However, if you go around and find issues with the code or can figure out a solution that is cleaner, it will be greatly appreciated.
JohnVonNeumann commented at 11:43 pm on November 7, 2017: none

That makes sense then, awesome, I left a small comment on something. Cheers for the advice!
laanwj closed this on Nov 8, 2017
laanwj referenced this in commit 77546a3182 on Nov 8, 2017
codablock referenced this in commit 799b047714 on Sep 26, 2019
codablock referenced this in commit 617c886233 on Sep 30, 2019
charlesrocket referenced this in commit c9744b1c6c on Dec 16, 2019
barrystyle referenced this in commit d22b26f664 on Jan 22, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors
TheBlueMatt sipa laanwj MarcoFalke PavelNiedoba JohnVonNeumann

Labels
Docs RPC/REST/ZMQ good first issue

Milestone
0.15.2