feat(platform-wallet): keyring_core secret backends — encrypted-file + OS keyring (secrets feature)

[Claudius-Maginificent]

DRAFT PR — do not merge until CI has been run and #3625 lands.

STACKED ON #3625 (feat/platform-wallet-sqlite-persistor). The base branch for this PR is feat/platform-wallet-sqlite-persistor, not v3.1-dev. Please diff against that base, and merge this PR only after #3625 lands (or rebase onto v3.1-dev at that point).

---

REWORK NOTE — reviewers please read first. This PR has been through a hybrid rework (see the live branch head). The original commits shipped a custom SecretStore trait surface and a cargo-deny CI job. The rework adopts the upstream keyring-core 1.0.0 SPI (CredentialStoreApi / CredentialApi) as the public contract while preserving the full security construction byte-for-byte behind it. The file-encryption stack (Argon2id with parameter floors, XChaCha20-Poly1305 with per-put random XNonce, AAD binding, passphrase-verification token, atomic 0600 writes) is unchanged. rekey() returns a typed FileStoreError::Busy rather than panicking when credentials are outstanding. Please diff against the base branch (feat/platform-wallet-sqlite-persistor) as usual.

---

Issue being fixed or feature implemented

Wallet apps built on rs-platform-wallet need a place to persist secret material — BIP-39 mnemonics, BIP-32 seeds, xpriv keys — so a wallet can be rehydrated from storage without the user re-entering their phrase every time. The SQLite persister introduced in #3625 deliberately stores only public/correlation state (UTXOs, identities, contacts, token balances). It must never hold secret bytes, and currently has no companion that does.

A mobile or desktop wallet built on rs-platform-wallet derives a BIP-39 mnemonic at setup, persists wallet state via the SQLite backend, and the user closes the app. On next launch the wallet must rehydrate without asking for the phrase again. That requires a store that is: (a) outside the SQLite file, so backups and DB exports stay secret-free; (b) authenticated and encrypted at rest; (c) keyed by wallet identity, not by the wallet file path. This PR delivers that capability as the secrets module, enabled by default.

---

What was done?

platform_wallet_storage::secrets — default-on, upstream SPI

The secrets feature is in the default feature set (default = ["sqlite", "cli", "secrets"]). The off-state (--no-default-features --features sqlite,cli) still builds and tests cleanly — proven by a runtime guard test (tests/secrets_off_state.rs) and confirmed by cargo clippy --no-default-features --features sqlite,cli in the gate set.

The public SPI is upstream keyring_core::api::{CredentialApi, CredentialStoreApi} from keyring-core 1.0.0. This crate contributes two production backends plus zeroizing wrappers — not a bespoke trait surface.

Key shape

upstream field	this crate's mapping
service	SERVICE_PREFIX + hex(wallet_id) — "dash.platform-wallet-storage/" + 64 hex chars; one namespace per wallet
user	label, validated against ^[A-Za-z0-9._-]{1,64}$ (SEC-REQ-4.3) at build time and at every operation

WalletId is a fixed 32-byte newtype. validated_label runs defence-in-depth because credentials are long-lived objects.

Zeroizing wrappers (src/secrets/secret.rs)

• SecretBytes — wraps Zeroizing<Vec<u8>>. Redacting Debug, no Display/Deref/Serialize, constant-time equality via subtle::ConstantTimeEq only. PartialEq/Eq are NOT implemented — == on SecretBytes or SecretString is a compile error, enforced by compile_fail doc-tests that serve as durable forward-compat guards. Best-effort mlock via region. Used for seeds, derived keys, AEAD key material, and decrypted plaintext.
• SecretString — in-crate type for mnemonics and passphrases. Same redacting Debug / no-Display / no-Deref / no-Serialize disciplines. Full-capacity zeroize on Drop. PartialEq removed; constant-time compare via ct_eq only.

The memory hygiene contract at the SPI seam: CredentialApi::get_secret returns Vec<u8>; callers MUST wrap it into SecretBytes::new(entry.get_secret()?) immediately, with no named intermediate binding. SecretBytes::new takes the Vec<u8> by value — the bare-buffer exposure window is zero statements.

EncryptedFileStore (src/secrets/file/)

Argon2id + XChaCha20-Poly1305 vault with the following security construction (unchanged from the original 10 commits):

• KDF: Argon2id (argon2 0.5.3, pinned), enforced floors m ≥ 19 MiB / t ≥ 2 / p = 1, shipped defaults m = 64 MiB / t = 3. Parameters stored in the file header so future hardening does not strand existing vaults.
• Salt: 32 bytes from OsRng/getrandom, per-vault, in the header. Never constant, never derived from wallet_id or path.
• AEAD: XChaCha20-Poly1305 (chacha20poly1305 0.10.1, pinned). No unauthenticated mode. aes-gcm is absent.
• Nonce: fresh random 24-byte XNonce per put, stored alongside ciphertext. Counters are explicitly forbidden (multi-process / restart / file-copy scenario causes catastrophic keystream reuse).
• AAD: canonical length-prefixed format_version ‖ wallet_id ‖ label. Decryption under a different (wallet_id, label) or rolled-back format_version fails the Poly1305 tag. Blob-swap and replay attacks are structurally rejected.
• Tag failure: returns error and exposes zero decrypted-but-unverified bytes (non-detached AEAD decrypt API — consistent with the lesson of RUSTSEC-2023-0096).
• Passphrase-verification token: header-stored token PWSVAULT-VERIFY-v1 detects a wrong passphrase before any entry decrypt attempt, preventing mixed-key vault corruption.
• File permissions: created at 0600 via O_EXCL + fchmod before any plaintext-derived byte is written. Pre-existing file with looser permissions surfaces as a typed error — never blindly overwritten.
• Atomic + durable writes: temp file at 0600 in same directory → write → fsync temp → rename over target → fsync directory. A crash mid-write never truncates the prior vault.
• Rekey: decrypt-all → re-derive with fresh salt + current params → re-encrypt with fresh per-entry nonces → same atomic replace. No .bak left holding old key material.

EncryptedFileStore now implements CredentialStoreApi + CredentialApi. File-backend-specific failure modes (WrongPassphrase, KdfFailure, MalformedVault, InsecurePermissions, etc.) bridge to the upstream error type via the FileStoreFailure boxed-marker mechanism described below.

OS keyring backend

secrets::default_credential_store() returns Arc<dyn CredentialStoreApi + Send + Sync> over the platform's native store (linux-keyutils-keyring-store → dbus-secret-service-keyring-store on Linux/FreeBSD; apple-native-keyring-store on macOS; windows-native-keyring-store on Windows). Fail-closed with keyring_core::Error::NoDefaultStore on headless / unknown OS — never a silent plaintext fallback.

The cross-SPI error bridge (src/secrets/file/error_bridge.rs)

keyring_core::Error has no WrongPassphrase variant. File-backend errors bridge as follows: WrongPassphrase boxes a FileStoreFailure marker inside keyring_core::Error::NoStorageAccess (matching the operator UX of "keyring locked"); other failure modes render into BadStoreFormat's static String payload. secrets::downcast_failure(&err) is the single recovery path for typed distinction.

FileStoreFailure is unit-variants only (#[derive(Copy)]) with a _assert_copy::<FileStoreFailure>() compile-time witness. No variant carries a path, passphrase, label, or stringified user payload. Display interpolates no user data.

cargo-deny removed; RustSec advisory check covers all deps

The dedicated cargo-deny CI job and deny.toml are deleted. Because secrets is now in the default feature set, the pinned crypto crates (argon2, chacha20poly1305, zeroize, subtle, region, keyring-core, getrandom, per-platform store crates) are unconditionally in the lockfile and therefore unconditionally in scope for the existing rustsec/audit-check job. No advisory coverage gap.

MemoryCredentialStore

Test-only backend, gated behind __secrets-test-helpers (unreachable from production builds). The dev-dependency for test helpers carries default-features = false, features = ["__test-helpers"] so the off-state test (secrets_off_state) is genuinely non-vacuous.

Secrets boundary: maintained and guarded

• tests/secrets_scan.rs — greps src/sqlite/schema/ and migrations/ for private, mnemonic, seed, xpriv, secret. Exempts src/secrets/ by design.
• tests/secrets_guard.rs — positive guard for src/secrets/. Forbids logging/format sinks that pair with expose_secret(...) on the same logical statement. Also forbids {:?}-debug-format paired with keyring_core::Error in any src/secrets/ file (Smythe EDIT-2: BadEncoding(Vec<u8>) / BadDataFormat(Vec<u8>, _) carry byte payloads in Debug; our backends never construct those variants with secret bytes, and the guard enforces it). Guard is proven non-vacuous by a plant test.
• tests/secrets_api.rs — shape guards: get_secret re-wraps through SecretBytes::new, redacting Debug on SecretBytes/SecretString, no Box<dyn Error> in src/secrets/.
• tests/secrets_off_state.rs — runtime guard that --no-default-features --features sqlite,cli builds without the secrets module (D4).

SECRETS.md reflects the delivered spec

SECRETS.md is rewritten as a present-state specification covering the keyring-core SPI, key shape, memory hygiene contract, both backends, the error bridge, and all audit hooks. No migration narrative.

Downstream cascade (deferred, noted)

PR #3692 (full wallet rehydration) and #3693 (contacts + identity-keys) are stacked on this branch and have been rebased onto the keyring-core SPI: their seed_provider_adapter.rs now wraps a CredentialStoreApi-based store using the SecretBytes::new(entry.get_secret()?) zeroization seam. The consumer seam lives in #3692 because it consumes platform_wallet::seed_provider types that exist only on that branch — it could not land here.

---

How Has This Been Tested?

The following commands form the intended gate. CI has not yet been run on this branch (it is a draft); substantive CI jobs will run when un-drafted. An internal multi-agent review (Smythe security re-validation, Marvin QA, Adams consistency) confirmed all CRITICAL and HIGH findings resolved after one targeted fix cycle — no open items remain in this PR's scope.

# Workspace compilation
cargo check --workspace

# Full test suite with all features (152 passed / 0 failed / 3 ignored,
# including 2 new compile_fail doc-tests)
cargo test --all-features

# Off-state: secrets module absent (D4 guard)
cargo test --no-default-features --features sqlite,cli

# Clippy — default features
cargo clippy --all-targets --all-features -- -D warnings

# Clippy — off-state
cargo clippy --no-default-features --features sqlite,cli -- -D warnings

# Audit hooks
cargo test -p rs-platform-wallet-storage secrets_scan   # SQLite side clean
cargo test -p rs-platform-wallet-storage secrets_guard  # secrets side clean (incl. EDIT-2 guard)

# Doc tests (incl. compile_fail guards on == for SecretBytes/SecretString)
cargo test --all-features --doc

Test categories covered:

• Round-trip via EncryptedFileStore and OS keyring (put → drop → reopen → get, byte-exact)
• delete_credential honours NoEntry contract on absent entry
• Wrong passphrase → typed error, zero decrypted bytes returned
• AAD blob-swap: ciphertext moved to different (wallet_id, label) → rejected
• Nonce uniqueness across multiple put operations on the same entry
• Header version rollback → VersionUnsupported
• Vault file at looser-than-0600 permissions → InsecurePermissions
• Crash-mid-write simulation: prior vault intact, no plaintext temp file left
• Rekey: old-key ciphertext unrecoverable, no .bak retained
• Debug of SecretBytes/SecretString is redacted
• MemoryCredentialStore unreachable without __secrets-test-helpers
• == on SecretBytes/SecretString is a compile error (compile_fail doc-tests)
• secrets_guard EDIT-2 scan: no {:?} paired with keyring_core::Error in src/secrets/
• Off-state (--no-default-features --features sqlite,cli): secrets module absent

---

Breaking Changes

None. No semver-breaking change to anything published. The retired SecretStore trait and SecretStoreError type were inside this unmerged PR — they never reached a release. The upstream-SPI adoption is purely internal to this PR's evolution. Existing builds without --features secrets compile exactly as before.

The downstream PRs #3692 and #3693 will require a rebase to update their adapter to the CredentialStoreApi-based seam — this is an intra-PR-stack concern, not a published API break.

---

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

---

Known limitations and accepted risks

• AR-7 / R3 — upstream key-wallet::compute_wallet_id Debug/log hygiene. Accepted residual, untouched by this rework. A follow-up is owed before wider rollout.
• Downstream cascade — PR feat(platform-wallet): watch-only rehydration from persistor (seedless load) #3692 and feat(platform-wallet): add contacts and identity-key rehydration (item G) #3693 need to rebase and update seed_provider_adapter.rs to the CredentialStoreApi-based seam. Tracked; deliberately deferred per rework plan.
• Headless Linux OS keyring — default_credential_store() fails closed (NoDefaultStore). No silent fallback. Operators on headless systems use EncryptedFileStore explicitly.

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

[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: 41883e0a-84d2-4108-990b-e3a0edb0e12a

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 feat/platform-wallet-storage-secrets

---

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 and usage tips.}

[Copilot]

Pull request overview

Adds a default-on secrets subsystem to platform-wallet-storage to persist wallet secret material outside SQLite, using the upstream keyring_core SPI and providing both an encrypted-file vault backend and an OS-keyring backend.

Changes:

• Introduces platform_wallet_storage::secrets (default feature) with EncryptedFileStore (Argon2id + XChaCha20-Poly1305) and default_credential_store() for OS keyrings.
• Adds secret-handling wrappers (SecretBytes, SecretString) plus validation and an error-bridging layer to keyring_core::Error.
• Adds multiple guard tests (secrets_scan, secrets_guard, API shape checks, and “secrets off” build-mode guard) and updates docs/README/Cargo features accordingly.

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
packages/rs-platform-wallet-storage/tests/secrets_scan.rs	Clarifies schema/migrations scan scope vs src/secrets/ exemption.
packages/rs-platform-wallet-storage/tests/secrets_off_state.rs	Adds runtime guard ensuring secrets compile out when feature is disabled.
packages/rs-platform-wallet-storage/tests/secrets_guard.rs	Adds string-level leak-prevention scans for the secrets module.
packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs	Build-only test asserting secrets surface is available in default build.
packages/rs-platform-wallet-storage/tests/secrets_api.rs	API/boundary shape tests for secrets SPI usage and error rendering.
packages/rs-platform-wallet-storage/src/secrets/validate.rs	Adds WalletId newtype + strict label allowlist validation.
packages/rs-platform-wallet-storage/src/secrets/secret.rs	Implements SecretBytes/SecretString zeroizing wrappers and CT equality.
packages/rs-platform-wallet-storage/src/secrets/mod.rs	Wires secrets submodules and public re-exports.
packages/rs-platform-wallet-storage/src/secrets/memory.rs	Adds in-RAM CredentialStoreApi test double behind __secrets-test-helpers.
packages/rs-platform-wallet-storage/src/secrets/keyring.rs	Adds OS-keyring default store constructor with fail-closed behavior.
packages/rs-platform-wallet-storage/src/secrets/file/mod.rs	Implements encrypted vault store + CredentialStoreApi/CredentialApi.
packages/rs-platform-wallet-storage/src/secrets/file/format.rs	Defines vault format framing and canonical AAD construction.
packages/rs-platform-wallet-storage/src/secrets/file/error.rs	Defines file-backend error taxonomy.
packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs	Bridges file-backend errors into keyring_core::Error + downcast helper.
packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs	Implements Argon2id KDF + XChaCha20-Poly1305 seal/open helpers.
packages/rs-platform-wallet-storage/src/lib.rs	Exposes secrets module behind feature and adds send/sync/object-safety checks.
packages/rs-platform-wallet-storage/SECRETS.md	Updates spec/docs to present-state secrets implementation and audit hooks.
packages/rs-platform-wallet-storage/README.md	Updates feature table and build modes to include secrets and helpers.
packages/rs-platform-wallet-storage/Cargo.toml	Adds secrets dependencies, platform store deps, features, and dev-dep tweaks.
Cargo.lock	Pulls in keyring-core + platform store crates + crypto dependencies.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lklimek]

Why do we need both FileStoreError and FileStoreFailure? I would merge FileStoreFailure into FileStoreError , and only have one error.rs file for errors here, not two. I would also impl From or similar for conversion to keyring_core. This approach will lose some details, but I think we prefer that over confusion caused by two similar error types.

[Claudius-Maginificent]

Done in b6a84fdc19 — FileStoreFailure collapsed into FileStoreError; error_bridge.rs deleted entirely. Single typed-error path, Corruption distinguished from WrongPassphrase. Leaving for your ack.

[lklimek]

Manual serialization when serde / bincode is already in the dependency tree is an overengineering. Use serde + json if possible. It will also make it easier to debug, and data will be encrypted anyway.

[Claudius-Maginificent]

Done in 8ab4208332 — replaced the manual byte-layout with versioned serde_json (two-step parse: envelope header then payload). Manual codec retired. Leaving for your ack.

[lklimek]

Let's remove this, use file store instead (in tempfile/tempdir).

[Claudius-Maginificent]

Done in e1c7fa9418 — MemoryCredentialStore removed; the __secrets-test-helpers feature retired. Tests now use the file store backed by tempfile::tempdir(). Leaving for your ack.

[lklimek]

remove historical references and our internal IDs like SEC-003 from comments.

[Claudius-Maginificent]

Done in dc492ccf89 — stripped the SEC-* / SECRET-* historical IDs from inline comments across the secrets module. Leaving for your ack.

[lklimek]

No need for the license, we own copyright, remove it.

[Claudius-Maginificent]

Done in dc492ccf89 — license header removed from secret.rs (and other secrets-module files). Leaving for your ack.

[lklimek]

This convinced me to have our own API that will never leak raw secret data, and only expose that. Maybe an enum object or sth like that.

[Claudius-Maginificent]

Done in 0066a5a472 — new public SecretStore API exposes SecretBytes only, never &[u8] / Vec<u8>. keyring_core::CredentialStoreApi adapter sits behind it. Leaving for your ack.

[thepastaclaw]

Code Review

The PR is close, but the encrypted file store still has one blocking correctness flaw: concurrent mutations can overwrite each other and silently lose secrets. I also confirmed three non-blocking contract issues: entry-corruption is misclassified as a wrong passphrase after header verification, the new Rust DriveInternalError FFI code is not decoded on the Swift side, and the Swift prover warm-up API documents stronger completion semantics than the Rust FFI actually provides.

_Note: Inline posting failed (command failed (1): python3 scripts/review_poster.py dashpay/platform 3672 8a5ef7a
STDOUT:

STDERR:
Traceback (most recent call last):
File "/Users/claw/.openclaw/workspace/scripts/review_poster.py", line 138, in
result = post_review(repo, pr_number, h), so I posted the same verified findings as a top-level review body._

Reviewed commit: 8a5ef7a

🔴 1 blocking | 🟡 3 suggestion(s)

Verified findings

blocking: Vault writes are unsynchronized read-modify-write cycles that can drop concurrent updates

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 242)

rekey, put, and delete all read the current vault snapshot, mutate an in-memory Vec<VaultEntry>, and then replace the whole file with write_vault(). There is no mutex, file lock, version check, or compare-and-swap across that transaction. The store is explicitly exposed behind Arc<dyn CredentialStoreApi + Send + Sync>, so two threads can legally operate on the same vault at once; separate processes opening the same directory have the same problem. If two writers start from snapshot N, each applies a different change, and the last rename wins, one write is silently lost. For a secret store this is data loss, not a benign race.

suggestion: Entry integrity failures are misreported as wrong passphrases after the header token already authenticated the key

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 251)

derive_and_verify() has already proved that the supplied passphrase matches the vault header before these entry decryptions run. After that point, crypto::open() failing on an entry means the entry ciphertext or its AAD is corrupted or has been moved between labels, not that the passphrase is wrong. Mapping those failures to WrongPassphrase here, and again in get(), collapses the distinct FileStoreError::Decrypt / FileStoreFailure::Decrypt path that error.rs, error_bridge.rs, and SECRETS.md document for callers using downcast_failure(). That prevents callers from distinguishing operator error from vault corruption or tampering.

suggestion: Swift does not decode the new Rust `DriveInternalError` FFI code

packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift (line 292)

Rust now emits DashSDKErrorCode::DriveInternalError = 10 and has tests asserting that dash_sdk::Error::DriveInternalError(...) maps to that code. SDKError.fromDashSDKError still handles only codes 1 through 9 and 99, so code 10 falls into the default branch and is surfaced as .unknown(message). That loses the new error classification exactly at the Swift boundary and breaks any Swift-side handling that needs to distinguish storage-layer Drive failures from generic unknown errors.

switch error.code {
case DashSDKErrorCode(rawValue: 1): // Invalid parameter
  return .invalidParameter(message)
case DashSDKErrorCode(rawValue: 2): // Invalid state
  return .invalidState(message)
case DashSDKErrorCode(rawValue: 3): // Network error
  return .networkError(message)
case DashSDKErrorCode(rawValue: 4): // Serialization error
  return .serializationError(message)
case DashSDKErrorCode(rawValue: 5): // Protocol error
  return .protocolError(message)
case DashSDKErrorCode(rawValue: 6): // Crypto error
  return .cryptoError(message)
case DashSDKErrorCode(rawValue: 7): // Not found
  return .notFound(message)
case DashSDKErrorCode(rawValue: 8): // Timeout
  return .timeout(message)
case DashSDKErrorCode(rawValue: 9): // Not implemented
  return .notImplemented(message)
case DashSDKErrorCode(rawValue: 10): // Drive internal error
  return .internalError(message)
case DashSDKErrorCode(rawValue: 99): // Internal error
  return .internalError(message)
default:
  return .unknown(message)
}

suggestion: Swift warm-up API advertises completion semantics that the Rust FFI explicitly does not provide

packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift (line 332)

The Swift API documents warmUpShieldedProver() as building the proving key so the first shielded send does not pay the 30-second cost inline, and its async signature invites callers to await readiness. The Rust FFI it wraps does not wait for that work: platform_wallet_shielded_warm_up_prover() only calls runtime().spawn_blocking(...) and returns immediately, and the Rust docs explicitly say the first send still pays the cost if it races the background warm-up. As written, await PlatformWalletManager.warmUpShieldedProver() only confirms that the warm-up task was scheduled, not that the prover is ready.

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

- [BLOCKING] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:242-331: Vault writes are unsynchronized read-modify-write cycles that can drop concurrent updates
  `rekey`, `put`, and `delete` all read the current vault snapshot, mutate an in-memory `Vec<VaultEntry>`, and then replace the whole file with `write_vault()`. There is no mutex, file lock, version check, or compare-and-swap across that transaction. The store is explicitly exposed behind `Arc<dyn CredentialStoreApi + Send + Sync>`, so two threads can legally operate on the same vault at once; separate processes opening the same directory have the same problem. If two writers start from snapshot N, each applies a different change, and the last rename wins, one write is silently lost. For a secret store this is data loss, not a benign race.
- [SUGGESTION] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:251-255: Entry integrity failures are misreported as wrong passphrases after the header token already authenticated the key
  `derive_and_verify()` has already proved that the supplied passphrase matches the vault header before these entry decryptions run. After that point, `crypto::open()` failing on an entry means the entry ciphertext or its AAD is corrupted or has been moved between labels, not that the passphrase is wrong. Mapping those failures to `WrongPassphrase` here, and again in `get()`, collapses the distinct `FileStoreError::Decrypt` / `FileStoreFailure::Decrypt` path that `error.rs`, `error_bridge.rs`, and `SECRETS.md` document for callers using `downcast_failure()`. That prevents callers from distinguishing operator error from vault corruption or tampering.
- [SUGGESTION] In `packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift`:292-314: Swift does not decode the new Rust `DriveInternalError` FFI code
  Rust now emits `DashSDKErrorCode::DriveInternalError = 10` and has tests asserting that `dash_sdk::Error::DriveInternalError(...)` maps to that code. `SDKError.fromDashSDKError` still handles only codes 1 through 9 and 99, so code 10 falls into the default branch and is surfaced as `.unknown(message)`. That loses the new error classification exactly at the Swift boundary and breaks any Swift-side handling that needs to distinguish storage-layer Drive failures from generic unknown errors.
- [SUGGESTION] In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift`:332-340: Swift warm-up API advertises completion semantics that the Rust FFI explicitly does not provide
  The Swift API documents `warmUpShieldedProver()` as building the proving key so the first shielded send does not pay the 30-second cost inline, and its `async` signature invites callers to `await` readiness. The Rust FFI it wraps does not wait for that work: `platform_wallet_shielded_warm_up_prover()` only calls `runtime().spawn_blocking(...)` and returns immediately, and the Rust docs explicitly say the first send still pays the cost if it races the background warm-up. As written, `await PlatformWalletManager.warmUpShieldedProver()` only confirms that the warm-up task was scheduled, not that the prover is ready.

[thepastaclaw]

Code Review

Three carried-forward prior findings still reproduce at 6aa2942: the file vault still does unsynchronized read-modify-write updates, Swift still drops FFI error code 10 into .unknown, and Swift’s shielded prover warm-up API still overstates completion semantics. The prior wrong-passphrase-versus-corruption finding is fixed. The latest delta also introduces a new blocking regression: rekeying one wallet mutates the store-wide passphrase for every wallet sharing that file-store directory.

🔴 2 blocking | 🟡 2 suggestion(s)

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

Review details

🤖 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-storage/src/secrets/file/mod.rs`:
- [BLOCKING] lines 216-394: Carried-forward prior finding: vault updates still race and can silently discard concurrent writes
  `put`, `delete`, and `rekey` still implement updates as `read_vault(path)` -> mutate an in-memory `Vec<VaultEntry>` -> `write_vault(path, ...)` with no mutex, file lock, generation check, or compare-and-swap. This is not an internal-only pattern: `EncryptedFileStore` is exposed through `CredentialStoreApi`, `build()` clones the inner `Arc`, and the public API is explicitly multi-wallet, so concurrent callers in one process are supported and separate processes can point at the same directory as well. If two writers start from the same snapshot, the later `persist(path)` replaces the whole file and silently drops the earlier mutation. Atomic rename protects crash consistency, but it does not protect against lost updates, so this remains a real integrity/data-loss bug in the secret store.

In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:
- [BLOCKING] lines 275-316: New finding in latest delta: `rekey(wallet_id, ...)` corrupts the store-wide passphrase for other wallets
  The new backend is explicitly one store directory for many wallet IDs: each vault file is namespaced by `wallet_id` (`vault_path`), and the public `SecretStore` routes arbitrary `WalletId` values through one `EncryptedFileStore`. `rekey(wallet_id, new_passphrase)` rewrites only that wallet's file, but then assigns `self.passphrase = new_passphrase` for the entire store. After rekeying wallet A, any existing wallet B in the same directory still has its old header and can no longer be opened because future reads derive with A's new passphrase. The `None` branch is worse: rekeying a nonexistent wallet updates the global passphrase without rewriting any vault file, which immediately orphans every existing wallet in that directory. That is a correctness and data-loss regression introduced by the per-wallet-file redesign.

In `packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift`:
- [SUGGESTION] lines 292-314: Carried-forward prior finding: Swift still does not map Rust `DriveInternalError` (code 10)
  Rust now emits `DashSDKErrorCode::DriveInternalError = 10` and the FFI conversion explicitly uses that code for `dash_sdk::Error::DriveInternalError(...)`, but `SDKError.fromDashSDKError` still only handles 1 through 9 and 99. An incoming code 10 therefore falls through the default arm and becomes `.unknown(message)`. That drops the new error classification at the Swift boundary and prevents Swift callers from distinguishing Drive-layer failures from generic unknown errors.

In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift`:
- [SUGGESTION] lines 332-340: Carried-forward prior finding: Swift warm-up API still promises readiness that the Rust FFI does not provide
  `warmUpShieldedProver()` is documented and typed in a way that invites callers to `await` prover readiness, but the Rust entry point it wraps explicitly returns immediately after scheduling `CachedOrchardProver::warm_up()` on a background worker. Readiness is exposed separately through `platform_wallet_shielded_prover_is_ready()`. As implemented, `await PlatformWalletManager.warmUpShieldedProver()` confirms only that the warm-up task was queued, not that the proving key is available, so the Swift API contract is still misleading.

Reviewed commit: 6aa2942d

GitHub rejected inline review comments for this update, so the findings are posted body-only.

[Claudius-Maginificent]

📦 Review-response push + base merge-up — 8a5ef7aabd..34c8ecbb9c

12 commits addressing the open review feedback + 1 merge-up commit pulling the recent base hardening + atomicity work from #3625.

Review-response commits

OID	Headline	Closes
b6a84fdc19	refactor!: unify FileStoreError, drop error_bridge; distinguish Corruption from WrongPassphrase	lklimek error.rs:25, copilot file/mod.rs:374
647567e322	fix: remove redundant SecretString Drop (UB) and dangling mlock on empty SecretBytes	copilot secret.rs:131,216
8ab4208332	feat!: serde_json vault format with versioned two-step parse (CMT-007)	lklimek format.rs:84
68ed3d13b5	fix: cross-platform atomic vault write via NamedTempFile::persist (CMT-009)	copilot file/mod.rs:224
0066a5a472	feat!: public SecretStore API exposing SecretBytes, never raw bytes (CMT-002)	lklimek mod.rs:32
c636ac07d0	refactor: string-only keyring_core From; typed-path error distinction	(architecture follow-up)
a5c5bf0c6a	fix: box typed FileStoreError into keyring_core NoStorageAccess for lossless SPI recovery	(post-rework loss-of-typed-error gap)
e1c7fa9418	refactor: remove MemoryCredentialStore; retire __secrets-test-helpers (CMT-008)	lklimek memory.rs:1
671ce69c3f	fix: enforce lowercase-hex service, widen expose_secret guard scan (CMT-012/010)	copilot file/mod.rs:351, copilot tests/secrets_guard.rs:61
dc492ccf89	docs: strip historical comments + license header (CMT-013/014)	lklimek secret.rs:3,17
c58a2b5d00	feat: log swallowed mlock + corruption/write failures (Display-only, secret-free)	(observability gap)
6aa2942d22	docs: drop deleted MemoryCredentialStore / __secrets-test-helpers references (QA-002)	(stale-doc QA finding)

Base merge-up — 34c8ecbb9c

Merged origin/feat/platform-wallet-sqlite-persistor (which moved from 8a5ef7aabd to fe01634684 since this PR was last rebased) — brings in:

• 10 hardening commits from feat(platform-wallet): add platform-wallet-storage crate (sqlite persister) #3625 (CMT-001..012 fixes — 2 BLOCKING data-loss, 2 HIGH security, 6 MEDIUM, 2 LOW)
• 11 atomicity commits (A-1..A-8 + N-1..N-10 — 4 HIGH, 5 MEDIUM, 1 LOW, 5 doc-only) including the breaking commit_writes → CommitReport signature change on SqlitePersister
• 1 polish commit (Drop side-effect comment + README operational notes)

Conflicts: only Cargo.lock (regenerated by cargo). Auto-merged: Cargo.toml, README.md, src/lib.rs (disjoint sections on each side). All 4 WalletStorageError additions (BlobTooLarge, AssetLockEntryMismatch, IntegrityCheckFailed from sqlite side; FileStore-related from secrets side) coexist with wildcard-free classifier matches intact.

Verification post-merge

• cargo fmt --check — clean
• cargo clippy -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi --all-targets -- -D warnings — clean
• cargo test -p platform-wallet-storage — all green (114 unit + integration + 12 hardening + 11 atomicity + secrets API + 5 restore_staged_validation)
• cargo test --doc -p platform-wallet-storage — 3 passed, 1 ignored

Thread status

• 8 copilot threads — auto-resolved (bot, fixes verified at file:line)
• 6 lklimek threads — replies posted with the addressing commit's OID; left unresolved for the author to ack

Head + state

Head	34c8ecbb9c
Mergeable	should refresh to MERGEABLE once GitHub re-evaluates against the merged base
Base	feat/platform-wallet-sqlite-persistor @ fe01634684 (#3625)

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

[thepastaclaw]

Code Review

Of the 4 prior findings from review 6aa2942d, rekey() changing the store-wide passphrase after re-encrypting only one wallet is STILL VALID, file-backed secret mutations remaining unsynchronized read-modify-write cycles is STILL VALID, forward-schema databases being accepted by SqlitePersister::open() is FIXED, and staged-restore validation failures deleting WAL/SHM before rejection is FIXED. The carried-forward problems are both in the file-backed secrets backend. The latest delta adds 4 new blocking regressions in SQLite restore/persist and SPV runtime paths, plus 4 valid lower-severity filesystem hardening issues; this requires changes before merge.

_Note: Inline posting failed (command failed (1): python3 scripts/review_poster.py dashpay/platform 3672 34c8ecb
STDOUT:

STDERR:
Traceback (most recent call last):
File "/Users/claw/.openclaw/workspace/scripts/review_poster.py", line 138, in
result = post_review(repo, pr_number, h), so I posted the same verified findings as a top-level review body._

Reviewed commit: 34c8ecb

🔴 6 blocking | 🟡 4 suggestion(s)

Verified findings

blocking: Carried-forward prior finding: `rekey()` changes the store passphrase globally after re-encrypting only one wallet vault

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 275)

EncryptedFileStoreInner stores a single passphrase for the whole store, but rekey() only rewrites the vault file for the supplied wallet_id before assigning self.passphrase = new_passphrase. The same store instance serves arbitrary wallet IDs through build(), put(), get(), and delete(), so rekeying wallet_a immediately makes every other existing vault in the directory unreadable through that store instance: subsequent operations derive with the new passphrase even though those other vault files are still encrypted with the old one. This is a real multi-wallet data-loss bug, not just a theoretical inconsistency.

blocking: Carried-forward prior finding: file-backed secret mutations still lose concurrent updates

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 321)

Both put() and delete() still implement mutations as read_vault -> mutate Vec<VaultEntry> -> write_vault with no in-process mutex and no inter-process file lock. CredentialStoreApi::build() returns independent credentials backed by the same Arc<EncryptedFileStoreInner>, so concurrent writers can read the same old snapshot and then each atomically replace the vault with conflicting results. Atomic rename prevents torn files, but it does not serialize the read-modify-write cycle, so the last writer silently discards the other update.

blocking: Latest-delta finding: `restore_from()` still deletes destination WAL/SHM before its last fallible steps

packages/rs-platform-wallet-storage/src/sqlite/backup.rs (line 242)

The staged-copy validation now runs before the unlink loop, which fixes the earlier rejection path, but the restore is still not atomic. restore_from() removes <dest>-wal and <dest>-shm before the later set_permissions() and tmp.persist(dest_db_path) calls. Either of those later operations can still fail, leaving the old main database file in place after its WAL/SHM sidecars have already been deleted. On a live WAL-mode database that drops committed-but-uncheckpointed state from the destination. The code comment claiming that either both the main DB and siblings are replaced or none are touched is therefore false in the current implementation.

blocking: Latest-delta finding: committed WAL/SHM sidecars are never re-chmodded after ordinary writes

packages/rs-platform-wallet-storage/src/sqlite/persister.rs (line 171)

SqlitePersister::open() calls apply_secure_permissions(&config.path) immediately after opening the database, but the WAL and SHM sidecars normally do not exist yet. The normal write path in write_changeset_in_one_tx() commits at line 650 and returns without reapplying the helper, and apply_secure_permissions() is only called from open, backup, and restore. That means the first transaction can create wallet.db-wal and wallet.db-shm with the process umask instead of 0600, exposing recently committed wallet pages on multi-user hosts. The new test only proves the helper works when called manually; it does not cover the persistence path that actually creates the sidecars.

        tx.commit()?;
        apply_secure_permissions(&self.config.path)?;
        Ok(())

blocking: Latest-delta finding: `asset_locks::apply()` still writes rows whose typed columns can contradict the serialized blob

packages/rs-platform-wallet-storage/src/sqlite/schema/asset_locks.rs (line 36)

The read path now rejects typed-column/blob mismatches in decode_row(), but the write path still does not validate that the map key op matches entry.out_point or that the typed account_index column matches entry.account_index. Unlike identity_keys::apply(), which now rejects mismatched typed-vs-blob representations before persisting them, asset_locks::apply() will serialize contradictory state to disk. Because AssetLockChangeSet and AssetLockEntry have public fields, a malformed caller can construct this inconsistency today. The result is durable on-disk corruption that the next load will hard-fail on.

blocking: Latest-delta finding: `SpvRuntime::run()` holds the client read lock across `await` and blocks `stop()` forever

packages/rs-platform-wallet/src/spv/runtime.rs (line 138)

run() takes self.client.read().await, borrows the inner DashSpvClient, and then awaits client.run() while the RwLock read guard is still alive. stop() needs self.client.write().await before it can call c.stop(), so the advertised shutdown path cannot progress while run() is active. This is not required by the client API: the vendored dash-spv revision used here implements Clone for DashSpvClient, so run() could clone the client out of the lock and drop the guard before awaiting. As written, background sync can deadlock its own shutdown.

        let client = {
            let client_guard = self.client.read().await;
            client_guard
                .as_ref()
                .cloned()
                .ok_or(PlatformWalletError::SpvNotRunning)?
        };

        let result = client
            .run()
            .await
            .map_err(|e| PlatformWalletError::SpvError(e.to_string()));

        let mut client = self.client.write().await;
        let _ = client.take();

        result

suggestion: Vault directory permissions are never hardened or validated

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 73)

EncryptedFileStore::open() creates or accepts the vault directory and immediately trusts it, but only individual vault files are later checked for 0600. On Unix, a default create_dir_all() under a normal umask can leave the directory traversable by other local users, which leaks wallet IDs through <wallet_id>.pwsvault filenames and allows deletion or replacement of vault files in shared or group-writable directories. That weakens the backend's local-filesystem trust boundary even when each vault file itself is 0600.

suggestion: `read_vault()` allocates the entire file before enforcing any size bound or authenticity check

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 217)

read_vault() performs fs::read(path) immediately after metadata lookup, so any replaced or hostile vault file is fully loaded into memory before JSON parsing, KDF parameter validation, or AEAD verification runs. That gives an attacker who can replace the file at rest a straightforward memory-exhaustion and availability kill switch on every get/put/delete/rekey path that opens the vault.

suggestion: Permission validation is vulnerable to a metadata-to-read race

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 217)

read_vault() checks permissions with fs::metadata(path) and then performs a separate fs::read(path). If an attacker can replace the directory entry between those two syscalls, the code can validate one file's mode bits and then parse different bytes. Combined with the missing directory hardening, this bypasses the intended permission gate and leaves the decrypt path open to rollback or denial-of-service through attacker-chosen vault contents.

suggestion: Windows builds still skip the pre-existing permissions check entirely

packages/rs-platform-wallet-storage/src/secrets/file/mod.rs (line 553)

On non-Unix targets check_perms() is a stub that always returns Ok(()), so the documented invariant that a pre-existing vault with loose permissions is rejected is not enforced on Windows at all. A Windows user can therefore open and keep using a vault file with an overly broad ACL, which exposes ciphertext and metadata to other local principals and gives them a practical tamper or rollback point.

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

- [BLOCKING] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:275-317: Carried-forward prior finding: `rekey()` changes the store passphrase globally after re-encrypting only one wallet vault
  `EncryptedFileStoreInner` stores a single `passphrase` for the whole store, but `rekey()` only rewrites the vault file for the supplied `wallet_id` before assigning `self.passphrase = new_passphrase`. The same store instance serves arbitrary wallet IDs through `build()`, `put()`, `get()`, and `delete()`, so rekeying `wallet_a` immediately makes every other existing vault in the directory unreadable through that store instance: subsequent operations derive with the new passphrase even though those other vault files are still encrypted with the old one. This is a real multi-wallet data-loss bug, not just a theoretical inconsistency.
- [BLOCKING] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:321-395: Carried-forward prior finding: file-backed secret mutations still lose concurrent updates
  Both `put()` and `delete()` still implement mutations as `read_vault -> mutate Vec<VaultEntry> -> write_vault` with no in-process mutex and no inter-process file lock. `CredentialStoreApi::build()` returns independent credentials backed by the same `Arc<EncryptedFileStoreInner>`, so concurrent writers can read the same old snapshot and then each atomically replace the vault with conflicting results. Atomic rename prevents torn files, but it does not serialize the read-modify-write cycle, so the last writer silently discards the other update.
- [BLOCKING] In `packages/rs-platform-wallet-storage/src/sqlite/backup.rs`:242-274: Latest-delta finding: `restore_from()` still deletes destination WAL/SHM before its last fallible steps
  The staged-copy validation now runs before the unlink loop, which fixes the earlier rejection path, but the restore is still not atomic. `restore_from()` removes `<dest>-wal` and `<dest>-shm` before the later `set_permissions()` and `tmp.persist(dest_db_path)` calls. Either of those later operations can still fail, leaving the old main database file in place after its WAL/SHM sidecars have already been deleted. On a live WAL-mode database that drops committed-but-uncheckpointed state from the destination. The code comment claiming that either both the main DB and siblings are replaced or none are touched is therefore false in the current implementation.
- [BLOCKING] In `packages/rs-platform-wallet-storage/src/sqlite/persister.rs`:171-177: Latest-delta finding: committed WAL/SHM sidecars are never re-chmodded after ordinary writes
  `SqlitePersister::open()` calls `apply_secure_permissions(&config.path)` immediately after opening the database, but the WAL and SHM sidecars normally do not exist yet. The normal write path in `write_changeset_in_one_tx()` commits at line 650 and returns without reapplying the helper, and `apply_secure_permissions()` is only called from open, backup, and restore. That means the first transaction can create `wallet.db-wal` and `wallet.db-shm` with the process umask instead of `0600`, exposing recently committed wallet pages on multi-user hosts. The new test only proves the helper works when called manually; it does not cover the persistence path that actually creates the sidecars.
- [BLOCKING] In `packages/rs-platform-wallet-storage/src/sqlite/schema/asset_locks.rs`:36-50: Latest-delta finding: `asset_locks::apply()` still writes rows whose typed columns can contradict the serialized blob
  The read path now rejects typed-column/blob mismatches in `decode_row()`, but the write path still does not validate that the map key `op` matches `entry.out_point` or that the typed `account_index` column matches `entry.account_index`. Unlike `identity_keys::apply()`, which now rejects mismatched typed-vs-blob representations before persisting them, `asset_locks::apply()` will serialize contradictory state to disk. Because `AssetLockChangeSet` and `AssetLockEntry` have public fields, a malformed caller can construct this inconsistency today. The result is durable on-disk corruption that the next load will hard-fail on.
- [BLOCKING] In `packages/rs-platform-wallet/src/spv/runtime.rs`:138-158: Latest-delta finding: `SpvRuntime::run()` holds the client read lock across `await` and blocks `stop()` forever
  `run()` takes `self.client.read().await`, borrows the inner `DashSpvClient`, and then awaits `client.run()` while the `RwLock` read guard is still alive. `stop()` needs `self.client.write().await` before it can call `c.stop()`, so the advertised shutdown path cannot progress while `run()` is active. This is not required by the client API: the vendored `dash-spv` revision used here implements `Clone` for `DashSpvClient`, so `run()` could clone the client out of the lock and drop the guard before awaiting. As written, background sync can deadlock its own shutdown.
- [SUGGESTION] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:73-76: Vault directory permissions are never hardened or validated
  `EncryptedFileStore::open()` creates or accepts the vault directory and immediately trusts it, but only individual vault files are later checked for `0600`. On Unix, a default `create_dir_all()` under a normal umask can leave the directory traversable by other local users, which leaks wallet IDs through `<wallet_id>.pwsvault` filenames and allows deletion or replacement of vault files in shared or group-writable directories. That weakens the backend's local-filesystem trust boundary even when each vault file itself is `0600`.
- [SUGGESTION] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:217-221: `read_vault()` allocates the entire file before enforcing any size bound or authenticity check
  `read_vault()` performs `fs::read(path)` immediately after metadata lookup, so any replaced or hostile vault file is fully loaded into memory before JSON parsing, KDF parameter validation, or AEAD verification runs. That gives an attacker who can replace the file at rest a straightforward memory-exhaustion and availability kill switch on every `get`/`put`/`delete`/`rekey` path that opens the vault.
- [SUGGESTION] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:217-220: Permission validation is vulnerable to a metadata-to-read race
  `read_vault()` checks permissions with `fs::metadata(path)` and then performs a separate `fs::read(path)`. If an attacker can replace the directory entry between those two syscalls, the code can validate one file's mode bits and then parse different bytes. Combined with the missing directory hardening, this bypasses the intended permission gate and leaves the decrypt path open to rollback or denial-of-service through attacker-chosen vault contents.
- [SUGGESTION] In `packages/rs-platform-wallet-storage/src/secrets/file/mod.rs`:553-556: Windows builds still skip the pre-existing permissions check entirely
  On non-Unix targets `check_perms()` is a stub that always returns `Ok(())`, so the documented invariant that a pre-existing vault with loose permissions is rejected is not enforced on Windows at all. A Windows user can therefore open and keep using a vault file with an overly broad ACL, which exposes ciphertext and metadata to other local principals and gives them a practical tamper or rollback point.

[thepastaclaw]

Supplemental correction to my review for 34c8ecbb: the prior-finding reconciliation in the review body accidentally dropped two explicitly assigned prior Swift findings. I rechecked all four prior findings from 6aa2942d against the current head; the correct reconciliation is:

• STILL VALID — packages/rs-platform-wallet-storage/src/secrets/file/mod.rs: vault updates are still unsynchronized read-modify-write cycles that can silently discard concurrent writes.
• STILL VALID — packages/rs-platform-wallet-storage/src/secrets/file/mod.rs: rekey(wallet_id, ...) still rewrites one wallet vault and then changes the store-wide passphrase used for every wallet in the directory.
• STILL VALID — packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift: Swift still does not map Rust FFI error code 10 (DriveInternalError), so it falls through to .unknown(message).
• STILL VALID — packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift: Swift warmUpShieldedProver() still promises stronger completion semantics than the Rust FFI provides; awaiting it only confirms the Rust warm-up task was queued, not that the prover is ready.

Those last two Swift items should be treated as carried-forward suggestion-level findings in addition to the findings listed in the review body.