[ANCHOR-1295]: SEP-31 coverage

[ceciliaromao]

Description

Closes the remaining SEP-31 gaps identified in the ANCHOR-1279 coverage audit's spec cross-check (docs/audits/ANCHOR-1279-sep-coverage-audit.md, "Step 4" section) that weren't already closed by #2004, #2006, or #2011:

• fee_details.details breakdown, quote-based path — was already working in production (the reference server's /rate genuinely returns a breakdown), just never asserted. Added the details array to Sep31Tests.kt's expected-transaction JSON.
• fee_details.details breakdown, no-quote path — a real, small product gap. Sep31Service's internal Context.fee only carried a bare Amount (total + asset) for the no-quote fee, discarding any breakdown the /rate response returned. Fixed by simplifying Context.fee to carry FeeDetails directly (dropping the now-redundant Amount type entirely from this code path) — every call site got shorter, not more complex. Follow-up review caught a precision bug in this same path: the persisted fee.total was being reformatted to the requested asset's decimal scale while fee.details kept the original (possibly higher-precision) scale from the /rate callback — since RestRateIntegration permits a fee denominated in the buy asset, this could desync total from sum(details). Fixed by persisting the callback's FeeDetails exactly as received; the requested-asset-scaled value is now used only for the internal amountIn/amountOut arithmetic, never persisted as the fee total.
• sender_id/receiver_id real-customer validation — re-investigated after review: verifyCustomerOwnershipAndKyc always reverse-looks-up an unclaimed id through customerIntegration (using the authenticated caller's own account/memo) before kycRequired is ever considered; only the later KYC-status enforcement is skipped when the asset doesn't advertise sep12.<role>. So a never-registered id is rejected with SepNotAuthorizedException unconditionally — there is no gap here, spec-compliant or otherwise. Sep31CustomerOwnershipTests.kt was corrected to assert this rejection instead of locking in the opposite (incorrect) expectation.
• SEP-31 POST /transactions 400 vs 500 distinction — Sep31Tests.kt's testBadAsset only asserted the generic SepException base type, which would also pass if the endpoint regressed to a 500. Fixed by adding an explicit HttpStatus.SC_BAD_REQUEST → SepValidationException mapping to the shared SepClient.handleResponse (previously only 403/404 were distinguished from the catch-all). Note: this touches lib-util's SepClient base class used by every SEP client, not just SEP-31 — broader than this PR's other changes. [ANCHOR-1295]: Add SEP-31 PATCH client and fix broken patch endpoint #2011 introduces the identical mapping independently (on a separate branch, for the same underlying reason); the two changes are identical so there's no conflict regardless of merge order.
• GET /info never advertised fields.transaction — a real product gap, not just missing coverage. SEP-31's /info fields object schema requires per-asset field declarations, but Sep31InfoResponse.AssetResponse had no fields property at all and sep31InfoResponseFromAssetInfoList never populated one — test_assets.json's USDC fixture already configured sep31.fields, but it was silently dropped before reaching any response. Added Sep31Info.fields (config) and Sep31InfoResponse.FieldsResponse/FieldResponse (wire model), wired through Sep31Service, and configured a real fields.transaction entry on the essential-tests USDC asset so the new e2e test has something concrete to check.
• Closed the remaining stellar-anchor-tests → AP-suite gaps for SEP-31 (table below): requires-JWT, no-amount-400, no-asset_code-400 (missing, not just invalid), non-existent-transaction-404, DIRECT_PAYMENT_SERVER format, and a real schema-compliance check on GET /transactions/:id (status enum, stellar_account_id/stellar_memo well-formedness) replacing the old field-subset LENIENT check.
• quotes_required: true was never exercised — every essential-tests SEP-31 asset had it false. Added a new, isolated SEP-31-only test asset (SRT, reusing the reference rate server's already-hardcoded SRT/USD price so no rate-server changes were needed) with quotes_required: true, and two new tests: rejects a POST with no quote_id, and creates/fetches a transaction with one.

Explicitly out of scope, not filed as a ticket yet: expired status / quote-expiry-driven auto-expiration. Confirmed via investigation there is no code path anywhere that transitions an in-flight SEP-31 transaction to EXPIRED when its bound quote expires — only a manual RPC (NotifyTransactionExpiredHandler) exists, unconditional, never checking expires_at. This is real feature work (a new scheduled job modeled on NonceCleanupJob, a store query, status transition, event publishing), not something that belongs in a coverage-closing PR.

Context

Continuation of #2004, #2006, and #2011 under ANCHOR-1295, part of the phased gap-closing plan from the ANCHOR-1279 SEP-compliance coverage audit. Targets develop directly (not chore/anchor-1279-sep-coverage-audit) since this PR touches production code (the Context.fee refactor and the new /info fields wiring in Sep31Service.java), not just tests — per our routing rule, only test-only branches stack on the audit branch.

This branch's history had drifted (it was originally forked from fix/anchor-1295-sep31-patch-client before that branch's own history got cleaned up) — reconstructed by cherry-picking this branch's 2 real commits directly onto current develop; the diff has grown since via code-review findings and, this round, by deliberately closing the remaining stellar-anchor-tests coverage gaps tracked in the table below.

Testing

• ./gradlew test
• Sep31ServiceTest: all passing, including the Context.fee/FeeDetails refactor's assertions and new fields-in-/info wiring tests
• Sep31CustomerOwnershipTests / Sep31Tests: new/updated e2e assertions for the fee breakdown, ownership-rejection, 400-vs-500, JWT/400/404 coverage, fields-in-/info, and quotes_required
• Ran the new SEP-12-independent e2e tests (JWT, no-amount, no-asset_code, 404, DIRECT_PAYMENT_SERVER format, fields in /info, quotes_required rejection) against a locally started reference server (./gradlew startServersWithTestProfile) — all pass. The remaining new tests that require creating a real SEP-12 customer (the full quotes_required create/fetch flow) hit the same pre-existing local-only Sep12Client.putCustomer limitation documented on prior PRs in this series (tied to ANCHOR-1281's callback/config wiring, not present locally) — relying on CI for those, consistent with past practice here.

Documentation

SepClient.handleResponse now maps HTTP 400 responses to SepValidationException instead of the generic SepException, for every SEP client built on this shared base class (previously only 403/404 were distinguished). GET /info can now include a fields object per asset (only when configured — omitted otherwise, so this is additive, not a breaking change). No other public API or user-facing behavior changes.

Known limitations

expired status auto-transition (see above) is real feature work, not test coverage — needs its own ticket before it can be picked up.

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, invalid and missing)
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 (reuses assertCompliesWithProtocolSchema)

14 Verified, 0 partial, 0 gap.

[Copilot]

Pull request overview

Extends SEP-31 fee-breakdown, customer-ownership, and PATCH transaction coverage.

Changes:

• Preserves no-quote fee details.
• Adds PATCH client and required-field update support.
• Expands fee, ownership, and PATCH tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Review
platform/src/test/kotlin/org/stellar/anchor/platform/service/TransactionServiceTest.kt	Tests required-info updates.
platform/src/main/java/org/stellar/anchor/platform/service/TransactionService.java	Implements required-field updates; scope differs from the PR description.
lib-util/src/main/kotlin/org/stellar/anchor/client/SepClient.kt	Adds PATCH support; HTTP 400 needs typed exception mapping, and scope needs clarification.
lib-util/src/main/kotlin/org/stellar/anchor/client/Sep31Client.kt	Adds SEP-31 transaction PATCH support.
essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt	Adds fee and PATCH assertions; generic exceptions do not verify HTTP 400.
essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31CustomerOwnershipTests.kt	Covers customer IDs when KYC is optional.
core/src/test/kotlin/org/stellar/anchor/sep31/Sep31ServiceTest.kt	Tests fee propagation and PATCH behavior.
core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java	Preserves fee details, but may round the total inconsistently with its breakdown.

Suppressed comments (3)

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:305

• This generic SepException assertion cannot distinguish the required 400 from a 500 because SepClient.handleResponse uses the same exception for both. Assert a typed 400 error after adding the corresponding client mapping so this test actually locks in the response-code contract.

    // Transaction is still pending_receiver, never entered pending_transaction_info_update.
    val ex =
      assertThrows<SepException> {

lib-util/src/main/kotlin/org/stellar/anchor/client/SepClient.kt:62

• The PR description says the PATCH client/endpoint work was already closed by #2011 and that this PR's diff contains exactly four fee/ownership files, but this hunk and the related PATCH changes are included here. Please rebase/retarget the branch to remove the already-landed work, or update the stated scope if these changes are intentionally part of this PR.

  fun httpPatch(url: String, requestBody: Map<String, Any>, jwt: String? = null): String? {

platform/src/main/java/org/stellar/anchor/platform/service/TransactionService.java:444

• This SEP-31 PATCH/required_info_updates implementation is the work attributed to #2011, but this PR says it closes only gaps not already covered by #2011 and that the reconstructed diff contains exactly four fee/ownership files. The current diff contains seven files plus the PATCH test hunks. Please rebase/drop the already-landed #2011 changes, or update the stated scope if they are intentionally part of this PR.

        // update required_info_updates: PlatformTransactionData only carries a flat list of field
        // names (shared with SEP-6), so it's expanded into the Sep31Info.Fields shape SEP-31's own
        // PATCH validation expects (Sep31Service#validatePatchTransactionFields only checks field
        // names, not the field metadata, so a placeholder AssetInfo.Field per name is sufficient).
        if (patch.getRequiredInfoUpdates() != null) {

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java:495

• If /rate returns a valid breakdown whose total has more precision than the asset's significant_decimals, feeStr is rounded but the detail amounts are copied unchanged. The persisted fee_details.total can then differ from the sum of fee_details.details (an invariant enforced by AssetValidationUtils.validateFeeDetails), so the new breakdown can be internally inconsistent. Please either reject over-precision or normalize the breakdown and total together.

    txn.setFeeDetails(new FeeDetails(feeStr, feeResponse.getAsset(), feeResponse.getDetails()));

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

api-schema/src/main/java/org/stellar/anchor/api/asset/Sep31Info.java:33

• The current SEP-31 specification marks fields as optional and deprecated, so saying the protocol “requires” it is inaccurate and conflicts with the nullable behavior described immediately afterward. Describe this as the legacy optional mechanism instead.

   * Advertised in `GET /info`'s `fields.transaction` so a sending anchor can discover which
   * `fields.transaction` entries to supply on `POST /transactions` -- SEP-31 requires this per the
   * `/info` response's fields object schema. Null (the default) means this asset advertises no
   * transaction fields.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

core/src/main/java/org/stellar/anchor/util/AssetValidator.java:225

• A configured fields block with no transaction key (for example, fields: {} or a misspelling) bypasses validation here. SEP-31 defines the fields object as containing a single transaction key, and the response mapper will otherwise silently omit this operator configuration. Reject that malformed shape explicitly.

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:470

• The new quotes_required success path never verifies that the fetched transaction retains the quote it was created with. SEP-31 says quote_id should be present when supplied to POST, so this test would pass if the association were silently dropped; compare the fetched quoteId with quote.id.

    val rawTxnJson = fetchRawTransaction(postTxResponse.id)
    val fetchedTxn = gson.fromJson(rawTxnJson, Sep31GetTransactionResponse::class.java)
    assertEquals(postTxResponse.id, fetchedTxn.transaction.id)

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:208

• started_at is required by the SEP-31 transaction schema, but treating it as optional lets a response that omits it pass this purported schema-compliance check. Assert its presence and string type before iterating over optional fields; deserialization will continue to validate its date-time format.
  core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java:457
• This Javadoc link cannot resolve because RestRateIntegration lives in the higher-level platform module and is neither imported nor available to core. Describe the rate callback behavior without linking to the platform implementation so generated documentation does not contain an unresolved reference.

core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java:811

• A missing configured field is raised as BadRequestException, so Sep31Controller bypasses its Sep31MissingFieldException handler and returns a generic error instead of SEP-31's structured {"error":"transaction_info_needed","fields":...} response. Build the missing Sep31Info.Fields payload and throw Sep31MissingFieldException (preferably with all missing fields) so clients can discover what to resubmit.

          throw new BadRequestException(
              String.format("missing required transaction field: %s", fieldName));

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:216

• The new schema check treats updated_at as any arbitrary string. Unlike started_at and completed_at, updated_at is not represented in Sep31GetTransactionResponse, so Gson deserialization cannot validate it; a malformed timestamp would therefore pass this purported protocol-schema check. Parse every present date-time field from the raw JSON.

core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java:519

• The PR describes fixing preservation of a higher-precision buy-asset fee, but this path has no regression assertion for that behavior: the new test uses an integral total and only checks details. Add a no-quote case where the requested asset has fewer decimals than the fee asset, then assert the persisted feeDetails.total and breakdown retain the callback's exact precision.

    // Persist the callback's fee exactly as received -- feeResponse.getTotal() is validated
    // against the sum of feeResponse.getDetails() at the fee asset's own precision by
    // RestRateIntegration, independent of reqAsset's scale (a fee may be denominated in the buy
    // asset). Reformatting it here would risk desyncing the stored total from the breakdown.
    txn.setFeeDetails(feeResponse);

[Copilot]

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:208

• started_at is required by the SEP-31 GET-transaction schema, but treating it as an optional field means this “schema compliance” helper passes a response that omits it entirely. Assert its presence and type before checking the genuinely optional strings.

core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java:523

• The precision-preservation behavior here has no regression case: the added tests use an integer fee and therefore would still pass if the fee were reformatted to the requested asset's scale again. Add a no-quote test with a buy-asset fee such as total 2.3456 (and matching details) while the sell asset has scale 2, then assert the persisted total and breakdown remain unchanged.

    // Persist the callback's fee exactly as received -- feeResponse.getTotal() is validated
    // against the sum of feeResponse.getDetails() at the fee asset's own precision by
    // RestRateIntegration, independent of reqAsset's scale (a fee may be denominated in the buy
    // asset). Reformatting it here would risk desyncing the stored total from the breakdown.
    txn.setFeeDetails(feeResponse);

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java:826

• Configured choices are advertised to clients but never enforced here; only presence is checked. For example, the updated fixture advertises type as SEPA/SWIFT, while the successful test at Sep31ServiceTest.kt:781-783 still submits "1". Validate any supplied value against fieldResponse.getChoices() and reject unsupported values with HTTP 400, otherwise /info and transaction acceptance disagree.

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:542

• This new test asserts the transient pending_receiver state even though Sep31EventProcessor.onTransactionCreated asynchronously advances every created transaction to pending_sender. Depending on callback timing, the fetch can observe either state, making the test flaky; assert the quote fields independently and allow both expected creation-time states (or explicitly coordinate event processing).

    val rawTxnJson = fetchRawTransaction(postTxResponse.id)
    val fetchedTxn = gson.fromJson(rawTxnJson, Sep31GetTransactionResponse::class.java)
    assertEquals(postTxResponse.id, fetchedTxn.transaction.id)
    assertEquals(PENDING_RECEIVER.status, fetchedTxn.transaction.status)
    assertCompliesWithProtocolSchema(rawTxnJson, fetchedTxn)

core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java:523

• The precision regression described by this change is not covered: the buy-asset test uses the integral fee "2" and never asserts the persisted feeDetails. A regression that reformats the total to the sell asset's two-decimal scale would still pass. Add a higher-precision buy-asset fee with matching details, then assert the transaction retains the exact total and breakdown.

    // Persist the callback's fee exactly as received -- feeResponse.getTotal() is validated
    // against the sum of feeResponse.getDetails() at the fee asset's own precision by
    // RestRateIntegration, independent of reqAsset's scale (a fee may be denominated in the buy
    // asset). Reformatting it here would risk desyncing the stored total from the breakdown.
    txn.setFeeDetails(feeResponse);

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:234

• started_at is a required SEP-31 transaction property, but placing it in the optional-field loop means a response that omits it passes this purported schema check (Gson only validates its format when it is present). The quote-required test relies on this helper, so the claimed schema-compliance gap remains. Require started_at explicitly, ideally by validating the raw response against the actual transaction JSON schema rather than maintaining a partial field list.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt:175

• The schema-compliance helper never requires started_at. A response with that mandatory SEP-31 field removed deserializes with startedAt == null and still passes because started_at is only checked when present in optionalStringFields, so this does not close the claimed GET-transaction schema coverage gap. Assert that the raw field is a non-null string and that the deserialized instant is non-null.