Make squidRouterApproveHash optional in fundEphemeral verification#1274
Open
ebma wants to merge 1 commit into
Open
Make squidRouterApproveHash optional in fundEphemeral verification#1274ebma wants to merge 1 commit into
ebma wants to merge 1 commit into
Conversation
Users whose wallet already holds a sufficient allowance for the squid router never broadcast the approve tx, so they have no approve hash to report. fundEphemeral previously parked such ramps forever waiting for it, even though the user had already paid. The approve hash is now verified only when reported (with unchanged rigor); squidRouterSwapHash — the tx that actually delivers funds to the ephemeral — remains mandatory. Skipping a genuinely-needed approve is safe: the swap's transferFrom reverts on-chain and the swap hash fails its receipt check. The verification gate now keys on the squidRouterSwap blueprint instead of the approve blueprint accordingly. Also updates the security spec (F-041 notes), the OpenAPI description of /v1/ramp/update, and adds corridor tests for both the swap-hash-only happy path and a tampered-but-reported approve hash.
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vrtx-dashboard ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vortexfi ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the SELL/offramp SquidRouter hash-verification flow so ramps no longer get stuck in fundEphemeral when no approval tx was needed (pre-existing allowance), while keeping strict by-hash validation for any hashes that are reported.
Changes:
- Make
squidRouterApproveHashoptional inFundEphemeralPhaseHandler.verifyUserSubmittedSquidHashes; still verify it rigorously when provided, and keepsquidRouterSwapHashmandatory. - Add corridor scenario coverage for “pre-existing allowance” and “tampered approve hash still fails” cases.
- Update security spec + OpenAPI docs to reflect the optional-approve semantics.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/security-spec/03-ramp-engine/transaction-validation.md | Documents optional-approve / mandatory-swap verification semantics for SELL SquidRouter flows. |
| docs/api/openapi/vortex.openapi.json | Updates /v1/ramp/update and squidRouterApproveHash descriptions to reflect optional approve hash. |
| docs/api/openapi/vortex.openapi.d.ts | Regenerates typings/comments to match updated OpenAPI wording. |
| apps/api/src/tests/corridors/brl-offramp-crosschain.scenario.test.ts | Adds scenario tests for missing approve hash (allowance) and for tampered approve hash verification. |
| apps/api/src/api/services/phases/handlers/fund-ephemeral-handler.ts | Implements optional approve-hash verification while keeping swap-hash verification mandatory. |
Comments suppressed due to low confidence (1)
docs/api/openapi/vortex.openapi.json:1279
squidRouterSwapHashis described as "if applicable", but for EVM-origin offramps it’s mandatory (the endpoint description and update-time validation gate require it). This field description should mention the conditional requirement to avoid integrators assuming it’s optional in the EVM-offramp case.
"description": "Transaction hash for Squid Router approval. Optional: omit when the wallet already holds a sufficient allowance and no approval transaction was submitted.",
"type": ["string", "null"]
},
"squidRouterSwapHash": {
"description": "Transaction hash for Squid Router swap, if applicable.",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Problem
FundEphemeralPhaseHandler.verifyUserSubmittedSquidHashesrequired bothsquidRouterApproveHashandsquidRouterSwapHashbefore proceeding. Users whose wallet already holds a sufficient allowance for the squid router never broadcast an approve tx, so they have no approve hash to report — and their ramp parked forever infundEphemeraleven though they had already paid.Fix
from,to, calldata, value).squidRouterSwapHash— the tx that actually delivers funds to our ephemeral — remains mandatory; the phase still parks recoverably until it arrives and still fails unrecoverably on content mismatch.squidRouterSwapblueprint instead of thesquidRouterApproveblueprint (they are always emitted together today, so this is behavior-equivalent, but it matches the new semantics).Skipping a genuinely-needed approve is safe: the swap's
transferFromreverts on-chain, so the swap hash fails its own receipt-status check.Note this matches
validateRampStateDatainramp.service.ts, which already only hard-requires the swap hash at start.Tests
New corridor tests in
brl-offramp-crosschain.scenario.test.ts:Existing suites (
brl-offramp-crosschain,brl-offramp,eur-offramp,mxn-offramp,sdk-contract.offramp) pass; lint + typecheck clean.Docs
docs/security-spec/03-ramp-engine/transaction-validation.md(F-041 sections) updated with the optional-approve semantics./v1/ramp/updatedescription and thesquidRouterApproveHashfield description updated invortex.openapi.json;.d.tsregenerated viabun docs:api:types.bun docs:api:exportdoesn't revert it.