Merge branch 'main' into nk/xrp-fees

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

@@ -228,9 +228,19 @@ default EscrowFinish normalizeCondition() {
         // In this case, we should try to read conditionRawValue to a Condition. If that fails, condition()
         // will remain empty, otherwise we will set condition().
         try {
-          Condition condition = CryptoConditionReader.readCondition(
-            BaseEncoding.base16().decode(conditionRawValue().get().toUpperCase(Locale.US))
-          );
+          byte[] conditionRawValueBytes = BaseEncoding.base16()
+            .decode(conditionRawValue().get().toUpperCase(Locale.US));
+          Condition condition = CryptoConditionReader.readCondition(conditionRawValueBytes);
+
+          // CryptoConditionReader.readCondition can succeed if the first part of the condition raw value
+          // is a valid condition, but the raw value has extra bytes afterward. Here, we check that the parsed
+          // condition is equivalent to the raw value when re-written.
+          if (!Arrays.equals(CryptoConditionWriter.writeCondition(condition), conditionRawValueBytes)) {
+            logger.warn("EscrowFinish Condition was malformed: mismatch between raw value and parsed condition. " +
+              "conditionRawValue() will contain the condition value, but condition() will be empty.");
+            return this;
+          }
+
           return EscrowFinish.builder().from(this)
             .condition(condition)
             .build();
@@ -300,9 +310,19 @@ default EscrowFinish normalizeFulfillment() {
         // In this case, we should try to read fulfillmentRawValue to a Condition. If that fails, fulfillment()
         // will remain empty, otherwise we will set fulfillment().
         try {
-          Fulfillment<?> fulfillment = CryptoConditionReader.readFulfillment(
-            BaseEncoding.base16().decode(fulfillmentRawValue().get().toUpperCase(Locale.US))
-          );
+          byte[] fulfillmentRawValueBytes = BaseEncoding.base16()
+            .decode(fulfillmentRawValue().get().toUpperCase(Locale.US));
+          Fulfillment<?> fulfillment = CryptoConditionReader.readFulfillment(fulfillmentRawValueBytes);
+
+          // CryptoConditionReader.readFulfillment can succeed if the first part of the fulfillment raw value
+          // is a valid fulfillment, but the raw value has extra bytes afterward. Here, we check that the parsed
+          // fulfillment is equivalent to the raw value when re-written.
+          if (!Arrays.equals(CryptoConditionWriter.writeFulfillment(fulfillment), fulfillmentRawValueBytes)) {
+            logger.warn("EscrowFinish Fulfillment was malformed: mismatch between raw value and parsed fulfillment. " +
+              "fulfillmentRawValue() will contain the fulfillment value, but fulfillment() will be empty.");
+            return this;
+          }
+
           return EscrowFinish.builder().from(this)
             .fulfillment(fulfillment)
             .build();
@@ -318,7 +338,7 @@ default EscrowFinish normalizeFulfillment() {
       }
 
     } catch (DerEncodingException e) {
-      // This should never happen. CryptoconditionWriter.writeCondition errantly declares that it can throw
+      // This should never happen. CryptoconditionWriter.writeFulfillment errantly declares that it can throw
       // a DerEncodingException, but nowhere in its implementation does it throw.
       throw new RuntimeException(e);
     }

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

@@ -28,6 +28,7 @@
 import com.google.common.primitives.UnsignedInteger;
 import com.ripple.cryptoconditions.Condition;
 import com.ripple.cryptoconditions.CryptoConditionReader;
+import com.ripple.cryptoconditions.CryptoConditionWriter;
 import com.ripple.cryptoconditions.Fulfillment;
 import com.ripple.cryptoconditions.PreimageSha256Condition;
 import com.ripple.cryptoconditions.PreimageSha256Fulfillment;
@@ -176,6 +177,29 @@ void normalizeWithNoConditionAndRawValueForMalformedCondition() {
     assertThat(actual.conditionRawValue()).isNotEmpty().get().isEqualTo("1234");
   }
 
+  /**
+   * This tests the case where conditionRawValue is present and is parseable to a Condition but when the
+   * parsed Condition is written to a byte array, the value differs from the conditionRawValue bytes. This
+   * can occur if the condition raw value contains a valid condition in the first 32 bytes, but also includes
+   * extra bytes afterward.
+   */
+  @Test
+  void normalizeConditionWithRawValueThatIsParseableButNotValid() {
+    EscrowFinish actual = EscrowFinish.builder()
+      .fee(XrpCurrencyAmount.ofDrops(1))
+      .account(Address.of("rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59Ba"))
+      .sequence(UnsignedInteger.ONE)
+      .owner(Address.of("rN7n7otQDd6FczFgLdSqtcsAUxDkw6fzRH"))
+      .offerSequence(UnsignedInteger.ZERO)
+      .conditionRawValue(GOOD_CONDITION_STR + GOOD_CONDITION_STR)
+      .build();
+
+    assertThat(actual.condition()).isEmpty();
+    assertThat(actual.conditionRawValue()).isNotEmpty().get().isEqualTo(
+      GOOD_CONDITION_STR + GOOD_CONDITION_STR
+    );
+  }
+
   @Test
   void normalizeWithNoConditionAndRawValueForBadHexLengthCondition() {
     EscrowFinish actual = EscrowFinish.builder()
@@ -284,6 +308,29 @@ void normalizeWithNoFulfillmentAndRawValueForMalformedFulfillment() {
     assertThat(actual.fulfillmentRawValue()).isNotEmpty().get().isEqualTo("1234");
   }
 
+  /**
+   * This tests the case where fulfillmentRawValue is present and is parseable to a Fulfillment<?> but when the
+   * parsed Fulfillment is written to a byte array, the value differs from the fulfillmentRawValue bytes. This
+   * can occur if the fulfillment raw value contains a valid fulfillment in the first 32 bytes, but also includes
+   * extra bytes afterward, such as in transaction 138543329687544CDAFCD3AB0DCBFE9C4F8E710397747BA7155F19426F493C8D.
+   */
+  @Test
+  void normalizeFulfillmentWithRawValueThatIsParseableButNotValid() {
+    EscrowFinish actual = EscrowFinish.builder()
+      .fee(XrpCurrencyAmount.ofDrops(1))
+      .account(Address.of("rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59Ba"))
+      .sequence(UnsignedInteger.ONE)
+      .owner(Address.of("rN7n7otQDd6FczFgLdSqtcsAUxDkw6fzRH"))
+      .offerSequence(UnsignedInteger.ZERO)
+      .fulfillmentRawValue(GOOD_FULFILLMENT_STR + GOOD_FULFILLMENT_STR)
+      .build();
+
+    assertThat(actual.fulfillment()).isEmpty();
+    assertThat(actual.fulfillmentRawValue()).isNotEmpty().get().isEqualTo(
+      GOOD_FULFILLMENT_STR + GOOD_FULFILLMENT_STR
+    );
+  }
+
   @Test
   void normalizeWithNoFulfillmentAndRawValueForBadHexLengthFulfillment() {
     EscrowFinish actual = EscrowFinish.builder()