Repository navigation
[ANCHOR-1296]: Fix SEP-6 withdraw refund memo parameter names and add validation - #2032
Merged
ceciliaromao merged 7 commits intoSep 30, 2026
Merged
Conversation
amandagonsalves
approved these changes
Sep 30, 2026
ceciliaromao
deleted the
fix/anchor-1296-sep6-withdraw-refund-memo-param
branch
September 30, 2026 18:08
ceciliaromao
added a commit
that referenced
this pull request
Oct 1, 2026
#2034) ### Description Partial release of the ANCHOR-1279 SEP-compliance coverage audit: brings everything accumulated on `chore/anchor-1279-sep-coverage-audit` into `develop` for the first time. Net diff is 14 files (+1791/−89) — most of the audit branch's 50 raw commits are re-merges of `develop` back into itself and carry no new content. Two pieces of work land here, each already reviewed and merged into the audit branch on its own PR, neither of which had reached `develop` before: - **SEP-31 coverage** (#2012) — closes the remaining SEP-31 gaps from the audit's spec cross-check: `fee_details.details` breakdown on both the quote and no-quote paths (plus a precision desync fix on the no-quote path), a `GET /info` `fields.transaction` gap that silently dropped configured field metadata, an HTTP 400-vs-500 distinction added to the shared `SepClient` base class, `quotes_required: true` coverage (previously never exercised by any asset), and the remaining `stellar-anchor-tests` → AP-suite gaps (table below). - **Last SEP-6 coverage-audit gaps** (#2031) — closes the audit's final five SEP-6 assertions: full transaction-schema validation on `GET /transaction`, `stellar.toml`'s `TRANSFER_SERVER` validity, `/info`'s per-asset field metadata, `/deposit-exchange`/`/withdraw-exchange` JWT and required-param coverage, the Amount Formula verified independently against the SEP-38 quote, and the six SEP-6 statuses no test had reached (`on_hold`, `pending_customer_info_update`, `pending_transaction_info_update`, `too_small`, `too_large`, `no_market`). ### Context - Both PRs targeted `chore/anchor-1279-sep-coverage-audit` as their base, per the project's routing rule for test-only/coverage branches — so this is the first time either reaches `develop`. - Going forward, every new ANCHOR-1279 branch targets `develop` directly; the audit branch is retired as a base after this merge. - Full audit trail: `docs/audits/ANCHOR-1279-sep-coverage-audit.md` (moved out of the tracked repo into the git-excluded `.specs/` in an earlier PR on this branch). ### Testing - `./gradlew test` - Verified with `git merge-tree` that this merges into current `develop` with no conflicts. - Both constituent PRs (#2012, #2031) passed CI and were reviewed independently before merging into the audit branch; no new changes are introduced by this merge itself. ### Documentation - `GET /info` (SEP-31) can now include a `fields` object per asset — additive, omitted when not configured. - `SepClient.handleResponse` now maps HTTP 400 to `SepValidationException` for every SEP client built on it (previously only 403/404 were distinguished). - No other public API or user-facing behavior changes. ### Known limitations - `expired` status / quote-expiry-driven auto-expiration for SEP-31 (identified during #2012, confirmed unimplemented) — real feature work, not filed as a ticket yet. - SEP-6 audit assertions #10/#11 ("SEP-9 fields match config") and #3/#4/#35/#36 (`authentication_required: false`) describe behavior that doesn't exist in this codebase — see the SEP-6 table's notes for what's verified instead. - Remaining ANCHOR-1296 (SEP-6) work — `fix/anchor-1296-sep6-withdraw-refund-memo-param` (#2032, open) and `feat/anchor-1296-sep6-patch-quote-claimable` (in progress) — ships in separate, later PRs directly against `develop`. ### `stellar-anchor-tests` → this repo's test suite (SEP-31) Mapping of all 14 SEP-31 assertions in `stellar-anchor-tests` to their equivalent in AP's own suite: | `stellar-anchor-tests` assertion | AP test | |---|---| | GET /info matches expected schema | ✅ `Sep31Tests.kt:56` | | Has expected asset enabled | ✅ `Sep31Tests.kt:56` | | Check optional transaction 'fields' | ✅ `Sep31ServiceTest.kt` (`test INFO response advertises fields...`) | | Has DIRECT_PAYMENT_SERVER attribute | ✅ `Sep31Tests.kt` (`test DIRECT_PAYMENT_SERVER has expected format`) | | Requires a SEP-10 JWT | ✅ `Sep31Tests.kt` (`test requires a SEP-10 JWT`) | | Can create a transaction | ✅ `Sep31Tests.kt:60` | | Returns 400 when no amount is given | ✅ `Sep31Tests.kt` (`test returns 400 when no amount is given`) | | Returns 400 when no asset_code is given | ✅ `Sep31Tests.kt` (`testBadAsset` + `test returns 400 when no asset_code is given`) | | Can fetch a created transaction | ✅ `Sep31Tests.kt:68` | | GET tx response complies with protocol schema | ✅ `Sep31Tests.kt` (`assertCompliesWithProtocolSchema`) | | Returns 404 for a non-existent transaction | ✅ `Sep31Tests.kt` (`test returns 404 for a non-existent transaction`) | | [quotes_required] can create a transaction | ✅ `Sep31Tests.kt` (`test quotes_required can create and fetch a transaction with a quote`) | | [quotes_required] can fetch a created transaction | ✅ same test | | [quotes_required] response complies with protocol schema | ✅ same test | **14 Verified, 0 partial, 0 gap.** ### `stellar-anchor-tests` → this repo's test suite (SEP-6) Mapping of all 38 SEP-6 assertions in `stellar-anchor-tests` to their equivalent in AP's own suite, closed across #2009, #2017, #2018, #2022, #2025 and #2031: | `stellar-anchor-tests` assertion | AP test | |---|---| | Deposit requires JWT if auth required | ✅ `Sep6Tests.kt` (`test sep6 deposit rejects request without JWT`) | | Deposit requires 'asset_code' | ✅ `Sep6Tests.kt` (`test sep6 deposit rejects request without asset_code`) | | Deposit requires 'account' if auth not required | N/A — `authentication_required` is hardcoded `true`; this branch never exists | | Deposit rejects invalid 'account' if auth not required | N/A — same as above | | Deposit rejects unsupported asset_code | ✅ `Sep6Tests.kt` (`test sep6 deposit rejects unsupported asset_code`) | | Deposit success response | ✅ `Sep6Tests.kt` (`test sep6 deposit`) | | GET /info matches expected schema | ✅ `Sep6Tests.kt` (`test Sep6 info endpoint`) — STRICT JSONAssert | | Asset enabled for deposit in /info | ✅ `Sep6Tests.kt` (`test Sep6 info endpoint`) | | Asset enabled for withdraw in /info | ✅ `Sep6Tests.kt` (`test Sep6 info endpoint`) | | SEP-9 fields for deposit match config | See Known limitations — ✅ verifies the actual field derivation instead | | SEP-9 fields for withdraw match config | See Known limitations — ✅ verifies the actual field derivation instead | | TOML has valid transfer server URL | ✅ `Sep6Tests.kt` (`test sep6 TOML TRANSFER_SERVER is a well-formed URL`) | | /transaction requires JWT | ✅ `Sep6Tests.kt` (`test sep6 GET transaction rejects request without JWT`) | | Record on /transaction after deposit | ✅ `Sep6Tests.kt` (`test sep6 deposit`) | | Record on /transaction after withdraw | ✅ `Sep6Tests.kt` (`test sep6 withdraw`) | | Deposit transaction schema on /transaction | ✅ `Sep6Tests.kt` (`test sep6 GET transaction returns a schema-complete deposit record`) | | Withdraw transaction schema on /transaction | ✅ `Sep6Tests.kt` (`test sep6 GET transaction returns a schema-complete withdrawal record`) | | 404 for nonexistent transaction ID | ✅ `Sep6Tests.kt` (`test sep6 GET transaction returns 404 for an unknown id`) | | 404 for nonexistent external transaction ID | ✅ `Sep6Tests.kt` (`test sep6 GET transaction returns 404 for an unknown external_transaction_id`) | | 404 for nonexistent Stellar transaction ID | ✅ `Sep6Tests.kt` (`test sep6 GET transaction returns 404 for an unknown stellar_transaction_id`) | | /transactions requires JWT | ✅ `Sep6Tests.kt` (`test sep6 GET transactions rejects request without JWT`) | | Record on /transactions after deposit | ✅ `Sep6Tests.kt` (`test sep6 deposit appears in transactions listing with valid schema`) | | Record on /transactions after withdraw | ✅ `Sep6Tests.kt` (`test sep6 withdrawal appears in transactions listing with valid schema`) | | Deposit schema on /transactions | ✅ same test | | Withdraw schema on /transactions | ✅ same test | | Empty list for accounts w/o transactions | ✅ `Sep6Tests.kt` (`test sep6 GET transactions returns empty list for account with no history`) | | Proper count for 'limit' param | ✅ `Sep6Tests.kt` (`test sep6 GET transactions honors limit parameter exactly`) | | Descending creation order | ✅ `Sep6Tests.kt` (`test sep6 GET transactions are ordered by started_at descending`) | | 'no_older_than' filtering | ✅ `Sep6Tests.kt` (`test sep6 GET transactions no_older_than filters strictly newer records`) | | kind=withdrawal filter | ✅ `Sep6Tests.kt` (`test sep6 GET transactions kind=withdrawal returns only the withdrawal`) | | kind=deposit filter | ✅ `Sep6Tests.kt` (`test sep6 GET transactions kind=deposit returns only the deposit`) | | Rejects bad asset_code on /transactions | ✅ `Sep6Tests.kt` (`test sep6 GET transactions rejects unsupported asset_code`) | | Withdraw requires JWT if auth required | ✅ `Sep6Tests.kt` (`test sep6 withdraw rejects request without JWT`) | | Withdraw requires asset_code | ✅ `Sep6Tests.kt` (`test sep6 withdraw rejects request without asset_code`) | | Withdraw requires 'account' if auth not required | N/A — same as deposit's equivalent | | Withdraw rejects invalid 'account' if auth not required | N/A — same as deposit's equivalent | | Withdraw rejects unsupported asset_code | ✅ `Sep6Tests.kt` (`test sep6 withdraw rejects unsupported asset_code`) | | Withdraw success response | ✅ `Sep6Tests.kt` (`test sep6 withdraw`) | **34 Verified, 0 partial, 4 N/A (unreachable — `authentication_required` is always `true`), 0 gap.** --------- Co-authored-by: Amanda Gonsalves <64379712+amandagonsalves@users.noreply.github.com>
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
2 participants
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.
Description
GET /sep6/withdraw: it read the refund memo fromrefundMemo/refundMemoTypequery params, but SEP-6 names themrefund_memo/refund_memo_type— the same names/withdraw-exchangealready used correctly. A spec-compliant wallet's refund memo was silently dropped. The endpoint now reads the spec names, and keeps the old camelCase names working as a deprecated compatibility alias: each field resolves independently, the spec name winning whenever it's non-empty./withdrawand/withdraw-exchange, mirroring SEP-31's existing validation exactly:refund_memoandrefund_memo_typemust be specified together or both omitted, and the pair's type/value combination is validated, with every failure mapped to a 400 instead of an unhandled 500.Context
test/anchor-1296-sep6-coverage) — fixing only the parameter name would have opened a new, unvalidated input path, so both changes land together here.developdirectly rather than the coverage-audit branch.Testing
./gradlew test./gradlew spotlessCheck :core:test :platform:test./gradlew :essential-tests:test --tests "*Sep6Tests*"— run twice in a row (51/51 passing)Documentation
GET /sep6/withdrawnow acceptsrefund_memo/refund_memo_typeper the SEP-6 spec. The previousrefundMemo/refundMemoTypenames still work, as a deprecated, non-spec compatibility alias — clients should migrate to the spec names./withdraw-exchangenow rejects a malformed refund memo with HTTP 400 instead of accepting it unvalidated.Known limitations