[romchornyi]

Basic explanation

What this does: It keeps the wallet's cached copy of an identity from going backwards or past the chain around an identity update, and keeps key derivation info from being lost when a store fails or a refresh finds a new key. Every identity update carries a revision number, the cached revision plus one, and Drive takes it only at exactly the identity's revision plus one, so a cache that drifts gets the next update refused.

Value: Identity updates (adding or disabling keys, including App Connect sign-in in dashwallet-ios#1202) no longer get refused after a refresh hits a lagging node or races the post-broadcast step, and keys survive a full disk during a restore and a restart after a refresh.

Risks: Low to medium, wallet-side only; no consensus code and no public API changes. An identity at u64::MAX is now refused instead of wrapping. Fetched identities are now folded into the cache by revision and proof height instead of replacing it.

Issue being fixed or feature implemented

The wallet builds every identity update from its local cache, at the cached revision plus one, and Drive accepts an update only at exactly the state revision plus one. On base the cache can drift from the chain: a fetch answered by a node one block behind rolls it back, and a refresh between the broadcast and the local bump leaves it one revision past the chain. Either way the next update is refused. This is fixed inside the wallet, between the SDK and the cache: fetches are folded in by revision, and the post-broadcast step takes the transition's revision.

The revision pin is out. An earlier version of this PR also added a caller-supplied revision argument, so a caller that records an update's revision before sending it (dashwallet-ios#1202) could not be surprised by a cache that moved in between. The maintainers agreed to settle an update's fate through a broadcast outbox instead (plan in the discussion below), which needs the cache invariant this PR keeps: the cache never runs ahead of the chain. The argument, its error and its FFI/Swift/Kotlin/JNI surface are removed.

Each piece kept here has a scenario test that drives the base code through a real flow (mock SDK, mock or real SQLite persister) and reproduces the defect:

• A1: the fetch fold. An update lands at revision 2. DashConnect's pre-update loadIdentity(atIndex:), or a pull-to-refresh, within about one block hits a node one block behind, which the SDK's height tolerance accepts. On base the cache goes back to revision 1, the disabled key is enabled again, the added key disappears, and the balance goes back to its pre-fee value. The next update then carries revision 2 and Drive refuses it.
• A2: key limits only rise. A protocol-14 key-limits top-up, followed within a block by a fetch from a lagging node. On base the cached budget and expiry go down, so the next top-up is computed from a stale total and consensus refuses it, or it lands below what was intended.
• A3: apply_landed_update. A load or refresh hits an up-to-date node between the update's snapshot and its post-broadcast revision bump. On base the cache ends at 3 while the chain is at 2, so the next update is refused. The bump also overwrites the on-chain disabled_at with the local clock.
• B1a: one changeset for a new identity and its keys. The disk fills (a real field case) between the identity-row write and the key-row write of a restore's discovery, and stays full until the app ends. On base, after a restart, the key rows have no breadcrumbs (watch-only), there is no scan record so startup skips discovery, and a refresh writes no key rows. Only a manual rescan or a load at index 0 heals it. Load by index had the same three-write shape.
• B3a: refresh stores the keys it changes. A key added on another device and picked up by refresh_identity stays in memory only. On base it is gone after a restart while the stored revision claims it.

The SQLite keep-breadcrumb rule and the changeset merge rule are what make B3a safe. A refresh row carries no breadcrumb, and on base a breadcrumb-less upsert replaced the whole SQLite row and wiped the stored breadcrumb. The Swift and Kotlin persisters already kept it.

Removed after the maintainer's review. The scenarios found no field trigger for the rest, so it is out of this PR:

• C2: re-associating an identity with another wallet;
• C3: two rejections of one key at different coordinates;
• C4: the Room 15 to 16 legacy-owner migration;
• B1b and B1c: the in-memory pending key-row queue;
• B2: the per-row Constraint tracing.

The external-key breadcrumb fix (#5501) moves to its own PR (link: #5540). The key-provenance work stays parked under #5525.

What was done?

• rs-platform-wallet:
  • update_identity_with_external_signer computes the transition's revision with checked_add, so an identity at u64::MAX is refused instead of wrapping.
  • apply_landed_update, the post-broadcast step. If the cache is behind the transition, it takes the transition's revision (not its own plus one) and the update's keys and disables, as before. If the cache is already at the transition's revision or past it, it already holds the update as Platform has it, so the payload is not applied over it. The added keys are stored from the cache's copies with the breadcrumb the update gives them. Both branches store the revision and the update's keys as one snapshot write and return a failed store, so a disable of a key the cache lacks still stores the landed revision.
  • ManagedIdentity::take_fetched_identity, the fetch fold shared by refresh, load by index and discovery. A fetch at the cache's revision or above is taken whole, key states included, as on base. A fetch at a lower revision leaves the cache's revision, keys and disables. It takes only a raised limit, or a disable it shows on a key the cache holds enabled, and never rewrites a disable the cache holds. Each key limit keeps the higher of the two values. The balance follows the proof's block height (fetch_with_metadata and the existing balance watermark), not the revision.
  • Load by index and discovery add a new identity with IdentityManager::add_identity_unstored and store it with its verified keys as one changeset (store_fetched_identity_keys).
  • Refresh stores the rows of the keys its fold adds or changes, without a breadcrumb, together with the scalar snapshot.
  • IdentityKeysChangeSet::merge: a later entry without a breadcrumb keeps the earlier entry's.
  • Drive's revision refusal. Shipped builds could leave a cached revision one past the chain (the A3 race), and no fetch heals that cache, because each one comes back older, so every update was refused with InvalidIdentityRevisionError (40203). That error carries the revision Drive holds. On that refusal the cached revision moves down to Drive's, only while the cache is still at the revision the refused transition was built from, and is stored with the scalar snapshot; the call returns the error and the next update carries the right revision. The cache is never raised from it. The error comes from one node without a proof: a node that lies low costs one refused update, and the next honest fetch brings the cache back.
• rs-platform-wallet-storage: the SQLite identity_keys writer keeps the breadcrumb (and its wallet id) a row holds when an upsert for the same row carries none. It does so only for a row that passes the reader's column checks (contradicts_columns, shared with load_state) and holds the same public key: the same type, data and hash. Any other stored row is replaced without its coordinates.

Known limitations:

• A heal through Drive's refusal costs one paid refusal: Drive charges for a revision mismatch, and checks the revision only in the block.
• A fetch at the cache's own revision is taken whole as on base, so a lagging proof at revision 0 can still undo a key state Core changed on a masternode identity. Tracked in platform-wallet: follow-ups left out of #5487 (identity fetch and update persistence) #5542.
• A breadcrumb that an older update fabricated for an external key now stays in SQLite, as it already did in Swift and Kotlin. This is cosmetic: the public-key binding refuses to sign with it. Tracked in platform-wallet: identity update gives every added key a wallet derivation breadcrumb, external keys included #5501.

How Has This Been Tested?

cargo test -p platform-wallet -p platform-wallet-ffi -p platform-wallet-storage -p rs-unified-sdk-jni --all-features: 3193 pass, 0 fail. cargo clippy for the same crates with --all-features --tests -- -D warnings is clean, and so is cargo fmt. The Swift, Kotlin and JNI sources are unchanged from v5.1-dev.

Tests added here:

• Scenario regressions (mock SDK, real fetch paths):
  • wallet/identity/network/lagging_fetch_tests.rs, A1: should_keep_a_landed_update_when_a_lagging_fetch_answers_older (load by index and refresh).
  • The same file, A2: should_keep_raised_key_limits_when_a_lagging_fetch_answers_older.
  • The same file, A3: should_not_put_the_cache_past_the_chain_when_a_fetch_lands_before_the_apply.
  • The same file: should_keep_a_second_disable_when_two_lagging_fetches_answer_older. Two revision-2 answers after an update at 3 disables a key again keep the revision and the disable.
• Scenario regressions (real SQLite, a disk that fills, restart):
  • rs-platform-wallet-storage/tests/integration/sqlite_identity_key_restore.rs, B1a: should_not_leave_watch_only_key_rows_when_the_disk_fills_during_a_restore.
  • The same file, B3a: should_store_a_key_a_refresh_finds_so_it_survives_a_restart.
  • The same file: should_store_a_key_core_re_enabled_so_it_survives_a_restart, a revision-0 key re-enabled by a refresh, through a restart.
  • The same file, a load-at-index control: should_store_a_key_a_load_at_index_finds_with_its_breadcrumb.
• Load by index stores in one changeset: loading::tests::should_store_a_newly_loaded_identity_with_its_keys_in_one_changeset. It fails on the base load path, which writes three changesets, the first with breadcrumb-less key rows.
• Revision: update_revision_tests::should_carry_one_above_the_cached_revision, should_refuse_an_identity_at_the_last_revision.
• Landed update: should_take_a_landed_update_only_into_a_cache_behind_it, should_store_an_added_key_with_its_breadcrumb_when_the_cache_is_ahead.
• Drive's revision refusal: should_take_drives_revision_from_an_update_refused_for_its_revision, should_keep_the_cached_revision_unless_drive_holds_a_lower_one. Landed update: should_store_a_disable_the_cache_already_holds_when_the_confirmation_comes_back, should_store_a_landed_revision_with_its_keys_in_one_write (an added key with a disable, a disable of a key the cache lacks, a failed store).
• Fold: should_keep_the_higher_of_each_key_limit, should_fold_an_older_fetch_s_key_decisions_into_the_cache_s_keys, should_keep_a_raised_limit_in_a_fetch_s_key_decisions, should_store_the_keys_a_refresh_adds_or_changes.
• Merge: should_keep_an_earlier_breadcrumb_when_a_later_key_entry_has_none.
• SQLite writer (sqlite_identity_keys_reader.rs): should_keep_a_stored_breadcrumb_when_an_upsert_carries_none, should_keep_a_stored_wallet_id_with_the_breadcrumb_when_an_upsert_carries_neither, should_replace_a_stored_row_that_does_not_decode, should_replace_an_oversized_stored_row_without_reading_it, should_lend_a_stored_breadcrumb_only_to_the_same_key_from_a_valid_row.

Breaking Changes

None. No public Rust, C ABI, Swift, Kotlin or JNI signature changes.

Checklist:

• I have performed a self-review of my own code
• My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
• 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
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · 8df7515

• Bots — thepastaclaw skipped by romchornyi
• Self-review — posted; again after any push
• Build green
• Approvals
  • wallet-storage (packages/rs-platform-wallet-storage/src/sqlite/schema/identity_keys.rs, packages/rs-platform-wallet-storage/tests/integration/main.rs, packages/rs-platform-wallet-storage/tests/integration/sqlite_identity_key_restore.rs and 1 more) — approved by lklimek
  • rs-platform-wallet (packages/rs-platform-wallet/src/changeset/changeset.rs, packages/rs-platform-wallet/src/error.rs, packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs and 7 more) — approved by llbartekll

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

• Bug Fixes
  • Identity updates now retain newer locally cached information when older network responses arrive, while incorporating eligible key changes and updated key limits.
  • Identity key derivation details and wallet associations are preserved across updates and app restarts, supporting more reliable restoration of watched keys.
  • Identity loads and refreshes save fetched identity and key information together, reducing the chance of partially saved state.
  • Balance updates use the newest verified information, preventing older responses from replacing newer balances.
  • Revision conflicts and overflow are handled more safely, and successfully completed updates are applied to the local identity state.
  • Failed key restoration can be retried on a later launch.

[coderabbitai]

Walkthrough

Identity updates now reconcile landed and refused revisions. Identity fetches fold state by revision and proof time, then persist identity snapshots and key rows together. Changeset and SQLite upserts preserve derivation metadata only for matching key records.

Changes: Identity updates and state

Layer / File(s)	Summary
Revision-aware update flow packages/rs-platform-wallet/src/error.rs, packages/rs-platform-wallet/src/wallet/identity/network/update.rs	Updates use checked revision increments, apply landed changes to cached identity state, and handle matching Drive revision refusals.
Fetched identity state folding packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs, packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/sync.rs, packages/rs-platform-wallet/src/wallet/identity/network/lagging_fetch_tests.rs	Fetched identities and keys are folded by revision and proof time. Key limits and balance watermarks retain newer cached values.
Derivation breadcrumb preservation packages/rs-platform-wallet/src/changeset/changeset.rs, packages/rs-platform-wallet-storage/src/sqlite/schema/identity_keys.rs, packages/rs-platform-wallet-storage/tests/integration/sqlite_identity_keys_reader.rs	Changeset and SQLite upserts retain derivation metadata only for matching key records. Tests cover mismatched, oversized, undecodable, and invalid rows.
Fetch and persist identity state packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs, packages/rs-platform-wallet/src/wallet/identity/network/loading.rs, packages/rs-platform-wallet/src/wallet/identity/network/signing_key.rs, packages/rs-platform-wallet/src/wallet/identity/state/manager/lifecycle.rs, packages/rs-platform-wallet-storage/tests/integration/*identity_key_restore.rs, packages/rs-platform-wallet-storage/tests/integration/main.rs	Discovery, loading, and refresh store fetched identity state and key rows together. Restart tests cover persistence failures and restored key breadcrumbs.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~50 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Platform
  participant IdentityFetch
  participant ManagedIdentity
  participant WalletPersister
  Platform->>IdentityFetch: Return identity and metadata
  IdentityFetch->>ManagedIdentity: Supply identity, key decisions, and block time
  ManagedIdentity->>ManagedIdentity: Fold fetched identity and key state
  ManagedIdentity->>WalletPersister: Store key rows and identity snapshot

Loading

Possibly related issues

• platform-wallet: follow-ups left out of #5487 (identity fetch and update persistence) #5542 — Covers identity revision and persistence behavior, including revision-refusal handling and landed-update writes.
• platform-wallet: key-origin provenance across wallets and composed rejections #5525 — Covers key-origin provenance behavior related to derivation breadcrumb preservation.

Suggested reviewers: thepastaclaw, quantumexplorer, shumkov

Merge Risk: Low · up to 8df75

After restart, manual-mode wallets can lose refusal recovery and incur another rejected update. Fix or accept this bounded risk before merging.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: keeping the cached identity synchronized with chain updates. This matches the pull request objectives and changeset.
Docstring Coverage		Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

⛔ Final review complete — 1 blocking finding(s) (commit 8df7515)

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Static verification found no blocking defects: the revision check precedes signing and broadcast, and the cache revision reconciliation avoids incrementing an already-refreshed revision. Two regression-coverage suggestions and one FFI documentation nit remain. Current Rust wallet, Swift SDK, and Kotlin SDK checks pass; PR Hygiene remains pending, and no local builds or tests were run.

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

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gemini-3.8-flash-high (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The change adds revision preconditions and concurrent cache-update handling across Rust, C FFI, and Swift, but is contained and does not meet the large-or-intricate requirement for critical.
• Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
• Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 71% left, 5h 100% left
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 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/update.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/update.rs:267-270: Add regression coverage for post-confirmation revision reconciliation
  The three new tests exercise only `update_revision`, not this separate cache reconciliation fix. Replacing this assignment with the previous `cached_revision + 1` would leave those tests passing while restoring the concurrent-refresh bug. Add coverage of the local-apply path for a submitted transition at revision 5 when the cache remains at 4, has refreshed to 5, or has advanced to 6, asserting final revisions of 5, 5, and 6 respectively. Exercise the production local-apply path through a mocked broadcast or an extracted helper used by that path.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/update.rs:399-409: Exercise revision rejection through the public async method
  The helper tests do not exercise `update_identity_with_external_signer_at_revision`, so they would still pass if the public method stopped forwarding the requested revision or moved validation after signing or broadcast. Add an async regression test using a cached identity, a recording signer, and an SDK mock with no network expectations; assert the structured mismatch fields, zero signer calls, and unchanged cached state. Cover both mismatch directions and assert `(expected, actual)` explicitly, including `(5, 4)` for the cache-behind case, which currently checks only the error variant. The crate already enables SDK mocks in its dev-dependencies, and `network/signing_key.rs` demonstrates local wallet construction with `SdkBuilder::new_mock()`.

In `packages/rs-platform-wallet-ffi/src/identity_update.rs`:
- [NITPICK] packages/rs-platform-wallet-ffi/src/identity_update.rs:327-334: Document the new FFI entry point's pointer safety contract
  The new public unsafe entry point documents revision behavior but not the caller obligations for its raw pointers. Its shared implementation reads the identity buffer, constructs slices from the supplied arrays, reads nested public-key buffers, and borrows the signer across worker execution. Add a `# Safety` section specifying readable lengths and alignment for nonempty arrays and nested buffers, a valid signer handle from the SDK's signer constructors, and the required lifetimes through completion of the call. The neighboring credit-transfer entry point provides a useful model. This is a documentation gap, not an ownership-layer violation; the crate explicitly allows `clippy::missing_safety_doc`, so CI does not enforce it.

[github-actions]

@coderabbitai review

No review for fde97b10 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The revision preflight is correctly ordered, the public-path tests cover both mismatch directions, and all three prior findings are fixed. Two blocking cache races remain: newer cached key state can be overwritten by an older landed transition payload, and an older refresh can overwrite the locally applied landed update. The Rust workspace test check was still in progress in the supplied CI snapshot.

🔴 2 blocking | 🟡 1 suggestion(s)

2 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: Prevent a stale refresh from undoing the landed cache update
packages/rs-platform-wallet/src/wallet/identity/network/loading.rs:339-365

refresh_identity fetches outside the wallet-manager lock and then unconditionally assigns the fetched identity under the write lock. An older fetch can therefore complete after apply_landed_update: it may fetch revision 4, the update may land at revision 5 and update the cache, and the refresh can then restore revision 4 and the old key set. A subsequent revision-qualified update sees the wrong cached revision and may build a duplicate revision-5 transition instead of refusing or submitting the intended revision. Apply fetched identities monotonically against the current cached revision, preserving newer key and persistence state, or serialize refresh application with landed-update reconciliation. The same policy should cover the other fetched-identity writer in load_identity_by_index_inner.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, rust-quality, security-auditor)}

🟡 Suggestion: Preserve revision mismatch as a typed FFI error
packages/rs-platform-wallet-ffi/src/error.rs:1125-1137

IdentityRevisionMismatch is the expected, pre-broadcast result of the new revision-qualified API, but the blanket FFI conversion falls through to ErrorUnknown. Mobile callers therefore cannot reliably distinguish an expected stale-cache/preflight mismatch from an SDK failure or another unmapped error without parsing the rendered message, and the expected/actual fields are lost as structured data. Append a dedicated FFI result code and mirror it in the Swift and Kotlin result-code mappings so callers can safely discard or reconcile the recorded revision.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, rust-quality, security-auditor)}

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is large and intricate and modifies identity update signing, key-handling, and broadcast behavior in packages/rs-platform-wallet/src/wallet/identity/network/update.rs and its FFI/Swift boundaries.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (lane failed), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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/update.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/update.rs:289-297: Do not apply an older transition payload over newer cached identity state
  `apply_landed_update` makes only the revision monotonic. It still applies `added_keys` and `disabled_ids` unconditionally, even when `cached_revision` is already greater than `transition_revision`. For example, an update at revision 5 can add key 3, a later update can land at revision 6 and disable key 3, and the cache can be refreshed to revision 6 before the revision-5 confirmation applies. The `add_key` call then restores the revision-5 active key and emits an upsert that can overwrite the newer persisted key row, so the wallet may select a disabled key for signing despite reporting revision 6. Apply the transition payload only when it is not older than the cached identity, or revision-guard each key mutation and its persistence changeset.

In `packages/rs-platform-wallet/src/wallet/identity/network/loading.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/loading.rs:339-365: Prevent a stale refresh from undoing the landed cache update
  `refresh_identity` fetches outside the wallet-manager lock and then unconditionally assigns the fetched identity under the write lock. An older fetch can therefore complete after `apply_landed_update`: it may fetch revision 4, the update may land at revision 5 and update the cache, and the refresh can then restore revision 4 and the old key set. A subsequent revision-qualified update sees the wrong cached revision and may build a duplicate revision-5 transition instead of refusing or submitting the intended revision. Apply fetched identities monotonically against the current cached revision, preserving newer key and persistence state, or serialize refresh application with landed-update reconciliation. The same policy should cover the other fetched-identity writer in `load_identity_by_index_inner`.

In `packages/rs-platform-wallet-ffi/src/error.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/error.rs:1125-1137: Preserve revision mismatch as a typed FFI error
  `IdentityRevisionMismatch` is the expected, pre-broadcast result of the new revision-qualified API, but the blanket FFI conversion falls through to `ErrorUnknown`. Mobile callers therefore cannot reliably distinguish an expected stale-cache/preflight mismatch from an SDK failure or another unmapped error without parsing the rendered message, and the `expected`/`actual` fields are lost as structured data. Append a dedicated FFI result code and mirror it in the Swift and Kotlin result-code mappings so callers can safely discard or reconcile the recorded revision.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[romchornyi]

/skip-bots

[github-actions]

Ready for review — wallet-storage: lklimek.
Full checklist in the description.

[llbartekll]

Re-approving at 8df7515. 51d7be8 does what we agreed: the _at_revision API, IdentityRevisionMismatch, code 62 and the FFI/Swift/Kotlin/JNI revision params are gone, and everything outside the two wallet crates matches the merge-base again, so no breaking change. The fold, apply_landed_update, the 40203 heal and the persistence fixes are unchanged from bf9b415. The reworded store-failure error in 8df7515 is accurate on both branches, since store_keys_with_snapshot layers the keys in memory before the store. Rust workspace tests are green here now, so the wallet crates got a real CI run. Fine with leaving the withExtendedLifetime change for a sweep across all the call sites.

[github-actions]

Policy satisfied — this can merge.
Full checklist in the description.