Fix broken unit tests

xrpl4j-core/src/main/java/org/xrpl/xrpl4j/crypto/keys/PrivateKey.java

@@ -119,6 +119,10 @@ public static PrivateKey fromPrefixedBytes(final UnsignedByteArray value) {
    */
   @Deprecated
   public static PrivateKey of(final UnsignedByteArray value) {
+    // Assumption: Any developer using this method before this method was deprecated (i.e., v3.3) will likely have
+    // expected `value` to have been prefixed. That said, this assumption is technically invalid because it's ambiguous
+    // what any particular developer would have meant (see #486) because prior to #486, there was no byte-length check
+    // on these bytes.
     return fromPrefixedBytes(value);
   }
 
@@ -128,16 +132,17 @@ public static PrivateKey of(final UnsignedByteArray value) {
    * @param value An {@link UnsignedByteArray} for this key's value.
    */
   private PrivateKey(final UnsignedByteArray value, final KeyType keyType) {
-    this.value = Objects.requireNonNull(value);
+    Objects.requireNonNull(value); // <-- Check not-null first.
     this.keyType = Objects.requireNonNull(keyType);
 
-    // We assert this precondition here instead of allowing 33 bytes because this is a private constructor that can be
-    // fully tested via unit test, so this precondition should never be violated, and if it is, then it's a bug in
-    // xrpl4j.
+    // We assert this precondition here because this is a private constructor that can be fully tested via unit test,
+    // so this precondition should never be violated, and if it is, then it's a bug in xrpl4j.
     Preconditions.checkArgument(
       value.length() == 32,
       "Byte values passed to this constructor must be 32 bytes long, with no prefix."
     );
+
+    this.value = UnsignedByteArray.of(value.toByteArray()); // <- Always copy to ensure immutability
   }
 
   /**
@@ -149,12 +154,12 @@ private PrivateKey(final UnsignedByteArray value, final KeyType keyType) {
    */
   @Deprecated
   public UnsignedByteArray value() {
-    // This is technically wrong (because `value()` had an ambiguous meaning prior to fixing #486), but this mirrors
-    // what's in v3 prior to fixing #486, and will be fixed in v4 once the deprecated `.value()` is removed.
+    // Check for empty value, which can occur if this PrivateKey is "destroyed" but still in memory.
     if (value.length() == 0) {
       return UnsignedByteArray.empty();
     } else {
-
+      // This is technically wrong (because `value()` had an ambiguous meaning prior to fixing #486), but this mirrors
+      // what's in v3 prior to fixing #486, and will be fixed in v4 once the deprecated `.value()` is removed.
       return this.valueWithPrefixedBytes();
     }
   }
@@ -168,9 +173,14 @@ public UnsignedByteArray value() {
    * @return An instance of {@link UnsignedByteArray}.
    */
   public UnsignedByteArray valueWithNaturalBytes() {
-    // Note: `toByteArray()` will perform a copy, which is what we want in order to enforce immutability of this
-    // PrivateKey (because Java 8 doesn't support immutable byte arrays).
-    return UnsignedByteArray.of(value.toByteArray());
+    // Check for empty value, which can occur if this PrivateKey is "destroyed" but still in memory.
+    if (value.length() == 0) {
+      return UnsignedByteArray.empty();
+    } else {
+      // Note: `toByteArray()` will perform a copy, which is what we want in order to enforce immutability of this
+      // PrivateKey (because Java 8 doesn't support immutable byte arrays).
+      return UnsignedByteArray.of(value.slice(0, 32).toByteArray());
+    }
   }
 
   /**
@@ -182,18 +192,23 @@ public UnsignedByteArray valueWithNaturalBytes() {
    * @return An instance of {@link UnsignedByteArray}.
    */
   public UnsignedByteArray valueWithPrefixedBytes() {
-    // Note: value.slice() will take a view of the existing UBA, then `.toByteArray()` will perform a copy, which is
-    // what we want in order to enforce immutability of this PrivateKey (because Java 8 doesn't support immutable byte
-    // arrays).
-    switch (keyType) {
-      case ED25519: {
-        return UnsignedByteArray.of(ED2559_PREFIX).append(value);
-      }
-      case SECP256K1: {
-        return UnsignedByteArray.of(SECP256K1_PREFIX).append(value);
-      }
-      default: {
-        throw new IllegalStateException(String.format("Invalid keyType=%s", keyType));
+    // Check for empty value, which can occur if this PrivateKey is "destroyed" but still in memory.
+    if (value.length() == 0) {
+      return UnsignedByteArray.empty();
+    } else {
+      // Note: value.slice() will take a view of the existing UBA, then `.toByteArray()` will perform a copy, which is
+      // what we want in order to enforce immutability of this PrivateKey (because Java 8 doesn't support immutable byte
+      // arrays).
+      switch (keyType) {
+        case ED25519: {
+          return UnsignedByteArray.of(ED2559_PREFIX).append(value);
+        }
+        case SECP256K1: {
+          return UnsignedByteArray.of(SECP256K1_PREFIX).append(value);
+        }
+        default: {
+          throw new IllegalStateException(String.format("Invalid keyType=%s", keyType));
+        }
       }
     }
   }

xrpl4j-core/src/test/java/org/xrpl/xrpl4j/codec/addresses/PrivateKeyCodecTest.java

@@ -23,7 +23,7 @@ public void encodeDecodeEdNodePrivate() {
     testEncodeDecode(
       prefixedNodePrivateKey -> privateKeyCodec.encodeNodePrivateKey(prefixedNodePrivateKey),
       prefixedNodePrivateKey -> privateKeyCodec.decodeNodePrivateKey(prefixedNodePrivateKey),
-      TestConstants.ED_PRIVATE_KEY.valueWithNaturalBytes(),
+      TestConstants.getEdPrivateKey().valueWithNaturalBytes(),
       "paZHnTCvwm4GsxZ7qiA2nUBKE2DLnCoDWYqyocVZfVEZx3kvA4u"
     );
   }
@@ -48,7 +48,7 @@ public void encodeDecodeEdAccountPrivateKey() {
     testEncodeDecode(
       prefixedAccountPrivateKey -> privateKeyCodec.encodeAccountPrivateKey(prefixedAccountPrivateKey),
       prefixedAccountPrivateKey -> privateKeyCodec.decodeAccountPrivateKey(prefixedAccountPrivateKey),
-      TestConstants.ED_PRIVATE_KEY.valueWithNaturalBytes(),
+      TestConstants.getEdPrivateKey().valueWithNaturalBytes(),
       "pwSmRvZy1c55Kb5tCpBZyq41noSmPn7ynFzUHu1MaoGLAP1VfrT"
     );
   }
@@ -73,7 +73,7 @@ public void encodeDecodeEcNodePrivate() {
     testEncodeDecode(
       prefixedNodePrivate -> privateKeyCodec.encodeNodePrivateKey(prefixedNodePrivate),
       prefixedNodePrivate -> privateKeyCodec.decodeNodePrivateKey(prefixedNodePrivate),
-      TestConstants.EC_PRIVATE_KEY.valueWithNaturalBytes(),
+      TestConstants.getEcPrivateKey().valueWithNaturalBytes(),
       "pa1UHARsPMiuDqrJLwFhzcJokoHgyiuaxgPhUGYhkG5ArCfG2vt"
     );
   }
@@ -83,7 +83,7 @@ public void encodeDecodeEcAccountPrivateKey() {
     testEncodeDecode(
       prefixedAccountPrivateKey -> privateKeyCodec.encodeAccountPrivateKey(prefixedAccountPrivateKey),
       prefixedNodePrivateKey -> privateKeyCodec.decodeAccountPrivateKey(prefixedNodePrivateKey),
-      TestConstants.EC_PRIVATE_KEY.valueWithNaturalBytes(),
+      TestConstants.getEcPrivateKey().valueWithNaturalBytes(),
       "pwkgeQKfaDDMV7w59LhhuEWMbpX3HG2iXxXGgZuij2j6z1RYY7n"
     );
   }

xrpl4j-core/src/test/java/org/xrpl/xrpl4j/crypto/TestConstants.java

@@ -39,11 +39,13 @@ public interface TestConstants {
 
   String ED_PRIVATE_KEY_HEX = "B224AFDCCEC7AA4E245E35452585D4FBBE37519BCA3929578BFC5BBD4640E163";
   String ED_PRIVATE_KEY_WITH_PREFIX_HEX = "ED" + ED_PRIVATE_KEY_HEX;
-  String ED_PRIVATE_KEY_B58 = "UzQrAxCr8oeMRzzm6FFVJao8xkFc3g2ZQBs5GNBWRhZg";
-  PrivateKey ED_PRIVATE_KEY = PrivateKey.fromNaturalBytes(
-    UnsignedByteArray.of(BaseEncoding.base16().decode(ED_PRIVATE_KEY_HEX)),
-    KeyType.ED25519
-  );
+
+  static PrivateKey getEdPrivateKey() {
+    return PrivateKey.fromNaturalBytes(
+      UnsignedByteArray.of(BaseEncoding.base16().decode(ED_PRIVATE_KEY_HEX)),
+      KeyType.ED25519
+    );
+  }
 
   // Secp256k1 Public Key
   String EC_PUBLIC_KEY_HEX = "027535A4E90B2189CF9885563F45C4F454B3BFAB21930089C3878A9427B4D648D9";
@@ -52,11 +54,12 @@ public interface TestConstants {
 
   String EC_PRIVATE_KEY_HEX = "DAD3C2B4BF921398932C889DE5335F89D90249355FC6FFB73F1256D2957F9F17";
   String EC_PRIVATE_KEY_WITH_PREFIX_HEX = "00" + EC_PRIVATE_KEY_HEX;
-  String EC_PRIVATE_KEY_WITH_PREFIX_B58 = "rEjDwJp2Pm3NrUtcf8v17jWopvqPJxyi5RTrDfhcJcWSi";
 
-  PrivateKey EC_PRIVATE_KEY = PrivateKey.fromNaturalBytes(
-    UnsignedByteArray.of(BaseEncoding.base16().decode(EC_PRIVATE_KEY_HEX)), KeyType.SECP256K1
-  );
+  static PrivateKey getEcPrivateKey() {
+    return PrivateKey.fromNaturalBytes(
+      UnsignedByteArray.of(BaseEncoding.base16().decode(EC_PRIVATE_KEY_HEX)), KeyType.SECP256K1
+    );
+  }
 
   // Both generated from Passphrase.of("hello")
   Address ED_ADDRESS = Address.of("rwGWYtRR6jJJJq7FKQg74YwtkiPyUqJ466");