feat(claim): Phase 2 Path A — SDK-driven claim flow with claim-tx-sig attribution#281
Merged
Conversation
…arsing Original Task 1 helper assumed spl-memo SIP:1: format (SDK's standalone send flow). Sipher's actual production deposits emit Anchor VaultWithdrawEvent in tx.meta.logMessages via the vault's withdraw_private instruction — no SPL memo. Code review caught this before any wrong tests landed; this revision rewrites Tasks 1 + 2 to parse the event-log format that sipher's own scanForPayments already uses (single source of truth for both scan AND claim event parsing). Also addresses reviewer findings: - spl-token-2022 support in SPL transferChecked finder - Inner-instruction search for Jupiter-routed deposits - Cleaner type guard via ParsedInstruction type narrowing - ATA-owner equality check against event stealth_recipient - Dropped unused invalid_withdraw_event code (parser skips malformed events and continues — code was unreachable)
- destinationWallet now required in both type + Anthropic schema (was optional with runtime throw; schema-vs-runtime gap) - Remove deriveDestinationFromSpending placeholder helper - viewingKey + spendingKey schema descriptions clarify hex-only input - Replace USDC_MINT fallback with invariant throw (dead code; was a devnet footgun) - Drop unused USDC_MINT import - Update input-validation tests to pass destinationWallet for type conformance (tests still verify the validation error path)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced May 17, 2026
Open
Member
Author
Follow-up issues filed
|
Path A made destinationWallet required in ClaimParams. consolidate.ts schedules action='claim' ops via createScheduledOp; without destinationWallet in params, COURIER would throw at the SDK boundary on every fire and reset the op to 'pending' for perpetual 60s retry. Use params.wallet as the destination — that's the user's main wallet, which is also where consolidate intends the consolidated funds to land. Caught by final cross-cutting code review of PR #281.
Member
Author
|
Final cross-cutting code review caught one Important issue: Fixed in |
PR #280's recursive typecheck (`pnpm -r --filter='!sipher' run typecheck`) exposed a build-order issue that the previous root-only typecheck masked. The agent package imports from @sipher/sdk (workspace dep) whose types live in `dist/index.d.ts` — without building the SDK first, tsc fails with 'Cannot find module @sipher/sdk' for every consumer. Local typecheck passes because dist/ exists from prior builds; CI does fresh install without building. This has been failing on main since PR #280 merged 2 days ago (last green: 7389a3e on PR #279 merge). Add a Build SDK step between Install and Typecheck. Fixes both the Typecheck and Test steps in one shot (tests also import the runtime @sipher/sdk via dist/index.js).
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
1 participant
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.
Summary
executeClaimnow resolves the stealth context from sipher'sVaultWithdrawEvent(mirroringscanForPayments), delegates ECDH derivation + SPL transfer + broadcast to@sip-protocol/sdk'sclaimStealthPayment, and returns the real claim tx signature.sipher_private_claim_completednow carriesdata.tx_signature = <claim_tx_sig>instead of the input deposit-tx-sig.Test plan
tests/tools/claim-helpers.test.ts— 12 tests (1 happy + 3 happy-path variants + 8 error paths covering all 4 discriminated codes incl. the Buffer-data ATA branch)tests/claim.test.ts— 13 tests (3 tool-definition + 3 happy-path + 4 input-validation regression + 3 SDK/helper error wraps)tests/integrations/torque/growth-hook.test.ts— +1 test locking in claim-tx-sig attribution prioritytests/tools.test.ts— duplicateexecuteClaimdescribe block removed; dispatch test updated to verify the new resolveStealthContext code pathBF47B9DC1FA320FA)Scope notes
Path A trade-offs are documented in
docs/superpowers/specs/2026-05-15-claim-phase-2-design.md(Amendment — SDK reality check). Summary:Path B (SDK extension + SignTxCard UX) is tracked as a post-judges follow-up.
Other deliberate scope choices:
destinationWalletis now required inClaimParams+ Anthropic schema (was optional with runtime throw — schema-vs-runtime gap closed). Auto-derive from spending pubkey is a follow-up.spl-token-2022(Token Extensions) supported alongsidespl-tokenin the SPL transfer finder.Mid-PR revision
A first attempt at Task 1 assumed sipher's deposits emit
spl-memoinstructions withSIP:1:format (the SDK's standalone send flow). Code review caught that sipher's actual production flow uses the vault'swithdraw_privateinstruction which emits an AnchorVaultWithdrawEventintx.meta.logMessages— no SPL memo. The plan was revised (commit7621df6) to parse the event-log format that sipher's ownscanForPaymentsalready uses (single source of truth for both scan AND claim event parsing). The first Task 1 attempt was reset before any wrong tests landed.Verification
https://sipher-api.sip-protocol.org/admin/api/torque/statusshould remain{ enabled: true, network: 'devnet', ingesterReachable: true }after deploy.Follow-ups (separate issues, post-merge)
Will be filed against this PR's merge:
destinationWalletfrom spending pubkey, human-readableamountdisplay in chat messagetests/**topackages/agent/tsconfig.json, extractnormalizeKeyto shared utility, addbeforeEachmock reset in claim.test.ts error-paths describePlan + spec references
docs/superpowers/plans/2026-05-17-claim-phase-2-path-a.md(this PR adds the plan as commit037cffa, revised as7621df6)docs/superpowers/specs/2026-05-15-claim-phase-2-design.md(on the design-specs PR docs(superpowers): 5 design specs from PR-E + sipher#262 follow-up list #276 — read the "Amendment — SDK reality check" section for Path A vs Path B trade-off)