Skip to content

feat(key-wallet): seed sync checkpoint from birth_height#693

Draft
xdustinface wants to merge 4 commits intorefactor/birth-height-constructionfrom
feat/feed-wallet-synced-height
Draft

feat(key-wallet): seed sync checkpoint from birth_height#693
xdustinface wants to merge 4 commits intorefactor/birth-height-constructionfrom
feat/feed-wallet-synced-height

Conversation

@xdustinface
Copy link
Copy Markdown
Collaborator

@xdustinface xdustinface commented Apr 25, 2026

WalletManager::import_wallet_from_extended_priv_key, import_wallet_from_xpub, and import_wallet_from_bytes (plus the matching FFI) gain a birth_height parameter so previously-used keys can rescan from a chosen height instead of being silently anchored at the chain tip.

Based on:

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.
`WalletInfoInterface::from_wallet` and `from_wallet_with_name` (plus the inherent `ManagedWalletInfo` constructors) now take `birth_height: CoreBlockHeight` and seed both checkpoint heights (`synced_height`, `last_processed_height`) to `birth_height.saturating_sub(1)` so the next block to scan is always `birth_height`.

Without this, every wallet-add path had to remember `set_birth_height` after construction. Forgetting it left the checkpoints at `0`, dragging `WalletManager::synced_height` (a min across wallets) back to genesis on every add. Moving the seed into construction makes the invariant a property of the type.

`ManagedWalletInfo::with_birth_height` is removed. It set only `birth_height` and had no callers.
Move sync checkpoint seeding into `ManagedWalletInfo` construction so it can't be forgotten, and let callers control the scan range when importing.

- `WalletInfoInterface::from_wallet` now takes `birth_height` and seeds `synced_height` and `last_processed_height` to `birth_height.saturating_sub(1)`. Previously every wallet-add path had to remember to call `set_birth_height` separately, and forgetting it dragged `WalletManager::synced_height` (a min across wallets) back to genesis on every add.
- `WalletManager::import_wallet_from_extended_priv_key`, `import_wallet_from_xpub`, and `import_wallet_from_bytes` (plus the matching FFI) gain a `birth_height` parameter so previously-used keys can rescan from a chosen height instead of being silently anchored at the chain tip.
- Unused `ManagedWalletInfo::with_birth_height` removed.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 25, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8358e4a1-2e28-407a-b2f2-7aa0774a38e1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/feed-wallet-synced-height

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2026

Codecov Report

❌ Patch coverage is 84.31373% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.08%. Comparing base (fe7b744) to head (50c6a0d).
⚠️ Report is 4 commits behind head on refactor/birth-height-construction.

Files with missing lines Patch % Lines
key-wallet-manager/src/lib.rs 55.55% 4 Missing ⚠️
...allet/managed_wallet_info/wallet_info_interface.rs 80.00% 2 Missing ⚠️
key-wallet-ffi/src/transaction.rs 0.00% 1 Missing ⚠️
key-wallet-ffi/src/transaction_checking.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                           @@
##           refactor/birth-height-construction     #693      +/-   ##
======================================================================
- Coverage                               70.09%   70.08%   -0.02%     
======================================================================
  Files                                     319      319              
  Lines                                   66770    66771       +1     
======================================================================
- Hits                                    46803    46797       -6     
- Misses                                  19967    19974       +7     
Flag Coverage Δ
core 75.81% <ø> (ø)
ffi 43.77% <71.42%> (-0.17%) ⬇️
rpc 20.00% <ø> (ø)
spv 86.58% <ø> (-0.08%) ⬇️
wallet 68.87% <86.36%> (+0.09%) ⬆️
Files with missing lines Coverage Δ
key-wallet-ffi/src/managed_wallet.rs 66.78% <100.00%> (-2.71%) ⬇️
key-wallet-ffi/src/wallet_manager.rs 64.65% <100.00%> (-4.14%) ⬇️
key-wallet-manager/src/accessors.rs 61.87% <ø> (ø)
key-wallet-manager/src/process_block.rs 91.70% <100.00%> (+3.82%) ⬆️
...-wallet/src/transaction_checking/wallet_checker.rs 99.28% <100.00%> (ø)
...c/wallet/managed_wallet_info/asset_lock_builder.rs 79.57% <100.00%> (ø)
key-wallet/src/wallet/managed_wallet_info/mod.rs 73.68% <100.00%> (+16.54%) ⬆️
key-wallet/src/wallet/mod.rs 88.10% <100.00%> (ø)
key-wallet-ffi/src/transaction.rs 0.00% <0.00%> (ø)
key-wallet-ffi/src/transaction_checking.rs 1.84% <0.00%> (ø)
... and 2 more

... and 17 files with indirect coverage changes

@xdustinface xdustinface force-pushed the refactor/birth-height-construction branch from 973115f to 5ee6537 Compare April 28, 2026 01:30
@github-actions github-actions Bot added the merge-conflict The PR conflicts with the target branch. label Apr 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-conflict The PR conflicts with the target branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant