feat(platform-wallet): expose an invitation's prospective identity id - #4332
Conversation
A spent DIP-13 voucher was only discoverable by attempting the claim: it surfaced as a raw "asset lock ... output N already completely used" after the invitee had picked a username and entered their PIN. Nothing lets a caller ask "is this invitation still good?" up front — there is no asset-lock-consumption query on the platform gRPC surface. Platform derives a created identity's id from the asset-lock outpoint, so the id an invitation *would* produce is knowable before the claim, and an identity already existing under it is exactly the "spent" signal. Adds `IdentityWallet::invitation_prospective_identity_id` (reusing the claim's own proof reconstruction, so the credit output is selected by pk-to-script match rather than assumed to be index 0) and the FFI/Swift wrappers. It hits the network — the funding transaction has to be refetched — but claims nothing and mutates no wallet state. Any failure is genuinely undetermined (wrong network, tx not yet propagated, transport error), which the docs state explicitly: callers must treat an error as "proceed", never as an answer either way.
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe wallet now derives the identity ID an invitation would create without claiming it. The capability is available through the Rust FFI and the asynchronous Swift wallet API. ChangesInvitation identity preflight
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SwiftClient
participant ManagedPlatformWallet
participant FFI
participant IdentityWallet
SwiftClient->>ManagedPlatformWallet: invitationProspectiveIdentityId(uri:)
ManagedPlatformWallet->>FFI: pass invitation URI
FFI->>IdentityWallet: derive prospective identity ID
IdentityWallet-->>FFI: return identifier or error
FFI-->>ManagedPlatformWallet: return identifier or error
ManagedPlatformWallet-->>SwiftClient: return Data or throw
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🕓 Ready for review — next in queue (commit 2e680ae) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rs-platform-wallet/src/wallet/identity/network/invitation.rs`:
- Around line 406-421: The documentation blocks are attached to the wrong
declarations. In
packages/rs-platform-wallet/src/wallet/identity/network/invitation.rs#L406-L421,
move the claim_invitation Rustdoc directly above claim_invitation, leaving the
prospective-ID documentation above invitation_prospective_identity_id; in
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift#L2211-L2226,
move the parseInvitation(uri:) documentation directly above
parseInvitation(uri:).
- Around line 421-437: Add automated tests covering
invitation_prospective_identity_id for a valid invitation, rejection when
voucher_key_network differs from the wallet network, selecting a credit output,
and invitations with no claim; verify the no-claim case does not mutate wallet
state. Reuse the existing invitation and wallet test fixtures/helpers, and
assert both returned errors/identifiers and wallet state where applicable.
🪄 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: 08479a46-d60b-4681-8c04-f5c9083896a1
📒 Files selected for processing (3)
packages/rs-platform-wallet-ffi/src/invitation.rspackages/rs-platform-wallet/src/wallet/identity/network/invitation.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift
The prospective-id declaration landed in the middle of the doc block above it, so `claim_invitation` and `parseInvitation(uri:)` lost their documentation entirely and the new API inherited it as a prefix. Move each block back onto the declaration it describes.
…output Output selection is already covered next to `voucher_output_index`; what was untested is that the derived id follows that selection. A voucher behind a decoy output must not produce the index-0 id — that would make the claimed-check answer about a stranger's identity and report a good voucher as spent.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The prospective-ID derivation correctly follows the voucher-controlled credit output, and the FFI/Swift ownership and pointer handling are sound. Two blocking API-contract issues remain: identity existence cannot detect invitations reclaimed through identity top-up, and deterministic malformed-link/wrong-network failures are documented and exposed like inconclusive transport failures.
Source: reviewers codex-general/gpt-5.6-sol, codex-ffi-engineer/gpt-5.6-sol, and codex-rust-quality/gpt-5.6-sol; final verifier codex-verifier/gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and 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),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 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/src/wallet/identity/network/invitation.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/invitation.rs:383-386: Identity existence does not detect every consumed invitation
An identity under the derived outpoint ID is not an exact signal that the asset lock has been consumed. The supported invitation-reclaim path in `platform_wallet_topup_identity_with_existing_asset_lock_signer` explicitly authorizes an `IdentityInvitation` lock with `consume_invitation_voucher: true` and consumes it through `IdentityTopUp`, which credits an existing identity rather than creating the derived identity. After that reclaim, fetching the prospective ID still returns no identity even though a claim will deterministically fail because the asset-lock output is already consumed. This recreates the late failure the new precheck is intended to prevent. The API must either query actual asset-lock consumption or explicitly limit its contract and consumer behavior to detecting consumption through identity creation; add coverage for reclaiming an invitation into an existing identity.
In `packages/rs-platform-wallet-ffi/src/invitation.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/invitation.rs:332-334: Definitive input errors are incorrectly documented as undetermined
Not every error from this function leaves the invitation's usability undetermined. `parse_invitation_uri` definitively rejects malformed links, while `invitation_prospective_identity_id` deterministically rejects a wallet-network mismatch; `claim_invitation` applies the same network guard, so the link cannot be claimed through the current wallet. Both currently reach Swift through the generic catch-all because `InvalidIdentityData` maps to `ErrorUnknown`, while the documentation tells callers to ignore every error and proceed. That sends users toward a claim that is already known to fail. Expose malformed-input and wrong-network failures through stable, distinguishable FFI/Swift result codes and reserve the “proceed because status is inconclusive” behavior for funding-transaction lookup and transport failures.
| /// Platform derives a created identity's id from the asset-lock outpoint, | ||
| /// so the id is knowable before the claim — and an identity already | ||
| /// existing under it is exactly the "this voucher has been spent" signal. | ||
| /// The claim itself is the only other way to learn that, which is why a |
There was a problem hiding this comment.
🔴 Blocking: Identity existence does not detect every consumed invitation
An identity under the derived outpoint ID is not an exact signal that the asset lock has been consumed. The supported invitation-reclaim path in platform_wallet_topup_identity_with_existing_asset_lock_signer explicitly authorizes an IdentityInvitation lock with consume_invitation_voucher: true and consumes it through IdentityTopUp, which credits an existing identity rather than creating the derived identity. After that reclaim, fetching the prospective ID still returns no identity even though a claim will deterministically fail because the asset-lock output is already consumed. This recreates the late failure the new precheck is intended to prevent. The API must either query actual asset-lock consumption or explicitly limit its contract and consumer behavior to detecting consumption through identity creation; add coverage for reclaiming an invitation into an existing identity.
source: ['codex']
| /// A failure here is genuinely undetermined — a wrong-network link, a tx that | ||
| /// has not propagated, a transport error — so callers must treat any error as | ||
| /// "proceed", never as "unclaimed" or "claimed". |
There was a problem hiding this comment.
🔴 Blocking: Definitive input errors are incorrectly documented as undetermined
Not every error from this function leaves the invitation's usability undetermined. parse_invitation_uri definitively rejects malformed links, while invitation_prospective_identity_id deterministically rejects a wallet-network mismatch; claim_invitation applies the same network guard, so the link cannot be claimed through the current wallet. Both currently reach Swift through the generic catch-all because InvalidIdentityData maps to ErrorUnknown, while the documentation tells callers to ignore every error and proceed. That sends users toward a claim that is already known to fail. Expose malformed-input and wrong-network failures through stable, distinguishable FFI/Swift result codes and reserve the “proceed because status is inconclusive” behavior for funding-transaction lookup and transport failures.
source: ['codex']
…initive errors Review follow-ups. **The precheck is one-way (blocking #1).** An identity at the derived id proves the voucher was claimed; its absence does NOT prove it is usable. The same lock can be consumed by `IdentityTopUp` — the reclaim path behind `platform_wallet_topup_identity_with_existing_asset_lock_signer` with `consume_invitation_voucher: true` — which credits an EXISTING identity and creates nothing at this id, so the check passes while a claim still fails deterministically. Platform exposes no client query for spent asset locks (drive tracks them under `SpentAssetLockTransactions`, but no DAPI endpoint surfaces it), so consumption cannot be checked here. Rather than imply a guarantee it cannot make, the contract now says so on both the library method and the FFI: reject on "identity exists", otherwise proceed WITHOUT concluding the voucher is good. It narrows the window for the late failure; it does not remove it. Covered by a test pinning the property that makes it one-way: the id is a pure function of the asset-lock outpoint, so it is unchanged by a reclaim that spends that same outpoint. That is the assumption a reader would otherwise make about the id encoding "spent". **Definitive errors are no longer reported as undetermined (blocking #2).** Two failures mean the link can never be claimed by this wallet, and both previously reached Swift through the catch-all while the docs said to ignore every error and proceed — steering users into a claim already known to fail: * malformed URI -> `ErrorInvalidParameter` * wrong network -> `ErrorInvalidNetwork` (checked at the FFI, as the withdrawal FFI does, so it gets a distinguishable code; `claim_invitation` applies the same guard and would refuse it too) Everything else — funding-tx propagation lag, transport failures — remains genuinely undetermined, and only those should be treated as "proceed". The FFI doc now separates the two classes instead of flattening them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4 participants
Issue being fixed or feature implemented
A spent DIP-13 voucher was only discoverable by attempting the claim. It surfaced as a raw
asset lock ... output N already completely usedafter the invitee had picked a username and entered their PIN — the worst possible moment to learn the link was dead. Nothing lets a caller ask "is this invitation still good?" up front, because there is no asset-lock-consumption query on the platform gRPC surface.Platform derives a created identity's id from the asset-lock outpoint, so the id an invitation would produce is knowable before the claim — and an identity already existing under that id is exactly the "this voucher is spent" signal.
What was done?
Adds
IdentityWallet::invitation_prospective_identity_idplus its FFI and Swift wrappers.claim_invitation, so a testnet link opened on mainnet reports the network mismatch rather than a confusing fetch miss.The docs state explicitly that any failure is genuinely undetermined — wrong network, transaction not yet propagated, transport error — and that callers must treat an error as "proceed", never as an answer either way. An unclaimed invitation is expected to miss the identity lookup, so a miss and a transport failure are indistinguishable at this layer and only the claim itself is authoritative.
How Has This Been Tested?
Testnet, on device, through the iOS invitation redeem screen (
dashpay/dashwallet-iosfeat/dashpay-invitations-claim, which is the only consumer today).A spent voucher now reports "already used" on the preview screen instead of failing after PIN entry. Verified the verdict independently of the app for the invitation used in testing: the asset lock transaction is chainlocked, the voucher key's
hash160matches credit output 0, and the derived identityD3RdTqKMMXsV511dQePU36huVk6Awa1VyGe2soyJ8YqWresolves on three separate evonodes with a non-empty identity and a drawn-down balance — i.e. the "spent" verdict this API returns is the correct one.An unclaimed invitation returns the prospective id and the claim proceeds normally.
Breaking Changes
None. Purely additive — a new read-only method on
IdentityWalletplus its FFI/Swift surface.Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit