XRPLF · sappenin · Oct 5, 2023 · Sep 28, 2023 · Sep 28, 2023 · Sep 28, 2023
@@ -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.
@@ -25,6 +25,7 @@
 
 import java.math.BigInteger;
 import java.util.Objects;
+
 import javax.security.auth.Destroyable;
 
 /**
@@ -42,6 +43,17 @@ private UnsignedByte(final int value) {
     this.value = value;
   }
 
+  /**
+   * Copy constructor. Constructs an {@link UnsignedByte} from an {@code UnsignedByte}.
+   *
+   * @param value An {@code int} value.
+   *
+   * @return An {@link UnsignedByte}.
+   */
+  public static UnsignedByte of(final UnsignedByte value) {
+    return new UnsignedByte(value.asInt());
+  }
+
   /**
    * Construct an {@link UnsignedByte} from an {@code int}.
    *
@@ -141,8 +153,8 @@ public boolean isNthBitSet(final int nth) {
   }
 
   /**
-   * Does a bitwise OR on this {@link UnsignedByte} and the given {@link UnsignedByte}, and returns a new {@link
-   * UnsignedByte} as the result.
+   * Does a bitwise OR on this {@link UnsignedByte} and the given {@link UnsignedByte}, and returns a new
+   * {@link UnsignedByte} as the result.
    *
    * @param unsignedByte The {@link UnsignedByte} to perform a bitwise OR on.
    *

@@ -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.
@@ -20,6 +20,7 @@
  * =========================LICENSE_END==================================
  */
 
+import com.google.common.base.Preconditions;
 import org.xrpl.xrpl4j.codec.addresses.KeyType;
 import org.xrpl.xrpl4j.codec.addresses.UnsignedByte;
 import org.xrpl.xrpl4j.codec.addresses.UnsignedByteArray;
@@ -32,11 +33,30 @@
 public class PrivateKey implements PrivateKeyable, javax.security.auth.Destroyable {
 
   /**
-   * Keys generated from the secp256k1 curve have 33 bytes in XRP Ledger. However, keys derived from the ed25519 curve
-   * have only 32 bytes, and so get prefixed with this HEX value so that all keys in the ledger are 33 bytes.
+   * @deprecated This value will be removed in a future version. Prefer {@link #ED2559_PREFIX} or
+   *   {@link #SECP256K1_PREFIX} instead.
    */
+  @Deprecated
   public static final UnsignedByte PREFIX = UnsignedByte.of(0xED);
 
+  /**
+   * Private keys (whether from the ed25519 or secp256k1 curves) have 32 bytes naturally. At the same time, secp256k1
+   * public keys have 33 bytes naturally, whereas ed25519 public keys have 32 bytes naturally. Because of this, in XRPL,
+   * ed25519 public keys are prefixed with a one-byte prefix (i.e., 0xED). For consistency, this library (and other XRPL
+   * tooling) also prepends all private keys with artificial prefixes (0xED for ed25519 or 0x00 for secp256k1). This
+   * value is the one-byte prefix for ed25519 keys.
+   */
+  public static final UnsignedByte ED2559_PREFIX = UnsignedByte.of(0xED);
+
+  /**
+   * Private keys (whether from the ed25519 or secp256k1 curves) have 32 bytes naturally. At the same time, secp256k1
+   * public keys have 33 bytes naturally, whereas ed25519 public keys have 32 bytes naturally. Because of this, in XRPL,
+   * ed25519 public keys are prefixed with a one-byte prefix (i.e., 0xED). For consistency, this library (and other XRPL
+   * tooling) also prepends all private keys with artificial prefixes (0xED for ed25519 or 0x00 for secp256k1).  This
+   * value is the one-byte prefix for secp256k1 keys.
+   */
+  public static final UnsignedByte SECP256K1_PREFIX = UnsignedByte.of(0x00);
+
   private final UnsignedByteArray value;
   private boolean destroyed;
 
@@ -48,6 +68,15 @@ public class PrivateKey implements PrivateKeyable, javax.security.auth.Destroyab
    * @return A {@link PrivateKey}.
    */
   public static PrivateKey of(final UnsignedByteArray value) {
+    Objects.requireNonNull(value);
+
+    final UnsignedByte firstByte = value.get(0);
+    Preconditions.checkArgument(
+      value.length() == 33 && (ED2559_PREFIX.equals(firstByte) || SECP256K1_PREFIX.equals(firstByte)),
+      "Constructing a PrivateKey with raw bytes requires a one-byte prefix in front of the 32 natural " +
+        "bytes of a private key. Use the prefix `0xED` for ed25519 private keys, or `0x00` for secp256k1 private keys."
+    );
+
     return new PrivateKey(value);
   }
 
@@ -69,14 +98,35 @@ public UnsignedByteArray value() {
     return UnsignedByteArray.of(value.toByteArray());
   }
 
+  /**
+   * Accessor for the byte value in {@link #value()} but in a more natural form (i.e., the size of the returned value
+   * will be 32 bytes). Natural ed25519 or secp256k1 private keys will ordinarily contain only 32 bytes. However, in
+   * XRPL, private keys are represented with a single-byte prefix (i.e., `0xED` for ed25519 and `0x00` for secp256k1
+   * keys).
+   *
+   * @return An instance of {@link UnsignedByteArray}.
+   */
+  public UnsignedByteArray valueWithoutPrefix() {
+    // 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).
+    return UnsignedByteArray.of(value.slice(1, 33).toByteArray());
+  }
+
   /**
    * The type of this key (either {@link KeyType#ED25519} or {@link KeyType#SECP256K1}).
    *
    * @return A {@link KeyType}.
    */
   public final KeyType keyType() {
     final UnsignedByte prefixByte = this.value().get(0);
-    return prefixByte.equals(PREFIX) ? KeyType.ED25519 : KeyType.SECP256K1;
+    if (ED2559_PREFIX.equals(prefixByte)) {
+      return KeyType.ED25519;
+    } else if (SECP256K1_PREFIX.equals(prefixByte)) {
+      return KeyType.SECP256K1;
+    } else {
+      throw new IllegalStateException("Prefix may only be 0xED or 0x00");
+    }
   }
 
   @Override

@@ -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.
@@ -172,6 +172,7 @@ static Seed secp256k1SeedFromEntropy(final Entropy entropy) {
    * @param base58EncodedSecret A base58-encoded {@link String} that represents an encoded seed.
    *
    * @return A {@link Seed}.
+   *
    * @see "https://xrpl.org/xrp-testnet-faucet.html"
    */
   static Seed fromBase58EncodedSecret(final Base58EncodedSecret base58EncodedSecret) {
@@ -313,8 +314,10 @@ public static KeyPair deriveKeyPair(final Seed seed) {
 
         Ed25519PublicKeyParameters publicKey = privateKey.generatePublicKey();
 
-        // XRPL ED25519 keys are prefixed with 0xED so that the keys are 33 bytes and match the length of secp256k1
-        // keys. Note that Bouncy Castle only deals with 32 byte keys, so we need to manually add the prefix
+        // Both ed25519 and secp256k1 _private_ keys are always 32 bytes long. However, in this library, both types of
+        // private key are prefixed with one byte (0xED for ed25519 and 0x00 for secp256k1) because this is what other
+        // libraries do, and also so that any particular set of 32 private key bytes can be properly disambiguated.
+        // See https://github.com/XRPLF/xrpl4j/issues/486 for more details.
         final UnsignedByte prefix = UnsignedByte.of(0xED);
         final UnsignedByteArray prefixedPrivateKey = UnsignedByteArray.of(prefix)
           .append(UnsignedByteArray.of(privateKey.getEncoded()));
@@ -386,11 +389,24 @@ private static KeyPair deriveKeyPair(final UnsignedByteArray seedBytes, final in
         final BigInteger privateKeyInt = derivePrivateKey(seedBytes, accountNumber);
         final UnsignedByteArray publicKeyInt = derivePublicKey(privateKeyInt);
 
+        // Both ed25519 and secp256k1 _private_ keys are always 32 bytes long. However, in this library, both types of
+        // private key are prefixed with one byte (0xED for ed25519 and 0x00 for secp256k1) because this is what other
+        // libraries do, and also so that any particular set of 32 private key bytes can be properly disambiguated.
+        // See https://github.com/XRPLF/xrpl4j/issues/486 for more details.
+        final UnsignedByteArray prefixedPrivateKey;
+        if (privateKeyInt.toByteArray().length == 32) {
+          prefixedPrivateKey = UnsignedByteArray.of(PrivateKey.SECP256K1_PREFIX)
+            .append(UnsignedByteArray.of(privateKeyInt.toByteArray()));
+        } else {
+          prefixedPrivateKey = UnsignedByteArray.of(privateKeyInt.toByteArray());
+        }
+
+        // No need to prefix secp256k1 public keys because they are always 33 bytes.
+        final UnsignedByteArray prefixedPublicKey = UnsignedByteArray.of(publicKeyInt.toByteArray());
+
         return KeyPair.builder()
-          .privateKey(PrivateKey.of(UnsignedByteArray.of(privateKeyInt.toByteArray())))
-          .publicKey(PublicKey.fromBase16EncodedPublicKey(
-            UnsignedByteArray.of(publicKeyInt.toByteArray()).hexValue()
-          ))
+          .privateKey(PrivateKey.of(prefixedPrivateKey))
+          .publicKey(PublicKey.fromBase16EncodedPublicKey(prefixedPublicKey.hexValue()))
           .build();
       }
 

@@ -85,7 +85,7 @@ public static PrivateKey toPrivateKey(final Ed25519PrivateKeyParameters ed25519P
 
     // XRPL ED25519 keys are prefixed with 0xED so that the keys are 33 bytes and match the length of sekp256k1 keys.
     // Bouncy Castle only deals with 32 byte keys, so we need to manually add the prefix
-    UnsignedByteArray prefixedPrivateKey = UnsignedByteArray.of(PrivateKey.PREFIX)
+    UnsignedByteArray prefixedPrivateKey = UnsignedByteArray.of(PrivateKey.ED2559_PREFIX)
       .append(UnsignedByteArray.of(ed25519PrivateKeyParameters.getEncoded()));
     return PrivateKey.of(prefixedPrivateKey);
   }
@@ -100,8 +100,10 @@ public static PrivateKey toPrivateKey(final Ed25519PrivateKeyParameters ed25519P
   public static PrivateKey toPrivateKey(final ECPrivateKeyParameters ecPrivateKeyParameters) {
     // Convert the HEX representation of the BigInteger into bytes.
     byte[] privateKeyBytes = BaseEncoding.base16().decode(ecPrivateKeyParameters.getD().toString(16).toUpperCase());
-    final UnsignedByteArray privateKeyUba = UnsignedByteArray.of(privateKeyBytes);
-    return PrivateKey.of(privateKeyUba);
+    return PrivateKey.of(
+      UnsignedByteArray.of(PrivateKey.SECP256K1_PREFIX) // <-- Per #486, always append 0x00 to secp256k1 private keys
+        .append(UnsignedByteArray.of(privateKeyBytes))
+    );
   }
 
   /**
@@ -134,7 +136,7 @@ public static PublicKey toPublicKey(final Ed25519PublicKeyParameters ed25519Publ
     Objects.requireNonNull(ed25519PublicKeyParameters);
     // XRPL ED25519 keys are prefixed with 0xED so that the keys are 33 bytes and match the length of sekp256k1 keys.
     // Bouncy Castle only deals with 32 byte keys, so we need to manually add the prefix
-    UnsignedByteArray prefixedPublicKey = UnsignedByteArray.of(PrivateKey.PREFIX)
+    UnsignedByteArray prefixedPublicKey = UnsignedByteArray.of(PrivateKey.ED2559_PREFIX)
       .append(UnsignedByteArray.of(ed25519PublicKeyParameters.getEncoded()));
 
     return PublicKey.builder()

@@ -37,7 +37,7 @@
 @Value.Immutable
 @JsonSerialize(as = ImmutableSignerListSet.class)
 @JsonDeserialize(as = ImmutableSignerListSet.class)
-public interface SignerListSet extends Transaction {
+public interface  SignerListSet extends Transaction {
 
   /**
    * Construct a builder for this class.

@@ -21,15 +21,27 @@
  */
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import org.junit.jupiter.api.Test;
+import org.mockito.internal.matchers.Null;
 
 import java.math.BigInteger;
 
+/**
+ * Unit tests for {@link UnsignedByte}.
+ */
 public class UnsignedByteTest {
 
   @Test
-  public void hexValue() {
+  void constructorTests() {
+    assertThrows(IllegalArgumentException.class, () -> UnsignedByte.of(-1));
+    assertThrows(NullPointerException.class, () -> UnsignedByte.of((UnsignedByte) null));
+    assertThat(UnsignedByte.of(UnsignedByte.of(0))).isEqualTo(UnsignedByte.of(0));
+  }
+
+  @Test
+  void hexValue() {
     assertThat(UnsignedByte.of(0).hexValue()).isEqualTo("00");
     assertThat(UnsignedByte.of(127).hexValue()).isEqualTo("7F");
     assertThat(UnsignedByte.of(128).hexValue()).isEqualTo("80");
@@ -38,23 +50,23 @@ public void hexValue() {
   }
 
   @Test
-  public void isNthBitSetAllZero() {
+  void isNthBitSetAllZero() {
     UnsignedByte value = UnsignedByte.of(0);
     for (int i = 1; i <= 8; i++) {
       assertThat(value.isNthBitSet(i)).isFalse();
     }
   }
 
   @Test
-  public void isNthBitSetAllSet() {
+  void isNthBitSetAllSet() {
     UnsignedByte value = UnsignedByte.of(new BigInteger("11111111", 2).intValue());
     for (int i = 1; i <= 8; i++) {
       assertThat(value.isNthBitSet(i)).isTrue();
     }
   }
 
   @Test
-  public void isNthBitSetEveryOther() {
+  void isNthBitSetEveryOther() {
     UnsignedByte value = UnsignedByte.of(new BigInteger("10101010", 2).intValue());
     assertThat(value.isNthBitSet(1)).isTrue();
     assertThat(value.isNthBitSet(2)).isFalse();
@@ -67,7 +79,7 @@ public void isNthBitSetEveryOther() {
   }
 
   @Test
-  public void intValue() {
+  void intValue() {
     assertThat(UnsignedByte.of(0x00).asInt()).isEqualTo(0);
     assertThat(UnsignedByte.of(0x0F).asInt()).isEqualTo(15);
     assertThat(UnsignedByte.of(0xFF).asInt()).isEqualTo(255);