fix(spv): atomically extend BIP44 receive pool on wallet import#830
Draft
lklimek wants to merge 1 commit into
Draft
fix(spv): atomically extend BIP44 receive pool on wallet import#830lklimek wants to merge 1 commit into
lklimek wants to merge 1 commit into
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3a2b32c to
a3f6814
Compare
7 tasks
On restart, WalletManager is initialised with the default gap limit (30 addresses), but the database may contain indices 30+ from prior sessions. SpvManager::load_wallet_from_seed now calls extend_bip44_receive_pool_locked while still holding the WalletManager write lock, so the monitored address set is fully populated before any observer (notably run_spv_loop's wait-for-wallets loop) sees wallet_count increment. The extension target is max_db_bip44_receive_index + LOOKAHEAD_SLACK (slack=100). A fresh import therefore covers ~100 addresses before the first scan, discovering historical UTXOs without a restart. A restart with DB-known indices at, say, 32-40 also works in a single scan. Database::max_bip44_receive_index queries the high-water BIP44 receive index (m/44'/.../0/N) directly from wallet_addresses; change-chain rows (m/.../1/*) are excluded by the LIKE pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
a3f6814 to
16a685c
Compare
This file contains hidden or 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
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.
Summary
Fixes the second of the four layers behind the zero-balance-on-import bug tracked in #829: addresses generated via the UI beyond the WalletManager's default BIP44 gap limit (30) are not re-registered into the live monitored set on startup, so SPV filter matching misses any UTXO at those higher indices.
This PR is now narrowly focused on the bug-2 fix only. The platform pin bump and
clear_data_dirfix have been extracted into #834 and must land first.Reproduction
Expected: The balance at the receiving address appears, because the WalletManager's monitored address set should include every index that exists in DET's local database.
Actual: The address shows zero balance. On restart, the WalletManager is reconstructed with
WalletAccountCreationOptions::Defaultwhich pre-generates BIP44 indices 0–29 only. Any indices DET previously stored inwallet_addressesbeyond 29 are never re-registered into the WalletManager, so the live monitored address set is narrower than DET's on-disk state. SPV filter matching misses any UTXO at indices ≥ 30.Observed with a reference testnet wallet that has a multi-output transaction at BIP44 indices 0 and 32–40 (motivating tx:
56bdab0ac5dfefc47e136eedeede07a83ff0f2cb753683578fd41677975f8b32in block 1,459,352). Only index 0 shows balance; 32–40 do not.Root cause
Upstream's BIP44 gap-limit reactive extension works as designed: if at least one address within the current gap is observed, the pool extends. But DET stores more addresses in its local DB than it restores to the live monitored set on startup, and there's no code path that reconciles the two.
There's also a second, subtler race: earlier drafts extended the pool after
load_wallet_from_seedreturned, meaningwm.wallet_count()could cross the threshold thatrun_spv_loopwaits on while the pool was still at its default size — SPV's filter pipeline then started matching against an incomplete set.Fix
extend_bip44_receive_pool_lockedruns inside the WalletManager write lock held byload_wallet_from_seed.wm.wallet_count()does not increment until the pool is final, closing the race. The extension target ismax_db_idx(the highest BIP44 receive index present in thewallet_addressesDB table), plus a small slack (LOOKAHEAD_SLACK = 100).Supporting helpers
Database::max_bip44_receive_index()— reads the DB'swallet_addressestable for the highest receive index known for a given wallet.SpvManager::extend_bip44_receive_pool_locked— called under the existing write lock; iteratesget_receive_address(wallet_id, i, BIP44, mark_used=true)up tomax_db_idx + LOOKAHEAD_SLACK. Uses upstream's existing gap-limit reactive extension without adding new API surface.Files
src/context/wallet_lifecycle.rsload_wallet_from_seed's write locksrc/database/wallet.rsDatabase::max_bip44_receive_indexhelper + 3 unit testssrc/spv/manager.rsSpvManager::extend_bip44_receive_pool_lockedTotal: +253 / -2 lines. Single commit.
Test plan
Automated (all green)
cargo build --all-features— cleancargo clippy --all-features --all-targets -- -D warnings— cleancargo +nightly fmt --all -- --check— cleancargo test --all-features --workspace --lib— 455 / 455 passManual on testnet
Extended SPV BIP44 receive pool wallet=… generated=<N> target_index=<max_db_idx>.rm -rf ~/.config/dash-evo-tool/data.db ~/.config/dash-evo-tool/spv/→ launch → fresh wallet with no historical UTXOs. Expected: no crash, standard checkpoint sync, balance = 0.What this PR does not carry
clear_data_dirfix — moved to #834 for focused review. That PR must land first (this branch is otherwise cleanly independent but relies on the bumped platform for its build).archive/spv-wallet-address-sync-2026-04-20for future reference. Until upstream fix(fees): consolidate Core and Platform fee estimation #651 lands, the sanctioned workaround for mid-session imports is "Clear SPV Data".Merge order
clear_data_dirfix)Related
clear_data_dirfix.archive/spv-wallet-address-sync-2026-04-20— preserved pre-strip state.🤖 Co-authored by Claudius the Magnificent AI Agent