fix(platform-wallet): keep note reservations on ambiguous shielded spend confirmation failures#3863
Merged
Merged
Conversation
…tep and keep unconfirmed broadcasts safe The shielded (Type-20) identity registration ran as one opaque broadcast_and_wait: any failure surfaced as ShieldedBroadcastFailed, the app pinned it on the "Generating Halo 2 proof" step, and the wallet released the spent notes' reservations — even when the transition had already executed on-chain and only the result-proof confirmation failed (observed live on devnet; server side since fixed in #3859). That treated a possibly-live identity as unregistered (orphaned-identity hazard) and invited double-spends. - rs-sdk: add identity_create_from_shielded_pool_transition (build + validate only) so callers can stage broadcast and wait separately. - platform-wallet: classify wait-stage failures; on an ambiguous post-broadcast error, retry fetching the identity by its derived id (success path if found), otherwise return the new ShieldedBroadcastUnconfirmed error and leave note reservations in place (in-memory only; the next nullifier sync reconciles them). - ffi: new result codes ErrorShieldedBroadcastFailed (16) and ErrorShieldedBroadcastUnconfirmed (17); the latter also writes the derived id to out_identity_id. - swift-sdk: mirror the codes; throw a typed ShieldedIdentityCreateUnconfirmedError carrying the derived id. - example app: new .unconfirmed controller phase that holds the identity slot against re-submission and marks the slot used; the shielded progress view gains a distinct "Waiting for platform confirmation" step, attributes broadcast rejections to the broadcast step, and renders an orange confirmation-pending state instead of a red failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…end confirmation failures Unshield, shielded transfer, and shielded withdrawal used the one-shot broadcast_and_wait and ran cancel_pending on ANY error, including post-broadcast wait failures (result-proof fetch/verify errors, timeouts, transport errors) where the relay had already accepted the transition and the spend may well have executed. Releasing the in-memory note reservations there invites re-selecting notes whose nullifiers may already be consumed on chain. Apply the same stage-split + error classification as the Type-20 identity-create flow: broadcast() and wait_for_response() are staged separately via a shared broadcast_shielded_spend helper. Broadcast-time rejections and StateTransitionBroadcastError (Platform ran the transition and rejected it on its merits) keep today's ShieldedBroadcastFailed + cancel_pending; any other wait failure maps to the new ShieldedSpendUnconfirmed variant, whose outer-match arm leaves the reservation in place — pending_nullifiers is in-memory only, so the next nullifier sync promotes the notes to spent if the spend landed, and an app restart frees them if it never did. Shield (Type 15) and ShieldFromAssetLock (Type 18) deliberately keep the one-shot helper: they take no note reservation, so an ambiguous wait failure has no local state to strand (documented at the shield broadcast site). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSeparates broadcast from proof confirmation for shielded operations, classifies ambiguous confirmation failures as new unconfirmed error variants, maps those through the FFI to Swift, and updates the Swift registration controller/views and send flow to retain reservations and display an ChangesShielded broadcast/confirm separation and unconfirmed state handling
Sequence Diagram(s)sequenceDiagram
participant Wallet as WalletOperation
participant Transition as StateTransition
participant Relay as Network/Relay
participant WalletLogic as Classification
Wallet->>Transition: broadcast(transition)
Transition->>Relay: send to platform relay
Relay-->>Transition: accepted or rejected
alt Broadcast rejected
Transition-->>Wallet: BroadcastFailed
Wallet->>WalletLogic: map -> ShieldedBroadcastFailed
Note right of WalletLogic: reservation can be released
else Broadcast accepted
Transition->>Relay: wait for proof result
alt Wait failed with consensus cause
Relay-->>Transition: WaitFailed(Some(cause))
Transition-->>Wallet: definitive rejection
Wallet->>WalletLogic: map -> ShieldedBroadcastFailed
Note right of WalletLogic: reservation can be released
else Wait failed without cause (timeout/transport)
Relay-->>Transition: WaitFailed(None)
Transition-->>Wallet: ambiguous outcome
Wallet->>WalletLogic: map -> ShieldedSpendUnconfirmed
Note right of WalletLogic: reservation must be retained
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 complete (commit 231f22b) |
thepastaclaw
suggested changes
Jun 11, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Code Review
The Rust-side intent is sound, but the helper still misclassifies the most common ambiguous case: DAPI-side wait timeouts (and other internal wait failures) are encoded as a StateTransitionBroadcastError with no consensus cause, so the current match arm releases note reservations in exactly the timeout case this PR is meant to protect. The FFI boundary also collapses the new ShieldedSpendUnconfirmed variant into a generic ErrorWalletOperation, which is acknowledged as a follow-up.
🔴 1 blocking | 🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-platform-wallet/src/wallet/shielded/operations.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/shielded/operations.rs:1262-1264: DAPI wait timeouts still release note reservations
This arm treats every `dash_sdk::Error::StateTransitionBroadcastError` from `wait_for_response` as a definitive Platform rejection and routes it to `ShieldedBroadcastFailed`, which makes the caller run `cancel_pending`. But the DAPI `wait_for_state_transition_result` service maps its own internal wait failures — including `DapiError::Timeout` — into a `WaitForStateTransitionResultResponse::Error` via `build_wait_for_state_transition_error_response` → `TenderdashStatus::new(code, msg, None)` → `StateTransitionBroadcastError { data: empty, .. }` (see `packages/rs-dapi/src/services/platform_service/error_mapping.rs:151` and the test at `wait_for_state_transition_result.rs:353`). The SDK's `TryFrom<StateTransitionBroadcastErrorProto>` (`packages/rs-sdk/src/error.rs:144`) populates `cause = Some(_)` only when `data` is non-empty, so DAPI timeouts arrive here as `StateTransitionBroadcastError` with `cause: None`. Those are post-broadcast ambiguous, not consensus rejections — the spend may still execute after the wait times out. As written, this preserves the exact double-spend/proof-waste failure mode the PR is meant to fix. Gate this arm on a real consensus cause and let the no-cause path fall through to the `ShieldedSpendUnconfirmed` arm.
In `packages/rs-platform-wallet-ffi/src/shielded_send.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/shielded_send.rs:318-442: FFI collapses ShieldedSpendUnconfirmed into generic ErrorWalletOperation
All three spend FFIs (transfer at 318-322, unshield at 376-380, withdraw at 438-442) map every `Err(_)` from the wallet call to `PlatformWalletFFIResultCode::ErrorWalletOperation`, so the new `PlatformWalletError::ShieldedSpendUnconfirmed` is indistinguishable on the C ABI from `ShieldedBroadcastFailed` or any other error. The identity-create sibling `platform_wallet_manager_shielded_identity_create_from_pool` already exposes dedicated codes 16/17 for the same distinction. With this PR's new behavior the wallet intentionally keeps the original spend's notes reserved on ambiguous confirmation failure — so a host that interprets the generic error as retriable will build a *second* valid spend by selecting *different* unreserved notes, producing a duplicate transfer/withdrawal if the first spend landed. The PR description frames this as a follow-up; flagging because in-PR symmetry with codes 16/17 would close the host-side window without widening scope much. Add a dedicated FFI result code (and corresponding Swift `PlatformWalletError` case) for `ShieldedSpendUnconfirmed`, and pattern-match it explicitly in all three spend FFIs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3863 +/- ##
=========================================
Coverage 70.73% 70.73%
=========================================
Files 20 20
Lines 2788 2788
=========================================
Hits 1972 1972
Misses 816 816
🚀 New features to boost your workflow:
|
…d and type spend results over FFI Review follow-ups from #3863: DAPI encodes its own wait-side failures (DapiError::Timeout, internal errors) as StateTransitionBroadcastError with empty consensus data (build_wait_for_state_transition_error_response), which the SDK decodes as cause: None. The previous classification treated every StateTransitionBroadcastError as a definitive Platform rejection, so a DAPI wait timeout still released note reservations — exactly the ambiguous case this PR protects. Gate the definitive arm on cause.is_some() in both broadcast_shielded_spend and the identity-create flow (where cause-less errors now reach the fetch-by-derived-id fallback instead of failing hard). Misreading a rejection as ambiguous only delays note release until the next sync or restart, so the gate errs in the safe direction. Adds unit tests for the classification. Also surface ShieldedSpendUnconfirmed as a dedicated FFI result code (ErrorShieldedSpendUnconfirmed = 18) in the three spend FFIs, mirrored in the Swift PlatformWalletResultCode/PlatformWalletError enums, so hosts can tell "may have executed, do NOT retry" from a retryable failure instead of both collapsing into ErrorWalletOperation — a host retry on the unconfirmed path could select different unreserved notes and double-send. ShieldedBroadcastFailed now maps to the existing code 16 for spends, matching the identity-create sibling. Co-Authored-By: Claude Fable 5 <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: 3
🤖 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/operations.rs`:
- Around line 1251-1259: The broadcast_shielded_spend function currently maps
any error from BroadcastStateTransition::broadcast to
PlatformWalletError::ShieldedBroadcastFailed; inspect
packages/rs-sdk/src/platform/transition/broadcast.rs and the underlying
broadcast_request_for_state_transition to enumerate error variants
(timeout/transport/DAPI vs. relay-ACK rejection) and change
broadcast_shielded_spend to match on the specific error variants: map only
definitive relay rejections to ShieldedBroadcastFailed, treat
timeout/transport/DAPI/unknown-outcome variants as an "ambiguous" result (e.g.,
return a distinct error variant or propagate the original error so callers know
outcome is unknown and should NOT clear reservations), and only clear
reservations when broadcast() returned a definitive relay-ACK success.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift`:
- Around line 24-27: The doc comment for the enum case
errorShieldedBroadcastFailed is too narrow (mentions only Type-20) but the FFI
can return ErrorShieldedBroadcastFailed for shielded spend operations
(transfer/unshield/withdraw) as well; update the comment above the case
errorShieldedBroadcastFailed to state it covers both Type-20 transition failures
and shielded spend ops (transfer, unshield, withdraw) where the relay/CheckTx or
platform execution rejects the tx, and apply the same expanded wording to the
other occurrence of ErrorShieldedBroadcastFailed in this file so callers are not
misled about semantics.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:
- Around line 1465-1480: The view marks `.unconfirmed` slots via
markIdentitySlotUsed(walletId:identityIndex:) but selection logic
(usedIdentityIndices(for:)) only reads persisted PersistentIdentity rows, so the
reservation isn't honored after the in-memory controller is gone; change the
source-of-truth so unconfirmed reservations are persisted or treated as
reserved: update markIdentitySlotUsed to create and persist a short-lived
PersistentIdentity (or a dedicated PersistentReservation entity) for the
walletId/identityIndex when state == .unconfirmed, ensure
usedIdentityIndices(for:) also reads that persisted reservation (or the new
entity), and prevent dismissing `.unconfirmed` until a PersistentIdentity
exists; touch functions CreateIdentityView state flow that set `.unconfirmed`,
markIdentitySlotUsed, and usedIdentityIndices(for:) to implement this.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bc283b5-9a66-4daf-82b1-46145debddec
📒 Files selected for processing (13)
packages/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/shielded/operations.rspackages/rs-sdk/src/platform/transition/identity_create_from_shielded_pool.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift
… the send flow
Host-side follow-ups for ErrorShieldedSpendUnconfirmed (code 18):
- SendViewModel catches PlatformWalletError.shieldedSpendUnconfirmed
before the generic catch and surfaces it through the non-error path
("transaction may have gone through — waiting for the next shielded
sync") instead of the retry-inviting error alert; the notes stay
reserved Rust-side, so a retry could double-send from other notes
- document the new throw on the shieldedTransfer / shieldedUnshield /
shieldedWithdraw wrappers
- add a unit test pinning map_spend_result's retry-relevant code split
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
approved these changes
Jun 12, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Code Review
Both prior findings from the c8c68c9 review are FIXED at head 327240c. The wait-failure classifier now gates the definitive ShieldedBroadcastFailed mapping on a populated consensus cause (with three unit tests), and the FFI/Swift surfaces expose a dedicated ErrorShieldedSpendUnconfirmed = 18 for the spend FFIs symmetric to the identity-create sibling. The latest delta introduces no new in-scope defects.
…s unconfirmed broadcast() runs through the dapi-client retry machinery, so a transport error or timeout cannot prove the transition never reached a node — the request may have been delivered with only the ACK lost, and AlreadyExists proves the opposite (the transition is already in the mempool or on chain). Mapping every broadcast() error to ShieldedBroadcastFailed released the note reservations in those unknown-outcome cases, letting a host retry select other unreserved notes and double-send if the original broadcast landed. Only a consensus verdict is definitive at the broadcast stage: a CheckTx rejection arrives as Error::Protocol(ConsensusError) (DAPI re-attaches the serialized consensus error as gRPC metadata, which the dapi-client decodes), so genuine rejections still release the notes and stay retryable. Everything else now classifies as ShieldedSpendUnconfirmed for spends and ShieldedBroadcastUnconfirmed (with the derived id, holding the slot) for identity-create — the same recovery path as a wait-stage ambiguity. The shared carries_consensus_rejection predicate also broadens the wait-stage classifier to accept the consensus-metadata shape as a verdict. Tests cover consensus/transport/AlreadyExists classification at both stages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ross controller teardown usedIdentityIndices(for:) was built only from PersistentIdentity rows, but an .unconfirmed Type-20 registration has no identity row until the next sync confirms it — so the markIdentitySlotUsed reservation evaporated with the in-memory controller and the same identityIndex became selectable again, re-opening the duplicate-submission path the .unconfirmed state exists to prevent. Union the persisted PersistentCoreAddress.isUsed reservations on the identity-registration account into the selection source of truth; over-reporting only skips an index (registration indices aren't gap-limited) while identity rows remain authoritative for confirmed identities. Also broaden the Swift code-16 docs (errorShieldedBroadcastFailed / shieldedBroadcastFailed) to cover the spend flows that now map to it, not just Type-20. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…it instead of failing fast Refine the broadcast-stage disposition from c38b99d, which classified every non-consensus broadcast() failure as unconfirmed immediately. Two problems with that: AlreadyExists (the tx IS in the mempool after a lost-ACK retry hit tenderdash's dedupe) was reported as "unconfirmed" when one wait_for_response away from a real confirmation — and for identity-create it skipped the fetch-by-derived-id fallback entirely; and the common offline failure (connect refused → gRPC Unavailable) stranded the note reservations until app restart even though nothing was ever delivered. Replace classify_spend_broadcast_failure with a three-way disposition: - definitive (broadcast_definitely_failed): consensus-verdict CheckTx rejections, server admission-refusals (InvalidArgument, ResourceExhausted = mempool full, …), connection-establishment failures (Unavailable, NoAvailableAddresses) → ShieldedBroadcastFailed and the reservations release, keeping offline sends immediately retryable; - no-verdict (AlreadyExists, TimeoutReached, Cancelled, gRPC DeadlineExceeded/Internal/Unknown/Aborted/DataLoss — shapes that can postdate delivery, including as the terminal retry error) → fall through to wait_for_response, which proves execution, returns a consensus rejection (definitive after all), or classifies the residual ambiguity as unconfirmed — for identity-create via the existing fetch-by-derived-id fallback. The wait-stage classification and carries_consensus_rejection are unchanged. Tests updated to pin the disposition table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Code Review
Latest delta (1fe849c..d0887c1) refines shielded-spend broadcast failure classification: introduces broadcast_definitely_failed predicate that distinguishes definitive-rejection gRPC codes from no-verdict shapes, with no-verdict cases falling through to the result wait. Prior review at 1fe849c found no issues, so no findings carry forward. The deny-list approach for Code::Unavailable/ResourceExhausted is a documented intentional UX tradeoff — the cancel_pending callsite (lines 433-441) explicitly notes that the on-chain nullifier set is the authoritative no-reuse guarantee and worst-case is a wasted proof, never fund loss. No in-scope blocking issues.
💬 1 nitpick(s)
… broadcast_definitely_failed Review nitpick on #3863: Unavailable is the common connect-refused shape but not an absolute never-delivered guarantee — HTTP/2 stream resets after the request bytes left surface the same code, and the dapi-client cross-address retry only retains the last transport error. Document the residual window and cross-reference the unshield finalize_pending rationale showing it is fund-safe (on-chain nullifier set; worst case a wasted proof), framing the classification as a deliberate UX trade. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
approved these changes
Jun 12, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Code Review
Prior nitpick on broadcast_definitely_failed's Unavailable rationale is FIXED at the current head: the docstring at packages/rs-platform-wallet/src/wallet/shielded/operations.rs:1342-1352 now explicitly covers HTTP/2 stream resets after the request bytes left, the dapi-client's cross-address retry retaining only the LAST transport error, and the fund-safety argument anchored on the on-chain nullifier set. The latest delta (231f22b) is documentation-only; cumulatively the PR's shielded-spend classification preserves reservations on ambiguous wait failures and releases them only on consensus rejections. No new in-scope findings across all six agents and CodeRabbit.
QuantumExplorer
added a commit
that referenced
this pull request
Jun 12, 2026
Reconciles with #3863, which extended the unconfirmed-broadcast handling to the other shielded spends and refined the identity-create classification itself: - adopt upstream's refined broadcast/wait classification (broadcast_definitely_failed transport triage; only a consensus `cause` makes a wait-stage StateTransitionBroadcastError definitive) and drop the superseded wait_error_is_definitive_rejection helper + tests (its cause-less-is-definitive assumption is now wrong); error_releases_note_reservation and its tests stay. - adopt upstream's ShieldedSpendUnconfirmed variant / FFI code 18 / Swift mirror, and extend the blanket From impl + mapping test to cover it alongside codes 16/17. - keep this branch's newer review-round state (FailureStage without .beforeBroadcast, gated .unconfirmed dismissal, isActive-based network-switch gate) over the older snapshots #3863 carried. - update slot-guard comments for upstream's usedIdentityIndices union of the persisted isUsed reservation: the reservation now also holds the slot across restarts, while the live controller remains a load-bearing guard because that write is best-effort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "4a32d6f92717d0549ed78d28fc4bc019f4154e2ed99d70b9b01e35d56c59623b"
)Xcode manual integration:
|
5 tasks
Merged
6 tasks
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
2 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.
Issue being fixed or feature implemented
unshield,shielded_transfer, andwithdrawinpackages/rs-platform-wallet/src/wallet/shielded/operations.rsused the one-shotbroadcast_and_waitSDK helper and rancancel_pendingon any error — including ambiguous post-broadcast wait failures (result-proof fetch/verify errors, timeouts, transport errors) where the relay had already accepted the transition and the spend may well have executed on chain. Releasing the in-memory note reservations in that window invites re-selecting notes whose nullifiers may already be consumed, building a second ~30 s Halo 2 proof doomed to a nullifier-already-used rejection.#3862 fixed exactly this for the Type-20 identity-create flow; this PR applies the same stage-split + error classification to the sibling note-spending flows.
Stacked on #3862 — the first commit here is a cherry-pick of that PR's commit (rebased over #3854's
terminalAtfreezing, which it conflicted with). Review only the second commit; once #3862 merges, a rebase drops the duplicate.What was done?
broadcast_shielded_spendhelper that stagesbroadcast()andwait_for_response()separately (sameBroadcastStateTransitiontrait callsbroadcast_and_waitis built from — no new SDK builder methods were needed, since these flows build theirStateTransitionlocally via the dpp builders):StateTransitionBroadcastErrorfrom the wait (Platform ran the transition and rejected it on its merits) → definitive:ShieldedBroadcastFailed, reservation released viacancel_pendingas before;PlatformWalletError::ShieldedSpendUnconfirmed { operation, reason }, and the flow's outer match leaves the reservation in place.pending_nullifiersis in-memory only: the next nullifier sync promotes the notes to spent if the transition landed, and an app restart frees them if it never did.shieldbroadcast site:shield(Type 15) andshield_from_asset_lock(Type 18) take no note reservation (mark_pendingnever runs), so an ambiguous wait failure has no local state to strand.FFI note:
platform-wallet-ffimaps the new variant through its existing catch-all (ErrorWalletOperation), so hosts see the explanatory message but not a distinct result code yet; surfacing a dedicated code (like #3862's code 17 for identity-create) is a follow-up.How Has This Been Tested?
cargo check -p platform-wallet --features shielded --all-targetsand--all-features --testscargo clippy -p platform-wallet --features shielded --all-targets— cleancargo test -p platform-wallet --features shielded— 220 passed, 0 failedcargo check -p platform-wallet-ffi --all-features,-p platform-wallet-storage --all-features,-p dash-sdk --all-targets(downstream consumers)cargo fmt --allBreaking Changes
None (new error variant on a non-exhaustively-matched enum; no consensus changes).
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI / UX
Behavioral
Tests