wallet: reserve walletrescan before checking wallet is at the tip
What the developer wrote
Authored by Pol Espinasa
wallet: reserve walletrescan before checking wallet is at the tip
What changed, and why it matters
This small change reorders two operations in the `importdescriptors` wallet command. Previously, the wallet first waited until it was caught up to the latest block, then tried to reserve a rescan. Now it reserves the rescan first, then waits to catch up. The likely goal is to prevent a race where another process starts a rescan in between those two steps, which could cause the import to proceed without the expected rescan or with conflicting rescan state. The practical security impact is limited and situational.
Treat as a minor hardening fix. Reviewers should confirm that holding the rescan reservation during `BlockUntilSyncedToCurrentChain()` does not block legitimate rescans or chain sync unnecessarily, and that no other RPCs have the same ordering issue. No urgent action is required for operators.
Security signals we found
TOCTOU/race-condition hardening in wallet rescan reservation
Reordering of synchronization and lock acquisition
Wallet RPC importdescriptors behavior change
Evidence from the diff
In src/wallet/rpc/backup.cpp, within importdescriptors, the call to wallet.BlockUntilSyncedToCurrentChain() was moved to after WalletRescanReserver::reserve(). The reserver holds the wallet’s rescan lock while BlockUntilSyncedToCurrentChain() may wait for the chain to advance. Reversing the order prevents a window where the wallet could be synced to the tip but not yet holding the rescan reservation, during which another RPC or background operation could acquire the rescan lock. This is a hardening fix against a TOCTOU-style race in wallet rescan coordination.
Changed components
src/wallet/rpc/backup.cppimportdescriptors RPCWalletRescanReserverCWallet::BlockUntilSyncedToCurrentChainInspect captured patch +4 / −4
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 396be628..44b49bea 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -380,15 +380,15 @@ RPCMethod importdescriptors()
if (!pwallet) return UniValue::VNULL;
CWallet& wallet{*pwallet};
- // Make sure the results are valid at least up to the most recent block
- // the user could have gotten from another RPC command prior to now
- wallet.BlockUntilSyncedToCurrentChain();
-
WalletRescanReserver reserver(*pwallet);
if (!reserver.reserve(/*with_passphrase=*/true)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait.");
}
+ // Make sure the results are valid at least up to the most recent block
+ // the user could have gotten from another RPC command prior to now
+ wallet.BlockUntilSyncedToCurrentChain();
+
// Ensure that the wallet is not locked for the remainder of this RPC, as
// the passphrase is used to top up the keypool.
LOCK(pwallet->m_relock_mutex);
Why this scored 36/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.