Repository navigation
Apply the SELL release gate to the ramp status endpoint - #1387
Merged
Merged
Conversation
GET /v1/ramp/{id}?showUnsignedTxs=true returned the raw unsignedTxs, so a
client could fetch the user's source-of-funds transactions for a SELL ramp
before every ephemeral presign was received and validated, bypassing the
gate that register and update enforce.
✅ Deploy Preview for vortexfi canceled.
|
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vrtx-dashboard canceled.
|
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The predicate unnecessarily performs potentially chain-backed validation for BUY status requests despite being a SELL-only gate.
Review effort: Balanced
Findings: 1
What changed in this PR
Applies SELL transaction-release filtering to ramp status responses.
Changes:
- Gates status
unsignedTxsusing presign validation. - Adds SELL regression tests.
- Updates the security specification.
| File | Description |
|---|---|
ramp.service.ts |
Filters status transactions. |
ramp.service.get-ramp-status.test.ts |
Tests withholding and release. |
transaction-validation.md |
Documents status-response gating. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The release gate only filters SELL responses, so validating presigns for a BUY status request added latency and a chain API dependency for nothing.
The release case persisted presignChecksPass, so it short-circuited before ephemeralPresignChecksPass and a regression dropping the dynamic check would have stayed green while SELL clients never received their transactions.
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
GET /v1/ramp/{id}?showUnsignedTxs=truereturnedrampState.unsignedTxsunfiltered. For a SELL ramp that let a client fetch the user's source-of-funds transactions before every ephemeral presigned tx had been received and validated. The register and update responses withhold exactly those txs viafilterUnsignedTxsForResponse. This is the case the gate exists for: an older SDK starting the user's transfer after the backend added ephemeral txs it cannot sign.Fix
getRampStatusnow returnsfilterUnsignedTxsForResponse(rampState, presignChecksPass || ephemeralPresignChecksPass(rampState)). That is the same predicateupdateRampuses, and it only runs whenshowUnsignedTxsis requested.Tests
ramp.service.get-ramp-status.test.tscase: a SELL ramp with its ephemeral presigns missing returns only the ephemeral txs. It fails without the fix.tsc --noEmitclean.Security spec
docs/security-spec/03-ramp-engine/transaction-validation.mdnow names all three responses that the gate covers.