Update per PR

XRPLF · Oct 5, 2023 · 408012b · 408012b
1 parent c4b5c89
commit 408012b
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 32 deletions.
diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/crypto/keys/bc/BcKeyUtils.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/crypto/keys/bc/BcKeyUtils.java
@@ -236,9 +236,9 @@ public static ECPrivateKeyParameters toEcPrivateKeyParams(final PrivateKey priva
     Objects.requireNonNull(privateKey);
     Preconditions.checkArgument(privateKey.keyType() == KeyType.SECP256K1, "KeyType must be SECP256K1");
 
-    final BigInteger privateKeyInt = new BigInteger(
-      BaseEncoding.base16().encode(privateKey.naturalBytes().toByteArray()), 16
-    );
-    return new ECPrivateKeyParameters(privateKeyInt, BcKeyUtils.PARAMS);
+    // From http://www.secg.org/sec1-v2.pdf: A PrivateKey consists of an elliptic curve secret key `d` which is an
+    // integer in the interval [1, n − 1]. Therefore, it is safe to assume that the signum below should always be 1.
+    final BigInteger secretKeyD = new BigInteger(1, privateKey.naturalBytes().toByteArray());
+    return new ECPrivateKeyParameters(secretKeyD, BcKeyUtils.PARAMS);
   }
 }
diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/crypto/signing/bc/BcSignatureService.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/crypto/signing/bc/BcSignatureService.java
@@ -94,27 +94,21 @@ protected synchronized Signature edDsaSign(
     Objects.requireNonNull(privateKey);
     Objects.requireNonNull(signableTransactionBytes);
 
-    final byte[] privateKeyBytes = new byte[32];
-    try {
-      System.arraycopy(privateKey.naturalBytes().toByteArray(), 0, privateKeyBytes, 0, 32);
-      Ed25519PrivateKeyParameters privateKeyParameters = new Ed25519PrivateKeyParameters(privateKeyBytes, 0);
-
-      final byte[] signableBytes = signableTransactionBytes.toByteArray();
-
-      ed25519Signer.reset();
-      ed25519Signer.init(true, privateKeyParameters);
-      ed25519Signer.update(signableTransactionBytes.toByteArray(), 0, signableBytes.length);
-
-      final UnsignedByteArray sigBytes = UnsignedByteArray.of(ed25519Signer.generateSignature());
-      return Signature.builder()
-        .value(sigBytes)
-        .build();
-    } finally {
-      // Clear out the copied array, which was only used for signing.
-      for (int i = 0; i < 32; i++) {
-        privateKeyBytes[i] = (byte) 0;
-      }
-    }
+    final Ed25519PrivateKeyParameters privateKeyParameters = BcKeyUtils.toEd25519PrivateKeyParams(privateKey);
+
+    final byte[] signableBytes = signableTransactionBytes.toByteArray();
+
+    ed25519Signer.reset();
+    ed25519Signer.init(true, privateKeyParameters);
+    ed25519Signer.update(signableBytes, 0, signableBytes.length);
+
+    final UnsignedByteArray sigBytes = UnsignedByteArray.of(ed25519Signer.generateSignature());
+    return Signature.builder()
+      .value(sigBytes)
+      .build();
+
+    // Note: Ed25519PrivateKeyParameters does not provide a destroy function, but it will be eligible for cleanup (in
+    // the next GC) once this function exits.
   }
 
   @SuppressWarnings("checkstyle:LocalVariableName")
@@ -125,12 +119,9 @@ protected synchronized Signature ecDsaSign(final PrivateKey privateKey, final Un
 
     final UnsignedByteArray messageHash = HashingUtils.sha512Half(transactionBytes);
 
-    // From http://www.secg.org/sec1-v2.pdf:  consists of an elliptic curve secret key `d` which is an integer in
-    // the interval [1, n − 1]
-    final BigInteger secretKeyD = new BigInteger(privateKey.prefixedBytes().toByteArray());
-    final ECPrivateKeyParameters parameters = new ECPrivateKeyParameters(secretKeyD, BcKeyUtils.PARAMS);
+    final ECPrivateKeyParameters ecPrivateKeyParams = BcKeyUtils.toEcPrivateKeyParams(privateKey);
 
-    ecdsaSigner.init(true, parameters);
+    ecdsaSigner.init(true, ecPrivateKeyParams);
     final BigInteger[] signatures = ecdsaSigner.generateSignature(messageHash.toByteArray());
     final BigInteger r = signatures[0];
     BigInteger s = signatures[1];

diff --git a/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/crypto/keys/SeedTest.java b/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/crypto/keys/SeedTest.java
@@ -226,10 +226,10 @@ void verifyBigIntegerConstructionForSecp256k1PrivateKeys() {
       BaseEncoding.base16().decode("53AC3F62A5A6E598C7D1E31AB92587C56823A1BE5C21E53ABE9D9A722E5236")
     );
     BigInteger unPaddedBitInt = new BigInteger(
-      ecSeedFor31BytePrivateKey.deriveKeyPair().privateKey().naturalBytes().toByteArray()
+      1, ecSeedFor31BytePrivateKey.deriveKeyPair().privateKey().naturalBytes().toByteArray()
     );
     BigInteger paddedBitInt = new BigInteger(
-      ecSeedFor31BytePrivateKey.deriveKeyPair().privateKey().prefixedBytes().toByteArray()
+      1, ecSeedFor31BytePrivateKey.deriveKeyPair().privateKey().prefixedBytes().toByteArray()
     );
 
     assertThat(shortBigInt).isEqualTo(unPaddedBitInt);