Skip to content

Commit

Permalink
Refactor EthPublicKeyUtils to convert public key between Java, BC and…
Browse files Browse the repository at this point in the history 
… Web3J libraries (Consensys#1037)

* Refactor EthPublicKeyUtils to convert public key between Java, Bouncycastle and Web3J libraries
* Use static init in EthAccountsResultProviderTest
* Simplify SecureRandom usage in EthPublicKeyUtil
* Refactor EthPublicKeyUtils
* Refactor Eth1SignatureUtilTest
  • Loading branch information
@usmansaleem
usmansaleem authored Nov 22, 2024
1 parent c15412e commit 90dba64
Show file tree
Hide file tree
Showing 17 changed files with 462 additions and 207 deletions.
3 changes: 2 additions & 1 deletion ...java/tech/pegasys/web3signer/tests/bulkloading/SecpV3KeystoresBulkLoadAcceptanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ static void initV3Keystores() throws IOException, GeneralSecurityException, Ciph
publicKeys = new ArrayList<>();
for (int i = 0; i < 4; i++) {
final ECKeyPair ecKeyPair = Keys.createEcKeyPair();
final ECPublicKey ecPublicKey = EthPublicKeyUtils.createPublicKey(ecKeyPair.getPublicKey());
final ECPublicKey ecPublicKey =
EthPublicKeyUtils.web3JPublicKeyToECPublicKey(ecKeyPair.getPublicKey());
final String publicKeyHex =
IdentifierUtils.normaliseIdentifier(EthPublicKeyUtils.toHexString(ecPublicKey));
publicKeys.add(publicKeyHex);
Expand Down
2 changes: 1 addition & 1 deletion ...sts/src/test/java/tech/pegasys/web3signer/tests/publickeys/YubiHsmKeysAcceptanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private void createConfigurationFiles(final Set<Integer> opaqueDataIds, final Ke
private String getPublicKey(final String key) {
return normaliseIdentifier(
EthPublicKeyUtils.toHexString(
EthPublicKeyUtils.createPublicKey(
EthPublicKeyUtils.web3JPublicKeyToECPublicKey(
Credentials.create(key).getEcKeyPair().getPublicKey())));
}
}
5 changes: 3 additions & 2 deletions ...-tests/src/test/java/tech/pegasys/web3signer/tests/signing/SecpSigningAcceptanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,14 @@ private void signAndVerifySignature(final String publicKeyHex) {

void verifySignature(final Bytes signature, final String publicKeyHex) {
final ECPublicKey expectedPublicKey =
EthPublicKeyUtils.createPublicKey(Bytes.fromHexString(publicKeyHex));
EthPublicKeyUtils.bytesToECPublicKey(Bytes.fromHexString(publicKeyHex));

final byte[] r = signature.slice(0, 32).toArray();
final byte[] s = signature.slice(32, 32).toArray();
final byte[] v = signature.slice(64).toArray();
final BigInteger messagePublicKey = recoverPublicKey(new SignatureData(v, r, s));
assertThat(EthPublicKeyUtils.createPublicKey(messagePublicKey)).isEqualTo(expectedPublicKey);
assertThat(EthPublicKeyUtils.web3JPublicKeyToECPublicKey(messagePublicKey))
.isEqualTo(expectedPublicKey);
}

private BigInteger recoverPublicKey(final SignatureData signature) {
Expand Down
7 changes: 2 additions & 5 deletions core/src/main/java/tech/pegasys/web3signer/core/Eth1AddressSignerIdentifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package tech.pegasys.web3signer.core;

import static org.web3j.crypto.Keys.getAddress;
import static tech.pegasys.web3signer.signing.secp256k1.EthPublicKeyUtils.ecPublicKeyToWeb3JPublicKey;
import static tech.pegasys.web3signer.signing.secp256k1.EthPublicKeyUtils.toHexString;
import static tech.pegasys.web3signer.signing.secp256k1.util.AddressUtil.remove0xPrefix;

Expand All @@ -31,11 +32,7 @@ public Eth1AddressSignerIdentifier(final String address) {
}

public static SignerIdentifier fromPublicKey(final ECPublicKey publicKey) {
return new Eth1AddressSignerIdentifier(getAddress(toHexString(publicKey)));
}

public static SignerIdentifier fromPublicKey(final String publicKey) {
return new Eth1AddressSignerIdentifier(getAddress(publicKey));
return new Eth1AddressSignerIdentifier(getAddress(ecPublicKeyToWeb3JPublicKey(publicKey)));
}

@Override
Expand Down
70 changes: 56 additions & 14 deletions core/src/test/java/tech/pegasys/web3signer/core/service/Eth1AddressSignerIdentifierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,53 +15,95 @@
import static org.assertj.core.api.Assertions.assertThat;

import tech.pegasys.web3signer.core.Eth1AddressSignerIdentifier;
import tech.pegasys.web3signer.core.util.PublicKeyUtils;
import tech.pegasys.web3signer.signing.secp256k1.EthPublicKeyUtils;
import tech.pegasys.web3signer.signing.secp256k1.SignerIdentifier;
import tech.pegasys.web3signer.signing.secp256k1.util.AddressUtil;

import java.security.KeyPair;
import java.security.interfaces.ECPublicKey;
import java.util.Locale;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.web3j.crypto.Keys;

class Eth1AddressSignerIdentifierTest {
private static KeyPair secp256k1KeyPair;
private static KeyPair secp256k1KeyPair2;

@BeforeAll
static void generateKeyPair() {
secp256k1KeyPair = EthPublicKeyUtils.generateK256KeyPair();
secp256k1KeyPair2 = EthPublicKeyUtils.generateK256KeyPair();
}

@Test
void prefixIsRemovedFromAddress() {
final Eth1AddressSignerIdentifier signerIdentifier = new Eth1AddressSignerIdentifier("0xAb");
assertThat(signerIdentifier.toStringIdentifier()).isEqualTo("ab");
// web3j.crypto.Keys.getAddress() returns lower case address without 0x prefix
final String address =
Keys.getAddress(
EthPublicKeyUtils.ecPublicKeyToWeb3JPublicKey(
(ECPublicKey) secp256k1KeyPair.getPublic()));
// forcefully convert first two alphabets to uppercase and add prefix
final String mixCaseAddress = "0X" + convertHexToMixCase(address);

final Eth1AddressSignerIdentifier signerIdentifier =
new Eth1AddressSignerIdentifier(mixCaseAddress);
assertThat(signerIdentifier.toStringIdentifier()).isEqualTo(address);
assertThat(signerIdentifier.toStringIdentifier()).doesNotStartWithIgnoringCase("0x");
assertThat(signerIdentifier.toStringIdentifier()).isLowerCase();
}

@Test
void validateWorksForSamePrimaryKey() {
final ECPublicKey publicKey = PublicKeyUtils.createKeyFrom("0xab");
final ECPublicKey publicKey = (ECPublicKey) secp256k1KeyPair.getPublic();
final SignerIdentifier signerIdentifier = Eth1AddressSignerIdentifier.fromPublicKey(publicKey);
assertThat(signerIdentifier.validate(publicKey)).isTrue();
}

@Test
void validateFailsForDifferentPrimaryKey() {
final ECPublicKey publicKey = PublicKeyUtils.createKeyFrom("0xab");
final ECPublicKey publicKey = (ECPublicKey) secp256k1KeyPair.getPublic();
final SignerIdentifier signerIdentifier = Eth1AddressSignerIdentifier.fromPublicKey(publicKey);
assertThat(signerIdentifier.validate(PublicKeyUtils.createKeyFrom("0xbb"))).isFalse();
assertThat(signerIdentifier.validate((ECPublicKey) secp256k1KeyPair2.getPublic())).isFalse();
}

@Test
void validateFailsForNullPrimaryKey() {
final ECPublicKey publicKey = PublicKeyUtils.createKeyFrom("0xab");
final ECPublicKey publicKey = (ECPublicKey) secp256k1KeyPair.getPublic();
final SignerIdentifier signerIdentifier = Eth1AddressSignerIdentifier.fromPublicKey(publicKey);
assertThat(signerIdentifier.validate(null)).isFalse();
}

@Test
void correctEth1AddressIsGeneratedFromPublicKey() {
final ECPublicKey publicKey = PublicKeyUtils.createKeyFrom("0xab");
final ECPublicKey publicKey = (ECPublicKey) secp256k1KeyPair.getPublic();
final SignerIdentifier signerIdentifier = Eth1AddressSignerIdentifier.fromPublicKey(publicKey);
final String prefixRemovedAddress =
AddressUtil.remove0xPrefix(
Keys.getAddress(EthPublicKeyUtils.toHexString(publicKey)).toLowerCase(Locale.US));
assertThat(signerIdentifier.toStringIdentifier()).isEqualTo(prefixRemovedAddress);

// web3j.crypto.Keys.getAddress() returns lower case address without 0x prefix
final String expectedAddress =
Keys.getAddress(EthPublicKeyUtils.ecPublicKeyToWeb3JPublicKey(publicKey));
assertThat(signerIdentifier.toStringIdentifier()).isEqualTo(expectedAddress);
assertThat(signerIdentifier.toStringIdentifier()).doesNotStartWithIgnoringCase("0x");
assertThat(signerIdentifier.toStringIdentifier()).isLowerCase();
}

/**
* Converts first two alphabets to uppercase that can be used to test the case sensitivity of the
* address
*
* @param input address string in hex, assuming all characters are lowercase.
* @return address with first two alphabets converted to uppercase
*/
private static String convertHexToMixCase(final String input) {
final char[] chars = input.toCharArray();
int count = 0;

for (int i = 0; i < chars.length && count < 2; i++) {
if (Character.isLetter(chars[i]) && Character.isLowerCase(chars[i])) {
chars[i] = Character.toUpperCase(chars[i]);
count++;
}
}

return new String(chars);
}
}
61 changes: 27 additions & 34 deletions ...test/java/tech/pegasys/web3signer/core/service/jsonrpc/EthAccountsResultProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,31 @@
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import com.google.common.collect.Sets;
import org.apache.tuweni.bytes.Bytes;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.web3j.crypto.Keys;

@SuppressWarnings("unchecked")
public class EthAccountsResultProviderTest {

final ECPublicKey publicKeyA = createKeyFrom("A".repeat(128));
final ECPublicKey publicKeyB = createKeyFrom("B".repeat(128));
final ECPublicKey publicKeyC = createKeyFrom("C".repeat(128));

final String addressA = Keys.getAddress(EthPublicKeyUtils.toHexString(publicKeyA));
final String addressB = Keys.getAddress(EthPublicKeyUtils.toHexString(publicKeyB));
final String addressC = Keys.getAddress(EthPublicKeyUtils.toHexString(publicKeyC));

final ECPublicKey createKeyFrom(final String hexString) {
return EthPublicKeyUtils.createPublicKey(Bytes.fromHexString(hexString));
private static ECPublicKey publicKeyA;
private static ECPublicKey publicKeyB;
private static ECPublicKey publicKeyC;

private static String addressA;
private static String addressB;
private static String addressC;

@BeforeAll
static void init() {
publicKeyA = (ECPublicKey) EthPublicKeyUtils.generateK256KeyPair().getPublic();
publicKeyB = (ECPublicKey) EthPublicKeyUtils.generateK256KeyPair().getPublic();
publicKeyC = (ECPublicKey) EthPublicKeyUtils.generateK256KeyPair().getPublic();

addressA = Keys.getAddress(EthPublicKeyUtils.toHexString(publicKeyA));
addressB = Keys.getAddress(EthPublicKeyUtils.toHexString(publicKeyB));
addressC = Keys.getAddress(EthPublicKeyUtils.toHexString(publicKeyC));
}

@Test
Expand Down Expand Up @@ -103,9 +109,7 @@ public void missingParametersIsOk() {
final JsonRpcRequest request = new JsonRpcRequest("2.0", "eth_accounts");
request.setId(new JsonRpcRequestId(id));

final Object body = resultProvider.createResponseResult(request);
assertThat(body).isInstanceOf(List.class);
final List<String> addressses = (List<String>) body;
final List<String> addressses = resultProvider.createResponseResult(request);
assertThat(addressses).containsExactly("0x" + addressA);
}

Expand All @@ -120,10 +124,7 @@ public void multipleValueFromBodyProviderInsertedToResult() {
request.setId(new JsonRpcRequestId(id));
request.setParams(emptyList());

final Object body = resultProvider.createResponseResult(request);

assertThat(body).isInstanceOf(List.class);
final List<String> reportedAddresses = (List<String>) body;
final List<String> reportedAddresses = resultProvider.createResponseResult(request);
assertThat(reportedAddresses)
.containsExactlyInAnyOrder("0x" + addressA, "0x" + addressB, "0x" + addressC);
}
Expand All @@ -139,25 +140,19 @@ public void accountsReturnedAreDynamicallyFetchedFromProvider() {
request.setId(new JsonRpcRequestId(1));
request.setParams(emptyList());

Object body = resultProvider.createResponseResult(request);
assertThat(body).isInstanceOf(List.class);
List<String> reportedAddresses = (List<String>) body;
List<String> reportedAddresses = resultProvider.createResponseResult(request);
assertThat(reportedAddresses)
.containsExactlyElementsOf(
List.of("0x" + addressA, "0x" + addressB, "0x" + addressC).stream()
Stream.of("0x" + addressA, "0x" + addressB, "0x" + addressC)
.sorted()
.collect(Collectors.toList()));

addresses.remove(publicKeyA);

body = resultProvider.createResponseResult(request);
assertThat(body).isInstanceOf(List.class);
reportedAddresses = (List<String>) body;
reportedAddresses = resultProvider.createResponseResult(request);
assertThat(reportedAddresses)
.containsExactlyElementsOf(
List.of("0x" + addressB, "0x" + addressC).stream()
.sorted()
.collect(Collectors.toList()));
Stream.of("0x" + addressB, "0x" + addressC).sorted().collect(Collectors.toList()));
}

@Test
Expand All @@ -169,12 +164,10 @@ public void accountsReturnedAreSortedAlphabetically() {
request.setId(new JsonRpcRequestId(1));
request.setParams(emptyList());

final Object body = resultProvider.createResponseResult(request);
assertThat(body).isInstanceOf(List.class);
List<String> reportedAddresses = (List<String>) body;
List<String> reportedAddresses = resultProvider.createResponseResult(request);
assertThat(reportedAddresses)
.containsExactlyElementsOf(
List.of("0x" + addressA, "0x" + addressB, "0x" + addressC).stream()
Stream.of("0x" + addressA, "0x" + addressB, "0x" + addressC)
.sorted()
.collect(Collectors.toList()));
}
Expand Down
6 changes: 4 additions & 2 deletions ...va/tech/pegasys/web3signer/core/service/jsonrpc/EthSignTransactionResultProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ public void ifAddressIsNotUnlockedExceptionIsThrownWithSigningNotUnlocked() {
public void signatureHasTheExpectedFormat() {
final Credentials cs =
Credentials.create("0x1618fc3e47aec7e70451256e033b9edb67f4c469258d8e2fbb105552f141ae41");
final ECPublicKey key = EthPublicKeyUtils.createPublicKey(cs.getEcKeyPair().getPublicKey());
final ECPublicKey key =
EthPublicKeyUtils.web3JPublicKeyToECPublicKey(cs.getEcKeyPair().getPublicKey());
final String addr = Keys.getAddress(EthPublicKeyUtils.toHexString(key));

final BigInteger v = BigInteger.ONE;
Expand Down Expand Up @@ -169,7 +170,8 @@ public void returnsExpectedSignatureForEip1559Transaction() {
private String executeEthSignTransaction(final JsonObject params) {
final Credentials cs =
Credentials.create("0x1618fc3e47aec7e70451256e033b9edb67f4c469258d8e2fbb105552f141ae41");
final ECPublicKey key = EthPublicKeyUtils.createPublicKey(cs.getEcKeyPair().getPublicKey());
final ECPublicKey key =
EthPublicKeyUtils.web3JPublicKeyToECPublicKey(cs.getEcKeyPair().getPublicKey());
final String addr = Keys.getAddress(EthPublicKeyUtils.toHexString(key));

doAnswer(
Expand Down
27 changes: 0 additions & 27 deletions core/src/test/java/tech/pegasys/web3signer/core/util/PublicKeyUtils.java
View file

This file was deleted.

Loading

0 comments on commit 90dba64

Please sign in to comment.