feat(key-wallet): seed sync checkpoint from birth_height#693
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughWallet import APIs in key-wallet-manager (import_wallet_from_extended_priv_key, import_wallet_from_xpub, import_wallet_from_bytes) now require an explicit birth_height parameter used to seed ManagedWalletInfo. The corresponding FFI function and documentation are updated to add this parameter, and existing test call sites are updated accordingly. ChangesExplicit birth_height for wallet import
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Test as test_serialized_wallets
participant Manager as WalletManager
participant Info as ManagedWalletInfo
Test->>Manager: import_wallet_from_bytes(bytes, 50_000)
Manager->>Manager: deserialize wallet, compute wallet_id
Manager->>Info: init with birth_height=50_000
Info-->>Manager: synced_height=49_999, last_processed_height=49_999
Manager-->>Test: WalletId
Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 @@
## dev #693 +/- ##
==========================================
+ Coverage 73.34% 73.66% +0.31%
==========================================
Files 324 324
Lines 72923 72927 +4
==========================================
+ Hits 53486 53722 +236
+ Misses 19437 19205 -232
|
973115f to
5ee6537
Compare
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
5ee6537 to
0a49ef4
Compare
50c6a0d to
92aa0ba
Compare
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
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.
92aa0ba to
848fdc8
Compare
ZocoLini
left a comment
There was a problem hiding this comment.
If we are going this way we also need method to expose the latest known and valid height before starting sync (ergo latest checkpoint)
How is it related or neccessary for this PR? This is just a preparation PR. I have a few coming. |
WalletManager::import_wallet_from_extended_priv_key,import_wallet_from_xpub, andimport_wallet_from_bytes(plus the matching FFI) gain abirth_heightparameter so previously-used keys can rescan from a chosen height instead of being silently anchored at the chain tip.Based on:
birth_heightinManagedWalletInfoctors #692Summary by CodeRabbit
New Features
Bug Fixes