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

Add support for unknown transactions #569

Open
wants to merge 6 commits into
base: main
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 @@ -49,6 +49,10 @@ public class TransactionFlags extends Flags {
TransactionFlags() {
}

public static TransactionFlags of(long value) {
return new TransactionFlags(value);
}

/**
* Flags indicating that a fully-canonical signature is required. This flag is highly recommended.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.common.primitives.UnsignedInteger;
import com.google.common.primitives.UnsignedLong;
import org.xrpl.xrpl4j.model.client.accounts.AccountTransactionsTransaction;
Expand All @@ -33,14 +36,18 @@
import org.xrpl.xrpl4j.model.transactions.Transaction;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Optional;
import java.util.Set;

/**
* Custom Jackson Deserializer for {@link AccountTransactionsTransaction}s. This is necessary because Jackson
* does not deserialize {@link com.fasterxml.jackson.annotation.JsonUnwrapped} fields intelligently.
*/
public class AccountTransactionsTransactionDeserializer extends StdDeserializer<AccountTransactionsTransaction<?>> {

public static final Set<String> EXTRA_TRANSACTION_FIELDS = Sets.newHashSet("ledger_index", "date", "hash");

/**
* No-args constructor.
*/
Expand All @@ -54,14 +61,20 @@ public AccountTransactionsTransaction<?> deserialize(
DeserializationContext ctxt
) throws IOException {
ObjectMapper objectMapper = (ObjectMapper) jsonParser.getCodec();
JsonNode node = objectMapper.readTree(jsonParser);
ObjectNode node = objectMapper.readTree(jsonParser);

Transaction transaction = objectMapper.readValue(node.toString(), Transaction.class);
long ledgerIndex = node.get("ledger_index").asLong(-1L);
String hash = node.get("hash").asText();
Optional<UnsignedLong> closeDate = Optional.ofNullable(node.get("date"))
.map(JsonNode::asLong)
.map(UnsignedLong::valueOf);

// The Transaction is @JsonUnwrapped in AccountTransactionsTransaction, which means these three fields
// get added to the Transaction.unknownFields Map. To prevent that, we simply remove them from the JSON, because
// they should only show up in AccountTransactionsTransaction
node.remove(EXTRA_TRANSACTION_FIELDS);
Transaction transaction = objectMapper.readValue(node.toString(), Transaction.class);

return AccountTransactionsTransaction.builder()
.transaction(transaction)
.ledgerIndex(LedgerIndex.of(UnsignedInteger.valueOf(ledgerIndex)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.collect.Sets;
import com.google.common.primitives.UnsignedInteger;
import com.google.common.primitives.UnsignedLong;
import org.xrpl.xrpl4j.model.client.common.LedgerIndex;
Expand All @@ -37,6 +38,7 @@

import java.io.IOException;
import java.util.Optional;
import java.util.Set;

/**
* Custom deserializer for {@link TransactionResult}, which wraps the {@link Transaction} fields in the result JSON.
Expand All @@ -48,6 +50,10 @@
*/
public class TransactionResultDeserializer<T extends Transaction> extends StdDeserializer<TransactionResult<T>> {

public static final Set<String> EXTRA_TRANSACTION_FIELDS = Sets.newHashSet(
"ledger_index", "date", "hash", "status", "validated", "meta", "metaData"
);

/**
* No-args constructor.
*/
Expand All @@ -60,10 +66,6 @@ public TransactionResult<T> deserialize(JsonParser jsonParser, DeserializationCo
ObjectMapper objectMapper = (ObjectMapper) jsonParser.getCodec();
ObjectNode objectNode = objectMapper.readTree(jsonParser);

JavaType javaType = objectMapper.getTypeFactory().constructType(new TypeReference<T>() {
});
T transaction = objectMapper.convertValue(objectNode, javaType);

LedgerIndex ledgerIndex = objectNode.has("ledger_index") ?
LedgerIndex.of(UnsignedInteger.valueOf(objectNode.get("ledger_index").asInt())) :
null;
Expand All @@ -73,6 +75,15 @@ public TransactionResult<T> deserialize(JsonParser jsonParser, DeserializationCo
Optional<TransactionMetadata> metadata = getTransactionMetadata(objectMapper, objectNode);
UnsignedLong closeDate = objectNode.has("date") ? UnsignedLong.valueOf(objectNode.get("date").asLong()) : null;

// The Transaction is @JsonUnwrapped in TransactionResult, which means these fields
// get added to the Transaction.unknownFields Map. To prevent that, we simply remove them from the JSON, because
// they should only show up in AccountTransactionsTransaction
objectNode.remove(EXTRA_TRANSACTION_FIELDS);

JavaType javaType = objectMapper.getTypeFactory().constructType(new TypeReference<T>() {
});
T transaction = objectMapper.convertValue(objectNode, javaType);

return TransactionResult.<T>builder()
.transaction(transaction)
.ledgerIndex(Optional.ofNullable(ledgerIndex))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
* =========================LICENSE_END==================================
*/

import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableBiMap;
Expand All @@ -30,6 +32,7 @@
import org.xrpl.xrpl4j.crypto.signing.Signature;

import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
Expand Down Expand Up @@ -90,6 +93,7 @@ public interface Transaction {
.put(ImmutableDidDelete.class, TransactionType.DID_DELETE)
.put(ImmutableOracleSet.class, TransactionType.ORACLE_SET)
.put(ImmutableOracleDelete.class, TransactionType.ORACLE_DELETE)
.put(ImmutableUnknownTransaction.class, TransactionType.UNKNOWN)
.build();

/**
Expand All @@ -106,6 +110,7 @@ public interface Transaction {
* @return A {@link TransactionType}.
*/
@JsonProperty("TransactionType")
@Value.Default // must be Default rather than Derived, otherwise Jackson treats "TransactionType" as an unknownField
default TransactionType transactionType() {
return typeMap.get(this.getClass());
}
Expand Down Expand Up @@ -220,4 +225,8 @@ default PublicKey signingPublicKey() {
@JsonProperty("NetworkID")
Optional<NetworkId> networkId();

@JsonAnyGetter
@JsonInclude(Include.NON_ABSENT)
Map<String, Object> unknownFields();

}
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,12 @@ public enum TransactionType {
* is subject to change.</p>
*/
@Beta
ORACLE_DELETE("OracleDelete");
ORACLE_DELETE("OracleDelete"),

/**
* The {@link TransactionType} for any transaction that is unrecognized/unsupported by xrpl4j.
*/
UNKNOWN("Unknown");

private final String value;

Expand All @@ -358,7 +363,7 @@ public static TransactionType forValue(String value) {
}
}

throw new IllegalArgumentException("No matching TransactionType enum value for String value " + value);
return UNKNOWN;
nkramer44 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package org.xrpl.xrpl4j.model.transactions;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import org.immutables.value.Value;
import org.immutables.value.Value.Immutable;
import org.xrpl.xrpl4j.model.flags.TransactionFlags;

/**
* Mapping for any transaction type that is unrecognized/unsupported by xrpl4j.
*/
@Immutable
@JsonSerialize(as = ImmutableUnknownTransaction.class)
@JsonDeserialize(as = ImmutableUnknownTransaction.class)
public interface UnknownTransaction extends Transaction {

/**
* Construct a {@code UnknownTransaction} builder.
*
* @return An {@link ImmutableUnknownTransaction.Builder}.
*/
static ImmutableUnknownTransaction.Builder builder() {
return ImmutableUnknownTransaction.builder();
}

/**
* The actual transaction type found in the {@code "TransactionType"} field of the transaction JSON.
*
* <p>This has to be a {@link String} because {@link Transaction#transactionType()} is a {@link TransactionType},
* which only has an UNKNOWN variant. Because this method is also annotated with {@link JsonProperty} of
* "TransactionType", this essentially overrides the "TransactionType" field in JSON, but {@link #transactionType()}
* will always be {@link TransactionType#UNKNOWN} and this field will contain the actual "TransactionType" field.
*
* @return A {@link String} containing the transaction type from JSON.
*/
@JsonProperty("TransactionType")
String unknownTransactionType();

/**
* The {@link TransactionType} of this UnknownTransaction, which will always be {@link TransactionType#UNKNOWN}.
* {@link #unknownTransactionType()} contains the actual transaction type value.
*
* @return {@link TransactionType#UNKNOWN}.
*/
@Override
@JsonIgnore
@Value.Derived
default TransactionType transactionType() {
return Transaction.super.transactionType();
}

/**
* A set of {@link TransactionFlags}.
*
* @return A {@link TransactionFlags}.
*/
@JsonProperty("Flags")
@Value.Default
default TransactionFlags flags() {
return TransactionFlags.EMPTY;

Check warning on line 62 in xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/UnknownTransaction.java

View check run for this annotation

Codecov / codecov/patch

xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/UnknownTransaction.java#L62

Added line #L62 was not covered by tests
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ static ImmutableUnlModify.Builder builder() {
*/
@Override
@JsonProperty("Account")
@Value.Derived
// FIXME: Have to make this Default, otherwise JsonAnySetter adds this field to unknownFields. Other option
// is that's totally fine, which i think it is -- we should let the JSON set this field if it's there
@Value.Default
Comment on lines +65 to +67
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sappenin we should discuss this change

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, if we wanted to keep the @Derived could make a special carve-out in the Deserializer to add Account into EXTRA_TRANSACTION_FIELDS.

However, that's ugly, and makes it so nobody can create a UnlModify with an account that's not account-zero. That's a non-goal ATM, but I think I prefer making this @Default just in case someone every wants to do that (e.g., for testing, or whatever).

default Address account() {
return ACCOUNT_ZERO;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,4 +312,58 @@ void testJsonWithXrpAmountBidMinAndMax() throws JSONException, JsonProcessingExc
"}";
assertCanSerializeAndDeserialize(ammBid, json);
}

@Test
void testJsonWithUnknownFields() throws JSONException, JsonProcessingException {
AmmBid bid = AmmBid.builder()
.account(Address.of("rJVUeRqDFNs2xqA7ncVE6ZoAhPUoaJJSQm"))
.signingPublicKey(
PublicKey.fromBase16EncodedPublicKey("02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC")
)
.asset(Issue.XRP)
.asset2(
Issue.builder()
.issuer(Address.of("rP9jPyP5kyvFRb6ZiRghAGw5u8SGAmU4bd"))
.currency("TST")
.build()
)
.addAuthAccounts(
AuthAccountWrapper.of(AuthAccount.of(Address.of("rMKXGCbJ5d8LbrqthdG46q3f969MVK2Qeg"))),
AuthAccountWrapper.of(AuthAccount.of(Address.of("rBepJuTLFJt3WmtLXYAxSjtBWAeQxVbncv")))
)
.fee(XrpCurrencyAmount.ofDrops(10))
.sequence(UnsignedInteger.valueOf(9))
.putUnknownFields("Foo", "Bar")
.build();

String json = "{\n" +
" \"Foo\" : \"Bar\",\n" +
" \"Account\" : \"rJVUeRqDFNs2xqA7ncVE6ZoAhPUoaJJSQm\",\n" +
" \"Asset\" : {\n" +
" \"currency\" : \"XRP\"\n" +
" },\n" +
" \"Asset2\" : {\n" +
" \"currency\" : \"TST\",\n" +
" \"issuer\" : \"rP9jPyP5kyvFRb6ZiRghAGw5u8SGAmU4bd\"\n" +
" },\n" +
" \"AuthAccounts\" : [\n" +
" {\n" +
" \"AuthAccount\" : {\n" +
" \"Account\" : \"rMKXGCbJ5d8LbrqthdG46q3f969MVK2Qeg\"\n" +
" }\n" +
" },\n" +
" {\n" +
" \"AuthAccount\" : {\n" +
" \"Account\" : \"rBepJuTLFJt3WmtLXYAxSjtBWAeQxVbncv\"\n" +
" }\n" +
" }\n" +
" ],\n" +
" \"Fee\" : \"10\",\n" +
" \"Sequence\" : 9,\n" +
" \"SigningPubKey\" : \"02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC\",\n" +
" \"TransactionType\" : \"AMMBid\"\n" +
"}";

assertCanSerializeAndDeserialize(bid, json);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,44 @@ void testJsonWithNonZeroFlags() throws JSONException, JsonProcessingException {

assertCanSerializeAndDeserialize(ammCreate, json);
}

@Test
void testJsonWithUnknownFields() throws JSONException, JsonProcessingException {
AmmCreate ammCreate = AmmCreate.builder()
.account(Address.of("rJVUeRqDFNs2xqA7ncVE6ZoAhPUoaJJSQm"))
.amount(
IssuedCurrencyAmount.builder()
.currency("TST")
.issuer(Address.of("rP9jPyP5kyvFRb6ZiRghAGw5u8SGAmU4bd"))
.value("25")
.build()
)
.amount2(XrpCurrencyAmount.ofDrops(250000000))
.fee(XrpCurrencyAmount.ofDrops(10))
.sequence(UnsignedInteger.valueOf(6))
.tradingFee(TradingFee.of(UnsignedInteger.valueOf(500)))
.signingPublicKey(
PublicKey.fromBase16EncodedPublicKey("02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC")
)
.putUnknownFields("Foo", "Bar")
.build();

String json = "{\n" +
" \"Foo\" : \"Bar\",\n" +
" \"Account\" : \"rJVUeRqDFNs2xqA7ncVE6ZoAhPUoaJJSQm\",\n" +
" \"Amount\" : {\n" +
" \"currency\" : \"TST\",\n" +
" \"issuer\" : \"rP9jPyP5kyvFRb6ZiRghAGw5u8SGAmU4bd\",\n" +
" \"value\" : \"25\"\n" +
" },\n" +
" \"Amount2\" : \"250000000\",\n" +
" \"Fee\" : \"10\",\n" +
" \"Sequence\" : 6,\n" +
" \"TradingFee\" : 500,\n" +
" \"SigningPubKey\" : \"02356E89059A75438887F9FEE2056A2890DB82A68353BE9C0C0C8F89C0018B37FC\",\n" +
" \"TransactionType\" : \"AMMCreate\"\n" +
"}";

assertCanSerializeAndDeserialize(ammCreate, json);
}
}
Loading
Loading