Merge branch 'main' into feat/issue-7273/parse-geth-genesis

Author: jflo

.github/workflows/develop-pr.yml

@@ -73,10 +73,6 @@ jobs:
         uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1
         with:
           cache-disabled: true
-      - name: install goss
-        run: |
-          mkdir -p docker/reports
-          curl -L https://github.com/aelsabbahy/goss/releases/download/v0.4.4/goss-${{ steps.prep.outputs.PLATFORM_PAIR }} -o ./docker/tests/goss-${{ steps.prep.outputs.PLATFORM_PAIR }}
       - name: build and test docker
         uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1
         env:

.github/workflows/develop.yml

@@ -72,10 +72,6 @@ jobs:
         uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1
         with:
           cache-disabled: true
-      - name: install goss
-        run: |
-          mkdir -p docker/reports
-          curl -L https://github.com/aelsabbahy/goss/releases/download/v0.4.4/goss-${{ steps.prep.outputs.PLATFORM_PAIR }} -o ./docker/tests/goss-${{ steps.prep.outputs.PLATFORM_PAIR }}
       - name: login to ${{ env.registry }}
         uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d
         with:

.github/workflows/draft-release.yml

@@ -226,11 +226,6 @@ jobs:
         with:
           cache-disabled: true
 
-      - name: install goss
-        run: |
-          mkdir -p docker/reports
-          curl -L https://github.com/aelsabbahy/goss/releases/download/v0.4.4/goss-${{ steps.prep.outputs.PLATFORM_PAIR }} -o ./docker/tests/goss-${{ steps.prep.outputs.PLATFORM_PAIR }}
-
       - name: login to ${{ env.registry }}
         uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d
         with:

app/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -777,16 +777,6 @@ protected BesuCommand(
     this.transactionValidatorServiceImpl = transactionValidatorServiceImpl;
   }
 
-  /**
-   * Parse Besu command line arguments. Visible for testing.
-   *
-   * @param resultHandler execution strategy. See PicoCLI. Typical argument is RunLast.
-   * @param parameterExceptionHandler Exception handler for handling parameters
-   * @param executionExceptionHandler Exception handler for business logic
-   * @param in Standard input stream
-   * @param args arguments to Besu command
-   * @return success or failure exit code.
-   */
   /**
    * Parses command line arguments and configures the application accordingly.
    *
@@ -798,6 +788,7 @@ protected BesuCommand(
    * @param args The command line arguments.
    * @return The execution result status code.
    */
+  @VisibleForTesting
   public int parse(
       final IExecutionStrategy resultHandler,
       final BesuParameterExceptionHandler parameterExceptionHandler,

app/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java

@@ -99,7 +99,9 @@
 import java.io.PrintStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
@@ -110,6 +112,7 @@
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Supplier;
+import javax.annotation.Nonnull;
 
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.vertx.core.Vertx;
@@ -455,26 +458,65 @@ protected TestBesuCommand parseCommand(
     final TestBesuCommand besuCommand = getTestBesuCommand(testType);
     besuCommands.add(besuCommand);
 
-    final File defaultKeyFile =
-        KeyPairUtil.getDefaultKeyFile(DefaultCommandValues.getDefaultBesuDataPath(besuCommand));
+    besuCommand.setBesuConfiguration(commonPluginConfiguration);
+
+    final List<String> argsList = constructArgsWithTmpDataPathIfNotSpecified(args);
+
+    // Determine the data directory that will be used and write the key file there
+    final Path dataDir = determineDataDir(argsList);
+    final File defaultKeyFile = KeyPairUtil.getDefaultKeyFile(dataDir);
     try {
+      // Ensure parent directory exists
+      defaultKeyFile.getParentFile().mkdirs();
       Files.writeString(defaultKeyFile.toPath(), keyPair.getPrivateKey().toString());
     } catch (final IOException e) {
       throw new RuntimeException(e);
     }
-    besuCommand.setBesuConfiguration(commonPluginConfiguration);
 
-    // parse using Ansi.OFF to be able to assert on non formatted output results
+    // parse using Ansi.OFF to be able to assert on non-formatted output results
     besuCommand.parse(
         new RunLast(),
         besuCommand.parameterExceptionHandler(),
         besuCommand.executionExceptionHandler(),
         in,
         mockBesuComponent,
-        args);
+        argsList.toArray(new String[0]));
     return besuCommand;
   }
 
+  @Nonnull
+  private static Path determineDataDir(final List<String> argsList) {
+    // Look for --data-path in the arguments
+    for (int i = 0; i < argsList.size() - 1; i++) {
+      if ("--data-path".equals(argsList.get(i))) {
+        return Paths.get(argsList.get(i + 1));
+      }
+    }
+    // If no explicit data-path, use default
+    return DefaultCommandValues.getDefaultBesuDataPath(null);
+  }
+
+  @Nonnull
+  private static List<String> constructArgsWithTmpDataPathIfNotSpecified(final String[] args) {
+    final List<String> argsList = new ArrayList<>(Arrays.asList(args));
+
+    boolean hasDataPath = argsList.stream().anyMatch(arg -> arg.contains("data-path"));
+    boolean hasConfigFile = argsList.stream().anyMatch(arg -> arg.contains("config-file"));
+
+    // if data-path is not set, set to a tmp dir
+    if (!hasDataPath && !hasConfigFile) {
+      try {
+        final Path tmpDir = Files.createTempDirectory("besu-test-");
+        tmpDir.toFile().deleteOnExit();
+        argsList.add(0, "--data-path");
+        argsList.add(1, tmpDir.toString());
+      } catch (IOException e) {
+        throw new RuntimeException("Failed to create temporary directory", e);
+      }
+    }
+    return argsList;
+  }
+
   private TestBesuCommand getTestBesuCommand(final TestType testType) {
     switch (testType) {
       case REQUIRED_OPTION:

build.gradle

@@ -31,6 +31,7 @@ plugins {
   id 'jacoco'
   id 'jacoco-report-aggregation'
   id 'org.sonarqube' version '6.2.0.5505'
+  id 'de.undercouch.download' version '5.6.0'
 }
 
 sonarqube {
@@ -880,24 +881,70 @@ task distDocker {
   }
 }
 
-task testDocker {
-  dependsOn distDocker
-  def dockerReportsDir = "docker/reports/"
+tasks.register("downloadGossBinaries") {
+  def gossVersion = 'v0.4.9'
+  def baseUrl = "https://github.com/goss-org/goss/releases/download/${gossVersion}"
+  def testDir = "${projectDir}/docker/tests"
 
-  doFirst {
-    new File(dockerReportsDir).mkdir()
-  }
+  // Detect current architecture
+  def osArch = System.getProperty("os.arch").toLowerCase()
+  def currentArch = (osArch.contains("aarch64") || osArch.contains("arm64")) ? "arm64" : "amd64"
 
   doLast {
-    exec {
-      def image = project.hasProperty('release.releaseVersion') ? "${dockerImageName}:" + project.property('release.releaseVersion') : "${dockerImageName}:${project.version}"
-      workingDir "${projectDir}/docker"
-      executable shell
-      args "-c", "./test.sh ${image}"
+    // Download goss binary for current architecture
+    def binaryFile = new File(testDir, "goss-linux-${currentArch}")
+
+    // Download binary if it doesn't exist
+    if (!binaryFile.exists()) {
+      println "Downloading goss-linux-${currentArch}..."
+      download.run {
+        src "${baseUrl}/goss-linux-${currentArch}"
+        dest binaryFile
+        overwrite false
+        onlyIfModified true
+      }
+
+      binaryFile.setExecutable(true)
+      println "Successfully downloaded goss-linux-${currentArch}"
+    } else {
+      println "goss-linux-${currentArch} already exists"
+    }
+
+    // Download dgoss if it doesn't exist
+    def dgossFile = new File(testDir, 'dgoss')
+    if (!dgossFile.exists()) {
+      println "Downloading dgoss..."
+      download.run {
+        src "${baseUrl}/dgoss"
+        dest dgossFile
+        overwrite false
+        onlyIfModified true
+      }
+
+      dgossFile.setExecutable(true)
+      println "Successfully downloaded dgoss"
+    } else {
+      println "dgoss already exists"
     }
   }
 }
 
+tasks.register("testDocker", Exec) {
+  dependsOn("distDocker", "downloadGossBinaries")
+
+  doFirst {
+    new File("docker/reports/").mkdir()
+  }
+
+  def image = project.hasProperty('release.releaseVersion')
+    ? "${dockerImageName}:" + project.property('release.releaseVersion')
+    : "${dockerImageName}:${project.version}"
+
+  workingDir "${projectDir}/docker"
+  executable shell
+  args "-c", "./test.sh ${image}"
+}
+
 task dockerUpload {
   dependsOn distDocker
   def architecture = System.getenv('architecture')

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/payload/SignedData.java

@@ -15,12 +15,14 @@
 package org.hyperledger.besu.consensus.common.bft.payload;
 
 import org.hyperledger.besu.crypto.SECPSignature;
+import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.core.Util;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
 import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 
+import java.math.BigInteger;
 import java.util.Objects;
 import java.util.StringJoiner;
 
@@ -33,6 +35,9 @@
  */
 public class SignedData<M extends Payload> implements Authored {
 
+  private static final BigInteger HALF_CURVE_ORDER =
+      SignatureAlgorithmFactory.getInstance().getHalfCurveOrder();
+
   private final Address sender;
   private final SECPSignature signature;
   private final M unsignedPayload;
@@ -44,6 +49,7 @@ public class SignedData<M extends Payload> implements Authored {
    * @param payload the payload
    * @param signature the signature
    * @return the signed data
+   * @throws IllegalArgumentException if the signature is invalid
    */
   public static <T extends Payload> SignedData<T> create(
       final T payload, final SECPSignature signature) {
@@ -52,6 +58,9 @@ public static <T extends Payload> SignedData<T> create(
   }
 
   private SignedData(final M unsignedPayload, final Address sender, final SECPSignature signature) {
+    if (!(signature.getS().compareTo(HALF_CURVE_ORDER) <= 0)) {
+      throw new IllegalArgumentException("Signature is invalid");
+    }
     this.unsignedPayload = unsignedPayload;
     this.sender = sender;
     this.signature = signature;

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/RoundState.java

@@ -21,14 +21,15 @@
 import org.hyperledger.besu.consensus.ibft.messagewrappers.Proposal;
 import org.hyperledger.besu.consensus.ibft.validation.MessageValidator;
 import org.hyperledger.besu.crypto.SECPSignature;
+import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.ethereum.core.Block;
 
 import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.stream.Collectors;
 
-import com.google.common.collect.Sets;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -45,8 +46,8 @@ public class RoundState {
 
   // Must track the actual Prepare message, not just the sender, as these may need to be reused
   // to send out in a PrepareCertificate.
-  private final Set<Prepare> prepareMessages = Sets.newLinkedHashSet();
-  private final Set<Commit> commitMessages = Sets.newLinkedHashSet();
+  private final Map<Address, Prepare> prepareMessages = new LinkedHashMap<>();
+  private final Map<Address, Commit> commitMessages = new LinkedHashMap<>();
 
   private boolean prepared = false;
   private boolean committed = false;
@@ -87,8 +88,8 @@ public boolean setProposedBlock(final Proposal msg) {
     if (!proposalMessage.isPresent()) {
       if (validator.validateProposal(msg)) {
         proposalMessage = Optional.of(msg);
-        prepareMessages.removeIf(p -> !validator.validatePrepare(p));
-        commitMessages.removeIf(p -> !validator.validateCommit(p));
+        prepareMessages.entrySet().removeIf(e -> !validator.validatePrepare(e.getValue()));
+        commitMessages.entrySet().removeIf(e -> !validator.validateCommit(e.getValue()));
         updateState();
         return true;
       }
@@ -104,7 +105,7 @@ public boolean setProposedBlock(final Proposal msg) {
    */
   public void addPrepareMessage(final Prepare msg) {
     if (!proposalMessage.isPresent() || validator.validatePrepare(msg)) {
-      prepareMessages.add(msg);
+      prepareMessages.putIfAbsent(msg.getAuthor(), msg);
       LOG.trace("Round state added prepare message prepare={}", msg);
     }
     updateState();
@@ -117,7 +118,7 @@ public void addPrepareMessage(final Prepare msg) {
    */
   public void addCommitMessage(final Commit msg) {
     if (!proposalMessage.isPresent() || validator.validateCommit(msg)) {
-      commitMessages.add(msg);
+      commitMessages.putIfAbsent(msg.getAuthor(), msg);
       LOG.trace("Round state added commit message commit={}", msg);
     }
 
@@ -173,7 +174,7 @@ public boolean isCommitted() {
    * @return the commit seals
    */
   public Collection<SECPSignature> getCommitSeals() {
-    return commitMessages.stream()
+    return commitMessages.values().stream()
         .map(cp -> cp.getSignedPayload().getPayload().getCommitSeal())
         .collect(Collectors.toList());
   }
@@ -185,7 +186,8 @@ public Collection<SECPSignature> getCommitSeals() {
    */
   public Optional<PreparedRoundArtifacts> constructPreparedRoundArtifacts() {
     if (isPrepared()) {
-      return Optional.of(new PreparedRoundArtifacts(proposalMessage.get(), prepareMessages));
+      return Optional.of(
+          new PreparedRoundArtifacts(proposalMessage.get(), prepareMessages.values()));
     }
     return Optional.empty();
   }