fix(platform-wallet): reconcile platform-address balances after top-up-from-addresses

[shumkov]

Issue being fixed or feature implemented

A user reported (2026-06-27, screenshots) that an identity top-up failed with:

Failed to top up identity from addresses: Protocol error: Insufficient combined address balances: total available is less than required 1500000

…even though the wallet displayed Platform Balance 1.19970434 DASH and the top-up sheet showed "Available: 1.19970434 DASH." As a downstream effect, DPNS registration also failed (Insufficient identity … balance 35781660 required 42539440) because the identity could never be topped up — leaving the user stuck in a loop.

With creditsPerDash = 100_000_000_000, the displayed ~120 billion credits vs. the protocol's real < 1,500,000 credits is an ~80,000× gap between the wallet's local platform-address balance and on-chain reality — a phantom balance.

What was done?

Root cause: IdentityWallet::top_up_from_addresses discarded the proof-attested AddressInfos the SDK returns — the new on-chain balance + bumped nonce of every platform address the top-up spent from. So the local platform-address balances stayed frozen at their pre-top-up values: the wallet kept showing the stale "Platform Balance", and the next top-up's input selection (driven by those stale balances) over-selected the now-drained addresses, which Drive rejected.

The fix:

• IdentityWallet::top_up_from_addresses now returns the proof-attested (AddressInfos, Credits) instead of discarding the infos.
• The caller routes those infos into PlatformAddressWallet::apply_top_up_reconciliation, which resolves each spent address through the address provider's persisted index ↔ address bijection (per_wallet_state). This covers addresses restored from disk that are no longer in the live derived pool — the exact cold/restored-wallet case the reported bug is about. It then applies the proof-attested post-spend balance in memory and persists a PlatformAddressChangeSet, so the displayed balance and the next input selection both reflect on-chain reality.
• Lock discipline: entries are resolved under the provider read lock (dropped), applied in memory under the wallet write lock (dropped), and persisted with no locks held — matching the transfer path.
• The shared entry-resolution is extracted into the unit-testable build_top_up_balance_entries.

This mirrors the sibling fund_from_asset_lock path, which already reconciles platform-address balances after spending (its comment documents this exact failure mode). Rust-only change in rs-platform-wallet + the top-up FFI.

How Has This Been Tested?

• build_top_up_entries_resolves_restored_address_outside_live_pool — unit test pinning that a spent address present only in the persisted bijection (restored, not in the live pool) is resolved to the correct derivation index with the proof's post-spend balance/nonce. This is the load-bearing logic and the case the original bug hinges on.
• apply_top_up_reconciliation_persists_decremented_balance — recording-persister integration test pinning that the reconciliation actually persists a PlatformAddressChangeSet carrying the decremented entry (not only that the pure builder maps it). Verified red→green: with the persist call removed (the original discard bug) it fails; restored, it passes.
• Full rs-platform-wallet test suite passes (204); cargo fmt clean; CI Tests green.

Not yet covered (network-gated): the FFI boundary actually routing the returned AddressInfos into apply_top_up_reconciliation, and Drive accepting a follow-up top-up, require a live testnet e2e (top-up → second top-up succeeds). Best confirmed by the affected user re-testing on their already-funded device.

Breaking Changes

None. IdentityWallet::top_up_from_addresses now returns (AddressInfos, Credits) instead of Credits; all in-tree callers are updated.

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

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Top-ups now return detailed address information alongside the updated balance, enabling proper reconciliation after spending.
  • Platform wallet balances are now automatically reconciled after a top-up, including addresses restored from storage.

• Bug Fixes

  • Fixed cases where spent platform addresses could keep showing stale balances or next-address selection.
  • Improved persistence so balance updates are reflected consistently after reconciliation.

[thepastaclaw]

✅ Review complete (commit c0d5ac7)

[coderabbitai]

📝 Walkthrough

Walkthrough

top_up_from_addresses now returns AddressInfos with Credits. PlatformAddressWallet adds apply_top_up_reconciliation to update in-memory balances and persist address changes from that proof data. The provider adds reconciliation helpers, and the FFI calls reconciliation after top-up.

Changes

Post-top-up address balance reconciliation

Layer / File(s)	Summary
top_up_from_addresses return type and docs packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs	Imports AddressInfos, updates method docs to document caller reconciliation responsibility, changes return type to Result<(AddressInfos, Credits)>, binds address_infos from SDK result, and returns the tuple.
Provider helpers: per_wallet_state and build_top_up_balance_entries packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs	Adds per_wallet_state crate-visible accessor, build_top_up_balance_entries which resolves spent P2PKH addresses to derivation indices via persisted bijections, and a regression test for restored-from-disk address resolution.
apply_top_up_reconciliation on PlatformAddressWallet packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs	Adds apply_top_up_reconciliation which reads provider state, updates in-memory managed-account balances under a write lock, and persists a PlatformAddressChangeSet while logging errors.
FFI wiring: reconcile after top-up packages/rs-platform-wallet-ffi/src/identity_top_up.rs	Clones platform_wallet, destructures (address_infos, new_balance) from top_up_from_addresses, and calls platform_wallet.apply_top_up_reconciliation(&address_infos).await.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

ready for final review

Suggested reviewers

• QuantumExplorer
• thepastaclaw

Poem

🐇 I hopped through balances, proof in my paw,
The spent addresses now follow the law.
Reconciled gently, with ledger tucked tight,
Hop-hop, all accounted, from morning to night.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main fix: reconciling platform-address balances after top-up-from-addresses.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/platform-address-topup-balance-staleness

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs (1)

205-255: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Cover the actual top_up_from_addresses success path.

Line 232 tests the shared helper directly, so this regression would still pass if top_up_from_addresses stopped reconciling or persisting address_infos at all. Given the reported bug was in that wiring, I'd add one method-level test with a fake persister/wallet manager to pin the end-to-end behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs`
around lines 205 - 255, The current test only covers the shared persistence
helper, so it would still pass even if `top_up_from_addresses` stopped
reconciling or saving `address_infos` correctly. Add a method-level test for
`top_up_from_addresses` itself using a fake wallet manager/persister to exercise
the success path end-to-end, and assert that the post-top-up reconciliation
persists the proof-backed balances and nonce for spent addresses rather than
relying only on `build_platform_address_persistence_entries`.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs`:
- Around line 205-255: The current test only covers the shared persistence
helper, so it would still pass even if `top_up_from_addresses` stopped
reconciling or saving `address_infos` correctly. Add a method-level test for
`top_up_from_addresses` itself using a fake wallet manager/persister to exercise
the success path end-to-end, and assert that the post-top-up reconciliation
persists the proof-backed balances and nonce for spent addresses rather than
relying only on `build_platform_address_persistence_entries`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1291921a-6ce0-4e2f-a4fc-d96fa68d77fc

📥 Commits

Reviewing files that changed from the base of the PR and between 1157ca7 and 6d867c6.

📒 Files selected for processing (3)

• packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs
• packages/rs-platform-wallet/src/wallet/platform_addresses/mod.rs
• packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.18%. Comparing base (48f0cc3) to head (c0d5ac7).
⚠️ Report is 2 commits behind head on v3.1-dev.

Additional details and impacted files

@@            Coverage Diff             @@
##           v3.1-dev    #3969    +/-   ##
==========================================
  Coverage     87.17%   87.18%            
==========================================
  Files          2629     2632     +3     
  Lines        327221   327563   +342     
==========================================
+ Hits         285265   285592   +327     
- Misses        41956    41971    +15     

Components	Coverage Δ
dpp	87.70% <ø> (ø)
drive	86.14% <ø> (ø)
drive-abci	89.45% <ø> (ø)
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

The PR correctly starts using the SDK-returned proof-backed address info after identity top-up, but the new reconciliation still misses a restored-wallet state that this fix needs to cover. The main correctness issue is that ownership and derivation indexes are resolved only from the live derived address pool, while persisted platform-address balances can be restored separately.

🔴 1 blocking | 🟡 2 suggestion(s)

🤖 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/identity/network/top_up_from_addresses.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs:124-137: Reconciliation ignores restored balances outside the live address pool
  The ownership lookup only scans `account.addresses.addresses`, so it only reconciles spent addresses that are currently present in the live derived-address pool. Restored platform-address state is broader than that: `initialize_from_persisted` rehydrates `account.address_balances` from persisted `found()` entries, and `PlatformPaymentAddressProvider::from_persisted` keeps the persisted `(address_index, address)` map even for addresses that are not currently in `managed.addresses.addresses`. If a top-up spends one of those restored cached rows, the SDK returns the proof-backed post-spend balance, but this loop leaves it out of `owned`; the in-memory balance and persisted row remain stale, which preserves the phantom balance this PR is intended to clear. Resolve spent wallet inputs from the persisted/indexed platform-address state as well as the live pool, or otherwise carry the stored derivation index for `address_balances` entries into this reconciliation.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs:163-164: Persist address reconciliation after releasing the wallet write lock
  This persistence call runs while the `wallet_manager` write guard acquired earlier in the block is still live. The FFI persistence backend executes its callback sequence inline, and the transfer path already avoids this pattern by building the changeset under the lock, dropping the guard, and then calling `store`. Keeping the write lock during persistence can unnecessarily block other wallet operations and creates avoidable re-entry or lock-order risk for persistence implementations. Build the identity/address changesets while the guard is held, drop the guard, then call the persister.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/top_up_from_addresses.rs:232-237: Regression test does not cover the top-up reconciliation path
  The regression was in `IdentityWallet::top_up_from_addresses` dropping or failing to persist the SDK-returned `address_infos`, but this test calls `build_platform_address_persistence_entries` directly. That proves the helper maps a provided owned address correctly, but it would still pass if the wallet method stopped invoking the helper, stopped storing the changeset, or failed the restored-address ownership lookup above. Add coverage around the wallet reconciliation path itself, or extract the reconciliation block into a unit-testable function that takes account state and returns the applied changeset.

[thepastaclaw]

Code Review

Prior reconciliation against 1bd68c3: #1 is FIXED by resolving spent addresses through the provider's persisted index/address bijection, #2 is FIXED because address reconciliation persistence now runs after provider and wallet locks are dropped, and #3 is STILL VALID and carried forward. I found no new latest-delta findings beyond the carried-forward test coverage gap. CodeRabbit provided no actionable inline findings, so there are no reactions.

🟡 1 suggestion(s)

🤖 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-ffi/src/identity_top_up.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/identity_top_up.rs:124-134: Top-up reconciliation integration remains untested
  The current regression coverage exercises `build_top_up_balance_entries`, but it still does not pin the production contract that fixes the reported stale-balance bug: `IdentityWallet::top_up_from_addresses` returns proof-backed `AddressInfos`, the caller routes those infos into `PlatformAddressWallet::apply_top_up_reconciliation`, and that path applies and persists the resulting `PlatformAddressChangeSet`. A future change could remove or skip lines 132-134, or regress `apply_top_up_reconciliation` so it no longer persists entries, while the helper-level test continues to pass. Add focused coverage for the FFI/top-up boundary or for `apply_top_up_reconciliation` with a recording persister so the actual reconciliation contract is tested, not only the pure entry builder.

[coderabbitai]

🧹 Nitpick comments (1)

packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs (1)

1323-1335: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Assert all persisted entries for the spent address match the proof.

Using .find(...) can miss a later conflicting/stale write for the same address. Collect matching entries and assert each one carries the proof-attested balance/nonce.

Strengthen the regression assertion

-        let entry = stored
+        let entries: Vec<_> = stored
             .iter()
             .filter_map(|cs| cs.platform_addresses.as_ref())
             .flat_map(|pa| pa.addresses.iter())
-            .find(|e| e.address == addr)
-            .expect("a persisted platform-address entry for the spent address");
-        assert_eq!(entry.account_index, ACCOUNT);
-        assert_eq!(
-            entry.funds.balance, 5,
-            "persists the proof's post-spend balance, not the stale pre-spend value"
-        );
-        assert_eq!(entry.funds.nonce, 4, "persists the bumped nonce");
+            .filter(|e| e.address == addr)
+            .collect();
+        assert!(
+            !entries.is_empty(),
+            "a persisted platform-address entry for the spent address"
+        );
+        for entry in entries {
+            assert_eq!(entry.account_index, ACCOUNT);
+            assert_eq!(
+                entry.funds.balance, 5,
+                "persists the proof's post-spend balance, not the stale pre-spend value"
+            );
+            assert_eq!(entry.funds.nonce, 4, "persists the bumped nonce");
+        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs` around
lines 1323 - 1335, The current assertion in the platform address persistence
test only checks the first matching entry found via the `stored`/`entry` lookup,
so it can miss later stale or conflicting writes for the same spent address.
Update this test to collect all persisted `platform_addresses.addresses` entries
matching `addr` from `recorder.stored` in `provider.rs`, then assert every
matching entry has the expected `account_index`, `funds.balance`, and
`funds.nonce` from the proof using the existing `ACCOUNT` and `entry.funds`
expectations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs`:
- Around line 1323-1335: The current assertion in the platform address
persistence test only checks the first matching entry found via the
`stored`/`entry` lookup, so it can miss later stale or conflicting writes for
the same spent address. Update this test to collect all persisted
`platform_addresses.addresses` entries matching `addr` from `recorder.stored` in
`provider.rs`, then assert every matching entry has the expected
`account_index`, `funds.balance`, and `funds.nonce` from the proof using the
existing `ACCOUNT` and `entry.funds` expectations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f610919-5993-4799-808c-2bbb6ae52e7f

📥 Commits

Reviewing files that changed from the base of the PR and between 1bd68c3 and c0d5ac7.

📒 Files selected for processing (1)

• packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs

[thepastaclaw]

Code Review

Carried-forward prior findings: none. The prior test-coverage finding is fixed by the new apply_top_up_reconciliation_persists_decremented_balance test, which covers the recording-persister contract that the previous review explicitly accepted as an equivalent fix path. New latest-delta findings: none; the remaining FFI/live top-up coverage gap is useful follow-up work but is not an in-scope PR finding after the reconciliation/persistence contract is pinned.