Fix SECP256R1AcceptanceTest (hyperledger#2321)

* don't set SECP256R1 as signature algorithm instance in the tests, only use it when the nodes are started as their own processes

Signed-off-by: Daniel Lehrner <daniel@io.builders>

* renamed some variables for better readability

Signed-off-by: Daniel Lehrner <daniel@io.builders>

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java

@@ -84,7 +84,7 @@ public class BesuNode implements NodeConfiguration, RunnableNode, AutoCloseable
   private static final Logger LOG = getLogger();
 
   private final Path homeDirectory;
-  private final KeyPair keyPair;
+  private KeyPair keyPair;
   private final Properties portsProperties = new Properties();
   private final Boolean p2pEnabled;
   private final NetworkingConfiguration networkingConfiguration;
@@ -141,7 +141,8 @@ public BesuNode(
       final List<String> staticNodes,
       final boolean isDnsEnabled,
       final Optional<PrivacyParameters> privacyParameters,
-      final List<String> runCommand)
+      final List<String> runCommand,
+      final Optional<KeyPair> keyPair)
       throws IOException {
     this.homeDirectory = dataPath.orElseGet(BesuNode::createTmpDataDirectory);
     keyfilePath.ifPresent(
@@ -152,7 +153,12 @@ public BesuNode(
             LOG.error("Could not find key file \"{}\" in resources", path);
           }
         });
-    this.keyPair = KeyPairUtil.loadKeyPair(homeDirectory);
+    keyPair.ifPresentOrElse(
+        (existingKeyPair) -> {
+          this.keyPair = existingKeyPair;
+          KeyPairUtil.storeKeyFile(existingKeyPair, homeDirectory);
+        },
+        () -> this.keyPair = KeyPairUtil.loadKeyPair(homeDirectory));
     this.name = name;
     this.miningParameters = miningParameters;
     this.jsonRpcConfiguration = jsonRpcConfiguration;

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNodeRunner.java

@@ -26,13 +26,17 @@
 public interface BesuNodeRunner {
 
   static BesuNodeRunner instance() {
-    if (Boolean.getBoolean("acctests.runBesuAsProcess")) {
+    if (isProcessBesuNodeRunner()) {
       return new ProcessBesuNodeRunner();
     } else {
       return new ThreadBesuNodeRunner();
     }
   }
 
+  static boolean isProcessBesuNodeRunner() {
+    return Boolean.getBoolean("acctests.runBesuAsProcess");
+  }
+
   void startNode(BesuNode node);
 
   void stopNode(BesuNode node);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java

@@ -15,6 +15,7 @@
 package org.hyperledger.besu.tests.acceptance.dsl.node.configuration;
 
 import org.hyperledger.besu.cli.config.NetworkName;
+import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
@@ -54,6 +55,7 @@ public class BesuNodeConfiguration {
   private final Optional<PrivacyParameters> privacyParameters;
   private final List<String> runCommand;
   private final NetworkName network;
+  private final Optional<KeyPair> keyPair;
 
   BesuNodeConfiguration(
       final String name,
@@ -79,7 +81,8 @@ public class BesuNodeConfiguration {
       final List<String> staticNodes,
       final boolean isDnsEnabled,
       final Optional<PrivacyParameters> privacyParameters,
-      final List<String> runCommand) {
+      final List<String> runCommand,
+      final Optional<KeyPair> keyPair) {
     this.name = name;
     this.miningParameters = miningParameters;
     this.jsonRpcConfiguration = jsonRpcConfiguration;
@@ -104,6 +107,7 @@ public class BesuNodeConfiguration {
     this.isDnsEnabled = isDnsEnabled;
     this.privacyParameters = privacyParameters;
     this.runCommand = runCommand;
+    this.keyPair = keyPair;
   }
 
   public String getName() {
@@ -201,4 +205,8 @@ public List<String> getRunCommand() {
   public NetworkName getNetwork() {
     return network;
   }
+
+  public Optional<KeyPair> getKeyPair() {
+    return keyPair;
+  }
 }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java

@@ -18,6 +18,7 @@
 import static java.util.Collections.singletonList;
 
 import org.hyperledger.besu.cli.config.NetworkName;
+import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis;
 import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration;
@@ -65,6 +66,7 @@ public class BesuNodeConfigurationBuilder {
   private boolean isDnsEnabled = false;
   private Optional<PrivacyParameters> privacyParameters = Optional.empty();
   private List<String> runCommand = new ArrayList<>();
+  private Optional<KeyPair> keyPair = Optional.empty();
 
   public BesuNodeConfigurationBuilder() {
     // Check connections more frequently during acceptance tests to cut down on
@@ -295,6 +297,11 @@ public BesuNodeConfigurationBuilder privacyParameters(final PrivacyParameters pr
     return this;
   }
 
+  public BesuNodeConfigurationBuilder keyPair(final KeyPair keyPair) {
+    this.keyPair = Optional.of(keyPair);
+    return this;
+  }
+
   public BesuNodeConfigurationBuilder run(final String... commands) {
     this.runCommand = List.of(commands);
     return this;
@@ -325,6 +332,7 @@ public BesuNodeConfiguration build() {
         staticNodes,
         isDnsEnabled,
         privacyParameters,
-        runCommand);
+        runCommand,
+        keyPair);
   }
 }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java

@@ -17,9 +17,7 @@
 import static java.util.Arrays.asList;
 import static java.util.stream.Collectors.toList;
 
-import org.hyperledger.besu.config.GenesisConfigFile;
-import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
-import org.hyperledger.besu.crypto.SignatureAlgorithmType;
+import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.enclave.EnclaveFactory;
 import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcApi;
@@ -41,7 +39,7 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.file.Paths;
-import java.util.Collections;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
@@ -54,8 +52,6 @@ public class BesuNodeFactory {
   private final NodeConfigurationFactory node = new NodeConfigurationFactory();
 
   public BesuNode create(final BesuNodeConfiguration config) throws IOException {
-    instantiateSignatureAlgorithmFactory(config);
-
     return new BesuNode(
         config.getName(),
         config.getDataPath(),
@@ -80,7 +76,8 @@ public BesuNode create(final BesuNodeConfiguration config) throws IOException {
         config.getStaticNodes(),
         config.isDnsEnabled(),
         config.getPrivacyParameters(),
-        config.getRunCommand());
+        config.getRunCommand(),
+        config.getKeyPair());
   }
 
   public BesuNode createMinerNode(final String name) throws IOException {
@@ -443,53 +440,69 @@ public BesuNode createQbftNodeWithValidators(final String name, final String...
   public BesuNode createNodeWithStaticNodes(final String name, final List<Node> staticNodes)
       throws IOException {
 
+    BesuNodeConfigurationBuilder builder =
+        createConfigurationBuilderWithStaticNodes(name, staticNodes);
+    return create(builder.build());
+  }
+
+  private BesuNodeConfigurationBuilder createConfigurationBuilderWithStaticNodes(
+      final String name, final List<Node> staticNodes) {
     final List<String> staticNodesUrls =
         staticNodes.stream()
             .map(node -> (RunnableNode) node)
             .map(RunnableNode::enodeUrl)
             .map(URI::toASCIIString)
             .collect(toList());
 
-    return create(
-        new BesuNodeConfigurationBuilder()
-            .name(name)
-            .jsonRpcEnabled()
-            .webSocketEnabled()
-            .discoveryEnabled(false)
-            .staticNodes(staticNodesUrls)
-            .bootnodeEligible(false)
-            .build());
+    return new BesuNodeConfigurationBuilder()
+        .name(name)
+        .jsonRpcEnabled()
+        .webSocketEnabled()
+        .discoveryEnabled(false)
+        .staticNodes(staticNodesUrls)
+        .bootnodeEligible(false);
   }
 
-  public BesuNode runCommand(final String command) throws IOException {
-    return create(new BesuNodeConfigurationBuilder().name("run " + command).run(command).build());
-  }
+  public BesuNode createNodeWithNonDefaultSignatureAlgorithm(
+      final String name, final String genesisPath, final KeyPair keyPair) throws IOException {
+    BesuNodeConfigurationBuilder builder =
+        createNodeConfigurationWithNonDefaultSignatureAlgorithm(
+            name, genesisPath, keyPair, new ArrayList<>());
+    builder.miningEnabled();
 
-  private void instantiateSignatureAlgorithmFactory(final BesuNodeConfiguration config) {
-    if (SignatureAlgorithmFactory.isInstanceSet()) {
-      return;
-    }
-
-    Optional<String> ecCurve = getEcCurveFromGenesisFile(config);
-
-    if (ecCurve.isEmpty()) {
-      SignatureAlgorithmFactory.setDefaultInstance();
-      return;
-    }
+    return create(builder.build());
+  }
 
-    SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create(ecCurve.get()));
+  public BesuNode createNodeWithNonDefaultSignatureAlgorithm(
+      final String name,
+      final String genesisPath,
+      final KeyPair keyPair,
+      final List<Node> staticNodes)
+      throws IOException {
+    BesuNodeConfigurationBuilder builder =
+        createNodeConfigurationWithNonDefaultSignatureAlgorithm(
+            name, genesisPath, keyPair, staticNodes);
+    return create(builder.build());
   }
 
-  private Optional<String> getEcCurveFromGenesisFile(final BesuNodeConfiguration config) {
-    Optional<String> genesisConfig =
-        config.getGenesisConfigProvider().create(Collections.emptyList());
+  public BesuNodeConfigurationBuilder createNodeConfigurationWithNonDefaultSignatureAlgorithm(
+      final String name,
+      final String genesisPath,
+      final KeyPair keyPair,
+      final List<Node> staticNodes) {
+    BesuNodeConfigurationBuilder builder =
+        createConfigurationBuilderWithStaticNodes(name, staticNodes);
 
-    if (genesisConfig.isEmpty()) {
-      return Optional.empty();
-    }
+    final GenesisConfigurationFactory genesis = new GenesisConfigurationFactory();
+    final String genesisData = genesis.readGenesisFile(genesisPath);
 
-    GenesisConfigFile genesisConfigFile = GenesisConfigFile.fromConfig(genesisConfig.get());
+    return builder
+        .devMode(false)
+        .genesisConfigProvider((nodes) -> Optional.of(genesisData))
+        .keyPair(keyPair);
+  }
 
-    return genesisConfigFile.getConfigOptions().getEcCurve();
+  public BesuNode runCommand(final String command) throws IOException {
+    return create(new BesuNodeConfigurationBuilder().name("run " + command).run(command).build());
   }
 }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java

@@ -118,7 +118,8 @@ public PrivacyNode(
             Collections.emptyList(),
             besuConfig.isDnsEnabled(),
             besuConfig.getPrivacyParameters(),
-            List.of());
+            List.of(),
+            Optional.empty());
   }
 
   public void testEnclaveConnection(final List<PrivacyNode> otherNodes) {

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/crypto/SECP256R1AcceptanceTest.java

@@ -16,45 +16,87 @@
 package org.hyperledger.besu.tests.acceptance.crypto;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assumptions.assumeThat;
 
+import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.crypto.SECP256R1;
 import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.tests.acceptance.dsl.AcceptanceTestBase;
 import org.hyperledger.besu.tests.acceptance.dsl.account.Account;
+import org.hyperledger.besu.tests.acceptance.dsl.node.BesuNodeRunner;
 import org.hyperledger.besu.tests.acceptance.dsl.node.Node;
+import org.hyperledger.besu.tests.acceptance.dsl.node.cluster.Cluster;
+import org.hyperledger.besu.tests.acceptance.dsl.node.cluster.ClusterConfiguration;
+import org.hyperledger.besu.tests.acceptance.dsl.node.cluster.ClusterConfigurationBuilder;
 
+import java.util.List;
+
+import org.apache.tuweni.bytes.Bytes32;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class SECP256R1AcceptanceTest extends AcceptanceTestBase {
   private Node minerNode;
-  private Node fullNode;
+  private Node otherNode;
+  private Cluster noDiscoveryCluster;
+
+  private static final String GENESIS_FILE = "/crypto/secp256r1.json";
 
-  protected static final String GENESIS_FILE = "/crypto/secp256r1.json";
+  private static final String MINER_NODE_PRIVATE_KEY =
+      "8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63";
+  private static final String OTHER_NODE_PRIVATE_KEY =
+      "c87509a1c067bbde78beb793e6fa76530b6382a4c0241e5e4a9ec0a0f44dc0d3";
+
+  private static final SECP256R1 SECP256R1_SIGNATURE_ALGORITHM = new SECP256R1();
 
   @Before
   public void setUp() throws Exception {
-    minerNode = besu.createCustomGenesisNode("node1", GENESIS_FILE, true, true);
-    fullNode = besu.createCustomGenesisNode("node2", GENESIS_FILE, false);
-    cluster.start(minerNode, fullNode);
-  }
+    KeyPair minerNodeKeyPair = createKeyPair(MINER_NODE_PRIVATE_KEY);
+    KeyPair otherNodeKeyPair = createKeyPair(OTHER_NODE_PRIVATE_KEY);
 
-  @Test
-  @Ignore
-  public void shouldConnectToOtherPeer() {
-    minerNode.verify(net.awaitPeerCount(1));
-    fullNode.verify(net.awaitPeerCount(1));
+    final ClusterConfiguration clusterConfiguration =
+        new ClusterConfigurationBuilder().awaitPeerDiscovery(false).build();
+    noDiscoveryCluster = new Cluster(clusterConfiguration, net);
+
+    minerNode =
+        besu.createNodeWithNonDefaultSignatureAlgorithm(
+            "minerNode", GENESIS_FILE, minerNodeKeyPair);
+    noDiscoveryCluster.start(minerNode);
+
+    otherNode =
+        besu.createNodeWithNonDefaultSignatureAlgorithm(
+            "otherNode", GENESIS_FILE, otherNodeKeyPair, List.of(minerNode));
+    cluster.addNode(otherNode);
   }
 
   @Test
-  @Ignore
   public void transactionShouldBeSuccessful() {
+    // SignatureAlgorithmFactory.instance is static. When ThreadBesuRunner is used, we cannot change
+    // the signature algorithm instance to SECP256R1 as it could influence other tests running at
+    // the same time. So we only execute the test when ProcessBesuNodeRunner is used, as there is
+    // not conflict because we use separate processes.
+    assumeThat(BesuNodeRunner.isProcessBesuNodeRunner()).isTrue();
+
+    minerNode.verify(net.awaitPeerCount(1));
+    otherNode.verify(net.awaitPeerCount(1));
+
     final Account recipient = accounts.createAccount("recipient");
 
     final Hash transactionHash =
         minerNode.execute(accountTransactions.createTransfer(recipient, 5), new SECP256R1());
     assertThat(transactionHash).isNotNull();
     cluster.verify(recipient.balanceEquals(5));
   }
+
+  @Override
+  public void tearDownAcceptanceTestBase() {
+    super.tearDownAcceptanceTestBase();
+
+    noDiscoveryCluster.stop();
+  }
+
+  private KeyPair createKeyPair(final String privateKey) {
+    return SECP256R1_SIGNATURE_ALGORITHM.createKeyPair(
+        SECP256R1_SIGNATURE_ALGORITHM.createPrivateKey(Bytes32.fromHexString(privateKey)));
+  }
 }

crypto/src/main/java/org/hyperledger/besu/crypto/KeyPair.java

@@ -76,6 +76,15 @@ public boolean equals(final Object other) {
     return this.privateKey.equals(that.privateKey) && this.publicKey.equals(that.publicKey);
   }
 
+  @Override
+  public String toString() {
+    return ("KeyPair{privateKey: "
+        + this.privateKey.toString()
+        + ", publicKey: "
+        + this.publicKey.toString()
+        + "]");
+  }
+
   public SECPPrivateKey getPrivateKey() {
     return privateKey;
   }