fix: shield asset-lock funding from all funds accounts incl. CoinJoin (#4073) - #4184
Open
bfoss765 wants to merge 17 commits into
Open
fix: shield asset-lock funding from all funds accounts incl. CoinJoin (#4073)#4184bfoss765 wants to merge 17 commits into
bfoss765 wants to merge 17 commits into
Conversation
bfoss765
requested review from
QuantumExplorer,
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
July 21, 2026 12:41
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughShielded asset-lock funding now supports an optional BIP32 funding path that restricts UTXO selection to one account. Typed insufficient-funds errors propagate through wallet, FFI, JNI, Kotlin, and Swift APIs. The Swift example app accepts the path for Core funding. ChangesAsset-lock funding path
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SwiftApp
participant SwiftSDK
participant JNI
participant WalletFFI
participant PlatformWallet
SwiftApp->>SwiftSDK: Submit Core asset-lock with fundingPath
SwiftSDK->>JNI: Marshal UTF-8 path
JNI->>WalletFFI: Pass path pointer and length
WalletFFI->>PlatformWallet: Select one funding account
PlatformWallet-->>WalletFFI: Return result code and message
WalletFFI-->>SwiftSDK: Return asset-lock result
Possibly related PRs
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
|
🔍 Review in progress — actively reviewing now (commit 11c3677) |
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 47 minutes. |
Collaborator
|
The fix is correct and still reproduces on tip (the #4073 symptom is live in
Minor: the SwiftExampleApp funding picker still gates on the single BIP44 account, so the UI blocks the exact scenario this fixes (follow-up); the upstream-behavior pin tests (router/gap-limit) will trip on any rust-dashcore pin change — deliberate?; tracker refs ("finding 5b52d9844055") → PR description. |
Collaborator
|
Two additional architecture blockers after checking the existing comments:
The existing typed-error and Swift BIP44-only preflight comments are correct and are not duplicated here. |
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 21, 2026
…vacy-domain funding gate (dashpay#4184) Addresses two must-fix reviewer findings on PR dashpay#4184. 1. FFI error arm for the asset-lock shortfall (dashpay#4073 request 3). `AssetLockInsufficientFunds` never crossed the FFI: with no arm in `From<PlatformWalletError>` it flattened to `ErrorUnknown(99)`, hiding a typed shortfall behind the catch-all and forcing hosts to string-match the Display text. Add: - `ErrorAssetLockInsufficientFunds = 26` (sibling of ErrorCoreInsufficientFunds = 22; existing codes unchanged), mapped in the From impl (the structured available/required duffs still travel in the message), plus a Rust test that the error crosses as code 26 (not 99) with the message verbatim. - Kotlin `DashSdkError.PlatformWallet.AssetLockInsufficientFunds` (26 ->) and Swift `.errorAssetLockInsufficientFunds`; cbindgen emits the C constant. The Display text is UNCHANGED ("asset lock coin selection is short: ...") so dash-wallet's existing substring matcher keeps working while it migrates to the typed code. 2. Privacy-domain co-spend gate. Largest-first could union ordinary BIP44/BIP32, CoinJoin, and DashPay-receiving funds into one L1 tx (with BIP44 change), irreversibly linking those domains. Default funding now stays within a single privacy domain: - Domains: Transparent {BIP44,BIP32} > CoinJoin > DashPay-receiving. Transparent is the only default-eligible domain because it holds the primary account and is the sole source of change (key-wallet derives change only on Standard accounts) — so any non-transparent spend inherently crosses into it. - New `CrossDomainConsent` (Denied default / Allowed opt-in) threaded through the builder, orchestration, `shielded_fund_from_asset_lock`, the JNI bridge, and the `platform_wallet_manager_shielded_fund_from_asset_lock` FFI (`allow_cross_domain: bool`). Wrapper methods keep every existing caller on the safe default. - Cross-domain refusal returns the typed `AssetLockCrossDomainConsentRequired` (FFI code 27; Kotlin/Swift mapped) carrying transparent/union/required duffs. - Watch-only DashpayExternalAccount exclusion preserved via the domain classifier (returns None); identity-funding single-BIP44 carve-out preserved. Tests: single-domain success without consent; cross-domain refused without consent (typed error) and succeeds with consent; existing union/CoinJoin/DashPay tests moved to the consented path. cargo test -p platform-wallet --lib = 504 passed; --features shielded = 634 passed; -p platform-wallet-ffi --lib = 198 passed. clippy clean on all three crates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Both must-fix items and both architecture blockers are addressed and pushed. On the reservation-ledger blocker: implemented upstream as you specified — a rust-dashcore key-wallet PR (opening shortly) adds |
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 35 minutes. |
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 21, 2026
…the signer wire Replace end-to-end message sniffing for the signer's "missing key" failure with a typed discriminator (dashpay#4060 finding 7): - rs-sdk-ffi: DashSDKSignerErrorCode { Generic = 0, SigningKeyUnavailable = 1, AuthenticationFailed = 2 (reserved) }; SignCompletionCallback and dash_sdk_sign_async_completion gain error_code: i32 (before error_message). SignResult stays Result<Vec<u8>, ProtocolError> (a new rs-dpp ProtocolError variant would carry serialization blast radius), so code 1 rides the single Rust-owned machine prefix DASH_SDK_SIGNER_ERR_KEY_UNAVAILABLE_PREFIX through ProtocolError::Generic — typed at both ABI edges, one constant bridging the string segment. This is an internal coordinated ABI change: every piece versions together in this monorepo. - rs-platform-wallet-ffi: PlatformWalletFFIResultCode:: ErrorSigningKeyUnavailable = 31 (codes 26-28 are reserved for dashpay#4185's reservation-token errors and 29/30 for dashpay#4184's asset-lock errors on sibling branches — documented in the enum as dashpay#4184 does). The From<dpp::ProtocolError> conversion restores the typed code from the prefix FIRST (before the loose keyword sniffs), and the From<PlatformWalletError> blanket impl restores it on the catch-all only (dedicated retry-semantics codes are never overridden) — covering the Sdk(dash_sdk::Error::Protocol(..)) wrapping path. - JNI/Kotlin: SignerNative.completeSign(token, signature, errorCode, errorMessage); KeystoreSigner passes SIGNER_ERROR_CODE_KEY_UNAVAILABLE on the null-key branch (keeping the MESSAGE_MARKER text for the transition window) and Generic everywhere else. DashSdkError maps 31 → PlatformWallet.SigningKeyUnavailable; the dashpay#4191 marker sniff on the catch-all codes remains as a deprecated old-native fallback with a removal note tied to the next minor release. - Swift: KeychainSigner trampolines forward the code (missing-row / missing-scalar outcomes classify as 1); PlatformWalletResultCode gains errorSigningKeyUnavailable = 31 → PlatformWalletError .signingKeyUnavailable (Kotlin parity). - Tests: rs-sdk-ffi completion-code tests (prefix present for code 1, absent for generic), platform-wallet-ffi prefix→31 tests on both conversion points, Kotlin code-31 + fallback-marker tests, Swift mapping and trampoline-classifier tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
|
Consolidated re-verification (two independent passes). The pushed substance checks out: typed codes 29/30 with by-ref mapping so the shielded entry point can't drift, Kotlin mappings + tests, the privacy-domain consent gate is thorough (transparent-vs-union precheck, fee-band reclassification, default-deny at every entry), and filing rust-dashcore#912 for the reservation ledger is the right split. The Rust asset-lock suite passes 8/8 locally. Remaining blockers:
Minor: the new |
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 22, 2026
blocker) PlatformWalletResultCode gained cases 29/30 (errorAssetLockInsufficientFunds, errorAssetLockCrossDomainConsentRequired) but PlatformWalletError had no matching cases, so init(result:) — which switches over result.code with no default — became non-exhaustive and the Swift package no longer compiled. Add the two matching cases (assetLockInsufficientFunds, assetLockCrossDomainConsentRequired) to PlatformWalletError, extend the errorDescription associated-value binding, and map both codes in init(result:). Semantics/messages mirror the Kotlin DashSdkError.PlatformWallet counterparts (codes 29/30). Verified: swiftc -typecheck of PlatformWalletResult.swift against a stub DashSDKFFI module built from the cbindgen-generated header now passes; removing the fix reproduces "switch must be exhaustive". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 22, 2026
ShieldedFundFromAssetLockView built its funding picker from BIP44 accounts only and never set allowCrossDomain, so the all-funds/cross-domain flow could not be exercised or consented to on iOS. Add a minimal "Allow cross-domain funds" toggle (transparent-only by default) wired through to shieldedFundFromAssetLock(allowCrossDomain:), so the code-30 gate is reachable. A tracked TODO on crossDomainConsentSection records the intended fuller UX (submit false, catch errorAssetLockCrossDomainConsentRequired, show the transparent/union/ required breakdown, then retry with true) as a follow-up to file — the PR is held for rust-dashcore#912 before that lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 22, 2026
dashpay#4184) - Remove the "finding <hash>" tracker references from build.rs comments (rationale text kept); these belong in the PR description, not the source. - Add an ABI/release-note line to the platform-wallet-ffi README recording the C-ABI break: platform_wallet_manager_shielded_fund_from_asset_lock gained a trailing `bool allow_cross_domain` parameter, plus result codes 29/30. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 22, 2026
`PlatformWalletResultCode` jumped from 25 straight to 98, so the three deferred build->broadcast/release codes this PR owns (26 StaleReservationToken, 27 ReservationTokenConsumed, 28 ReservationWalletMismatch) fell through to `.errorUnknown` on iOS, erasing their distinct retry semantics. Add the three raw codes to `PlatformWalletResultCode`, matching cases to `PlatformWalletError`, and map them in both `init(ffi:)` and `init(result:)`. The `init(result:)` switch (no default) stays exhaustive — the same non-exhaustive-switch class shumkov flagged on dashpay#4184. Messages pass the Rust `Display` string straight through, matching the Kotlin SDK's mapping verbatim. Verified with `swiftc -parse` (the DashSDKFFI xcframework — cbindgen header + cdylib — is built separately by build_ios.sh and is not present in this checkout, so a full `swift build` type-check isn't possible here). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![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/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ShieldedFundFromAssetLockView.swift`:
- Around line 275-302: Update the canSubmit logic in
ShieldedFundFromAssetLockView so enabling allowCrossDomain no longer requires
the selected BIP44 balance to cover the full lock amount. Permit submission when
cross-domain consent is enabled, while preserving the existing balance
validation when it is disabled and letting Rust perform the authoritative
union-funds check.
🪄 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: 3eef4ea0-742b-45b1-883c-1ec0700a620b
📒 Files selected for processing (19)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/FundingNative.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/rs-platform-wallet-ffi/README.mdpackages/rs-platform-wallet-ffi/src/error.rspackages/rs-platform-wallet-ffi/src/shielded_send.rspackages/rs-platform-wallet/src/error.rspackages/rs-platform-wallet/src/lib.rspackages/rs-platform-wallet/src/test_support.rspackages/rs-platform-wallet/src/wallet/asset_lock/build.rspackages/rs-platform-wallet/src/wallet/asset_lock/mod.rspackages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rspackages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rspackages/rs-platform-wallet/src/wallet/shielded/seed_pool.rspackages/rs-unified-sdk-jni/src/funding.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedFunding.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ShieldedFundFromAssetLockView.swift
Contributor
Author
|
Addressed the independent findings; the reservation race stays held for rust-dashcore#912.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rs-platform-wallet-ffi/src/shielded_send.rs (1)
1079-1085: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale error-reference comment.
AssetLockCrossDomainConsentRequiredis no longer present inPlatformWalletError/ the FFI code mapping, so this comment should only coverAssetLockInsufficientFundsand drop the cross-domain consent rationale.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-platform-wallet-ffi/src/shielded_send.rs` around lines 1079 - 1085, The error-preservation comment in the result handling block should no longer reference AssetLockCrossDomainConsentRequired or cross-domain consent behavior. Update it to document only preservation of AssetLockInsufficientFunds as a dedicated FFI error code, while retaining the existing generic-error distinction and message-prefix rationale.
🤖 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/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs`:
- Line 153: Limit shielded asset-lock funding to the primary account until
reservation bookkeeping supports the selected funding_path, or carry that
account/path through every reservation and release operation in the asset-lock
build flow. Update the funding_path handling in the shielded builder and the
reservation logic in build.rs so rejected or concurrent secondary-account builds
cannot reserve, release, or reselect inputs under the primary
BIP44/account-index path.
---
Outside diff comments:
In `@packages/rs-platform-wallet-ffi/src/shielded_send.rs`:
- Around line 1079-1085: The error-preservation comment in the result handling
block should no longer reference AssetLockCrossDomainConsentRequired or
cross-domain consent behavior. Update it to document only preservation of
AssetLockInsufficientFunds as a dedicated FFI error code, while retaining the
existing generic-error distinction and message-prefix rationale.
🪄 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: 850ec629-a6b8-4066-93fd-aafdb3f1dd72
📒 Files selected for processing (19)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/FundingNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.ktpackages/rs-platform-wallet-ffi/src/asset_lock/build.rspackages/rs-platform-wallet-ffi/src/error.rspackages/rs-platform-wallet-ffi/src/shielded_send.rspackages/rs-platform-wallet/src/error.rspackages/rs-platform-wallet/src/wallet/asset_lock/build.rspackages/rs-platform-wallet/src/wallet/asset_lock/orchestration.rspackages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rspackages/rs-platform-wallet/src/wallet/identity/network/invitation.rspackages/rs-platform-wallet/src/wallet/identity/network/registration.rspackages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rspackages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rspackages/rs-platform-wallet/src/wallet/shielded/seed_pool.rspackages/rs-unified-sdk-jni/src/funding.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedFunding.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ShieldedFundFromAssetLockView.swift
💤 Files with no reviewable changes (3)
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift
- packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
- packages/rs-platform-wallet-ffi/src/error.rs
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 23, 2026
…the signer wire Replace end-to-end message sniffing for the signer's "missing key" failure with a typed discriminator (dashpay#4060 finding 7): - rs-sdk-ffi: DashSDKSignerErrorCode { Generic = 0, SigningKeyUnavailable = 1, AuthenticationFailed = 2 (reserved) }; SignCompletionCallback and dash_sdk_sign_async_completion gain error_code: i32 (before error_message). SignResult stays Result<Vec<u8>, ProtocolError> (a new rs-dpp ProtocolError variant would carry serialization blast radius), so code 1 rides the single Rust-owned machine prefix DASH_SDK_SIGNER_ERR_KEY_UNAVAILABLE_PREFIX through ProtocolError::Generic — typed at both ABI edges, one constant bridging the string segment. This is an internal coordinated ABI change: every piece versions together in this monorepo. - rs-platform-wallet-ffi: PlatformWalletFFIResultCode:: ErrorSigningKeyUnavailable = 31 (codes 26-28 are reserved for dashpay#4185's reservation-token errors and 29/30 for dashpay#4184's asset-lock errors on sibling branches — documented in the enum as dashpay#4184 does). The From<dpp::ProtocolError> conversion restores the typed code from the prefix FIRST (before the loose keyword sniffs), and the From<PlatformWalletError> blanket impl restores it on the catch-all only (dedicated retry-semantics codes are never overridden) — covering the Sdk(dash_sdk::Error::Protocol(..)) wrapping path. - JNI/Kotlin: SignerNative.completeSign(token, signature, errorCode, errorMessage); KeystoreSigner passes SIGNER_ERROR_CODE_KEY_UNAVAILABLE on the null-key branch (keeping the MESSAGE_MARKER text for the transition window) and Generic everywhere else. DashSdkError maps 31 → PlatformWallet.SigningKeyUnavailable; the dashpay#4191 marker sniff on the catch-all codes remains as a deprecated old-native fallback with a removal note tied to the next minor release. - Swift: KeychainSigner trampolines forward the code (missing-row / missing-scalar outcomes classify as 1); PlatformWalletResultCode gains errorSigningKeyUnavailable = 31 → PlatformWalletError .signingKeyUnavailable (Kotlin parity). - Tests: rs-sdk-ffi completion-code tests (prefix present for code 1, absent for generic), platform-wallet-ffi prefix→31 tests on both conversion points, Kotlin code-31 + fallback-marker tests, Swift mapping and trampoline-classifier tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Jul 23, 2026
…vacy-domain funding gate (dashpay#4184) Addresses two must-fix reviewer findings on PR dashpay#4184. 1. FFI error arm for the asset-lock shortfall (dashpay#4073 request 3). `AssetLockInsufficientFunds` never crossed the FFI: with no arm in `From<PlatformWalletError>` it flattened to `ErrorUnknown(99)`, hiding a typed shortfall behind the catch-all and forcing hosts to string-match the Display text. Add: - `ErrorAssetLockInsufficientFunds = 26` (sibling of ErrorCoreInsufficientFunds = 22; existing codes unchanged), mapped in the From impl (the structured available/required duffs still travel in the message), plus a Rust test that the error crosses as code 26 (not 99) with the message verbatim. - Kotlin `DashSdkError.PlatformWallet.AssetLockInsufficientFunds` (26 ->) and Swift `.errorAssetLockInsufficientFunds`; cbindgen emits the C constant. The Display text is UNCHANGED ("asset lock coin selection is short: ...") so dash-wallet's existing substring matcher keeps working while it migrates to the typed code. 2. Privacy-domain co-spend gate. Largest-first could union ordinary BIP44/BIP32, CoinJoin, and DashPay-receiving funds into one L1 tx (with BIP44 change), irreversibly linking those domains. Default funding now stays within a single privacy domain: - Domains: Transparent {BIP44,BIP32} > CoinJoin > DashPay-receiving. Transparent is the only default-eligible domain because it holds the primary account and is the sole source of change (key-wallet derives change only on Standard accounts) — so any non-transparent spend inherently crosses into it. - New `CrossDomainConsent` (Denied default / Allowed opt-in) threaded through the builder, orchestration, `shielded_fund_from_asset_lock`, the JNI bridge, and the `platform_wallet_manager_shielded_fund_from_asset_lock` FFI (`allow_cross_domain: bool`). Wrapper methods keep every existing caller on the safe default. - Cross-domain refusal returns the typed `AssetLockCrossDomainConsentRequired` (FFI code 27; Kotlin/Swift mapped) carrying transparent/union/required duffs. - Watch-only DashpayExternalAccount exclusion preserved via the domain classifier (returns None); identity-funding single-BIP44 carve-out preserved. Tests: single-domain success without consent; cross-domain refused without consent (typed error) and succeeds with consent; existing union/CoinJoin/DashPay tests moved to the consented path. cargo test -p platform-wallet --lib = 504 passed; --features shielded = 634 passed; -p platform-wallet-ffi --lib = 198 passed. clippy clean on all three crates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
blocker) PlatformWalletResultCode gained cases 29/30 (errorAssetLockInsufficientFunds, errorAssetLockCrossDomainConsentRequired) but PlatformWalletError had no matching cases, so init(result:) — which switches over result.code with no default — became non-exhaustive and the Swift package no longer compiled. Add the two matching cases (assetLockInsufficientFunds, assetLockCrossDomainConsentRequired) to PlatformWalletError, extend the errorDescription associated-value binding, and map both codes in init(result:). Semantics/messages mirror the Kotlin DashSdkError.PlatformWallet counterparts (codes 29/30). Verified: swiftc -typecheck of PlatformWalletResult.swift against a stub DashSDKFFI module built from the cbindgen-generated header now passes; removing the fix reproduces "switch must be exhaustive". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
ShieldedFundFromAssetLockView built its funding picker from BIP44 accounts only and never set allowCrossDomain, so the all-funds/cross-domain flow could not be exercised or consented to on iOS. Add a minimal "Allow cross-domain funds" toggle (transparent-only by default) wired through to shieldedFundFromAssetLock(allowCrossDomain:), so the code-30 gate is reachable. A tracked TODO on crossDomainConsentSection records the intended fuller UX (submit false, catch errorAssetLockCrossDomainConsentRequired, show the transparent/union/ required breakdown, then retry with true) as a follow-up to file — the PR is held for rust-dashcore#912 before that lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
dashpay#4184) - Remove the "finding <hash>" tracker references from build.rs comments (rationale text kept); these belong in the PR description, not the source. - Add an ABI/release-note line to the platform-wallet-ffi README recording the C-ABI break: platform_wallet_manager_shielded_fund_from_asset_lock gained a trailing `bool allow_cross_domain` parameter, plus result codes 29/30. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…re-scope The dashpay#4184 re-scope dropped AssetLockCrossDomainConsentRequired (code 30) but left references behind: a DashSdkErrorTest mapping (broke :sdk:test compile), the rs-platform-wallet-ffi README ABI note, and a shielded_send.rs comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…ntracts (dashpay#4184) Address maintainer/CodeRabbit review findings on the dashpay#4184 asset-lock funding re-scope. 1. Reserve the selected-account inputs (correctness / double-spend). `build_asset_lock_tx_from_selected_account` seeded inputs via `add_inputs` with NO ReservationSet, so a re-scoped shielded build reserved nothing — a concurrent normal Core send (the BIP44 `build_asset_lock_with_signer` path) or a CoinJoin mix could reselect the same UTXOs in the build->broadcast->reconcile window (the wallet write lock + `shield_guard` only serialize shielded builds). Point the key-wallet builder's `set_funding` at the SELECTED funds account instead of `add_inputs`, so the chosen inputs are reserved in that account's OWN reservation ledger (the same single-account machinery the BIP44 builder uses) and held across build->broadcast. Release on a rejected broadcast is now path-aware (`release_asset_lock_funding_reservation`): shielded funding releases from the account named by `funding_path` (possibly CoinJoin/DashPay); every other funding type from the BIP44 account. Adds two tests: a selected-account build reserves its CoinJoin input across broadcast, and a rejected selected-account broadcast releases it. 2. Restore `WalletOperation` for the generic shielded-funding failure arm (was regressed to `Unknown(99)`): `platform_wallet_error_code_or_wallet_operation` keeps typed codes but maps the catch-all to `ErrorWalletOperation`, at both the fund and resume-fund FFI entry points. 3. JNI: read `fundingPath` with a strict reader that throws on a genuine JNI read error instead of silently degrading a money-source param to the default BIP44 account; JVM null still means "absent". 4. Rewrite the stale multi-account-union/consent docs in `orchestration.rs` (FromWalletBalance) to the as-built single-path design. 5. Swift example: the funding-account picker now surfaces non-Standard funds accounts (CoinJoin/DashPay) with type labels, and `canSubmit` no longer gates on the selected Core account's balance when an explicit `fundingPath` is supplied — so a CoinJoin path is exercisable end-to-end. Validation: cargo build -p platform-wallet -p platform-wallet-ffi -p rs-unified-sdk-jni --features shielded; cargo test -p platform-wallet --features shielded (639 passed); cargo test -p platform-wallet-ffi --features shielded (215+26+6 passed); ./gradlew :sdk:assemble -x lint (BUILD SUCCESSFUL). Swift not built locally (no xcframework). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
review blocker 2) Reviewer (thepastaclaw) blocker 2 — build.rs:423-424: build_asset_lock_tx_from_selected_account passed the BIP44 change account (&bip44_acc) as set_funding's `acc`, but set_funding calls funds_acc.next_change_address(Some(&acc.account_xpub)) on the SELECTED funds account before the set_change_address override. For an explicitly selected Standard BIP32 account with no pre-generated unused internal address, that derived [1, index] from the wrong (BIP44) xpub and recorded it under the BIP32 account's own path, poisoning that pool so a later normal BIP32 send could use a change entry whose signer key does not match the address. Now resolve the wallet-level Account whose account-level derivation path equals funding_path and pass ITS xpub to set_funding, while keeping the separate BIP44 set_change_address override. Default BIP44 funding resolves to bip44_acc (unchanged); non-Standard (CoinJoin/DashPay) accounts fail change derivation regardless, so the xpub is immaterial and the bip44_acc fallback preserves prior behavior. Also (thepastaclaw nitpick, test_support.rs): move the DashPay fixture rustdoc so it attaches to split_funded_wallet_manager_dashpay rather than foreign_contact_account_xpub. Blocker 1 (build.rs:427-473, owner-guarded reservation rollback on the pre-broadcast abandonment path) is NOT addressed here: the pinned key-wallet rev 70d4bf8 exposes no owner-guarded release primitive (ReservationSet has only reserve/reserved/release keyed by outpoint; release_reservation is unconditional) and no reservation token, so the required release_if_owner(token) mechanism (rust-dashcore#916 / dashpay#4185) cannot be implemented against this pin without an upstream dependency change — the very atomic-reservation fix this PR is held for. Deferred pending that adoption rather than substituting an unconditional release on the money path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…adcast path build_signed reserves the selected inputs in the funding account's ReservationSet and normally holds them from build through broadcast. But several post-signing, pre-broadcast error paths could return without rolling that reservation back, stranding the inputs until the reservation-TTL backstop (~24 blocks) and causing false shortfalls / withheld coins in the meantime (dashpay#4184 review, thepastaclaw): - build_asset_lock_tx_from_selected_account (shielded selected-account path): the credit-account lookups, peek_next_path, the external signer.public_key round-trip, and mark_first_pool_index_used are all fallible after build_signed. Derive the credit key in an inner future and, on any error, release the reservation on the selected funds account (matched by funding_path, mirroring release_asset_lock_funding_reservation) before propagating. - build_asset_lock_transaction (non-shielded / upstream builder path): the two defensive credit-key arms after build_asset_lock_with_signer abandon a signed+reserved tx; release on the BIP44 account at account_index. - broadcast_funded_asset_lock invitation durability branch: when the funding-index persist/flush fails we abort before broadcast; release the reservation via release_asset_lock_funding_reservation before bailing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…on window Addresses the outstanding review findings on dashpay#4184 at a9e418a. Reservation lifecycle, delegated (non-shielded) builder — blocker. `build_asset_lock_with_signer` reserves the transaction's inputs inside `build_signed` and rolls that back only when *input* signing fails. Its credit-output loop runs afterwards, and a failure there returns `Err` with a fully-signed transaction abandoned and its inputs still reserved. That reservation cannot be rolled back from platform-wallet at this pin: the error carries no transaction to hand `release_reservation`, and the account's `ReservationSet` is `pub(crate)` to key-wallet. The abandon path is therefore removed rather than rolled back. Of the loop's fallible steps only the external `signer.public_key` round-trip is genuinely reachable — resolving the credit account, `peek_next_path` and `mark_first_pool_index_used` cannot fail once `peek_next_funding_address` has resolved and peeked that same account earlier in the call under the same held wallet write lock. So the signer round-trip now happens up front, before anything is reserved, and the builder receives a `PrefetchedCreditKeySigner` that answers the repeat request from cache. `peek_next_funding_address` returns the peeked path alongside the address to feed it. Standard BIP32 regression coverage. Adds the test the reviewer asked for around the `set_funding` xpub lookup: it exhausts a selected BIP32 account's pre-generated unused internal entries so `next_change_address` must derive a fresh one, builds through that account's explicit path, and asserts every internal-pool entry is still signable at its own recorded path. Reverting the lookup to the BIP44 xpub fails it. SwiftExampleApp funding picker. Picker rows were all tagged with a bare `accountIndex`, which account families reuse — BIP44 #0, BIP32 #0 and CoinJoin #0 collided, so SwiftUI could not tell which row was chosen and a non-BIP44 row without a BIP44 counterpart fed an invalid change account to Rust. The picker is now restricted to BIP44 accounts (the mandatory change sink, listed even at zero balance so the dashpay#4073 case still works) and the other funds accounts are shown read-only. Public API documentation. `account_index` / `fundingAccountIndex` is documented as the change sink that doubles as the default input source, rather than unconditionally as the input source, across the Rust FFI, Swift and Kotlin surfaces — it contradicted the `funding_path` docs immediately below it. Tests: platform-wallet 511/511 (2 new), platform-wallet-ffi 205/205 + 26 + 6; cargo fmt applied; clippy clean on both crates (only pre-existing warnings in recovery.rs / withdrawal.rs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 3, 2026
…the signer wire Replace end-to-end message sniffing for the signer's "missing key" failure with a typed discriminator (dashpay#4060 finding 7): - rs-sdk-ffi: DashSDKSignerErrorCode { Generic = 0, SigningKeyUnavailable = 1, AuthenticationFailed = 2 (reserved) }; SignCompletionCallback and dash_sdk_sign_async_completion gain error_code: i32 (before error_message). SignResult stays Result<Vec<u8>, ProtocolError> (a new rs-dpp ProtocolError variant would carry serialization blast radius), so code 1 rides the single Rust-owned machine prefix DASH_SDK_SIGNER_ERR_KEY_UNAVAILABLE_PREFIX through ProtocolError::Generic — typed at both ABI edges, one constant bridging the string segment. This is an internal coordinated ABI change: every piece versions together in this monorepo. - rs-platform-wallet-ffi: PlatformWalletFFIResultCode:: ErrorSigningKeyUnavailable = 31 (codes 26-28 are reserved for dashpay#4185's reservation-token errors and 29/30 for dashpay#4184's asset-lock errors on sibling branches — documented in the enum as dashpay#4184 does). The From<dpp::ProtocolError> conversion restores the typed code from the prefix FIRST (before the loose keyword sniffs), and the From<PlatformWalletError> blanket impl restores it on the catch-all only (dedicated retry-semantics codes are never overridden) — covering the Sdk(dash_sdk::Error::Protocol(..)) wrapping path. - JNI/Kotlin: SignerNative.completeSign(token, signature, errorCode, errorMessage); KeystoreSigner passes SIGNER_ERROR_CODE_KEY_UNAVAILABLE on the null-key branch (keeping the MESSAGE_MARKER text for the transition window) and Generic everywhere else. DashSdkError maps 31 → PlatformWallet.SigningKeyUnavailable; the dashpay#4191 marker sniff on the catch-all codes remains as a deprecated old-native fallback with a removal note tied to the next minor release. - Swift: KeychainSigner trampolines forward the code (missing-row / missing-scalar outcomes classify as 1); PlatformWalletResultCode gains errorSigningKeyUnavailable = 31 → PlatformWalletError .signingKeyUnavailable (Kotlin parity). - Tests: rs-sdk-ffi completion-code tests (prefix present for code 1, absent for generic), platform-wallet-ffi prefix→31 tests on both conversion points, Kotlin code-31 + fallback-marker tests, Swift mapping and trampoline-classifier tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
…the signer wire Replace end-to-end message sniffing for the signer's "missing key" failure with a typed discriminator (dashpay#4060 finding 7): - rs-sdk-ffi: DashSDKSignerErrorCode { Generic = 0, SigningKeyUnavailable = 1, AuthenticationFailed = 2 (reserved) }; SignCompletionCallback and dash_sdk_sign_async_completion gain error_code: i32 (before error_message). SignResult stays Result<Vec<u8>, ProtocolError> (a new rs-dpp ProtocolError variant would carry serialization blast radius), so code 1 rides the single Rust-owned machine prefix DASH_SDK_SIGNER_ERR_KEY_UNAVAILABLE_PREFIX through ProtocolError::Generic — typed at both ABI edges, one constant bridging the string segment. This is an internal coordinated ABI change: every piece versions together in this monorepo. - rs-platform-wallet-ffi: PlatformWalletFFIResultCode:: ErrorSigningKeyUnavailable = 31 (codes 26-28 are reserved for dashpay#4185's reservation-token errors and 29/30 for dashpay#4184's asset-lock errors on sibling branches — documented in the enum as dashpay#4184 does). The From<dpp::ProtocolError> conversion restores the typed code from the prefix FIRST (before the loose keyword sniffs), and the From<PlatformWalletError> blanket impl restores it on the catch-all only (dedicated retry-semantics codes are never overridden) — covering the Sdk(dash_sdk::Error::Protocol(..)) wrapping path. - JNI/Kotlin: SignerNative.completeSign(token, signature, errorCode, errorMessage); KeystoreSigner passes SIGNER_ERROR_CODE_KEY_UNAVAILABLE on the null-key branch (keeping the MESSAGE_MARKER text for the transition window) and Generic everywhere else. DashSdkError maps 31 → PlatformWallet.SigningKeyUnavailable; the dashpay#4191 marker sniff on the catch-all codes remains as a deprecated old-native fallback with a removal note tied to the next minor release. - Swift: KeychainSigner trampolines forward the code (missing-row / missing-scalar outcomes classify as 1); PlatformWalletResultCode gains errorSigningKeyUnavailable = 31 → PlatformWalletError .signingKeyUnavailable (Kotlin parity). - Tests: rs-sdk-ffi completion-code tests (prefix present for code 1, absent for generic), platform-wallet-ffi prefix→31 tests on both conversion points, Kotlin code-31 + fallback-marker tests, Swift mapping and trampoline-classifier tests. 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
|
… on base's upstreamed router fix (dashpay#4074) Rebase of PR dashpay#4074 (fix/kotlin-sdk-assetlock-multi-account) onto feat/kotlin-sdk-and-example-app. The platform-wallet asset-lock changes are preserved; the PR's OWN rust-dashcore customization is dropped because the base branch now carries the same fix upstream. What this brings to the asset-lock code path (packages/rs-platform-wallet): - Multi-account funding builder `build_asset_lock_tx_from_all_funding_accounts` that unions spendable UTXOs across BIP44 + CoinJoin + DashPay funds accounts (dashpay#4073), with LargestFirst coin selection pinned for the many-small-denomination CoinJoin shape. - Exclusion of watch-only `DashpayExternalAccount` UTXOs from the union — those are a contact's coins the local mnemonic can't sign; selecting one yields an invalid input signature. The receiving (ours) DashPay account stays included. - `NoUtxosAvailable` mapped to the typed asset-lock insufficient-funds error. - Regression tests covering the router-fix persistence path (CoinJoin + DashpayReceivingFunds legs) and the watch-only exclusion, plus split-funded test fixtures (`split_funded_wallet_manager`, `..._dashpay`, `..._many_coinjoin`). Why the PR's rust-dashcore vendoring/[patch] is DROPPED: The PR originally shipped the asset-lock transaction-router fix by pinning rust-dashcore at 1860089e and redirecting it via a `[patch]` to a bfoss765 fork (rev e8c7335 = 1860089e + the router fix + a CoinJoin gap-limit 30->100 bump). The base branch's rust-dashcore rev 19690d31 now contains BOTH fixes upstream: * `TransactionRouter::get_relevant_account_types(AssetLock)` includes CoinJoin, DashpayReceivingFunds, and DashpayExternalAccount (via `fund_bearing_account_types()`); * `DEFAULT_COINJOIN_GAP_LIMIT = 100`. So the fork [patch], the pinned 1860089e rev, and the leftover `third_party/rust-dashcore` are all obsolete and removed. Cargo.toml/Cargo.lock are taken as-is from base (rust-dashcore resolves from dashpay @ 19690d31, no patch table). Because base's fix debits CoinJoin/DashPay asset-lock spends via the normal `check_core_transaction` scan, the PR's earlier broadcast-time `debit_router_omitted_asset_lock_spends` mitigation is gone — as it already was in the PR's final state (dashpay/dash-wallet#1507). History note: the PR's 10 original commits touched the same four files the base branch had independently rewritten (+928 lines), and included add-then-remove churn (the interim mitigation) plus vendor-then-git-patch churn that base's upstreamed fix makes moot. They are collapsed into this single commit to keep the rebased history coherent. Verified: `cargo check -p platform-wallet --all-targets`, `-p rs-unified-sdk-jni`, `-p platform-wallet-ffi` all green; `cargo test -p platform-wallet --lib` = 502 passed / 0 failed, including the router-fix and watch-only-exclusion regression tests, against base's 19690d31. Original commits folded in: 9be2e14, 5376235, e1593f4, da97eec (app-code only; vendoring dropped), ce482fd (app-code only; vendored gap-limit dropped), 380645a, b43caed (dropped: pure [patch] plumbing), a5ea9e5, 77561d2, 189e068. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vacy-domain funding gate (dashpay#4184) Addresses two must-fix reviewer findings on PR dashpay#4184. 1. FFI error arm for the asset-lock shortfall (dashpay#4073 request 3). `AssetLockInsufficientFunds` never crossed the FFI: with no arm in `From<PlatformWalletError>` it flattened to `ErrorUnknown(99)`, hiding a typed shortfall behind the catch-all and forcing hosts to string-match the Display text. Add: - `ErrorAssetLockInsufficientFunds = 26` (sibling of ErrorCoreInsufficientFunds = 22; existing codes unchanged), mapped in the From impl (the structured available/required duffs still travel in the message), plus a Rust test that the error crosses as code 26 (not 99) with the message verbatim. - Kotlin `DashSdkError.PlatformWallet.AssetLockInsufficientFunds` (26 ->) and Swift `.errorAssetLockInsufficientFunds`; cbindgen emits the C constant. The Display text is UNCHANGED ("asset lock coin selection is short: ...") so dash-wallet's existing substring matcher keeps working while it migrates to the typed code. 2. Privacy-domain co-spend gate. Largest-first could union ordinary BIP44/BIP32, CoinJoin, and DashPay-receiving funds into one L1 tx (with BIP44 change), irreversibly linking those domains. Default funding now stays within a single privacy domain: - Domains: Transparent {BIP44,BIP32} > CoinJoin > DashPay-receiving. Transparent is the only default-eligible domain because it holds the primary account and is the sole source of change (key-wallet derives change only on Standard accounts) — so any non-transparent spend inherently crosses into it. - New `CrossDomainConsent` (Denied default / Allowed opt-in) threaded through the builder, orchestration, `shielded_fund_from_asset_lock`, the JNI bridge, and the `platform_wallet_manager_shielded_fund_from_asset_lock` FFI (`allow_cross_domain: bool`). Wrapper methods keep every existing caller on the safe default. - Cross-domain refusal returns the typed `AssetLockCrossDomainConsentRequired` (FFI code 27; Kotlin/Swift mapped) carrying transparent/union/required duffs. - Watch-only DashpayExternalAccount exclusion preserved via the domain classifier (returns None); identity-funding single-BIP44 carve-out preserved. Tests: single-domain success without consent; cross-domain refused without consent (typed error) and succeeds with consent; existing union/CoinJoin/DashPay tests moved to the consented path. cargo test -p platform-wallet --lib = 504 passed; --features shielded = 634 passed; -p platform-wallet-ffi --lib = 198 passed. clippy clean on all three crates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cross-PR collision: the split-build-broadcast branch (dashpay#4185) already allocates 26-28 for the reservation-token errors on the same base, and both PRs would merge without textual conflict, silently misclassifying errors on whichever lands second. Codes 26-28 are now documented as reserved for dashpay#4185. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 2: the pre-selection consent gate compares fee-exclusive totals, so a transparent balance in [target, target+fee) passed the gate and then died in selection as plain insufficient-funds — the host never learned that consent would unlock the spend. The selection-failure mapping now converts the shortfall to AssetLockCrossDomainConsentRequired when consent is denied and the union covers what selection reported missing. Adds the fee-band test and the denied-both-short test the review found uncovered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
blocker) PlatformWalletResultCode gained cases 29/30 (errorAssetLockInsufficientFunds, errorAssetLockCrossDomainConsentRequired) but PlatformWalletError had no matching cases, so init(result:) — which switches over result.code with no default — became non-exhaustive and the Swift package no longer compiled. Add the two matching cases (assetLockInsufficientFunds, assetLockCrossDomainConsentRequired) to PlatformWalletError, extend the errorDescription associated-value binding, and map both codes in init(result:). Semantics/messages mirror the Kotlin DashSdkError.PlatformWallet counterparts (codes 29/30). Verified: swiftc -typecheck of PlatformWalletResult.swift against a stub DashSDKFFI module built from the cbindgen-generated header now passes; removing the fix reproduces "switch must be exhaustive". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ShieldedFundFromAssetLockView built its funding picker from BIP44 accounts only and never set allowCrossDomain, so the all-funds/cross-domain flow could not be exercised or consented to on iOS. Add a minimal "Allow cross-domain funds" toggle (transparent-only by default) wired through to shieldedFundFromAssetLock(allowCrossDomain:), so the code-30 gate is reachable. A tracked TODO on crossDomainConsentSection records the intended fuller UX (submit false, catch errorAssetLockCrossDomainConsentRequired, show the transparent/union/ required breakdown, then retry with true) as a follow-up to file — the PR is held for rust-dashcore#912 before that lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dashpay#4184) - Remove the "finding <hash>" tracker references from build.rs comments (rationale text kept); these belong in the PR description, not the source. - Add an ABI/release-note line to the platform-wallet-ffi README recording the C-ABI break: platform_wallet_manager_shielded_fund_from_asset_lock gained a trailing `bool allow_cross_domain` parameter, plus result codes 29/30. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…optional derivation path (drop multi-account union + consent gate) Replace the multi-account union + privacy-domain consent gate with a single account-by-derivation-path selection. The caller either accepts the default (unmixed BIP44 account) or passes one explicit BIP32 path (e.g. the DIP-9 CoinJoin account) as a nullable String, threaded Kotlin -> JNI -> FFI -> Rust as Option<DerivationPath>. No union across accounts, no consent prompt. - rs-platform-wallet: drop build_asset_lock_tx_from_all_funding_accounts, CrossDomainConsent/PrivacyDomain, and AssetLockCrossDomainConsentRequired; add build_asset_lock_tx_from_selected_account (single account by path, change routed to the BIP44 account since non-Standard accounts cannot derive change); collapse the four *_with_consent wrapper pairs into a single funding_path arg. - rs-platform-wallet-ffi: swap allow_cross_domain bool for funding_path (ptr+len UTF-8 BIP32 string); drop error code 30. - rs-unified-sdk-jni / kotlin-sdk: allowCrossDomain: Boolean -> fundingPath: String?; drop DashSdkError.PlatformWallet.AssetLockCrossDomainConsentRequired. - Replace the union/consent tests with single-path tests (explicit CoinJoin path, default BIP44, no-union proof, typed shortfall, watch-only refusal). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g path (drop cross-domain consent + code 30) Mirror the Kotlin re-scope (2d86a29) on the second consumer of the same C ABI. The FFI dropped the allow_cross_domain bool for a nullable funding_path (ptr+len UTF-8 BIP32 string) and removed error code 30, so the old Swift signature no longer compiles. - PlatformWalletManagerShieldedFunding.swift: allowCrossDomain: Bool -> fundingPath: String? = nil; marshal the optional path as raw UTF-8 bytes via a new withOptionalFundingPath helper (nil/empty -> nil ptr + 0 len), mirroring withOptionalSurplusOutput, and pass funding_path_ptr/ funding_path_len to the C function. - PlatformWalletResult.swift: drop the errorAssetLockCrossDomainConsentRequired (code 30) FFI case and the assetLockCrossDomainConsentRequired PlatformWalletError case and their mappings. - SwiftExampleApp ShieldedFundFromAssetLockView: replace the cross-domain consent toggle with an optional funding-path text field; pass fundingPath (nil when blank) to the SDK call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…re-scope The dashpay#4184 re-scope dropped AssetLockCrossDomainConsentRequired (code 30) but left references behind: a DashSdkErrorTest mapping (broke :sdk:test compile), the rs-platform-wallet-ffi README ABI note, and a shielded_send.rs comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntracts (dashpay#4184) Address maintainer/CodeRabbit review findings on the dashpay#4184 asset-lock funding re-scope. 1. Reserve the selected-account inputs (correctness / double-spend). `build_asset_lock_tx_from_selected_account` seeded inputs via `add_inputs` with NO ReservationSet, so a re-scoped shielded build reserved nothing — a concurrent normal Core send (the BIP44 `build_asset_lock_with_signer` path) or a CoinJoin mix could reselect the same UTXOs in the build->broadcast->reconcile window (the wallet write lock + `shield_guard` only serialize shielded builds). Point the key-wallet builder's `set_funding` at the SELECTED funds account instead of `add_inputs`, so the chosen inputs are reserved in that account's OWN reservation ledger (the same single-account machinery the BIP44 builder uses) and held across build->broadcast. Release on a rejected broadcast is now path-aware (`release_asset_lock_funding_reservation`): shielded funding releases from the account named by `funding_path` (possibly CoinJoin/DashPay); every other funding type from the BIP44 account. Adds two tests: a selected-account build reserves its CoinJoin input across broadcast, and a rejected selected-account broadcast releases it. 2. Restore `WalletOperation` for the generic shielded-funding failure arm (was regressed to `Unknown(99)`): `platform_wallet_error_code_or_wallet_operation` keeps typed codes but maps the catch-all to `ErrorWalletOperation`, at both the fund and resume-fund FFI entry points. 3. JNI: read `fundingPath` with a strict reader that throws on a genuine JNI read error instead of silently degrading a money-source param to the default BIP44 account; JVM null still means "absent". 4. Rewrite the stale multi-account-union/consent docs in `orchestration.rs` (FromWalletBalance) to the as-built single-path design. 5. Swift example: the funding-account picker now surfaces non-Standard funds accounts (CoinJoin/DashPay) with type labels, and `canSubmit` no longer gates on the selected Core account's balance when an explicit `fundingPath` is supplied — so a CoinJoin path is exercisable end-to-end. Validation: cargo build -p platform-wallet -p platform-wallet-ffi -p rs-unified-sdk-jni --features shielded; cargo test -p platform-wallet --features shielded (639 passed); cargo test -p platform-wallet-ffi --features shielded (215+26+6 passed); ./gradlew :sdk:assemble -x lint (BUILD SUCCESSFUL). Swift not built locally (no xcframework). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
review blocker 2) Reviewer (thepastaclaw) blocker 2 — build.rs:423-424: build_asset_lock_tx_from_selected_account passed the BIP44 change account (&bip44_acc) as set_funding's `acc`, but set_funding calls funds_acc.next_change_address(Some(&acc.account_xpub)) on the SELECTED funds account before the set_change_address override. For an explicitly selected Standard BIP32 account with no pre-generated unused internal address, that derived [1, index] from the wrong (BIP44) xpub and recorded it under the BIP32 account's own path, poisoning that pool so a later normal BIP32 send could use a change entry whose signer key does not match the address. Now resolve the wallet-level Account whose account-level derivation path equals funding_path and pass ITS xpub to set_funding, while keeping the separate BIP44 set_change_address override. Default BIP44 funding resolves to bip44_acc (unchanged); non-Standard (CoinJoin/DashPay) accounts fail change derivation regardless, so the xpub is immaterial and the bip44_acc fallback preserves prior behavior. Also (thepastaclaw nitpick, test_support.rs): move the DashPay fixture rustdoc so it attaches to split_funded_wallet_manager_dashpay rather than foreign_contact_account_xpub. Blocker 1 (build.rs:427-473, owner-guarded reservation rollback on the pre-broadcast abandonment path) is NOT addressed here: the pinned key-wallet rev 70d4bf8 exposes no owner-guarded release primitive (ReservationSet has only reserve/reserved/release keyed by outpoint; release_reservation is unconditional) and no reservation token, so the required release_if_owner(token) mechanism (rust-dashcore#916 / dashpay#4185) cannot be implemented against this pin without an upstream dependency change — the very atomic-reservation fix this PR is held for. Deferred pending that adoption rather than substituting an unconditional release on the money path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…adcast path build_signed reserves the selected inputs in the funding account's ReservationSet and normally holds them from build through broadcast. But several post-signing, pre-broadcast error paths could return without rolling that reservation back, stranding the inputs until the reservation-TTL backstop (~24 blocks) and causing false shortfalls / withheld coins in the meantime (dashpay#4184 review, thepastaclaw): - build_asset_lock_tx_from_selected_account (shielded selected-account path): the credit-account lookups, peek_next_path, the external signer.public_key round-trip, and mark_first_pool_index_used are all fallible after build_signed. Derive the credit key in an inner future and, on any error, release the reservation on the selected funds account (matched by funding_path, mirroring release_asset_lock_funding_reservation) before propagating. - build_asset_lock_transaction (non-shielded / upstream builder path): the two defensive credit-key arms after build_asset_lock_with_signer abandon a signed+reserved tx; release on the BIP44 account at account_index. - broadcast_funded_asset_lock invitation durability branch: when the funding-index persist/flush fails we abort before broadcast; release the reservation via release_asset_lock_funding_reservation before bailing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on window Addresses the outstanding review findings on dashpay#4184 at a9e418a. Reservation lifecycle, delegated (non-shielded) builder — blocker. `build_asset_lock_with_signer` reserves the transaction's inputs inside `build_signed` and rolls that back only when *input* signing fails. Its credit-output loop runs afterwards, and a failure there returns `Err` with a fully-signed transaction abandoned and its inputs still reserved. That reservation cannot be rolled back from platform-wallet at this pin: the error carries no transaction to hand `release_reservation`, and the account's `ReservationSet` is `pub(crate)` to key-wallet. The abandon path is therefore removed rather than rolled back. Of the loop's fallible steps only the external `signer.public_key` round-trip is genuinely reachable — resolving the credit account, `peek_next_path` and `mark_first_pool_index_used` cannot fail once `peek_next_funding_address` has resolved and peeked that same account earlier in the call under the same held wallet write lock. So the signer round-trip now happens up front, before anything is reserved, and the builder receives a `PrefetchedCreditKeySigner` that answers the repeat request from cache. `peek_next_funding_address` returns the peeked path alongside the address to feed it. Standard BIP32 regression coverage. Adds the test the reviewer asked for around the `set_funding` xpub lookup: it exhausts a selected BIP32 account's pre-generated unused internal entries so `next_change_address` must derive a fresh one, builds through that account's explicit path, and asserts every internal-pool entry is still signable at its own recorded path. Reverting the lookup to the BIP44 xpub fails it. SwiftExampleApp funding picker. Picker rows were all tagged with a bare `accountIndex`, which account families reuse — BIP44 #0, BIP32 #0 and CoinJoin #0 collided, so SwiftUI could not tell which row was chosen and a non-BIP44 row without a BIP44 counterpart fed an invalid change account to Rust. The picker is now restricted to BIP44 accounts (the mandatory change sink, listed even at zero balance so the dashpay#4073 case still works) and the other funds accounts are shown read-only. Public API documentation. `account_index` / `fundingAccountIndex` is documented as the change sink that doubles as the default input source, rather than unconditionally as the input source, across the Rust FFI, Swift and Kotlin surfaces — it contradicted the `funding_path` docs immediately below it. Tests: platform-wallet 511/511 (2 new), platform-wallet-ffi 205/205 + 26 + 6; cargo fmt applied; clippy clean on both crates (only pre-existing warnings in recovery.rs / withdrawal.rs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two mechanical CI-hard failures on bd19a3e: 1. `cargo fmt --check --all` failed on the JNI funding-path bridge (rs-unified-sdk-jni/src/funding.rs). Applied rustfmt's replacement. 2. The wallet clippy job (`--all-features -- --no-deps -D warnings`) failed with `dead_code` on five PR-added asset-lock fixtures in platform-wallet's `test_support`. The module is compiled into the normal lib target under `--all-features` (it is gated `cfg(any(test, feature = "test-utils"))`), so `pub(crate)` helpers consumed only by `#[cfg(test)]` unit tests have no consumer there. Gated `split_funded_wallet_manager`, `DashpayLeg`, `foreign_contact_account_xpub`, `split_funded_wallet_manager_dashpay` and `split_funded_wallet_manager_many_coinjoin` on `cfg(test)`, plus the imports that only they use (`OutPoint`/`TxOut`/`Txid`, `Utxo`). `cfg(test)` — not `cfg(any(test, feature = "test-utils"))` — matches the convention this file already documents on `RejectFirstBroadcaster` and applies to all four mock broadcasters. It cannot break the cross-crate test build: the only external consumer (rs-platform-wallet-ffi's broadcast tests) uses `funded_spv_core_wallet` and `WalletSigner`, both `pub` and both left untouched, while the five gated items are `pub(crate)` and were never reachable from outside. Also addresses the two review suggestions, both additive: - Added `delegated_builder_serves_credit_key_from_prefetch_cache`. The existing failure test's signer rejects every `public_key` call, so it only proves a prefetch failure is reservation-free. The new recording signer answers exactly one request and poisons the rest, pinning that the builder's credit-key request is served from `PrefetchedCreditKeySigner`'s cache and that the prefetched path is exactly the credit path the builder returns. - Added four `parse_optional_derivation_path` tests covering null, zero length with a non-null pointer, a valid account-level path, invalid UTF-8 and malformed syntax — pinning that the funding-path parser fails closed with ErrorInvalidParameter instead of silently falling back to the default BIP44 source. No error codes touched; ErrorAssetLockInsufficientFunds stays 29. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rest of the wallet-crate docs use the American spelling ~2:1; align the new release-notes bullet with it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…OMAIN-OK The abandon-path rollback added by this PR walks all_funding_accounts() to reach the owning account's release_reservation. dashpay#4247's funding-domain guardrail (wallet::funding_privacy) treats any unannotated use of the wallet-wide funds iterator as a potential cross-account funding union and fails the build — so the two PRs are individually green but fail together, as the v41int13 integration confirmed. The site selects no coins: it looks up the ONE account matching the already-chosen funding_path, releases that account's reservation, and breaks. Annotate it accordingly. Comment only, no behavior change. Co-Authored-By: Claude Opus 4.8 <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
3 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.
Shields asset-lock funding from all funds accounts: a multi-account funding builder (
build_asset_lock_tx_from_all_funding_accounts) that unions spendable UTXOs across BIP44 + CoinJoin + DashPay funds accounts with LargestFirst coin selection, excludes watch-onlyDashpayExternalAccountUTXOs (a contact's coins the local mnemonic cannot sign), and mapsNoUtxosAvailableto the typed asset-lock insufficient-funds error, with regression tests for the router-fix persistence path and the watch-only exclusion.Re-opens #4074 which was auto-closed when the #3999 base branch was deleted; rebased onto
v4.1-dev. The PR's own rust-dashcore router customization stays dropped —v4.1-dev's rust-dashcore pin already carries that fix upstream (rust-dashcore#867), which the ported regression tests confirm against the pin. The platform-side changes inrs-platform-walletare NOT inv4.1-devand are all retained; the only conflict was a trivial import union intest_support.rs.Verified:
cargo test -p platform-wallet— 502 tests pass (38 asset-lock).🤖 Generated with Claude Code
Review-response summary (2026-07-21)
AssetLockInsufficientFundsnow crosses as dedicated code 29 (ErrorAssetLockInsufficientFunds), mapped in Kotlin and Swift, with an FFI-level test pinning the numeric code and verbatim message. The Display text is unchanged from what dash-wallet already matches, so no host breakage; hosts should migrate from substring-matching to the typed code at their convenience. Codes 26–28 are deliberately skipped — they're allocated by the reservation-token errors on the split-build-broadcast branch (feat(kotlin-sdk): split build/broadcast with reservation release for BIP70-style deferred submission #4185); a comment in the enum documents the reservation so the two PRs can't collide.allow_cross_domain, default false everywhere, including the resume path); refusal is typed code 30 (ErrorAssetLockCrossDomainConsentRequired) carrying transparent/union/required amounts. The identity-funding carve-out is unchanged — consent is ignored there, pinned by test. Round-2 addition: a fee-band shortfall (transparent covers the amount but not amount+fee, union covers it) is reclassified to consent-required at the selection-failure site, so hosts prompt instead of dead-ending; both the fee-band and denied-both-short paths are now tested.cargo test -p platform-wallet --lib→ 506 passed;--features shielded --lib→ 636 passed;-p platform-wallet-ffi --lib→ 198 passed; clippy clean on all three crates.allowCrossDomain = trueafter explicit user opt-in; dash-wallet will need a consent touchpoint before adopting the next AAR.Summary by CodeRabbit
New Features
Bug Fixes
Provenance (tracker refs moved from code comments per review)
The
build.rsfunding-eligibility comments previously carried an internal review-tracking tokenfinding 5b52d9844055(4 sites). Removed from the comments (rationale text kept); it tracked the watch-onlyDashpayExternalAccountownership carve-out now expressed through the privacy-domain map.C-ABI note
platform_wallet_manager_shielded_fund_from_asset_lockgained a trailingallow_cross_domain: boolparameter — a C-ABI break — and result codes 29/30 (ErrorAssetLockInsufficientFunds/ErrorAssetLockCrossDomainConsentRequired) are added. Seepackages/rs-platform-wallet-ffi/README.md.