Enable wallet key imports without rescan in pruned mode. #6645

pull gmaxwell wants to merge 1 commits into bitcoin:master from gmaxwell:prune_imports changing 1 files +9 −6

gmaxwell commented at 1:31 AM on September 7, 2015: contributor

Complete rescan is incompatible with pruning, but rescan is optional on our wallet key import RPCs. Import on use is very useful in some common situations in conjunction with pruning, e.g. merchant payment tracking.

This reenables importprivkey/importaddress/importpubkey when rescan is not used.

In the future we should consider changing the rescan argument to allow depth or date to allow limited rescanning when compatible with the retained block depth.

Enable wallet key imports without rescan in pruned mode.

Complete rescan is incompatible with pruning, but rescan is optional on
 our wallet key import RPCs.  Import on use is very useful in some common
 situations in conjunction with pruning, e.g. merchant payment tracking.

This reenables importprivkey/importaddress/importpubkey when rescan
 is not used.

In the future we should consider changing the rescan argument to allow depth
 or date to allow limited rescanning when compatible with the retained
 block depth.

77c6072473

gmaxwell commented at 1:32 AM on September 7, 2015: contributor

I only performed the most trivial of tests. I am opening this PR somewhat prematurely because I don't know why it wasn't done this way in the first place, and wanted to find out if there was some huge reason I was missing before I spent more time testing. :)
gmaxwell commented at 2:35 AM on September 7, 2015: contributor

@jonasschnelli Above question is wrt 7a12119d7a9084010725cf5b95bafcc3cf2c1d71.
jonasschnelli commented at 4:11 AM on September 7, 2015: contributor

Nice!

Right. I should have this done in the fist place.

utACK.
jonasschnelli commented at 4:12 AM on September 7, 2015: contributor

There is also a PR that would optimize importing depth: #6570
gmaxwell commented at 5:12 AM on September 7, 2015: contributor

Awesome. Okay I'll run it through some more test cases then!
gmaxwell added the label Wallet on Sep 7, 2015
fanquake commented at 12:50 PM on September 7, 2015: member

utACK
dcousens commented at 12:55 PM on September 7, 2015: contributor

utACK && concept ACK
gmaxwell commented at 11:16 PM on September 7, 2015: contributor

I've tested this more extensively now and can't seem to break it.
in src/wallet/rpcdump.cpp:None in 77c6072473
```
 111 | @@ -114,6 +112,9 @@ UniValue importprivkey(const UniValue& params, bool fHelp)
 112 |      if (params.size() > 2)
 113 |          fRescan = params[2].get_bool();
 114 |  
 115 | +    if (fRescan && fPruneMode)
 116 | +        throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode");
```
btcdrak commented at 11:27 PM on September 7, 2015:

Maybe "Wallet rescanning is disabled in pruned mode"?

gmaxwell commented at 11:36 PM on September 7, 2015:

Technically it is the blockchain that is rescanned. But these options have a boolean parameter "rescan" which defaults, so what its saying is that this option is disabled.

I think if I were to clarify this further I would say that the rescan option was disabled. Or maybe to be kind to translaters say something like the requested rescan depth was too deep (as I expect we'll further parameterize this in the future).

btcdrak commented at 11:39 PM on September 7, 2015:

You could just say "The -rescan option is disabled in pruned mode" which would be more clear (to me at least :)

laanwj commented at 2:18 PM on September 8, 2015:

But this isn't about the -rescan option, but rescans triggered some other way. I think the message is OK.

laanwj commented at 2:20 PM on September 8, 2015:

The only improvement I can think of would be to add a comment to the error message that it is possible to pass 'false' as the rescan parameter to avoid this error.

You don't have to worry about translators, RPC error messages aren't translated.
laanwj commented at 2:54 PM on September 8, 2015: member

utACK
laanwj merged this on Sep 23, 2015
laanwj closed this on Sep 23, 2015
laanwj referenced this in commit 4dbd43ed0a on Sep 23, 2015
luke-jr referenced this in commit 23d35c315a on Dec 28, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels