Skip to content

Commit

Permalink
Fix 529: Make all AMM transaction IssuedCurrencyAmount fields Currenc…
Browse files Browse the repository at this point in the history 
…yAmounts (#534)

make all AMM transaction IssuedCurrencyAmount fields CurrencyAmounts

Co-authored-by: David Fuelling <[email protected]>
  • Loading branch information
@nkramer44 @sappenin
nkramer44 and sappenin authored Jun 11, 2024
1 parent 2a68c4a commit 55975e1
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 9 deletions.
24 changes: 20 additions & 4 deletions xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/AmmBid.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,35 @@ default TransactionFlags flags() {
* Pay at least this amount for the slot. Setting this value higher makes it harder for others to outbid you. If
* omitted, pay the minimum necessary to win the bid.
*
* @return An optionally present {@link IssuedCurrencyAmount}.
* <p>
* In a well-formed transaction, this field is always an {@link IssuedCurrencyAmount}. However, the XRPL will fail AMM
* transactions that specify {@link XrpCurrencyAmount}s with a {@code tec} error code, which means these malformed
* transactions can be included in validated ledgers. Therefore, this field is typed as a {@link CurrencyAmount} so
* that malformed transactions can be correctly deserialized. See <a
* href="https://github.com/XRPLF/xrpl4j/issues/529">#529</a>
* </p>
*
* @return An optionally present {@link CurrencyAmount}.
*/
@JsonProperty("BidMin")
Optional<IssuedCurrencyAmount> bidMin();
Optional<CurrencyAmount> bidMin();

/**
* Pay at most this amount for the slot. If the cost to win the bid is higher than this amount, the transaction fails.
* If omitted, pay as much as necessary to win the bid.
*
* @return An optionally present {@link IssuedCurrencyAmount}.
* <p>
* In a well-formed transaction, this field is always an {@link IssuedCurrencyAmount}. However, the XRPL will fail AMM
* transactions that specify {@link XrpCurrencyAmount}s with a {@code tec} error code, which means these malformed
* transactions can be included in validated ledgers. Therefore, this field is typed as a {@link CurrencyAmount} so
* that malformed transactions can be correctly deserialized. See <a
* href="https://github.com/XRPLF/xrpl4j/issues/529">#529</a>
* </p>
*
* @return An optionally present {@link CurrencyAmount}.
*/
@JsonProperty("BidMax")
Optional<IssuedCurrencyAmount> bidMax();
Optional<CurrencyAmount> bidMax();

/**
* A list of up to 4 additional accounts that you allow to trade at the discounted fee. This cannot include the
Expand Down
12 changes: 10 additions & 2 deletions xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/AmmDeposit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,18 @@ static ImmutableAmmDeposit.Builder builder() {
/**
* How many of the AMM's LP Tokens to buy.
*
* @return An optionally present {@link IssuedCurrencyAmount}.
* <p>
* In a well-formed transaction, this field is always an {@link IssuedCurrencyAmount}. However, the XRPL will fail AMM
* transactions that specify {@link XrpCurrencyAmount}s with a {@code tec} error code, which means these malformed
* transactions can be included in validated ledgers. Therefore, this field is typed as a {@link CurrencyAmount} so
* that malformed transactions can be correctly deserialized. See <a
* href="https://github.com/XRPLF/xrpl4j/issues/529">#529</a>
* </p>
*
* @return An optionally present {@link CurrencyAmount}.
*/
@JsonProperty("LPTokenOut")
Optional<IssuedCurrencyAmount> lpTokenOut();
Optional<CurrencyAmount> lpTokenOut();

/**
* An optional {@link TradingFee} to set on the AMM instance. This field is only honored if the AMM's LP token balance
Expand Down
10 changes: 9 additions & 1 deletion xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/AmmWithdraw.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,17 @@ static ImmutableAmmWithdraw.Builder builder() {
/**
* How many of the AMM's LP Tokens to buy.
*
* <p>
* In a well-formed transaction, this field is always an {@link IssuedCurrencyAmount}. However, the XRPL will fail AMM
* transactions that specify {@link XrpCurrencyAmount}s with a {@code tec} error code, which means these malformed
* transactions can be included in validated ledgers. Therefore, this field is typed as a {@link CurrencyAmount} so
* that malformed transactions can be correctly deserialized. See <a
* href="https://github.com/XRPLF/xrpl4j/issues/529">#529</a>
* </p>
*
* @return An optionally present {@link IssuedCurrencyAmount}.
*/
@JsonProperty("LPTokensIn")
Optional<IssuedCurrencyAmount> lpTokensIn();
Optional<CurrencyAmount> lpTokensIn();

}
63 changes: 63 additions & 0 deletions xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/AmmBidTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.json.JSONException;
import org.junit.jupiter.api.Test;
import org.xrpl.xrpl4j.crypto.keys.PublicKey;
import org.xrpl.xrpl4j.crypto.signing.Signature;
import org.xrpl.xrpl4j.model.AbstractJsonTest;
import org.xrpl.xrpl4j.model.flags.TransactionFlags;
import org.xrpl.xrpl4j.model.ledger.AuthAccount;
Expand Down Expand Up @@ -249,4 +250,66 @@ void testJsonWithMinAndMax() throws JSONException, JsonProcessingException {

assertCanSerializeAndDeserialize(bid, json);
}

/**
* Test that ensures the problematic transaction found in <a
* href="https://github.com/XRPLF/xrpl4j/issues/529">#529</a> is deserializable.
*/
@Test
void testJsonWithXrpAmountBidMinAndMax() throws JSONException, JsonProcessingException {
AmmBid ammBid = AmmBid.builder()
.account(Address.of("rammersz4CroiyvbkzeZN1sBDCK9P8DvxF"))
.asset(Issue.XRP)
.asset2(
Issue.builder()
.issuer(Address.of("rswh1fvyLqHizBS2awu1vs6QcmwTBd9qiv"))
.currency("XAH")
.build()
)
.addAuthAccounts(
AuthAccountWrapper.of(
AuthAccount.of(Address.of("rapido5rxPmP4YkMZZEeXSHqWefxHEkqv6"))
)
)
.bidMax(XrpCurrencyAmount.ofDrops(10))
.bidMin(XrpCurrencyAmount.ofDrops(10))
.fee(XrpCurrencyAmount.ofDrops(10))
.flags(TransactionFlags.FULLY_CANONICAL_SIG)
.sequence(UnsignedInteger.valueOf(87704195))
.signingPublicKey(
PublicKey.fromBase16EncodedPublicKey("ED2D15BC6B61D6520011E4C794C5B320E584106154D0865BB095D70DA9A2A57B57")
)
.transactionSignature(
Signature.fromBase16("F652BD5369F6EE9A8A1490BD37B8240CEE2B4B6EF94D22EC2DBB6912AA729B829" +
"FC3D7E24B30A1E6CC11F868CE229B105398719152B9BEE8992A56D654F79C0A")
)
.build();
String json = "{\n" +
" \"Account\": \"rammersz4CroiyvbkzeZN1sBDCK9P8DvxF\",\n" +
" \"Asset\": {\n" +
" \"currency\": \"XRP\"\n" +
" },\n" +
" \"Asset2\": {\n" +
" \"currency\": \"XAH\",\n" +
" \"issuer\": \"rswh1fvyLqHizBS2awu1vs6QcmwTBd9qiv\"\n" +
" },\n" +
" \"AuthAccounts\": [\n" +
" {\n" +
" \"AuthAccount\": {\n" +
" \"Account\": \"rapido5rxPmP4YkMZZEeXSHqWefxHEkqv6\"\n" +
" }\n" +
" }\n" +
" ],\n" +
" \"BidMax\": \"10\",\n" +
" \"BidMin\": \"10\",\n" +
" \"Fee\": \"10\",\n" +
" \"Flags\": 2147483648,\n" +
" \"Sequence\": 87704195,\n" +
" \"SigningPubKey\": \"ED2D15BC6B61D6520011E4C794C5B320E584106154D0865BB095D70DA9A2A57B57\",\n" +
" \"TransactionType\": \"AMMBid\",\n" +
" \"TxnSignature\": \"F652BD5369F6EE9A8A1490BD37B8240CEE2B4B6EF94D22EC2DBB6912AA729B829FC3D7E24B30A" +
"1E6CC11F868CE229B105398719152B9BEE8992A56D654F79C0A\"\n" +
"}";
assertCanSerializeAndDeserialize(ammBid, json);
}
}
41 changes: 41 additions & 0 deletions xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/AmmDepositTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,47 @@ void constructLpTokenDepositAndTestJson() throws JSONException, JsonProcessingEx
assertCanSerializeAndDeserialize(deposit, json);
}

@Test
void constructLpTokenDepositWithXrpLpTokenAmountAndTestJson() throws JSONException, JsonProcessingException {
AmmDeposit deposit = AmmDeposit.builder()
.account(Address.of("rJVUeRqDFNs2xqA7ncVE6ZoAhPUoaJJSQm"))
.fee(XrpCurrencyAmount.ofDrops(10))
.signingPublicKey(
PublicKey.fromBase16EncodedPublicKey("02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC")
)
.flags(AmmDepositFlags.LP_TOKEN)
.asset(Issue.XRP)
.asset2(
Issue.builder()
.issuer(Address.of("rP9jPyP5kyvFRb6ZiRghAGw5u8SGAmU4bd"))
.currency("TST")
.build()
)
.lpTokenOut(XrpCurrencyAmount.ofDrops(10))
.build();

assertThat(deposit.flags()).isEqualTo(AmmDepositFlags.LP_TOKEN);

String json = "{\n" +
" \"Account\" : \"" + deposit.account() + "\",\n" +
" \"LPTokenOut\" : \"10\",\n" +
" \"Asset2\" : {\n" +
" \"currency\" : \"TST\",\n" +
" \"issuer\" : \"rP9jPyP5kyvFRb6ZiRghAGw5u8SGAmU4bd\"\n" +
" },\n" +
" \"Asset\" : {\n" +
" \"currency\" : \"XRP\"\n" +
" },\n" +
" \"Fee\" : \"10\",\n" +
" \"Flags\" : " + AmmDepositFlags.LP_TOKEN + ",\n" +
" \"Sequence\" : 0,\n" +
" \"SigningPubKey\" : \"02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC\",\n" +
" \"TransactionType\" : \"AMMDeposit\"\n" +
"}";

assertCanSerializeAndDeserialize(deposit, json);
}

@Test
void constructTwoAssetDepositAndTestJson() throws JSONException, JsonProcessingException {
AmmDeposit deposit = AmmDeposit.builder()
Expand Down
22 changes: 22 additions & 0 deletions xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/AmmWithdrawTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,28 @@ void constructLpTokenWithdrawAndTestJson() throws JSONException, JsonProcessingE
assertCanSerializeAndDeserialize(withdraw, json);
}

@Test
void constructLpTokenWithdrawWithXrpCurrencyAmountAndTestJson() throws JSONException, JsonProcessingException {
AmmWithdraw withdraw = baseBuilder()
.flags(AmmWithdrawFlags.LP_TOKEN)
.lpTokensIn(XrpCurrencyAmount.ofDrops(10))
.build();

String json = "{\n" +
" \"Account\" : \"rJVUeRqDFNs2xqA7ncVE6ZoAhPUoaJJSQm\",\n" +
" \"LPTokensIn\" : \"10\"," +
" \"Asset\" : " + objectMapper.writeValueAsString(withdraw.asset()) + "," +
" \"Asset2\" : " + objectMapper.writeValueAsString(withdraw.asset2()) + "," +
" \"Fee\" : \"10\",\n" +
" \"Flags\" : " + AmmWithdrawFlags.LP_TOKEN + ",\n" +
" \"Sequence\" : 0,\n" +
" \"SigningPubKey\" : \"02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC\",\n" +
" \"TransactionType\" : \"AMMWithdraw\"\n" +
"}";

assertCanSerializeAndDeserialize(withdraw, json);
}

@Test
void constructWithdrawAllAndTestJson() throws JSONException, JsonProcessingException {
AmmWithdraw withdraw = baseBuilder()
Expand Down
36 changes: 34 additions & 2 deletions xrpl4j-integration-tests/src/test/java/org/xrpl/xrpl4j/tests/LedgerResultIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -25,9 +25,17 @@

import org.junit.jupiter.api.Test;
import org.xrpl.xrpl4j.client.JsonRpcClientErrorException;
import org.xrpl.xrpl4j.client.XrplClient;
import org.xrpl.xrpl4j.model.client.common.LedgerSpecifier;
import org.xrpl.xrpl4j.model.client.ledger.LedgerRequestParams;
import org.xrpl.xrpl4j.model.client.ledger.LedgerResult;
import org.xrpl.xrpl4j.model.client.transactions.TransactionResult;
import org.xrpl.xrpl4j.model.transactions.AmmBid;
import org.xrpl.xrpl4j.model.transactions.Hash256;
import org.xrpl.xrpl4j.model.transactions.Transaction;
import org.xrpl.xrpl4j.tests.environment.XrplEnvironment;

import java.util.Optional;

/**
* These tests ensure {@link LedgerResult}s can be constructed from any JSON responses rippled sends back.
Expand Down Expand Up @@ -88,4 +96,28 @@ void getClosedLedgerResult() throws JsonRpcClientErrorException {
assertThat(ledgerResult.ledger().closeTimeHuman()).isNotEmpty();
assertThat(ledgerResult.ledger().parentCloseTime()).isNotEmpty();
}

/**
* Pulls down ledger BAA4AF508E9A9CB7685D9E61B82486681E52E9B920904B0650499497F050D8FA, which had an
* {@link org.xrpl.xrpl4j.model.transactions.AmmBid} transaction with an
* {@link org.xrpl.xrpl4j.model.transactions.XrpCurrencyAmount} in the {@link AmmBid#bidMax()} field.
*/
@Test
void canDeserializeLedger_Issue529() throws JsonRpcClientErrorException {
XrplClient mainnetClient = XrplEnvironment.getConfiguredMainnetEnvironment().getXrplClient();
LedgerResult ledger = mainnetClient.ledger(
LedgerRequestParams.builder()
.transactions(true)
.ledgerSpecifier(
LedgerSpecifier.of(Hash256.of("BAA4AF508E9A9CB7685D9E61B82486681E52E9B920904B0650499497F050D8FA")))
.build()
);

Optional<TransactionResult<? extends Transaction>> problematicTransaction = ledger.ledger().transactions().stream()
.filter(transaction -> transaction.hash()
.equals(Hash256.of("6A8BC948E1309219EA8E14905D0EA9BB94E24429DE5B15CDE8916CDBCE42034B")))
.findFirst();

assertThat(problematicTransaction).isNotEmpty();
}
}

0 comments on commit 55975e1

Please sign in to comment.