diff --git a/README.md b/README.md index 06e609e1cb..8ea9327e6e 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ [![License](https://badgen.net/badge/license/Apache%202/blue?icon=github&label=License)](https://github.com/stellar/anchor-platform/blob/develop/LICENSE) [![GitHub Version](https://badgen.net/github/release/stellar/anchor-platform?icon=github&label=Latest%20release)](https://github.com/stellar/anchor-platform/releases) -[![Docker](https://badgen.net/badge/Latest%20Release/v4.7.1/blue?icon=docker)](https://hub.docker.com/r/stellar/anchor-platform/tags?page=1&name=4.7.1) +[![Docker](https://badgen.net/badge/Latest%20Release/v4.8.0/blue?icon=docker)](https://hub.docker.com/r/stellar/anchor-platform/tags?page=1&name=4.8.0) ![Develop Branch](https://github.com/stellar/anchor-platform/actions/workflows/on_push_to_develop.yml/badge.svg?branch=develop)
diff --git a/api-schema/src/main/java/org/stellar/anchor/api/asset/Sep31Info.java b/api-schema/src/main/java/org/stellar/anchor/api/asset/Sep31Info.java index be98db8313..064cb9dcbe 100644 --- a/api-schema/src/main/java/org/stellar/anchor/api/asset/Sep31Info.java +++ b/api-schema/src/main/java/org/stellar/anchor/api/asset/Sep31Info.java @@ -26,6 +26,14 @@ public class Sep31Info { */ Sep12Info sep12; + /** + * 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. + */ + Fields fields; + @Data public static class ReceiveOperation { @SerializedName("min_amount") diff --git a/api-schema/src/main/java/org/stellar/anchor/api/sep/sep31/Sep31InfoResponse.java b/api-schema/src/main/java/org/stellar/anchor/api/sep/sep31/Sep31InfoResponse.java index 45dc73b530..268501ad1f 100644 --- a/api-schema/src/main/java/org/stellar/anchor/api/sep/sep31/Sep31InfoResponse.java +++ b/api-schema/src/main/java/org/stellar/anchor/api/sep/sep31/Sep31InfoResponse.java @@ -36,6 +36,8 @@ public static class AssetResponse { List fundingMethods; Sep12Response sep12; + + FieldsResponse fields; } @Data @@ -53,4 +55,16 @@ public static class Sep12TypesResponse { public static class Sep12TypeResponse { String description; } + + @Data + public static class FieldsResponse { + Map transaction; + } + + @Data + public static class FieldResponse { + String description; + List choices; + boolean optional; + } } diff --git a/build.gradle.kts b/build.gradle.kts index 25e3f226ad..ed98a8b3ef 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -213,7 +213,7 @@ subprojects { allprojects { group = "org.stellar.anchor-sdk" - version = "4.7.1" + version = "4.8.0" tasks.jar { manifest { diff --git a/core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java b/core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java index fa49908375..2a274fe44a 100644 --- a/core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java +++ b/core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java @@ -26,6 +26,7 @@ import java.util.*; import lombok.Data; import lombok.SneakyThrows; +import org.apache.commons.lang3.StringUtils; import org.stellar.anchor.api.asset.AssetInfo; import org.stellar.anchor.api.asset.Sep31Info; import org.stellar.anchor.api.asset.StellarAssetInfo; @@ -47,7 +48,6 @@ import org.stellar.anchor.api.sep.sep31.Sep31PatchTransactionRequest; import org.stellar.anchor.api.sep.sep31.Sep31PostTransactionRequest; import org.stellar.anchor.api.sep.sep31.Sep31PostTransactionResponse; -import org.stellar.anchor.api.shared.Amount; import org.stellar.anchor.api.shared.FeeDetails; import org.stellar.anchor.api.shared.StellarId; import org.stellar.anchor.asset.AssetService; @@ -213,15 +213,7 @@ public Sep31PostTransactionResponse postTransaction( sep12Config != null && sep12Config.getReceiver() != null); Sep38Quote quote = Context.get().getQuote(); - FeeDetails feeDetails; - - if (quote != null) { - feeDetails = quote.getFee(); - } else { - Amount fee = Context.get().getFee(); - - feeDetails = new FeeDetails(fee.getAmount(), fee.getAsset(), null); - } + FeeDetails feeDetails = quote != null ? quote.getFee() : Context.get().getFee(); Instant now = Instant.now(); Sep31Transaction txn = @@ -458,51 +450,77 @@ void updateTxAmountsBasedOnQuote() throws ServerErrorException { /** * updateTxAmountsWhenNoQuoteWasUsed will update the transaction amountIn and amountOut based on * the request amount and the fee. + * + *

{@code request.getAmount()} is always denominated in the sell asset ({@code asset_code}), + * regardless of {@code paymentType} -- see {@link #updateFee()}. {@code paymentType} only + * controls whether the fee is added into amount_in (STRICT_RECEIVE) or subtracted out of + * amount_out (STRICT_SEND), and that combination is only performed when the fee is itself + * denominated in the sell asset -- {@link RestRateIntegration} also permits a fee denominated in + * the buy asset, which cannot be combined with the sell-side amount without mixing units. When + * destination_asset requests a real conversion, amount_out is left unset here: the /rate used is + * only INDICATIVE, and per SEP-31 amount_out for a destination_asset conversion is only known + * once the Receiving Anchor actually receives the incoming payment and can apply a firm rate. + * + * @throws ServerErrorException if the /rate response's fee is denominated in an asset that is + * neither the sell asset nor the buy asset -- an invalid upstream response, not bad input + * from the SEP-31 caller. */ - void updateTxAmountsWhenNoQuoteWasUsed() { + void updateTxAmountsWhenNoQuoteWasUsed() throws ServerErrorException { Sep31PostTransactionRequest request = Context.get().getRequest(); Sep31Transaction txn = Context.get().getTransaction(); - Amount feeResponse = Context.get().getFee(); - + FeeDetails feeResponse = Context.get().getFee(); AssetInfo reqAsset = Context.get().getAsset(); int scale = reqAsset.getSignificantDecimals(); BigDecimal reqAmount = decimal(request.getAmount(), scale); - BigDecimal fee = decimal(feeResponse.getAmount(), scale); - BigDecimal amountIn; - BigDecimal amountOut; + String amountInAsset = reqAsset.getId(); + String amountOutAsset = + (request.getDestinationAsset() == null) ? amountInAsset : request.getDestinationAsset(); + boolean isSameAsset = amountInAsset.equals(amountOutAsset); boolean strictSend = sep31Config.getPaymentType() == STRICT_SEND; - if (strictSend) { - // amount_in = req.amount - // amount_out = amount_in - amount fee + boolean feeInSellAsset = amountInAsset.equals(feeResponse.getAsset()); + boolean feeInBuyAsset = amountOutAsset.equals(feeResponse.getAsset()); + if (!feeInSellAsset && !feeInBuyAsset) { + infoF( + "Fee asset ({}) from /rate response matches neither the sell asset ({}) nor the buy " + + "asset ({})", + feeResponse.getAsset(), + amountInAsset, + amountOutAsset); + throw new ServerErrorException( + String.format( + "Fee asset [%s] must match either the sell asset [%s] or the buy asset [%s]", + feeResponse.getAsset(), amountInAsset, amountOutAsset)); + } + + BigDecimal amountIn; + if (strictSend || !feeInSellAsset) { amountIn = reqAmount; - amountOut = amountIn.subtract(fee); } else { - // amount_in = req.amount + fee - // amount_out = req.amount - amountIn = reqAmount.add(fee); - amountOut = reqAmount; + // STRICT_RECEIVE, fee denominated in the sell asset: amount_in = amount + fee. + amountIn = reqAmount.add(decimal(feeResponse.getTotal(), scale)); } - debugF("Updating transaction ({}) with fee ({}) - reqAsset ({})", txn.getId(), fee, reqAsset); - - String amountInAsset = reqAsset.getId(); - String amountOutAsset = request.getDestinationAsset(); + debugF( + "Updating transaction ({}) with fee ({}) - reqAsset ({})", + txn.getId(), + feeResponse, + reqAsset); - boolean isSimpleQuote = Objects.equals(amountInAsset, amountOutAsset); - - // Update transaction txn.setAmountIn(formatAmount(amountIn, scale)); txn.setAmountExpected(formatAmount(amountIn, scale)); txn.setAmountInAsset(amountInAsset); - if (isSimpleQuote) { + txn.setAmountOutAsset(amountOutAsset); + if (isSameAsset) { + BigDecimal amountOut = + strictSend ? amountIn.subtract(decimal(feeResponse.getTotal(), scale)) : reqAmount; txn.setAmountOut(formatAmount(amountOut, scale)); } - txn.setAmountOutAsset(amountOutAsset); - // Update fee - String feeStr = formatAmount(fee, scale); - txn.setFeeDetails(new FeeDetails(feeStr, feeResponse.getAsset())); - Context.get().getFee().setAmount(feeStr); + // 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); } public Sep31GetTransactionResponse getTransaction(WebAuthJwt token, String id) @@ -717,26 +735,26 @@ void updateFee() throws SepValidationException, AnchorException { infoF("Quote: ({}) is missing the 'fee' field", quote.getId()); throw new SepValidationException("Quote is missing the 'fee' field"); } - Amount fee = new Amount(quote.getFee().getTotal(), quote.getFee().getAsset()); - Context.get().setFee(fee); + Context.get().setFee(quote.getFee()); return; } Sep31PostTransactionRequest request = Context.get().getRequest(); String assetName = Context.get().getAsset().getId(); + String destAsset = + (request.getDestinationAsset() == null) ? assetName : request.getDestinationAsset(); infoF("Requesting fee for request ({})", request); + // request.getAmount() is always denominated in asset_code (the sell asset) per SEP-31, + // regardless of paymentType -- so sell_amount is always what's fixed here. paymentType only + // affects how the fee combines with it afterward, in updateTxAmountsWhenNoQuoteWasUsed. var rate = rateIntegration .getRate( GetRateRequest.builder() .type(GetRateRequest.Type.INDICATIVE) - .sellAmount(request.getAmount()) .sellAsset(assetName) - .buyAsset( - (request.getDestinationAsset() == null) - ? assetName - : request.getDestinationAsset()) - .buyAmount(null) + .sellAmount(request.getAmount()) + .buyAsset(destAsset) .clientId(getClientName()) .build()) .getRate(); @@ -745,8 +763,7 @@ void updateFee() throws SepValidationException, AnchorException { throw new SepValidationException("Fee is not present in /rate response"); } infoF("Fee for request ({}) is ({})", request, fee); - Amount amountFee = Amount.create(fee.getTotal(), fee.getAsset()); - Context.get().setFee(amountFee); + Context.get().setFee(fee); } String getClientName() { @@ -754,23 +771,29 @@ String getClientName() { } /** - * validateRequiredFields validates only that the `POST /transactions` or `PATCH - * /transactions/{id}` request body's `fields.transaction` map is present and that the requested - * asset is configured for SEP-31 receive. + * validateRequiredFields validates that the `POST /transactions` or `PATCH /transactions/{id}` + * request body's `fields.transaction` map is present, that the requested asset is configured for + * SEP-31 receive, and that every configured field with {@code optional: false} is actually + * present (and non-blank) in the request. * - *

It intentionally does NOT validate individual field values against a per-asset "required - * fields" spec, and never throws {@link Sep31MissingFieldException} -- the SEP-31 spec itself - * deprecates the `/info` `fields` key and the request's `fields.transaction` map (see {@link + *

The SEP-31 spec deprecates the whole `/info` `fields` key and the request's + * `fields.transaction` map (see {@link * org.stellar.anchor.api.sep.sep31.Sep31PostTransactionRequest#fields}, marked * {@code @Deprecated}) in favor of SEP-12 customer fields: "Pass SEP-9 fields via SEP-12 PUT * /customer instead." KYC completeness for `sender_id`/`receiver_id` is instead enforced by * {@link #verifyCustomerOwnershipAndKyc}, which throws {@link Sep31CustomerInfoNeededException} - * -- the spec-compliant replacement for this mechanism. + * -- the spec-compliant replacement for this mechanism. But as long as an asset's config still + * sets {@code optional: false} on a field, `/info` advertises it as required (see {@link + * #fieldsResponseFromConfig}) -- leaving it unenforced here would let a client see a field + * promised as required and still have its omission silently accepted, contrary to the + * `transaction_info_needed` contract the config implies. * * @throws BadRequestException if the asset is invalid or the `fields` map is missing from the * request + * @throws Sep31MissingFieldException if a field configured with {@code optional: false} is + * missing/blank from the request */ - void validateRequiredFields() throws BadRequestException { + void validateRequiredFields() throws BadRequestException, Sep31MissingFieldException { AssetInfo assetInfo = Context.get().getAsset(); if (assetInfo == null) { infoF("Missing asset information for request ({})", Context.get().getRequest()); @@ -791,6 +814,35 @@ void validateRequiredFields() throws BadRequestException { Context.get().getRequest()); throw new BadRequestException("'fields' field must have one 'transaction' field"); } + + if (fieldSpecs.getFields() != null && fieldSpecs.getFields().getTransaction() != null) { + Map missingFields = new LinkedHashMap<>(); + for (Map.Entry entry : + fieldSpecs.getFields().getTransaction().entrySet()) { + String fieldName = entry.getKey(); + Sep31InfoResponse.FieldResponse fieldResponse = entry.getValue(); + if (fieldResponse != null + && !fieldResponse.isOptional() + && StringUtils.isBlank(requestFields.get(fieldName))) { + missingFields.put( + fieldName, + AssetInfo.Field.builder() + .description(fieldResponse.getDescription()) + .choices(fieldResponse.getChoices()) + .optional(false) + .build()); + } + } + if (!missingFields.isEmpty()) { + infoF( + "Missing required transaction fields [{}] for request ({})", + missingFields.keySet(), + Context.get().getRequest()); + Sep31Info.Fields fields = new Sep31Info.Fields(); + fields.setTransaction(missingFields); + throw new Sep31MissingFieldException(fields); + } + } } @SneakyThrows @@ -809,6 +861,7 @@ private static Sep31InfoResponse sep31InfoResponseFromAssetInfoList(List transaction = new HashMap<>(); + fieldsConfig + .getTransaction() + .forEach( + (fieldName, field) -> { + Sep31InfoResponse.FieldResponse fieldResponse = new Sep31InfoResponse.FieldResponse(); + fieldResponse.setDescription(field.getDescription()); + fieldResponse.setChoices(field.getChoices()); + fieldResponse.setOptional(field.isOptional()); + transaction.put(fieldName, fieldResponse); + }); + Sep31InfoResponse.FieldsResponse fieldsResponse = new Sep31InfoResponse.FieldsResponse(); + fieldsResponse.setTransaction(transaction); + return fieldsResponse; + } + @Data public static class Context { private Sep31Transaction transaction; private Sep31PostTransactionRequest request; private Sep38Quote quote; private WebAuthJwt webAuthJwt; - private Amount fee; + private FeeDetails fee; private AssetInfo asset; private Map transactionFields; private static ThreadLocal context = new ThreadLocal<>(); diff --git a/core/src/main/java/org/stellar/anchor/util/AssetValidator.java b/core/src/main/java/org/stellar/anchor/util/AssetValidator.java index 16d96befc1..5126b343db 100644 --- a/core/src/main/java/org/stellar/anchor/util/AssetValidator.java +++ b/core/src/main/java/org/stellar/anchor/util/AssetValidator.java @@ -205,6 +205,25 @@ static void validateSep31(Sep31Info sep31Info, String assetId) throws InvalidCon } } + // Validate fields.transaction entries + if (sep31Info.getFields() != null && sep31Info.getFields().getTransaction() != null) { + for (Map.Entry entry : + sep31Info.getFields().getTransaction().entrySet()) { + String fieldName = entry.getKey(); + AssetInfo.Field field = entry.getValue(); + if (field == null) { + errors.add( + format( + "Asset %s: SEP-31 fields.transaction.%s must not be empty.", assetId, fieldName)); + } else if (StringUtils.isBlank(field.getDescription())) { + errors.add( + format( + "Asset %s: SEP-31 fields.transaction.%s 'description' must not be blank.", + assetId, fieldName)); + } + } + } + if (!errors.isEmpty()) { throw new InvalidConfigException(errors); } diff --git a/core/src/test/kotlin/org/stellar/anchor/sep31/Sep31ServiceTest.kt b/core/src/test/kotlin/org/stellar/anchor/sep31/Sep31ServiceTest.kt index bafbbc26ab..16ffd0c371 100644 --- a/core/src/test/kotlin/org/stellar/anchor/sep31/Sep31ServiceTest.kt +++ b/core/src/test/kotlin/org/stellar/anchor/sep31/Sep31ServiceTest.kt @@ -23,13 +23,14 @@ import org.stellar.anchor.api.asset.StellarAssetInfo import org.stellar.anchor.api.callback.CustomerIntegration import org.stellar.anchor.api.callback.GetCustomerRequest import org.stellar.anchor.api.callback.GetCustomerResponse +import org.stellar.anchor.api.callback.GetRateRequest import org.stellar.anchor.api.callback.GetRateResponse import org.stellar.anchor.api.callback.RateIntegration import org.stellar.anchor.api.exception.* import org.stellar.anchor.api.sep.sep12.Sep12Status import org.stellar.anchor.api.sep.sep31.* import org.stellar.anchor.api.sep.sep31.Sep31PostTransactionRequest.Sep31TxnFields -import org.stellar.anchor.api.shared.Amount +import org.stellar.anchor.api.shared.FeeDescription import org.stellar.anchor.api.shared.FeeDetails import org.stellar.anchor.api.shared.SepDepositInfo import org.stellar.anchor.api.shared.StellarId @@ -80,8 +81,8 @@ class Sep31ServiceTest { private const val feeJson = """ { - "amount": "2", - "asset": "USDC" + "total": "2", + "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" } """ @@ -233,7 +234,7 @@ class Sep31ServiceTest { private lateinit var sep31Service: Sep31Service private lateinit var request: Sep31PostTransactionRequest private lateinit var txn: Sep31Transaction - private lateinit var fee: Amount + private lateinit var fee: FeeDetails private lateinit var asset: AssetInfo private lateinit var quote: PojoSep38Quote private lateinit var patchRequest: Sep31PatchTransactionRequest @@ -271,34 +272,164 @@ class Sep31ServiceTest { request = gson.fromJson(requestJson, Sep31PostTransactionRequest::class.java) txn = gson.fromJson(txnJson, PojoSep31Transaction::class.java) txn.creator = StellarId.builder().account(TestHelper.TEST_ACCOUNT).memo(null).build() - fee = gson.fromJson(feeJson, Amount::class.java) + fee = gson.fromJson(feeJson, FeeDetails::class.java) asset = gson.fromJson(assetJson, StellarAssetInfo::class.java) quote = gson.fromJson(quoteJson, PojoSep38Quote::class.java) patchRequest = gson.fromJson(patchTxnRequestJson, Sep31PatchTransactionRequest::class.java) } @Test - fun `test update transaction amounts when no quote was used`() { - request.destinationAsset = - "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" + fun `test update transaction amounts when no quote was used for STRICT_SEND, same asset`() { + // request.getAmount() is always denominated in the sell asset -- for STRICT_SEND, amount_in is + // exactly that, and amount_out (known immediately since no real conversion is involved) is the + // fee subtracted out. + every { sep31Config.paymentType } returns STRICT_SEND + request.amount = "100" + request.destinationAsset = null + fee.total = "2" + fee.asset = asset.id Context.get().transaction = txn Context.get().request = request Context.get().fee = fee Context.get().asset = asset - every { sep31Config.paymentType } returns STRICT_SEND - request.amount = "100" - fee.amount = "2" sep31Service.updateTxAmountsWhenNoQuoteWasUsed() - assertEquals(txn.amountIn, "100") - assertEquals(txn.amountOut, "98") + assertEquals("100", txn.amountIn) + assertEquals("98", txn.amountOut) + assertEquals(asset.id, txn.amountInAsset) + assertEquals(asset.id, txn.amountOutAsset) + } + + @Test + fun `test update transaction amounts when no quote was used for STRICT_RECEIVE, same asset`() { + // For STRICT_RECEIVE, the fee is added into amount_in (see Sep31Config.PaymentType) -- + // safe here since, with no destination_asset, the fee can only be denominated in the same + // (sell) asset as the request. every { sep31Config.paymentType } returns STRICT_RECEIVE + request.amount = "100" + request.destinationAsset = null + fee.total = "2" + fee.asset = asset.id + Context.get().transaction = txn + Context.get().request = request + Context.get().fee = fee + Context.get().asset = asset + sep31Service.updateTxAmountsWhenNoQuoteWasUsed() + assertEquals("102", txn.amountIn) assertEquals("100", txn.amountOut) } + @Test + fun `test update transaction amounts when no quote was used defers amount_out for a cross-asset conversion`() { + // The /rate used here is only INDICATIVE, not firm. Per SEP-31, when destination_asset + // requests a real conversion, amount_out is only known once the Receiving Anchor actually + // receives the incoming payment and can apply a firm rate -- so it must be left unset here, + // regardless of paymentType. + every { sep31Config.paymentType } returns STRICT_RECEIVE + request.amount = "100" + request.destinationAsset = stellarJPYC + fee.total = "2" + fee.asset = stellarJPYC // fee denominated in the buy asset -- a valid /rate response shape. + txn.amountOut = null + Context.get().transaction = txn + Context.get().request = request + Context.get().fee = fee + Context.get().asset = asset + + sep31Service.updateTxAmountsWhenNoQuoteWasUsed() + + // Cannot be combined with the sell-side amount_in without mixing units, and isn't needed to + // compute amount_out anyway since amount_out is deferred. + assertEquals("100", txn.amountIn) + assertNull(txn.amountOut) + assertEquals(stellarJPYC, txn.amountOutAsset) + } + + @Test + fun `test update transaction amounts when no quote was used still combines a sell-side fee for STRICT_RECEIVE even in a cross-asset conversion`() { + // The fee happens to be denominated in the sell asset here even though destination_asset + // requests a real conversion -- combining it into amount_in is still safe (same units), even + // though amount_out itself remains deferred (see the test above). + every { sep31Config.paymentType } returns STRICT_RECEIVE + request.amount = "100" + request.destinationAsset = stellarJPYC + fee.total = "2" + fee.asset = asset.id + txn.amountOut = null + Context.get().transaction = txn + Context.get().request = request + Context.get().fee = fee + Context.get().asset = asset + + sep31Service.updateTxAmountsWhenNoQuoteWasUsed() + + assertEquals("102", txn.amountIn) + assertNull(txn.amountOut) + } + + @Test + fun `test update transaction amounts when no quote was used rejects a fee denominated in an unrelated third asset`() { + // The fee is denominated in neither the sell asset (asset.id) nor the buy asset (stellarJPYC) + // -- treating it as the buy asset's amount here would silently mix units. + every { sep31Config.paymentType } returns STRICT_RECEIVE + request.amount = "100" + request.destinationAsset = stellarJPYC + fee.total = "2" + fee.asset = stellarUSDC + Context.get().transaction = txn + Context.get().request = request + Context.get().fee = fee + Context.get().asset = asset + + assertThrows { sep31Service.updateTxAmountsWhenNoQuoteWasUsed() } + } + + @Test + fun `test updateFee always fixes sell_amount to request amount for STRICT_SEND`() { + Context.reset() + Context.get().request = request + Context.get().webAuthJwt = TestHelper.createWebAuthJwt() + Context.get().asset = asset + every { sep31Config.paymentType } returns STRICT_SEND + val rateRequestSlot = slot() + every { rateIntegration.getRate(capture(rateRequestSlot)) } returns + GetRateResponse(GetRateResponse.Rate.builder().fee(fee).build()) + + request.amount = "100" + request.destinationAsset = null + sep31Service.updateFee() + + assertEquals("100", rateRequestSlot.captured.sellAmount) + assertNull(rateRequestSlot.captured.buyAmount) + } + + @Test + fun `test updateFee always fixes sell_amount to request amount for STRICT_RECEIVE too`() { + // request.getAmount() is always denominated in the sell asset per SEP-31, regardless of + // paymentType -- so the /rate request always fixes sell_amount, never buy_amount, to it. + // Fixing buy_amount instead for STRICT_RECEIVE would ask /rate for exactly request.getAmount() + // of the *destination* asset, which is a different, unrelated quantity whenever a real + // cross-asset conversion is involved. + Context.reset() + Context.get().request = request + Context.get().webAuthJwt = TestHelper.createWebAuthJwt() + Context.get().asset = asset + every { sep31Config.paymentType } returns STRICT_RECEIVE + val rateRequestSlot = slot() + every { rateIntegration.getRate(capture(rateRequestSlot)) } returns + GetRateResponse(GetRateResponse.Rate.builder().fee(fee).build()) + + request.amount = "100" + request.destinationAsset = stellarJPYC + sep31Service.updateFee() + + assertEquals("100", rateRequestSlot.captured.sellAmount) + assertNull(rateRequestSlot.captured.buyAmount) + } + @Test fun `test quotes supported and required validation`() { val ex: AnchorException = assertThrows { @@ -743,7 +874,13 @@ class Sep31ServiceTest { customerIntegration, ) every { rateIntegration.getRate(any()) } returns - GetRateResponse(GetRateResponse.Rate.builder().fee(FeeDetails("2", "stellar:USDC")).build()) + GetRateResponse( + GetRateResponse.Rate.builder() + .fee( + FeeDetails("2", "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5") + ) + .build() + ) } private val noSep12AssetJson = @@ -782,7 +919,13 @@ class Sep31ServiceTest { customerIntegration, ) every { rateIntegration.getRate(any()) } returns - GetRateResponse(GetRateResponse.Rate.builder().fee(FeeDetails("2", "stellar:USDC")).build()) + GetRateResponse( + GetRateResponse.Rate.builder() + .fee( + FeeDetails("2", "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5") + ) + .build() + ) } private fun ownershipTestRequest(senderId: String? = null, receiverId: String? = null) = @@ -1086,6 +1229,35 @@ class Sep31ServiceTest { assertEquals("text", txnSlot.captured.refundMemoType) } + @Test + fun `test postTransaction carries the fee breakdown through the no-quote path`() { + useNoSep12AssetService() + every { rateIntegration.getRate(any()) } returns + GetRateResponse( + GetRateResponse.Rate.builder() + .fee( + FeeDetails( + "2", + "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5", + listOf(FeeDescription("Sell fee", null, "2")) + ) + ) + .build() + ) + val postTxRequest = ownershipTestRequest() + + val txnSlot = slot() + every { txnStore.save(capture(txnSlot)) } answers + { + firstArg().also { it.id = "ABC-123" } + } + + val jwtToken = TestHelper.createWebAuthJwt(accountMemo = TestHelper.TEST_MEMO) + assertDoesNotThrow { sep31Service.postTransaction(jwtToken, postTxRequest) } + + assertEquals(listOf(FeeDescription("Sell fee", null, "2")), txnSlot.captured.feeDetails.details) + } + @Test fun `test postTransaction rejects a refund memo without a refund memo type`() { useNoSep12AssetService() @@ -1400,6 +1572,7 @@ class Sep31ServiceTest { verify(exactly = 1) { customerIdOwnerStore.verifyOrClaim("needs-info-but-not-required", any(), any(), any()) } + verify(exactly = 0) { customerIntegration.getCustomer(any()) } } @Test @@ -1570,7 +1743,13 @@ class Sep31ServiceTest { // Provide fee response. every { rateIntegration.getRate(any()) } returns - GetRateResponse(GetRateResponse.Rate.builder().fee(FeeDetails("2", "stellar:USDC")).build()) + GetRateResponse( + GetRateResponse.Rate.builder() + .fee( + FeeDetails("2", "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5") + ) + .build() + ) // Make sure we can get the sender and receiver customers val mockCustomer = GetCustomerResponse() @@ -1594,7 +1773,22 @@ class Sep31ServiceTest { private val usdcJson = """ - {"enabled":true,"quotes_supported":true,"quotes_required":true,"min_amount":1,"max_amount":1000000,"funding_methods":["SEPA","SWIFT"]} + { + "enabled":true, + "quotes_supported":true, + "quotes_required":true, + "min_amount":1, + "max_amount":1000000, + "funding_methods":["SEPA","SWIFT"], + "fields": { + "transaction": { + "receiver_account_number": {"description": "bank account number of the destination", "optional": false}, + "type": {"description": "type of deposit to make", "choices": ["SEPA", "SWIFT"], "optional": false}, + "receiver_routing_number": {"description": "routing number of the destination bank account", "optional": false}, + "receiver_phone_number": {"description": "phone number of the receiver", "optional": true} + } + } + } """ .trimIndent() @@ -1659,6 +1853,135 @@ class Sep31ServiceTest { ) } + @Test + fun `test INFO response advertises fields when the asset configures it`() { + val withFieldsAssetJson = + """ + { + "items": [ + { + "id": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5", + "distribution_account": "GA7FYRB5VREZKOBIIKHG5AVTPFGWUBPOBF7LTYG4GTMFVIOOD2DWAL7I", + "significant_decimals": 2, + "sep31": { + "enabled": true, + "receive": {"min_amount": 1, "max_amount": 1000000, "methods": ["SEPA", "SWIFT"]}, + "quotes_supported": false, + "quotes_required": false, + "fields": { + "transaction": { + "receiver_account_number": { + "description": "Bank account number of the receiver.", + "optional": false + } + } + } + } + } + ] + } + """ + .trimIndent() + val infoOnlyService = + Sep31Service( + languageConfig, + sep31Config, + txnStore, + quoteStore, + DefaultAssetService.fromJsonContent(withFieldsAssetJson), + rateIntegration, + eventService, + Clock.systemUTC(), + exchangeAmountsCalculator, + customerIdOwnerStore, + customerIntegration, + ) + + val fields = infoOnlyService.info.receive["USDC"]!!.fields + assertEquals(setOf("receiver_account_number"), fields.transaction.keys) + val field = fields.transaction["receiver_account_number"]!! + assertEquals("Bank account number of the receiver.", field.description) + assertFalse(field.isOptional) + } + + @Test + fun `test INFO response omits fields when the asset doesn't configure it`() { + assertNull(sep31Service.info.receive["JPYC"]!!.fields) + } + + @Test + fun `test asset config silently drops a null field definition instead of crashing`() { + // A bare "receiver_account_number:" in YAML (or a literal JSON null) deserializes to a null + // map value, but DefaultAssetService's internal round-trip (raw Map -> gson.toJson -> + // JsonObject -> gson.toJson -> StellarAssetInfo) uses a default (non-serializeNulls) Gson, + // which drops null map entries when re-serializing -- confirmed by reproducing the exact + // pipeline standalone. So a null field entry never reaches AssetValidator or Sep31Service at + // all: it's silently absent from the loaded config rather than rejected at startup or causing + // an NPE. This is a pre-existing quirk of the shared parsing pipeline (affects any Map-valued + // asset config, not unique to `fields`); documenting the actual behavior here rather than + // fixing the broader pipeline, which is outside this field-validation work's scope. + val nullFieldDefinitionJson = + """ + { + "items": [ + { + "id": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5", + "distribution_account": "GA7FYRB5VREZKOBIIKHG5AVTPFGWUBPOBF7LTYG4GTMFVIOOD2DWAL7I", + "significant_decimals": 2, + "sep31": { + "enabled": true, + "receive": {"min_amount": 1, "max_amount": 1000000, "methods": ["SEPA", "SWIFT"]}, + "quotes_supported": false, + "quotes_required": false, + "fields": { + "transaction": { + "receiver_account_number": null + } + } + } + } + ] + } + """ + .trimIndent() + + val nullFieldAssetService = DefaultAssetService.fromJsonContent(nullFieldDefinitionJson) + val fields = nullFieldAssetService.getAssets().first().sep31.fields + assertTrue(fields == null || fields.transaction.isEmpty()) + } + + @Test + fun `test asset config rejects a blank field description`() { + val blankFieldDescriptionJson = + """ + { + "items": [ + { + "id": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5", + "distribution_account": "GA7FYRB5VREZKOBIIKHG5AVTPFGWUBPOBF7LTYG4GTMFVIOOD2DWAL7I", + "significant_decimals": 2, + "sep31": { + "enabled": true, + "receive": {"min_amount": 1, "max_amount": 1000000, "methods": ["SEPA", "SWIFT"]}, + "quotes_supported": false, + "quotes_required": false, + "fields": { + "transaction": { + "receiver_account_number": {"optional": false} + } + } + } + } + ] + } + """ + .trimIndent() + + assertThrows { + DefaultAssetService.fromJsonContent(blankFieldDescriptionJson) + } + } + @Test fun `test asset config rejects a blank sep12 description`() { val blankSep12DescriptionJson = @@ -1705,6 +2028,41 @@ class Sep31ServiceTest { assetInfo.id = originalId val ex3 = assertThrows { sep31Service.validateRequiredFields() } assertEquals("'fields' field must have one 'transaction' field", ex3.message) + + // USDC's config (test_assets.json) marks receiver_routing_number/receiver_account_number/type + // as required -- /info advertises that, so it must actually be enforced here. A missing + // required field must surface as the SEP-31 transaction_info_needed contract, not a plain + // BadRequestException, so sending anchors can discover what to add before retrying. + Context.get().transactionFields = mapOf("receiver_routing_number" to "123", "type" to "SWIFT") + val ex4 = assertThrows { sep31Service.validateRequiredFields() } + assertEquals(setOf("receiver_account_number"), ex4.missingFields.transaction.keys) + assertEquals( + "bank account number of the destination", + ex4.missingFields.transaction["receiver_account_number"]!!.description, + ) + + // A whitespace-only value must not satisfy a required field either. + Context.get().transactionFields = + mapOf( + "receiver_routing_number" to "123", + "receiver_account_number" to " ", + "type" to "SWIFT", + ) + val ex5 = assertThrows { sep31Service.validateRequiredFields() } + assertEquals(setOf("receiver_account_number"), ex5.missingFields.transaction.keys) + + // Every missing field must be reported together, not just the first one encountered. + Context.get().transactionFields = mapOf("receiver_routing_number" to "123") + val ex6 = assertThrows { sep31Service.validateRequiredFields() } + assertEquals(setOf("receiver_account_number", "type"), ex6.missingFields.transaction.keys) + + Context.get().transactionFields = + mapOf( + "receiver_routing_number" to "123", + "receiver_account_number" to "456", + "type" to "SWIFT" + ) + assertDoesNotThrow { sep31Service.validateRequiredFields() } } @Test @@ -1718,7 +2076,7 @@ class Sep31ServiceTest { request.destinationAsset = "USDC" sep31Service.updateFee() var fee = Context.get().fee - assertEquals(quote.fee.total, fee.amount) + assertEquals(quote.fee.total, fee.total) assertEquals(quote.fee.asset, fee.asset) // No quote @@ -1738,13 +2096,13 @@ class Sep31ServiceTest { request.destinationAsset = "USDC" sep31Service.updateFee() fee = Context.get().fee - assertEquals("10", fee.amount) + assertEquals("10", fee.total) assertEquals("stellar:USDC:GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN", fee.asset) request.destinationAsset = null sep31Service.updateFee() fee = Context.get().fee - assertEquals("10", fee.amount) + assertEquals("10", fee.total) assertEquals("stellar:USDC:GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN", fee.asset) } diff --git a/core/src/test/resources/test_assets.json.quotes_not_supported b/core/src/test/resources/test_assets.json.quotes_not_supported index bd6a204424..f5f19e45de 100644 --- a/core/src/test/resources/test_assets.json.quotes_not_supported +++ b/core/src/test/resources/test_assets.json.quotes_not_supported @@ -53,6 +53,7 @@ "fields": { "transaction": { "type": { + "description": "type of deposit to make", "choices": [ "SWIFT" ] diff --git a/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31CustomerOwnershipTests.kt b/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31CustomerOwnershipTests.kt index 958336231a..1f69142203 100644 --- a/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31CustomerOwnershipTests.kt +++ b/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31CustomerOwnershipTests.kt @@ -120,6 +120,21 @@ class Sep31CustomerOwnershipTests : IntegrationTestBase(TestConfig()) { } } + // verifyCustomerOwnershipAndKyc (Sep31Service.java) always reverse-looks-up an unclaimed + // customer id through customerIntegration -- using the *authenticated caller's* account/memo, + // not the id itself -- before kycRequired is ever considered; only the later KYC-status + // enforcement is skipped when the asset doesn't advertise sep12.. A never-registered + // random id can't match the caller's own customer record, so the claim is rejected regardless + // of whether KYC is required for that role. + @Test + fun `test caller cannot claim a receiver_id with no matching SEP-12 customer record even when KYC is not required`() { + val neverRegisteredReceiverId = java.util.UUID.randomUUID().toString() + + assertThrows { + sep31Client.postTransaction(mkTxnRequest(neverRegisteredReceiverId)) + } + } + @Test fun `test same caller can reuse a receiver_id it already owns`() { val receiverCustomerRequest = diff --git a/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31PlatformApiTests.kt b/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31PlatformApiTests.kt index 0d9ae93759..28e5ed4948 100644 --- a/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31PlatformApiTests.kt +++ b/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31PlatformApiTests.kt @@ -1,5 +1,7 @@ package org.stellar.anchor.platform.integrationtest +import io.ktor.http.Url +import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.MethodOrderer import org.junit.jupiter.api.Order import org.junit.jupiter.api.Test @@ -7,9 +9,12 @@ import org.junit.jupiter.api.TestMethodOrder import org.stellar.anchor.api.sep.sep12.Sep12PutCustomerRequest import org.stellar.anchor.api.sep.sep31.Sep31PostTransactionRequest import org.stellar.anchor.util.GsonUtils +import org.stellar.reference.client.AnchorReferenceServerClient @TestMethodOrder(MethodOrderer.OrderAnnotation::class) class Sep31PlatformApiTests : PlatformApiTests() { + private val anchorReferenceServerClient = + AnchorReferenceServerClient(Url(config.env["reference.server.url"]!!)) /** * 1. pending_receiver -> request_onchain_funds (called by reference server Sep31EventProcessor) * 2. pending_sender -> notify_onchain_funds_received @@ -62,6 +67,11 @@ class Sep31PlatformApiTests : PlatformApiTests() { val receiveRequest = gson.fromJson(receiveRequestJson, Sep31PostTransactionRequest::class.java) val receiveResponse = sep31Client.postTransaction(receiveRequest) + // This flow drives the transaction through RPC calls itself (including an error/recovery + // path the reference server doesn't know about), so it must opt out of the reference server's + // own automatic advancement -- otherwise both race to advance the same transaction. + runBlocking { anchorReferenceServerClient.skipSep31AutoAdvance(receiveResponse.id) } + repeat(5) { if (sep31Client.getTransaction(receiveResponse.id).transaction.status == "pending_sender") return@repeat @@ -145,7 +155,7 @@ private const val SEP_31_RECEIVE_REFUNDED_SHORT_FLOW_ACTION_RESPONSES = }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "transfer_received_at": "2024-06-13T20:02:49Z", @@ -196,7 +206,7 @@ private const val SEP_31_RECEIVE_REFUNDED_SHORT_FLOW_ACTION_RESPONSES = }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "transfer_received_at": "2024-06-13T20:02:49Z", @@ -340,7 +350,7 @@ private const val SEP_31_RECEIVE_COMPLETE_FULL_WITH_RECOVERY_FLOW_ACTION_RESPONS }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "started_at": "2024-06-25T20:33:17.013738Z", @@ -393,7 +403,7 @@ private const val SEP_31_RECEIVE_COMPLETE_FULL_WITH_RECOVERY_FLOW_ACTION_RESPONS }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "started_at": "2024-06-25T20:33:17.013738Z", @@ -446,7 +456,7 @@ private const val SEP_31_RECEIVE_COMPLETE_FULL_WITH_RECOVERY_FLOW_ACTION_RESPONS }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "started_at": "2024-06-25T20:33:17.013738Z", @@ -499,7 +509,7 @@ private const val SEP_31_RECEIVE_COMPLETE_FULL_WITH_RECOVERY_FLOW_ACTION_RESPONS }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "started_at": "2024-06-25T20:33:17.013738Z", @@ -552,7 +562,7 @@ private const val SEP_31_RECEIVE_COMPLETE_FULL_WITH_RECOVERY_FLOW_ACTION_RESPONS }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "started_at": "2024-06-25T20:33:17.013738Z", @@ -606,7 +616,7 @@ private const val SEP_31_RECEIVE_COMPLETE_FULL_WITH_RECOVERY_FLOW_ACTION_RESPONS }, "amount_out": {}, "fee_details": { - "total": "1", + "total": "1.00", "asset": "stellar:USDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5" }, "started_at": "2024-06-25T20:33:17.013738Z", diff --git a/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt b/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt index 88f199c415..2177955abe 100644 --- a/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt +++ b/essential-tests/src/testFixtures/kotlin/org/stellar/anchor/platform/integrationtest/Sep31Tests.kt @@ -1,16 +1,22 @@ package org.stellar.anchor.platform.integrationtest +import io.ktor.http.Url import java.time.Instant +import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.* import org.junit.jupiter.api.Assertions.* import org.junit.jupiter.api.parallel.Execution import org.junit.jupiter.api.parallel.ExecutionMode.SAME_THREAD +import org.skyscreamer.jsonassert.Customization import org.skyscreamer.jsonassert.JSONAssert import org.skyscreamer.jsonassert.JSONCompareMode import org.skyscreamer.jsonassert.JSONCompareMode.LENIENT +import org.skyscreamer.jsonassert.comparator.CustomComparator import org.springframework.data.domain.Sort.Direction import org.springframework.data.domain.Sort.Direction.DESC -import org.stellar.anchor.api.exception.SepException +import org.stellar.anchor.api.exception.SepNotAuthorizedException +import org.stellar.anchor.api.exception.SepNotFoundException +import org.stellar.anchor.api.exception.SepValidationException import org.stellar.anchor.api.platform.* import org.stellar.anchor.api.platform.PlatformTransactionData.Sep.SEP_31 import org.stellar.anchor.api.platform.PlatformTransactionData.builder @@ -34,7 +40,11 @@ import org.stellar.anchor.platform.integrationtest.Sep12Tests.Companion.testCust import org.stellar.anchor.platform.printRequest import org.stellar.anchor.util.GsonUtils import org.stellar.anchor.util.Log.debug +import org.stellar.anchor.util.MemoHelper +import org.stellar.anchor.util.SepHelper import org.stellar.anchor.util.StringHelper.json +import org.stellar.reference.client.AnchorReferenceServerClient +import org.stellar.sdk.MuxedAccount lateinit var savedTxn: Sep31GetTransactionResponse @@ -49,6 +59,8 @@ class Sep31Tests : IntegrationTestBase(TestConfig()) { Sep38Client(toml.getString("ANCHOR_QUOTE_SERVER"), this.token.token) private val platformApiClient: PlatformApiClient = PlatformApiClient(AuthHelper.forNone(), config.env["platform.server.url"]!!) + private val anchorReferenceServerClient = + AnchorReferenceServerClient(Url(config.env["reference.server.url"]!!)) @Test fun `test info endpoint`() { @@ -57,6 +69,34 @@ class Sep31Tests : IntegrationTestBase(TestConfig()) { JSONAssert.assertEquals(expectedSep31Info, gson.toJson(info), JSONCompareMode.STRICT) } + @Test + fun `test DIRECT_PAYMENT_SERVER has expected format`() { + val directPaymentServerUrl = toml.getString("DIRECT_PAYMENT_SERVER") + assertNotNull(directPaymentServerUrl) + assertFalse( + directPaymentServerUrl.endsWith("/"), + "DIRECT_PAYMENT_SERVER must not end with a '/'" + ) + // Parse the URI rather than substring-matching it, so a value like "httpsx://example.com", + // "http://evil.example/?localhost", "https:foo" (no host), or "ftp://localhost" (wrong scheme + // for the local exemption) isn't mistaken for a compliant/exempted URL. + val uri = java.net.URI(directPaymentServerUrl) + assertTrue( + !uri.host.isNullOrBlank(), + "DIRECT_PAYMENT_SERVER must be an absolute URI with a non-blank host" + ) + // URI schemes and hosts are case-insensitive (RFC 3986), so "HTTPS://" or "http://LOCALHOST" + // are equally valid -- compare case-insensitively rather than rejecting them. + val scheme = uri.scheme?.lowercase() + val host = uri.host?.lowercase() + val isLocalHttpException = + scheme == "http" && (host == "localhost" || host == "host.docker.internal") + assertTrue( + scheme == "https" || isLocalHttpException, + "DIRECT_PAYMENT_SERVER must use https (http exempted only for localhost/host.docker.internal, for local testing)" + ) + } + @Test @Order(30) fun `test post and get transactions`() { @@ -64,11 +104,234 @@ class Sep31Tests : IntegrationTestBase(TestConfig()) { val postTxResponse = createTx(senderCustomer, receiverCustomer) + // `savedTxn` gets patched directly by a later ordered test (`test patch, get and compare`), + // so it must opt out of Sep31EventProcessor's automatic advancement -- otherwise the + // reference server's own reaction to this transaction's status races that later patch. + runBlocking { anchorReferenceServerClient.skipSep31AutoAdvance(postTxResponse.id) } + // GET Sep31 transaction - savedTxn = sep31Client.getTransaction(postTxResponse.id) - JSONAssert.assertEquals(expectedTxn, json(savedTxn), LENIENT) + val rawTxnJson = fetchRawTransaction(postTxResponse.id) + savedTxn = gson.fromJson(rawTxnJson, Sep31GetTransactionResponse::class.java) + // Sep31EventProcessor.onTransactionCreated unconditionally calls request_onchain_funds as + // soon as it sees the creation event -- it doesn't consult the skip-auto-advance + // registration above (only onTransactionStatusChanged does), and that registration itself + // adds a round trip before this GET, widening the window for that automatic call to land + // first. So the transaction may already have moved from pending_receiver to pending_sender + // by the time this GET runs; accept either rather than pinning down that transient status. + JSONAssert.assertEquals( + expectedTxn, + rawTxnJson, + CustomComparator( + LENIENT, + Customization("transaction.status") { o1, o2 -> + val acceptable = setOf(PENDING_RECEIVER.status, PENDING_SENDER.status) + o1 in acceptable && o2 in acceptable + }, + ), + ) assertEquals(postTxResponse.id, savedTxn.transaction.id) - assertEquals(PENDING_RECEIVER.status, savedTxn.transaction.status) + assertTrue( + savedTxn.transaction.status == PENDING_RECEIVER.status || + savedTxn.transaction.status == PENDING_SENDER.status, + "Expected status to be pending_receiver or pending_sender, was ${savedTxn.transaction.status}", + ) + assertCompliesWithProtocolSchema(rawTxnJson, savedTxn) + } + + private fun fetchRawTransaction(txId: String): String { + return sep31Client.httpGet( + "${toml.getString("DIRECT_PAYMENT_SERVER")}/transactions/$txId", + this.token.token + )!! + } + + /** + * Validates the *raw* response body against the SEP-31 GET-transaction schema before any + * deserialization can drop unknown properties or coerce types -- `rawJson` is the exact string + * the server returned. `txn` (already deserialized by the caller) is only used for the semantic + * checks below that a structural schema can't express: `stellar_account_id` / + * `stellar_memo`+`stellar_memo_type` well-formedness. Gson already guarantees `started_at`/ + * `completed_at` parse as valid date-times on `txn`, since a malformed value would have failed + * deserialization before this method is even reached. + */ + private fun assertCompliesWithProtocolSchema(rawJson: String, txn: Sep31GetTransactionResponse) { + val root = com.google.gson.JsonParser.parseString(rawJson).asJsonObject + assertTrue(root.has("transaction"), "response body must have a 'transaction' object") + // The stellar-anchor-tests getTransactionSchema sets `additionalProperties: false` on the + // root object -- deserialization alone can't catch a violation of that (Gson silently drops + // unknown fields), so the raw JSON's key set must be checked explicitly here. + assertEquals( + setOf("transaction"), + root.keySet(), + "response body must not have top-level properties other than 'transaction'" + ) + val transaction = root.getAsJsonObject("transaction") + + assertTrue(transaction.has("id") && !transaction.get("id").isJsonNull, "'id' is required") + assertTrue( + transaction.get("id").asJsonPrimitive.isString, + "'id' must be a string, not a coerced numeric/boolean value" + ) + assertFalse(transaction.get("id").asString.isBlank(), "'id' must not be blank") + + val validStatuses = SepHelper.sep31Statuses.map { it.status }.toSet() + assertTrue( + transaction.has("status") && !transaction.get("status").isJsonNull, + "'status' is required" + ) + assertTrue( + validStatuses.contains(transaction.get("status").asString), + "'${transaction.get("status").asString}' is not a status defined by the SEP-31 GET-transaction schema" + ) + + assertTrue( + transaction.has("fee_details") && !transaction.get("fee_details").isJsonNull, + "'fee_details' is required" + ) + val feeDetails = transaction.getAsJsonObject("fee_details") + assertTrue( + feeDetails.has("total") && feeDetails.get("total").asJsonPrimitive.isString, + "'fee_details.total' is required and must be a string" + ) + assertTrue( + feeDetails.has("asset") && feeDetails.get("asset").asJsonPrimitive.isString, + "'fee_details.asset' is required and must be a string" + ) + if (feeDetails.has("details") && !feeDetails.get("details").isJsonNull) { + assertTrue(feeDetails.get("details").isJsonArray, "'fee_details.details' must be an array") + feeDetails.getAsJsonArray("details").forEach { detail -> + val detailObj = detail.asJsonObject + assertTrue( + detailObj.has("name") && detailObj.get("name").asJsonPrimitive.isString, + "'fee_details.details[].name' is required and must be a string" + ) + assertTrue( + detailObj.has("amount") && detailObj.get("amount").asJsonPrimitive.isString, + "'fee_details.details[].amount' is required and must be a string" + ) + if (detailObj.has("description") && !detailObj.get("description").isJsonNull) { + assertTrue( + detailObj.get("description").asJsonPrimitive.isString, + "'fee_details.details[].description' must be a string when present" + ) + } + } + } + + val optionalStringFields = + listOf( + "status_message", + "amount_in", + "amount_in_asset", + "amount_out", + "amount_out_asset", + "amount_fee", + "amount_fee_asset", + "quote_id", + "stellar_account_id", + "stellar_memo_type", + "stellar_memo", + "started_at", + "updated_at", + "completed_at", + "stellar_transaction_id", + "external_transaction_id", + "required_info_message", + ) + for (field in optionalStringFields) { + if (transaction.has(field) && !transaction.get(field).isJsonNull) { + assertTrue( + transaction.get(field).asJsonPrimitive.isString, + "'$field' must be a string when present" + ) + } + } + if (transaction.has("status_eta") && !transaction.get("status_eta").isJsonNull) { + assertTrue( + transaction.get("status_eta").asJsonPrimitive.isNumber, + "'status_eta' must be a number when present" + ) + } + if (transaction.has("refunded") && !transaction.get("refunded").isJsonNull) { + assertTrue( + transaction.get("refunded").asJsonPrimitive.isBoolean, + "'refunded' must be a boolean when present" + ) + } + if (transaction.has("refunds") && !transaction.get("refunds").isJsonNull) { + assertTrue( + transaction.get("refunds").isJsonObject, + "'refunds' must be an object when present" + ) + val refunds = transaction.getAsJsonObject("refunds") + assertTrue( + refunds.has("amount_refunded") && refunds.get("amount_refunded").asJsonPrimitive.isString, + "'refunds.amount_refunded' is required and must be a string" + ) + assertTrue( + refunds.has("amount_fee") && refunds.get("amount_fee").asJsonPrimitive.isString, + "'refunds.amount_fee' is required and must be a string" + ) + assertTrue(refunds.has("payments"), "'refunds.payments' is required") + assertTrue(refunds.get("payments").isJsonArray, "'refunds.payments' must be an array") + refunds.getAsJsonArray("payments").forEach { payment -> + val paymentObj = payment.asJsonObject + assertTrue( + paymentObj.has("id") && paymentObj.get("id").asJsonPrimitive.isString, + "'refunds.payments[].id' is required and must be a string" + ) + assertTrue( + paymentObj.has("amount") && paymentObj.get("amount").asJsonPrimitive.isString, + "'refunds.payments[].amount' is required and must be a string" + ) + assertTrue( + paymentObj.has("fee") && paymentObj.get("fee").asJsonPrimitive.isString, + "'refunds.payments[].fee' is required and must be a string" + ) + } + } + if ( + transaction.has("required_info_updates") && + !transaction.get("required_info_updates").isJsonNull + ) { + assertTrue( + transaction.get("required_info_updates").isJsonObject, + "'required_info_updates' must be an object when present" + ) + } + + // Semantic checks a structural schema can't express. + txn.transaction.stellarAccountId?.let { + // stellar_account_id can be a muxed M... destination (the production request validator + // accepts these, see SepRequestValidator#validateAccount) as well as a classic G... account + // -- MuxedAccount accepts both encodings, unlike KeyPair.fromAccountId which only accepts + // classic accounts. + try { + MuxedAccount(it) + } catch (e: Exception) { + fail("'stellar_account_id' must be a valid Stellar account (classic or muxed)", e) + } + } + // Validated independently of whether `stellar_memo` is present, so a response carrying an + // invalid `stellar_memo_type` with no memo doesn't slip past this check unnoticed. + txn.transaction.stellarMemoType?.let { + assertTrue( + it == "text" || it == "id" || it == "hash", + "'$it' is not a stellar_memo_type defined by the SEP-31 GET-transaction schema" + ) + } + txn.transaction.stellarMemo?.let { + try { + // MemoHelper.makeMemo (not Memo.id's Long overload) supports the full uint64 range SEP-31 + // memo ids can carry, not just what fits in a signed 64-bit Long. + MemoHelper.makeMemo(it, txn.transaction.stellarMemoType) + } catch (e: Exception) { + fail( + "invalid 'stellar_memo' for 'stellar_memo_type' (${txn.transaction.stellarMemoType})", + e + ) + } + } } private fun mkCustomers(): Pair { @@ -236,7 +499,88 @@ class Sep31Tests : IntegrationTestBase(TestConfig()) { val txnRequest = gson.fromJson(postTxnRequest, Sep31PostTransactionRequest::class.java) txnRequest.assetCode = "bad-asset-code" txnRequest.receiverId = pr!!.id - assertThrows { sep31Client.postTransaction(txnRequest) } + assertThrows { sep31Client.postTransaction(txnRequest) } + } + + @Test + fun `test returns 400 when no asset_code is given`() { + // The asset lookup (and its 400 rejection) happens before any SEP-12 customer check, so a + // mock sender/receiver id is enough -- no real customer needs to be registered. + val txnRequest = gson.fromJson(postTxnRequest, Sep31PostTransactionRequest::class.java) + txnRequest.assetCode = null + assertThrows { sep31Client.postTransaction(txnRequest) } + } + + @Test + fun `test returns 400 when no amount is given`() { + // amount validation happens before any SEP-12 customer check, so a mock sender/receiver id is + // enough -- no real customer needs to be registered. + val txnRequest = gson.fromJson(postTxnRequest, Sep31PostTransactionRequest::class.java) + txnRequest.amount = null + assertThrows { sep31Client.postTransaction(txnRequest) } + } + + @Test + fun `test requires a SEP-10 JWT`() { + val unauthenticatedClient = Sep31Client(toml.getString("DIRECT_PAYMENT_SERVER"), "") + val txnRequest = gson.fromJson(postTxnRequest, Sep31PostTransactionRequest::class.java) + assertThrows { unauthenticatedClient.postTransaction(txnRequest) } + } + + @Test + fun `test returns 404 for a non-existent transaction`() { + assertThrows { sep31Client.getTransaction("not-an-id") } + } + + @Test + fun `test quotes_required rejects a transaction with no quote_id`() { + // preValidateQuote's quote_id check runs before any SEP-12 customer check, so mock + // sender/receiver ids are enough -- no real customer needs to be registered. + val txnRequest = gson.fromJson(postTxnRequest, Sep31PostTransactionRequest::class.java) + txnRequest.assetCode = "SRT" + txnRequest.assetIssuer = srtAssetIssuer + txnRequest.quoteId = null + assertThrows { sep31Client.postTransaction(txnRequest) } + } + + @Test + fun `test quotes_required can create and fetch a transaction with a quote`() { + val (senderCustomer, receiverCustomer) = mkCustomers() + val quote = sep38Client.postQuote("stellar:SRT:$srtAssetIssuer", "10", "iso4217:USD") + + val txnRequest = gson.fromJson(postTxnRequest, Sep31PostTransactionRequest::class.java) + txnRequest.assetCode = "SRT" + txnRequest.assetIssuer = srtAssetIssuer + txnRequest.senderId = senderCustomer.id + txnRequest.receiverId = receiverCustomer.id + txnRequest.quoteId = quote.id + val postTxResponse = sep31Client.postTransaction(txnRequest) + assertNotNull(postTxResponse.id) + + val rawTxnJson = fetchRawTransaction(postTxResponse.id) + val fetchedTxn = gson.fromJson(rawTxnJson, Sep31GetTransactionResponse::class.java) + assertEquals(postTxResponse.id, fetchedTxn.transaction.id) + // Sep31EventProcessor.onTransactionCreated unconditionally calls request_onchain_funds as + // soon as it sees the creation event, so this transaction may already have moved from + // pending_receiver to pending_sender by the time this GET runs -- accept either rather than + // pinning down that transient status. + assertTrue( + fetchedTxn.transaction.status == PENDING_RECEIVER.status || + fetchedTxn.transaction.status == PENDING_SENDER.status, + "Expected status to be pending_receiver or pending_sender, was ${fetchedTxn.transaction.status}", + ) + assertCompliesWithProtocolSchema(rawTxnJson, fetchedTxn) + + // Beyond id/status, verify the transaction actually reflects the quote used to create it -- + // rather than merely accepting quote_id while computing amounts independently. + val transaction = fetchedTxn.transaction + assertEquals(quote.id, transaction.quoteId) + assertEquals(quote.sellAmount, transaction.amountIn) + assertEquals(quote.sellAsset, transaction.amountInAsset) + assertEquals(quote.buyAmount, transaction.amountOut) + assertEquals(quote.buyAsset, transaction.amountOutAsset) + assertEquals(quote.fee.total, transaction.feeDetails.total) + assertEquals(quote.fee.asset, transaction.feeDetails.asset) } @Test @@ -259,6 +603,8 @@ class Sep31Tests : IntegrationTestBase(TestConfig()) { } } +private const val srtAssetIssuer = "GCDNJUBQSX7AJWLJACMJ7I4BC3Z47BQUTMHEICZLE6MU4KQBRYG5JY6B" + private const val postTxnRequest = """{ "amount": "10", @@ -287,7 +633,14 @@ private const val expectedTxn = "amount_out_asset": "stellar:JPYC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP", "fee_details": { "total": "1.00", - "asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP" + "asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP", + "details": [ + { + "name": "Sell fee", + "description": "Fee related to selling the asset.", + "amount": "1.00" + } + ] } } } @@ -311,6 +664,31 @@ private const val expectedSep31Info = "quotes_required": false, "min_amount": 0, "max_amount": 10, + "funding_methods": ["SEPA","SWIFT"], + "fields": { + "transaction": { + "receiver_account_number": { + "description": "Bank account number of the receiver.", + "optional": false + }, + "receiver_routing_number": { + "description": "Bank routing number of the receiver.", + "optional": true + }, + "type": { + "description": "Type of the receiver's bank transfer.", + "optional": true, + "choices": ["SEPA", "SWIFT"] + } + } + } + }, + "SRT": { + "enabled": true, + "quotes_supported": true, + "quotes_required": true, + "min_amount": 0, + "max_amount": 1000000, "funding_methods": ["SEPA","SWIFT"] } } diff --git a/kotlin-reference-server/src/main/kotlin/org/stellar/reference/client/AnchorReferenceServerClient.kt b/kotlin-reference-server/src/main/kotlin/org/stellar/reference/client/AnchorReferenceServerClient.kt index 40ede53734..f4ef5c9e87 100644 --- a/kotlin-reference-server/src/main/kotlin/org/stellar/reference/client/AnchorReferenceServerClient.kt +++ b/kotlin-reference-server/src/main/kotlin/org/stellar/reference/client/AnchorReferenceServerClient.kt @@ -76,4 +76,27 @@ class AnchorReferenceServerClient(val endpoint: Url) { } return gson.fromJson(response.body(), SendEventRequest::class.java) } + + // Test-only: opts a SEP-31 transaction out of Sep31EventProcessor's automatic advancement, for a + // test that drives it through RPC calls itself instead (see Sep31TestRoute.kt). Ktor's default + // HttpClient doesn't throw on a non-2xx response, and the caller relies on this registration to + // avoid racing the reference server -- checking the status here turns a silent no-op (e.g. the + // route not being registered) into a clear failure instead of a flaky race down the line. + suspend fun skipSep31AutoAdvance(transactionId: String) { + val response = + client.post { + url { + this.protocol = endpoint.protocol + host = endpoint.host + port = endpoint.port + encodedPath = "/sep31/transactions/$transactionId/skip-auto-advance" + } + } + if (!response.status.isSuccess()) { + throw IllegalStateException( + "Failed to register transaction($transactionId) to skip auto-advance: " + + "${response.status}" + ) + } + } } diff --git a/kotlin-reference-server/src/main/kotlin/org/stellar/reference/event/processor/Sep31EventProcessor.kt b/kotlin-reference-server/src/main/kotlin/org/stellar/reference/event/processor/Sep31EventProcessor.kt index 05d371d92e..18ef5b0e99 100644 --- a/kotlin-reference-server/src/main/kotlin/org/stellar/reference/event/processor/Sep31EventProcessor.kt +++ b/kotlin-reference-server/src/main/kotlin/org/stellar/reference/event/processor/Sep31EventProcessor.kt @@ -1,5 +1,6 @@ package org.stellar.reference.event.processor +import java.util.concurrent.ConcurrentHashMap import kotlinx.coroutines.runBlocking import org.stellar.anchor.api.callback.GetCustomerRequest import org.stellar.anchor.api.platform.* @@ -18,6 +19,17 @@ class Sep31EventProcessor( companion object { val requiredKyc = listOf("bank_account_number", "bank_account_type", "bank_number", "bank_branch_number") + + // Lets a test that drives a transaction through RPC calls itself (e.g. exercising an + // error/recovery path this processor doesn't know about) opt that one transaction out of + // automatic advancement, instead of disabling it for every transaction. Populated only via the + // test-only `/sep31/transactions/{id}/skip-auto-advance` route (see Sep31TestRoute.kt) -- kept + // out of transaction data so this can't be triggered by a real anchor's own message content. + private val manualRpcTestTransactions = ConcurrentHashMap.newKeySet() + + fun skipAutoAdvance(transactionId: String) { + manualRpcTestTransactions.add(transactionId) + } } override suspend fun onQuoteCreated(event: SendEventRequest) { @@ -55,6 +67,13 @@ class Sep31EventProcessor( override suspend fun onTransactionStatusChanged(event: SendEventRequest) { val transaction = event.payload.transaction!! + if (manualRpcTestTransactions.contains(transaction.id)) { + log.info { + "Transaction ${transaction.id} opts out of automatic advancement -- skipping reaction to" + + " status ${transaction.status}" + } + return + } when (val status = transaction.status) { PENDING_SENDER -> { log.info { "Transaction ${transaction.id} is in pending_sender status" } diff --git a/kotlin-reference-server/src/main/kotlin/org/stellar/reference/sep31/Sep31TestRoute.kt b/kotlin-reference-server/src/main/kotlin/org/stellar/reference/sep31/Sep31TestRoute.kt index 7333b56f7a..7e98ec6e0f 100644 --- a/kotlin-reference-server/src/main/kotlin/org/stellar/reference/sep31/Sep31TestRoute.kt +++ b/kotlin-reference-server/src/main/kotlin/org/stellar/reference/sep31/Sep31TestRoute.kt @@ -12,6 +12,7 @@ import kotlinx.coroutines.launch import org.stellar.reference.ClientException import org.stellar.reference.data.ErrorResponse import org.stellar.reference.data.Success +import org.stellar.reference.event.processor.Sep31EventProcessor import org.stellar.reference.service.sep31.ReceiveService private val log = KotlinLogging.logger {} @@ -41,4 +42,30 @@ fun Route.testSep31(receiveService: ReceiveService) { } } } + + // Lets a test that drives a SEP-31 transaction through RPC calls itself (e.g. exercising an + // error/recovery path Sep31EventProcessor doesn't know about) opt that one transaction out of + // the processor's automatic advancement, instead of disabling it for every transaction. + route("/sep31/transactions/{transactionId}/skip-auto-advance") { + post { + try { + val transactionId = + call.parameters["transactionId"] + ?: throw ClientException("Missing transactionId parameter") + + Sep31EventProcessor.skipAutoAdvance(transactionId) + + call.respond(Success(transactionId)) + } catch (e: ClientException) { + log.error { e } + call.respond(HttpStatusCode.BadRequest, ErrorResponse(e.message!!)) + } catch (e: Exception) { + log.error { e } + call.respond( + HttpStatusCode.InternalServerError, + ErrorResponse("Error occurred: ${e.message}"), + ) + } + } + } } diff --git a/lib-util/src/main/kotlin/org/stellar/anchor/client/SepClient.kt b/lib-util/src/main/kotlin/org/stellar/anchor/client/SepClient.kt index ec80b7f803..8bed3427bf 100644 --- a/lib-util/src/main/kotlin/org/stellar/anchor/client/SepClient.kt +++ b/lib-util/src/main/kotlin/org/stellar/anchor/client/SepClient.kt @@ -10,6 +10,7 @@ import org.apache.hc.core5.http.HttpStatus import org.stellar.anchor.api.exception.SepException import org.stellar.anchor.api.exception.SepNotAuthorizedException import org.stellar.anchor.api.exception.SepNotFoundException +import org.stellar.anchor.api.exception.SepValidationException import org.stellar.anchor.api.sep.SepExceptionResponse import org.stellar.anchor.util.GsonUtils @@ -71,6 +72,13 @@ open class SepClient { val sepException = gson.fromJson(responseBody, SepExceptionResponse::class.java) throw SepNotFoundException(sepException.error) } + HttpStatus.SC_BAD_REQUEST -> { + // 400 bodies vary in shape across SEPs (a plain "error" string, SEP-31's + // customer_info_needed {"type": ...}, transaction_info_needed {"error", "fields"}, etc.), + // so the raw body is preserved as-is rather than parsed and reduced to a single field -- + // that also sidesteps a null-body edge case a strict parse would need to guard against. + throw SepValidationException(responseBody) + } else -> throw SepException(responseBody) } } diff --git a/platform/src/main/java/org/stellar/anchor/platform/component/share/ObservingAccountsBeans.java b/platform/src/main/java/org/stellar/anchor/platform/component/share/ObservingAccountsBeans.java index 9217d07fe2..a79f178aca 100644 --- a/platform/src/main/java/org/stellar/anchor/platform/component/share/ObservingAccountsBeans.java +++ b/platform/src/main/java/org/stellar/anchor/platform/component/share/ObservingAccountsBeans.java @@ -20,10 +20,16 @@ public PaymentObservingAccountsManager paymentObservingAccountsManager( PaymentObservingAccountsManager bean = new PaymentObservingAccountsManager(paymentObservingAccountStore); - if (env.getProperty("sep31.enabled", Boolean.class, false)) { + if (shouldStartEvictionScheduler(env)) { bean.start(); } return bean; } + + static boolean shouldStartEvictionScheduler(Environment env) { + return env.getProperty("sep6.enabled", Boolean.class, false) + || env.getProperty("sep24.enabled", Boolean.class, false) + || env.getProperty("sep31.enabled", Boolean.class, false); + } } diff --git a/platform/src/main/java/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManager.java b/platform/src/main/java/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManager.java index da6ce5573b..455f5b3f04 100644 --- a/platform/src/main/java/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManager.java +++ b/platform/src/main/java/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManager.java @@ -60,7 +60,7 @@ public void evictAndPersist() { this.evict(getEvictMaxIdleTime()); Log.debug("Persisting accounts..."); for (ObservingAccount account : this.getAccounts()) { - store.upsert(account.account, account.lastObserved); + persistUpsert(account.account, account.lastObserved); } } @@ -83,21 +83,65 @@ public void upsert(String account, AccountType type) { */ public void upsert(ObservingAccount observingAccount) { if (observingAccount != null) { - ObservingAccount existingAccount = allAccounts.get(observingAccount.account); - if (existingAccount == null) { - allAccounts.put(observingAccount.account, observingAccount); - // update the database - store.upsert(observingAccount.account, observingAccount.lastObserved); - } else { - existingAccount.account = observingAccount.account; - existingAccount.lastObserved = observingAccount.lastObserved; - if (existingAccount.type == AccountType.TRANSIENT) { - existingAccount.type = observingAccount.type; - } + String canonicalAccount = safeCanonicalize(observingAccount.account); + ObservingAccount merged = + allAccounts.compute( + canonicalAccount, + (key, existing) -> { + if (existing == null) { + return new ObservingAccount( + canonicalAccount, observingAccount.lastObserved, observingAccount.type); + } + Instant lastObserved = + observingAccount.lastObserved.isAfter(existing.lastObserved) + ? observingAccount.lastObserved + : existing.lastObserved; + AccountType type = + existing.type == AccountType.TRANSIENT ? observingAccount.type : existing.type; + return new ObservingAccount(canonicalAccount, lastObserved, type); + }); + + boolean persisted = persistUpsert(merged.account, merged.lastObserved); + if (persisted && !canonicalAccount.equals(observingAccount.account)) { + persistDelete(observingAccount.account); } } } + private static String canonicalize(String account) { + if (account != null && account.startsWith("M")) { + return new MuxedAccount(account).getAccountId(); + } + return account; + } + + private static String safeCanonicalize(String account) { + try { + return canonicalize(account); + } catch (RuntimeException ex) { + Log.errorEx(String.format("Failed to canonicalize observing account %s", account), ex); + return account; + } + } + + private boolean persistUpsert(String account, Instant lastObserved) { + try { + store.upsert(account, lastObserved); + return true; + } catch (RuntimeException ex) { + Log.errorEx(String.format("Failed to persist observing account %s", account), ex); + return false; + } + } + + private void persistDelete(String account) { + try { + store.delete(account); + } catch (RuntimeException ex) { + Log.errorEx(String.format("Failed to delete stale observing account %s", account), ex); + } + } + /** * Gets the list of observed accounts. * @@ -119,17 +163,14 @@ public boolean lookupAndUpdate(String account) { return false; } - // MuxedAccount handles both muxed and non-muxed accounts - if (account.startsWith("M")) { - // If the account is a muxed account, we need to extract the G-account ID - MuxedAccount muxedAccount = new MuxedAccount(account); - account = muxedAccount.getAccountId(); - } + String canonicalAccount = canonicalize(account); - ObservingAccount acct = allAccounts.get(account); - if (acct == null) return false; - acct.lastObserved = Instant.now(); - return true; + ObservingAccount updated = + allAccounts.computeIfPresent( + canonicalAccount, + (key, existing) -> + new ObservingAccount(existing.account, Instant.now(), existing.type)); + return updated != null; } /** @@ -144,7 +185,7 @@ public void evict(Duration maxIdleTime) { Duration idleTime = Duration.between(Instant.now(), acct.lastObserved).abs(); if (idleTime.compareTo(maxIdleTime) > 0) { allAccounts.remove(acct.account); - store.delete(acct.account); + persistDelete(acct.account); } } } diff --git a/platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java b/platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java index 3022a17827..b664d76f74 100644 --- a/platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java +++ b/platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java @@ -346,12 +346,30 @@ protected void updateTransactionWithRpcRequest( } Log.infoF("Memo set to {} {}", txn31.getStellarMemoType(), txn31.getStellarMemo()); - - paymentObservingAccountsManager.upsert( - txn31.getToAccount(), PaymentObservingAccountsManager.AccountType.TRANSIENT); break; default: break; } } + + @Override + protected void afterTransactionSaved(JdbcSepTransaction txn, RequestOnchainFundsRequest request) + throws AnchorException { + String toAccount; + switch (Sep.from(txn.getProtocol())) { + case SEP_6: + toAccount = ((JdbcSep6Transaction) txn).getToAccount(); + break; + case SEP_24: + toAccount = ((JdbcSep24Transaction) txn).getToAccount(); + break; + case SEP_31: + toAccount = ((JdbcSep31Transaction) txn).getToAccount(); + break; + default: + return; + } + paymentObservingAccountsManager.upsert( + toAccount, PaymentObservingAccountsManager.AccountType.TRANSIENT); + } } diff --git a/platform/src/main/java/org/stellar/anchor/platform/rpc/RpcTransactionStatusHandler.java b/platform/src/main/java/org/stellar/anchor/platform/rpc/RpcTransactionStatusHandler.java index 4e1e163fca..02263448f9 100644 --- a/platform/src/main/java/org/stellar/anchor/platform/rpc/RpcTransactionStatusHandler.java +++ b/platform/src/main/java/org/stellar/anchor/platform/rpc/RpcTransactionStatusHandler.java @@ -132,6 +132,8 @@ protected abstract SepTransactionStatus getNextStatus(JdbcSepTransaction txn, T protected abstract void updateTransactionWithRpcRequest(JdbcSepTransaction txn, T request) throws AnchorException; + protected void afterTransactionSaved(JdbcSepTransaction txn, T request) throws AnchorException {} + protected JdbcSepTransaction getTransaction(String transactionId) throws AnchorException { Sep31Transaction txn31 = txn31Store.findByTransactionId(transactionId); if (txn31 != null) { @@ -216,6 +218,8 @@ protected void updateTransaction(JdbcSepTransaction txn, T request) throws Ancho break; } + afterTransactionSaved(txn, request); + updateMetrics(txn); } diff --git a/platform/src/test/kotlin/org/stellar/anchor/platform/component/share/ObservingAccountsBeansTest.kt b/platform/src/test/kotlin/org/stellar/anchor/platform/component/share/ObservingAccountsBeansTest.kt new file mode 100644 index 0000000000..3c4b231d81 --- /dev/null +++ b/platform/src/test/kotlin/org/stellar/anchor/platform/component/share/ObservingAccountsBeansTest.kt @@ -0,0 +1,38 @@ +package org.stellar.anchor.platform.component.share + +import io.mockk.every +import io.mockk.mockk +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.springframework.core.env.Environment + +class ObservingAccountsBeansTest { + private fun envWith(sep6: Boolean, sep24: Boolean, sep31: Boolean): Environment { + val env = mockk() + every { env.getProperty("sep6.enabled", Boolean::class.javaObjectType, false) } returns sep6 + every { env.getProperty("sep24.enabled", Boolean::class.javaObjectType, false) } returns sep24 + every { env.getProperty("sep31.enabled", Boolean::class.javaObjectType, false) } returns sep31 + return env + } + + @Test + fun `test scheduler is not started when no protocol using transient observation is enabled`() { + assertFalse(ObservingAccountsBeans.shouldStartEvictionScheduler(envWith(false, false, false))) + } + + @Test + fun `test scheduler is started when only sep6 is enabled`() { + assertTrue(ObservingAccountsBeans.shouldStartEvictionScheduler(envWith(true, false, false))) + } + + @Test + fun `test scheduler is started when only sep24 is enabled`() { + assertTrue(ObservingAccountsBeans.shouldStartEvictionScheduler(envWith(false, true, false))) + } + + @Test + fun `test scheduler is started when only sep31 is enabled`() { + assertTrue(ObservingAccountsBeans.shouldStartEvictionScheduler(envWith(false, false, true))) + } +} diff --git a/platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManagerTest.kt b/platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManagerTest.kt index 3d31238744..672f710656 100644 --- a/platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManagerTest.kt +++ b/platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/PaymentObservingAccountsManagerTest.kt @@ -7,6 +7,9 @@ import java.time.Duration import java.time.Instant import java.time.temporal.ChronoUnit.DAYS import java.time.temporal.ChronoUnit.HOURS +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit import org.junit.jupiter.api.* import org.junit.jupiter.api.Assertions.* import org.stellar.anchor.platform.data.PaymentObservingAccount @@ -21,8 +24,8 @@ class PaymentObservingAccountsManagerTest { private val testAcct2 = KeyPair.random().accountId private val testAcct3 = KeyPair.random().accountId private val testAcct4 = KeyPair.random().accountId - private val testMuxAcct100 = MuxedAccount(testAcct4, BigInteger("100")).accountId - private val testMuxAcct200 = MuxedAccount(testAcct4, BigInteger("100")).accountId + private val testMuxAcct100 = MuxedAccount(testAcct4, BigInteger("100")).address + private val testMuxAcct200 = MuxedAccount(testAcct4, BigInteger("200")).address @Test fun `test add and lookup`() { @@ -125,19 +128,164 @@ class PaymentObservingAccountsManagerTest { assertTrue(obs.lookupAndUpdate(testMuxAcct100)) assertTrue(obs.lookupAndUpdate(testMuxAcct200)) } + + @Test + fun `test registering by muxed account is found by the base account`() { + val obs = PaymentObservingAccountsManager(paymentObservingAccountStore) + obs.initialize() + + obs.upsert(testMuxAcct100, TRANSIENT) + assertEquals(1, obs.accounts.size) + + assertTrue(obs.lookupAndUpdate(testAcct4)) + assertTrue(obs.lookupAndUpdate(testMuxAcct100)) + assertTrue(obs.lookupAndUpdate(testMuxAcct200)) + } + + @Test + fun `test loading a legacy muxed row canonicalizes and deletes the stale row`() { + paymentObservingAccountStore.upsert(testMuxAcct100, Instant.now()) + assertEquals(1, paymentObservingAccountStore.list().size) + assertEquals(testMuxAcct100, paymentObservingAccountStore.list()[0].account) + + val obs = PaymentObservingAccountsManager(paymentObservingAccountStore) + obs.initialize() + + val persisted = paymentObservingAccountStore.list() + assertEquals(1, persisted.size) + assertEquals(testAcct4, persisted[0].account) + assertTrue(obs.lookupAndUpdate(testAcct4)) + } + + @Test + fun `test upsert does not throw when the store fails to persist`() { + val obs = PaymentObservingAccountsManager(ThrowingPaymentObservingAccountStore()) + obs.initialize() + + assertDoesNotThrow { obs.upsert(testAcct1, TRANSIENT) } + assertEquals(1, obs.accounts.size) + assertTrue(obs.lookupAndUpdate(testAcct1)) + + assertDoesNotThrow { obs.evict(Duration.ZERO) } + assertEquals(0, obs.accounts.size) + } + + @Test + fun `test legacy muxed row is not deleted when the canonical write fails`() { + val store = FlakyPaymentObservingAccountStore() + store.upsert(testMuxAcct100, Instant.now()) + assertEquals(1, store.list().size) + + store.failNextUpsert = true + val obs = PaymentObservingAccountsManager(store) + obs.initialize() + + val persisted = store.list() + assertEquals(1, persisted.size) + assertEquals(testMuxAcct100, persisted[0].account) + assertTrue(obs.lookupAndUpdate(testAcct4)) + } + + @Test + fun `test a stale duplicate does not clobber a newer lastObserved timestamp`() { + val obs = PaymentObservingAccountsManager(paymentObservingAccountStore) + obs.initialize() + + val newer = Instant.now() + val older = newer.minusSeconds(60) + + obs.upsert(PaymentObservingAccountsManager.ObservingAccount(testAcct4, newer, TRANSIENT)) + obs.upsert(PaymentObservingAccountsManager.ObservingAccount(testMuxAcct100, older, TRANSIENT)) + + assertEquals(1, obs.accounts.size) + assertEquals(newer, obs.accounts[0].lastObserved) + } + + @Test + fun `test concurrent upserts never lose the newest lastObserved under race`() { + val obs = PaymentObservingAccountsManager(NoopPaymentObservingAccountStore()) + obs.initialize() + + val threadCount = 32 + val perThread = 500 + val pool = Executors.newFixedThreadPool(threadCount) + val start = CountDownLatch(1) + val base = Instant.now() + + try { + repeat(threadCount) { threadIndex -> + pool.submit { + start.await() + for (i in 0 until perThread) { + val ts = base.plusNanos((threadIndex.toLong() * perThread + i) * 1000) + obs.upsert(PaymentObservingAccountsManager.ObservingAccount(testAcct1, ts, TRANSIENT)) + } + } + } + start.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(30, TimeUnit.SECONDS), "upsert pool did not finish in time") + } finally { + pool.shutdownNow() + } + + val expectedMax = base.plusNanos((threadCount.toLong() * perThread - 1) * 1000) + val finalAccount = obs.accounts.first { it.account == testAcct1 } + assertEquals(expectedMax, finalAccount.lastObserved) + } } class MemoryPaymentObservingAccountStore : PaymentObservingAccountStore(null) { private val accounts = mutableListOf() - override fun list(): List = accounts + public override fun list(): List = accounts - override fun upsert(account: String?, lastObserved: Instant?) { + public override fun upsert(account: String?, lastObserved: Instant?) { accounts.removeIf { it.account == account } accounts.add(PaymentObservingAccount(account, lastObserved)) } + public override fun delete(account: String) { + accounts.removeIf { it.account == account } + } +} + +class ThrowingPaymentObservingAccountStore : PaymentObservingAccountStore(null) { + override fun list(): List = emptyList() + + override fun upsert(account: String?, lastObserved: Instant?) { + throw RuntimeException("store unavailable") + } + override fun delete(account: String) { + throw RuntimeException("store unavailable") + } +} + +class NoopPaymentObservingAccountStore : PaymentObservingAccountStore(null) { + override fun list(): List = emptyList() + + override fun upsert(account: String?, lastObserved: Instant?) {} + + override fun delete(account: String) {} +} + +class FlakyPaymentObservingAccountStore : PaymentObservingAccountStore(null) { + private val accounts = mutableListOf() + var failNextUpsert = false + + public override fun list(): List = accounts + + public override fun upsert(account: String?, lastObserved: Instant?) { + if (failNextUpsert) { + failNextUpsert = false + throw RuntimeException("store unavailable") + } + accounts.removeIf { it.account == account } + accounts.add(PaymentObservingAccount(account, lastObserved)) + } + + public override fun delete(account: String) { accounts.removeIf { it.account == account } } } diff --git a/platform/src/test/kotlin/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandlerTest.kt b/platform/src/test/kotlin/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandlerTest.kt index 0a81393f03..a554286fe1 100644 --- a/platform/src/test/kotlin/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandlerTest.kt +++ b/platform/src/test/kotlin/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandlerTest.kt @@ -611,6 +611,16 @@ class RequestOnchainFundsHandlerTest { verify(exactly = 0) { txn6Store.save(any()) } verify(exactly = 0) { txn31Store.save(any()) } verify(exactly = 1) { sepTransactionCounter.increment() } + verify(exactly = 1) { + paymentObservingAccountsManager.upsert( + DESTINATION_ACCOUNT, + PaymentObservingAccountsManager.AccountType.TRANSIENT, + ) + } + verifyOrder { + txn24Store.save(any()) + paymentObservingAccountsManager.upsert(any(), any()) + } val expectedSep24Txn = JdbcSep24Transaction() expectedSep24Txn.kind = WITHDRAWAL.kind @@ -1329,6 +1339,16 @@ class RequestOnchainFundsHandlerTest { verify(exactly = 0) { txn24Store.save(any()) } verify(exactly = 0) { txn31Store.save(any()) } verify(exactly = 1) { sepTransactionCounter.increment() } + verify(exactly = 1) { + paymentObservingAccountsManager.upsert( + DESTINATION_ACCOUNT, + PaymentObservingAccountsManager.AccountType.TRANSIENT, + ) + } + verifyOrder { + txn6Store.save(any()) + paymentObservingAccountsManager.upsert(any(), any()) + } val expectedSep6Txn = JdbcSep6Transaction() expectedSep6Txn.kind = kind @@ -1736,6 +1756,16 @@ class RequestOnchainFundsHandlerTest { verify(exactly = 0) { txn6Store.save(any()) } verify(exactly = 0) { txn24Store.save(any()) } verify(exactly = 1) { sepTransactionCounter.increment() } + verify(exactly = 1) { + paymentObservingAccountsManager.upsert( + DESTINATION_ACCOUNT, + PaymentObservingAccountsManager.AccountType.TRANSIENT, + ) + } + verifyOrder { + txn31Store.save(any()) + paymentObservingAccountsManager.upsert(any(), any()) + } val expectedSep31Txn = JdbcSep31Transaction() expectedSep31Txn.status = PENDING_SENDER.toString() diff --git a/service-runner/src/main/resources/config/assets.yaml b/service-runner/src/main/resources/config/assets.yaml index bede3a049a..d99a06bf86 100644 --- a/service-runner/src/main/resources/config/assets.yaml +++ b/service-runner/src/main/resources/config/assets.yaml @@ -79,6 +79,23 @@ items: - SWIFT quotes_supported: true quotes_required: false + # optional: false is enforced by Sep31Service#validateRequiredFields -- a POST /transactions + # request missing this field is rejected with 400. The essential-tests fixture always + # supplies it (see Sep31Tests.kt), so this also exercises that enforcement end-to-end. + fields: + transaction: + receiver_account_number: + description: Bank account number of the receiver. + optional: false + receiver_routing_number: + description: Bank routing number of the receiver. + optional: true + type: + description: Type of the receiver's bank transfer. + optional: true + choices: + - SEPA + - SWIFT sep38: enabled: true exchangeable_assets: @@ -137,6 +154,27 @@ items: exchangeable_assets: - iso4217:USD + # SEP-31-only asset dedicated to exercising quotes_required: true -- kept separate from + # USDC/JPYC above (both quotes_required: false) so this doesn't change their behavior for any + # other test. The reference rate server already has a hardcoded price for this pair (SRT/USD). + - id: stellar:SRT:GCDNJUBQSX7AJWLJACMJ7I4BC3Z47BQUTMHEICZLE6MU4KQBRYG5JY6B + distribution_account: GBN4NNCDGJO4XW4KQU3CBIESUJWFVBUZPOKUZHT7W7WRB7CWOA7BXVQF + significant_decimals: 4 + sep31: + enabled: true + receive: + min_amount: 0 + max_amount: 1000000 + methods: + - SEPA + - SWIFT + quotes_supported: true + quotes_required: true + sep38: + enabled: true + exchangeable_assets: + - iso4217:USD + # Native asset - id: stellar:native distribution_account: GBN4NNCDGJO4XW4KQU3CBIESUJWFVBUZPOKUZHT7W7WRB7CWOA7BXVQF diff --git a/service-runner/src/main/resources/version-info.properties b/service-runner/src/main/resources/version-info.properties index 5342eb3c73..e0da18dcc7 100644 --- a/service-runner/src/main/resources/version-info.properties +++ b/service-runner/src/main/resources/version-info.properties @@ -1 +1 @@ -version=4.7.1 +version=4.8.0