test(dash-spv): add multi-wallet integration tests#697
Draft
xdustinface wants to merge 5 commits intov0.42-devfrom
Draft
test(dash-spv): add multi-wallet integration tests#697xdustinface wants to merge 5 commits intov0.42-devfrom
xdustinface wants to merge 5 commits intov0.42-devfrom
Conversation
commit 8a0f06f3806253ce554d2bdb9f73981b3accfa83
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 25 20:41:27 2026 +1000
feat(dash-spv-cli): accept birth-height on `import` so wallets actually rescan
The interactive CLI's `import mnemonic`, `import xprv`, and `import xpub` commands previously fixed each new wallet's `birth_height` to the current chain tip and printed "note: historical rescan is not implemented yet". With the per-wallet rescan refactor in place, that ceiling is no longer a limitation, but the imported wallet still ended up with `synced_height = tip - 1`, leaving only the tip block to rescan and missing all on-chain history.
Each `import` subcommand now accepts an optional trailing decimal `birth-height`. If present it parses as a `u32` and is passed straight through to wallet creation; if omitted the import defaults to `0`, i.e. a full rescan from genesis. The path / xprv / xpub payload may contain spaces; the parser treats the last whitespace-separated token as the birth-height only if it parses as `u32`, so existing callers that pass paths with spaces (e.g. `import mnemonic /tmp/wallet words.txt`) continue to work.
For `xprv` and `xpub` imports the manager-level constructors still default `birth_height` to `last_processed_height()` and `synced_height` to `0`. The CLI now also calls `update_wallet_synced_height` and `update_wallet_last_processed_height` with `birth_height.saturating_sub(1)` immediately after import so the per-wallet checkpoint matches the chosen birth height. Extending the manager-side import APIs to take an explicit `birth_height` is a follow-up.
`execute_command` no longer needs the SPV client handle and drops the unused parameter.
Adds unit tests for the new parser cases and updates the in-app help text to document the optional argument.
commit 1ca8a5c48fa4fcb6745c583f8d8bd49a05f13231
Author: xdustinface <xdustinfacex@gmail.com>
Date: Wed Apr 22 08:23:53 2026 +1000
feat: add interactive dash-spv tui
commit 7b44ce290ca1747dc11ed95263fc75d728e78c92
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 25 19:41:18 2026 +1000
Squashed commit of the following:
commit 055dfc9
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 25 09:26:13 2026 +1000
enforce `Send + Sync + 'static` on generic type of the wallet manager
commit 032d2af
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 25 00:34:05 2026 +1000
refactor(key-wallet-manager): track wallet heights per wallet
Move committed sync height into wallet metadata and expose it through `WalletInfoInterface` so `WalletManager` no longer relies on global cached heights.
Compute manager-level synced_height and tip_height as the minimum across wallet infos, propagate committed sync updates per wallet, and keep tip_height as per-wallet applied chain state.
Update tests and example messaging to match the new aggregation semantics.
Initialize a new wallet's per-wallet `synced_height` and `last_processed_height` to `birth_height.saturating_sub(1)` in `create_wallet_from_mnemonic` and `create_wallet_from_mnemonic_return_serialized_bytes`. Without this the metadata defaults zero them, which would drag the aggregate `WalletManager::synced_height()` min back to 0 whenever a wallet is added and force any consumer to rescan from genesis instead of from the wallet's actual birth height.
…eight When `wallet.synced_height()` drops below `FiltersManager`'s `progress.committed_height()`, a wallet was added or moved behind the current scan position and needs catch-up coverage. Add a check at the top of `FiltersManager::tick()` that detects this regression, clears in-flight pipeline state, lowers `committed_height` to the new aggregate min, and re-enters `start_download()`. The check runs in `Syncing`, `Synced`, and `WaitForEvents` states so idle additions are caught on the next 100ms tick. Add `test_wallet_added_at_runtime_catches_up` in `dash-spv/tests/dashd_sync/tests_basic.rs`. After initial sync with `W1`, mine a block funding `W2`'s address and add `W2` at runtime with `birth_height` before that block. Assert the rescan picks up `W2`'s funding transaction and `W1`'s state is unchanged. Then add `W3` with `birth_height` beyond the tip and assert no spurious rescan or regression in either existing wallet.
When a wallet is added behind the current scan progress, only that wallet now goes through filter matching and block processing. Already-synced wallets are completely untouched: their addresses are not tested against any filter, no matched block is processed for them, and their per-wallet `synced_height` and `last_processed_height` stay where they were. `WalletInterface` is restructured around per-wallet operations: - `process_block_for_wallets(block, height, wallets)` replaces the global `process_block` and only updates the listed wallets. - `wallets_behind(height)` returns the wallet ids that still need filter coverage at `height`. - `monitored_addresses_for(wallet_id)` and `wallet_synced_height(wallet_id)` give per-wallet projections for filter matching. - `update_wallet_synced_height(wallet_id, height)` and `update_wallet_last_processed_height(wallet_id, height)` advance one wallet at a time and are monotonic, so a rescan that lowers the aggregate min never regresses an already-synced wallet's per-wallet checkpoint. - `BlockProcessingResult.new_addresses_by_wallet` and `CheckTransactionsResult.new_addresses_by_wallet` carry gap-limit discoveries with wallet attribution so re-matching is also per-wallet. `FiltersManager.scan_batch` now reads each behind wallet's `(synced_height, addresses)`, projects the batch's filters to heights that wallet hasn't yet covered, and matches them only against that wallet's address set. The per-block result is a `BTreeMap<FilterMatchKey, BTreeSet<WalletId>>` that flows through the renamed `BlocksNeeded` payload to `BlocksManager`. `FiltersBatch` records the wallet set scanned at scan time so the commit phase advances only those wallets' `synced_height`. Block-collision in `blocks_remaining` merges wallet sets instead of dropping the late-discovered wallet (TODO: the rare race where a block has already been delivered for processing before the merge is acknowledged in code). `BlocksManager` queues blocks with their wallet sets, threads them through the pipeline, and calls `process_block_for_wallets` with that exact set rather than every wallet in the manager. `BlockProcessed` carries `new_addresses_by_wallet` so `FiltersManager` can route gap-limit re-matches per wallet via `rescan_batch`. `process_block_for_wallets` always refreshes the cached balance after applying a block's transactions, even when the block height is below a wallet's current `last_processed_height` (a rescan path), because UTXOs may have been added or removed. Add `test_runtime_wallet_rescan_does_not_regress_other_wallet_height` checks inline in `test_wallet_added_at_runtime_catches_up`, asserting that during W2's catch-up rescan W1's per-wallet `synced_height` and `last_processed_height` never decrease. Closes [#114](xdustinface#114).
Adds `tests_multi_wallet.rs` plus extensions to `tests_basic.rs` and `tests_restart.rs` covering per-wallet sync behavior: - Two wallets in one `WalletManager` sync independently. - Runtime wallet-add with `birth_height` before, at, and beyond tip, asserting existing wallets' `synced_height` and `last_processed_height` never regress. - Deep historical rescan triggered by a `birth_height = 0` runtime add. - A block funding two wallets is attributed correctly to each after both are added at runtime. - Runtime add against a `Syncing`-state `FiltersManager`. - Runtime-add rescan composed with a live tip advance. - Restart of a fully-synced client emits no rescan, then runtime-adding a second wallet only catches up the new wallet.
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #697 +/- ##
=============================================
- Coverage 70.46% 69.88% -0.59%
=============================================
Files 319 321 +2
Lines 66613 67453 +840
=============================================
+ Hits 46937 47137 +200
- Misses 19676 20316 +640
|
github-actions
Bot
added
the
merge-conflict
Contributor
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
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
1 participant
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.
Adds
tests_multi_wallet.rsplus extensions totests_basic.rsandtests_restart.rscovering per-wallet sync behavior:WalletManagersync independently.birth_heightbefore, at, and beyond tip, asserting existing wallets'synced_heightandlast_processed_heightnever regress.birth_height = 0runtime add.Syncing-stateFiltersManager.Based on:
birth_heightinManagedWalletInfoctors #692birth_height#693