Fixes #590 (#594)

Fixes #590 by removing the `Account` property from any incoming `UnlModify` JSON about to be deserialized. This fixes #590 because the JSON returned by the rippled/clio API v1 has a bug where the account value in `UnlModify` transactions is an empty string, when serialized throws an exception because empty string is not a valid Address. By removing the property from incoming JSON, the Java value for the `Account` property is always set to ACCOUNT_ZERO via a default method. Without this fix, the `Account` will also errantly end up in the `unknownFields map of the ultimate Java object, which is incorrect.

Author: sappenin

xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/jackson/modules/TransactionDeserializer.java

@@ -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.
@@ -27,6 +27,7 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.xrpl.xrpl4j.model.transactions.Transaction;
 import org.xrpl.xrpl4j.model.transactions.TransactionType;
+import org.xrpl.xrpl4j.model.transactions.UnlModify;
 
 import java.io.IOException;
 
@@ -45,10 +46,23 @@ protected TransactionDeserializer() {
 
   @Override
   public Transaction deserialize(JsonParser jsonParser, DeserializationContext ctxt) throws IOException {
-    ObjectMapper objectMapper = (ObjectMapper) jsonParser.getCodec();
-    ObjectNode objectNode = objectMapper.readTree(jsonParser);
+    final ObjectMapper objectMapper = (ObjectMapper) jsonParser.getCodec();
+    final ObjectNode objectNode = objectMapper.readTree(jsonParser);
 
     TransactionType transactionType = TransactionType.forValue(objectNode.get("TransactionType").asText());
-    return objectMapper.treeToValue(objectNode, Transaction.typeMap.inverse().get(transactionType));
+    final Class<? extends Transaction> transactionTypeClass = Transaction.typeMap.inverse().get(transactionType);
+
+    // Fixes #590 by removing the `Account` property from any incoming `UnlModify` JSON about to be deserialized. 
+    // This fixes #590 because the JSON returned by the rippled/clio API v1 has a bug where the account value in 
+    // `UnlModify` transactions is an empty string. When this value is deserialized, an exception is thrown because 
+    // the empty string value is not a valid `Address`. By removing the property from incoming JSON, the Java value 
+    // for the `Account` property is always set to ACCOUNT_ZERO via a default method. One other side effect of this 
+    // fix is that `Account` property will not be errantly added to `unknownFields map of the ultimate Java object,
+    // which is incorrect.
+    if (UnlModify.class.isAssignableFrom(transactionTypeClass)) {
+      objectNode.remove("Account");
+    }
+
+    return objectMapper.treeToValue(objectNode, transactionTypeClass);
   }
 }

xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/UnlModify.java

@@ -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.
@@ -28,8 +28,8 @@
 import org.xrpl.xrpl4j.model.client.common.LedgerIndex;
 
 /**
- * A {@link UnlModify} pseudo-transaction marks a change to the Negative UNL,
- * indicating that a trusted validator has gone offline or come back online.
+ * A {@link UnlModify} pseudo-transaction marks a change to the Negative UNL, indicating that a trusted validator has
+ * gone offline or come back online.
  *
  * @see "https://xrpl.org/unlmodify.html"
  */
@@ -49,36 +49,35 @@ static ImmutableUnlModify.Builder builder() {
     return ImmutableUnlModify.builder();
   }
 
-
   /**
-   * This field is overridden in this class because of a bug in rippled that causes this field to be missing
-   * in API responses. In other pseudo-transactions such as {@link SetFee} and {@link EnableAmendment}, the rippled
-   * API sets the {@code account} field to a special XRPL address called ACCOUNT_ZERO, which is the base58
-   * encoding of the number zero. Because rippled does not set the {@code account} field of the {@link UnlModify}
-   * pseudo-transaction, this override will always set the field to ACCOUNT_ZERO to avoid deserialization issues
-   * and to be consistent with other pseudo-transactions.
+   * This field is overridden in this class because of a bug in rippled that causes this field to be missing in API
+   * responses. In other pseudo-transactions such as {@link SetFee} and {@link EnableAmendment}, the rippled API sets
+   * the {@code account} field to a special XRPL address called ACCOUNT_ZERO, which is the base58 encoding of the number
+   * zero. Because rippled does not set the {@code account} field of the {@link UnlModify} pseudo-transaction, this
+   * override will always set the field to ACCOUNT_ZERO to avoid deserialization issues and to be consistent with other
+   * pseudo-transactions.
    *
    * @return Always returns ACCOUNT_ZERO, which is the base58 encoding of the number zero.
    */
   @Override
   @JsonProperty("Account")
-  @Value.Default
+  @Value.Default // Must be `Default` not `Derived`, else this field will be serialized into `unknownFields`.
   default Address account() {
     return ACCOUNT_ZERO;
   }
 
   /**
-   * The {@link LedgerIndex} where this pseudo-transaction appears.
-   * This distinguishes the pseudo-transaction from other occurrences of the same change.
+   * The {@link LedgerIndex} where this pseudo-transaction appears. This distinguishes the pseudo-transaction from other
+   * occurrences of the same change.
    *
    * @return A {@link LedgerIndex} to indicates where the tx appears.
    */
   @JsonProperty("LedgerSequence")
   LedgerIndex ledgerSequence();
 
   /**
-   * If 1, this change represents adding a validator to the Negative UNL. If 0, this change represents
-   * removing a validator from the Negative UNL.
+   * If 1, this change represents adding a validator to the Negative UNL. If 0, this change represents removing a
+   * validator from the Negative UNL.
    *
    * @return An {@link UnsignedInteger} denoting either 0 or 1.
    */

xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/NegativeTransactionMetadataTest.java

@@ -1,7 +1,6 @@
 package org.xrpl.xrpl4j.model.transactions;
 
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
-import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.jupiter.api.Test;
@@ -112,6 +111,29 @@ void deserializeLedgerResultWithNegativeAmounts(String ledgerResultFileName) thr
     });
   }
 
+  /**
+   * This test validates that the ledger 94084608 and all of its transactions and metadata are handled correctly, even
+   * in the presence of a `UnlModify` transaction that has an empty `Account`.
+   */
+  @ParameterizedTest
+  @ValueSource(strings = {
+    "ledger-result-94084608.json" // <-- See https://github.com/XRPLF/xrpl4j/issues/590
+  })
+  void deserializeLedgerResultWithSpecialObjects(String ledgerResultFileName) throws IOException {
+    Objects.requireNonNull(ledgerResultFileName);
+
+    File jsonFile = new File(
+      "src/test/resources/special-object-ledgers/" + ledgerResultFileName
+    );
+
+    LedgerResult ledgerResult = objectMapper.readValue(jsonFile, LedgerResult.class);
+
+    ledgerResult.ledger().transactions().forEach(transactionResult -> {
+      assertThat(transactionResult.metadata().isPresent()).isTrue();
+      transactionResult.metadata().ifPresent(this::handleTransactionMetadata);
+    });
+  }
+
   /**
    * This test validates that the ledger 87704323 and all of its transactions and metadata are handled correctly, even
    * in the presence of negative XRP or IOU amounts.
@@ -144,9 +166,13 @@ private void handleTransactionMetadata(final TransactionMetadata transactionMeta
           } else if (ledgerEntryType.equals(MetaLedgerEntryType.RIPPLE_STATE)) {
             handleMetaLedgerObject((MetaRippleStateObject) createdNode.newFields());
           } else if (ledgerEntryType.equals(MetaLedgerEntryType.DIRECTORY_NODE)) {
-            logger.warn("Ignoring ledger entry type {}", ledgerEntryType);
+            logger.warn("Ignoring CreatedNode ledger entry type {}", ledgerEntryType);
+          } else if (ledgerEntryType.equals(MetaLedgerEntryType.NEGATIVE_UNL)) {
+            logger.warn(
+              "Ignoring DeletedNode ledger entry type {}. See https://github.com/XRPLF/xrpl4j/issues/16",
+              ledgerEntryType);
           } else {
-            throw new RuntimeException("Unhandled ledger entry type: " + ledgerEntryType);
+            throw new RuntimeException("Unhandled CreatedNode ledger entry type: " + ledgerEntryType);
           }
         },
         (modifiedNode) -> {
@@ -159,8 +185,19 @@ private void handleTransactionMetadata(final TransactionMetadata transactionMeta
                 handleMetaLedgerObject((MetaAccountRootObject) metaLedgerObject);
               } else if (ledgerEntryType.equals(MetaLedgerEntryType.RIPPLE_STATE)) {
                 handleMetaLedgerObject((MetaRippleStateObject) metaLedgerObject);
+              } else if (ledgerEntryType.equals(MetaLedgerEntryType.DIRECTORY_NODE)) {
+                logger.warn("Ignoring ModifiedNode ledger entry type {}", ledgerEntryType);
+              } else if (ledgerEntryType.equals(MetaLedgerEntryType.NEGATIVE_UNL)) {
+                logger.warn(
+                  "Ignoring DeletedNode ledger entry type {}. See https://github.com/XRPLF/xrpl4j/issues/16",
+                  ledgerEntryType);
+              } else if (ledgerEntryType.equals(MetaLedgerEntryType.AMM)) {
+                logger.warn(
+                  "Ignoring DeletedNode ledger entry type {}. See https://github.com/XRPLF/xrpl4j/issues/591",
+                  ledgerEntryType);
               } else {
-                throw new RuntimeException("Unhandled ledger entry type: " + ledgerEntryType);
+                throw new RuntimeException(
+                  "Unhandled ModifiedNode PreviousFields ledger entry type: " + ledgerEntryType);
               }
             });
 
@@ -173,10 +210,15 @@ private void handleTransactionMetadata(final TransactionMetadata transactionMeta
                 handleMetaLedgerObject((MetaAccountRootObject) metaLedgerObject);
               } else if (ledgerEntryType.equals(MetaLedgerEntryType.RIPPLE_STATE)) {
                 handleMetaLedgerObject((MetaRippleStateObject) metaLedgerObject);
-              } else if (ledgerEntryType.equals(MetaLedgerEntryType.DIRECTORY_NODE)) {
-                logger.warn("Ignoring ledger entry type {}", ledgerEntryType);
+              } else if (
+                ledgerEntryType.equals(MetaLedgerEntryType.DIRECTORY_NODE)) {
+                logger.warn("Ignoring ModifiedNode ledger entry type {}", ledgerEntryType);
+              } else if (ledgerEntryType.equals(MetaLedgerEntryType.AMM)) {
+                logger.warn(
+                  "Ignoring ModifiedNode ledger entry type {}. See See https://github.com/XRPLF/xrpl4j/issues/591",
+                  ledgerEntryType);
               } else {
-                throw new RuntimeException("Unhandled ledger entry type: " + ledgerEntryType);
+                throw new RuntimeException("Unhandled ModifiedNode FinalFields ledger entry type: " + ledgerEntryType);
               }
             });
         },
@@ -191,7 +233,8 @@ private void handleTransactionMetadata(final TransactionMetadata transactionMeta
               } else if (ledgerEntryType.equals(MetaLedgerEntryType.RIPPLE_STATE)) {
                 handleMetaLedgerObject((MetaRippleStateObject) metaLedgerObject);
               } else {
-                throw new RuntimeException("Unhandled ledger entry type: " + ledgerEntryType);
+                throw new RuntimeException(
+                  "Unhandled DeletedNode PreviousFields ledger entry type: " + ledgerEntryType);
               }
             });
 
@@ -204,14 +247,15 @@ private void handleTransactionMetadata(final TransactionMetadata transactionMeta
           } else if (ledgerEntryType.equals(MetaLedgerEntryType.RIPPLE_STATE)) {
             handleMetaLedgerObject((MetaRippleStateObject) finalFields);
           } else if (ledgerEntryType.equals(MetaLedgerEntryType.TICKET)) {
-            logger.info("Ignoring ledger entry type {} because it has no currency values for negative checking",
-              ledgerEntryType);
+            logger.info(
+              "Ignoring ledger entry type {} because it has no currency values for negative checking", ledgerEntryType
+            );
           } else if (ledgerEntryType.equals(MetaLedgerEntryType.DIRECTORY_NODE)) {
-            logger.warn("Ignoring ledger entry type {}", ledgerEntryType);
+            logger.warn("Ignoring DeletedNode ledger entry type {}", ledgerEntryType);
           } else if (ledgerEntryType.equals(MetaLedgerEntryType.NFTOKEN_OFFER)) {
             handleMetaLedgerObject((MetaNfTokenOfferObject) finalFields);
           } else {
-            throw new RuntimeException("Unhandled ledger entry type: " + ledgerEntryType);
+            throw new RuntimeException("Unhandled DeletedNode FinalFields ledger entry type: " + ledgerEntryType);
           }
         }
       );
@@ -292,4 +336,4 @@ private void handleMetaLedgerObject(MetaNfTokenOfferObject metaNfTokenOfferObjec
           issuedCurrencyAmount.value().startsWith("-"))
       ));
   }
-}
+}