- strUsage += HelpMessageOpt("-prune=<n>", strprintf(_("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex and -rescan. "
+ strUsage += HelpMessageOpt("-prune=<n>", strprintf(_("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex and importing keys or addresses may fail."
"Warning: Reverting this setting requires re-downloading the entire blockchain. "
"(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >%u = automatically prune block files to stay under the specified target size in MiB)"), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024));
strUsage += HelpMessageOpt("-reindex-chainstate", _("Rebuild chain state from the currently indexed blocks"));
It happens outside of this call (somewhere in wallet.cpp). But the comment doesn't say "It" calls rescanblockchain, it either says "you" can call rescanblockchain().
// Recovery procedure:
// move wallet file to wallet.timestamp.bak
// Call Salvage with fAggressive=true to
// get as much data as possible.
// Rewrite salvaged data to fresh wallet file
// call rescanblockchain (RPC) so any missing
// transactions will be found.
This function does:
move wallet file to wallet.timestamp.bak
Call Salvage with fAggressive=true to get as much data as possible.
Rewrite salvaged data to fresh wallet file
but it doesn't call rescanblockchain, and I can't see rescanblockchain being called by the callers.
Yes. You right. The rescan is not part of the call. But the part that calls CDB::Recover does always call a rescan. But now I got your point. We should refer to ScanForWalletTransactions() instead to rescanblockchain (RPC). Right?
It's confusing. But CDB::Recover only gets calls by setting -salvagewallet which does set -rescan. Maybe I should remove that comment part... I don't know what's best because -rescan is currently mentioned there.
ah, makes sense now. Thank you. I wasn't looking in init.cpp, but now I think I see how this fits together. CWallet::Verify() is called first, which calls CWalletDB::Recover() if -salvagewallet is set. Later in AppInitMain(), we call CWallet::AppInitMain(), which is where the rescan happens if -salvagewallet is set.
Perhaps change the comment to something like:
// Try to recover the wallet:
// - move wallet file to wallet.timestamp.bak
// - call Salvage with fAggressive=true to get as much data as possible.
// - rewrite salvaged data to fresh wallet file
//
// CWallet::AppInitMain() will later run a full blockchain rescan to find
// any missing transactions.
This should check against pIndexStop too, so error won't trigger unnecessarily in cases where missing blocks are noncontinuous (which can happen with manual pruning).
+ block = block->pprev;
+ }
+
+ if (pIndexStart->nHeight < block->nHeight) {
+ throwJSONRPCError(RPC_WALLET_ERROR, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height.");
strUsage += HelpMessageOpt("-txconfirmtarget=<n>", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET));
strUsage += HelpMessageOpt("-walletnotify=<cmd>", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)"));
- strUsage += HelpMessageOpt("-zapwallettxes=<mode>", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") +
+ strUsage += HelpMessageOpt("-zapwallettxes=<mode>", _("Delete all wallet transactions and only recover those parts of the blockchain through a rescan on startup") +
"" + _("(1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)"));
Is there any advantage to getting rid of the -rescan option if we still have keep all the rescan logic below? Is there a future cleanup or new feature that will be easier to implement with the option gone?
The -rescan option is global while this PR changes it (partially) to per-wallet. Still, -zapwallettx, etc. need to also be moved to per-wallet basis (I don't know how right now).
But removing the global -rescan option has to be done sooner or later if we want proper multiwallet.
This is confusing: the if statement requires !needsRescan. Shouldn't that be the other way around? If needsRescan is true, we should rescan. I think the if statement should be:
if (needsRescan || GetBoolArg("-salvagewallet", true) || GetBoolArg("-zapwallettxes", true))
re: zapwallet and salvagewallet. I agree that they should be changed to RPCs for multiwallet.
No. I don't think so.
If that if statement is true, we load the wallet best block which prevents a complete rescan (so it's the opposite).
So we only load the wallet best block (== no rescan) if needsRescan is false and -salvagewallet and --zapwallettxes` have not been set.
I can't see where
rescanblockchain()is being called. Am I missing something?