Skip to content

docs(platform-wallet): error-code registry for the FFI result space #4261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bfoss765 wants to merge 11 commits into from
Closed

docs(platform-wallet): error-code registry for the FFI result space #4261

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bb37a70
docs(ffi): error-code registry for the FFI result space
bfoss765 Aug 1, 2026
690d464
docs(ffi): record the 29/30 resolution as landed; add missing 27/28 c…
bfoss765 Aug 1, 2026
6f2f2d0
docs(ffi): give the unlisted allocations rows; correct provenance; re…
bfoss765 Aug 2, 2026
e915e5c
docs(ffi): record #4268's merged 27 and the trio's move to 34-36
bfoss765 Aug 3, 2026
c6fb09a
docs(ffi): registry — vacated codes 28/30 are reserved, not free; one…
bfoss765 Aug 3, 2026
2d2c6c8
docs(ffi): register 37 = ErrorShieldedInviteAlreadyClaimed; record th…
bfoss765 Aug 3, 2026
9f3dab1
docs(ffi): one allocation frontier at 38; complete the open-PR inventory
bfoss765 Aug 3, 2026
7802342
docs(ffi): #4196 has restacked; spell out Swift's three mirror sites
bfoss765 Aug 3, 2026
ae21604
docs(ffi): record 31 as merged ABI; sweep the stale open-PR state
bfoss765 Aug 4, 2026
695ca92
docs(ffi): name ErrorTransactionBuild as the code-32 peer; qualify ho…
bfoss765 Aug 4, 2026
b7ba62e
docs(ffi): finish the MD018 sweep for leading PR references
bfoss765 Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 157 additions & 0 deletions packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
# FFI Error-Code Registry

Single source of truth for the integer values of
`PlatformWalletFFIResultCode` (`packages/rs-platform-wallet-ffi/src/error.rs`).

Every value in that enum is **public ABI**. `cbindgen` emits it into the
generated C header, and hosts compare against the integer — Swift
(`packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift`)
mirrors it as a `RawRepresentable` enum, Kotlin
(`packages/kotlin-sdk/.../errors/DashSdkError.kt`) branches on it in
`fromPlatformWalletNative`. A shipped host binary that was compiled against
one numbering keeps using that numbering.

This file exists because several feature branches allocate into the same
integer range in parallel. A duplicate discriminant in two branches does **not**
produce a textual merge conflict — the second merge silently misclassifies
errors on every host — so allocations have to be reconciled here, in one place,
rather than in each branch's diff.

@thepastaclaw thepastaclaw Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Duplicate Rust discriminants do not silently reach runtime

A combined Rust enum containing two variants with the same explicit value fails compilation with E0081, and Swift rejects duplicate raw enum values as well. A textual merge can therefore leave an invalid tree, but that tree cannot successfully build and silently misclassify runtime errors. The actual ABI hazard is that independent branches or releases can reuse or renumber an integer relative to hosts already compiled against another meaning, as #3968 currently does with shipped code 26. State that cross-branch and cross-version failure mode instead.

Suggested change
This file exists because several feature branches allocate into the same
integer range in parallel. A duplicate discriminant in two branches does **not**
produce a textual merge conflict — the second merge silently misclassifies
errors on every host — so allocations have to be reconciled here, in one place,
rather than in each branch's diff.
This file exists because several feature branches allocate into the same
integer range in parallel. A combined Rust enum with duplicate discriminants
fails to compile, but independent branches can still reuse or renumber an ABI
value relative to a host that has already compiled the other meaning. Such a
version mismatch silently misclassifies errors on that host, so allocations
have to be reconciled here, in one place, rather than in each branch's diff.

source: ['codex']

@bfoss765 bfoss765 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9f3dab1.

You are right, and the file contradicted itself on it: the preamble said a duplicate discriminant "silently misclassifies errors on every host", while the code-32 section said the opposite — "this one was not a paper conflict… produced a hard error[E0081]: discriminant value 32 assigned more than once".

The preamble now splits the two shapes explicitly:

  • Two different variant names on one integer — the merged enum fails to compile with E0081. Loud, but only after someone merges both branches into one tree; neither branch's own CI sees it, because neither branch contains both variants. This is how the code-32 collision was caught.
  • The same meaning moving to a different integer, or a host mirror left un-updated — nothing fails to compile, and a shipped host silently reads the new integer as whatever the old one meant. This is the failure the file mainly exists to prevent.

The collision-history paragraph that repeated the old claim was corrected the same way: it now says neither compiler ever sees the E0081 because neither tree contains both variants, rather than implying duplicates are inherently silent.

@thepastaclaw thepastaclaw Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 690d464 — Duplicate Rust discriminants do not silently reach runtime no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.


## Rules

1. **Claim the next free integer** from the table below — the first value not
listed as merged, proposed, or reserved. Do not reuse a gap unless this file
marks it free.
2. **Record the claim in this file in the same PR** that adds the variant. A PR
that adds a code without a row here is incomplete.
3. **Never renumber a code after it has shipped in a release.** Deprecate
instead: leave the row, mark it deprecated, and allocate a new integer. Codes
that are still only proposed (unmerged) may be renumbered to resolve a
collision; codes on `v4.2-dev` may not.
4. **Do not reuse a retired integer.** Mark it reserved and move on.
5. **Update the mirrors in the same PR**: the Rust enum, the Swift
`PlatformWalletResultCode` + its `init(result:)` switch, and — where the code
deserves typed handling — the Kotlin `fromPlatformWalletNative` mapping and
`DashSdkErrorTest`. Kotlin is allowed to be non-exhaustive: unmapped codes
fall through to `PlatformWallet.Generic(code, …)`, which preserves the
integer. Swift is exhaustive; an unmirrored code surfaces as
`.errorUnknown` there and loses its identity.

@thepastaclaw thepastaclaw Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Require the actual C-to-Swift result-code switch

The generated C enum is first converted by PlatformWalletResultCode.init(ffi:) at PlatformWalletResult.swift:75-138. That switch has a default that maps an omitted native constant to .errorUnknown. The registry instead names a nonexistent PlatformWalletResultCode.init(result:); the actual init(result:) belongs to the downstream PlatformWalletError conversion. A contributor could therefore add the Swift raw case and downstream error handling but omit init(ffi:), losing the native code's identity before typed handling sees it. Require all three Swift locations explicitly.

Suggested change
5. **Update the mirrors in the same PR**: the Rust enum, the Swift
`PlatformWalletResultCode` + its `init(result:)` switch, and — where the code
deserves typed handling — the Kotlin `fromPlatformWalletNative` mapping and
`DashSdkErrorTest`. Kotlin is allowed to be non-exhaustive: unmapped codes
fall through to `PlatformWallet.Generic(code, …)`, which preserves the
integer. Swift is exhaustive; an unmirrored code surfaces as
`.errorUnknown` there and loses its identity.
5. **Update the mirrors in the same PR**: the Rust enum; the Swift
`PlatformWalletResultCode`, its `init(ffi:)` switch that maps the generated C
constants, and `PlatformWalletError` + its `init(result:)` switch; and — where
the code deserves typed handling — the Kotlin `fromPlatformWalletNative`
mapping and `DashSdkErrorTest`. Kotlin is allowed to be non-exhaustive:
unmapped codes fall through to `PlatformWallet.Generic(code, …)`, which
preserves the integer. Swift's `init(ffi:)` has an unknown-value fallback;
omitting its mapping surfaces the new code as `.errorUnknown` and loses its
identity.

source: ['codex']

@bfoss765 bfoss765 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7802342.

You're right that rule 5 named a PlatformWalletResultCode.init(result:) that does not exist — init(result:) belongs to the downstream PlatformWalletError. The gap you describe is real and #4204 is sitting in it right now: at d78b940a03 it has the raw case and the init(ffi:) arm but no PlatformWalletError case, so the exhaustive init(result:) no longer compiles.

Rather than list the three locations, rule 5 now enumerates them and states how each one fails, since the two failure modes are opposite and that is the part worth remembering:

  1. PlatformWalletResultCode — the raw case.
  2. PlatformWalletResultCode.init(ffi:) — the arm mapping the generated C constant. Has a default: yielding .errorUnknown, so omitting it compiles fine and silently loses the code's identity before typed handling sees it.
  3. PlatformWalletError — the typed case and its init(result:) arm. That switch is exhaustive with no default:, so adding (1) without this makes it non-exhaustive and the Swift package stops compiling.

Kotlin's non-exhaustive Generic(code, …) fallback is unchanged and now sits after the three Swift sites rather than being interleaved with them.

@thepastaclaw thepastaclaw Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 690d464 — Require the actual C-to-Swift result-code switch no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

6. **Blocks 98–99 are terminal sentinels** (`NotFound`, `ErrorUnknown`) and are
not an allocation frontier. New codes go after the highest allocated value
below them.

## Merged allocations (`v4.2-dev`)

These are shipped ABI. Do not renumber.

| Code | Name | Notes |
| ---: | --- | --- |
| 0 | `Success` | |
| 1 | `ErrorInvalidHandle` | |
| 2 | `ErrorInvalidParameter` | |
| 3 | `ErrorNullPointer` | |
| 4 | `ErrorSerialization` | |
| 5 | `ErrorDeserialization` | |
| 6 | `ErrorWalletOperation` | |
| 7 | `ErrorIdentityNotFound` | |
| 8 | `ErrorContactNotFound` | |
| 9 | `ErrorInvalidNetwork` | |
| 10 | `ErrorInvalidIdentifier` | |
| 11 | `ErrorMemoryAllocation` | |
| 12 | `ErrorUtf8Conversion` | |
| 13 | `ErrorArithmeticOverflow` | Reserved slot — declared, no in-tree producer; holds the number for the mapping arriving via #3549 |

@thepastaclaw thepastaclaw Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: ErrorArithmeticOverflow already has an in-tree producer

The exact PR base already produces this code in packages/rs-platform-wallet-ffi/src/shielded_send.rs:204-213: platform_wallet_shielded_estimate_fee maps shielded fee-formula failures to ErrorArithmeticOverflow. That producer entered through commit c9f8ef57925 and is present on v4.1-dev; the current #3549 diff does not add this mapping. Calling the slot producerless and awaiting #3549 gives incorrect provenance for an ABI registry.

Suggested change
| 13 | `ErrorArithmeticOverflow` | Reserved slot — declared, no in-tree producer; holds the number for the mapping arriving via #3549 |
| 13 | `ErrorArithmeticOverflow` | Produced by `platform_wallet_shielded_estimate_fee` when shielded fee computation overflows |

source: ['codex']

@bfoss765 bfoss765 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9f3dab1.

Confirmed on v4.2-dev at 5d68612a45: packages/rs-platform-wallet-ffi/src/shielded_send.rs:210 returns PlatformWalletFFIResultCode::ErrorArithmeticOverflow, with the contract described at line 180. So the row's "declared, no in-tree producer" was wrong, and crediting #3549 with the eventual mapping was misleading.

The row now names shielded_send.rs as the producer. It also records that the variant's own rustdoc in error.rs still calls itself a reserved slot with no producer — that comment is stale for the same reason, and should be corrected by whichever PR touches it next. Left as a note rather than a code change since this PR is documentation-only.

@thepastaclaw thepastaclaw Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 690d464 — ErrorArithmeticOverflow already has an in-tree producer no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

| 14 | `ErrorNoSelectableInputs` | |
| 15 | `ErrorWalletAlreadyExists` | |
| 16 | `ErrorShieldedBroadcastFailed` | |
| 17 | `ErrorShieldedBroadcastUnconfirmed` | |
| 18 | `ErrorShieldedSpendUnconfirmed` | |
| 19 | `ErrorShieldedNoRecordedAnchor` | |
| 20 | `ErrorTransactionBroadcastUnconfirmed` | |
| 21 | `ErrorAddressNonceMismatch` | |
| 22 | `ErrorCoreInsufficientFunds` | |
| 23 | `ErrorAssetLockNotTracked` | |
| 24 | `ErrorAssetLockAlreadyConsumed` | |
| 25 | `ErrorAssetLockFundingMismatch` | |
| 26 | `ErrorTransactionBroadcastRejected` | Merged in `9302c62e8b`; took a number several open branches had been treating as free |
| 98 | `NotFound` | Sentinel — `Option` returned as an error |
| 99 | `ErrorUnknown` | Sentinel — unmapped/flattened errors |

**Next free integer: 34** (see the proposed table; 27–33 are claimed).

## Proposed allocations (open PRs)

Not yet ABI. Numbers here may still move; they move by agreement recorded in
this file.

| Code | Name | Owning PR | Status |
| ---: | --- | --- | --- |
| 27 | `ErrorStaleReservationToken` | #4185 | In review (also carried by #4256) |
| 28 | `ErrorReservationTokenConsumed` | #4185 | In review (also carried by #4256) |
| 29 | `ErrorReservationWalletMismatch` | #4185 | **Collision** — see below |
| 29 | `ErrorAssetLockInsufficientFunds` | #4184 | **Collision** — see below |
| 30 | — | — | **Unallocated.** Reserved in sibling comments only; see below |
| 31 | `ErrorSigningKeyUnavailable` | #4183 | In review (also carried by #4204) |
| 32 | `ErrorTransactionBuild` | #4247 | In review (also carried by #4256) |
| 33 | `ErrorTransactionSigning` | #4256 | In review |

Open PRs that touch `rs-platform-wallet-ffi` but claim **no** new code: #4186,
#4191, #4194, #4195, #4240, #4251, #4258.

@thepastaclaw thepastaclaw Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Registry omits incompatible allocations from active PRs

The open-PR inventory and survey provenance omit branches that modify this exact ABI enum. At current head 5931df745a, #3968 assigns ErrorPersisterTransient = 26, ErrorPersisterFatal = 27, and renumbers the already-shipped ErrorTransactionBroadcastRejected from 26 to 28. At head 93d0bd49b, #3954 assigns ErrorShutdownIncomplete = 27. This conflicts with shipped code 26 and with #4185's code-27 claim, while #3968 and #3954 also assign different meanings to 27. An existing Swift host would classify #3968's persister-transient code 26 as .errorTransactionBroadcastRejected, and its actual rejection code 28 would fall through init(ffi:) to .errorUnknown. #4259 at 64146a2bb6 should also be recorded as carrying the same code-31 allocation as #4183. The supposedly complete no-new-code inventory additionally omits open PRs #3417, #3549, #3992, and #4243. Reconcile the incompatible claims and recompute the next-free value before presenting this file as the source of truth.

source: ['codex']

@thepastaclaw thepastaclaw Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 690d464 — Registry omits incompatible allocations from active PRs no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

@bfoss765 bfoss765 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9f3dab1 — the last outstanding part of this finding.

Re-checked each sub-claim against 2d2c6c8 rather than trusting the earlier auto-resolve:

The inventory was rebuilt on 2026-08-03 from each PR's actual file list plus the error.rs at its head, and it now reads #3417, #3549, #3992, #4186, #4191, #4194, #4195, #4243. Four entries were also removed, each recorded with its reason so they are not silently re-added: #4240 and #4251 touch no file under this crate at their heads; #4258 merged into v4.2-dev on 2026-08-03; #4264 is closed, with its error.rs change carried by #4243. #4243 is called out explicitly — it does modify error.rs, but only to map new wallet errors onto the existing ErrorInvalidParameter, and this list tracks integer claims, not file touches.

@thepastaclaw thepastaclaw Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Make the no-new-code PR inventory complete and accurate

The list does not match the open PRs that modify this crate. GitHub's exact PR file lists show that #3417, #3549, #3992, #4243, and #4264 modify packages/rs-platform-wallet-ffi without adding a result-code integer, but they are omitted. #4243 and #4264 even modify error.rs, intentionally mapping new wallet errors to the existing ErrorInvalidParameter code. Conversely, the exact surveyed heads for listed PRs #4240 and #4251 contain no file under this crate. These discrepancies do not affect the next-free integer, but they make the registry's open-PR survey incomplete and inaccurate.

Suggested change
Open PRs that touch `rs-platform-wallet-ffi` but claim **no** new code: #4186,
#4191, #4194, #4195, #4240, #4251, #4258.
Open PRs that touch `rs-platform-wallet-ffi` but claim **no** new code: #3417,
#3549, #3992, #4186, #4191, #4194, #4195, #4243, #4258, #4264.

source: ['claude', 'codex']

@bfoss765 bfoss765 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9f3dab1.

Rebuilt the inventory on 2026-08-03 from each PR's file list and the error.rs at its head rather than applying the suggestion verbatim, because two of its entries had moved since it was written:

  • #4264 is closed. Its error.rs change — mapping new wallet errors onto the existing ErrorInvalidParameter — is carried by #4243, which is open and is in the list.
  • #4258 has merged into v4.2-dev (ce8233edb7). It claimed no code, so the merged table is unchanged, but it is no longer an open PR.

The list is now #3417, #3549, #3992, #4186, #4191, #4194, #4195, #4243. Your two removals are applied as written: #4240 and #4251 touch no file under this crate at their heads. All four removals are recorded in a short table with the reason for each, so a later pass does not re-add them from an older revision of this file. #4243's error.rs involvement is called out explicitly for the same reason — touching error.rs is not the same as claiming an integer.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

@thepastaclaw thepastaclaw Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Fix the MD018 warnings for leading PR references

Lines beginning directly with #<number> are interpreted by markdownlint as malformed ATX headings and trigger MD018. Wrap each leading PR reference in backticks or escape its hash. This affects lines 105, 115, 133, 137, 180, 185, 225, 231, 235, 247, 248, 283, 297, 359, 361, and 365–367.

source: ['coderabbit']

@thepastaclaw thepastaclaw Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 2d2c6c8 — Fix the MD018 warnings for leading PR references no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

@bfoss765 bfoss765 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9f3dab1.

Flagging that the auto-resolve on this thread was premature: markdownlint-cli2 still reported 18 MD018 violations at 2d2c6c8. The lines beginning with a bare #<number> were still there — 106, 116, 134, 138, 215, 220, 260, 266, 270, 282, 283, 318, 332, 394, 396, 400, 401, 402.

Fixed by rewording so the PR reference is no longer the first token on the line (PR #3968 must keep 26…, the #4185 numbering of…), rather than by escaping or code-fencing. That keeps GitHub's PR autolinks intact, which \#4196 or `#4196` would have broken.

markdownlint-cli2 now reports 0 MD018 on this file. MD004 also went to 0 in the same pass — the file had mixed */- top-level bullets, now all *. MD013 is down from 19 to 18, all of them long table rows.


## Contested and pending

### 29 — `ErrorReservationWalletMismatch` (#4185) vs `ErrorAssetLockInsufficientFunds` (#4184)

Both PR heads define code 29. This is the known collision: review on #4185
directed that PR to keep #4184's `29 = ErrorAssetLockInsufficientFunds` and move
`ErrorReservationWalletMismatch` to 30. That renumber has not landed on #4185's
head, and #4256 (stacked downstream) carries the pre-renumber `29`.

Resolution of record: **#4184 keeps 29; #4185 moves to 30**, propagated through
the Rust enum, the FFI `From` mapping, Swift `PlatformWalletResult`, Kotlin
`DashSdkError` (+ `DashSdkErrorTest`), and the JNI rustdoc — plus #4256, which
inherits the value.

### 30 — reserved in comments for a variant that no longer exists

`ErrorAssetLockCrossDomainConsentRequired` is named as the holder of 30 in
in-tree comments on #4183, #4204, and #4247/#4256's numbering rationale. It is
**not defined anywhere** — #4184, the PR that would have introduced it, does not
contain it after a re-scope. 30 is therefore free, and is the slot the #4185
renumber above should take. The stale "reserved for the consent code" comments

@thepastaclaw thepastaclaw Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Code 30 is marked free even though the registry assigns it to #4185

The table marks 30 as Unallocated, and the later section explicitly calls it free, while the resolution of record assigns that value to #4185's ErrorReservationWalletMismatch; the stated next-free value of 34 also assumes that ownership. Rule 1 permits contributors to claim a gap when this file marks it free, so another PR could legitimately take 30 while #4185 follows the recorded renumber directive. Record 30 as allocated to #4185 and describe its current use of 29 only as stale surveyed-head state.

source: ['codex']

@thepastaclaw thepastaclaw Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 690d464 — Code 30 is marked free even though the registry assigns it to #4185 no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

@bfoss765 bfoss765 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9f3dab1.

The contradiction was real and still live at 2d2c6c8, despite the earlier auto-resolve on this thread. The table rows and the frontier paragraph said RESERVED, but two other places said the opposite:

  • the code-30 section: "30 is free once more, and is deliberately not being reissued"
  • the collision history: "28 and 30 stay free. Do not reissue them in this review cycle"

Under rule 1 ("Do not reuse a gap unless this file marks it free") those two sentences licensed exactly the claim the rest of the file forbids, so the document carried two allocation frontiers at once.

All four places now say reserved-not-free and say it the same way. The code-30 heading is now "30 — vacated, then RESERVED (not free)", the body distinguishes vacated from freed, and the collision-history paragraph ends with an explicit pointer back to rule 1: reusing a gap requires this file to mark it free, and this file marks neither 28 nor 30 free. The frontier paragraph makes the same statement rather than just asserting 38.

should be dropped by whichever PR touches them next.

### 26 — `ErrorStaleReservationToken` on #4196 collides with merged ABI

#4196 (stacked on #4185) branched before `26 = ErrorTransactionBroadcastRejected`
merged, and its head numbers the reservation trio **26 / 27 / 28**. Merging it as
it stands would give 26 two meanings and would contradict #4185's own 27 / 28 / 29
for the same three names. #4196 needs a rebase onto current `v4.2-dev` and must
adopt whatever numbering #4185 lands with. No new integers are needed for it.

### 31 vs 33 — two signing-related codes, deliberately distinct

Review on #4256 suggested mapping its signing failure onto 31. #4256 declined and
took 33, on the grounds that 31 (`ErrorSigningKeyUnavailable`, #4183) asserts a
specific contract — the signer holds no usable private key for a requested public
key, restored from a typed signer completion code — whereas #4256's
`BuilderError::SigningFailed` also covers unresolved derivation paths, sighash
failures, and malformed signature encodings. Both codes are currently allocated.
Maintainers may still choose to collapse them; that decision belongs to #4183 and
#4256 jointly and should be recorded here.

## Sibling FFI crates

`rs-sdk-ffi`'s `DashSDKErrorCode` (`packages/rs-sdk-ffi/src/error.rs`) is a
**separate** integer space (0–10, plus `InternalError = 99`) and is not contested
by any of the PRs above — none of them modify it. Do not assume a number means
the same thing in both enums.

## Survey provenance

Compiled 2026-08-01 against `v4.2-dev` at `ed4116b26c` and the following PR
heads: #4183 `2cd948331b`, #4184 `a9e418af50`, #4185 `7d85953c2a`, #4186
`6f7abbadc1`, #4191 `8acb0bd14c`, #4194 `9efc0b7e3a`, #4195 `4f2eb06d64`, #4196
`ea4f783490`, #4204 `7bc8a845c6`, #4240 `9328609a16`, #4247 `72c000dcfd`, #4251
`176f8ed3eb`, #4256 `d8943ccf10`, #4258 `5adfc40032`. Rows describing open PRs
reflect those heads and go stale as the PRs are updated; the merged table does
not.
6 changes: 6 additions & 0 deletions packages/rs-platform-wallet-ffi/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ Error codes:
- `PLATFORM_WALLET_FFI_ERROR_CONTACT_NOT_FOUND` - Contact not found
- And more...

The result codes are **public ABI**: their integer values are consumed by the
generated C header and mirrored by the Swift and Kotlin SDKs. Before adding a
new code, read [ERROR_CODE_REGISTRY.md](ERROR_CODE_REGISTRY.md) — it holds the
authoritative integer→name allocation, the rule for claiming the next free
value, and the currently contested allocations across open PRs.

## Testing

Run the test suite:
Expand Down
Loading