Skip to content

Commit

Permalink
Add test coverage for java precompiles (#7446)
Browse files Browse the repository at this point in the history
For tests that have a native/java switch ensure that the java path gets
the same tests native paths do.

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Danno Ferrin <danno@numisight.com>
  • Loading branch information
shemnon and macfarla authored Aug 13, 2024
1 parent 1a91545 commit a55c331
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 51 deletions.
4 changes: 2 additions & 2 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -1503,7 +1503,7 @@ public static Optional<Boolean> getColorEnabled() {

private void configureNativeLibs() {
if (unstableNativeLibraryOptions.getNativeAltbn128()
&& AbstractAltBnPrecompiledContract.isNative()) {
&& AbstractAltBnPrecompiledContract.maybeEnableNative()) {
logger.info("Using the native implementation of alt bn128");
} else {
AbstractAltBnPrecompiledContract.disableNative();
Expand All @@ -1519,7 +1519,7 @@ private void configureNativeLibs() {
}

if (unstableNativeLibraryOptions.getNativeSecp()
&& SignatureAlgorithmFactory.getInstance().isNative()) {
&& SignatureAlgorithmFactory.getInstance().maybeEnableNative()) {
logger.info("Using the native implementation of the signature algorithm");
} else {
SignatureAlgorithmFactory.getInstance().disableNative();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,9 @@ public Optional<SECPPublicKey> recoverPublicKeyFromSignature(
final Bytes32 dataHash, final SECPSignature signature) {
final BigInteger publicKeyBI =
recoverFromSignature(signature.getRecId(), signature.getR(), signature.getS(), dataHash);
return Optional.of(SECPPublicKey.create(publicKeyBI, ALGORITHM));
return publicKeyBI == null
? Optional.empty()
: Optional.of(SECPPublicKey.create(publicKeyBI, ALGORITHM));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ public Blake2bfMessageDigest clone() throws CloneNotSupportedException {
/**
* Implementation of the `F` compression function of the Blake2b cryptographic hash function.
*
* <p>RFC - https://tools.ietf.org/html/rfc7693
* <p>RFC - <a href="https://tools.ietf.org/html/rfc7693">...</a>
*
* <p>Adapted from - https://github.com/keep-network/blake2b/blob/master/compression/f.go
* <p>Adapted from - <a
* href="https://github.com/keep-network/blake2b/blob/master/compression/f.go">...</a>
*
* <p>Optimized for 64-bit platforms
*/
Expand Down Expand Up @@ -93,12 +94,7 @@ public static class Blake2bfDigest implements Digest, Cloneable {
private static boolean useNative;

static {
try {
useNative = LibBlake2bf.ENABLED;
} catch (UnsatisfiedLinkError ule) {
LOG.info("blake2bf native precompile not available: {}", ule.getMessage());
useNative = false;
}
maybeEnableNative();
}

/** Instantiates a new Blake2bf digest. */
Expand Down Expand Up @@ -130,6 +126,21 @@ public Blake2bfDigest clone() throws CloneNotSupportedException {
return cloned;
}

/**
* Attempt to enable the native libreary
*
* @return true if the native library was successfully enabled.
*/
public static boolean maybeEnableNative() {
try {
useNative = LibBlake2bf.ENABLED;
} catch (UnsatisfiedLinkError | NoClassDefFoundError e) {
LOG.info("blake2bf native precompile not available: {}", e.getMessage());
useNative = false;
}
return useNative;
}

/** Disable native. */
public static void disableNative() {
useNative = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public void disableNative() {
*
* @return true if the native library was enabled.
*/
@Override
public boolean maybeEnableNative() {
try {
useNative = LibSecp256k1.CONTEXT != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,22 @@ public boolean isNative() {
return useNative;
}

/**
* Attempt to enable the native library for secp256r1
*
* @return true if the native library was enabled.
*/
@Override
public boolean maybeEnableNative() {
try {
useNative = BesuNativeEC.INSTANCE != null;
} catch (UnsatisfiedLinkError | NoClassDefFoundError e) {
LOG.info("Native secp256r1 not available - {}", e.getMessage());
useNative = false;
}
return useNative;
}

/**
* SECP256R1 is using the non-deterministic implementation of K calculation (standard)
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ public interface SignatureAlgorithm {
/** Disable native. */
void disableNative();

/**
* Attempt to enable the native library.
*
* @return true if the native library was enabled
*/
boolean maybeEnableNative();

/**
* Is native enabled.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public static String suiteName() {
}

@Test
public void recoverPublicKeyFromSignature() {
void recoverPublicKeyFromSignature() {
final SECPPrivateKey privateKey =
secp256R1.createPrivateKey(
new BigInteger("c85ef7d79691fe79573b1a7064c19c1a9819ebdbd1faaab1a8ec92344438aaf4", 16));
Expand All @@ -139,20 +139,20 @@ public void recoverPublicKeyFromSignature() {

final SECPPublicKey recoveredPublicKey =
secp256R1.recoverPublicKeyFromSignature(dataHash, signature).get();
assertThat(recoveredPublicKey.toString()).isEqualTo(keyPair.getPublicKey().toString());
assertThat(recoveredPublicKey).hasToString(keyPair.getPublicKey().toString());
}

@Test
public void signatureGenerationVerificationAndPubKeyRecovery() {
void signatureGenerationVerificationAndPubKeyRecovery() {
signTestVectors.forEach(
signTestVector -> {
final SECPPrivateKey privateKey =
secp256R1.createPrivateKey(new BigInteger(signTestVector.getPrivateKey(), 16));
final BigInteger publicKeyBigInt = new BigInteger(signTestVector.getPublicKey(), 16);
secp256R1.createPrivateKey(new BigInteger(signTestVector.privateKey(), 16));
final BigInteger publicKeyBigInt = new BigInteger(signTestVector.publicKey(), 16);
final SECPPublicKey publicKey = secp256R1.createPublicKey(publicKeyBigInt);
final KeyPair keyPair = secp256R1.createKeyPair(privateKey);

final Bytes32 dataHash = keccak256(Bytes.wrap(signTestVector.getData().getBytes(UTF_8)));
final Bytes32 dataHash = keccak256(Bytes.wrap(signTestVector.data().getBytes(UTF_8)));

final SECPSignature signature = secp256R1.sign(dataHash, keyPair);
assertThat(secp256R1.verify(dataHash, signature, publicKey)).isTrue();
Expand All @@ -165,44 +165,22 @@ public void signatureGenerationVerificationAndPubKeyRecovery() {
}

@Test
public void invalidFileThrowsInvalidKeyPairException() throws Exception {
void invalidFileThrowsInvalidKeyPairException() throws Exception {
final File tempFile = Files.createTempFile(suiteName(), ".keypair").toFile();
tempFile.deleteOnExit();
Files.write(tempFile.toPath(), "not valid".getBytes(UTF_8));
Files.writeString(tempFile.toPath(), "not valid");
assertThatThrownBy(() -> KeyPairUtil.load(tempFile))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
public void invalidMultiLineFileThrowsInvalidIdException() throws Exception {
void invalidMultiLineFileThrowsInvalidIdException() throws Exception {
final File tempFile = Files.createTempFile(suiteName(), ".keypair").toFile();
tempFile.deleteOnExit();
Files.write(tempFile.toPath(), "not\n\nvalid".getBytes(UTF_8));
Files.writeString(tempFile.toPath(), "not\n\nvalid");
assertThatThrownBy(() -> KeyPairUtil.load(tempFile))
.isInstanceOf(IllegalArgumentException.class);
}

private static class SignTestVector {
private final String privateKey;
private final String publicKey;
private final String data;

public SignTestVector(final String privateKey, final String publicKey, final String data) {
this.privateKey = privateKey;
this.publicKey = publicKey;
this.data = data;
}

public String getPrivateKey() {
return privateKey;
}

public String getPublicKey() {
return publicKey;
}

public String getData() {
return data;
}
}
private record SignTestVector(String privateKey, String publicKey, String data) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.apache.tuweni.bytes.Bytes;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

Expand All @@ -36,8 +38,14 @@ class AltBN128PairingPrecompiledContractTest {
private final AltBN128PairingPrecompiledContract istanbulContract =
AltBN128PairingPrecompiledContract.istanbul(gasCalculator);

@Test
void compute_validPoints() {
@ParameterizedTest
@ValueSource(booleans = {true, false})
void compute_validPoints(final boolean useNative) {
if (useNative) {
AbstractAltBnPrecompiledContract.maybeEnableNative();
} else {
AbstractAltBnPrecompiledContract.disableNative();
}
final Bytes input = validPointBytes();
final Bytes result = byzantiumContract.computePrecompile(input, messageFrame).getOutput();
assertThat(result).isEqualTo(AltBN128PairingPrecompiledContract.TRUE);
Expand Down Expand Up @@ -80,8 +88,14 @@ Bytes validPointBytes() {
return Bytes.concatenate(g1Point0, g2Point0, g1Point1, g2Point1);
}

@Test
void compute_invalidPointsOutsideSubgroupG2() {
@ParameterizedTest
@ValueSource(booleans = {true, false})
void compute_invalidPointsOutsideSubgroupG2(final boolean useNative) {
if (useNative) {
AbstractAltBnPrecompiledContract.maybeEnableNative();
} else {
AbstractAltBnPrecompiledContract.disableNative();
}
final Bytes g1Point0 =
Bytes.concatenate(
Bytes.fromHexString(
Expand Down Expand Up @@ -122,11 +136,13 @@ void compute_invalidPointsOutsideSubgroupG2() {

@Test
void gasPrice_byzantium() {
// gas calculation is java only
assertThat(byzantiumContract.gasRequirement(validPointBytes())).isEqualTo(260_000L);
}

@Test
void gasPrice_istanbul() {
// gas calculation is java only
assertThat(istanbulContract.gasRequirement(validPointBytes())).isEqualTo(113_000L);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

import org.hyperledger.besu.crypto.Blake2bfMessageDigest.Blake2bfDigest;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.gascalculator.PetersburgGasCalculator;

Expand Down Expand Up @@ -67,9 +68,24 @@ static Arguments[] parameters() {

@ParameterizedTest
@MethodSource("parameters")
void shouldRunFCompression(
void shouldRunFCompressionNative(
final String inputString, final String expectedResult, final long expectedGasUsed) {
Blake2bfDigest.maybeEnableNative();

testFCompression(inputString, expectedResult, expectedGasUsed);
}

@ParameterizedTest
@MethodSource("parameters")
void shouldRunFCompressionJava(
final String inputString, final String expectedResult, final long expectedGasUsed) {
Blake2bfDigest.disableNative();

testFCompression(inputString, expectedResult, expectedGasUsed);
}

private void testFCompression(
final String inputString, final String expectedResult, final long expectedGasUsed) {
final Bytes input = Bytes.fromHexString(inputString);
final Bytes expectedComputation =
expectedResult == null ? null : Bytes.fromHexString(expectedResult);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,18 @@ static Arguments[] parameters() {

@ParameterizedTest
@MethodSource("parameters")
void shouldRecoverAddress(final String inputString, final String expectedResult) {
void shouldRecoverAddressNative(final String inputString, final String expectedResult) {
contract.signatureAlgorithm.maybeEnableNative();
final Bytes input = Bytes.fromHexString(inputString);
final Bytes expected =
expectedResult == null ? Bytes.EMPTY : Bytes32.fromHexString(expectedResult);
assertThat(contract.computePrecompile(input, messageFrame).getOutput()).isEqualTo(expected);
}

@ParameterizedTest
@MethodSource("parameters")
void shouldRecoverAddressJava(final String inputString, final String expectedResult) {
contract.signatureAlgorithm.disableNative();
final Bytes input = Bytes.fromHexString(inputString);
final Bytes expected =
expectedResult == null ? Bytes.EMPTY : Bytes32.fromHexString(expectedResult);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,27 @@ static Arguments[] parameters() {

@ParameterizedTest
@MethodSource("parameters")
void testPrecompiledContract(
void testPrecompiledContractNative(
final String inputString,
final String precompiledResult,
final Long eip198Gas,
final Long eip2565Gas) {
BigIntegerModularExponentiationPrecompiledContract.maybeEnableNative();
testComputation(inputString, precompiledResult);
}

@ParameterizedTest
@MethodSource("parameters")
void testPrecompiledContractJava(
final String inputString,
final String precompiledResult,
final Long eip198Gas,
final Long eip2565Gas) {
BigIntegerModularExponentiationPrecompiledContract.disableNative();
testComputation(inputString, precompiledResult);
}

private void testComputation(final String inputString, final String precompiledResult) {
assumeThat(precompiledResult).isNotNull();
final Bytes input = Bytes.fromHexString(inputString);
final Bytes expected = Bytes.fromHexString(precompiledResult);
Expand Down

0 comments on commit a55c331

Please sign in to comment.