fix(swift-example-app): gate identity resumes by funding type#4015
fix(swift-example-app): gate identity resumes by funding type#4015thepastaclaw wants to merge 2 commits into
Conversation
|
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 (3)
📝 WalkthroughWalkthroughThe Swift example app now limits generic resumable registrations to funding types ChangesResumable registration gates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
…ing types The Identities-tab Resumable Registrations surface filtered `PersistentAssetLock` rows on `statusRaw` and the per-slot anti-join alone. Every asset lock on the active network was in scope — including `fundingTypeRaw == 4` (AssetLockAddressTopUp) and `5` (AssetLockShieldedAddressTopUp), whose scripts commit to an address-flow recipient. Tapping Resume on one of those rows routed the lock into `CreateIdentityView(preselectedAssetLock:)` and would fire an IdentityCreate the user never intended. Add `fundingTypeRaw` to `AssetLockResumeRow` and require `0...2` (IdentityRegistration / IdentityTopUp / IdentityTopUpNotBound) in `crossWalletResumableLocks` before the status and anti-join gates. Invitation vouchers (3) remain reserved for the explicit reclaim flow, while address-flow locks (4 and 5) stay on their proper surfaces. Also refresh stale docs/comments that still described Broadcast rows as spinner-only and Consumed as perpetual-spinner — PR dashpay#4010 made every resumable row a Resume button. Regression coverage in `CreateIdentityResumableTests` accepts funding types 0...2 and rejects invitation, address, shielded- address, negative, and unknown discriminators fail-closed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…able filter The `allAssetLocks` @query comment predates the current `crossWalletResumableLocks` shape and still described the filter as "status >= InstantSendLocked". The floor is actually `>= 1` (Broadcast), and the filter now admits only generic identity- resume funding types 0...2 and excludes in-flight controller slots. Bring the docstring in line so future readers don't work off the old contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
a81cae4 to
9894f56
Compare
|
✅ Final review complete — no blockers (commit 9894f56) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex + Sonnet
This PR is a documentation-only follow-up syncing the allAssetLocks @query docstring with the fail-closed fundingTypeRaw in 0...2 whitelist introduced in the immediately preceding commit (638826f). I verified the parent fix directly: it correctly replaces the prior blacklist (fundingTypeRaw != 3) with an explicit whitelist, closing the gap where address (4) and shielded-address (5) top-up locks could reach CreateIdentityView(preselectedAssetLock:), and traced the sole call site confirming no bypass exists. Both Claude and Codex (general + security-auditor) independently found zero in-scope issues; the PR is clean and well-tested.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed) - Verifier:
claude-sonnet-5— final-verifier - Sonnet reviewers:
claude-sonnet-5— general (completed),claude-sonnet-5— security-auditor (completed)
Note: GitHub does not allow PastaClaw to submit an approval or request changes on their own PR, so the canonical verifier result is transported as a COMMENT review.
QuantumExplorer
left a comment
QuantumExplorer
left a comment
There was a problem hiding this comment.
Reviewed and verified independently:
- Gap is real: base only blacklisted IdentityInvitation (3). Address (4) / shielded (5) top-up locks persist through the same Broadcast→IS/CL lifecycle with pool-index
identityIndexRaw, so an interrupted address-funding flow surfaced as a resumable registration whenever the matching identity slot was free. The Rust resolver (resolve_funding_with_is_timeout_fallback) only refuses type 3 — a resumed type-4/5 lock would be consumed into IdentityCreate, misdirecting the funds. - Whitelist is the sole gate:
.unusedAssetLockfunding is only enterable viaCreateIdentityView(preselectedAssetLock:), whose single call site is the resumable-rows sheet; no in-view lock picker bypass. - Discriminators in the docstrings match the Rust serde mapping (0–5) exactly; executed the filter logic standalone against all new test cases plus the status-floor / per-wallet anti-join /
UInt32(bitPattern:)invariants — all pass. - The earlier Swift SDK CI failure was the known runner keychain flake (
keychainError(-61)in IdentityResolverSignIntegrationTests, untouched by this PR); green on rerun.
Follow-up (separate PR): mirror this gate in the Rust resolver for types 4/5 as defense-in-depth, like the existing invitation-voucher refusal.
2 participants
Follow-up to #4010. That PR was merged before the automated review landed; the review found that the Resumable Registrations list could expose address-funding asset locks to the identity registration resume path.
What changed
Validation
Created from tracker issue thepastaclaw/tracker#1809.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation