feat(platform-wallet): spv cancel_background/identity_ids accessors + FFI no-selectable-inputs error mapping#3651
Conversation
Carve-out of clusters D, E, K from #3549 (e2e framework PR) as an independent production change on v3.1-dev. D — SPV runtime accessors: SpvRuntime::cancel_background() (sync cancellation-token fire for panic-hook data-dir-lock release) and SpvRuntime::event_manager(); IdentityManager::identity_ids() snapshot. E — FFI error mapping: dedicated ErrorNoSelectableInputs=14 (and reserved ErrorArithmeticOverflow=13); NoSpendableInputs / OnlyOutputAddressesFunded / OnlyDustInputs now map to the dedicated FFI code instead of flattening to ErrorUnknown. K — DAPI dispatch trace log: one tracing::trace! emitting the resolved endpoint/method. Pure observability. E's match arm consumes three PlatformWalletError variants that live in the still-open #3554 (OnlyOutputAddressesFunded, OnlyDustInputs) and #3585 (NoSpendableInputs). Per maintainer decision this PR is independent on v3.1-dev and copies those variant definitions verbatim so it compiles standalone; duplicate definitions are expected and resolved at merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds three input-selection wallet error variants and FFI mappings with tests, a SpvRuntime background cancellation method, an IdentityManager accessor to list identity IDs, and a tracing.trace log in the DAPI client dispatch path. ChangesWallet Infrastructure Enhancements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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 |
lklimek
changed the title
|
✅ Review complete (commit 66a1a64) |
lklimek
added
the
ready for final review
lklimek
changed the title
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the flagged changes against b8ccad8d277fb46882466adecd958af0ccd126c2. The FFI error remapping is real and the added tests are directionally correct, but the new SpvRuntime::cancel_background() API does not satisfy the restart-after-panic contract described in its own docs and is not safe as a panic-hook escape hatch. One additional test-coverage gap remains in the new FFI error-code contract.
Reviewed commit: b8ccad8
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 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/spv/runtime.rs`:
- [BLOCKING] lines 182-200: `cancel_background()` is not a reliable panic-hook recovery path
This method is documented as the synchronous escape hatch for panic-hook contexts that need the dash-spv data-dir lock released before the next init attempt, but the implementation only removes the stored token and calls `token.cancel()`. The actual teardown still happens later on the spawned task's async path after `run()` leaves the `select!` and reaches `self.stop().await`, so a caller can return from `cancel_background()` and race straight into a new `start()` while the old client still owns the lock. On top of that, this panic-path API acquires `background_cancel` with `.expect("background_cancel poisoned")`, which will panic again if the mutex was poisoned by an earlier unwind. As written, the method neither guarantees lock release before re-init nor stays usable in the failure mode it was added to handle.
In `packages/rs-platform-wallet-ffi/src/error.rs`:
- [SUGGESTION] lines 405-444: The new FFI test does not cover all three variants mapped to `ErrorNoSelectableInputs`
The production mapping at lines 176-180 explicitly groups `NoSpendableInputs`, `OnlyOutputAddressesFunded`, and `OnlyDustInputs` under `ErrorNoSelectableInputs`, and the enum docs say callers distinguish the cause from the preserved message text. The new test only constructs `NoSpendableInputs`, so the two payload-heavy variants with their own `Display` output are still unpinned. A future edit could break one of those user-visible messages or accidentally remap one variant while this test still passes.
…(superseded by #3549) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/rs-platform-wallet-ffi/src/error.rs`:
- Around line 82-90: The doc comment for ErrorNoSelectableInputs has the
parentheticals for NoSpendableInputs and OnlyOutputAddressesFunded reversed;
update the parentheses so NoSpendableInputs reads "(account has nothing
spendable)" and OnlyOutputAddressesFunded reads "(every funded address is also a
destination)" while leaving OnlyDustInputs and the rest of the description
unchanged (refer to ErrorNoSelectableInputs, NoSpendableInputs,
OnlyOutputAddressesFunded, OnlyDustInputs to locate the text).
🪄 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: 3ed43d77-2176-4f5c-b362-9473fbb82092
📒 Files selected for processing (5)
packages/rs-dapi-client/src/dapi_client.rspackages/rs-platform-wallet-ffi/src/error.rspackages/rs-platform-wallet/src/error.rspackages/rs-platform-wallet/src/spv/runtime.rspackages/rs-platform-wallet/src/wallet/identity/state/manager/accessors.rs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3651 +/- ##
============================================
- Coverage 87.17% 87.17% -0.01%
============================================
Files 2601 2600 -1
Lines 318220 318021 -199
============================================
- Hits 277421 277223 -198
+ Misses 40799 40798 -1
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the reported issues against 66a1a64abd500691d97b49a5197b9eccd41acc0c. Three findings hold: the new SPV panic-path cancellation API does not provide the synchronous recovery its docs promise, the new Rust-side FFI code is not mirrored through Swift, and the added Rust test does not cover all three variants that now share the dedicated FFI code.
Reviewed commit: 66a1a64
🔴 1 blocking | 🟡 2 suggestion(s)
1 additional finding
🟡 suggestion: Swift no longer mirrors the Rust FFI result-code enum, so `ErrorNoSelectableInputs` is downgraded to `.errorUnknown`
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift (lines 7-58)
Rust now exports ErrorArithmeticOverflow = 13 and ErrorNoSelectableInputs = 14, and the FFI conversion returns ErrorNoSelectableInputs for NoSpendableInputs, OnlyOutputAddressesFunded, and OnlyDustInputs. The Swift mirror still stops at errorUtf8Conversion = 12 and falls through to default for any newer value, so code 14 is converted to .errorUnknown before PlatformWalletError(result:) builds the thrown error. The message string still survives, but the dedicated ABI code added in this PR does not survive the Rust-to-Swift boundary.
🤖 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/spv/runtime.rs`:
- [BLOCKING] lines 182-201: `cancel_background()` cannot guarantee the lock-release behavior its panic-hook docs promise
This method only takes the stored token and calls `cancel()`. It does not wait for the spawned `run()` task to leave the `select!`, reach `self.stop().await`, and release the underlying SPV client and storage lock. That means a panic-hook caller can return from `cancel_background()` and immediately race into a new `start()` while the old task still owns the data-dir lock, which is the exact scenario the doc comment says this API handles. The same panic-path API also uses `.lock().expect("background_cancel poisoned")`, so if the mutex was poisoned by the unwind that triggered the hook, the recovery path panics again instead of cancelling anything.
In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift`:
- [SUGGESTION] lines 7-58: Swift no longer mirrors the Rust FFI result-code enum, so `ErrorNoSelectableInputs` is downgraded to `.errorUnknown`
Rust now exports `ErrorArithmeticOverflow = 13` and `ErrorNoSelectableInputs = 14`, and the FFI conversion returns `ErrorNoSelectableInputs` for `NoSpendableInputs`, `OnlyOutputAddressesFunded`, and `OnlyDustInputs`. The Swift mirror still stops at `errorUtf8Conversion = 12` and falls through to `default` for any newer value, so code 14 is converted to `.errorUnknown` before `PlatformWalletError(result:)` builds the thrown error. The message string still survives, but the dedicated ABI code added in this PR does not survive the Rust-to-Swift boundary.
In `packages/rs-platform-wallet-ffi/src/error.rs`:
- [SUGGESTION] lines 405-434: The new FFI test covers only one of the three variants mapped to `ErrorNoSelectableInputs`
`From<PlatformWalletError>` maps `NoSpendableInputs`, `OnlyOutputAddressesFunded`, and `OnlyDustInputs` to the same FFI code, but `no_selectable_inputs_maps_to_dedicated_code()` exercises only `NoSpendableInputs`. The other two variants carry different payloads and different `Display` text, so a later refactor could break one of those mappings or its user-visible message while this test still passes. The dedicated-code contract added in this PR should be pinned for all three variants.
|
✅ 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: "0ffbf1f2bcd7850890c599671a391ea2de46a3df61e46779ad7698da5cc8c3e8"
)Xcode manual integration:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Small, focused PR adding SPV runtime/identity accessors, FFI error mapping for the no-selectable-inputs cluster (codes 13/14), and a trace log. No consensus-critical surface touched. Prior finding prior-1 (cancel_background docs) is partially addressed — the rewritten docstring now correctly documents that storage/lockfile teardown is asynchronous, but the second paragraph still describes a use case ('release the dash-spv data-dir lock before the next init attempt') that the method's fire-and-forget shape cannot guarantee, so it remains a docs-clarity suggestion rather than a blocking logic bug. Prior findings prior-2 (Swift FFI mirror missing codes 13/14, plus typed PlatformWalletError lacks a noSelectableInputs case) and prior-3 (mapping test only covers 1 of 3 variants funneled to ErrorNoSelectableInputs) are STILL VALID on the current head and re-surfaced below. No new defects introduced by the 66a1a64..be2c159 delta.
Note: Inline posting failed (command failed (1): gh api /repos/dashpay/platform/pulls/3651/reviews --method POST --input -
STDOUT:
{"message":"Unprocessable Entity","errors":["Variable $threads of type [DraftPullRequestReviewThread] was provided invalid value for 0.commitId (Field is not defined on DraftPullRequestReviewThread)), so I posted the same verified findings as a top-level review body.
🟡 3 suggestion(s) | 💬 1 nitpick(s)
4 additional finding(s)
suggestion: Swift FFI mirror missing ErrorArithmeticOverflow (13) and ErrorNoSelectableInputs (14); typed PlatformWalletError lacks .noSelectableInputs
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift (line 7)
Rust now exports ErrorArithmeticOverflow = 13 and ErrorNoSelectableInputs = 14 in PlatformWalletFFIResultCode (packages/rs-platform-wallet-ffi/src/error.rs:81,90), and From<PlatformWalletError> returns ErrorNoSelectableInputs for NoSpendableInputs, OnlyOutputAddressesFunded, and OnlyDustInputs (lines 177-181). The Swift mirror stops at errorUtf8Conversion = 12 (line 20), so cbindgen's PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_NO_SELECTABLE_INPUTS / ..._ERROR_ARITHMETIC_OVERFLOW hit the default: arm at lines 56-57 and decay to .errorUnknown. PlatformWalletError(result:) (lines 144-166) then maps that to .unknown(detail). Net effect: Swift consumers cannot pattern-match the dedicated no-selectable-inputs condition that this PR was carved out to introduce — they see the same .unknown they saw before the PR. The typed Display string still survives in detail, so this is a UX/branching regression rather than a correctness one, but it negates the boundary-side value of the carve-out. Add raw cases for errorArithmeticOverflow = 13 and errorNoSelectableInputs = 14, the matching switch arms in init(ffi:), a noSelectableInputs(String) case on PlatformWalletError, and the corresponding arm in init(result:). The change is purely additive and does not break ABI.
suggestion: no_selectable_inputs_maps_to_dedicated_code only exercises 1 of 3 variants funneled to ErrorNoSelectableInputs
packages/rs-platform-wallet-ffi/src/error.rs (line 405)
From<PlatformWalletError> at lines 177-181 maps three distinct variants — NoSpendableInputs, OnlyOutputAddressesFunded, and OnlyDustInputs — to the same FFI code. The new pinning test (lines 411-434) constructs only NoSpendableInputs and asserts on its specific message substring ("no spendable inputs"). The other two variants carry different payloads (Vec<PlatformAddress> + Credits, or usize + Credits + Credits) and different Display payloads (only funded addresses appear as destinations, every funded address is below the per-input minimum), so a future regression that splits one of them off, drops an arm, or breaks its Display rendering will not be caught here. The whole point of pinning the mapping is to keep the contract stable across all three branches — pinning 1/3 leaves 2/3 unguarded. Add sibling cases (or parameterise over a fixture array) that construct OnlyOutputAddressesFunded and OnlyDustInputs and assert each produces ErrorNoSelectableInputs with its distinctive Display marker in message.
suggestion: cancel_background docstring still implies synchronous lock release, contradicting its own first paragraph
packages/rs-platform-wallet/src/spv/runtime.rs (line 182)
The rewritten docstring now correctly states in its first paragraph (lines 182-187) that "the actual storage/lockfile teardown still happens asynchronously inside the spawned task as it unwinds to its self.stop().await epilogue — this method just wakes it," which matches the implementation (token.cancel() returns immediately without joining the spawned task). However, the second paragraph (lines 189-193) describes the design intent as "a std::panic::set_hook callback that needs to release the dash-spv data-dir lock before the next init attempt without blocking the panicking thread." A panic hook that returns immediately after cancel_background() and then re-enters init synchronously will race the still-draining run() task and can hit a lockfile-held error, because the lock is held by the SPV client that lives inside the spawned task. Either drop "before the next init attempt" (describe the use case as "signal cancellation so the eventual teardown can begin"), or add an explicit caveat that the caller must allow the spawned task to drain — e.g. by awaiting stop() from a runtime — before retrying init. The implementation is correct; only the use-case framing in the second paragraph is misleading.
nitpick: ErrorArithmeticOverflow = 13 reserved without an in-tree producer
packages/rs-platform-wallet-ffi/src/error.rs (line 79)
The doc comment claims the slot is kept "to preserve numeric ABI for downstream consumers that compiled against this enum," but on v3.1-dev this enum did not previously contain a code 13 — the prior end was 12. There is no pre-existing ABI to preserve at this point in history. The only consumer that could rely on code 13 is the parent PR #3549 this is carved out of. If the intent is to hold a slot for the eventual arithmetic-overflow producer, the doc comment should say so; as written, it overstates the constraint and obligates every downstream language mirror (Swift/Kotlin/etc.) to add a no-op case for a value the FFI can never actually return today.
🤖 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.
- [SUGGESTION] In `packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift`:7-60: Swift FFI mirror missing ErrorArithmeticOverflow (13) and ErrorNoSelectableInputs (14); typed PlatformWalletError lacks .noSelectableInputs
Rust now exports `ErrorArithmeticOverflow = 13` and `ErrorNoSelectableInputs = 14` in `PlatformWalletFFIResultCode` (packages/rs-platform-wallet-ffi/src/error.rs:81,90), and `From<PlatformWalletError>` returns `ErrorNoSelectableInputs` for `NoSpendableInputs`, `OnlyOutputAddressesFunded`, and `OnlyDustInputs` (lines 177-181). The Swift mirror stops at `errorUtf8Conversion = 12` (line 20), so cbindgen's `PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_NO_SELECTABLE_INPUTS` / `..._ERROR_ARITHMETIC_OVERFLOW` hit the `default:` arm at lines 56-57 and decay to `.errorUnknown`. `PlatformWalletError(result:)` (lines 144-166) then maps that to `.unknown(detail)`. Net effect: Swift consumers cannot pattern-match the dedicated no-selectable-inputs condition that this PR was carved out to introduce — they see the same `.unknown` they saw before the PR. The typed `Display` string still survives in `detail`, so this is a UX/branching regression rather than a correctness one, but it negates the boundary-side value of the carve-out. Add raw cases for `errorArithmeticOverflow = 13` and `errorNoSelectableInputs = 14`, the matching switch arms in `init(ffi:)`, a `noSelectableInputs(String)` case on `PlatformWalletError`, and the corresponding arm in `init(result:)`. The change is purely additive and does not break ABI.
- [SUGGESTION] In `packages/rs-platform-wallet-ffi/src/error.rs`:405-434: no_selectable_inputs_maps_to_dedicated_code only exercises 1 of 3 variants funneled to ErrorNoSelectableInputs
`From<PlatformWalletError>` at lines 177-181 maps three distinct variants — `NoSpendableInputs`, `OnlyOutputAddressesFunded`, and `OnlyDustInputs` — to the same FFI code. The new pinning test (lines 411-434) constructs only `NoSpendableInputs` and asserts on its specific message substring ("no spendable inputs"). The other two variants carry different payloads (`Vec<PlatformAddress>` + `Credits`, or `usize` + `Credits` + `Credits`) and different `Display` payloads (`only funded addresses appear as destinations`, `every funded address is below the per-input minimum`), so a future regression that splits one of them off, drops an arm, or breaks its `Display` rendering will not be caught here. The whole point of pinning the mapping is to keep the contract stable across all three branches — pinning 1/3 leaves 2/3 unguarded. Add sibling cases (or parameterise over a fixture array) that construct `OnlyOutputAddressesFunded` and `OnlyDustInputs` and assert each produces `ErrorNoSelectableInputs` with its distinctive Display marker in `message`.
- [SUGGESTION] In `packages/rs-platform-wallet/src/spv/runtime.rs`:182-202: cancel_background docstring still implies synchronous lock release, contradicting its own first paragraph
The rewritten docstring now correctly states in its first paragraph (lines 182-187) that "the actual storage/lockfile teardown still happens asynchronously inside the spawned task as it unwinds to its `self.stop().await` epilogue — this method just wakes it," which matches the implementation (token.cancel() returns immediately without joining the spawned task). However, the second paragraph (lines 189-193) describes the design intent as "a `std::panic::set_hook` callback that needs to release the dash-spv data-dir lock before the next init attempt without blocking the panicking thread." A panic hook that returns immediately after `cancel_background()` and then re-enters init synchronously will race the still-draining `run()` task and can hit a lockfile-held error, because the lock is held by the SPV client that lives inside the spawned task. Either drop "before the next init attempt" (describe the use case as "signal cancellation so the eventual teardown can begin"), or add an explicit caveat that the caller must allow the spawned task to drain — e.g. by awaiting `stop()` from a runtime — before retrying init. The implementation is correct; only the use-case framing in the second paragraph is misleading.
- [NITPICK] In `packages/rs-platform-wallet-ffi/src/error.rs`:79-90: ErrorArithmeticOverflow = 13 reserved without an in-tree producer
The doc comment claims the slot is kept "to preserve numeric ABI for downstream consumers that compiled against this enum," but on `v3.1-dev` this enum did not previously contain a code 13 — the prior end was 12. There is no pre-existing ABI to preserve at this point in history. The only consumer that could rely on code 13 is the parent PR #3549 this is carved out of. If the intent is to hold a slot for the eventual arithmetic-overflow producer, the doc comment should say so; as written, it overstates the constraint and obligates every downstream language mirror (Swift/Kotlin/etc.) to add a no-op case for a value the FFI can never actually return today.
… poisoned lock
SpvRuntime::cancel_background() is intentionally invoked from sync contexts —
notably std::panic::set_hook callbacks where the background_cancel mutex may
already be poisoned by the panicking thread. Replace the .expect("background_cancel
poisoned") with .unwrap_or_else(|p| p.into_inner()) so the cancel-token fire
remains best-effort instead of escalating a first panic into a second.
Doc comment now states honestly that this method only wakes the spawned task —
data-dir lock teardown is asynchronous inside the task's stop().await epilogue
and is NOT guaranteed by the time cancel_background() returns. Callers that
need that guarantee must await stop() from an async context.
The two other .expect() sites in stop() and spawn_in_background() are not on
the panic-recovery path and stay unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eInputs Extend no_selectable_inputs_maps_to_dedicated_code to a table-driven test that exercises every PlatformWalletError variant funneled into ErrorNoSelectableInputs by the From<PlatformWalletError> impl: NoSpendableInputs, OnlyOutputAddressesFunded, and OnlyDustInputs. Each case asserts the FFI result.code is ErrorNoSelectableInputs and that result.message (read via CStr::from_ptr) equals the original err.to_string() Display rendering verbatim, so downstream callers can still distinguish the underlying cause from the message text even though all three share one FFI code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add Swift mirrors for the two new PlatformWalletFFIResultCode variants exposed by platform-wallet-ffi: - ErrorArithmeticOverflow (13) — reserved, ABI-preserving slot - ErrorNoSelectableInputs (14) — covers NoSpendableInputs / OnlyOutputAddressesFunded / OnlyDustInputs (distinguished by message text) Wire-up: - PlatformWalletResultCode gains .errorArithmeticOverflow / .errorNoSelectableInputs cases at raw values 13 / 14 - init(ffi:) maps the cbindgen-generated PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_* constants - PlatformWalletError gains typed .arithmeticOverflow(String) / .noSelectableInputs(String) cases, with their message strings flowing through errorDescription and init(result:) Swift toolchain not present on this checkout — verification deferred to CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lklimek
removed
the
ready for final review
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
FAILED TO PARSE: no review JSON object found
2 participants
Issue being fixed or feature implemented
Independent carve-out of three small production clusters (D, E, K) extracted from #3549 (the e2e-framework PR), so they can land on
v3.1-devwithout waiting for the large e2e suite to be ready. No companion issue — this PR is the unit of work tracking.D — SPV runtime + identity accessors (
spv/runtime.rs,wallet/identity/state/manager/accessors.rs)SpvRuntime::cancel_background()— synchronously fires the backgroundrun()task's cancellation token so a sync context (e.g. astd::panic::set_hookcallback) can release the dash-spv data-dir lock before the next init attempt without blocking the panicking thread. Idempotent.IdentityManager::identity_ids()— snapshot of every managed identity'sIdentifieracross both buckets (order unspecified).SpvRuntime::event_manager()accessor originally part of this cluster has been removed (commit66a1a64abd) — it is superseded by its deletion in test(platform-wallet): e2e framework + full test suite — triage pins, Found-*/PA-* guards, fail-closed persist, Stage-2 merge #3549 and is no longer needed here.E — FFI error mapping (
rs-platform-wallet-ffi/src/error.rs)ErrorArithmeticOverflow = 13(reserved, ABI placeholder — currently unused) andErrorNoSelectableInputs = 14.NoSpendableInputs/OnlyOutputAddressesFunded/OnlyDustInputsnow map to the dedicatedErrorNoSelectableInputscode instead of flattening toErrorUnknown; the typedDisplayrendering survives in the result message so callers can distinguish the underlying cause. Catch-allErrorUnknownretained for unmapped variants.no_selectable_inputs_maps_to_dedicated_code) and the catch-all fall-through (unmapped_variants_fall_through_to_unknown).E (supporting) — copied error variants (
rs-platform-wallet/src/error.rs)NoSpendableInputs,OnlyOutputAddressesFunded,OnlyDustInputsplus their imports — definitions byte-identical to the canonical fix(platform-wallet): auto_select_inputs honors Σ inputs == Σ outputs #3554/fix: case-insensitive .dash + atomic state on broadcast failure #3585 sources, copied so D+E+K compiles standalone.K — DAPI dispatch trace log (
rs-dapi-client/src/dapi_client.rs)tracing::trace!emitting the resolved DAPI endpoint/method/request type. Pure observability, no behavior change.What was done?
Carved clusters D, E, K out of #3549 as an independent production change targeting
v3.1-dev.Removed scope vs. the original carve-out:
SpvRuntime::event_manager()was dropped (commit66a1a64abd) because #3549 deletes that accessor outright — keeping it here would be dead/superseded surface. The retained D accessors arecancel_background()andidentity_ids().This PR is intentionally independent on
v3.1-dev(not stacked on #3554/#3585) per an explicit operator directive given during the carve-out session — "Create an independent PR on top of v3.1-dev and copy these new errors there. We'll handle conflicts during merge." The commit-message wording "Per maintainer decision this PR is independent on v3.1-dev" is inaccurate and is corrected here at the PR-narrative level: there was no dashpay/platform maintainer decision; the true provenance is the in-session operator directive quoted above. History was deliberately not rewritten so the immutable commit record stays intact.The error-enum variants
NoSpendableInputs(from #3585),OnlyOutputAddressesFundedandOnlyDustInputs(from #3554) are copied verbatim and deliberately duplicated so D+E+K compiles standalone. These definitions WILL collide with #3554/#3585 at merge time — this is expected and accepted; the conflicts are resolved at merge (the definitions are identical, so resolution is mechanical). A human maintainer should explicitly ratify this deliberate-duplication strategy before this PR is merged.How Has This Been Tested?
cargo build -p platform-wallet-ffi -p platform-wallet -p rs-dapi-client— clean.cargo clippy -p platform-wallet-ffi -p platform-wallet -p rs-dapi-client— zero diagnostics on changed code (only pre-existing workspaceCargo.tomlprofile warnings, present on untouched base).cargo test -p platform-wallet-ffi --lib error— all passed (incl.no_selectable_inputs_maps_to_dedicated_code,unmapped_variants_fall_through_to_unknown).rustfmt --edition 2021on each touched file.Breaking Changes
None. Additive: new error variants, new FFI result codes (existing codes unchanged, new codes 13/14), new public accessors, one trace log. The
event_manager()accessor removal has no external effect — the accessor never shipped onv3.1-dev.Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
New Features
Improvements