fix: defensively copy UnsignedByte instances in UnsignedByteArray

xrpl4j-core/src/main/java/org/xrpl/xrpl4j/codec/addresses/UnsignedByteArray.java

@@ -40,9 +40,58 @@ public class UnsignedByteArray implements Destroyable {
   private final List<UnsignedByte> unsignedBytes;
   private boolean destroyed;
 
+  /**
+   * Required-args constructor.
+   *
+   * <p>Note: {@link UnsignedByte} is mutable (see {@link UnsignedByte#destroy()}), so this constructor defensively
+   * copies both the supplied {@link List} and every {@link UnsignedByte} in it. This guarantees that destroying this
+   * array can never mutate an {@link UnsignedByte} owned by the caller (e.g., a shared static constant).
+   *
+   * @param unsignedBytes A {@link List} of {@link UnsignedByte}s.
+   */
   public UnsignedByteArray(final List<UnsignedByte> unsignedBytes) {
     Objects.requireNonNull(unsignedBytes);
-    this.unsignedBytes = unsignedBytes;
+    this.unsignedBytes = copyOf(unsignedBytes);
+  }
+
+  /**
+   * Marker used to select the non-copying private constructor. Exists only to make call sites self-documenting about
+   * the fact that the constructor takes ownership of (and does not copy) the supplied {@link List}.
+   */
+  private enum Ownership {
+    /**
+     * The caller guarantees that the supplied {@link List} and every {@link UnsignedByte} in it were freshly allocated
+     * and are not shared with anyone else.
+     */
+    TAKE
+  }
+
+  /**
+   * Private constructor that takes ownership of {@code unsignedBytes} without copying. Only for use by
+   * factories in this class that have just allocated fresh {@link UnsignedByte} instances.
+   *
+   * @param unsignedBytes A freshly allocated {@link List} of freshly allocated {@link UnsignedByte}s.
+   * @param ownership     An {@link Ownership} marker that distinguishes this constructor from the public (copying) one
+   *                      and documents the caller's intent.
+   */
+  private UnsignedByteArray(final List<UnsignedByte> unsignedBytes, final Ownership ownership) {
+    Objects.requireNonNull(ownership);
+    this.unsignedBytes = Objects.requireNonNull(unsignedBytes);
+  }
+
+  /**
+   * Deep-copies a {@link List} of {@link UnsignedByte}s so the result shares no instances with the input.
+   *
+   * @param source The {@link List} to copy.
+   *
+   * @return A new {@link List} containing new {@link UnsignedByte} instances with equal values.
+   */
+  private static List<UnsignedByte> copyOf(final List<UnsignedByte> source) {
+    final List<UnsignedByte> copy = new ArrayList<>(source.size());
+    for (UnsignedByte unsignedByte : source) {
+      copy.add(UnsignedByte.of(unsignedByte));
+    }
+    return copy;
   }
 
   /**
@@ -59,22 +108,25 @@ public static UnsignedByteArray of(final byte[] bytes) {
     for (int i = 0; i < bytes.length; i++) {
       unsignedBytes.add(i, UnsignedByte.of(bytes[i]));
     }
-    return new UnsignedByteArray(unsignedBytes);
+    return new UnsignedByteArray(unsignedBytes, Ownership.TAKE); // <-- all instances freshly allocated above.
   }
 
   /**
-   * Creates an {@link UnsignedByteArray} from one or more {@link UnsignedByte} values.
+   * Creates an {@link UnsignedByteArray} from one or more {@link UnsignedByte} values. The supplied values are copied,
+   * so the returned array never aliases the caller's instances.
    *
    * @param first An {@link UnsignedByte} to include in the resulting {@link UnsignedByteArray}.
    * @param rest  Other {@link UnsignedByte}s to include in the resulting {@link UnsignedByteArray}.
    *
    * @return An {@link UnsignedByteArray} with the given {@link UnsignedByte}s.
    */
   public static UnsignedByteArray of(UnsignedByte first, UnsignedByte... rest) {
-    List<UnsignedByte> unsignedBytes = new ArrayList<>();
-    unsignedBytes.add(first);
-    Collections.addAll(unsignedBytes, rest);
-    return new UnsignedByteArray(unsignedBytes);
+    List<UnsignedByte> unsignedBytes = new ArrayList<>(1 + rest.length);
+    unsignedBytes.add(UnsignedByte.of(first));
+    for (UnsignedByte unsignedByte : rest) {
+      unsignedBytes.add(UnsignedByte.of(unsignedByte));
+    }
+    return new UnsignedByteArray(unsignedBytes, Ownership.TAKE); // <-- all instances freshly copied above.
   }
 
   /**
@@ -83,7 +135,7 @@ public static UnsignedByteArray of(UnsignedByte first, UnsignedByte... rest) {
    * @return An empty {@link UnsignedByteArray}.
    */
   public static UnsignedByteArray empty() {
-    return new UnsignedByteArray(new ArrayList<>());
+    return new UnsignedByteArray(new ArrayList<>(), Ownership.TAKE);
   }
 
   /**
@@ -94,7 +146,7 @@ public static UnsignedByteArray empty() {
    * @return An {@link UnsignedByteArray} of the requested size with all 0 {@link UnsignedByte}s
    */
   public static UnsignedByteArray ofSize(int size) {
-    return new UnsignedByteArray(fill(size));
+    return new UnsignedByteArray(fill(size), Ownership.TAKE); // <-- `fill` allocates fresh instances.
   }
 
   /**
@@ -107,7 +159,7 @@ public static UnsignedByteArray ofSize(int size) {
   public static UnsignedByteArray fromHex(String hex) {
     Objects.requireNonNull(hex);
     List<UnsignedByte> unsignedBytes = ByteUtils.parse(hex.toUpperCase(Locale.ENGLISH));
-    return new UnsignedByteArray(unsignedBytes);
+    return new UnsignedByteArray(unsignedBytes, Ownership.TAKE); // <-- `parse` allocates fresh instances.
   }
 
   protected static List<UnsignedByte> fill(int amount) {
@@ -172,38 +224,40 @@ public UnsignedByte get(int index) {
   }
 
   /**
-   * Appends an {@link UnsignedByte} to this {@link UnsignedByteArray}.
+   * Appends a copy of an {@link UnsignedByte} to this {@link UnsignedByteArray}. Note: this method mutates the instance
+   * and returns the same instance (mainly for call chaining convenience).
    *
-   * @param unsignedByte An {@link UnsignedByte} to append.
+   * @param unsignedByte An {@link UnsignedByte} to append (copied; the caller's instance is never aliased).
    *
    * @return This {@link UnsignedByteArray}, with the given {@link UnsignedByte} appended.
    */
   public UnsignedByteArray append(UnsignedByte unsignedByte) {
-    unsignedBytes.add(unsignedByte);
+    unsignedBytes.add(UnsignedByte.of(unsignedByte));
     return this;
   }
 
   /**
-   * Appends the given bytes to the end of this array. Note: this method mutates the instance and returns the same
-   * instance (mainly for call chaining convenience).
+   * Appends copies of the given bytes to the end of this array. Note: this method mutates the instance and returns the
+   * same instance (mainly for call chaining convenience). The elements of {@code array} are copied, so the two arrays
+   * never share {@link UnsignedByte} instances.
    *
    * @param array An {@link UnsignedByteArray} to append to this {@link UnsignedByteArray}.
    *
    * @return the same instance.
    */
   public UnsignedByteArray append(UnsignedByteArray array) {
-    unsignedBytes.addAll(array.getUnsignedBytes());
+    unsignedBytes.addAll(copyOf(array.getUnsignedBytes()));
     return this;
   }
 
   /**
-   * Sets the value at the given index to the given {@link UnsignedByte}.
+   * Sets the value at the given index to a copy of the given {@link UnsignedByte}.
    *
    * @param index The index to set.
-   * @param value The {@link UnsignedByte} to set at the given index.
+   * @param value The {@link UnsignedByte} to set at the given index (copied; the caller's instance is never aliased).
    */
   public void set(int index, UnsignedByte value) {
-    unsignedBytes.set(index, value);
+    unsignedBytes.set(index, UnsignedByte.of(value));
   }
 
   /**
@@ -212,10 +266,10 @@ public void set(int index, UnsignedByte value) {
    * @param startIndex start index (inclusive)
    * @param endIndex   end index (exclusive)
    *
-   * @return An {@link UnsignedByteArray} containing the sliced elements.
+   * @return A new, independent {@link UnsignedByteArray} containing copies of the sliced elements.
    */
   public UnsignedByteArray slice(int startIndex, int endIndex) {
-    return new UnsignedByteArray(unsignedBytes.subList(startIndex, endIndex));
+    return new UnsignedByteArray(unsignedBytes.subList(startIndex, endIndex)); // <-- public ctor copies.
   }
 
   /**
@@ -226,9 +280,9 @@ public UnsignedByteArray slice(int startIndex, int endIndex) {
    */
   @Beta
   public UnsignedByteArray reverse() {
-    List<UnsignedByte> reversed = new ArrayList<>(unsignedBytes);
+    List<UnsignedByte> reversed = copyOf(unsignedBytes);
     Collections.reverse(reversed);
-    return new UnsignedByteArray(reversed);
+    return new UnsignedByteArray(reversed, Ownership.TAKE); // <-- `copyOf` allocates fresh instances.
   }
 
   @Override

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

@@ -223,4 +223,93 @@ void destroy() {
     assertThat(uba.hexValue()).isEqualTo("");
     assertThat(uba.hashCode()).isEqualTo(32);
   }
+  /////////
+  // Defensive copying: an UnsignedByteArray must never alias UnsignedByte instances it did not create, because
+  // UnsignedByte is mutable (destroy() zeroes it) and callers may pass shared/static constants.
+  /////////
+
+  @Test
+  void constructorCopiesListAndElements() {
+    UnsignedByte shared = UnsignedByte.of(0xED);
+    List<UnsignedByte> list = new ArrayList<>();
+    list.add(shared);
+    UnsignedByteArray array = new UnsignedByteArray(list);
+
+    assertThat(array.get(0)).isNotSameAs(shared);
+    array.destroy();
+    assertThat(shared.asInt()).isEqualTo(0xED);
+    assertThat(shared.isDestroyed()).isFalse();
+    assertThat(list).containsExactly(shared); // caller's list untouched
+  }
+
+  @Test
+  void ofUnsignedBytesCopiesElements() {
+    UnsignedByte first = UnsignedByte.of(0xED);
+    UnsignedByte second = UnsignedByte.of(0x01);
+    UnsignedByteArray array = UnsignedByteArray.of(first, second);
+
+    assertThat(array.get(0)).isNotSameAs(first);
+    assertThat(array.get(1)).isNotSameAs(second);
+    array.destroy();
+    assertThat(first.asInt()).isEqualTo(0xED);
+    assertThat(first.isDestroyed()).isFalse();
+    assertThat(second.asInt()).isEqualTo(0x01);
+    assertThat(second.isDestroyed()).isFalse();
+  }
+
+  @Test
+  void appendUnsignedByteCopiesElement() {
+    UnsignedByte shared = UnsignedByte.of(0xED);
+    UnsignedByteArray array = UnsignedByteArray.empty().append(shared);
+
+    assertThat(array.get(0)).isNotSameAs(shared);
+    array.destroy();
+    assertThat(shared.asInt()).isEqualTo(0xED);
+    assertThat(shared.isDestroyed()).isFalse();
+  }
+
+  @Test
+  void appendUnsignedByteArrayCopiesElements() {
+    UnsignedByteArray source = UnsignedByteArray.of(new byte[] {(byte) 0xED, 0x01});
+    UnsignedByteArray array = UnsignedByteArray.empty().append(source);
+
+    assertThat(array.get(0)).isNotSameAs(source.get(0));
+    array.destroy();
+    assertThat(source.hexValue()).isEqualTo("ED01");
+    assertThat(source.get(0).isDestroyed()).isFalse();
+    assertThat(source.isDestroyed()).isFalse();
+  }
+
+  @Test
+  void setCopiesElement() {
+    UnsignedByte shared = UnsignedByte.of(0xED);
+    UnsignedByteArray array = UnsignedByteArray.ofSize(1);
+    array.set(0, shared);
+
+    assertThat(array.get(0)).isNotSameAs(shared);
+    array.destroy();
+    assertThat(shared.asInt()).isEqualTo(0xED);
+    assertThat(shared.isDestroyed()).isFalse();
+  }
+
+  @Test
+  void sliceDoesNotAliasParent() {
+    UnsignedByteArray parent = UnsignedByteArray.of(new byte[] {0, 8, 9, 1});
+    UnsignedByteArray slice = parent.slice(1, 3);
+
+    slice.destroy();
+    assertThat(parent.hexValue()).isEqualTo("00080901");
+    assertThat(parent.length()).isEqualTo(4);
+    assertThat(parent.isDestroyed()).isFalse();
+  }
+
+  @Test
+  void reverseDoesNotAliasSource() {
+    UnsignedByteArray source = UnsignedByteArray.of(new byte[] {1, 2});
+    UnsignedByteArray reversed = source.reverse();
+
+    reversed.destroy();
+    assertThat(source.hexValue()).isEqualTo("0102");
+    assertThat(source.isDestroyed()).isFalse();
+  }
 }

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

@@ -0,0 +1,32 @@
+package org.xrpl.xrpl4j.codec.addresses;
+
+import java.lang.reflect.Field;
+
+/**
+ * Test-only helpers for {@link UnsignedByte}.
+ */
+public final class UnsignedByteTestUtils {
+
+  private UnsignedByteTestUtils() {
+  }
+
+  /**
+   * Forcibly resets a (possibly shared/static) {@link UnsignedByte} to the supplied value and clears its destroyed
+   * flag. Used as a safety net so that a regression in defensive copying cannot poison every other test in the fork.
+   *
+   * @param target The {@link UnsignedByte} to reset.
+   * @param value  The value to restore.
+   */
+  public static void reset(final UnsignedByte target, final int value) {
+    try {
+      Field valueField = UnsignedByte.class.getDeclaredField("value");
+      valueField.setAccessible(true);
+      valueField.setInt(target, value);
+      Field destroyedField = UnsignedByte.class.getDeclaredField("destroyed");
+      destroyedField.setAccessible(true);
+      destroyedField.setBoolean(target, false);
+    } catch (ReflectiveOperationException e) {
+      throw new IllegalStateException(e);
+    }
+  }
+}

xrpl4j-core/src/test/java/org/xrpl/xrpl4j/codec/binary/types/IssueTypeTest.java

@@ -164,4 +164,19 @@ private void assertInvalidMptId(String invalidId) {
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessageContaining("mpt_issuance_id must be a 48-character hexadecimal string");
   }
+
+  /**
+   * Regression: MPT issues embed the static ACCOUNT_ONE bytes. Destroying one serialized value must not corrupt the
+   * shared constant and thereby every later MPT serialization.
+   */
+  @Test
+  void destroyingMptIssueValueDoesNotCorruptLaterSerializations() throws JsonProcessingException {
+    JsonNode mptIssue = objectMapper.createObjectNode()
+      .set("mpt_issuance_id", new TextNode("00000002AE123A8556F3CF91154711376AFB0F894F832B3D"));
+
+    String expectedHex = codec.fromJson(mptIssue).toHex();
+    codec.fromJson(mptIssue).value().destroy();
+
+    assertThat(codec.fromJson(mptIssue).toHex()).isEqualTo(expectedHex);
+  }
 }