Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ANCHOR-784] Remove 0 amounts from SEP-6 transaction responses #1469

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import org.apache.commons.lang3.StringUtils;

@Data
@Builder
Expand All @@ -15,7 +16,7 @@ public class Amount {
public Amount() {}

public static @Nullable Amount create(String amount, String asset) {
if (amount == null) {
if (amount == null && StringUtils.isEmpty(asset)) {
return null;
}

Expand Down
11 changes: 0 additions & 11 deletions core/src/main/java/org/stellar/anchor/sep6/Sep6Service.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.stellar.anchor.api.sep.SepTransactionStatus;
import org.stellar.anchor.api.sep.sep6.*;
import org.stellar.anchor.api.sep.sep6.InfoResponse.*;
import org.stellar.anchor.api.shared.FeeDetails;
import org.stellar.anchor.asset.AssetService;
import org.stellar.anchor.auth.Sep10Jwt;
import org.stellar.anchor.client.ClientFinder;
Expand Down Expand Up @@ -199,16 +198,11 @@ public StartDepositResponse depositExchange(Sep10Jwt token, StartDepositExchange
exchangeAmountsCalculator.calculateFromQuote(
request.getQuoteId(), sellAsset, request.getAmount());
} else {
// TODO(philip): remove this
// If a quote is not provided, set the fee and out amounts to 0.
// The business server should use the request_offchain_funds RPC to update the amounts.
amounts =
Amounts.builder()
.amountIn(request.getAmount())
.amountInAsset(sellAsset.getSep38AssetName())
.amountOut("0")
.amountOutAsset(buyAsset.getSep38AssetName())
.feeDetails(new FeeDetails("0", sellAsset.getSep38AssetName(), null))
.build();
}

Expand Down Expand Up @@ -373,16 +367,11 @@ public StartWithdrawResponse withdrawExchange(
exchangeAmountsCalculator.calculateFromQuote(
request.getQuoteId(), sellAsset, request.getAmount());
} else {
// TODO(philip): remove this
// If a quote is not provided, set the fee and out amounts to 0.
// The business server should use the request_onchain_funds RPC to update the amounts.
amounts =
Amounts.builder()
.amountIn(request.getAmount())
.amountInAsset(sellAsset.getSep38AssetName())
.amountOut("0")
.amountOutAsset(buyAsset.getSep38AssetName())
.feeDetails(new FeeDetails("0", sellAsset.getSep38AssetName(), null))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ public class PojoSep6Transaction implements Sep6Transaction {
List<FeeDescription> feeDetailsList;

public void setFeeDetails(FeeDetails feeDetails) {
setAmountFee(feeDetails.getTotal());
setAmountFeeAsset(feeDetails.getAsset());
setFeeDetailsList(feeDetails.getDetails());
if (feeDetails != null) {
setAmountFee(feeDetails.getTotal());
setAmountFeeAsset(feeDetails.getAsset());
setFeeDetailsList(feeDetails.getDetails());
}
}

public FeeDetails getFeeDetails() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,7 @@ class Sep6ServiceTestData {
"requestAssetIssuer": "GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"amountIn": "100",
"amountInAsset": "iso4217:USD",
"amountOut": "0",
"amountOutAsset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"amountFee": "0",
"amountFeeAsset": "iso4217:USD",
"amountExpected": "100",
"sep10Account": "GBLGJA4TUN5XOGTV6WO2BWYUI2OZR5GYQ5PDPCRMQ5XEPJOYWB2X4CJO",
"sep10AccountMemo": "123",
Expand All @@ -363,10 +360,8 @@ class Sep6ServiceTestData {
},
"amount_in": { "amount": "100", "asset": "iso4217:USD" },
"amount_out": {
"amount": "0",
"asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP"
},
"amount_fee": { "amount": "0", "asset": "iso4217:USD" },
"destination_account": "GBLGJA4TUN5XOGTV6WO2BWYUI2OZR5GYQ5PDPCRMQ5XEPJOYWB2X4CJO",
"client_domain": "vibrant.stellar.org",
"client_name": "vibrant",
Expand Down Expand Up @@ -579,10 +574,7 @@ class Sep6ServiceTestData {
"requestAssetIssuer": "GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"amountIn": "100",
"amountInAsset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"amountOut": "0",
"amountOutAsset": "iso4217:USD",
"amountFee": "0",
"amountFeeAsset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"amountExpected": "100",
"sep10Account": "GBLGJA4TUN5XOGTV6WO2BWYUI2OZR5GYQ5PDPCRMQ5XEPJOYWB2X4CJO",
"sep10AccountMemo": "123",
Expand Down Expand Up @@ -613,11 +605,7 @@ class Sep6ServiceTestData {
"amount": "100",
"asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP"
},
"amount_out": { "amount": "0", "asset": "iso4217:USD" },
"amount_fee": {
"amount": "0",
"asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP"
},
"amount_out": { "asset": "iso4217:USD" },
"source_account": "GBLGJA4TUN5XOGTV6WO2BWYUI2OZR5GYQ5PDPCRMQ5XEPJOYWB2X4CJO",
"refund_memo": "some text",
"refund_memo_type": "text",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,7 @@ class Sep6Tests : AbstractIntegrationTests(TestConfig()) {
"status": "incomplete",
"amount_in": "1",
"amount_in_asset": "iso4217:USD",
"amount_out": "0",
"amount_out_asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"amount_fee": "0",
"amount_fee_asset": "iso4217:USD",
"to": "GDJLBYYKMCXNVVNABOE66NYXQGIA5AC5D223Z2KF6ZEYK4UBCA7FKLTG"
}
}
Expand Down Expand Up @@ -299,10 +296,7 @@ class Sep6Tests : AbstractIntegrationTests(TestConfig()) {
"status": "incomplete",
"amount_in": "1",
"amount_in_asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"amount_out": "0",
"amount_out_asset": "iso4217:USD",
"amount_fee": "0",
"amount_fee_asset": "stellar:USDC:GDQOE23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP",
"from": "GDJLBYYKMCXNVVNABOE66NYXQGIA5AC5D223Z2KF6ZEYK4UBCA7FKLTG"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ public abstract class JdbcSepTransaction {
public abstract String getProtocol();

public void setFeeDetails(FeeDetails feeDetails) {
setAmountFee(feeDetails.getTotal());
setAmountFeeAsset(feeDetails.getAsset());
setFeeDetailsList(feeDetails.getDetails());
if (feeDetails != null) {
setAmountFee(feeDetails.getTotal());
setAmountFeeAsset(feeDetails.getAsset());
setFeeDetailsList(feeDetails.getDetails());
}
}

public FeeDetails getFeeDetails() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ class DoStellarPaymentHandlerTest {
expectedResponse.kind = DEPOSIT
expectedResponse.status = PENDING_STELLAR
expectedResponse.amountExpected = Amount(null, "")
expectedResponse.amountOut = Amount(null, AMOUNT_OUT_ASSET)
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
expectedResponse.destinationAccount = TO_ACCOUNT

Expand Down Expand Up @@ -365,6 +366,7 @@ class DoStellarPaymentHandlerTest {
expectedResponse.kind = DEPOSIT
expectedResponse.status = PENDING_TRUST
expectedResponse.amountExpected = Amount(null, "")
expectedResponse.amountOut = Amount(null, AMOUNT_OUT_ASSET)
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
expectedResponse.destinationAccount = TO_ACCOUNT

Expand Down Expand Up @@ -536,6 +538,7 @@ class DoStellarPaymentHandlerTest {
expectedResponse.kind = PlatformTransactionData.Kind.from(kind)
expectedResponse.status = PENDING_STELLAR
expectedResponse.amountExpected = Amount(null, "")
expectedResponse.amountOut = Amount(null, AMOUNT_OUT_ASSET)
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
expectedResponse.transferReceivedAt = transferReceivedAt
expectedResponse.destinationAccount = TO_ACCOUNT
Expand Down Expand Up @@ -623,6 +626,7 @@ class DoStellarPaymentHandlerTest {
expectedResponse.kind = PlatformTransactionData.Kind.from(kind)
expectedResponse.status = PENDING_TRUST
expectedResponse.amountExpected = Amount(null, "")
expectedResponse.amountOut = Amount(null, AMOUNT_OUT_ASSET)
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
expectedResponse.transferReceivedAt = transferReceivedAt
expectedResponse.destinationAccount = TO_ACCOUNT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ class NotifyAmountsUpdatedHandlerTest {
expectedResponse.kind = WITHDRAWAL
expectedResponse.status = PENDING_ANCHOR
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount("0.9", STELLAR_USDC)
expectedResponse.amountFee = Amount("0.1", STELLAR_USDC)
expectedResponse.feeDetails = Amount("0.1", STELLAR_USDC).toRate()
Expand Down Expand Up @@ -438,6 +439,7 @@ class NotifyAmountsUpdatedHandlerTest {
expectedResponse.kind = PlatformTransactionData.Kind.from(kind)
expectedResponse.status = PENDING_ANCHOR
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount("0.9", STELLAR_USDC)
expectedResponse.amountFee = Amount("0.1", STELLAR_USDC)
expectedResponse.feeDetails = Amount("0.1", STELLAR_USDC).toRate()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,8 +447,10 @@ class NotifyOffchainFundsReceivedHandlerTest {
expectedResponse.kind = DEPOSIT
expectedResponse.status = PENDING_ANCHOR
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
expectedResponse.amountIn = Amount("1", FIAT_USD)
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("1", FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)

JSONAssert.assertEquals(
gson.toJson(expectedResponse),
Expand Down Expand Up @@ -535,6 +537,9 @@ class NotifyOffchainFundsReceivedHandlerTest {
expectedResponse.externalTransactionId = EXTERNAL_TX_ID
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)

JSONAssert.assertEquals(
gson.toJson(expectedResponse),
Expand Down Expand Up @@ -607,6 +612,9 @@ class NotifyOffchainFundsReceivedHandlerTest {
expectedResponse.status = PENDING_ANCHOR
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)

JSONAssert.assertEquals(
gson.toJson(expectedResponse),
Expand Down Expand Up @@ -835,6 +843,8 @@ class NotifyOffchainFundsReceivedHandlerTest {
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
expectedResponse.transferReceivedAt = sep6TxnCapture.captured.transferReceivedAt
expectedResponse.amountIn = Amount("1", FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.customers = Customers(StellarId(null, null, null), StellarId(null, null, null))

Expand Down Expand Up @@ -927,6 +937,9 @@ class NotifyOffchainFundsReceivedHandlerTest {
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
expectedResponse.transferReceivedAt = sep6TxnCapture.captured.transferReceivedAt
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)
expectedResponse.customers = Customers(StellarId(null, null, null), StellarId(null, null, null))

JSONAssert.assertEquals(
Expand Down Expand Up @@ -1002,6 +1015,9 @@ class NotifyOffchainFundsReceivedHandlerTest {
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
expectedResponse.transferReceivedAt = sep6TxnCapture.captured.transferReceivedAt
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)
expectedResponse.customers = Customers(StellarId(null, null, null), StellarId(null, null, null))

JSONAssert.assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,10 @@ class NotifyOnchainFundsReceivedHandlerTest {
expectedResponse.kind = WITHDRAWAL
expectedResponse.status = PENDING_ANCHOR
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
expectedResponse.amountIn = Amount("1", FIAT_USD)
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("1", FIAT_USD)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong, we default to the request asset code when the assets are not found in the transaction.

AssetInfo info = service.getAsset(txn.getRequestAssetCode(), txn.getRequestAssetIssuer());

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is possible that the transaction asset had not been written by RPC and we need to respond with the asset id.

Same logic exists at:

AssetInfo info = service.getAsset(txn.getRequestAssetCode(), txn.getRequestAssetIssuer());

expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)
expectedResponse.stellarTransactions = stellarTransactions

JSONAssert.assertEquals(
Expand Down Expand Up @@ -528,6 +530,9 @@ class NotifyOnchainFundsReceivedHandlerTest {
expectedResponse.status = PENDING_ANCHOR
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)
expectedResponse.stellarTransactions = stellarTransactions

JSONAssert.assertEquals(
Expand Down Expand Up @@ -891,8 +896,10 @@ class NotifyOnchainFundsReceivedHandlerTest {
expectedResponse.status = PENDING_ANCHOR
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
expectedResponse.transferReceivedAt = Instant.parse(STELLAR_PAYMENT_DATE)
expectedResponse.amountIn = Amount("1", FIAT_USD)
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("1", FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)
expectedResponse.stellarTransactions = stellarTransactions
expectedResponse.customers = Customers(StellarId(null, null, null), StellarId(null, null, null))

Expand Down Expand Up @@ -983,6 +990,9 @@ class NotifyOnchainFundsReceivedHandlerTest {
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
expectedResponse.transferReceivedAt = Instant.parse(STELLAR_PAYMENT_DATE)
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount(null, FIAT_USD)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount(null, FIAT_USD)
expectedResponse.stellarTransactions = stellarTransactions
expectedResponse.customers = Customers(StellarId(null, null, null), StellarId(null, null, null))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ class NotifyRefundPendingHandlerTest {
expectedResponse.status = PENDING_EXTERNAL
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("1", STELLAR_USDC)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount("0.1", FIAT_USD)
expectedResponse.feeDetails = Amount("0.1", FIAT_USD).toRate()
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
Expand Down Expand Up @@ -462,6 +463,7 @@ class NotifyRefundPendingHandlerTest {
expectedResponse.status = PENDING_EXTERNAL
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("2.2", STELLAR_USDC)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount("0.1", FIAT_USD)
expectedResponse.feeDetails = Amount("0.1", FIAT_USD).toRate()
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
Expand Down Expand Up @@ -555,6 +557,7 @@ class NotifyRefundPendingHandlerTest {
expectedResponse.status = PENDING_ANCHOR
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("1", STELLAR_USDC)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount("0.1", FIAT_USD)
expectedResponse.feeDetails = Amount("0.1", FIAT_USD).toRate()
expectedResponse.updatedAt = sep24TxnCapture.captured.updatedAt
Expand Down Expand Up @@ -747,6 +750,7 @@ class NotifyRefundPendingHandlerTest {
expectedResponse.status = PENDING_EXTERNAL
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("1", STELLAR_USDC)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount("0.1", FIAT_USD)
expectedResponse.feeDetails = Amount("0.1", FIAT_USD).toRate()
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
Expand Down Expand Up @@ -868,6 +872,7 @@ class NotifyRefundPendingHandlerTest {
expectedResponse.status = PENDING_EXTERNAL
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("2.2", STELLAR_USDC)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount("0.1", FIAT_USD)
expectedResponse.feeDetails = Amount("0.1", FIAT_USD).toRate()
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
Expand Down Expand Up @@ -973,6 +978,7 @@ class NotifyRefundPendingHandlerTest {
expectedResponse.status = PENDING_ANCHOR
expectedResponse.amountExpected = Amount(null, FIAT_USD)
expectedResponse.amountIn = Amount("1", STELLAR_USDC)
expectedResponse.amountOut = Amount(null, FIAT_USD)
expectedResponse.amountFee = Amount("0.1", FIAT_USD)
expectedResponse.feeDetails = Amount("0.1", FIAT_USD).toRate()
expectedResponse.updatedAt = sep6TxnCapture.captured.updatedAt
Expand Down
Loading
Loading