fix(swift-sdk): detect later added wallet tx to a synced spv client#4062
Conversation
|
✅ Review complete (commit 15dd0eb) |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesLate wallet balance backfill fix
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant IntegrationTest
participant SPVClient
participant WalletManager
participant Blockchain
IntegrationTest->>Blockchain: Fund and confirm receive address
IntegrationTest->>SPVClient: Start and sync to chain tip
IntegrationTest->>WalletManager: Import wallet with birthHeight 0
SPVClient->>WalletManager: Backfill wallet history and balance
WalletManager-->>IntegrationTest: Expose updated spendable balance
Suggested reviewers: 🚥 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 |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Verified PR #4062 at 6834301 against the changed Rust wallet lifecycle code, the Swift integration test, and the FFI balance read path. The reseed copies the same balance fields used by the existing restored-wallet path into the same atomic balance object Swift reads, and the test covers the intended late-import SPV backfill scenario. No in-scope review findings were confirmed.
Source: reviewers claude-general (opus), codex-general (gpt-5.5), claude-ffi-engineer (opus), codex-ffi-engineer (gpt-5.5); verifier codex (gpt-5.5).
6834301 to
15dd0eb
Compare
A wallet added while SPV is already synced (e.g. importing an
existing mnemonic with
birth_height = 0) has its historicalfunds backfilled by the SPV rescan that
insert_walletabovetriggers. That rescan can complete — emitting the
BlockProcessedevent that carries the post-backfill balance —before this wallet lands in
self.wallets, soBalanceUpdateHandlerdrops those events (the wallet isn't inthe map yet) and the atomic stays at zero even though the inner
ManagedWalletInfobalance is correct. Mirror the inner balanceinto the atomic here (as
manager::loaddoes for restoredwallets); any later block events are applied normally now that
the wallet is mapped.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Bug Fixes
Tests