feat(kotlin-sdk)!: keystore rework — policy-alias split, layered key recovery, durable repair, structured signer errors (stacked on #4191) - #4183
Merged
QuantumExplorer merged 32 commits intoAug 4, 2026
Conversation
bfoss765
requested review from
QuantumExplorer,
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
July 21, 2026 12:40
Contributor
|
Warning Review limit reached
Next review available in: 57 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (34)
📝 WalkthroughWalkthroughAdds policy-based keystore aliases and migration handling, transactional pending identity-key tracking, structured signer errors, typed platform-wallet errors, and managed-identity lookup classification across Kotlin, Rust, and Swift. ChangesKeystore policy and identity-key recovery
Pending identity-key repair state
Signer and platform-wallet error flows
Schema and parity artifacts
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Signer
participant KotlinJNI
participant RustFFI
participant PlatformWallet
Signer->>KotlinJNI: completeSign(errorCode, errorMessage)
KotlinJNI->>RustFFI: forward structured signer error
RustFFI->>PlatformWallet: map signing-key-unavailable prefix to code 31
PlatformWallet-->>Signer: return typed platform-wallet error
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Collaborator
|
🕓 Ready for review — 11 ahead in queue (commit 6e076c0) |
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt`:
- Around line 549-562: Update the migration block around store.edit in
WalletStorage so CancellationException is not swallowed by runCatching; rethrow
cancellation while retaining the existing handling for non-cancellation failures
and successful migration behavior.
🪄 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: 987bcd6c-12ec-4a70-8f81-bdad970986df
📒 Files selected for processing (15)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicy.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreKeyGenPolicyTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/ManagedIdentityNotFoundTranslationTest.ktpackages/rs-platform-wallet-ffi/src/dashpay.rs
Collaborator
|
This needs reconciliation with #4172 before it can merge — the branch is
Suggestion: split the unmanaged-identity-reads + typed |
Collaborator
|
Additional findings after checking the existing threads and the current #4172 base:
I am not repeating the existing rebase/invalidation-recovery and cancellation comments; those remain valid. The earlier |
Contributor
Author
|
Took your suggestion: the conflict-free slices are split out as #4191 — unmanaged-identity reads (including both |
bfoss765
force-pushed
the
port/v4.1/android-integration
branch
from
July 21, 2026 23:26
00804cd to
ce3490a
Compare
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 21, 2026
…as split The inherited dashpay#4172 instrumented tests hardcode the legacy KEYS_ALIAS and the single-alias API; after the alias split the default-policy storage writes under KEYS_ALIAS_AUTH_GATED, so simulating keypair replacement by deleting the legacy alias no longer invalidates anything (the CI failure shumkov diagnosed on storeIfAbsentRederivesWhenTheKeysAliasKeypairWasReplaced). - WalletStorageOwnershipTest: the replaced-keypair, rejected-blob, and no-regeneration probes delete/inspect KEYS_ALIAS_AUTH_GATED; the fingerprint-capture test uses encryptForIdentityKeysAlias (and pins the captured producing alias); the stale-invalidation-cleanup test uses the alias-parameterized deleteIdentityKeysAliasIfCurrentGeneration. - KeystoreSignerInstrumentedTest: canSignWith rejection simulates replacement of the policy alias. The five DashPayUnlockAndSyncTest / WalletManagerRoundTripTest failures from PR dashpay#4183's CI run were NOT test or SDK defects: the branch predated dashpay#4172's workflow hardening (screen_off_timeout / stayon / dismiss-keyguard + the hard deviceLocked=0 guard), so the emulator re-locked mid-run and every MASTER_ALIAS operation threw InvalidKeyException. Rebuilding on the current base inherits the fixed workflow; those tests are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
changed the title
bfoss765
changed the title
Collaborator
|
Consolidated re-review of the rework (two independent passes). First the good news: all seven previously-blocking items are genuinely resolved — invalidated-key cleanup survives with generation-checked deletion, the retrieve ladder layers legacy shape-dispatch before the fingerprint gate, However, the rework surfaces new blocking findings:
|
Contributor
Author
|
All five new findings addressed (255 unit tests, 0 failures):
Two honest residuals, both fail-closed (neither can persist a wrong key): the real deriver's FFI pubkey-equality is exercised on-device rather than in a JVM unit test (AndroidKeyStore crypto can't run on the JVM) — it's in our device test plan; and on API 29–32 the numeric error code isn't available, so an exotic "reports-secure-but-rejects" OEM quirk on old Android wouldn't be text-classified, though genuine lockless devices are still caught by the primary |
bfoss765
force-pushed
the
port/v4.1/android-integration
branch
from
July 23, 2026 01:10
8313192 to
48c4726
Compare
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 23, 2026
…otFound Split out of dashpay#4183 (port of dashpay#4060) per review: the unmanaged-identity read fixes are conflict-free on v4.1-dev and should not be blocked on the Keystore rework. The FFI's blanket Option -> result conversion reports an identity the wallet does not manage as PlatformWalletFFIResultCode::NotFound (98), so the Kotlin callers' zero-handle checks were dead and every local read over an unmanaged identity threw instead of returning absence. - Dashpay: route getManagedIdentity through translateManagedIdentityNotFoundToZero so contacts()/syncState()/ payments()/sendContactRequest() treat "not managed" as null/empty/ false; ErrorInvalidHandle still propagates. - Sweep the remaining dead `== 0L` sites: ManagedPlatformWallet .inMemoryIdentityStates (one unmanaged/just-removed id no longer throws through the whole listing) and IdentityRegistration .contestedDpnsNames (the intended "identity is not managed by this wallet" NotFound now actually surfaces). - platform-wallet-ffi: platform_wallet_get_managed_identity keeps the three outcomes distinct (classify_managed_identity_outcome) so a stale/removed wallet surfaces ErrorInvalidHandle and never masquerades as an unmanaged identity; unit tests pin both arms. - DashSdkError: name the code (PLATFORM_WALLET_NOT_FOUND_CODE = 98); the 98 -> DashSdkError.NotFound mapping is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 23, 2026
Split out of dashpay#4183 (port of dashpay#4060) per review: the typed signing error is conflict-free on v4.1-dev and should not be blocked on the Keystore rework. The KeystoreSigner "missing key" completion error travels as free text through Rust and used to come back as an opaque PlatformWallet.Generic (or WalletOperation) failure. It is now built from a shared MESSAGE_MARKER constant and recognized on the Kotlin boundary as the typed DashSdkError.PlatformWallet.SigningKeyUnavailable, so hosts can route users to key repair instead of showing a generic error. The marker is only consulted on the catch-all codes (6 / else), so the dedicated retry-semantics types are never overridden. Known limitation (kept as-is from dashpay#4183 by request): the discriminator is message-text-based (message.contains on the marker); a structured error code across the FFI boundary is follow-up work in the parent PR line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 23, 2026
…as split The inherited dashpay#4172 instrumented tests hardcode the legacy KEYS_ALIAS and the single-alias API; after the alias split the default-policy storage writes under KEYS_ALIAS_AUTH_GATED, so simulating keypair replacement by deleting the legacy alias no longer invalidates anything (the CI failure shumkov diagnosed on storeIfAbsentRederivesWhenTheKeysAliasKeypairWasReplaced). - WalletStorageOwnershipTest: the replaced-keypair, rejected-blob, and no-regeneration probes delete/inspect KEYS_ALIAS_AUTH_GATED; the fingerprint-capture test uses encryptForIdentityKeysAlias (and pins the captured producing alias); the stale-invalidation-cleanup test uses the alias-parameterized deleteIdentityKeysAliasIfCurrentGeneration. - KeystoreSignerInstrumentedTest: canSignWith rejection simulates replacement of the policy alias. The five DashPayUnlockAndSyncTest / WalletManagerRoundTripTest failures from PR dashpay#4183's CI run were NOT test or SDK defects: the branch predated dashpay#4172's workflow hardening (screen_off_timeout / stayon / dismiss-keyguard + the hard deviceLocked=0 guard), so the emulator re-locked mid-run and every MASTER_ALIAS operation threw InvalidKeyException. Rebuilding on the current base inherits the fixed workflow; those tests are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt (1)
1001-1005: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
runCatchingaround a suspend call swallowsCancellationException.reconstructPendingIdentityKeysFromPersistence(...)is asuspendfunction;runCatchingcatchesThrowable, so ifloadPersistedWalletsis cancelled while this best-effort reconstruction is suspended, the cancellation is absorbed and the coroutine keeps going into the wallet-restore loop instead of unwinding. Given this PR already tightenedCancellationExceptionhandling elsewhere, rethrow it here too.♻️ Rethrow cancellation, swallow the rest
- runCatching { - persistenceHandler.reconstructPendingIdentityKeysFromPersistence( - isPrivateKeyDecryptable = { walletStorage.isPrivateKeyDecryptable(it) }, - ) - } + try { + persistenceHandler.reconstructPendingIdentityKeysFromPersistence( + isPrivateKeyDecryptable = { walletStorage.isPrivateKeyDecryptable(it) }, + ) + } catch (c: kotlinx.coroutines.CancellationException) { + throw c + } catch (_: Throwable) { + // Best-effort: a reconstruction failure must not block wallet restore. + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt` around lines 1001 - 1005, Update the runCatching block around reconstructPendingIdentityKeysFromPersistence to rethrow CancellationException while continuing to swallow other failures. Preserve the existing best-effort behavior for non-cancellation exceptions and allow coroutine cancellation to propagate before the wallet-restore loop continues.
🤖 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
`@packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/wallet/WalletKeyHealthSheet.kt`:
- Around line 94-102: Update the state passed from the probe block around
probeIdentityKeyRecoverability to represent key recoverability rather than
keystore-entry presence, and rename the corresponding property and usages in
KeyRow. Preserve the false result for failed probes, but render that state as an
unrecoverable/decryption failure instead of “no Keystore entry,” while retaining
the existing message for genuinely absent entries.
- Around line 94-102: Move the synchronous recoverability probing in the
key-health report producer off the composition thread by wrapping the
report-building work, including probeIdentityKeyRecoverability, in
withContext(Dispatchers.IO), then assign the completed report to produceState’s
value. Keep the existing probe results and fallback behavior unchanged.
---
Nitpick comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt`:
- Around line 1001-1005: Update the runCatching block around
reconstructPendingIdentityKeysFromPersistence to rethrow CancellationException
while continuing to swallow other failures. Preserve the existing best-effort
behavior for non-cancellation exceptions and allow coroutine cancellation to
propagate before the wallet-restore loop continues.
🪄 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 Plus
Run ID: 1d0971c9-bcbb-40ff-8147-c9ef5050c3d3
📒 Files selected for processing (50)
docs/dashpay/KOTLIN_MIGRATION_LEFTOVERS.mddocs/sdk/KOTLIN_SWIFT_SHARED_PARITY_SPEC.mddocs/sdk/sdk-parity-manifest.jsonpackages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/wallet/WalletKeyHealthSheet.ktpackages/kotlin-sdk/PARITY_SUMMARY.mdpackages/kotlin-sdk/sdk/schemas/org.dashfoundation.dashsdk.persistence.DashDatabase/8.jsonpackages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabaseMigrationTest.ktpackages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerInstrumentedTest.ktpackages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/WalletStorageOwnershipTest.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/SignerNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabase.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/PublicKeyDao.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/PublicKeyEntity.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/IdentityKeyPrivateKeyDeriver.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicy.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyUnavailableException.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreKeyGenPolicyTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerCompletionCodeTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.ktpackages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/ManagedIdentityNotFoundTranslationTest.ktpackages/rs-platform-wallet-ffi/src/dashpay.rspackages/rs-platform-wallet-ffi/src/error.rspackages/rs-sdk-ffi/src/signer.rspackages/rs-sdk-ffi/src/test_utils.rspackages/rs-sdk-ffi/src/token/claim.rspackages/rs-sdk-ffi/src/token/config_update.rspackages/rs-sdk-ffi/src/token/destroy_frozen_funds.rspackages/rs-sdk-ffi/src/token/emergency_action.rspackages/rs-sdk-ffi/src/token/freeze.rspackages/rs-sdk-ffi/src/token/mint.rspackages/rs-sdk-ffi/src/token/purchase.rspackages/rs-sdk-ffi/src/token/set_price.rspackages/rs-sdk-ffi/src/token/transfer.rspackages/rs-sdk-ffi/src/token/unfreeze.rspackages/rs-unified-sdk-jni/src/signer.rspackages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swiftpackages/swift-sdk/SwiftTests/SwiftDashSDKTests/ErrorHandlingTests.swift
🚧 Files skipped from review as they are similar to previous changes (36)
- packages/swift-sdk/SwiftTests/SwiftDashSDKTests/ErrorHandlingTests.swift
- packages/rs-sdk-ffi/src/test_utils.rs
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/SignerNative.kt
- packages/rs-sdk-ffi/src/token/emergency_action.rs
- packages/rs-sdk-ffi/src/token/set_price.rs
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/PublicKeyDao.kt
- packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerInstrumentedTest.kt
- packages/rs-sdk-ffi/src/token/claim.rs
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyUnavailableException.kt
- packages/rs-sdk-ffi/src/token/config_update.rs
- packages/rs-sdk-ffi/src/token/transfer.rs
- packages/rs-sdk-ffi/src/token/unfreeze.rs
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift
- packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/ManagedIdentityNotFoundTranslationTest.kt
- docs/sdk/sdk-parity-manifest.json
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt
- packages/rs-sdk-ffi/src/token/mint.rs
- packages/rs-unified-sdk-jni/src/signer.rs
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabase.kt
- packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerCompletionCodeTest.kt
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/PublicKeyEntity.kt
- packages/rs-platform-wallet-ffi/src/error.rs
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicy.kt
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
- packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift
- packages/kotlin-sdk/PARITY_SUMMARY.md
- packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyTest.kt
- packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/WalletStorageOwnershipTest.kt
- packages/rs-platform-wallet-ffi/src/dashpay.rs
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt
- packages/kotlin-sdk/sdk/schemas/org.dashfoundation.dashsdk.persistence.DashDatabase/8.json
- packages/rs-sdk-ffi/src/signer.rs
- packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt
Collaborator
|
Round-3 verification (two independent passes, reconciled): all five previous findings are genuinely fixed — breadcrumb-driven repair with the index parameters removed from the API entirely, fail-closed durable writes, derived-pubkey verification before persistence, narrowed classifier, cancellation rethrow, honest DEVICE_BOUND docs. Two new findings from the deeper pass:
Nit: several new comments carry fix-round narration (issue/finding/reviewer provenance) — keep the invariant text, move the provenance to the PR description. |
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 23, 2026
…re (dashpay#4183) The repair path derived the KEYPAIR and required its public half to equal the key's stored on-chain data before persisting. For ECDSA_HASH160 / EDDSA_25519_HASH160 keys DPP stores the 20-byte HASH160 of the pubkey as that data, not the pubkey, so the raw 33-vs-20-byte contentEquals could never match and those key types were permanently un-repairable. derivedPublicKeyMatches now takes the DPP key-type discriminant and HASH160s the derived pubkey (RIPEMD160(SHA256)) before comparing for HASH160 types; every other type keeps the plain content comparison. keyType is threaded through PrivateKeyDeriver.deriveAndStore; the repair path reads it from the persisted row's breadcrumbs, the store path passes it from the persist callback. Adds a pure-Kotlin Hash160 helper (public bytes only — no derivation/secrets, within the CLAUDE.md doctrine) pinned to RIPEMD-160 reference vectors, and tests proving a HASH160-type key repairs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…re (dashpay#4183) The repair path derived the KEYPAIR and required its public half to equal the key's stored on-chain data before persisting. For ECDSA_HASH160 / EDDSA_25519_HASH160 keys DPP stores the 20-byte HASH160 of the pubkey as that data, not the pubkey, so the raw 33-vs-20-byte contentEquals could never match and those key types were permanently un-repairable. derivedPublicKeyMatches now takes the DPP key-type discriminant and HASH160s the derived pubkey (RIPEMD160(SHA256)) before comparing for HASH160 types; every other type keeps the plain content comparison. keyType is threaded through PrivateKeyDeriver.deriveAndStore; the repair path reads it from the persisted row's breadcrumbs, the store path passes it from the persist callback. Adds a pure-Kotlin Hash160 helper (public bytes only — no derivation/secrets, within the CLAUDE.md doctrine) pinned to RIPEMD-160 reference vectors, and tests proving a HASH160-type key repairs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dashpay#4183) LOCK_SCREEN_KEYGEN_REJECTION_CODES included android.security.KeyStoreException code 4, but 4 is ERROR_INTERNAL_SYSTEM_ERROR — a generic/transient fault, not the no-secure-lock-screen signal (code 3). Classifying it as no-lock-screen silently and permanently downgraded an AUTH_GATED identity key to the weaker DEVICE_BOUND alias on a transient error. Keep only the KeyMint-specific 10309 in the numeric set. A genuine transient internal error now falls through and is rethrown by resolveIdentityKeysWriteAlias as a retryable write failure instead of a security downgrade; the explicit lock-screen message path still classifies real no-LSKF rejections. Adds a negative test that code 4 does not classify, and repoints the existing numeric-code tests at 10309. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ancellation (dashpay#4183) onSigningKeyInvalidated wrapped the durable recordSigningKeyInvalidated write in a bare runCatching {} that swallowed a failed OR cancelled write while the sign still returned typed code 31 — the pending-repair signal was lost silently and a cancelled signer scope was masked. Replace with a try/catch that rethrows CancellationException and, on other failures, logs loudly and rethrows so the signer's own best-effort guard (not this lambda) is the single place that treats bookkeeping failure as non-fatal; the repair stays retryable (durable rows are untouched; the next sign attempt / next load reconstruction re-runs it). Also fix loadPersistedWallets: its bare runCatching around the suspend reconstructPendingIdentityKeysFromPersistence swallowed CancellationException — rethrow it (a best-effort reconstruction failure is still absorbed and logged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ashpay#4183) The WalletKeyHealthSheet recoverability probes (which decrypt each Keystore blob) ran inside produceState on the composition/Main thread; wrap the whole report build in withContext(Dispatchers.IO). The state was named hasPrivateKey and every failed probe was labeled "Missing — no Keystore entry", but the probe can't tell a truly-absent key from a present- but-stranded/undecryptable one. Rename to isRecoverable and relabel ("Unrecoverable — key material missing or stranded; re-derive to repair", summary "Unrecoverable key material") so the UI reflects recoverability, not presence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address the actionable review blockers on dashpay#4183 (keystore rework). Four blockers fixed; three items deliberately left for human decision (see PR notes) because a correct fix is not a minimal, verifiable edit. thepastaclaw blockers: - KeystoreManager.resolveIdentityKeysWriteAlias / effectiveKeySecurityPolicy: probe the CURRENT lock-screen state BEFORE accepting an existing auth-gated alias. Removing the secure lock screen permanently invalidates the auth-gated key's private half while Android retains its public half, so reusing the alias would encrypt new scalars under a key that can never decrypt (and silently bypass requireAuthGated). Both the write path and effective-policy reporting now degrade to DEVICE_BOUND on a now-lockless device. Test updated to assert the corrected semantics. - PlatformWalletPersistenceHandler.reconstructPendingIdentityKeysFromPersistence: the suspend isPrivateKeyDecryptable probe was wrapped in runCatching, which swallowed CancellationException and returned normally. Rethrow cancellation; only genuine probe failures become an unusable result. - PlatformWalletPersistenceHandler.onPersistIdentityKeyRemoval: a removed identity key stayed in pendingIdentityKeys as a phantom repair target. Stage a pending-clear delta with the round (published on commit, discarded on rollback) keyed by (identityId, keyId). Adds commit + rollback tests. shumkov P1 (correctness): - KeystoreSigner: the suspend onSigningKeyInvalidated callback was wrapped in runCatching (swallowing cancellation one layer too low), and the outer catch(Exception) would re-swallow a rethrow. Rethrow CancellationException at both sites so cancellation propagates instead of being masked as a native completion. Validation: :sdk:compileDebugKotlin + :sdk:compileDebugUnitTestKotlin clean; full :sdk:testDebugUnitTest 265 tests, 0 failures. Not applied (need human decision, documented in the PR report): - EDDSA (type-4) key repair: the resolver derive is ECDSA-only, so the Ed25519 public-key comparison can never match; a correct fix needs an Ed25519 pubkey derivation the Kotlin comparison path does not have (FFI change + native rebuild, or an Ed25519 impl matching dalek's seed convention). - Numeric lock-screen classifier (10309): the reviewer's claim that getNumericErrorCode() cannot return 10309 contradicts the existing tests that assert exactly that; the correct Android mechanism (message parse / getInternalErrorCode / a different public constant) cannot be determined without the reviewer's exact comment, and a wrong guess risks a security downgrade misclassification. - error.rs generic-signer misclassification: a classifier-only starts_with change does not close the hole because signer.rs emits a foreign generic-code message verbatim; a correct fix also needs producer-side marker escaping at the completion boundary (coordinated multi-file change, beyond a clean suggestion). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…4183 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion (+ code-31 classification) (dashpay#4183 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eration wrappers Several public signing paths flattened every SDK failure into an operation-specific string variant before the FFI boundary could inspect it: token transfer -> TokenError, DPNS registration and document replace -> InvalidIdentityData. A genuine SigningKeyUnavailable completion still left the reserved machine prefix inside those strings, but the resulting variant reached the FFI catch-all and flattened to ErrorUnknown, losing the host's key-repair routing (code 31). Add platform_wallet::error::preserve_signer_key_unavailable_or, which keeps a structured key-unavailable signer failure verbatim under PlatformWalletError::Sdk (the one shape the FFI maps to ErrorSigningKeyUnavailable) and hands every other error to the caller's stringifying wrapper unchanged. The check is structural and position-0 only -- never a substring sniff -- so a foreign signer that merely mentions the marker is not misrouted into key repair. Route the three named wrapper sites through it. The pure-logic crate mirrors the reserved prefix rather than depend on the FFI crate; a compile-time assertion in platform-wallet-ffi pins the mirror byte-identical to rs-sdk-ffi's canonical constant, so drift is a build failure rather than a silent code-31 regression. Refs: dashpay#4183 review (thepastaclaw), dashpay#4060 finding 7 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…peration wrappers The earlier fix rerouted three stringifying wrappers (token transfer, DPNS register, document replace) through preserve_signer_key_unavailable_or so a structured SigningKeyUnavailable (code 31) survives to the FFI. Their sibling operation wrappers still flattened the same signer failure into TokenError / InvalidIdentityData strings, dropping the typed discriminator. Reroute every remaining signer-bearing stringifying wrapper the same way: - Token ops: burn, mint, freeze, unfreeze, claim, destroy_frozen_funds, pause, resume, purchase, set_price, update_config - Document ops: create, delete, transfer, set_price, purchase - Credit ops: withdraw, transfer_credits, transfer_credits_to_addresses - register_from_addresses (nested so the existing address-nonce promotion is preserved unchanged, code-31 checked first) No unrelated error mapping changed. Paths that already preserve the SDK error via map_err(Sdk) (sdk_writer put_document/send_contact_request, invitation, identity registration) are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
force-pushed
the
port/v4.1/android-integration
branch
from
August 3, 2026 23:27
2cd9483 to
8387858
Compare
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
Resolves the two blocking review findings and the live suggestions on dashpay#4261. Blocking — advance dashpay#3968's reissue frontier to 38. Code 37 is allocated to dashpay#4204, so the branch-specific guidance in the non-conforming section and in the 27/28 detail section could still have sent dashpay#3968 to 37 and recreated the collision with ErrorShieldedInviteAlreadyClaimed. Both references now say 38+, and both state that the reserved 28 and 30 are not available either. Blocking — 28 and 30 were labelled RESERVED in the table and the frontier paragraph but "free" in the code-30 section and the collision history. Rule 1 lets a contributor claim a gap this file marks free, so the two labels gave two different allocation frontiers. Everything now says reserved-not-free and cites rule 1 explicitly. Open-PR inventory: verified 2026-08-03 against each PR's file list and the error.rs at its head. Adds dashpay#3417, dashpay#3549, dashpay#3992 and dashpay#4243 (all touch the crate without claiming an integer; dashpay#4243 maps new wallet errors onto the existing ErrorInvalidParameter). Removes dashpay#4240 and dashpay#4251, whose heads touch no file under this crate; removes dashpay#4258, merged on 2026-08-03; records dashpay#4264 as closed with its work carried by dashpay#4243. Also in this pass: - dashpay#4204's Swift mirror is recorded as PARTIALLY fixed. At d78b940 the raw case and its init(ffi:) arm exist, but PlatformWalletError has no typed case and its exhaustive init(result:) has no arm, so the Swift package does not compile as written. Rule 5's Swift clause is not yet satisfied there. - The preamble no longer claims duplicate discriminants are always silent. Two different names on one integer is an E0081 after a merge (that is how the code-32 collision surfaced); the silent case is a meaning moving to a new integer, or an un-updated host mirror. - Code 13 ErrorArithmeticOverflow does have an in-tree producer (shielded_send.rs); the row said it had none. - dashpay#3968 is no longer described as simply un-rebased: its head does contain the 2026-08-01 base, it is behind the current base, and a rebase alone fixes nothing because git sees no conflict — the branch must edit its own enum. - Provenance re-read on 2026-08-03 from GitHub rather than carried forward, now as a table with a note per PR. dashpay#4183 and dashpay#4184 were rebased onto 5d68612 today and keep 31 and 29 respectively; their stale in-enum reservation comments were corrected in the same rebase, so the code-30 section now lists dashpay#4204 as the only branch still carrying one. - markdownlint: MD018 (18 occurrences) and MD004 (mixed bullet styles) are both clean; MD013 is down to long table rows only.
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…otFound Split out of dashpay#4183 (port of dashpay#4060) per review: the unmanaged-identity read fixes are conflict-free on v4.1-dev and should not be blocked on the Keystore rework. The FFI's blanket Option -> result conversion reports an identity the wallet does not manage as PlatformWalletFFIResultCode::NotFound (98), so the Kotlin callers' zero-handle checks were dead and every local read over an unmanaged identity threw instead of returning absence. - Dashpay: route getManagedIdentity through translateManagedIdentityNotFoundToZero so contacts()/syncState()/ payments()/sendContactRequest() treat "not managed" as null/empty/ false; ErrorInvalidHandle still propagates. - Sweep the remaining dead `== 0L` sites: ManagedPlatformWallet .inMemoryIdentityStates (one unmanaged/just-removed id no longer throws through the whole listing) and IdentityRegistration .contestedDpnsNames (the intended "identity is not managed by this wallet" NotFound now actually surfaces). - platform-wallet-ffi: platform_wallet_get_managed_identity keeps the three outcomes distinct (classify_managed_identity_outcome) so a stale/removed wallet surfaces ErrorInvalidHandle and never masquerades as an unmanaged identity; unit tests pin both arms. - DashSdkError: name the code (PLATFORM_WALLET_NOT_FOUND_CODE = 98); the 98 -> DashSdkError.NotFound mapping is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
Split out of dashpay#4183 (port of dashpay#4060) per review: the typed signing error is conflict-free on v4.1-dev and should not be blocked on the Keystore rework. The KeystoreSigner "missing key" completion error travels as free text through Rust and used to come back as an opaque PlatformWallet.Generic (or WalletOperation) failure. It is now built from a shared MESSAGE_MARKER constant and recognized on the Kotlin boundary as the typed DashSdkError.PlatformWallet.SigningKeyUnavailable, so hosts can route users to key repair instead of showing a generic error. The marker is only consulted on the catch-all codes (6 / else), so the dedicated retry-semantics types are never overridden. Known limitation (kept as-is from dashpay#4183 by request): the discriminator is message-text-based (message.contains on the marker); a structured error code across the FFI boundary is follow-up work in the parent PR line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…as split The inherited dashpay#4172 instrumented tests hardcode the legacy KEYS_ALIAS and the single-alias API; after the alias split the default-policy storage writes under KEYS_ALIAS_AUTH_GATED, so simulating keypair replacement by deleting the legacy alias no longer invalidates anything (the CI failure shumkov diagnosed on storeIfAbsentRederivesWhenTheKeysAliasKeypairWasReplaced). - WalletStorageOwnershipTest: the replaced-keypair, rejected-blob, and no-regeneration probes delete/inspect KEYS_ALIAS_AUTH_GATED; the fingerprint-capture test uses encryptForIdentityKeysAlias (and pins the captured producing alias); the stale-invalidation-cleanup test uses the alias-parameterized deleteIdentityKeysAliasIfCurrentGeneration. - KeystoreSignerInstrumentedTest: canSignWith rejection simulates replacement of the policy alias. The five DashPayUnlockAndSyncTest / WalletManagerRoundTripTest failures from PR dashpay#4183's CI run were NOT test or SDK defects: the branch predated dashpay#4172's workflow hardening (screen_off_timeout / stayon / dismiss-keyguard + the hard deviceLocked=0 guard), so the emulator re-locked mid-run and every MASTER_ALIAS operation threw InvalidKeyException. Rebuilding on the current base inherits the fixed workflow; those tests are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…-> 37 and mirror it (dashpay#4204) 32 is allocated to `ErrorTransactionBuild` (dashpay#4247, also carried by dashpay#4256) in ERROR_CODE_REGISTRY.md (dashpay#4261). This variant took 32 without a registry row, so the two collide as a hard `E0081: discriminant value 32 assigned more than once` the moment both land — reproduced on a real integration merge, not hypothetical. 27-36 are all claimed (27 ErrorShutdownIncomplete via the merged dashpay#4268; 29 dashpay#4184; 31 dashpay#4183; 32/33 37 is the allocation frontier. The code was also unmirrored on BOTH hosts, which is the more dangerous half: Swift is exhaustive, so it surfaced as .errorUnknown and lost its identity; Kotlin fell through to Generic(32), and in any tree carrying "shielded invite already claimed" as "reservation wallet mismatch". That matters on the claim-recovery path specifically — the error is raised from four sites in shielded/operations.rs, three inside the recovery function. Adds the typed Kotlin PlatformWallet.ShieldedInviteAlreadyClaimed (terminal, inherited isRetryable = false), the Swift enum case + init(ffi:) arm, a DashSdkErrorTest assertion pinning 37, and refreshes the stale Swift reservation comment the registry asked the next toucher to drop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
Rebased onto
|
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
… sites Three more live review findings, all verified against current PR heads. Rule 5 named a `PlatformWalletResultCode.init(result:)` that does not exist — `init(result:)` belongs to the downstream `PlatformWalletError`. As written, a contributor could add the Swift raw case and the typed error handling and still omit `PlatformWalletResultCode.init(ffi:)`, which is where the generated C constant is recognised; that switch has a `default:` yielding `.errorUnknown`, so the omission compiles and silently loses the code's identity before typed handling sees it. Rule 5 now enumerates all three Swift sites and says how each one fails: (1) the raw case, (2) the `init(ffi:)` arm — silent, and (3) `PlatformWalletError` + its `init(result:)` arm — a hard compile error, since that switch is exhaustive with no `default:`. That third failure is exactly what dashpay#4204 is sitting on at `d78b940a03`. dashpay#4196 is no longer blocked. Its head moved to `12492e8c54`, the restack onto dashpay#4185 is done, dashpay#4185's head `8813e98533` is an ancestor, the trio reads 34/35/36, and the PR is MERGEABLE against v4.2-dev. Verified the numeric references it owns were carried too: the `StaleReservationToken` KDoc and `fromPlatformWalletNative` mapping in `DashSdkError.kt` both read 34, and the V2 broadcast KDoc in `ManagedCoreWallet.kt` reads 34 with the rest symbolic. `PlatformWalletError::StaleReservation` refers to the code symbolically and never carried a number. The section is now a resolution rather than an open item; the account of why the restack was hard is kept, since that was the substance of the delay. The code-30 sweep was overstated. "No PR anywhere defines a code 30" is false for the surveyed heads — dashpay#4185 and dashpay#4256 both did; that was the allocation, not a competing claim. It now reads "no PR unrelated to dashpay#4185 defines a code 30", which is the claim that actually supports the conclusion. The list of branches carrying the stale consent-code reservation is corrected to dashpay#4183, dashpay#4204 and dashpay#4256's pre-renumber rationale (dashpay#4247 was never one of them). Provenance and the proposed table pick up dashpay#4196's new head. markdownlint MD018/MD004 remain at 0.
QuantumExplorer
approved these changes
Aug 4, 2026
This was referenced Aug 4, 2026
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 4, 2026
…-> 37 and mirror it (dashpay#4204) 32 is allocated to `ErrorTransactionBuild` (dashpay#4247, also carried by dashpay#4256) in ERROR_CODE_REGISTRY.md (dashpay#4261). This variant took 32 without a registry row, so the two collide as a hard `E0081: discriminant value 32 assigned more than once` the moment both land — reproduced on a real integration merge, not hypothetical. 27-36 are all claimed (27 ErrorShutdownIncomplete via the merged dashpay#4268; 29 dashpay#4184; 31 dashpay#4183; 32/33 37 is the allocation frontier. The code was also unmirrored on BOTH hosts, which is the more dangerous half: Swift is exhaustive, so it surfaced as .errorUnknown and lost its identity; Kotlin fell through to Generic(32), and in any tree carrying "shielded invite already claimed" as "reservation wallet mismatch". That matters on the claim-recovery path specifically — the error is raised from four sites in shielded/operations.rs, three inside the recovery function. Adds the typed Kotlin PlatformWallet.ShieldedInviteAlreadyClaimed (terminal, inherited isRetryable = false), the Swift enum case + init(ffi:) arm, a DashSdkErrorTest assertion pinning 37, and refreshes the stale Swift reservation comment the registry asked the next toucher to drop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 4, 2026
…pay#4268 claimed dashpay#4268 merged `ErrorShutdownIncomplete = 27` into the v4.2-dev FFI ABI, colliding with this PR's `ErrorStaleReservationToken = 27`. Renumber the deferred build/broadcast trio to the contiguous block 34-36, which sits above every code currently claimed by a merged commit or an open PR: 27 ErrorShutdownIncomplete MERGED, dashpay#4268 29 ErrorAssetLockInsufficientFunds dashpay#4184 31 ErrorSigningKeyUnavailable dashpay#4183, dashpay#4259 32 ErrorTransactionBuild dashpay#4247, dashpay#4256 33 ErrorTransactionSigning dashpay#4256 28 and 30 are vacated and return to the free pool. Applied across the Rust enum, the FFI/JNI rustdoc, the Kotlin mapping + KDoc + tests, and the Swift mirror (which has no compile-time cross-ABI check, so it was verified by grep). Also addresses three review suggestions: * `PlatformWalletInfo::generation` is now `pub(crate)`. It was publicly assignable through `state_mut()` / `state_mut_blocking()`, so downstream safe code could swap the `Arc` while `PlatformWallet` and `CoreWallet` kept the original — splitting the generation identity `Arc::ptr_eq` compares, which would make `is_current_generation()` reject a live wallet, turn generation-bound reservation cleanup into a no-op, and let teardown exclude through a different lifecycle gate than the payments it must fence. All construction and mutation sites are already inside the crate. * `buildSignedPayment` now runs under `opWithCleanupOnCancellation`. Native finalization mints the token before the blocking JNI call returns, so `withContext`'s prompt-cancellation handoff could discard the completed `SignedCoreTransaction` and leave the reservation to the GC Cleaner or the TTL. The discarded result is now closed deterministically. * Native code 26 (`ErrorTransactionBroadcastRejected`) no longer falls through to `PlatformWallet.Generic`. It maps to a dedicated `TransactionBroadcastRejected` subtype so callers can tell a definitively rejected, consumed-and-released payment (rebuild it) from an unrelated generic wallet failure, with its non-retry-in-place semantics pinned in `DashSdkErrorTest`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 4, 2026
Re-verified the whole document against the CURRENT `origin/v4.2-dev` (`97904ed2fc`), not the `f53e5eef0a` the review comment cited and not the `5d68612a45` this file was last compiled against. `ErrorSigningKeyUnavailable = 31` is merged ABI. It landed in `189a3abb1c` (dashpay#4183, stacked on dashpay#4191) together with its Rust C-facing discriminant and complete Swift and Kotlin mirrors — the raw case, the `init(ffi:)` arm, the typed `PlatformWalletError` case with its `init(result:)` arm, and Kotlin's `31 -> PlatformWallet.SigningKeyUnavailable`. Leaving it under "Proposed allocations", whose preamble explicitly permits renumbering, contradicted rule 3. Moved to the merged table. Four PRs merged into `v4.2-dev` on 2026-08-04 and this file still treated all four as open: dashpay#4191 (`0e2282b586`), dashpay#4183 (`189a3abb1c`), dashpay#4277 (`6704a41a85`), dashpay#4251 (`7afc8a8ff3`). Only dashpay#4183 claimed an integer; the other three claimed none, and dashpay#4277 is now recorded as the merged precedent for "touches error.rs but allocates nothing" (it routes TxMetadataPayloadTooLarge onto the existing ErrorInvalidParameter). Dependent sections updated so nothing implies 31 may still move: the frontier breakdown (unchanged at 38), the proposed table, the inherited-code table (31 is trunk now, not an inheritable claim), the collision-history bullet list, the no-new-code open-PR inventory, the 31-vs-33 note (collapsing 31 is no longer available; only dashpay#4256's 33 is still open), and the survey provenance plus the PR-heads-of-record table. Also refreshed, because a re-dated provenance section must not carry claims that are now false: dashpay#4204's and dashpay#4256's Swift mirror gaps are both closed, and the stale ErrorAssetLockCrossDomainConsentRequired comments are gone from every branch that carried them. Every discriminant, mirror, PR state, and SHA above was read from git or the GitHub API on 2026-08-04. The four merge SHAs were confirmed ancestors of `97904ed2fc`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HashEngineering
added a commit
to HashEngineering/platform
that referenced
this pull request
Aug 4, 2026
…code 31 from message signing Review finding on dashpay#4259 (CodeRabbit, Major). Investigated and found NOT fixable at the producer as described; recording the evidence in-code and pinning it with a test so it does not have to be re-derived. The finding asks `sign_message` to route signer key-unavailable failures through `preserve_signer_key_unavailable_or` before building `MessageSigningFailed`. That helper does exist (dashpay#4183 shipped it in this crate) and is the right tool — for the STATE-TRANSITION signing paths, whose failures are `dash_sdk::Error`, which is where document replace, DPNS and token transfer already use it. Message signing is a different surface. It calls key-wallet's `Signer::sign_ecdsa`, whose error is the associated type `S::Error`, bounded only by `Display + Send + Sync + 'static`. There is no enum to match, and passing it to the helper does not compile: error[E0308]: mismatched types expected enum `dash_sdk::Error` Nor does a producer exist. The only production impl, `rs_sdk_ffi::MnemonicResolverCoreSigner`, has `Error = MnemonicResolverSignerError` — a typed enum that never stamps the reserved marker; its `NotFound` ("mnemonic not found in keychain") IS the key-unavailable case but is indistinguishable once `Display`ed. The marker is produced only in rs-sdk-ffi's state-transition completion callback (`SignResult = Result<Vec<u8>, ProtocolError>`), never on a `sign_ecdsa` path. So a position-0 check would match nothing today, and a `contains` check is the substring sniff dashpay#4183's review rejected. Corrects an earlier note of mine that blamed the marker constant's visibility: it IS visible here now, mirrored as `SIGNER_KEY_UNAVAILABLE_PREFIX` and pinned byte-identical by a compile-time assertion. The blocker is the error type. Closing it needs an upstream change — the signer rendering its key-unavailable variants with the marker at position 0, or key-wallet tightening `Signer::Error` to something matchable. Both touch shared, externally-consumed surfaces and want their own review. `signer_key_unavailable_is_not_preserved_during_message_signing` pins the current behaviour and shows the mechanism: the marker survives but mid-string, which is precisely why a position-0 match cannot recover it. It is written to FAIL if an upstream fix ever makes code 31 reachable, with instructions to flip it. The existing fall-through test continues to pin genuinely-internal failures, and capability refusals still take the `MessageSigningFailed` path unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 4, 2026
…-> 37 and mirror it (dashpay#4204) 32 is allocated to `ErrorTransactionBuild` (dashpay#4247, also carried by dashpay#4256) in ERROR_CODE_REGISTRY.md (dashpay#4261). This variant took 32 without a registry row, so the two collide as a hard `E0081: discriminant value 32 assigned more than once` the moment both land — reproduced on a real integration merge, not hypothetical. 27-36 are all claimed (27 ErrorShutdownIncomplete via the merged dashpay#4268; 29 dashpay#4184; 31 dashpay#4183; 32/33 37 is the allocation frontier. The code was also unmirrored on BOTH hosts, which is the more dangerous half: Swift is exhaustive, so it surfaced as .errorUnknown and lost its identity; Kotlin fell through to Generic(32), and in any tree carrying "shielded invite already claimed" as "reservation wallet mismatch". That matters on the claim-recovery path specifically — the error is raised from four sites in shielded/operations.rs, three inside the recovery function. Adds the typed Kotlin PlatformWallet.ShieldedInviteAlreadyClaimed (terminal, inherited isRetryable = false), the Swift enum case + init(ffi:) arm, a DashSdkErrorTest assertion pinning 37, and refreshes the stale Swift reservation comment the registry asked the next toucher to drop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
QuantumExplorer
added a commit
that referenced
this pull request
Aug 4, 2026
Two conflicts, both "each side added an arm", resolved by keeping both: - `packages/rs-platform-wallet-ffi/src/error.rs`: the txMetadata caller-input arms (payload size / wire version / encryptionKeyIndex -> ErrorInvalidParameter) and v4.2-dev's structural `Sdk(Protocol(Generic))` signer-prefix arm (#4183) both sit before the `_` catch-all. The signer arm stays last so it keeps its catch-all-only role. - `DashSdkError.kt`: this branch's typed `2 -> PlatformWallet.InvalidParameter` arm plus v4.2-dev's code-6 signer-marker fallback. Note v4.2-dev merged the encrypted-txMetadata foundation as #4277 and then reverted it in #4279, so the net base diff for those files is empty and the full stack is carried here.
HashEngineering
added a commit
to HashEngineering/platform
that referenced
this pull request
Aug 4, 2026
…de-31 promotion the FFI does not perform The variant doc still carried the qa3-era claim that a signer's typed key-unavailable completion gets promoted to ErrorSigningKeyUnavailable by the FFI catch-all — the opposite of the documented behavior everywhere else after the dashpay#4183 convergence. Reworded to state the real contract: signer failures fall through to ErrorUnknown, only address-resolution key-unavailable reaches code 31, with pointers to the FFI conversion NOTE that carries the full type chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #4191 (
port/v4.1/unmanaged-identity-reads) — review only thecommits above
ce3a17d71f. Once #4191 merges,git rebase --onto origin/v4.1-dev ce3a17d71fand force-push with--force-with-lease.This replaces the old single-squash
port/v4.1/android-integration(preserved at
backup/port-4060-pre-rework). That squash predated #4172 andsilently rewrote the exact regions #4172 later hardened; this branch is a
rebuild on the current base with each review finding resolved explicitly.
The unmanaged-identity reads and the text-classified SigningKeyUnavailable
from the old squash shipped separately as #4191 and are NOT duplicated here.
Scope — Keystore only, one logical commit per concern:
KeySecurityPolicy(AUTH_GATED default /DEVICE_BOUND opt-in), two dedicated RSA aliases, legacy
KEYS_ALIASread-only; fix(kotlin-sdk): harden identity-key Keystore recovery and reconcile parity docs #4172's fingerprint capture + invalidation recovery survive
alias-parameterized (finding 1).
redirect to the DEVICE_BOUND alias (never a silently gate-less key under
the auth-gated alias);
effectiveKeySecurityPolicy()+requireAuthGatedstrict mode; per-blob alias tag routes reads;tightened KeyMint failure classifier (finding 4).
never applies to legacy blobs, finding 2), fingerprint fast path with
KPIE rethrow-after-generation-checked-cleanup (brick-loop fix, finding 1),
mismatch/missing fp routes into former-RSA recovery, migration forward.
Cheap
canSignWithnever decrypts; the real-decrypt probe moved toprobeIdentityKeyRecoverability(finding 3).pendingIdentityKeysStateFlow,round-transactional staging.
restart reconstruction incl. blobs stranded by keypair replacement
(finding 5).
replacePrivateKey+forcederive path +real-decrypt verification before
markIdentityKeyRepaired(finding 6).PlatformWallet.NotFound(Swift parity; releasenote in the commit).
error_codeon the signcompletion ABI,
ErrorSigningKeyUnavailable = 31(26-30 reserved forfeat(kotlin-sdk): split build/broadcast with reservation release for BIP70-style deferred submission #4185/fix: shield asset-lock funding from all funds accounts incl. CoinJoin (#4073) #4184), Kotlin/Swift typed mappings; MESSAGE_MARKER sniff retained as a
deprecated fallback for the fix(kotlin-sdk): unmanaged-identity reads return absence + typed SigningKeyUnavailable (split from #4183) #4191 merge-order transition (mixed
old-native/new-Kotlin artifacts are unsupported — the completion JNI
arity changed) (finding 7). Known residual: the
Rust-internal segment rides a stable machine prefix through
ProtocolError::Generic— a typed rs-dpp variant was deliberatelyavoided (serialization blast radius).
InvalidKeyException failures were the pre-fix(kotlin-sdk): harden identity-key Keystore recovery and reconcile parity docs #4172 emulator-relock flake —
inherited workflow guard covers them).
convergent, pending-repair gap recorded for Swift, code-31 capability),
migration ladder v8 note.
CI expectations: emulator job runs
:sdk:connectedDebugAndroidTest(watchthe
deviceLocked=0guard); swift job rebuilds the xcframework so the5-arg completion ABI and code 31 compile against fresh headers.
Round-3 addendum: verification passes found and fixed three repair-routing defects (health-probe fingerprint disproof for replaced aliases, typed first-attempt invalidation errors, durable repair seeding for legacy and former-RSA keys — commits
6d1fa5be83,f6fe865e56,7ee6e4b7f9,ce3490a3a0). Unit suite: 249 passing; instrumented suite runs on this repo's emulator job.🤖 Generated with Claude Code
Summary by CodeRabbit