fix(backend-e2e): stabilize PR #860 platform-wallet e2e suite + SEC-001 hardening

[lklimek]

Why this PR exists

• Problem: PR feat: rewrite Dash Evo Tool onto the new platform-wallet #860 (the platform-wallet backend rewrite) shipped a large new wallet backend, but its backend-e2e suite was red against live testnet — a mix of real regressions, stale test expectations, and test-infra gaps that made it impossible to tell genuine bugs from noise. We could not gate the rewrite on a suite we couldn't trust.
• What breaks without it: feat: rewrite Dash Evo Tool onto the new platform-wallet #860 ships with a real funds bug — Core sends were gated on the confirmed balance while the coin selector and the "Max" button use spendable, so users get "insufficient confirmed balance" rejections on funds they can actually spend; plus wrong error semantics (unknown wallet → WalletNotLoaded instead of WalletNotFound), a top-up fee that misreports after a backend reload, and an e2e suite too flaky/red to validate the migration.
• Blocking relationship: Stacked atop PR feat: rewrite Dash Evo Tool onto the new platform-wallet #860 (docs/platform-wallet-migration-design). Stabilizes its e2e suite and folds in the SEC-001 at-rest identity-key encryption hardening.

What was done

Product fixes

• QA-002 — unknown seed hash now returns typed WalletNotFound (was WalletNotLoaded).
• QA-006 — meaningful top-up actual-fee after a backend reload, centralized in model/fee_estimation.rs (+ a stale-HIGH balance_before guard so it can't report the full minted amount as the "fee").
• QA-010 — gate Core sends on spendable balance (= confirmed + unconfirmed), matching the coin selector + Max button, across send_screen, dashpay send_payment, and the send dialog. Fixes false "insufficient funds" rejections on IS-locked/spendable funds.
• SEC-001 hardening (reconciled from the feat: rewrite Dash Evo Tool onto the new platform-wallet #860 branch and extended): fail-closed when opt-in identity-key protection would leave resident plaintext; defensive guard for legacy Encrypted keys; zeroize the migration rollback clone.

Test / harness fixes

• QA-003 restart-in-place reconnect contract; QA-004 poll for key visibility (vs fixed sleep); QA-005 tc_012 address-advance marked PENDING; QA-008 real curve-point fixture; QA-013 bounded registration retry; QA-015 stop_spv rustdoc + inline comment; QA-016 cold-boot funding buffer; QA-017 funded-wallet SPV-pickup budget 30s→120s.
• Dashpay backend-e2e module deferred pending upstream dashpay/platform#3841.

Quality — a parallel grumpy-review (security / Rust / consistency) returned 9 findings, all LOW; 8 were applied (fee fallback, key zeroize, error copy, stale comments/docs, test asserts).

Testing

• cargo build --all-features, cargo clippy --all-features --all-targets -- -D warnings, cargo +nightly fmt — all clean.
• Unit tests pass (incl. 18/18 fee, 12/12 identity-protect).
• Backend-e2e final run vs live testnet (3 healthy DAPI nodes): 54 passed / 12 failed. All 12 reds are environmental/upstream, not code regressions:
  • QA-009 asset-lock IS finality (tc_004, tc_018) + fixture cascade (2): upstream IS-proof not arriving within the hardcoded 300s wait (unchanged from old DET) — testnet Core-P2P/IS degradation.
  • 7× SPV wallet-registration timeout at 120s: testnet Core-P2P degradation (budget bump cut the cascade ~18→7).
  • tc_014: pre-existing environmental failure (DAPI exhaustion at the withdrawal step).
• SEC-001 reconciled commits independently security-audited: fail-closed verified, no bypass, no secret leak.

Known gaps (deferred with upstream references)

• QA-005 receive-address advancement → dashpay/rust-dashcore#818 (next_unused_and_reserve); tc_012 advance assertion PENDING.
• dashpay backend-e2e → dashpay/platform#3841 ("complete dashpay"); module gated out, re-enable on land.
• QA-009 asset-lock IS finality → upstream platform-wallet SPV IS-detection / testnet IS health; not DET-tunable (300s hardcoded upstream).
• Two LOW pre-existing at-rest-residual edges (SEC-001-B/C) tracked for follow-up.

Reviewing this PR

Because #865 was squash-merged into the base, the commit list looks long (~60 commits) — but the Files changed diff is clean: only the e2e-suite stabilization + the grumpy-review fixes, since the SEC-001 content already matches the base. Please review via Files changed, not the commit list.

Attribution

🤖 Generated with Claude Code

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

Summary by CodeRabbit

• New Features

  • Wallet send screens now show and use spendable balance, so “Max” and send validation better match what can actually be spent.
  • Identity protection now gives clearer guidance when protection can’t be completed, including a new message for identities stored in an older format.

• Bug Fixes

  • Improved identity protection safety to avoid partial changes on incorrect passwords or legacy key formats.
  • Receive address generation now checks that the wallet exists before continuing.
  • SPV reconnect now preserves the same wallet backend across restarts for smoother recovery.

[thepastaclaw]

Code Review

Latest delta (d63ad9f → b98515d) is a focused commit that resolves the prior blocking SEC-001 finding: unseal_identity_keys now calls verify_existing_protection_password as a preflight before any per-key downgrade, mirroring seal_identity_keys, and a Mixed-password regression test (unseal_mixed_password_aborts_without_partial_downgrade) proves the CWE-460 partial-downgrade window is closed for opt-out. No carried-forward prior blockers remain (only one prior finding was in flight; both general agents and both rust-quality agents agree it is FIXED). Two new in-scope suggestions hold up under verification: (1) the QA-006 top-up fee helper only falls back at the exact stale-LOW / stale-HIGH boundaries, so partial-stale offsets still produce a wrong displayed actual fee; (2) the SEC-001 plaintext guard now triggers for legacy Encrypted keys too, but reuses an error whose recovery copy (close/reopen) cannot resolve that branch, leaving the user in a retry loop. Neither is a blocker — the first is a UX-accuracy gap in a display path, the second a copy mismatch for a no-current-producer code path — so review_action is COMMENT.

🟡 2 suggestion(s)

1 additional finding(s) omitted (not in diff).

🤖 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 `src/backend_task/identity/protect_identity_keys.rs`:
- [SUGGESTION] src/backend_task/identity/protect_identity_keys.rs:174-179: Legacy `Encrypted` keys hit a guard whose recovery action cannot help them
  `reject_resident_identity_plaintext` now returns `TaskError::IdentityKeyProtectionIncomplete` for BOTH residual plaintext (`has_plaintext_for_vault`) AND legacy `Encrypted` keys (`has_encrypted_legacy_keys`). The error's user-facing copy in `src/backend_task/error.rs:280-283` is `"Some of this identity's keys are not fully protected yet. Close and reopen the application, then try protecting this identity again."` That action is correct for the residual-plaintext branch (the eager load-path vault migration retries on next start), but the doc comments on `has_encrypted_legacy_keys` describe these as decode-only with no current producer and no current vault migration path — so closing and reopening will keep hitting the same guard, leaving the user in a retry loop with no self-service exit. Per the project's error-message rules every message must include a concrete action the user can take themselves. Either split the legacy-encrypted case into its own `TaskError` variant with copy that points to a path the user can actually take (e.g. recreate the identity from the wallet, or that this identity needs a developer-assisted migration), or drop the `has_encrypted_legacy_keys` branch from this guard if the eager load-path will already keep these keys from reaching protect.

In `src/model/fee_estimation.rs`:
- [SUGGESTION] src/model/fee_estimation.rs:436-452: Top-up actual-fee helper only catches exact-boundary stale balances
  `resolve_identity_topup_actual_fee` falls back to the estimate only when `delta_fee == 0` (stale-LOW saturates the increase to the full mint) or `delta_fee == expected_credits` (stale-HIGH saturates the increase to zero). Any partial stale offset — for example a cached `balance_before` that is stale-LOW by less than the real fee, or stale-HIGH by less than the mint amount — passes the `0 < delta_fee && delta_fee < expected_credits` guard and is returned as the "actual" fee even though the subtraction is invalid unless `balance_before` is the true pre-top-up value. Concretely, for a 500 000-duff top-up that really paid a 3 000 000-credit fee but whose cached `balance_before` is stale-LOW by 1 000 000 credits, the helper reports 2 000 000 credits as the actual fee. The doc comment frames the staleness mode as "collapses the computed fee to zero", which matches the implementation but understates the gap: non-boundary stale offsets are silently trusted. Impact is bounded (display only, no funds at risk) but the helper does not fully deliver QA-006's stale-safe guarantee. Consider either refreshing `balance_before` from the live source before the call, sanity-checking the delta against the deterministic estimate's plausible range, or documenting the residual so the UI knows the value is only trustworthy when caches are coherent.

[thepastaclaw]

Clearing my stale CHANGES_REQUESTED state. The prior SEC-001 blocker is fixed at b98515d; only the nonblocking suggestions from my latest review remain.

[lklimek]

Re: the review note (review #4578554863) that IdentityKeyProtectionIncomplete's "close and reopen" instruction is a no-exit retry loop for legacy Encrypted keys — addressed in d78b9306.

Split into a dedicated TaskError::IdentityKeyProtectionLegacyFormat variant for the legacy-Encrypted branch, with an honest, code-verified recovery action: "Some of this identity's keys are saved in an older format that cannot be protected. Load this identity again using its recovery phrase or private key, then try protecting it." reject_resident_identity_plaintext now routes per branch (legacy checked first, since re-loading also clears resident plaintext); the resident-plaintext branch keeps the original "close and reopen" message, which is correct there.

Verified the action is real: Encrypted is decode-only with no migration, but re-loading goes through insert/update_local_qualified_identity, which overwrites the stored blob, and load paths never emit Encrypted keys — so the legacy entries get replaced with protect-compatible ones via the existing "Load Identity" flow. Regression test protect_rejects_legacy_encrypted_key_with_distinct_error added.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[lklimek]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

tests/backend-e2e/core_tasks.rs (1)

233-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Funding send + fixed 5s sleep is now dead weight and a flakiness source.

Since RefreshSingleKeyWalletInfo returns SingleKeyWalletsUnsupported unconditionally (independent of funding), the real send to the single-key address and the fixed sleep(5s) no longer gate the assertion that follows. They add a live-testnet send that can fail or flake on a path whose only verified outcome is the typed error — which runs counter to this PR's stabilization goal. Consider dropping the funding/sleep until the single-key send flow is reinstated, or keep them only if the intent is explicitly to smoke-test the framework HD send here.

🤖 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 `@tests/backend-e2e/core_tasks.rs` around lines 233 - 259, The funding send and
fixed 5-second sleep in the single-key wallet test are unnecessary and introduce
flakiness because the assertion only verifies the unsupported error from
RefreshSingleKeyWalletInfo. Remove the live transfer to the single-key address
and the subsequent sleep from core_tasks.rs unless this test is meant to
explicitly smoke-test the framework wallet send path; if so, keep the funding
step only with clear intent and separate the unsupported-refresh assertion from
any propagation timing.

src/backend_task/wallet/generate_receive_address.rs (1)

12-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression test for the new error split.

Lines 19-20 change retry semantics, not just wording. tests/backend-e2e/framework/harness.rs:is_transient_registration_error retries WalletNotLoaded-style startup failures but treats WalletNotFound as terminal, so this distinction should be pinned with a task-level test for “unknown seed hash → WalletNotFound” (and ideally “known wallet still loading → WalletNotLoaded”).

🤖 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 `@src/backend_task/wallet/generate_receive_address.rs` around lines 12 - 21,
Add a regression test covering the new error split in generate_receive_address:
verify that an unknown seed hash returns TaskError::WalletNotFound, and that a
known wallet still loading continues to surface WalletNotLoaded. Use the
generate_receive_address flow and the wallet store lookup in BackendTask to pin
the behavior change, and make sure the test reflects the retry semantics
expected by is_transient_registration_error in the backend-e2e harness.

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

Inline comments:
In `@src/backend_task/identity/top_up_identity.rs`:
- Around line 20-21: The identity top-up flow is creating a fresh
PlatformFeeEstimator instead of using the AppContext fee model, so fee estimates
can drift from the active network multiplier. Update the logic in
top_up_identity to call self.fee_estimator() and use that estimator for
estimate_identity_topup, and make the related resolver fallback/plausibility
band code at the referenced match area use the same estimator path so all
reported fees stay multiplier-aware.

In `@src/context/identity_db.rs`:
- Around line 324-328: After store_all succeeds in the migration flow, the taken
value still holds zeroizing plaintext while persist(qi) runs; capture
taken.len() into migrated, then explicitly drop taken before calling persist(qi)
so the plaintext is released earlier. Keep the existing zeroization cleanup with
before.take_plaintext_for_vault() after the vault write path, and use the taken
and persist(qi) symbols in identity_db.rs to update the ownership order.

In `@tests/backend-e2e/framework/wait.rs`:
- Around line 102-108: The diagnostic comment in wait_balance_timeout is stale
because the code now logs spendable and total rather than confirmed and total.
Update the nearby comment in wait_balance_timeout to match the current
snapshot_balance usage and the spendable/total values, keeping the wording
aligned with the format! message and the spendable, total binding so future
readers are not misled.

---

Nitpick comments:
In `@src/backend_task/wallet/generate_receive_address.rs`:
- Around line 12-21: Add a regression test covering the new error split in
generate_receive_address: verify that an unknown seed hash returns
TaskError::WalletNotFound, and that a known wallet still loading continues to
surface WalletNotLoaded. Use the generate_receive_address flow and the wallet
store lookup in BackendTask to pin the behavior change, and make sure the test
reflects the retry semantics expected by is_transient_registration_error in the
backend-e2e harness.

In `@tests/backend-e2e/core_tasks.rs`:
- Around line 233-259: The funding send and fixed 5-second sleep in the
single-key wallet test are unnecessary and introduce flakiness because the
assertion only verifies the unsupported error from RefreshSingleKeyWalletInfo.
Remove the live transfer to the single-key address and the subsequent sleep from
core_tasks.rs unless this test is meant to explicitly smoke-test the framework
wallet send path; if so, keep the funding step only with clear intent and
separate the unsupported-refresh assertion from any propagation timing.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 98636966-3cbc-4c36-8662-cbf9a13c1f13

📥 Commits

Reviewing files that changed from the base of the PR and between 2242bc7 and d78b930.

📒 Files selected for processing (22)

• src/backend_task/error.rs
• src/backend_task/identity/protect_identity_keys.rs
• src/backend_task/identity/top_up_identity.rs
• src/backend_task/wallet/generate_receive_address.rs
• src/context/identity_db.rs
• src/context/wallet_lifecycle.rs
• src/model/fee_estimation.rs
• src/model/qualified_identity/encrypted_key_storage.rs
• src/ui/dashpay/send_payment.rs
• src/ui/wallets/send_screen.rs
• src/ui/wallets/wallets_screen/dialogs.rs
• src/wallet_backend/mod.rs
• tests/backend-e2e/core_tasks.rs
• tests/backend-e2e/dashpay_tasks.rs
• tests/backend-e2e/framework/harness.rs
• tests/backend-e2e/framework/wait.rs
• tests/backend-e2e/identity_cold_boot.rs
• tests/backend-e2e/identity_in_vault_sign.rs
• tests/backend-e2e/main.rs
• tests/backend-e2e/spv_reconnect.rs
• tests/backend-e2e/wallet_tasks.rs
• tests/backend-e2e/z_broadcast_st_tasks.rs

[thepastaclaw]

Code Review

Latest delta (b98515d → d78b930) cleanly resolves both prior findings and introduces no in-scope defects. prior-b985-1 is FIXED: a dedicated TaskError::IdentityKeyProtectionLegacyFormat variant with a concrete self-service recovery instruction is returned for legacy Encrypted keys, guarded ahead of the resident-plaintext branch, with a new protect_rejects_legacy_encrypted_key_with_distinct_error regression test. prior-b985-2 is FIXED: resolve_identity_topup_actual_fee now adds an estimate × 2 plausibility band that catches partial-stale balance_before values inflating the measured delta, with a dedicated test_identity_topup_actual_fee_rejects_partial_stale_inflated_delta test; the residual on the stale-LOW deflation side is documented in code with a justification. No new in-scope findings: codex-rust-quality's lone suggestion to swap PlatformFeeEstimator::new() for self.fee_estimator() in top_up_identity.rs is a pre-existing project-wide pattern (30+ call sites) that this PR did not introduce or change, and the same agent already records it as an out-of-scope follow-up; it is dropped here. No CodeRabbit comments.

[lklimek]

All 5 CodeRabbit items from review #4579124870 are addressed (pushed in 829c1d30):

• top_up_identity.rs:20 — now uses self.fee_estimator() instead of PlatformFeeEstimator::new(), so the displayed estimate and the estimate × 2 fee-plausibility band track the active fee multiplier (5957c1a2). Scoped to this site only — it's user-facing and feeds the band; the project-wide ::new() pattern stays deferred.
• identity_db.rs:324 — drop(taken); before persist() to minimize the in-memory residency of the migrated plaintext key bytes (e97592e0).
• tests/backend-e2e/framework/wait.rs:102 — corrected the stale comment ("confirmed and total" → "spendable and total") to match the code (829c1d30).
• tests/backend-e2e/core_tasks.rs (TC-009) — removed the dead single-key funding-send + 5s sleep; forward-traced and verified it had zero effect on the assertion (RefreshSingleKeyWalletInfo returns SingleKeyWalletsUnsupported unconditionally) and was wasting real testnet funds (829c1d30).
• generate_receive_address.rs — added regression test asserting an unknown seed hash returns WalletNotFound (the feat: rewrite Dash Evo Tool onto the new platform-wallet #860 WalletNotFound/WalletNotLoaded split) (ad8738a3).

Gate: build + clippy --all-features --all-targets -D warnings + +nightly fmt clean; fee_estimation 19/19, identity_db 23/23, new receive-address test 1/1, backend-e2e compiles. The three inline threads (fee estimator, key residency, stale comment) are resolved.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[lklimek]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thepastaclaw]

Code Review

Incremental delta from d78b930 to 829c1d3 is small and focused, with no carried-forward prior findings from that review. Verification confirms two nonblocking in-scope items: a QA-006 sibling in top_up_identity_from_platform_addresses still uses a hardcoded PlatformFeeEstimator::new() despite the same-PR fix to its sibling, and the TC-009 header comment still describes the funding step that this PR removed.

🟡 1 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.

• [SUGGESTION] src/backend_task/identity/mod.rs:896: QA-006 sibling: top_up_identity_from_platform_addresses still ignores active fee multiplier — This PR's fix in src/backend_task/identity/top_up_identity.rs correctly routes the top-up estimate through self.fee_estimator() so the reported FeeResult.estimated tracks the cached fee_multiplier_permille. The sibling entry point top_up_identity_from_platform_addresses calls the same estimate_identity_topup() method on a hardcoded PlatformFeeEstimator::new(), so under any non-default fee multiplier the platform-addresses top-up will misreport its estimate in exactly the way QA-006 describes. Same domain, same function, same one-line fix; AppContext::fee_estimator() is already in scope here.

🤖 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 `src/backend_task/identity/mod.rs`:
- [SUGGESTION] src/backend_task/identity/mod.rs:896: QA-006 sibling: top_up_identity_from_platform_addresses still ignores active fee multiplier
  This PR's fix in `src/backend_task/identity/top_up_identity.rs` correctly routes the top-up estimate through `self.fee_estimator()` so the reported `FeeResult.estimated` tracks the cached `fee_multiplier_permille`. The sibling entry point `top_up_identity_from_platform_addresses` calls the same `estimate_identity_topup()` method on a hardcoded `PlatformFeeEstimator::new()`, so under any non-default fee multiplier the platform-addresses top-up will misreport its estimate in exactly the way QA-006 describes. Same domain, same function, same one-line fix; `AppContext::fee_estimator()` is already in scope here.

In `tests/backend-e2e/core_tasks.rs`:
- [NITPICK] tests/backend-e2e/core_tasks.rs:200-207: TC-009 header comment still describes the removed funding step
  The latest delta removed the framework-wallet funding send from TC-009, and the test body now goes straight from constructing the `SingleKeyWallet` to asserting that `RefreshSingleKeyWalletInfo` returns `SingleKeyWalletsUnsupported`. The header comment still claims "The funding step still exercises a real send from the framework HD wallet to the single-key address", which contradicts the test body and will mislead anyone re-enabling this flow into thinking funding coverage is still present.

[lklimek]

Addressed the two remaining items from the latest review in 9218eb51:

• Fee estimator (suggestion) — completed the top-up estimator wiring: top_up_identity_from_platform_addresses (mod.rs:896) now uses self.fee_estimator(), matching top_up_identity.rs from the prior round. Scope is deliberately top-up estimates only — they feed the resolve_identity_topup_actual_fee plausibility band, so they must track the active multiplier. The transfer/other identity estimate sites (transfer_to_addresses, estimate_credit_transfer, register_identity, add_key, register_dpns, withdraw) stay on the default multiplier as part of the project-wide fee-multiplier cleanup, which is deferred and tracked separately — drawing the line at top-ups avoids a partial, inconsistent transfer-path fix in this PR.
• TC-009 comment (nitpick) — rewrote the header to present-state, dropping the stale "still exercises a real send…" line (the funding step was removed earlier in this PR); thread resolved.

Gate: build + clippy --all-features --all-targets -D warnings + +nightly fmt clean; fee_estimation 19/19, --lib identity 132/132, backend-e2e compiles.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[thepastaclaw]

Code Review

Both prior findings are FIXED in the latest delta: top_up_identity_from_platform_addresses now routes through self.fee_estimator() (prior-829-1), and the TC-009 header comment now matches the present-state test body (prior-829-2). One new in-scope suggestion from codex-rust-quality survives verification: the same line at src/backend_task/identity/mod.rs:896 still uses the wallet-funded estimate_identity_topup() even though the function is explicitly the platform-address top-up, and a dedicated estimate_identity_topup_from_addresses(input_count) exists in model/fee_estimation.rs and is already used by the matching UI screen. No CodeRabbit inline comment IDs were supplied, so coderabbit_reactions is empty.

🟡 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 `src/backend_task/identity/mod.rs`:
- [SUGGESTION] src/backend_task/identity/mod.rs:896: Use the platform-address top-up estimator on the address-funded path
  `top_up_identity_from_platform_addresses` calls `estimate_identity_topup()`, which is the wallet-funded variant and ignores per-input costs. The model already provides `estimate_identity_topup_from_addresses(input_count)` (model/fee_estimation.rs:376), which factors in `address_funding_asset_lock_cost`, `address_funds_transfer_input_cost * inputs`, plus a 20% safety buffer — and the matching UI screen already calls it (`src/ui/identities/top_up_identity_screen/by_platform_address.rs:103`). Because the backend returns `FeeResult::new(estimated_fee, estimated_fee)` from the same `u64`, using the wallet-funded estimator can under-report fees for this flow and silently diverges the UI estimate from the backend estimate, especially with >1 input.

[thepastaclaw]

🟡 Suggestion: Use the platform-address top-up estimator on the address-funded path

top_up_identity_from_platform_addresses calls estimate_identity_topup(), which is the wallet-funded variant and ignores per-input costs. The model already provides estimate_identity_topup_from_addresses(input_count) (model/fee_estimation.rs:376), which factors in address_funding_asset_lock_cost, address_funds_transfer_input_cost * inputs, plus a 20% safety buffer — and the matching UI screen already calls it (src/ui/identities/top_up_identity_screen/by_platform_address.rs:103). Because the backend returns FeeResult::new(estimated_fee, estimated_fee) from the same u64, using the wallet-funded estimator can under-report fees for this flow and silently diverges the UI estimate from the backend estimate, especially with >1 input.

Suggested change

	let estimated_fee = self.fee_estimator().estimate_identity_topup();
	let estimated_fee = self
	.fee_estimator()
	.estimate_identity_topup_from_addresses(inputs.len());

_{source: ['codex']}