refactor(platform-wallet): consolidate address-balance reconciliation into one guarded seam

[QuantumExplorer]

Issue being fixed or feature implemented

Follow-up to the review of #3969. rs-platform-wallet carried four hand-rolled copies of the platform-address balance reconciliation (translate proof-attested AddressInfos → apply to ManagedPlatformAccount → persist a PlatformAddressChangeSet), each with its own gaps:

• transfer.rs and withdrawal.rs resolved derivation indexes only through the live derived pool — and withdrawal's unwrap_or(0) fallback could corrupt index 0 for restored addresses no longer in a live pool.
• register_from_addresses.rs discarded _address_infos entirely (its own TODO asked for reconciliation), and transfer_to_addresses.rs left wallet-owned recipient balances stale until the next BLAST sync.
• The top-up path required the FFI to make two calls with a "caller MUST reconcile" doc-contract.
• None of the copies coordinated with the 15s background sync, so a sync pass racing a state transition could overwrite fresh proof-attested balances with stale scanned ones — and the provider's committed found map (the sync-diff baseline and SDK sync seed) diverged from what reconciliation applied.

What was done?

• One shared seam: PlatformAddressWallet::reconcile_address_infos now owns translate → freshness-guard → apply → persist. transfer, withdraw, fund_from_asset_lock, and all identity address flows route through it. build_top_up_balance_entries / build_transfer_persistence_entries are generalized into build_address_balance_entries over an index-resolver: resolution goes through the provider's persisted index↔address bijection first (restored-wallet coverage), falling back to the live pools for addresses derived since the last sync (fresh change addresses); pool-resolved pairs are merged into the bijection. The withdrawal index-0 corruption is gone.
• Previously discarded proofs wired in: IdentityWallet::register_from_addresses and transfer_credits_to_addresses_with_external_signer now return their proof-attested AddressInfos; external recipients are skipped by the seam.
• Composite orchestration on PlatformWallet (top_up_from_addresses, register_from_addresses, transfer_credits_to_addresses_with_external_signer): the struct that owns both .identity() and .platform() composes the identity op with the reconciliation, so the FFI makes one call and the "caller MUST reconcile" contract disappears. FFI signatures are unchanged (internals only — no header regeneration needed).
• Freshness guard against the background sync: the seam commits through PlatformPaymentAddressProvider::commit_reconciliation while holding the provider write lock across both the provider commit and the account-balance write (lock order provider → wallet-manager, matching the sync callbacks). Entries whose nonce is below the committed found seed's are dropped (a sync or later transition already committed fresher state); identical entries are dropped as no-ops; proof-attested removals (zero funds) drop the address from the seed, mirroring absent handling. The committed found map is updated so the sync-diff baseline and the SDK sync seed agree with what was applied. sync_balances now persists its diff before releasing the provider lock, so a fresher reconciliation row can never be overwritten on disk by an older sync diff.

Behavioral notes: withdrawal now persists its changeset internally (matching transfer — the FFI still also returns it; host-side application is an idempotent upsert), and transfer reconciles owned addresses across all the wallet's platform payment accounts rather than only the input account.

How Has This Been Tested?

• cargo test -p platform-wallet --lib — 209 passed, including new tests: freshness-guard stale-nonce drop, unchanged no-op drop, fresh-entry commit to found, zero-funds removal from the seed, live-pool fallback extending the bijection, and the retargeted persist-contract integration test now also asserting the committed seed carries the reconciled funds.
• cargo check -p platform-wallet-ffi, cargo check -p platform-wallet --all-targets and --all-features --tests, plus reverse deps platform-wallet-storage --all-targets and rs-unified-sdk-ffi — all clean.
• cargo clippy -p platform-wallet --lib --tests and -p platform-wallet-ffi — no warnings; cargo fmt --all applied.

Breaking Changes

None on consensus or FFI surfaces. Within the platform-wallet crate (pre-release, internal): IdentityWallet::register_from_addresses / transfer_credits_to_addresses_with_external_signer now also return AddressInfos, and PlatformAddressWallet::apply_top_up_reconciliation is replaced by reconcile_address_infos.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 62617ea2-dc38-4bda-bf76-8198f6f6cdd1

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 claude/interesting-kapitsa-970c85

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

✅ Review complete (commit 1d48d6f)

[thepastaclaw]

Code Review

PR consolidates four hand-rolled address-balance reconciliation copies into a single guarded seam. The core design is sound and well-tested. Three concrete suggestions surface: the reconcile seam persists outside the provider lock while the sibling sync path was just changed to persist inside it (asymmetric with the invariant this PR documents), commit_reconciliation unconditionally returns entries the outcome doc says are committed even when the target account isn't tracked in per_wallet, and reset_sync_state inverts the crate's provider→wallet_manager lock order.

Source: reviewers: opus and claude-sonnet-5 for general, security-auditor, rust-quality, and ffi-engineer; gpt-5.5[high] Codex lanes failed at ACP initialize and are recorded as failed; verifier: opus.

🟡 2 suggestion(s) | 💬 1 nitpick(s)

Findings not posted inline (1)

These findings could not be anchored to the current diff, but they are still part of this review.

• [NITPICK] packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs:433-446: reset_sync_state inverts the crate's documented provider→wallet_manager lock order — reset_sync_state takes wallet_manager.write() first (lines 434-441), releases it, then takes provider.write() (lines 442-445). Every other path in this module — sync_balances, the AddressProvider callbacks, and reconcile_address_infos — takes provider.write() first and holds it acro...

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs:325-342: reconcile_address_infos persists after releasing the provider lock, breaking the invariant sync.rs was just changed to establish
  `reconcile_address_infos` drops the provider write guard at line 325 and only then calls `self.persister.store(cs.clone().into())` at line 333. The companion change in `sync.rs` (lines 155-168) moves its `persister.store` call *inside* the same provider write lock with an explicit comment documenting why: `reconcile_address_infos` "serializes on this lock and persists its proof-attested entries after acquiring it, so keeping this store inside the critical section guarantees any reconciliation that observed this pass's committed state also lands on disk after it — releasing first would let a fresher reconciliation row be overwritten by this (older) diff."

  The reconcile side of that invariant is not upheld. Because `WalletPersister::store` is a synchronous call into an `Arc<dyn PlatformWalletPersistence>` reachable from any Tokio worker, once `drop(guard)` runs a background sync (or a second concurrent reconciliation) can fully acquire the provider lock, commit a fresher `found` seed, and persist its own diff *before* this reconciliation's delayed `store()` finally fires — at which point the older reconciliation row overwrites the fresher sync row on disk. In-memory state stays correct because both writers apply the newer value under the lock, and the next successful sync heals the disk row; but the durable state is silently stale during that window, which is exactly the property this PR advertises as fixed.

  Moving the `self.persister.store(...)` call to just before `drop(guard)` (mirroring `sync.rs`) closes the race with no extra locking cost.

In `packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs:956-1006: commit_reconciliation pushes entries into the outcome even when the account isn't tracked in per_wallet
  `outcome.entries.push(entry)` at line 1005 runs unconditionally, outside the `if let Some(state) = account_state { ... }` block (lines 979-1004). `account_state` is `wallet_state.get_mut(&entry.account_index)`; it is guaranteed `Some` only when the resolver picked `entry.account_index` from iterating `wallet_state` itself, not when it picked from the live-pool fallback (`pool_indexes.get(p2pkh).copied()` at line 950).

  `pool_indexes` is built in `reconcile_address_infos` by scanning `info.core_wallet.all_platform_payment_managed_accounts()` — every key-class-0 platform-payment account on the live wallet — not filtered to accounts already tracked in the provider's `per_wallet` state. If a platform-payment account exists in the live wallet but is not yet tracked by the provider (e.g. added after the provider snapshot was built), an address in that account resolves via the pool fallback to an `account_index` for which `wallet_state.get_mut(...)` returns `None`. In that case: `state.found.insert/remove` never runs, the bijection merge never runs, but the entry is still pushed into `outcome.entries`, so the caller applies it to the live `ManagedPlatformAccount` and persists a `PlatformAddressChangeSet` for it — with none of the `tracing::warn!`/`tracing::debug!` diagnostics the rest of the function uses for skipped entries.

  This contradicts `ReconciliationOutcome::entries`' own doc comment ("Already committed to the provider's `found` map / bijection") and undermines the PR's stated goal that "the committed found map is updated so the sync-diff baseline and the SDK sync seed agree with what was applied." Either move the `outcome.entries.push(entry)` inside the `if let Some(state) = account_state` block (dropping the entry with a log if the account isn't tracked) or update the resolver to skip pool-only addresses whose account isn't in `wallet_state`.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.19%. Comparing base (ee21592) to head (1d48d6f).
⚠️ Report is 1 commits behind head on v4.0-dev.

Additional details and impacted files

@@            Coverage Diff            @@
##           v4.0-dev    #3987   +/-   ##
=========================================
  Coverage     87.19%   87.19%           
=========================================
  Files          2634     2634           
  Lines        327718   327718           
=========================================
+ Hits         285747   285748    +1     
+ Misses        41971    41970    -1     

Components	Coverage Δ
dpp	87.70% <ø> (ø)
drive	86.14% <ø> (ø)
drive-abci	89.45% <ø> (+<0.01%)	⬆️
sdk	∅ <ø> (∅)
dapi-client	∅ <ø> (∅)
platform-version	∅ <ø> (∅)
platform-value	92.20% <ø> (ø)
platform-wallet	∅ <ø> (∅)
drive-proof-verifier	49.55% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[thepastaclaw]

Code Review

Delta cleanly resolves both prior suggestion-level findings: reconcile_address_infos now persists before releasing the provider write lock (mirroring sync.rs, restoring the total-order invariant), and commit_reconciliation's pool fallback is restricted to accounts the provider tracks, with a defensive continue+warn inside the commit loop and a new regression test. Only the nitpick-level reset_sync_state lock-inversion is carried forward (prior-3, code unchanged in this delta). No new defects introduced.

Source: reviewers: opus (general), claude-sonnet-5 (general), gpt-5.5[high] (general, failed), opus (security-auditor), claude-sonnet-5 (security-auditor), gpt-5.5[high] (security-auditor, failed), opus (rust-quality), claude-sonnet-5 (rust-quality, failed), gpt-5.5[high] (rust-quality, failed), opus (ffi-engineer), claude-sonnet-5 (ffi-engineer), gpt-5.5[high] (ffi-engineer, failed); verifier: opus; specialists: security-auditor, rust-quality, ffi-engineer

💬 1 nitpick(s)

Findings not posted inline (1)

These findings could not be anchored to the current diff, but they are still part of this review.

• [NITPICK] packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs:440-453: reset_sync_state inverts the crate's provider→wallet_manager lock acquisition order — Carried forward from the prior review (prior-3, unchanged in this delta). reset_sync_state acquires wallet_manager.write() first (lines 441-448), releases it, then acquires provider.write() (lines 449-452). Every other seam in this module — sync_balances, the AddressProvider callbacks,...