docs(platform-wallet): error-code registry for the FFI result space - #4318
Open
bfoss765 wants to merge 16 commits into
Open
docs(platform-wallet): error-code registry for the FFI result space#4318bfoss765 wants to merge 16 commits into
bfoss765 wants to merge 16 commits into
Conversation
Four open PRs allocate discriminants into PlatformWalletFFIResultCode in parallel, with no shared source of truth. A duplicate discriminant does not produce a textual merge conflict, so the second merge silently misclassifies errors on every host — one such collision (code 29) has already required a renumber directive in review. Adds packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md: the merged allocation table (0-26, 98, 99), the proposed allocations with owning PRs (27-33), the rule for claiming the next free integer, the ABI-stability rule (never renumber after release, deprecate instead), and the currently contested allocations. Links it from the crate README's error-handling section. Docs only — no enum, mapping, or mirror is changed. Renumbering remains the owning PRs' work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ollisions The 29 collision is resolved and the renumber has now landed on #4185's branch: #4184 keeps 29 (ErrorAssetLockInsufficientFunds), #4185 takes 30 (ErrorReservationWalletMismatch). Table rows updated to match the code. Fixes the "30 is both free and assigned" inconsistency: the next-free line claimed 27-33 were claimed while the table showed 30 unallocated. 30 is now genuinely allocated to #4185, so the two agree. Adds allocations the survey had omitted, verified 2026-08-01 by reading error.rs at the head of all 62 open PRs: - #3968 numbers 26/27/28 (Persister* + a pre-merge TransactionBroadcastRejected) -> contradicts merged ABI at 26 and collides with #4185 at 27 and 28 - #3954 numbers ErrorShutdownIncomplete = 27 -> collides with #4185 at 27 - #4259 carries ErrorSigningKeyUnavailable = 31, inherited from #4183 rather than a new allocation The same sweep confirms no open PR anywhere defines a code 30.
…cord #4196 scope Clears the two review blockers on #4261 and re-syncs the registry with what the code on each branch actually does, re-read at every head rather than trusted from this file. Blocker (a) — #3968 / #3954 / #4259 were described in prose but had no rows, which is exactly what rule 2 forbids. They now have them: - A "Non-conforming allocations" table for #3968 (26/27/28) and #3954 (27). These are deliberately kept out of the proposed table: each row is a claim to be withdrawn and reissued, not an allocation of record. - An inherited-code table for the 31 that #4204 and #4259 carry but did not allocate (#4183 owns it), so it is not double-counted. - #4196 is recorded as claiming no integer at all: it routes a new token-less `StaleReservation` variant through the existing `ErrorStaleReservationToken`. The #3968 half is the serious one and is called out as such. Its 28 is not a new claim — it *moves the already-shipped* `ErrorTransactionBroadcastRejected` off 26 to make room for its own persister code. Rule 3 forbids that: a host compiled against merged ABI returns 26 for a broadcast rejection, and after #3968 the same condition returns 28 while 26 means a transient persister failure. Neither branch's diff shows the contradiction. Blocker (b) — 30 marked both free and assigned was already resolved by the preceding commit; verified consistent here (30 is allocated to #4185 throughout, frontier is 34, and the one remaining "genuinely free" is past tense explaining why #4185 could take it). Also corrected, all verified against the branches: - Survey provenance had #4185 at `0b0d5c76d6` labelled "(post-renumber)". Wrong twice: that commit is the *parent* of the renumber `d854debb`, and the head has since moved to `6c37e8679e`. #4184, #4247 and #4256 SHAs refreshed too. - #4256 has now taken 30 (`9481e5783b`) and dropped its stale "30 is reserved for the consent code" rationale; the equivalent comments on #4183 and #4204 are flagged as still present. - #4184 has a comment-only drift: it reserves "Codes 27-28" but names three codes. Correct when the trio was 27/28/29; it is now 27/28/30. Its discriminant is right and is the resolution of record — only the prose is stale, and #4184 is left untouched. - The #4196 section now records why the restack has not happened: its three own commits conflict in 3 files / 10 hunks against #4185's head, and the registry redesign underneath it (mandatory `registered_height`, new `WalletRemoved` variant, owner-stamped funding token) makes it author work rather than conflict resolution. Its trio numbers come from the #4185 copy it carries, so the restack fixes 28 -> 30 for free; the number #4196 itself must chase is 27, not 30. Verified: cargo fmt --all -- --check clean; cargo test -p platform-wallet-ffi -p platform-wallet = 738 passed / 0 failed. Docs-only change.
#4268 merged `ErrorShutdownIncomplete = 27` into the v4.2-dev ABI on 2026-08-02, taking the number #4185 had held. #4185 and #4256 moved the deferred-token trio to the contiguous block 34-36 in response. Registry changes: * 27 enters the merged table, owned by #4268. * The proposed table moves the trio to 34/35/36 and marks 28 and 30 free but deliberately not reissued. Next free integer is now 37. * New "Collision history" section records all three numberings of the trio (26/27/28 -> 27/28/30 -> 34/35/36) and, more usefully, corrects this file's own reasoning: on 2026-08-01 it recorded #3954's `ErrorShutdownIncomplete = 27` as a non-conforming claim that had to be withdrawn because #4185's claim was older. Seniority among open PRs does not decide an ABI number — merging does. #3954 was closed, its work landed as #4268, and 27 is now merged ABI. The trio therefore moved above every claimed number rather than into the next free gap, so nothing currently in flight can hit it again. * #3968's 27 is re-characterised: it was a proposed-vs-proposed collision, and is now a contradiction of merged ABI. Its frontier is 37+. * #4196 is now two moves behind at 26/27/28; the doc reference it owns has to chase 34, not 27. * Records a mirror gap found while grepping for this move: #4256 declares `ErrorTransactionBuild` (32) and `ErrorTransactionSigning` (33) in Rust and maps both in Kotlin, but declares neither in Swift, so both reach Swift hosts as `.errorUnknown`. Rule 5's Swift clause; left for that PR's author. * Provenance re-verified against v4.2-dev `5d68612a45`, including the check that 32 and 33 were already taken — which is why the trio went to 34-36 and not 32-34. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… allocation frontier
…e 32 collision #4204 allocated 32 without a row here, colliding with #4247's ErrorTransactionBuild. Caught for real during the v41int13 integration as an E0081, not on paper — the first collision this file has actually stopped. #4204 moves to 37; frontier advances to 38. Also records that the code was unmirrored on both hosts, which had Kotlin misclassifying it as ReservationWalletMismatch wherever #4185's 32 was present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves the two blocking review findings and the live suggestions on #4261. Blocking — advance #3968's reissue frontier to 38. Code 37 is allocated to #4204, so the branch-specific guidance in the non-conforming section and in the 27/28 detail section could still have sent #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 #3417, #3549, #3992 and #4243 (all touch the crate without claiming an integer; #4243 maps new wallet errors onto the existing ErrorInvalidParameter). Removes #4240 and #4251, whose heads touch no file under this crate; removes #4258, merged on 2026-08-03; records #4264 as closed with its work carried by #4243. Also in this pass: - #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. - #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. #4183 and #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 #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.
Three more live review findings, all verified against current PR heads. Rule 5 named a `PlatformWalletResultCode.init(result:)` that does not exist — `init(result:)` belongs to the downstream `PlatformWalletError`. As written, a contributor could add the Swift raw case and the typed error handling and still omit `PlatformWalletResultCode.init(ffi:)`, which is where the generated C constant is recognised; that switch has a `default:` yielding `.errorUnknown`, so the omission compiles and silently loses the code's identity before typed handling sees it. Rule 5 now enumerates all three Swift sites and says how each one fails: (1) the raw case, (2) the `init(ffi:)` arm — silent, and (3) `PlatformWalletError` + its `init(result:)` arm — a hard compile error, since that switch is exhaustive with no `default:`. That third failure is exactly what #4204 is sitting on at `d78b940a03`. #4196 is no longer blocked. Its head moved to `12492e8c54`, the restack onto #4185 is done, #4185's head `8813e98533` is an ancestor, the trio reads 34/35/36, and the PR is MERGEABLE against v4.2-dev. Verified the numeric references it owns were carried too: the `StaleReservationToken` KDoc and `fromPlatformWalletNative` mapping in `DashSdkError.kt` both read 34, and the V2 broadcast KDoc in `ManagedCoreWallet.kt` reads 34 with the rest symbolic. `PlatformWalletError::StaleReservation` refers to the code symbolically and never carried a number. The section is now a resolution rather than an open item; the account of why the restack was hard is kept, since that was the substance of the delay. The code-30 sweep was overstated. "No PR anywhere defines a code 30" is false for the surveyed heads — #4185 and #4256 both did; that was the allocation, not a competing claim. It now reads "no PR unrelated to #4185 defines a code 30", which is the claim that actually supports the conclusion. The list of branches carrying the stale consent-code reservation is corrected to #4183, #4204 and #4256's pre-renumber rationale (#4247 was never one of them). Provenance and the proposed table pick up #4196's new head. markdownlint MD018/MD004 remain at 0.
Re-verified the whole document against the CURRENT `origin/v4.2-dev` (`97904ed2fc`), not the `f53e5eef0a` the review comment cited and not the `5d68612a45` this file was last compiled against. `ErrorSigningKeyUnavailable = 31` is merged ABI. It landed in `189a3abb1c` (#4183, stacked on #4191) together with its Rust C-facing discriminant and complete Swift and Kotlin mirrors — the raw case, the `init(ffi:)` arm, the typed `PlatformWalletError` case with its `init(result:)` arm, and Kotlin's `31 -> PlatformWallet.SigningKeyUnavailable`. Leaving it under "Proposed allocations", whose preamble explicitly permits renumbering, contradicted rule 3. Moved to the merged table. Four PRs merged into `v4.2-dev` on 2026-08-04 and this file still treated all four as open: #4191 (`0e2282b586`), #4183 (`189a3abb1c`), #4277 (`6704a41a85`), #4251 (`7afc8a8ff3`). Only #4183 claimed an integer; the other three claimed none, and #4277 is now recorded as the merged precedent for "touches error.rs but allocates nothing" (it routes TxMetadataPayloadTooLarge onto the existing ErrorInvalidParameter). Dependent sections updated so nothing implies 31 may still move: the frontier breakdown (unchanged at 38), the proposed table, the inherited-code table (31 is trunk now, not an inheritable claim), the collision-history bullet list, the no-new-code open-PR inventory, the 31-vs-33 note (collapsing 31 is no longer available; only #4256's 33 is still open), and the survey provenance plus the PR-heads-of-record table. Also refreshed, because a re-dated provenance section must not carry claims that are now false: #4204's and #4256's Swift mirror gaps are both closed, and the stale ErrorAssetLockCrossDomainConsentRequired comments are gone from every branch that carried them. Every discriminant, mirror, PR state, and SHA above was read from git or the GitHub API on 2026-08-04. The four merge SHAs were confirmed ancestors of `97904ed2fc`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…st outcomes Two corrections from the #4261 review round: - The code-32 collision peer was misidentified. #4204 defined ErrorShieldedInviteAlreadyClaimed = 32 (b6992a5) and #4247 defines ErrorTransactionBuild = 32 (error.rs:257) — that is the E0081 pair. ErrorReservationWalletMismatch never held 32; its history runs 26/28 -> 30 -> 36. - "silently wrong on every host" overstated it. The three host outcomes differ and only one is a misclassification; spell them out. Also describe the inherited-code table as reduced to one row and retained for provenance rather than gone, since the row is rendered right below. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three leading `#<number>` line starts survived the earlier sweep (lines 173, 177, 334) and markdownlint still read them as malformed ATX headings. markdownlint-cli2 now reports zero MD018 on this file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
requested review from
QuantumExplorer,
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
August 6, 2026 01:52
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds a central ChangesFFI error-code registry
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
|
🕓 Ready for review — 1 ahead in queue (commit e88336a) |
thepastaclaw
suggested changes
Aug 6, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The documented numeric allocations still match the active replacement branches, including their Rust, Swift, and Kotlin mirrors. However, the registry identifies closed PRs as the current allocation owners and records an obsolete base revision, which defeats this PR's stated purpose of providing an authoritative coordination point; refresh the ownership, inventory, and provenance before merging.
Source: reviewer backends gpt-5.6-sol (codex general) and gpt-5.6-sol (codex ffi-engineer); final verifier backend gpt-5.6-sol (codex). openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and not reviewer evidence.
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Opus: not run (deferred by blocker gate)
🔴 1 blocking
🤖 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-ffi/ERROR_CODE_REGISTRY.md`:
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:115-130: Replace closed allocation owners with the active successor PRs
This section is explicitly the allocation table for open PRs, but every named owner is now closed. The active in-repository successors are #4316 for code 29; #4310 for code 32, also carried by #4311; #4311 for code 33; #4308 for codes 34–36, also carried by #4309, #4310, and #4311; and #4313 for code 37. Their current heads retain the documented Rust discriminants and corresponding Swift/Kotlin mappings, so the numbers remain correct but ownership does not. The same stale references continue through the open-PR inventory, contested-allocation history, and provenance, which calls `97904ed2fc` the current base even though this PR's merge base is post-#4305 `b703f82ab5`. Because the README directs contributors here as the authoritative place to coordinate claims and mirror changes, linking claims to closed PRs sends updates and reconciliation work to the wrong branches. Refresh the proposed table and all dependent ownership/provenance references against the active successor heads.
The proposed-allocations table named the fork-era owners (#4184, #4185, #4204, #4247, #4256), all closed when the estate was recreated in-repository. Ownership now names the active successors (29 -> #4316, 32 -> #4310, 33 -> #4311, 34-36 -> #4308, 37 -> #4313, carriers updated), the no-code inventory is marked as the fork-era snapshot it is, and the provenance base is date-stamped instead of claiming to be current. Collision history keeps the fork-era numbers — it is record. 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: 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/ERROR_CODE_REGISTRY.md`:
- Line 301: Refresh the allocation-status documentation around the “Still
outstanding” entry and the August 4 open-PR inventory: replace stale `#4196`
references and head-table entries with the current successor PRs, or explicitly
mark those rows as historical and remove claims that they represent open PRs.
Keep the status consistent with the restack closure and fork-era replacement
notes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fae0d6d3-100f-4cf9-af8e-4cd2f069f39b
📒 Files selected for processing (2)
packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.mdpackages/rs-platform-wallet-ffi/README.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4318 +/- ##
============================================
- Coverage 87.78% 87.59% -0.20%
============================================
Files 2677 2707 +30
Lines 342371 345716 +3345
============================================
+ Hits 300551 302817 +2266
- Misses 41820 42899 +1079
🚀 New features to boost your workflow:
|
thepastaclaw
suggested changes
Aug 11, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The registry remains materially behind the target branch: several proposed owners are already merged or closed, and codes 37–41 are now shipped ABI while the document still allocates 37 elsewhere and advertises 38 as free. Both issues undermine this PR's stated purpose as the authoritative allocation source and must be synchronized before merge.
Source: reviewer backend gpt-5.6-sol (Codex general); final verifier backend gpt-5.6-sol (Codex verifier).
Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol; not reviewer evidence.
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 blocking
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 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-ffi/ERROR_CODE_REGISTRY.md`:
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:106-113: Account for the merged allocations at codes 37 through 41
The current `v4.2-dev` ABI assigns 37–40 to `ErrorDocumentNotForSale`, `ErrorDocumentPriceChanged`, `ErrorInsufficientIdentityCredits`, and `ErrorContestedNameNotTradable` from merged #4348 (`6373e00f0c`), and assigns 41 to `ErrorShieldedInsufficientBalance` from merged #4360 (`e0b8baa850`). The registry instead says 38 is the next free value and still assigns 37 to #4313's `ErrorShieldedInviteAlreadyClaimed`; #4313 remains open, is currently unmergeable, and its head still defines that variant as 37. Rebasing it onto the target would therefore produce a duplicate-discriminant collision with the shipped code 37. Add the merged 37–41 rows, advance the allocation frontier to 42, and record that #4313 must take a fresh value.
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:120-140: Replace closed allocation owners with the active successor PRs
(existing thread: https://github.com/dashpay/platform/pull/4318#discussion_r3726182822)
The ownership update still does not describe the current state. #4308 merged codes 34–36 into `v4.2-dev` as commit `438153da39`, so those codes belong in the merged table rather than under open proposals. #4310 and #4311 closed without merging on 2026-08-10, while #4316 closed on 2026-08-07 and its active code-29 continuation is #4361. The stale state continues later in the file: line 301 calls #4196 “Still outstanding” even though lines 383–396 describe its resolution as closed, and lines 565–590 present an August 4 head inventory as rows describing open PRs. Because this registry is the coordination source for new allocations, these entries direct contributors to inactive branches and misclassify shipped ABI as provisional. Move 34–36 into the merged table, point code 29 to #4361, remove or reserve abandoned 32/33 claims as appropriate, and rewrite the old inventory as explicitly historical or replace it with current heads.
The proposed-allocation era this file tracked has largely settled: #4308 merged the reservation trio (34-36, 438153d), #4348 took 37-40 (6373e00) and #4360 took 41 (e0b8baa) - all now in the merged table. Codes 29/32/33 lapse to RESERVED (their in-repo owners closed without merging), the shielded-invite claim loses 37 to merged ABI and takes the frontier on revival, the frontier moves 38 -> 42, the #4196 'still outstanding' note is corrected to its 2026-08-03 close-out, and the 2026-08-04 heads-of-record table is labeled the historical snapshot it is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/ERROR_CODE_REGISTRY.md (1)
145-150: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUpdate the stale frontier references to 42.
The merged table establishes 42 as the next allocatable integer at Lines 114-122. The proposed-allocation section still says the frontier is unchanged at 38, and the
#3968guidance still says to allocate from 38+. Codes 38–41 are already documented as merged, so those instructions can cause an ABI collision.Update the current references to 42 and 42+. Keep 38 only in clearly dated historical snapshots.
Proposed fix
- the frontier is unchanged at 38. + the frontier is 42. - from the frontier (38+) + from the frontier (42+)Also applies to: 230-236
🤖 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/ERROR_CODE_REGISTRY.md` around lines 145 - 150, Update the proposed-allocation guidance and frontier references in ERROR_CODE_REGISTRY.md from 38 to 42, including the `#3968` instruction to allocate from 42+. Preserve 38 only where it appears in clearly dated historical snapshots, and ensure the current frontier statement reflects 42 without changing merged-code history.
🤖 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/ERROR_CODE_REGISTRY.md`:
- Around line 131-133: Rewrite the affected lines in ERROR_CODE_REGISTRY.md so
each pull request reference, including `#4184` and `#4316`, follows normal prose
rather than appearing as the first token. Keep the references unescaped and
outside backticks so GitHub autolinking remains active, while preserving the
existing meaning and table details.
- Around line 103-110: Add typed SDK mappings for ErrorStaleReservationToken,
ErrorReservationTokenConsumed, ErrorReservationWalletMismatch,
ErrorDocumentNotForSale, ErrorDocumentPriceChanged,
ErrorInsufficientIdentityCredits, ErrorContestedNameNotTradable, and
ErrorShieldedInsufficientBalance in DashSdkError.kt and
PlatformWalletResult.swift, assigning ABI codes 34–41 consistently. Replace the
existing generic/unknown handling for these codes while preserving fallback
behavior for unmapped codes.
---
Outside diff comments:
In `@packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md`:
- Around line 145-150: Update the proposed-allocation guidance and frontier
references in ERROR_CODE_REGISTRY.md from 38 to 42, including the `#3968`
instruction to allocate from 42+. Preserve 38 only where it appears in clearly
dated historical snapshots, and ensure the current frontier statement reflects
42 without changing merged-code history.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b236099-1e70-41e5-ad12-bf6f1e79f81b
📒 Files selected for processing (1)
packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md
…ence (MD018) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
suggested changes
Aug 11, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The merged 34–41 ABI entries are now documented, but the registry still omits the active code-29 allocation, contains live instructions to allocate from the obsolete code-38 frontier, and presents code 42 as both proposed and free. These contradictions undermine the PR's stated goal of providing an authoritative collision-prevention registry, so three blocking documentation fixes remain.
Source: Codex general reviewer backend gpt-5.6-sol; Codex FFI engineer reviewer backend gpt-5.6-sol; final Codex verifier backend gpt-5.6-sol.
Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol; not reviewer evidence.
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 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-ffi/ERROR_CODE_REGISTRY.md`:
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:145: Resolve whether code 42 is allocated or free
The proposed-allocation table lists `37→42` for open PR #4313 and says that PR will renumber its Rust and host mappings to 42 when revived, while lines 114–122 simultaneously declare that any new code takes 42. Rule 1 says an integer listed as proposed is not free, so these instructions can lead two contributors to claim the same value. Either reserve 42 for #4313 now and advance the public frontier to 43, or remove the concrete 42 assignment and state that #4313 must claim whichever frontier is current when its branch is updated.
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:141: Replace closed allocation owners with the active successor PRs
(existing thread: https://github.com/dashpay/platform/pull/4318#discussion_r3726182822)
Code 29 is not merely an ownerless reservation after #4316 closed. Open PR #4361, at head `fc97ca38d07b7225723b1e54283f92ccc26e718e`, explicitly salvages this allocation: it defines `ErrorAssetLockInsufficientFunds = 29`, maps `29` to Kotlin's typed `PlatformWallet.AssetLockInsufficientFunds`, and pins the Rust discriminant in a test. Its Swift mirror is still incomplete—the PR changes no Swift file, and its inherited `PlatformWalletResult.swift` contains only a comment about code 29, not the required raw case, `init(ffi:)` arm, typed error case, and `init(result:)` arm. Recording 29 as lapsed and ownerless hides both an active claim and the mirror work still required by rule 5. Record #4361 as the current owner and mark the Swift mirror incomplete; also update the surrounding statements that currently attribute the lapse to #4316.
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:147-150: Account for the merged allocations at codes 37 through 41
(existing thread: https://github.com/dashpay/platform/pull/4318#discussion_r3758342008)
The merged table and primary frontier statement now include codes 37–41, but this paragraph still says the frontier is unchanged at 38. The same live instruction remains at lines 233–234 and 390–392, where #3968 is told to allocate its persister errors from `38+`. On current `v4.2-dev`, codes 38–41 already belong to `ErrorDocumentPriceChanged`, `ErrorInsufficientIdentityCredits`, `ErrorContestedNameNotTradable`, and `ErrorShieldedInsufficientBalance`, with corresponding host mappings. Following these instructions would therefore assign duplicate ABI discriminants. Replace every current `38`/`38+` frontier directive with the post-41 frontier that remains free after resolving #4313's code-42 status; the explicitly dated August 4 survey at line 563 can retain 38 as historical state.
The shielded-invite row both promised 42 to #4313-on-revival and named 42 the next allocatable integer, letting two contributors claim the same value. The held PR now explicitly holds nothing; it takes whatever the frontier is at revival, recording the claim here first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
suggested changes
Aug 11, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The latest commit resolves the code-42 ambiguity by making clear that held PR #4313 owns no number and must claim the current frontier when revived. Two blocking registry contradictions remain: active PR #4361 owns code 29 despite the registry calling it ownerless, and several live instructions still advertise the obsolete code-38 frontier even though codes 38–41 are merged ABI.
Source: Codex general reviewer backend gpt-5.6-sol; Codex FFI engineer reviewer backend gpt-5.6-sol; final verifier backend gpt-5.6-sol; orchestration-only openclaw-agent/cliproxy/gpt-5.6-sol is not reviewer evidence.
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 blocking
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 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-ffi/ERROR_CODE_REGISTRY.md`:
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:141: Replace closed allocation owners with the active successor PRs
(existing thread: https://github.com/dashpay/platform/pull/4318#discussion_r3726182822)
Code 29 is not ownerless after #4316 closed. Open PR #4361, currently at `fc97ca38d07b7225723b1e54283f92ccc26e718e`, defines `ErrorAssetLockInsufficientFunds = 29`, maps native code 29 to Kotlin's typed `PlatformWallet.AssetLockInsufficientFunds`, and pins the Rust discriminant in a test. Its file list contains no Swift change, and its inherited `PlatformWalletResult.swift` only mentions code 29 in a comment; it lacks the raw case, generated-C conversion arm, typed error case, and result-conversion arm required by rule 5. Describing 29 as a lapsed, ownerless reservation hides both the active allocation and its incomplete Swift mirror. Record #4361 as the current owner, mark the Swift mirror incomplete, and update the surrounding statements that attribute the final lapse to #4316.
- [BLOCKING] packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:147-150: Account for the merged allocations at codes 37 through 41
(existing thread: https://github.com/dashpay/platform/pull/4318#discussion_r3758342008)
The merged table and primary frontier statement correctly account for codes 37–41, but this live paragraph still says the frontier is unchanged at 38. The same obsolete instruction appears at lines 233–234 and 390–392, where #3968 is directed to allocate its persister errors from `38+`. Current `v4.2-dev` assigns 38–41 to `ErrorDocumentPriceChanged`, `ErrorInsufficientIdentityCredits`, `ErrorContestedNameNotTradable`, and `ErrorShieldedInsufficientBalance`, and the corresponding host mappings are present. Following these instructions would assign duplicate public ABI discriminants. Replace the current `38` and `38+` frontier directives with `42` and `42+`; the explicitly dated August 4 survey may retain 38 as historical state.
…ontier reference The registry called 29 lapsed, but #4361's head declares ErrorAssetLockInsufficientFunds = 29 — the typed asset-lock shortfall carried the allocation forward when #4316 closed. Row 29 now records the live owner and lineage. The remaining instructions that still pointed PR #3968 (and any new allocation) at the obsolete 38 frontier now point at the frontier note, and the 2026-08-04 verification checklist is date-stamped as the historical snapshot it is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Continues #4261 — moved from a fork branch to an in-repo branch (rebased onto v4.2-dev post-#4305) so maintainers can push changes directly, per review request. Full review history on #4261.
Problem
PlatformWalletFFIResultCode(packages/rs-platform-wallet-ffi/src/error.rs) is public ABI —cbindgenemits the integers into the generated C header, Swift mirrors them inPlatformWalletResultCode, and Kotlin branches on them infromPlatformWalletNative. Four open PRs are currently allocating adjacent integers in that enum with no shared source of truth.A duplicate discriminant in two branches produces no textual merge conflict. The second merge just silently misclassifies errors on every host. That is not hypothetical here:
29is already claimed by two PRs, and review on #4185 had to issue a renumber directive (ErrorReservationWalletMismatch29 → 30) to resolve it. Three separate PRs now carry hand-written comments trying to reserve ranges on each other's behalf, and one of those reservations is for a variant that no longer exists.This PR adds a registry so the allocation is decided in one file instead of in four diffs.
What's here
packages/rs-platform-wallet-ffi/ERROR_CODE_REGISTRY.md:Plus a six-line pointer from the crate README's Error Handling section.
Docs-only. Zero code risk. No enum, no
Frommapping, no Swift/Kotlin mirror is touched, and nothing is renumbered — that stays the owning PRs' work. This PR only writes down what the branches already say.Codes claimed by the in-flight PRs
Reconciling in one place, so this is the table to correct:
ErrorStaleReservationTokenErrorReservationTokenConsumedErrorReservationWalletMismatchErrorAssetLockInsufficientFundsErrorSigningKeyUnavailableErrorTransactionBuildErrorTransactionSigningOpen PRs touching this crate that claim no new code: #4186, #4191, #4194, #4195, #4240, #4251, #4258. Next free integer: 34.
rs-sdk-ffi'sDashSDKErrorCodeis a separate integer space and is not modified by any of these PRs.Surveying the branches turned up a second collision that does not appear to have been raised in review.
#4196 branched before
26 = ErrorTransactionBroadcastRejectedmerged tov4.2-dev(in9302c62e8b), and its head numbers the reservation trio 26 / 27 / 28:So as it stands, #4196 would give
26two meanings on merge, and it also contradicts #4185 — the PR it is stacked on — which numbers those same three names 27 / 28 / 29. The fix is a rebase onto currentv4.2-devplus adopting whatever numbering #4185 lands with; no new integers are needed. Flagging rather than fixing, per the docs-only scope.Code 30 is free, despite what three branches say
ErrorAssetLockCrossDomainConsentRequiredis named as the holder of30in in-tree comments on #4183, #4204, and in #4247/#4256's numbering rationale. It is not defined anywhere — #4184, the PR that would have introduced it, dropped it in a re-scope.30is therefore genuinely free, and is the slot the #4185 renumber should take. Those stale comments should be dropped by whichever PR touches them next.Ambiguity left open for maintainers
Review on #4256 suggested mapping its signing failure onto
31; #4256 declined and took33, arguing that31(ErrorSigningKeyUnavailable, #4183) asserts a narrower contract — the signer holds no usable private key for a requested public key — whileBuilderError::SigningFailedalso covers unresolved derivation paths, sighash failures, and malformed signature encodings. Both are currently allocated. Collapsing them is a reasonable call, but it belongs to #4183 and #4256 jointly; the registry records the split as-is rather than pre-judging it.Please correct the table
The merged rows were read straight from
v4.2-devated4116b26c. The proposed rows were read from each PR's head commit (SHAs listed in the doc's provenance section) and will go stale as those PRs are updated. If a maintainer has a different intent for any allocation — particularly30, and the31/33split — please say so on this PR and I'll amend, or edit the file directly. The point is that there is now one place where that correction lands.🤖 Generated with Claude Code
Summary by CodeRabbit