From d081c1754c1718435e88f4b86fb17868a9089b4b Mon Sep 17 00:00:00 2001 From: Usman Saleem Date: Tue, 1 Oct 2024 15:31:56 +1000 Subject: [PATCH 01/14] fix: Fix DNSDaemonTest - remove flaky assertion (#7701) * fix: Fix DNSDaemonTest - remove flaky assertion Signed-off-by: Usman Saleem --------- Signed-off-by: Usman Saleem --- .../besu/ethereum/p2p/discovery/dns/DNSDaemonTest.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/DNSDaemonTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/DNSDaemonTest.java index fd8ba1382d..94d9c75e4a 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/DNSDaemonTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/DNSDaemonTest.java @@ -14,8 +14,6 @@ */ package org.hyperledger.besu.ethereum.p2p.discovery.dns; -import static org.assertj.core.api.Assertions.assertThat; - import java.security.Security; import java.util.concurrent.atomic.AtomicInteger; @@ -26,7 +24,6 @@ import io.vertx.junit5.Checkpoint; import io.vertx.junit5.VertxExtension; import io.vertx.junit5.VertxTestContext; import org.bouncycastle.jce.provider.BouncyCastleProvider; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -136,10 +133,4 @@ class DNSDaemonTest { .setWorkerPoolSize(1); vertx.deployVerticle(dnsDaemon, options); } - - @AfterEach - @DisplayName("Check that the vertx worker verticle is still there") - void lastChecks(final Vertx vertx) { - assertThat(vertx.deploymentIDs()).isNotEmpty().hasSize(2); - } } From e522b63bed3d4398b05ba7f81b7ff0076cf753b7 Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Tue, 1 Oct 2024 08:16:59 -0400 Subject: [PATCH 02/14] dont' activate flight recorder dumps by default, RIP my ssd (#7692) * dont' dump flight recorder captures by default, RIP my ssd --------- Signed-off-by: Justin Florentine --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index ee4392c9fc..1429ed9c79 100644 --- a/build.gradle +++ b/build.gradle @@ -690,7 +690,7 @@ startScripts { "-XX:G1ConcRefinementThreads=2", "-XX:G1HeapWastePercent=15", "-XX:MaxGCPauseMillis=100", - "-XX:StartFlightRecording,dumponexit=true,settings=default.jfc", + "-XX:StartFlightRecording,settings=default.jfc", "-Xlog:jfr*=off" ] unixStartScriptGenerator.template = resources.text.fromFile("${projectDir}/besu/src/main/scripts/unixStartScript.txt") From 7af03b7295cc208d9f3fe9cd2872d49f87f3920a Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 1 Oct 2024 18:00:20 +0200 Subject: [PATCH 03/14] Align gas cap for transaction simulation to Geth approach (#7703) Signed-off-by: Fabio Di Fabio --- CHANGELOG.md | 1 + .../transaction/TransactionSimulator.java | 47 ++++++++--- .../transaction/TransactionSimulatorTest.java | 84 ++++++++++++++----- 3 files changed, 99 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dce16b3f9d..02b499b178 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - LUKSO Cancun Hardfork [#7686](https://github.com/hyperledger/besu/pull/7686) - Add configuration of Consolidation Request Contract Address via genesis configuration [#7647](https://github.com/hyperledger/besu/pull/7647) - Interrupt pending transaction processing on block creation timeout [#7673](https://github.com/hyperledger/besu/pull/7673) +- Align gas cap calculation for transaction simulation to Geth approach [#7703](https://github.com/hyperledger/besu/pull/7703) ### Bug fixes - Fix mounted data path directory permissions for besu user [#7575](https://github.com/hyperledger/besu/pull/7575) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java index 29459ba3da..5e78256a78 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java @@ -230,16 +230,9 @@ public class TransactionSimulator { final Account sender = updater.get(senderAddress); final long nonce = sender != null ? sender.getNonce() : 0L; - long gasLimit = - callParams.getGasLimit() >= 0 - ? callParams.getGasLimit() - : blockHeaderToProcess.getGasLimit(); - if (rpcGasCap > 0) { - gasLimit = rpcGasCap; - LOG.trace( - "Gas limit capped at {} for transaction simulation due to provided RPC gas cap.", - rpcGasCap); - } + final long simulationGasCap = + calculateSimulationGasCap(callParams.getGasLimit(), blockHeaderToProcess.getGasLimit()); + final Wei value = callParams.getValue() != null ? callParams.getValue() : Wei.ZERO; final Bytes payload = callParams.getPayload() != null ? callParams.getPayload() : Bytes.EMPTY; @@ -265,7 +258,7 @@ public class TransactionSimulator { header, senderAddress, nonce, - gasLimit, + simulationGasCap, value, payload, blobGasPrice); @@ -291,6 +284,38 @@ public class TransactionSimulator { return Optional.of(new TransactionSimulatorResult(transaction, result)); } + private long calculateSimulationGasCap( + final long userProvidedGasLimit, final long blockGasLimit) { + final long simulationGasCap; + + // when not set gas limit is -1 + if (userProvidedGasLimit >= 0) { + if (rpcGasCap > 0 && userProvidedGasLimit > rpcGasCap) { + LOG.trace( + "User provided gas limit {} is bigger than the value of rpc-gas-cap {}, setting simulation gas cap to the latter", + userProvidedGasLimit, + rpcGasCap); + simulationGasCap = rpcGasCap; + } else { + LOG.trace("Using provided gas limit {} set as simulation gas cap", userProvidedGasLimit); + simulationGasCap = userProvidedGasLimit; + } + } else { + if (rpcGasCap > 0) { + LOG.trace( + "No user provided gas limit, setting simulation gas cap to the value of rpc-gas-cap {}", + rpcGasCap); + simulationGasCap = rpcGasCap; + } else { + simulationGasCap = blockGasLimit; + LOG.trace( + "No user provided gas limit and rpc-gas-cap options is not set, setting simulation gas cap to block gas limit {}", + blockGasLimit); + } + } + return simulationGasCap; + } + private Optional buildTransaction( final CallParameter callParams, final TransactionValidationParams transactionValidationParams, diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java index 0aac2f026f..73fa402abb 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java @@ -79,7 +79,8 @@ public class TransactionSimulatorTest { private static final Address DEFAULT_FROM = Address.fromHexString("0x0000000000000000000000000000000000000000"); - private static final long GASCAP = 500L; + private static final long GAS_CAP = 500000L; + private static final long TRANSFER_GAS_LIMIT = 21000L; private TransactionSimulator transactionSimulator; private TransactionSimulator cappedTransactionSimulator; @@ -96,7 +97,7 @@ public class TransactionSimulatorTest { this.transactionSimulator = new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule, 0); this.cappedTransactionSimulator = - new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule, GASCAP); + new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule, GAS_CAP); } @Test @@ -124,7 +125,7 @@ public class TransactionSimulatorTest { .type(TransactionType.FRONTIER) .nonce(1L) .gasPrice(callParameter.getGasPrice()) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(blockHeader.getGasLimit()) .to(callParameter.getTo()) .sender(callParameter.getFrom()) .value(callParameter.getValue()) @@ -155,7 +156,7 @@ public class TransactionSimulatorTest { .type(TransactionType.FRONTIER) .nonce(1L) .gasPrice(Wei.ZERO) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(blockHeader.getGasLimit()) .to(callParameter.getTo()) .sender(callParameter.getFrom()) .value(callParameter.getValue()) @@ -175,7 +176,8 @@ public class TransactionSimulatorTest { @Test public void shouldSetFeePerGasToZeroWhenExceedingBalanceAllowed() { - final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ONE, Wei.ONE); + final CallParameter callParameter = + eip1559TransactionCallParameter(Wei.ONE, Wei.ONE, TRANSFER_GAS_LIMIT); final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE); @@ -187,7 +189,7 @@ public class TransactionSimulatorTest { .type(TransactionType.EIP1559) .chainId(BigInteger.ONE) .nonce(1L) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(TRANSFER_GAS_LIMIT) .maxFeePerGas(Wei.ZERO) .maxPriorityFeePerGas(Wei.ZERO) .to(callParameter.getTo()) @@ -223,7 +225,7 @@ public class TransactionSimulatorTest { .type(TransactionType.FRONTIER) .nonce(1L) .gasPrice(callParameter.getGasPrice()) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(blockHeader.getGasLimit()) .to(callParameter.getTo()) .sender(callParameter.getFrom()) .value(callParameter.getValue()) @@ -244,7 +246,8 @@ public class TransactionSimulatorTest { @Test public void shouldNotSetFeePerGasToZeroWhenExceedingBalanceIsNotAllowed() { - final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ONE, Wei.ONE); + final CallParameter callParameter = + eip1559TransactionCallParameter(Wei.ONE, Wei.ONE, TRANSFER_GAS_LIMIT); final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE); @@ -256,7 +259,7 @@ public class TransactionSimulatorTest { .type(TransactionType.EIP1559) .chainId(BigInteger.ONE) .nonce(1L) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(TRANSFER_GAS_LIMIT) .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow()) .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow()) .to(callParameter.getTo()) @@ -349,7 +352,7 @@ public class TransactionSimulatorTest { .type(TransactionType.FRONTIER) .nonce(1L) .gasPrice(callParameter.getGasPrice()) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(blockHeader.getGasLimit()) .to(callParameter.getTo()) .sender(callParameter.getFrom()) .value(callParameter.getValue()) @@ -390,7 +393,7 @@ public class TransactionSimulatorTest { .type(TransactionType.FRONTIER) .nonce(1L) .gasPrice(callParameter.getGasPrice()) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(blockHeader.getGasLimit()) .to(callParameter.getTo()) .sender(callParameter.getFrom()) .value(callParameter.getValue()) @@ -479,7 +482,7 @@ public class TransactionSimulatorTest { .type(TransactionType.FRONTIER) .nonce(1L) .gasPrice(callParameter.getGasPrice()) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(blockHeader.getGasLimit()) .to(callParameter.getTo()) .sender(callParameter.getFrom()) .value(callParameter.getValue()) @@ -509,7 +512,7 @@ public class TransactionSimulatorTest { .type(TransactionType.EIP1559) .chainId(BigInteger.ONE) .nonce(1L) - .gasLimit(callParameter.getGasLimit()) + .gasLimit(blockHeader.getGasLimit()) .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow()) .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow()) .to(callParameter.getTo()) @@ -530,7 +533,7 @@ public class TransactionSimulatorTest { @Test public void shouldCapGasLimitWhenOriginalTransactionExceedsGasCap() { final CallParameter callParameter = - eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GASCAP + 1); + eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GAS_CAP + 1); final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE); @@ -542,7 +545,7 @@ public class TransactionSimulatorTest { .type(TransactionType.EIP1559) .chainId(BigInteger.ONE) .nonce(1L) - .gasLimit(GASCAP) + .gasLimit(GAS_CAP) .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow()) .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow()) .to(callParameter.getTo()) @@ -566,11 +569,48 @@ public class TransactionSimulatorTest { } @Test - public void shouldUseRpcGasCapWhenCapIsHigherThanGasLimit() { - // generate a transaction with a gas limit that is lower than the gas cap, - // expect the gas cap to override parameter gas limit + public void shouldUseProvidedGasLimitWhenBelowRpcCapGas() { final CallParameter callParameter = - eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GASCAP - 1); + eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GAS_CAP / 2); + + final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE); + + mockBlockchainForBlockHeader(blockHeader); + mockWorldStateForAccount(blockHeader, callParameter.getFrom(), 1L); + + final Transaction expectedTransaction = + Transaction.builder() + .type(TransactionType.EIP1559) + .chainId(BigInteger.ONE) + .nonce(1L) + .gasLimit(GAS_CAP / 2) + .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow()) + .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow()) + .to(callParameter.getTo()) + .sender(callParameter.getFrom()) + .value(callParameter.getValue()) + .payload(callParameter.getPayload()) + .signature(FAKE_SIGNATURE) + .build(); + + mockProtocolSpecForProcessWithWorldUpdater(); + + // call process with original transaction + cappedTransactionSimulator.process( + callParameter, + TransactionValidationParams.transactionSimulator(), + OperationTracer.NO_TRACING, + 1L); + + // expect overwritten transaction to be processed + verifyTransactionWasProcessed(expectedTransaction); + } + + @Test + public void shouldUseRpcGasCapWhenGasLimitNoPresent() { + // generate call parameters that do not specify a gas limit, + // expect the rpc gas cap to be used for simulation + final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, -1); final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE); @@ -591,7 +631,7 @@ public class TransactionSimulatorTest { .value(callParameter.getValue()) .payload(callParameter.getPayload()) .signature(FAKE_SIGNATURE) - .gasLimit(GASCAP) + .gasLimit(GAS_CAP) .build(); // call process with original transaction @@ -781,7 +821,7 @@ public class TransactionSimulatorTest { return new CallParameter( Address.fromHexString("0x0"), Address.fromHexString("0x0"), - 0, + -1, gasPrice, Wei.of(0), Bytes.EMPTY); @@ -793,7 +833,7 @@ public class TransactionSimulatorTest { private CallParameter eip1559TransactionCallParameter( final Wei maxFeePerGas, final Wei maxPriorityFeePerGas) { - return eip1559TransactionCallParameter(maxFeePerGas, maxPriorityFeePerGas, 0L); + return eip1559TransactionCallParameter(maxFeePerGas, maxPriorityFeePerGas, -1); } private CallParameter eip1559TransactionCallParameter( From 6c47cc9d4e8edf5df12d70ef953d9e25574c0994 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Tue, 1 Oct 2024 15:10:44 -0700 Subject: [PATCH 04/14] add Luis as a maintainer (#7603) Signed-off-by: garyschulte Co-authored-by: Sally MacFarlane --- MAINTAINERS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS.md b/MAINTAINERS.md index 17958bdbd4..6b699a6dca 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -22,6 +22,7 @@ | Justin Florentine| jflo | RoboCopsGoneMad | | Jason Frame | jframe | jframe | | Joshua Fernandes | joshuafernandes | joshuafernandes | +| Luis Pinto | lu-pinto | lu-pinto | Lucas Saldanha | lucassaldanha | lucassaldanha | | Sally MacFarlane | macfarla | macfarla | | Karim Taam | matkt | matkt | From 5f6f7a077a7ace10743aba29c9a3df3a3bee4946 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 2 Oct 2024 09:05:11 +1000 Subject: [PATCH 05/14] Moved maintainers to emeritus (#7611) * move antoine to emeritus Signed-off-by: Sally MacFarlane * moved jiri and antony to emeritus Signed-off-by: Sally MacFarlane * add adrian Signed-off-by: Sally MacFarlane --------- Signed-off-by: Sally MacFarlane Co-authored-by: Usman Saleem --- MAINTAINERS.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS.md b/MAINTAINERS.md index 6b699a6dca..cf040c25bc 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -9,15 +9,11 @@ | Name | Github | LFID | | ---------------- | ---------------- | ---------------- | | Ameziane Hamlat | ahamlat | ahamlat | -| Adrian Sutton | ajsutton | ajsutton | -| Antony Denyer | antonydenyer | antonydenyer | -| Antoine Toulme | atoulme | atoulme | | Daniel Lehrner | daniellehrner | daniellehrner | | Diego López León | diega | diega | | Fabio Di Fabio | fab-10 | fab-10 | | Gabriel Trintinalia | gabriel-trintinalia | gabrieltrintinalia | | Gary Schulte | garyschulte | GarySchulte | -| Jiri Peinlich | gezero | JiriPeinlich | | Gabriel Fukushima| gfukushima | gfukushima | | Justin Florentine| jflo | RoboCopsGoneMad | | Jason Frame | jframe | jframe | @@ -38,11 +34,15 @@ | Name | Github | LFID | |------------------|------------------|------------------| | Abdel Bakhta | abdelhamidbakhta | abdelhamidbakhta | +| Adrian Sutton | ajsutton | ajsutton | +| Antony Denyer | antonydenyer | antonydenyer | +| Antoine Toulme | atoulme | atoulme | | Byron Gravenorst | bgravenorst | bgravenorst | | Chris Hare | CjHare | cjhare | | David Mechler | davemec | davemec | | Edward Evans | EdJoJob | EdJoJob | | Edward Mack | edwardmack | mackcom | +| Jiri Peinlich | gezero | JiriPeinlich | | Frank Li | frankisawesome | frankliawesome | | Ivaylo Kirilov | iikirilov | iikirilov | | Madeline Murray | MadelineMurray | madelinemurray | From db1899c227f04b8beb92a1de9603325d9716ecc2 Mon Sep 17 00:00:00 2001 From: Bhanu Pulluri <59369753+pullurib@users.noreply.github.com> Date: Tue, 1 Oct 2024 19:45:34 -0400 Subject: [PATCH 06/14] Expose revert reason field through GraphQL (#7677) Signed-off-by: Bhanu Pulluri Co-authored-by: Bhanu Pulluri Co-authored-by: Sally MacFarlane --- .../pojoadapter/TransactionAdapter.java | 18 ++++++++++++++++++ .../api/src/main/resources/schema.graphqls | 3 +++ 2 files changed, 21 insertions(+) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java index d5eacf8e35..f8e60c5527 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/TransactionAdapter.java @@ -26,6 +26,7 @@ import org.hyperledger.besu.ethereum.api.query.TransactionWithMetadata; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.LogWithMetadata; import org.hyperledger.besu.ethereum.core.Transaction; +import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput; @@ -292,6 +293,23 @@ public class TransactionAdapter extends AdapterBase { : Optional.of((long) receipt.getStatus())); } + /** + * Retrieves the revert reason of the transaction, if any. + * + *

This method uses the getReceipt method to get the receipt of the transaction. It then checks + * the revert reason of the receipt. It would be an empty Optional for successful transactions. + * Otherwise, it returns an Optional containing the revert reason. + * + * @param environment the data fetching environment. + * @return an Optional containing a Bytes object representing the revert reason of the + * transaction, or an empty Optional . + */ + public Optional getRevertReason(final DataFetchingEnvironment environment) { + return getReceipt(environment) + .map(TransactionReceiptWithMetadata::getReceipt) + .flatMap(TransactionReceipt::getRevertReason); + } + /** * Retrieves the gas used by the transaction. * diff --git a/ethereum/api/src/main/resources/schema.graphqls b/ethereum/api/src/main/resources/schema.graphqls index 87c7509b39..19d3ffea46 100644 --- a/ethereum/api/src/main/resources/schema.graphqls +++ b/ethereum/api/src/main/resources/schema.graphqls @@ -579,6 +579,9 @@ type Transaction { BlobVersionedHashes is a set of hash outputs from the blobs in the transaction. """ blobVersionedHashes: [Bytes32!] + + """Reason returned when transaction fails.""" + revertReason: Bytes } """EIP-4895""" From fbec990bd2583752026b1f70a9f45e69685afe80 Mon Sep 17 00:00:00 2001 From: Chaminda Divitotawela Date: Wed, 2 Oct 2024 10:53:03 +1000 Subject: [PATCH 07/14] Update release process (#7707) Previously GitHub release will be created which triggers building and publishing the artifacts. Release engineers found workflow could fail after the GitHub release created. Community may subscribed to GitHub releases but if the workflow failed, artifacts for the release would not available. Proposed solution requires release engineer to run a GitHub workflow manually by providing the Git tag which creates a draft GitHub release. During this workflow, release artifacts binary distribution, docker images (not latest), Artifactory jars created and published. Release engineer can update the release notes in draft release and publish the release. At the time when the release is published, release engineer is confident all the artifacts are ready. Upon publishing the release, another workflow is triggered to promote the release version of docker images to latest Signed-off-by: Chaminda Divitotawela Co-authored-by: Simon Dudley --- .github/workflows/docker-promote.yml | 81 +++++ .../{release.yml => draft-release.yml} | 314 +++++++++++------- 2 files changed, 267 insertions(+), 128 deletions(-) create mode 100644 .github/workflows/docker-promote.yml rename .github/workflows/{release.yml => draft-release.yml} (59%) diff --git a/.github/workflows/docker-promote.yml b/.github/workflows/docker-promote.yml new file mode 100644 index 0000000000..858ea7d20e --- /dev/null +++ b/.github/workflows/docker-promote.yml @@ -0,0 +1,81 @@ +name: Docker Promote + +run-name: "Docker Promote ${{ github.event.release.name }}" + +on: + release: + types: [released] + +env: + registry: docker.io + GRADLE_OPTS: "-Dorg.gradle.parallel=true -Dorg.gradle.caching=true" + +jobs: + validate: + runs-on: ubuntu-22.04 + env: + RELEASE_NAME: "${{ github.event.release.name }}" + steps: + - name: Pre-process Release Name + id: pre_process_release_name + run: | + # strip all whitespace + RELEASE_NAME="${RELEASE_NAME//[[:space:]]/}" + if [[ ! "$RELEASE_NAME" =~ ^[0-9]+\.[0-9]+(\.[0-9]+)?(-.*)?$ ]]; then + echo "Release name does not conform to a valid besu release format YY.M.v[-suffix], e.g. 24.8.0-RC1." + exit 1 + fi + echo "release_name=$RELEASE_NAME" >> $GITHUB_OUTPUT # Set as output using the new syntax + outputs: + release_name: ${{ steps.pre_process_release_name.outputs.release_name }} + + docker-promote: + needs: [validate] + env: + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + runs-on: ubuntu-22.04 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + + - name: Setup Java + uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 + with: + distribution: temurin + java-version: 21 + cache: gradle + + - name: Login to ${{ env.registry }} + uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d + with: + registry: ${{ env.registry }} + username: ${{ secrets.DOCKER_USER_RW }} + password: ${{ secrets.DOCKER_PASSWORD_RW }} + + - name: Setup Gradle + uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 + with: + cache-disabled: true + + - name: Docker upload + run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_NAME }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" dockerUploadRelease + + - name: Docker manifest + run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_NAME }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" manifestDockerRelease + + docker-verify: + needs: [validate, docker-promote] + env: + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + runs-on: ubuntu-22.04 + permissions: + contents: read + actions: write + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + + - name: Trigger container verify + run: echo '{"version":"${{ env.RELEASE_NAME }}","verify-latest-version":"true"}' | gh workflow run container-verify.yml --json + env: + GH_TOKEN: ${{ github.token }} diff --git a/.github/workflows/release.yml b/.github/workflows/draft-release.yml similarity index 59% rename from .github/workflows/release.yml rename to .github/workflows/draft-release.yml index 7c9b70891d..f6e01e1890 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/draft-release.yml @@ -1,21 +1,35 @@ -name: release +name: Draft Release + +run-name: "Draft Release ${{ inputs.tag }}" on: - release: - types: [released] + workflow_dispatch: + inputs: + tag: + required: true env: registry: docker.io GRADLE_OPTS: "-Dorg.gradle.parallel=true -Dorg.gradle.caching=true" jobs: - preprocess_release: + validate: runs-on: ubuntu-22.04 + env: + RELEASE_NAME: "${{ inputs.tag }}" steps: + - name: Check default branch + run: | + echo "Current Branch: ${{ github.ref_name }}" + echo "Default Branch: ${{ github.event.repository.default_branch }}" + if [[ ${{ github.ref_name }} != ${{ github.event.repository.default_branch }} ]] + then + echo "This workflow can only be run on default branch" + exit 1 + fi + - name: Pre-process Release Name id: pre_process_release_name - env: - RELEASE_NAME: "${{ github.event.release.name }}" run: | # strip all whitespace RELEASE_NAME="${RELEASE_NAME//[[:space:]]/}" @@ -24,35 +38,47 @@ jobs: exit 1 fi echo "release_name=$RELEASE_NAME" >> $GITHUB_OUTPUT # Set as output using the new syntax + + # Perform a tag checkout to ensure tag is available + - name: Verify tag Exist + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + ref: ${{ steps.pre_process_release_name.outputs.release_name }} + fetch-depth: 1 + outputs: release_name: ${{ steps.pre_process_release_name.outputs.release_name }} - artifacts: + build: runs-on: ubuntu-22.04 - needs: preprocess_release + needs: validate env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job - permissions: - contents: write + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job outputs: tarSha: ${{steps.hashes.outputs.tarSha}} zipSha: ${{steps.hashes.outputs.zipSha}} steps: - - name: checkout + - name: Checkout tag uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + ref: ${{ env.RELEASE_NAME }} + - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: distribution: temurin java-version: 21 - - name: setup gradle + + - name: Setup gradle uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 with: cache-disabled: true - - name: assemble release + + - name: Assemble release run: ./gradlew -Prelease.releaseVersion=${{env.RELEASE_NAME}} -Pversion=${{env.RELEASE_NAME}} assemble - - name: hashes + + - name: Hashes id: hashes run: | cd build/distributions @@ -60,37 +86,56 @@ jobs: echo "tarSha=$(shasum -a 256 besu*.tar.gz)" echo "zipSha=$(shasum -a 256 besu*.zip)" >> $GITHUB_OUTPUT echo "tarSha=$(shasum -a 256 besu*.tar.gz)" >> $GITHUB_OUTPUT - - name: upload tarball + shasum -a 256 besu-${{env.RELEASE_NAME}}.tar.gz > besu-${{env.RELEASE_NAME}}.tar.gz.sha256 + shasum -a 256 besu-${{env.RELEASE_NAME}}.zip > besu-${{env.RELEASE_NAME}}.zip.sha256 + + - name: Upload tarball uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 with: - path: 'build/distributions/besu*.tar.gz' + path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.tar.gz' name: besu-${{ env.RELEASE_NAME }}.tar.gz compression-level: 0 + - name: upload zipfile uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 with: - path: 'build/distributions/besu*.zip' + path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.zip' name: besu-${{ env.RELEASE_NAME }}.zip compression-level: 0 - testWindows: + - name: upload checksum zip + uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 + with: + path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.zip.sha256' + name: besu-${{ env.RELEASE_NAME }}.zip.sha256 + compression-level: 0 + + - name: upload checksum tar.gz + uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 + with: + path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.tar.gz.sha256' + name: besu-${{ env.RELEASE_NAME }}.tar.gz.sha256 + compression-level: 0 + + test-windows: runs-on: windows-2022 - needs: artifacts - timeout-minutes: 10 + needs: ["build"] + timeout-minutes: 5 steps: - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: distribution: temurin java-version: 21 + - name: Download zip uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe with: pattern: besu-*.zip merge-multiple: true - - name: test Besu + + - name: Test run: | - dir unzip besu-*.zip -d besu-tmp cd besu-tmp mv besu-* ../besu @@ -98,81 +143,49 @@ jobs: besu\bin\besu.bat --help besu\bin\besu.bat --version - publish: - runs-on: ubuntu-22.04 - needs: [preprocess_release, testWindows, artifacts] - env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job - permissions: - contents: write - steps: - - name: Download archives - uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe - with: - pattern: besu-* - merge-multiple: true - path: 'build/distributions' - - name: Upload Release assets - uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 - with: - append_body: true - files: | - build/distributions/besu*.tar.gz - build/distributions/besu*.zip - body: | - ${{needs.artifacts.outputs.tarSha}} - ${{needs.artifacts.outputs.zipSha}} - - - - artifactoryPublish: + test-linux: runs-on: ubuntu-22.04 - needs: [preprocess_release, artifacts] - env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job + needs: ["build"] + timeout-minutes: 5 steps: - - name: checkout - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: distribution: temurin java-version: 21 - - name: setup gradle - uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 + + - name: Download tar.gz + uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe with: - cache-disabled: true - - name: Artifactory Publish - env: - ARTIFACTORY_USER: ${{ secrets.BESU_ARTIFACTORY_USER }} - ARTIFACTORY_KEY: ${{ secrets.BESU_ARTIFACTORY_TOKEN }} - run: ./gradlew -Prelease.releaseVersion=${{ env.RELEASE_NAME }} -Pversion=${{env.RELEASE_NAME}} artifactoryPublish + pattern: besu-*.tar.gz + merge-multiple: true + + - name: Test + run: | + tar zxvf besu-*.tar.gz + rm -f besu-*.tar.gz + mv besu-* besu-test + besu-test/bin/besu --help + besu-test/bin/besu --version - hadolint: + docker-lint: runs-on: ubuntu-22.04 + needs: [test-linux, test-windows] + env: + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job steps: - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - - name: Set up Java - uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: - distribution: temurin - java-version: 21 - - name: setup gradle - uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 - with: - cache-disabled: true + ref: ${{ env.RELEASE_NAME }} + - name: hadoLint run: docker run --rm -i hadolint/hadolint < docker/Dockerfile - - buildDocker: - needs: [preprocess_release, hadolint] + + docker-publish: + needs: [validate, docker-lint] env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job - permissions: - contents: read - packages: write - + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job strategy: fail-fast: false matrix: @@ -192,30 +205,39 @@ jobs: echo "PLATFORM_PAIR=linux-arm64" >> $GITHUB_OUTPUT echo "ARCH=arm64" >> $GITHUB_OUTPUT fi + - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + ref: ${{ env.RELEASE_NAME }} + - name: short sha id: shortSha run: echo "sha=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT + - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: distribution: temurin java-version: 21 + - name: setup gradle 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: registry: ${{ env.registry }} username: ${{ secrets.DOCKER_USER_RW }} password: ${{ secrets.DOCKER_PASSWORD_RW }} + - name: build and test docker uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 env: @@ -223,94 +245,130 @@ jobs: with: cache-disabled: true arguments: testDocker -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_NAME}} -Prelease.releaseVersion=${{ env.RELEASE_NAME }} + - name: publish env: architecture: ${{ steps.prep.outputs.ARCH }} run: ./gradlew --no-daemon dockerUpload -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_NAME}} -Prelease.releaseVersion=${{ env.RELEASE_NAME }} - multiArch: - needs: [preprocess_release, buildDocker] - env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job + docker-manifest: + needs: [validate, docker-publish] runs-on: ubuntu-22.04 - permissions: - contents: read - packages: write + env: + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job steps: - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + ref: ${{ env.RELEASE_NAME }} + - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: distribution: temurin java-version: 21 + - name: setup gradle uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 with: cache-disabled: true + - name: login to ${{ env.registry }} uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d with: registry: ${{ env.registry }} username: ${{ secrets.DOCKER_USER_RW }} password: ${{ secrets.DOCKER_PASSWORD_RW }} + - name: multi-arch docker run: ./gradlew manifestDocker -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_NAME}} -Prelease.releaseVersion=${{ env.RELEASE_NAME }} - amendNotes: - needs: [preprocess_release, multiArch] + docker-verify: + needs: [validate,docker-manifest] env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + runs-on: ubuntu-22.04 + permissions: + contents: read + actions: write + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + + - name: Trigger container verify + run: echo '{"version":"${{ env.RELEASE_NAME }}","verify-latest-version":"false"}' | gh workflow run container-verify.yml --json + env: + GH_TOKEN: ${{ github.token }} + + release-draft: runs-on: ubuntu-22.04 + needs: [validate, test-linux, test-windows] permissions: contents: write + env: + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} steps: - - name: add pull command to release notes - uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 + - name: Checkout Repo + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 with: - append_body: true - body: | - `docker pull ${{env.registry}}/${{secrets.DOCKER_ORG}}/besu:${{env.RELEASE_NAME}}` + ref: ${{ env.RELEASE_NAME }} - dockerPromoteX64: - needs: [preprocess_release, multiArch] - env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job + - name: Download Besu artifacts + uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe + with: + pattern: besu-${{env.RELEASE_NAME}}* + merge-multiple: true + + - name: Draft release notes + run: | + echo "## ${{env.RELEASE_NAME}}" > draft-release-notes.md + echo "## Upcoming Breaking Changes" >> draft-release-notes.md + echo "## Breaking Changes" >> draft-release-notes.md + echo "## Additions and Improvements" >> draft-release-notes.md + echo "## Bug fixes" >> draft-release-notes.md + echo "`$(cat besu-${{env.RELEASE_NAME}}.zip.sha256)`" >> draft-release-notes.md + echo "`$(cat besu-${{env.RELEASE_NAME}}.tar.gz.sha256)`" >> draft-release-notes.md + cat besu-${{env.RELEASE_NAME}}.zip.sha256 >> draft-release-notes.md + cat besu-${{env.RELEASE_NAME}}.tar.gz.sha256 >> draft-release-notes.md + + - name: Draft release + run: | + gh release create \ + --draft \ + --title=${{env.RELEASE_NAME}} \ + --notes-file draft-release-notes.md \ + --verify-tag ${{env.RELEASE_NAME}} \ + besu-${{env.RELEASE_NAME}}.tar.gz \ + besu-${{env.RELEASE_NAME}}.zip \ + besu-${{env.RELEASE_NAME}}.zip.sha256 \ + besu-${{env.RELEASE_NAME}}.tar.gz.sha256 + env: + GH_TOKEN: ${{ github.token }} + + artifactory: runs-on: ubuntu-22.04 + needs: [validate, test-linux, test-windows] + env: + RELEASE_NAME: ${{ needs.validate.outputs.release_name }} steps: - - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - - uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 + - name: checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + ref: ${{ env.RELEASE_NAME }} + + - name: Set up Java + uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: distribution: temurin java-version: 21 - cache: gradle - - name: login to ${{ env.registry }} - uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d - with: - registry: ${{ env.registry }} - username: ${{ secrets.DOCKER_USER_RW }} - password: ${{ secrets.DOCKER_PASSWORD_RW }} - - name: Setup Gradle + + - name: setup gradle uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 with: cache-disabled: true - - name: Docker upload - run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_NAME }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" dockerUploadRelease - - name: Docker manifest - run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_NAME }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" manifestDockerRelease - verifyContainer: - needs: [preprocess_release, dockerPromoteX64] - env: - RELEASE_NAME: ${{ needs.preprocess_release.outputs.release_name }} # Use the output from the pre_process_release job - runs-on: ubuntu-22.04 - permissions: - contents: read - actions: write - steps: - - name: Checkout - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - - name: Trigger container verify - run: echo '{"version":"${{ env.RELEASE_NAME }}","verify-latest-version":"true"}' | gh workflow run container-verify.yml --json + - name: Artifactory Publish env: - GH_TOKEN: ${{ github.token }} + ARTIFACTORY_USER: ${{ secrets.BESU_ARTIFACTORY_USER }} + ARTIFACTORY_KEY: ${{ secrets.BESU_ARTIFACTORY_TOKEN }} + run: ./gradlew -Prelease.releaseVersion=${{ env.RELEASE_NAME }} -Pversion=${{env.RELEASE_NAME}} artifactoryPublish From 49bf37cc31f22b29e539e205bd362695e7e9aeb6 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 2 Oct 2024 12:45:26 +1000 Subject: [PATCH 08/14] Expose chain ID in the BlockchainService plugin API (#7702) Signed-off-by: Gabriel-Trintinalia --- CHANGELOG.md | 1 + .../hyperledger/besu/services/BlockchainServiceImpl.java | 9 +++++++++ plugin-api/build.gradle | 2 +- .../besu/plugin/services/BlockchainService.java | 9 ++++++++- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b499b178..cb965bfb10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Add configuration of Consolidation Request Contract Address via genesis configuration [#7647](https://github.com/hyperledger/besu/pull/7647) - Interrupt pending transaction processing on block creation timeout [#7673](https://github.com/hyperledger/besu/pull/7673) - Align gas cap calculation for transaction simulation to Geth approach [#7703](https://github.com/hyperledger/besu/pull/7703) +- Expose chainId in the `BlockchainService` [7702](https://github.com/hyperledger/besu/pull/7702) ### Bug fixes - Fix mounted data path directory permissions for besu user [#7575](https://github.com/hyperledger/besu/pull/7575) diff --git a/besu/src/main/java/org/hyperledger/besu/services/BlockchainServiceImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BlockchainServiceImpl.java index 1f014ee061..e981a3e78d 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BlockchainServiceImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BlockchainServiceImpl.java @@ -30,6 +30,7 @@ import org.hyperledger.besu.plugin.data.BlockHeader; import org.hyperledger.besu.plugin.data.TransactionReceipt; import org.hyperledger.besu.plugin.services.BlockchainService; +import java.math.BigInteger; import java.util.List; import java.util.Optional; import java.util.function.Supplier; @@ -182,4 +183,12 @@ public class BlockchainServiceImpl implements BlockchainService { } }; } + + @Override + public Optional getChainId() { + if (protocolSchedule == null) { + return Optional.empty(); + } + return protocolSchedule.getChainId(); + } } diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index 6d3a1f93a9..dd2da0a15e 100644 --- a/plugin-api/build.gradle +++ b/plugin-api/build.gradle @@ -71,7 +71,7 @@ Calculated : ${currentHash} tasks.register('checkAPIChanges', FileStateChecker) { description = "Checks that the API for the Plugin-API project does not change without deliberate thought" files = sourceSets.main.allJava.files - knownHash = '5H+3gUzCwZtLByfnk11kf+kAPwykQ+WR+n3xWgyfsyY=' + knownHash = '4jVaj9yW88nHbX0KmTR3dPQRvj9x8Pvh5E9Ry7KRT6w=' } check.dependsOn('checkAPIChanges') diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BlockchainService.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BlockchainService.java index 84a573aadc..a89d1144af 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BlockchainService.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/BlockchainService.java @@ -22,6 +22,7 @@ import org.hyperledger.besu.plugin.data.BlockContext; import org.hyperledger.besu.plugin.data.BlockHeader; import org.hyperledger.besu.plugin.data.TransactionReceipt; +import java.math.BigInteger; import java.util.List; import java.util.Optional; @@ -106,5 +107,11 @@ public interface BlockchainService extends BesuService { * @throws UnsupportedOperationException if the network is a PoS network */ void setSafeBlock(Hash blockHash) throws IllegalArgumentException, UnsupportedOperationException; - ; + + /** + * Get the chain id + * + * @return the chain id + */ + Optional getChainId(); } From 9310e1031df10107cfb07516fc245798bf4e493d Mon Sep 17 00:00:00 2001 From: Karim Taam Date: Wed, 2 Oct 2024 10:00:52 +0200 Subject: [PATCH 09/14] Fix storage range issue during snapsync (#7624) Signed-off-by: Karim Taam Co-authored-by: Simon Dudley --- .../request/StorageRangeDataRequest.java | 10 +- .../snapsync/AccountHealingTrackingTest.java | 105 +++++++++++++++--- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java index 14f92e2c3f..4c50926303 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/StorageRangeDataRequest.java @@ -16,7 +16,6 @@ package org.hyperledger.besu.ethereum.eth.sync.snapsync.request; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.RequestType.STORAGE_RANGE; import static org.hyperledger.besu.ethereum.eth.sync.snapsync.StackTrie.FlatDatabaseUpdater.noop; -import static org.hyperledger.besu.ethereum.trie.RangeManager.MAX_RANGE; import static org.hyperledger.besu.ethereum.trie.RangeManager.MIN_RANGE; import static org.hyperledger.besu.ethereum.trie.RangeManager.findNewBeginElementInRange; import static org.hyperledger.besu.ethereum.trie.RangeManager.getRangeCount; @@ -192,12 +191,13 @@ public class StorageRangeDataRequest extends SnapDataRequest { getRootHash(), accountHash, storageRoot, key, value); childRequests.add(storageRangeDataRequest); }); - if (startKeyHash.equals(MIN_RANGE) && endKeyHash.equals(MAX_RANGE)) { - // need to heal this account storage - downloadState.addAccountToHealingList(CompactEncoding.bytesToPath(accountHash)); - } }); + if (startKeyHash.equals(MIN_RANGE) && !taskElement.proofs().isEmpty()) { + // need to heal this account storage + downloadState.addAccountToHealingList(CompactEncoding.bytesToPath(accountHash)); + } + return childRequests.stream(); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java index 70bdbf8b80..8e4e40311f 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/AccountHealingTrackingTest.java @@ -64,9 +64,7 @@ public class AccountHealingTrackingTest { DataStorageConfiguration.DEFAULT_BONSAI_CONFIG); private WorldStateStorageCoordinator worldStateStorageCoordinator; - private WorldStateProofProvider worldStateProofProvider; - private MerkleTrie accountStateTrie; @Mock SnapWorldDownloadState snapWorldDownloadState; @@ -82,9 +80,7 @@ public class AccountHealingTrackingTest { } @Test - void avoidMarkingAccountWhenStorageProofValid() { - - // generate valid proof + void shouldMarkAccountForHealingWhenStorageProofIsReceived() { final Hash accountHash = Hash.hash(accounts.get(0)); final StateTrieAccountValue stateTrieAccountValue = StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); @@ -108,7 +104,7 @@ public class AccountHealingTrackingTest { root -> RangeStorageEntriesCollector.collectEntries( collector, visitor, root, Hash.ZERO)); - // generate the proof + final List proofs = worldStateProofProvider.getStorageProofRelatedNodes( Hash.wrap(storageTrie.getRootHash()), accountHash, Hash.ZERO); @@ -127,11 +123,53 @@ public class AccountHealingTrackingTest { snapWorldDownloadState, worldStateProofProvider, slots, new ArrayDeque<>(proofs)); storageRangeDataRequest.getChildRequests( snapWorldDownloadState, worldStateStorageCoordinator, null); + + verify(snapWorldDownloadState).addAccountToHealingList(any(Bytes.class)); + } + + @Test + void shouldNotMarkAccountForHealingWhenAllStorageIsReceivedWithoutProof() { + final Hash accountHash = Hash.hash(accounts.get(0)); + final StateTrieAccountValue stateTrieAccountValue = + StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); + + final StoredMerklePatriciaTrie storageTrie = + new StoredMerklePatriciaTrie<>( + new StoredNodeFactory<>( + (location, hash) -> + worldStateKeyValueStorage.getAccountStorageTrieNode( + accountHash, location, hash), + Function.identity(), + Function.identity()), + stateTrieAccountValue.getStorageRoot()); + + final RangeStorageEntriesCollector collector = + RangeStorageEntriesCollector.createCollector(Hash.ZERO, MAX_RANGE, 10, Integer.MAX_VALUE); + final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); + final TreeMap slots = + (TreeMap) + storageTrie.entriesFrom( + root -> + RangeStorageEntriesCollector.collectEntries( + collector, visitor, root, Hash.ZERO)); + + final StorageRangeDataRequest storageRangeDataRequest = + SnapDataRequest.createStorageRangeDataRequest( + Hash.wrap(accountStateTrie.getRootHash()), + accountHash, + storageTrie.getRootHash(), + Hash.ZERO, + MAX_RANGE); + storageRangeDataRequest.addResponse( + snapWorldDownloadState, worldStateProofProvider, slots, new ArrayDeque<>()); + storageRangeDataRequest.getChildRequests( + snapWorldDownloadState, worldStateStorageCoordinator, null); + verify(snapWorldDownloadState, never()).addAccountToHealingList(any(Bytes.class)); } @Test - void markAccountOnInvalidStorageProof() { + void shouldMarkAccountForHealingOnInvalidStorageProof() { final Hash accountHash = Hash.hash(accounts.get(0)); final StateTrieAccountValue stateTrieAccountValue = StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); @@ -157,8 +195,7 @@ public class AccountHealingTrackingTest { } @Test - void markAccountOnPartialStorageRange() { - // generate valid proof + void shouldMarkAccountForHealingOnInvalidStorageWithoutProof() { final Hash accountHash = Hash.hash(accounts.get(0)); final StateTrieAccountValue stateTrieAccountValue = StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); @@ -174,11 +211,46 @@ public class AccountHealingTrackingTest { stateTrieAccountValue.getStorageRoot()); final RangeStorageEntriesCollector collector = - RangeStorageEntriesCollector.createCollector( + RangeStorageEntriesCollector.createCollector(Hash.ZERO, MAX_RANGE, 1, Integer.MAX_VALUE); + final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); + final TreeMap slots = + (TreeMap) + storageTrie.entriesFrom( + root -> + RangeStorageEntriesCollector.collectEntries( + collector, visitor, root, Hash.ZERO)); + + final StorageRangeDataRequest storageRangeDataRequest = + SnapDataRequest.createStorageRangeDataRequest( + Hash.wrap(accountStateTrie.getRootHash()), + accountHash, + storageTrie.getRootHash(), Hash.ZERO, - MAX_RANGE, - 1, - Integer.MAX_VALUE); // limit to 1 in order to have a partial range + MAX_RANGE); + storageRangeDataRequest.addResponse( + snapWorldDownloadState, worldStateProofProvider, slots, new ArrayDeque<>()); + + verify(snapWorldDownloadState).addAccountToHealingList(any(Bytes.class)); + } + + @Test + void shouldMarkAccountForHealingOnPartialStorageRange() { + final Hash accountHash = Hash.hash(accounts.get(0)); + final StateTrieAccountValue stateTrieAccountValue = + StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); + + final StoredMerklePatriciaTrie storageTrie = + new StoredMerklePatriciaTrie<>( + new StoredNodeFactory<>( + (location, hash) -> + worldStateKeyValueStorage.getAccountStorageTrieNode( + accountHash, location, hash), + Function.identity(), + Function.identity()), + stateTrieAccountValue.getStorageRoot()); + + final RangeStorageEntriesCollector collector = + RangeStorageEntriesCollector.createCollector(Hash.ZERO, MAX_RANGE, 1, Integer.MAX_VALUE); final TrieIterator visitor = RangeStorageEntriesCollector.createVisitor(collector); final TreeMap slots = (TreeMap) @@ -186,7 +258,7 @@ public class AccountHealingTrackingTest { root -> RangeStorageEntriesCollector.collectEntries( collector, visitor, root, Hash.ZERO)); - // generate the proof + final List proofs = worldStateProofProvider.getStorageProofRelatedNodes( Hash.wrap(storageTrie.getRootHash()), accountHash, Hash.ZERO); @@ -205,14 +277,14 @@ public class AccountHealingTrackingTest { snapWorldDownloadState, worldStateProofProvider, slots, new ArrayDeque<>(proofs)); verify(snapWorldDownloadState, never()).addAccountToHealingList(any(Bytes.class)); - // should mark during the getchild request storageRangeDataRequest.getChildRequests( snapWorldDownloadState, worldStateStorageCoordinator, null); + verify(snapWorldDownloadState).addAccountToHealingList(any(Bytes.class)); } @Test - void avoidMarkingAccountOnValidStorageTrieNodeDetection() { + void shouldNotMarkAccountForHealingOnValidStorageTrieNodeDetection() { final Hash accountHash = Hash.hash(accounts.get(0)); final StateTrieAccountValue stateTrieAccountValue = StateTrieAccountValue.readFrom(RLP.input(accountStateTrie.get(accountHash).orElseThrow())); @@ -223,6 +295,7 @@ public class AccountHealingTrackingTest { Hash.wrap(accountStateTrie.getRootHash()), Bytes.EMPTY); storageTrieNodeHealingRequest.getExistingData(worldStateStorageCoordinator); + verify(snapWorldDownloadState, never()).addAccountToHealingList(any(Bytes.class)); } } From 94099d18f5e4901b8e66c8d7e88662aedbd3cd3b Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Wed, 2 Oct 2024 14:32:48 +0200 Subject: [PATCH 10/14] Add suport for --plugins option in acceptance tests (#7713) Signed-off-by: Fabio Di Fabio --- .../besu/tests/acceptance/dsl/node/BesuNode.java | 7 +++++++ .../dsl/node/ProcessBesuNodeRunner.java | 5 +++++ .../acceptance/dsl/node/ThreadBesuNodeRunner.java | 15 +++++++++++++-- .../node/configuration/BesuNodeConfiguration.java | 7 +++++++ .../BesuNodeConfigurationBuilder.java | 8 ++++++++ .../dsl/node/configuration/BesuNodeFactory.java | 1 + 6 files changed, 41 insertions(+), 2 deletions(-) diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java index 157421531b..4e48be0401 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java @@ -128,6 +128,7 @@ public class BesuNode implements NodeConfiguration, RunnableNode, AutoCloseable private boolean useWsForJsonRpc = false; private String token = null; private final List plugins = new ArrayList<>(); + private final List requestedPlugins; private final List extraCLIOptions; private final List staticNodes; private boolean isDnsEnabled = false; @@ -163,6 +164,7 @@ public class BesuNode implements NodeConfiguration, RunnableNode, AutoCloseable final boolean secp256k1Native, final boolean altbn128Native, final List plugins, + final List requestedPlugins, final List extraCLIOptions, final List staticNodes, final boolean isDnsEnabled, @@ -224,6 +226,7 @@ public class BesuNode implements NodeConfiguration, RunnableNode, AutoCloseable LOG.error("Could not find plugin \"{}\" in resources", pluginName); } }); + this.requestedPlugins = requestedPlugins; engineRpcConfiguration.ifPresent( config -> MergeConfigOptions.setMergeEnabled(config.isEnabled())); this.extraCLIOptions = extraCLIOptions; @@ -738,6 +741,10 @@ public class BesuNode implements NodeConfiguration, RunnableNode, AutoCloseable return plugins; } + public List getRequestedPlugins() { + return requestedPlugins; + } + @Override public List getExtraCLIOptions() { return extraCLIOptions; diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java index 6e00701ef2..62951a442d 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java @@ -451,6 +451,11 @@ public class ProcessBesuNodeRunner implements BesuNodeRunner { params.add("--logging=" + level); } + if (!node.getRequestedPlugins().isEmpty()) { + params.add( + "--plugins=" + node.getRequestedPlugins().stream().collect(Collectors.joining(","))); + } + params.addAll(node.getRunCommand()); return params; } diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java index 90de0b0e95..d4334fd93c 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java @@ -39,6 +39,7 @@ import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; +import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.BlobCacheModule; @@ -302,6 +303,12 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner { return toProvide.getExtraCLIOptions(); } + @Provides + @Named("RequestedPlugins") + public List provideRequestedPlugins() { + return toProvide.getRequestedPlugins(); + } + @Provides Path provideDataDir() { return toProvide.homeDirectory(); @@ -469,7 +476,8 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner { final RpcEndpointServiceImpl rpcEndpointServiceImpl, final BesuConfiguration commonPluginConfiguration, final PermissioningServiceImpl permissioningService, - final @Named("ExtraCLIOptions") List extraCLIOptions) { + final @Named("ExtraCLIOptions") List extraCLIOptions, + final @Named("RequestedPlugins") List requestedPlugins) { final CommandLine commandLine = new CommandLine(CommandSpec.create()); final BesuPluginContextImpl besuPluginContext = new BesuPluginContextImpl(); besuPluginContext.addService(StorageService.class, storageService); @@ -504,7 +512,10 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner { besuPluginContext.addService(PrivacyPluginService.class, new PrivacyPluginServiceImpl()); besuPluginContext.initialize( - new PluginConfiguration.Builder().pluginsDir(pluginsPath).build()); + new PluginConfiguration.Builder() + .pluginsDir(pluginsPath) + .requestedPlugins(requestedPlugins.stream().map(PluginInfo::new).toList()) + .build()); besuPluginContext.registerPlugins(); commandLine.parseArgs(extraCLIOptions.toArray(new String[0])); diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java index c77a6c8ac7..ba69e4ddd7 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java @@ -64,6 +64,7 @@ public class BesuNodeConfiguration { private final boolean secp256k1Native; private final boolean altbn128Native; private final List plugins; + private final List requestedPlugins; private final List extraCLIOptions; private final List staticNodes; private final boolean isDnsEnabled; @@ -102,6 +103,7 @@ public class BesuNodeConfiguration { final boolean secp256k1Native, final boolean altbn128Native, final List plugins, + final List requestedPlugins, final List extraCLIOptions, final List staticNodes, final boolean isDnsEnabled, @@ -137,6 +139,7 @@ public class BesuNodeConfiguration { this.secp256k1Native = secp256k1Native; this.altbn128Native = altbn128Native; this.plugins = plugins; + this.requestedPlugins = requestedPlugins; this.extraCLIOptions = extraCLIOptions; this.staticNodes = staticNodes; this.isDnsEnabled = isDnsEnabled; @@ -239,6 +242,10 @@ public class BesuNodeConfiguration { return plugins; } + public List getRequestedPlugins() { + return requestedPlugins; + } + public List getExtraCLIOptions() { return extraCLIOptions; } diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java index d8b5e0f904..ead01ce97d 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java @@ -93,6 +93,7 @@ public class BesuNodeConfigurationBuilder { private boolean secp256K1Native = true; private boolean altbn128Native = true; private final List plugins = new ArrayList<>(); + private final List requestedPlugins = new ArrayList<>(); private final List extraCLIOptions = new ArrayList<>(); private List staticNodes = new ArrayList<>(); private boolean isDnsEnabled = false; @@ -448,6 +449,12 @@ public class BesuNodeConfigurationBuilder { return this; } + public BesuNodeConfigurationBuilder requestedPlugins(final List requestedPlugins) { + this.requestedPlugins.clear(); + this.requestedPlugins.addAll(requestedPlugins); + return this; + } + public BesuNodeConfigurationBuilder extraCLIOptions(final List extraCLIOptions) { this.extraCLIOptions.clear(); this.extraCLIOptions.addAll(extraCLIOptions); @@ -545,6 +552,7 @@ public class BesuNodeConfigurationBuilder { secp256K1Native, altbn128Native, plugins, + requestedPlugins, extraCLIOptions, staticNodes, isDnsEnabled, diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java index 0451d2a721..b39274c3d9 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java @@ -77,6 +77,7 @@ public class BesuNodeFactory { config.isSecp256k1Native(), config.isAltbn128Native(), config.getPlugins(), + config.getRequestedPlugins(), config.getExtraCLIOptions(), config.getStaticNodes(), config.isDnsEnabled(), From f4dc48d94d276f1ed504b707b8f9ccf278a5cc63 Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Wed, 2 Oct 2024 11:46:34 -0400 Subject: [PATCH 11/14] Simplifying BesuCommand step 1 (#7682) Simplifying BesuCommand --------- Signed-off-by: Justin Florentine --- .../dsl/node/ProcessBesuNodeRunner.java | 2 +- .../org/hyperledger/besu/cli/BesuCommand.java | 305 ++++-------------- .../{stable => }/DataStorageOptions.java | 3 +- .../stable/ApiConfigurationOptions.java | 1 + .../stable/EngineRPCConfiguration.java | 36 +++ .../cli/options/stable/EngineRPCOptions.java | 81 +++++ .../cli/options/stable/GraphQlOptions.java | 1 + .../options/stable/JsonRpcHttpOptions.java | 34 +- .../options/stable/MetricsOptionGroup.java | 1 + .../options/stable/P2PDiscoveryOptions.java | 238 ++++++++++++++ .../options/stable/PermissionsOptions.java | 1 + .../options/unstable/MetricsCLIOptions.java | 1 + .../P2PTLSConfigOptions.java | 2 +- .../subcommands/storage/TrieLogHelper.java | 4 +- .../besu/components/BesuCommandModule.java | 16 +- .../besu/cli/CommandTestAbstract.java | 2 +- .../stable/DataStorageOptionsTest.java | 1 + .../besu/config/MergeConfigOptions.java | 2 + .../discovery/P2PDiscoveryConfiguration.java | 39 +++ 19 files changed, 501 insertions(+), 269 deletions(-) rename besu/src/main/java/org/hyperledger/besu/cli/options/{stable => }/DataStorageOptions.java (99%) create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCConfiguration.java create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCOptions.java create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PDiscoveryOptions.java rename besu/src/main/java/org/hyperledger/besu/cli/options/{stable => unstable}/P2PTLSConfigOptions.java (99%) create mode 100644 ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/P2PDiscoveryConfiguration.java diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java index 62951a442d..e08fb29f67 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java @@ -17,8 +17,8 @@ package org.hyperledger.besu.tests.acceptance.dsl.node; import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.UTF_8; +import org.hyperledger.besu.cli.options.DataStorageOptions; import org.hyperledger.besu.cli.options.TransactionPoolOptions; -import org.hyperledger.besu.cli.options.stable.DataStorageOptions; import org.hyperledger.besu.cli.options.unstable.NetworkingOptions; import org.hyperledger.besu.ethereum.api.jsonrpc.ipc.JsonRpcIpcConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index e93518cad4..fa59f48dd7 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -24,7 +24,6 @@ import static org.hyperledger.besu.cli.config.NetworkName.MAINNET; import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPENDENCY_WARNING_MSG; import static org.hyperledger.besu.cli.util.CommandLineUtils.isOptionSet; import static org.hyperledger.besu.controller.BesuController.DATABASE_PATH; -import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration.DEFAULT_ENGINE_JSON_RPC_PORT; import static org.hyperledger.besu.ethereum.api.jsonrpc.authentication.EngineAuthService.EPHEMERAL_JWT_FILE; import static org.hyperledger.besu.nat.kubernetes.KubernetesNatManager.DEFAULT_BESU_SERVICE_NAME_FILTER; @@ -38,22 +37,22 @@ import org.hyperledger.besu.cli.config.EthNetworkConfig; import org.hyperledger.besu.cli.config.NetworkName; import org.hyperledger.besu.cli.config.ProfilesCompletionCandidates; import org.hyperledger.besu.cli.converter.MetricCategoryConverter; -import org.hyperledger.besu.cli.converter.PercentageConverter; -import org.hyperledger.besu.cli.converter.SubnetInfoConverter; import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty; import org.hyperledger.besu.cli.error.BesuExecutionExceptionHandler; import org.hyperledger.besu.cli.error.BesuParameterExceptionHandler; +import org.hyperledger.besu.cli.options.DataStorageOptions; import org.hyperledger.besu.cli.options.MiningOptions; import org.hyperledger.besu.cli.options.TransactionPoolOptions; import org.hyperledger.besu.cli.options.stable.ApiConfigurationOptions; -import org.hyperledger.besu.cli.options.stable.DataStorageOptions; +import org.hyperledger.besu.cli.options.stable.EngineRPCConfiguration; +import org.hyperledger.besu.cli.options.stable.EngineRPCOptions; import org.hyperledger.besu.cli.options.stable.EthstatsOptions; import org.hyperledger.besu.cli.options.stable.GraphQlOptions; import org.hyperledger.besu.cli.options.stable.JsonRpcHttpOptions; import org.hyperledger.besu.cli.options.stable.LoggingLevelOption; import org.hyperledger.besu.cli.options.stable.MetricsOptionGroup; import org.hyperledger.besu.cli.options.stable.NodePrivateKeyFileOption; -import org.hyperledger.besu.cli.options.stable.P2PTLSConfigOptions; +import org.hyperledger.besu.cli.options.stable.P2PDiscoveryOptions; import org.hyperledger.besu.cli.options.stable.PermissionsOptions; import org.hyperledger.besu.cli.options.stable.PluginsConfigurationOptions; import org.hyperledger.besu.cli.options.stable.RpcWebsocketOptions; @@ -67,6 +66,7 @@ import org.hyperledger.besu.cli.options.unstable.MetricsCLIOptions; import org.hyperledger.besu.cli.options.unstable.NatOptions; import org.hyperledger.besu.cli.options.unstable.NativeLibraryOptions; import org.hyperledger.besu.cli.options.unstable.NetworkingOptions; +import org.hyperledger.besu.cli.options.unstable.P2PTLSConfigOptions; import org.hyperledger.besu.cli.options.unstable.PrivacyPluginOptions; import org.hyperledger.besu.cli.options.unstable.RPCOptions; import org.hyperledger.besu.cli.options.unstable.SynchronizerOptions; @@ -124,6 +124,7 @@ import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolCo import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; import org.hyperledger.besu.ethereum.mainnet.FrontierTargetingGasLimitCalculator; import org.hyperledger.besu.ethereum.p2p.config.DiscoveryConfiguration; +import org.hyperledger.besu.ethereum.p2p.discovery.P2PDiscoveryConfiguration; import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; import org.hyperledger.besu.ethereum.p2p.peers.StaticNodesParser; @@ -209,7 +210,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.math.BigInteger; -import java.net.InetAddress; import java.net.SocketException; import java.net.URI; import java.net.URL; @@ -247,7 +247,6 @@ import io.vertx.core.Vertx; import io.vertx.core.VertxOptions; import io.vertx.core.json.DecodeException; import io.vertx.core.metrics.MetricsOptions; -import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt256; import org.slf4j.Logger; @@ -416,7 +415,9 @@ public class BesuCommand implements DefaultCommandValues, Runnable { // P2P Discovery Option Group @CommandLine.ArgGroup(validate = false, heading = "@|bold P2P Discovery Options|@%n") - P2PDiscoveryOptionGroup p2PDiscoveryOptionGroup = new P2PDiscoveryOptionGroup(); + P2PDiscoveryOptions p2PDiscoveryOptions = new P2PDiscoveryOptions(); + + P2PDiscoveryConfiguration p2PDiscoveryConfig; private final TransactionSelectionServiceImpl transactionSelectionServiceImpl; private final TransactionPoolValidatorServiceImpl transactionValidatorServiceImpl; @@ -424,163 +425,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable { private final BlockchainServiceImpl blockchainServiceImpl; private BesuComponent besuComponent; - static class P2PDiscoveryOptionGroup { - - // Public IP stored to prevent having to research it each time we need it. - private InetAddress autoDiscoveredDefaultIP = null; - - // Completely disables P2P within Besu. - @Option( - names = {"--p2p-enabled"}, - description = "Enable P2P functionality (default: ${DEFAULT-VALUE})", - arity = "1") - private final Boolean p2pEnabled = true; - - // Boolean option to indicate if peers should NOT be discovered, default to - // false indicates that - // the peers should be discovered by default. - // - // This negative option is required because of the nature of the option that is - // true when - // added on the command line. You can't do --option=false, so false is set as - // default - // and you have not to set the option at all if you want it false. - // This seems to be the only way it works with Picocli. - // Also many other software use the same negative option scheme for false - // defaults - // meaning that it's probably the right way to handle disabling options. - @Option( - names = {"--discovery-enabled"}, - description = "Enable P2P discovery (default: ${DEFAULT-VALUE})", - arity = "1") - private final Boolean peerDiscoveryEnabled = true; - - // A list of bootstrap nodes can be passed - // and a hardcoded list will be used otherwise by the Runner. - // NOTE: we have no control over default value here. - @Option( - names = {"--bootnodes"}, - paramLabel = "", - description = - "Comma separated enode URLs for P2P discovery bootstrap. " - + "Default is a predefined list.", - split = ",", - arity = "0..*") - private final List bootNodes = null; - - @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. - @Option( - names = {"--p2p-host"}, - paramLabel = MANDATORY_HOST_FORMAT_HELP, - description = "IP address this node advertises to its peers (default: ${DEFAULT-VALUE})", - arity = "1") - private String p2pHost = autoDiscoverDefaultIP().getHostAddress(); - - @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. - @Option( - names = {"--p2p-interface"}, - paramLabel = MANDATORY_HOST_FORMAT_HELP, - description = - "The network interface address on which this node listens for P2P communication (default: ${DEFAULT-VALUE})", - arity = "1") - private String p2pInterface = NetworkUtility.INADDR_ANY; - - @Option( - names = {"--p2p-port"}, - paramLabel = MANDATORY_PORT_FORMAT_HELP, - description = "Port on which to listen for P2P communication (default: ${DEFAULT-VALUE})", - arity = "1") - private final Integer p2pPort = EnodeURLImpl.DEFAULT_LISTENING_PORT; - - @Option( - names = {"--max-peers", "--p2p-peer-upper-bound"}, - paramLabel = MANDATORY_INTEGER_FORMAT_HELP, - description = "Maximum P2P connections that can be established (default: ${DEFAULT-VALUE})") - private final Integer maxPeers = DEFAULT_MAX_PEERS; - - @Option( - names = {"--remote-connections-limit-enabled"}, - description = - "Whether to limit the number of P2P connections initiated remotely. (default: ${DEFAULT-VALUE})") - private final Boolean isLimitRemoteWireConnectionsEnabled = true; - - @Option( - names = {"--remote-connections-max-percentage"}, - paramLabel = MANDATORY_DOUBLE_FORMAT_HELP, - description = - "The maximum percentage of P2P connections that can be initiated remotely. Must be between 0 and 100 inclusive. (default: ${DEFAULT-VALUE})", - arity = "1", - converter = PercentageConverter.class) - private final Percentage maxRemoteConnectionsPercentage = - Fraction.fromFloat(DEFAULT_FRACTION_REMOTE_WIRE_CONNECTIONS_ALLOWED).toPercentage(); - - @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. - @CommandLine.Option( - names = {"--discovery-dns-url"}, - description = "Specifies the URL to use for DNS discovery") - private String discoveryDnsUrl = null; - - @Option( - names = {"--random-peer-priority-enabled"}, - description = - "Allow for incoming connections to be prioritized randomly. This will prevent (typically small, stable) networks from forming impenetrable peer cliques. (default: ${DEFAULT-VALUE})") - private final Boolean randomPeerPriority = Boolean.FALSE; - - @Option( - names = {"--banned-node-ids", "--banned-node-id"}, - paramLabel = MANDATORY_NODE_ID_FORMAT_HELP, - description = "A list of node IDs to ban from the P2P network.", - split = ",", - arity = "1..*") - void setBannedNodeIds(final List values) { - try { - bannedNodeIds = - values.stream() - .filter(value -> !value.isEmpty()) - .map(EnodeURLImpl::parseNodeId) - .collect(Collectors.toList()); - } catch (final IllegalArgumentException e) { - throw new ParameterException( - new CommandLine(this), - "Invalid ids supplied to '--banned-node-ids'. " + e.getMessage()); - } - } - - // Boolean option to set that in a PoA network the bootnodes should always be queried during - // peer table refresh. If this flag is disabled bootnodes are only sent FINDN requests on first - // startup, meaning that an offline bootnode or network outage at the client can prevent it - // discovering any peers without a restart. - @Option( - names = {"--poa-discovery-retry-bootnodes"}, - description = - "Always use of bootnodes for discovery in PoA networks. Disabling this reverts " - + " to the same behaviour as non-PoA networks, where neighbours are only discovered from bootnodes on first startup." - + "(default: ${DEFAULT-VALUE})", - arity = "1") - private final Boolean poaDiscoveryRetryBootnodes = true; - - private Collection bannedNodeIds = new ArrayList<>(); - - // Used to discover the default IP of the client. - // Loopback IP is used by default as this is how smokeTests require it to be - // and it's probably a good security behaviour to default only on the localhost. - private InetAddress autoDiscoverDefaultIP() { - autoDiscoveredDefaultIP = - Optional.ofNullable(autoDiscoveredDefaultIP).orElseGet(InetAddress::getLoopbackAddress); - - return autoDiscoveredDefaultIP; - } - - @Option( - names = {"--net-restrict"}, - arity = "1..*", - split = ",", - converter = SubnetInfoConverter.class, - description = - "Comma-separated list of allowed IP subnets (e.g., '192.168.1.0/24,10.0.0.0/8').") - private List allowedSubnets; - } - @Option( names = {"--sync-mode"}, paramLabel = MANDATORY_MODE_FORMAT_HELP, @@ -647,42 +491,9 @@ public class BesuCommand implements DefaultCommandValues, Runnable { // Engine JSON-PRC Options @CommandLine.ArgGroup(validate = false, heading = "@|bold Engine JSON-RPC Options|@%n") - EngineRPCOptionGroup engineRPCOptionGroup = new EngineRPCOptionGroup(); - - static class EngineRPCOptionGroup { - @Option( - names = {"--engine-rpc-enabled"}, - description = - "enable the engine api, even in the absence of merge-specific configurations.") - private final Boolean overrideEngineRpcEnabled = false; - - @Option( - names = {"--engine-rpc-port", "--engine-rpc-http-port"}, - paramLabel = MANDATORY_PORT_FORMAT_HELP, - description = "Port to provide consensus client APIS on (default: ${DEFAULT-VALUE})", - arity = "1") - private final Integer engineRpcPort = DEFAULT_ENGINE_JSON_RPC_PORT; + EngineRPCOptions engineRPCOptions = new EngineRPCOptions(); - @Option( - names = {"--engine-jwt-secret"}, - paramLabel = MANDATORY_FILE_FORMAT_HELP, - description = "Path to file containing shared secret key for JWT signature verification") - private final Path engineJwtKeyFile = null; - - @Option( - names = {"--engine-jwt-disabled"}, - description = "Disable authentication for Engine APIs (default: ${DEFAULT-VALUE})") - private final Boolean isEngineAuthDisabled = false; - - @Option( - names = {"--engine-host-allowlist"}, - paramLabel = "[,...]... or * or all", - description = - "Comma separated list of hostnames to allow for ENGINE API access (applies to both HTTP and websockets), or * to accept any host (default: ${DEFAULT-VALUE})", - defaultValue = "localhost,127.0.0.1") - private final JsonRPCAllowlistHostsProperty engineHostsAllowlist = - new JsonRPCAllowlistHostsProperty(); - } + EngineRPCConfiguration engineRPCConfig = engineRPCOptions.toDomainObject(); // JSON-RPC HTTP Options @CommandLine.ArgGroup(validate = false, heading = "@|bold JSON-RPC HTTP Options|@%n") @@ -1409,13 +1220,13 @@ public class BesuCommand implements DefaultCommandValues, Runnable { private Runner buildRunner() { return synchronize( besuController, - p2PDiscoveryOptionGroup.p2pEnabled, + p2PDiscoveryConfig.p2pEnabled(), p2pTLSConfiguration, - p2PDiscoveryOptionGroup.peerDiscoveryEnabled, + p2PDiscoveryConfig.peerDiscoveryEnabled(), ethNetworkConfig, - p2PDiscoveryOptionGroup.p2pHost, - p2PDiscoveryOptionGroup.p2pInterface, - p2PDiscoveryOptionGroup.p2pPort, + p2PDiscoveryConfig.p2pHost(), + p2PDiscoveryConfig.p2pInterface(), + p2PDiscoveryConfig.p2pPort(), graphQLConfiguration, jsonRpcConfiguration, engineJsonRpcConfiguration, @@ -1614,7 +1425,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { private void validateOptions() { validateRequiredOptions(); issueOptionWarnings(); - validateP2PInterface(p2PDiscoveryOptionGroup.p2pInterface); + validateP2PInterface(p2PDiscoveryOptions.p2pInterface); validateMiningParams(); validateNatParams(); validateNetStatsParams(); @@ -1750,13 +1561,12 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } private void ensureValidPeerBoundParams() { - maxPeers = p2PDiscoveryOptionGroup.maxPeers; + maxPeers = p2PDiscoveryOptions.maxPeers; final Boolean isLimitRemoteWireConnectionsEnabled = - p2PDiscoveryOptionGroup.isLimitRemoteWireConnectionsEnabled; + p2PDiscoveryOptions.isLimitRemoteWireConnectionsEnabled; if (isLimitRemoteWireConnectionsEnabled) { final float fraction = - Fraction.fromPercentage(p2PDiscoveryOptionGroup.maxRemoteConnectionsPercentage) - .getValue(); + Fraction.fromPercentage(p2PDiscoveryOptions.maxRemoteConnectionsPercentage).getValue(); checkState( fraction >= 0.0 && fraction <= 1.0, "Fraction of remote connections allowed must be between 0.0 and 1.0 (inclusive)."); @@ -1820,7 +1630,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { logger, commandLine, "--p2p-enabled", - !p2PDiscoveryOptionGroup.p2pEnabled, + !p2PDiscoveryOptions.p2pEnabled, asList( "--bootnodes", "--discovery-enabled", @@ -1861,6 +1671,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } private void configure() throws Exception { + p2PDiscoveryConfig = p2PDiscoveryOptions.toDomainObject(); + engineRPCConfig = engineRPCOptions.toDomainObject(); checkPortClash(); checkIfRequiredPortsAreAvailable(); syncMode = getDefaultSyncModeIfNotSet(); @@ -1870,25 +1682,18 @@ public class BesuCommand implements DefaultCommandValues, Runnable { jsonRpcConfiguration = jsonRpcHttpOptions.jsonRpcConfiguration( - hostsAllowlist, - p2PDiscoveryOptionGroup.autoDiscoverDefaultIP().getHostAddress(), - unstableRPCOptions.getHttpTimeoutSec()); + hostsAllowlist, p2PDiscoveryOptions.p2pHost, unstableRPCOptions.getHttpTimeoutSec()); if (isEngineApiEnabled()) { - engineJsonRpcConfiguration = - createEngineJsonRpcConfiguration( - engineRPCOptionGroup.engineRpcPort, engineRPCOptionGroup.engineHostsAllowlist); + engineJsonRpcConfiguration = createEngineJsonRpcConfiguration(); } p2pTLSConfiguration = p2pTLSConfigOptions.p2pTLSConfiguration(commandLine); graphQLConfiguration = graphQlOptions.graphQLConfiguration( - hostsAllowlist, - p2PDiscoveryOptionGroup.autoDiscoverDefaultIP().getHostAddress(), - unstableRPCOptions.getHttpTimeoutSec()); + hostsAllowlist, p2PDiscoveryOptions.p2pHost, unstableRPCOptions.getHttpTimeoutSec()); + webSocketConfiguration = rpcWebsocketOptions.webSocketConfiguration( - hostsAllowlist, - p2PDiscoveryOptionGroup.autoDiscoverDefaultIP().getHostAddress(), - unstableRPCOptions.getWsTimeoutSec()); + hostsAllowlist, p2PDiscoveryConfig.p2pHost(), unstableRPCOptions.getWsTimeoutSec()); jsonRpcIpcConfiguration = jsonRpcIpcConfiguration( unstableIpcOptions.isEnabled(), @@ -2008,32 +1813,31 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .requiredBlocks(requiredBlocks) .reorgLoggingThreshold(reorgLoggingThreshold) .evmConfiguration(unstableEvmOptions.toDomainObject()) - .maxPeers(p2PDiscoveryOptionGroup.maxPeers) + .maxPeers(p2PDiscoveryOptions.maxPeers) .maxRemotelyInitiatedPeers(maxRemoteInitiatedPeers) - .randomPeerPriority(p2PDiscoveryOptionGroup.randomPeerPriority) + .randomPeerPriority(p2PDiscoveryOptions.randomPeerPriority) .chainPruningConfiguration(unstableChainPruningOptions.toDomainObject()) .cacheLastBlocks(numberOfblocksToCache) .genesisStateHashCacheEnabled(genesisStateHashCacheEnabled) .besuComponent(besuComponent); } - private JsonRpcConfiguration createEngineJsonRpcConfiguration( - final Integer engineListenPort, final List allowCallsFrom) { + private JsonRpcConfiguration createEngineJsonRpcConfiguration() { jsonRpcHttpOptions.checkDependencies(logger, commandLine); final JsonRpcConfiguration engineConfig = jsonRpcHttpOptions.jsonRpcConfiguration( - allowCallsFrom, - p2PDiscoveryOptionGroup.autoDiscoverDefaultIP().getHostAddress(), + engineRPCConfig.engineHostsAllowlist(), + p2PDiscoveryConfig.p2pHost(), unstableRPCOptions.getWsTimeoutSec()); - engineConfig.setPort(engineListenPort); + engineConfig.setPort(engineRPCConfig.engineRpcPort()); engineConfig.setRpcApis(Arrays.asList("ENGINE", "ETH")); engineConfig.setEnabled(isEngineApiEnabled()); - if (!engineRPCOptionGroup.isEngineAuthDisabled) { + if (!engineRPCConfig.isEngineAuthDisabled()) { engineConfig.setAuthenticationEnabled(true); engineConfig.setAuthenticationAlgorithm(JwtAlgorithm.HS256); - if (Objects.nonNull(engineRPCOptionGroup.engineJwtKeyFile) - && java.nio.file.Files.exists(engineRPCOptionGroup.engineJwtKeyFile)) { - engineConfig.setAuthenticationPublicKeyFile(engineRPCOptionGroup.engineJwtKeyFile.toFile()); + if (Objects.nonNull(engineRPCConfig.engineJwtKeyFile()) + && java.nio.file.Files.exists(engineRPCConfig.engineJwtKeyFile())) { + engineConfig.setAuthenticationPublicKeyFile(engineRPCConfig.engineJwtKeyFile().toFile()); } else { logger.warn( "Engine API authentication enabled without key file. Expect ephemeral jwt.hex file in datadir"); @@ -2090,7 +1894,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .enabled(metricsOptionGroup.getMetricsEnabled()) .host( Strings.isNullOrEmpty(metricsOptionGroup.getMetricsHost()) - ? p2PDiscoveryOptionGroup.autoDiscoverDefaultIP().getHostAddress() + ? p2PDiscoveryOptions.p2pHost : metricsOptionGroup.getMetricsHost()) .port(metricsOptionGroup.getMetricsPort()) .protocol(metricsOptionGroup.getMetricsProtocol()) @@ -2098,7 +1902,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .pushEnabled(metricsOptionGroup.getMetricsPushEnabled()) .pushHost( Strings.isNullOrEmpty(metricsOptionGroup.getMetricsPushHost()) - ? p2PDiscoveryOptionGroup.autoDiscoverDefaultIP().getHostAddress() + ? p2PDiscoveryOptions.autoDiscoverDefaultIP().getHostAddress() : metricsOptionGroup.getMetricsPushHost()) .pushPort(metricsOptionGroup.getMetricsPushPort()) .pushInterval(metricsOptionGroup.getMetricsPushInterval()) @@ -2441,7 +2245,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .apiConfiguration(apiConfiguration) .pidPath(pidPath) .dataDir(dataDir()) - .bannedNodeIds(p2PDiscoveryOptionGroup.bannedNodeIds) + .bannedNodeIds(p2PDiscoveryConfig.bannedNodeIds()) .metricsSystem((ObservableMetricsSystem) besuComponent.getMetricsSystem()) .permissioningService(permissioningService) .metricsConfiguration(metricsConfiguration) @@ -2453,8 +2257,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .storageProvider(keyValueStorageProvider(keyValueStorageName)) .rpcEndpointService(rpcEndpointServiceImpl) .enodeDnsConfiguration(getEnodeDnsConfiguration()) - .allowedSubnets(p2PDiscoveryOptionGroup.allowedSubnets) - .poaDiscoveryRetryBootnodes(p2PDiscoveryOptionGroup.poaDiscoveryRetryBootnodes) + .allowedSubnets(p2PDiscoveryConfig.allowedSubnets()) + .poaDiscoveryRetryBootnodes(p2PDiscoveryConfig.poaDiscoveryRetryBootnodes()) .build(); addShutdownHook(runner); @@ -2529,7 +2333,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } } - if (p2PDiscoveryOptionGroup.bootNodes == null) { + if (p2PDiscoveryOptions.bootNodes == null) { builder.setBootNodes(new ArrayList<>()); } builder.setDnsDiscoveryUrl(null); @@ -2541,8 +2345,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable { builder.setNetworkId(networkId); } - if (p2PDiscoveryOptionGroup.discoveryDnsUrl != null) { - builder.setDnsDiscoveryUrl(p2PDiscoveryOptionGroup.discoveryDnsUrl); + if (p2PDiscoveryOptions.discoveryDnsUrl != null) { + builder.setDnsDiscoveryUrl(p2PDiscoveryOptions.discoveryDnsUrl); } else { final Optional discoveryDnsUrlFromGenesis = genesisConfigOptionsSupplier.get().getDiscoveryOptions().getDiscoveryDnsUrl(); @@ -2550,9 +2354,9 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } List listBootNodes = null; - if (p2PDiscoveryOptionGroup.bootNodes != null) { + if (p2PDiscoveryOptions.bootNodes != null) { try { - listBootNodes = buildEnodes(p2PDiscoveryOptionGroup.bootNodes, getEnodeDnsConfiguration()); + listBootNodes = buildEnodes(p2PDiscoveryOptions.bootNodes, getEnodeDnsConfiguration()); } catch (final IllegalArgumentException e) { throw new ParameterException(commandLine, e.getMessage()); } @@ -2564,7 +2368,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } } if (listBootNodes != null) { - if (!p2PDiscoveryOptionGroup.peerDiscoveryEnabled) { + if (!p2PDiscoveryOptions.peerDiscoveryEnabled) { logger.warn("Discovery disabled: bootnodes will be ignored."); } DiscoveryConfiguration.assertValidBootnodes(listBootNodes); @@ -2699,12 +2503,12 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .filter(port -> port > 0) .forEach( port -> { - if (port.equals(p2PDiscoveryOptionGroup.p2pPort) + if (port.equals(p2PDiscoveryConfig.p2pPort()) && (NetworkUtility.isPortUnavailableForTcp(port) || NetworkUtility.isPortUnavailableForUdp(port))) { unavailablePorts.add(port); } - if (!port.equals(p2PDiscoveryOptionGroup.p2pPort) + if (!port.equals(p2PDiscoveryConfig.p2pPort()) && NetworkUtility.isPortUnavailableForTcp(port)) { unavailablePorts.add(port); } @@ -2724,15 +2528,14 @@ public class BesuCommand implements DefaultCommandValues, Runnable { */ private List getEffectivePorts() { final List effectivePorts = new ArrayList<>(); - addPortIfEnabled( - effectivePorts, p2PDiscoveryOptionGroup.p2pPort, p2PDiscoveryOptionGroup.p2pEnabled); + addPortIfEnabled(effectivePorts, p2PDiscoveryOptions.p2pPort, p2PDiscoveryOptions.p2pEnabled); addPortIfEnabled( effectivePorts, graphQlOptions.getGraphQLHttpPort(), graphQlOptions.isGraphQLHttpEnabled()); addPortIfEnabled( effectivePorts, jsonRpcHttpOptions.getRpcHttpPort(), jsonRpcHttpOptions.isRpcHttpEnabled()); addPortIfEnabled( effectivePorts, rpcWebsocketOptions.getRpcWsPort(), rpcWebsocketOptions.isRpcWsEnabled()); - addPortIfEnabled(effectivePorts, engineRPCOptionGroup.engineRpcPort, isEngineApiEnabled()); + addPortIfEnabled(effectivePorts, engineRPCConfig.engineRpcPort(), isEngineApiEnabled()); addPortIfEnabled( effectivePorts, metricsOptionGroup.getMetricsPort(), @@ -2859,7 +2662,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } private boolean isEngineApiEnabled() { - return engineRPCOptionGroup.overrideEngineRpcEnabled || isMergeEnabled(); + return engineRPCConfig.overrideEngineRpcEnabled() || isMergeEnabled(); } private SyncMode getDefaultSyncModeIfNotSet() { diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/DataStorageOptions.java similarity index 99% rename from besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java rename to besu/src/main/java/org/hyperledger/besu/cli/options/DataStorageOptions.java index 3d53a595ec..b442f6ee03 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/DataStorageOptions.java @@ -12,7 +12,7 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.cli.options.stable; +package org.hyperledger.besu.cli.options; import static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration.DEFAULT_BONSAI_LIMIT_TRIE_LOGS_ENABLED; import static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration.DEFAULT_BONSAI_MAX_LAYERS_TO_LOAD; @@ -22,7 +22,6 @@ import static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration. import static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration.Unstable.DEFAULT_BONSAI_CODE_USING_CODE_HASH_ENABLED; import static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration.Unstable.DEFAULT_BONSAI_FULL_FLAT_DB_ENABLED; -import org.hyperledger.besu.cli.options.CLIOptions; import org.hyperledger.besu.cli.util.CommandLineUtils; import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration; import org.hyperledger.besu.ethereum.worldstate.ImmutableDataStorageConfiguration; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/ApiConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/ApiConfigurationOptions.java index fbed68de0c..d6bc17026c 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/ApiConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/ApiConfigurationOptions.java @@ -28,6 +28,7 @@ import picocli.CommandLine; * Handles configuration options for the API in Besu, including gas price settings, RPC log range, * and trace filter range. */ +// TODO: implement CLIOption public class ApiConfigurationOptions { /** Default constructor. */ public ApiConfigurationOptions() {} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCConfiguration.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCConfiguration.java new file mode 100644 index 0000000000..c0eb9d1be0 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCConfiguration.java @@ -0,0 +1,36 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.cli.options.stable; + +import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty; + +import java.nio.file.Path; + +/** + * Command line options for configuring Engine RPC on the node. + * + * @param overrideEngineRpcEnabled enable the engine api, even in the absence of merge-specific + * configurations. + * @param engineRpcPort Port to provide consensus client APIS on + * @param engineJwtKeyFile Path to file containing shared secret key for JWT signature verification + * @param isEngineAuthDisabled Disable authentication for Engine APIs + * @param engineHostsAllowlist List of hosts to allowlist for Engine APIs + */ +public record EngineRPCConfiguration( + Boolean overrideEngineRpcEnabled, + Integer engineRpcPort, + Path engineJwtKeyFile, + Boolean isEngineAuthDisabled, + JsonRPCAllowlistHostsProperty engineHostsAllowlist) {} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCOptions.java new file mode 100644 index 0000000000..1aa5b3d326 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/EngineRPCOptions.java @@ -0,0 +1,81 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.cli.options.stable; + +import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration.DEFAULT_ENGINE_JSON_RPC_PORT; + +import org.hyperledger.besu.cli.DefaultCommandValues; +import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty; +import org.hyperledger.besu.cli.options.CLIOptions; +import org.hyperledger.besu.cli.util.CommandLineUtils; + +import java.nio.file.Path; +import java.util.List; + +import picocli.CommandLine; + +/** Command line options for configuring Engine RPC on the node. */ +public class EngineRPCOptions implements CLIOptions { + + /** Default constructor */ + public EngineRPCOptions() {} + + @CommandLine.Option( + names = {"--engine-rpc-enabled"}, + description = "enable the engine api, even in the absence of merge-specific configurations.") + private final Boolean overrideEngineRpcEnabled = false; + + @CommandLine.Option( + names = {"--engine-rpc-port", "--engine-rpc-http-port"}, + paramLabel = DefaultCommandValues.MANDATORY_PORT_FORMAT_HELP, + description = "Port to provide consensus client APIS on (default: ${DEFAULT-VALUE})", + arity = "1") + private final Integer engineRpcPort = DEFAULT_ENGINE_JSON_RPC_PORT; + + @CommandLine.Option( + names = {"--engine-jwt-secret"}, + paramLabel = DefaultCommandValues.MANDATORY_FILE_FORMAT_HELP, + description = "Path to file containing shared secret key for JWT signature verification") + private final Path engineJwtKeyFile = null; + + @CommandLine.Option( + names = {"--engine-jwt-disabled"}, + description = "Disable authentication for Engine APIs (default: ${DEFAULT-VALUE})") + private final Boolean isEngineAuthDisabled = false; + + @CommandLine.Option( + names = {"--engine-host-allowlist"}, + paramLabel = "[,...]... or * or all", + description = + "Comma separated list of hostnames to allow for ENGINE API access (applies to both HTTP and websockets), or * to accept any host (default: ${DEFAULT-VALUE})", + defaultValue = "localhost,127.0.0.1") + private final JsonRPCAllowlistHostsProperty engineHostsAllowlist = + new JsonRPCAllowlistHostsProperty(); + + @Override + public EngineRPCConfiguration toDomainObject() { + return new EngineRPCConfiguration( + overrideEngineRpcEnabled, + engineRpcPort, + engineJwtKeyFile, + isEngineAuthDisabled, + engineHostsAllowlist); + } + + @Override + public List getCLIOptions() { + return CommandLineUtils.getCLIOptions(this, new EngineRPCOptions()); + } +} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/GraphQlOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/GraphQlOptions.java index 77cc170305..6aac24a6fb 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/GraphQlOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/GraphQlOptions.java @@ -29,6 +29,7 @@ import org.slf4j.Logger; import picocli.CommandLine; /** Handles configuration options for the GraphQL HTTP service in Besu. */ +// TODO: implement CLIOptions public class GraphQlOptions { @CommandLine.Option( names = {"--graphql-http-enabled"}, diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/JsonRpcHttpOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/JsonRpcHttpOptions.java index 026b83b553..1c7e0f543e 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/JsonRpcHttpOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/JsonRpcHttpOptions.java @@ -52,6 +52,7 @@ import picocli.CommandLine; * Handles configuration options for the JSON-RPC HTTP service, including validation and creation of * a JSON-RPC configuration. */ +// TODO: implement CLIOption public class JsonRpcHttpOptions { @CommandLine.Option( names = {"--rpc-http-enabled"}, @@ -265,37 +266,50 @@ public class JsonRpcHttpOptions { /** * Creates a JsonRpcConfiguration based on the provided options. * - * @param hostsAllowlist List of hosts allowed - * @param defaultHostAddress Default host address - * @param timoutSec timeout in seconds - * @return A JsonRpcConfiguration instance + * @return configuration populated from options or defaults */ - public JsonRpcConfiguration jsonRpcConfiguration( - final List hostsAllowlist, final String defaultHostAddress, final Long timoutSec) { + public JsonRpcConfiguration jsonRpcConfiguration() { final JsonRpcConfiguration jsonRpcConfiguration = JsonRpcConfiguration.createDefault(); jsonRpcConfiguration.setEnabled(isRpcHttpEnabled); - jsonRpcConfiguration.setHost( - Strings.isNullOrEmpty(rpcHttpHost) ? defaultHostAddress : rpcHttpHost); jsonRpcConfiguration.setPort(rpcHttpPort); jsonRpcConfiguration.setMaxActiveConnections(rpcHttpMaxConnections); jsonRpcConfiguration.setCorsAllowedDomains(rpcHttpCorsAllowedOrigins); jsonRpcConfiguration.setRpcApis(rpcHttpApis.stream().distinct().collect(Collectors.toList())); jsonRpcConfiguration.setNoAuthRpcApis( rpcHttpApiMethodsNoAuth.stream().distinct().collect(Collectors.toList())); - jsonRpcConfiguration.setHostsAllowlist(hostsAllowlist); jsonRpcConfiguration.setAuthenticationEnabled(isRpcHttpAuthenticationEnabled); jsonRpcConfiguration.setAuthenticationCredentialsFile(rpcHttpAuthenticationCredentialsFile); jsonRpcConfiguration.setAuthenticationPublicKeyFile(rpcHttpAuthenticationPublicKeyFile); jsonRpcConfiguration.setAuthenticationAlgorithm(rpcHttpAuthenticationAlgorithm); jsonRpcConfiguration.setTlsConfiguration(rpcHttpTlsConfiguration()); - jsonRpcConfiguration.setHttpTimeoutSec(timoutSec); jsonRpcConfiguration.setMaxBatchSize(rpcHttpMaxBatchSize); jsonRpcConfiguration.setMaxRequestContentLength(rpcHttpMaxRequestContentLength); jsonRpcConfiguration.setPrettyJsonEnabled(prettyJsonEnabled); return jsonRpcConfiguration; } + /** + * Creates a JsonRpcConfiguration based on the provided options. + * + * @param hostsAllowlist List of hosts allowed + * @param defaultHostAddress Default host address + * @param timoutSec timeout in seconds + * @return A JsonRpcConfiguration instance + */ + public JsonRpcConfiguration jsonRpcConfiguration( + final List hostsAllowlist, final String defaultHostAddress, final Long timoutSec) { + + final JsonRpcConfiguration jsonRpcConfiguration = this.jsonRpcConfiguration(); + + jsonRpcConfiguration.setHost( + Strings.isNullOrEmpty(rpcHttpHost) ? defaultHostAddress : rpcHttpHost); + jsonRpcConfiguration.setHostsAllowlist(hostsAllowlist); + ; + jsonRpcConfiguration.setHttpTimeoutSec(timoutSec); + return jsonRpcConfiguration; + } + /** * Checks dependencies between options. * diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MetricsOptionGroup.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MetricsOptionGroup.java index add2bf1655..1c17e1cf9d 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MetricsOptionGroup.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MetricsOptionGroup.java @@ -30,6 +30,7 @@ import java.util.Set; import picocli.CommandLine; /** Command line options for configuring metrics. */ +// TODO: implement CLIOption and rename to drop the Group public class MetricsOptionGroup { @CommandLine.Option( names = {"--metrics-enabled"}, diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PDiscoveryOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PDiscoveryOptions.java new file mode 100644 index 0000000000..21c24b3e27 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PDiscoveryOptions.java @@ -0,0 +1,238 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.cli.options.stable; + +import org.hyperledger.besu.cli.DefaultCommandValues; +import org.hyperledger.besu.cli.converter.PercentageConverter; +import org.hyperledger.besu.cli.converter.SubnetInfoConverter; +import org.hyperledger.besu.cli.options.CLIOptions; +import org.hyperledger.besu.cli.util.CommandLineUtils; +import org.hyperledger.besu.ethereum.p2p.discovery.P2PDiscoveryConfiguration; +import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; +import org.hyperledger.besu.util.NetworkUtility; +import org.hyperledger.besu.util.number.Fraction; +import org.hyperledger.besu.util.number.Percentage; + +import java.net.InetAddress; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +import org.apache.commons.net.util.SubnetUtils; +import org.apache.tuweni.bytes.Bytes; +import picocli.CommandLine; + +/** Command line options for configuring P2P discovery on the node. */ +public class P2PDiscoveryOptions implements CLIOptions { + + /** Default constructor */ + public P2PDiscoveryOptions() {} + + // Public IP stored to prevent having to research it each time we need it. + private InetAddress autoDiscoveredDefaultIP = null; + + /** Completely disables P2P within Besu. */ + @CommandLine.Option( + names = {"--p2p-enabled"}, + description = "Enable P2P functionality (default: ${DEFAULT-VALUE})", + arity = "1") + public final Boolean p2pEnabled = true; + + /** + * Boolean option to indicate if peers should NOT be discovered, default to false indicates that + * the peers should be discovered by default. + */ + // + // This negative option is required because of the nature of the option that is + // true when + // added on the command line. You can't do --option=false, so false is set as + // default + // and you have not to set the option at all if you want it false. + // This seems to be the only way it works with Picocli. + // Also many other software use the same negative option scheme for false + // defaults + // meaning that it's probably the right way to handle disabling options. + @CommandLine.Option( + names = {"--discovery-enabled"}, + description = "Enable P2P discovery (default: ${DEFAULT-VALUE})", + arity = "1") + public final Boolean peerDiscoveryEnabled = true; + + /** + * A list of bootstrap nodes can be passed and a hardcoded list will be used otherwise by the + * Runner. + */ + // NOTE: we have no control over default value here. + @CommandLine.Option( + names = {"--bootnodes"}, + paramLabel = "", + description = + "Comma separated enode URLs for P2P discovery bootstrap. " + + "Default is a predefined list.", + split = ",", + arity = "0..*") + public final List bootNodes = null; + + /** The IP the node advertises to peers for P2P communication. */ + @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. + @CommandLine.Option( + names = {"--p2p-host"}, + paramLabel = DefaultCommandValues.MANDATORY_HOST_FORMAT_HELP, + description = "IP address this node advertises to its peers (default: ${DEFAULT-VALUE})", + arity = "1") + public String p2pHost = autoDiscoverDefaultIP().getHostAddress(); + + /** The network interface address on which this node listens for P2P communication. */ + @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. + @CommandLine.Option( + names = {"--p2p-interface"}, + paramLabel = DefaultCommandValues.MANDATORY_HOST_FORMAT_HELP, + description = + "The network interface address on which this node listens for P2P communication (default: ${DEFAULT-VALUE})", + arity = "1") + public String p2pInterface = NetworkUtility.INADDR_ANY; + + /** The port on which this node listens for P2P communication. */ + @CommandLine.Option( + names = {"--p2p-port"}, + paramLabel = DefaultCommandValues.MANDATORY_PORT_FORMAT_HELP, + description = "Port on which to listen for P2P communication (default: ${DEFAULT-VALUE})", + arity = "1") + public final Integer p2pPort = EnodeURLImpl.DEFAULT_LISTENING_PORT; + + /** The maximum number of peers this node can connect to. */ + @CommandLine.Option( + names = {"--max-peers", "--p2p-peer-upper-bound"}, + paramLabel = DefaultCommandValues.MANDATORY_INTEGER_FORMAT_HELP, + description = "Maximum P2P connections that can be established (default: ${DEFAULT-VALUE})") + public final Integer maxPeers = DefaultCommandValues.DEFAULT_MAX_PEERS; + + /** Boolean option to limit the number of P2P connections initiated remotely. */ + @CommandLine.Option( + names = {"--remote-connections-limit-enabled"}, + description = + "Whether to limit the number of P2P connections initiated remotely. (default: ${DEFAULT-VALUE})") + public final Boolean isLimitRemoteWireConnectionsEnabled = true; + + /** The maximum percentage of P2P connections that can be initiated remotely. */ + @CommandLine.Option( + names = {"--remote-connections-max-percentage"}, + paramLabel = DefaultCommandValues.MANDATORY_DOUBLE_FORMAT_HELP, + description = + "The maximum percentage of P2P connections that can be initiated remotely. Must be between 0 and 100 inclusive. (default: ${DEFAULT-VALUE})", + arity = "1", + converter = PercentageConverter.class) + public final Percentage maxRemoteConnectionsPercentage = + Fraction.fromFloat(DefaultCommandValues.DEFAULT_FRACTION_REMOTE_WIRE_CONNECTIONS_ALLOWED) + .toPercentage(); + + /** The URL to use for DNS discovery. */ + @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. + @CommandLine.Option( + names = {"--discovery-dns-url"}, + description = "Specifies the URL to use for DNS discovery") + public String discoveryDnsUrl = null; + + /** Boolean option to allow for incoming connections to be prioritized randomly. */ + @CommandLine.Option( + names = {"--random-peer-priority-enabled"}, + description = + "Allow for incoming connections to be prioritized randomly. This will prevent (typically small, stable) networks from forming impenetrable peer cliques. (default: ${DEFAULT-VALUE})") + public final Boolean randomPeerPriority = Boolean.FALSE; + + /** A list of node IDs to ban from the P2P network. */ + @CommandLine.Option( + names = {"--banned-node-ids", "--banned-node-id"}, + paramLabel = DefaultCommandValues.MANDATORY_NODE_ID_FORMAT_HELP, + description = "A list of node IDs to ban from the P2P network.", + split = ",", + arity = "1..*") + void setBannedNodeIds(final List values) { + try { + bannedNodeIds = + values.stream() + .filter(value -> !value.isEmpty()) + .map(EnodeURLImpl::parseNodeId) + .collect(Collectors.toList()); + } catch (final IllegalArgumentException e) { + throw new CommandLine.ParameterException( + new CommandLine(this), "Invalid ids supplied to '--banned-node-ids'. " + e.getMessage()); + } + } + + // Boolean option to set that in a PoA network the bootnodes should always be queried during + // peer table refresh. If this flag is disabled bootnodes are only sent FINDN requests on first + // startup, meaning that an offline bootnode or network outage at the client can prevent it + // discovering any peers without a restart. + @CommandLine.Option( + names = {"--poa-discovery-retry-bootnodes"}, + description = + "Always use of bootnodes for discovery in PoA networks. Disabling this reverts " + + " to the same behaviour as non-PoA networks, where neighbours are only discovered from bootnodes on first startup." + + "(default: ${DEFAULT-VALUE})", + arity = "1") + private final Boolean poaDiscoveryRetryBootnodes = true; + + private Collection bannedNodeIds = new ArrayList<>(); + + /** + * Auto-discovers the default IP of the client. + * + * @return machine loopback address + */ + // Loopback IP is used by default as this is how smokeTests require it to be + // and it's probably a good security behaviour to default only on the localhost. + public InetAddress autoDiscoverDefaultIP() { + autoDiscoveredDefaultIP = + Optional.ofNullable(autoDiscoveredDefaultIP).orElseGet(InetAddress::getLoopbackAddress); + + return autoDiscoveredDefaultIP; + } + + @CommandLine.Option( + names = {"--net-restrict"}, + arity = "1..*", + split = ",", + converter = SubnetInfoConverter.class, + description = + "Comma-separated list of allowed IP subnets (e.g., '192.168.1.0/24,10.0.0.0/8').") + private List allowedSubnets; + + @Override + public P2PDiscoveryConfiguration toDomainObject() { + return new P2PDiscoveryConfiguration( + p2pEnabled, + peerDiscoveryEnabled, + p2pHost, + p2pInterface, + p2pPort, + maxPeers, + isLimitRemoteWireConnectionsEnabled, + maxRemoteConnectionsPercentage, + randomPeerPriority, + bannedNodeIds, + allowedSubnets, + poaDiscoveryRetryBootnodes, + bootNodes, + discoveryDnsUrl); + } + + @Override + public List getCLIOptions() { + return CommandLineUtils.getCLIOptions(this, new P2PDiscoveryOptions()); + } +} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java index f1eafaab16..4dc693d36e 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java @@ -30,6 +30,7 @@ import org.slf4j.Logger; import picocli.CommandLine; /** Handles configuration options for permissions in Besu. */ +// TODO: implement CLIOption public class PermissionsOptions { @CommandLine.Option( names = {"--permissions-nodes-config-file-enabled"}, diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MetricsCLIOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MetricsCLIOptions.java index 95164c31cc..4149d9869e 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MetricsCLIOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MetricsCLIOptions.java @@ -23,6 +23,7 @@ import java.util.List; import picocli.CommandLine; /** The Metrics cli options. */ +// TODO: combine into MetricsOptionGroup, use Unstable inner class pattern (see MiningOptions) public class MetricsCLIOptions implements CLIOptions { private static final String TIMERS_ENABLED_FLAG = "--Xmetrics-timers-enabled"; private static final String IDLE_TIMEOUT_FLAG = "--Xmetrics-idle-timeout"; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PTLSConfigOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/P2PTLSConfigOptions.java similarity index 99% rename from besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PTLSConfigOptions.java rename to besu/src/main/java/org/hyperledger/besu/cli/options/unstable/P2PTLSConfigOptions.java index c3f8c56219..5939eb3d39 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/P2PTLSConfigOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/P2PTLSConfigOptions.java @@ -12,7 +12,7 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.cli.options.stable; +package org.hyperledger.besu.cli.options.unstable; import static java.util.Arrays.asList; import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_KEYSTORE_TYPE; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelper.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelper.java index 6bcfe4d353..1ac309c430 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelper.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogHelper.java @@ -15,11 +15,11 @@ package org.hyperledger.besu.cli.subcommands.storage; import static com.google.common.base.Preconditions.checkArgument; -import static org.hyperledger.besu.cli.options.stable.DataStorageOptions.BONSAI_STORAGE_FORMAT_MAX_LAYERS_TO_LOAD; +import static org.hyperledger.besu.cli.options.DataStorageOptions.BONSAI_STORAGE_FORMAT_MAX_LAYERS_TO_LOAD; import static org.hyperledger.besu.controller.BesuController.DATABASE_PATH; import static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration.DEFAULT_BONSAI_TRIE_LOG_PRUNING_WINDOW_SIZE; -import org.hyperledger.besu.cli.options.stable.DataStorageOptions; +import org.hyperledger.besu.cli.options.DataStorageOptions; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; diff --git a/besu/src/main/java/org/hyperledger/besu/components/BesuCommandModule.java b/besu/src/main/java/org/hyperledger/besu/components/BesuCommandModule.java index 59e2d60bf3..e757fd62a2 100644 --- a/besu/src/main/java/org/hyperledger/besu/components/BesuCommandModule.java +++ b/besu/src/main/java/org/hyperledger/besu/components/BesuCommandModule.java @@ -20,7 +20,10 @@ import org.hyperledger.besu.chainexport.RlpBlockExporter; import org.hyperledger.besu.chainimport.JsonBlockImporter; import org.hyperledger.besu.chainimport.RlpBlockImporter; import org.hyperledger.besu.cli.BesuCommand; +import org.hyperledger.besu.cli.options.stable.P2PDiscoveryOptions; +import org.hyperledger.besu.cli.options.unstable.RPCOptions; import org.hyperledger.besu.controller.BesuController; +import org.hyperledger.besu.ethereum.p2p.discovery.P2PDiscoveryConfiguration; import org.hyperledger.besu.metrics.prometheus.MetricsConfiguration; import org.hyperledger.besu.services.BesuPluginContextImpl; @@ -53,7 +56,6 @@ public class BesuCommandModule { new BesuPluginContextImpl(), System.getenv(), commandLogger); - besuCommand.toCommandLine(); return besuCommand; } @@ -63,6 +65,18 @@ public class BesuCommandModule { return provideFrom.metricsConfiguration(); } + @Provides + @Singleton + RPCOptions provideRPCOptions() { + return RPCOptions.create(); + } + + @Provides + @Singleton + P2PDiscoveryConfiguration provideP2PDiscoveryConfiguration() { + return new P2PDiscoveryOptions().toDomainObject(); + } + @Provides @Named("besuCommandLogger") @Singleton diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 65e51c9213..7657dcd26a 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -34,9 +34,9 @@ import org.hyperledger.besu.chainexport.RlpBlockExporter; import org.hyperledger.besu.chainimport.JsonBlockImporter; import org.hyperledger.besu.chainimport.RlpBlockImporter; import org.hyperledger.besu.cli.config.EthNetworkConfig; +import org.hyperledger.besu.cli.options.DataStorageOptions; import org.hyperledger.besu.cli.options.MiningOptions; import org.hyperledger.besu.cli.options.TransactionPoolOptions; -import org.hyperledger.besu.cli.options.stable.DataStorageOptions; import org.hyperledger.besu.cli.options.stable.EthstatsOptions; import org.hyperledger.besu.cli.options.unstable.EthProtocolOptions; import org.hyperledger.besu.cli.options.unstable.MetricsCLIOptions; diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/DataStorageOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/stable/DataStorageOptionsTest.java index 29fa3d6607..6be6429adb 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/DataStorageOptionsTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/options/stable/DataStorageOptionsTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration.MINIMUM_BONSAI_TRIE_LOG_RETENTION_LIMIT; import org.hyperledger.besu.cli.options.AbstractCLIOptionsTest; +import org.hyperledger.besu.cli.options.DataStorageOptions; import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration; import org.hyperledger.besu.ethereum.worldstate.ImmutableDataStorageConfiguration; import org.hyperledger.besu.plugin.services.storage.DataStorageFormat; diff --git a/config/src/main/java/org/hyperledger/besu/config/MergeConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/MergeConfigOptions.java index 53faf69052..6ba865e113 100644 --- a/config/src/main/java/org/hyperledger/besu/config/MergeConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/MergeConfigOptions.java @@ -17,6 +17,8 @@ package org.hyperledger.besu.config; import java.util.concurrent.atomic.AtomicBoolean; /** The Merge config options. */ +// TODO: naming this with Options as the suffix is misleading, it should be MergeConfig - doesn't +// use picocli public class MergeConfigOptions { private static final AtomicBoolean mergeEnabled = new AtomicBoolean(false); diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/P2PDiscoveryConfiguration.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/P2PDiscoveryConfiguration.java new file mode 100644 index 0000000000..721a3100a0 --- /dev/null +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/P2PDiscoveryConfiguration.java @@ -0,0 +1,39 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.p2p.discovery; + +import org.hyperledger.besu.util.number.Percentage; + +import java.util.Collection; +import java.util.List; + +import org.apache.commons.net.util.SubnetUtils; +import org.apache.tuweni.bytes.Bytes; + +public record P2PDiscoveryConfiguration( + Boolean p2pEnabled, + Boolean peerDiscoveryEnabled, + String p2pHost, + String p2pInterface, + Integer p2pPort, + Integer maxPeers, + Boolean isLimitRemoteWireConnectionsEnabled, + Percentage maxRemoteConnectionsPercentage, + Boolean randomPeerPriority, + Collection bannedNodeIds, + List allowedSubnets, + Boolean poaDiscoveryRetryBootnodes, + List bootNodes, + String discoveryDnsUrl) {} From 8cf20ed7f5ece5f7b11127b1fdf917ee93d15179 Mon Sep 17 00:00:00 2001 From: Chaminda Divitotawela Date: Thu, 3 Oct 2024 12:07:19 +1000 Subject: [PATCH 12/14] Changes to release workflow (#7711) * Changes to release workflow - Workflow environment variable changed to RELEASE_VERSION instead of RELEASE_NAME to make it more meaningful - Workflows draft-release and docker-promote previously trigger workflow container-verify. This can result in trigger is successful but actual verification workflow fails. This will not be reflected in the draft-release and docker-promote workflows. Container verify code is embedded in the draft-release and docker-promote workflows to avoid this confusion Signed-off-by: Chaminda Divitotawela * PR comment improvements Signed-off-by: Chaminda Divitotawela * Match the only tags starting with word latest for latest check Signed-off-by: Chaminda Divitotawela --------- Signed-off-by: Chaminda Divitotawela Co-authored-by: Simon Dudley --- .github/workflows/BesuContainerVerify.sh | 2 +- .github/workflows/docker-promote.yml | 64 ++++++--- .github/workflows/draft-release.yml | 160 +++++++++++++---------- 3 files changed, 139 insertions(+), 87 deletions(-) diff --git a/.github/workflows/BesuContainerVerify.sh b/.github/workflows/BesuContainerVerify.sh index 81537f3264..1560de5c44 100644 --- a/.github/workflows/BesuContainerVerify.sh +++ b/.github/workflows/BesuContainerVerify.sh @@ -57,7 +57,7 @@ else fi # For the latest tag check the version match -if [[ ${TAG} == "latest" && ${CHECK_LATEST} == "true" ]] +if [[ ${TAG} =~ ^latest && ${CHECK_LATEST} == "true" ]] then _VERSION_IN_LOG=$(docker logs ${CONTAINER_NAME} | grep "#" | grep "Besu version" | cut -d " " -f 4 | sed 's/\s//g') echo "Extracted version from logs [$_VERSION_IN_LOG]" diff --git a/.github/workflows/docker-promote.yml b/.github/workflows/docker-promote.yml index 858ea7d20e..18a3cc3170 100644 --- a/.github/workflows/docker-promote.yml +++ b/.github/workflows/docker-promote.yml @@ -14,25 +14,25 @@ jobs: validate: runs-on: ubuntu-22.04 env: - RELEASE_NAME: "${{ github.event.release.name }}" + RELEASE_VERSION: "${{ github.event.release.name }}" steps: - name: Pre-process Release Name - id: pre_process_release_name + id: pre_process_release_version run: | # strip all whitespace - RELEASE_NAME="${RELEASE_NAME//[[:space:]]/}" - if [[ ! "$RELEASE_NAME" =~ ^[0-9]+\.[0-9]+(\.[0-9]+)?(-.*)?$ ]]; then + RELEASE_VERSION="${RELEASE_VERSION//[[:space:]]/}" + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+(\.[0-9]+)?(-.*)?$ ]]; then echo "Release name does not conform to a valid besu release format YY.M.v[-suffix], e.g. 24.8.0-RC1." exit 1 fi - echo "release_name=$RELEASE_NAME" >> $GITHUB_OUTPUT # Set as output using the new syntax + echo "release_version=$RELEASE_VERSION" >> $GITHUB_OUTPUT # Set as output using the new syntax outputs: - release_name: ${{ steps.pre_process_release_name.outputs.release_name }} + release_version: ${{ steps.pre_process_release_version.outputs.release_version }} docker-promote: needs: [validate] env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} runs-on: ubuntu-22.04 steps: - name: Checkout @@ -58,24 +58,52 @@ jobs: cache-disabled: true - name: Docker upload - run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_NAME }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" dockerUploadRelease + run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_VERSION }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" dockerUploadRelease - name: Docker manifest - run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_NAME }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" manifestDockerRelease + run: ./gradlew "-Prelease.releaseVersion=${{ env.RELEASE_VERSION }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" manifestDockerRelease docker-verify: - needs: [validate, docker-promote] + needs: [validate,docker-promote] env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job - runs-on: ubuntu-22.04 - permissions: - contents: read - actions: write + CONTAINER_NAME: besu-check + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} + runs-on: ${{ matrix.combination.runner }} + timeout-minutes: 4 + strategy: + matrix: + combination: + - tag: latest-amd64 + platform: 'linux/amd64' + runner: ubuntu-22.04 + - tag: latest + platform: '' + runner: ubuntu-22.04 + - tag: latest-arm64 + platform: '' + runner: besu-arm64 + - tag: latest + platform: '' + runner: besu-arm64 + steps: - name: Checkout uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + sparse-checkout: '.github/workflows/BesuContainerVerify.sh' - - name: Trigger container verify - run: echo '{"version":"${{ env.RELEASE_NAME }}","verify-latest-version":"true"}' | gh workflow run container-verify.yml --json + - name: Start container + run: | + PLATFORM_OPT="" + [[ x${{ matrix.combination.platform }} != 'x' ]] && PLATFORM_OPT="--platform ${{ matrix.combination.platform }}" + docker run -d $PLATFORM_OPT --name ${{ env.CONTAINER_NAME }} ${{ secrets.DOCKER_ORG }}/besu:${{ matrix.combination.tag }} + + - name: Verify besu container + run: bash .github/workflows/BesuContainerVerify.sh env: - GH_TOKEN: ${{ github.token }} + TAG: ${{ matrix.combination.tag }} + VERSION: ${{ env.RELEASE_VERSION }} + CHECK_LATEST: true + + - name: Stop container + run: docker stop ${{ env.CONTAINER_NAME }} diff --git a/.github/workflows/draft-release.yml b/.github/workflows/draft-release.yml index f6e01e1890..8dc61bec46 100644 --- a/.github/workflows/draft-release.yml +++ b/.github/workflows/draft-release.yml @@ -16,7 +16,7 @@ jobs: validate: runs-on: ubuntu-22.04 env: - RELEASE_NAME: "${{ inputs.tag }}" + RELEASE_VERSION: "${{ inputs.tag }}" steps: - name: Check default branch run: | @@ -24,36 +24,36 @@ jobs: echo "Default Branch: ${{ github.event.repository.default_branch }}" if [[ ${{ github.ref_name }} != ${{ github.event.repository.default_branch }} ]] then - echo "This workflow can only be run on default branch" + echo "This workflow can only be run on default branch. This is not an issue for hot fixes as code is checked out from the tag" exit 1 fi - + - name: Pre-process Release Name - id: pre_process_release_name + id: validate_release_version run: | # strip all whitespace - RELEASE_NAME="${RELEASE_NAME//[[:space:]]/}" - if [[ ! "$RELEASE_NAME" =~ ^[0-9]+\.[0-9]+(\.[0-9]+)?(-.*)?$ ]]; then + RELEASE_VERSION="${RELEASE_VERSION//[[:space:]]/}" + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+(\.[0-9]+)?(-.*)?$ ]]; then echo "Release name does not conform to a valid besu release format YY.M.v[-suffix], e.g. 24.8.0-RC1." exit 1 fi - echo "release_name=$RELEASE_NAME" >> $GITHUB_OUTPUT # Set as output using the new syntax - + echo "release_version=$RELEASE_VERSION" >> $GITHUB_OUTPUT # Set as output using the new syntax + # Perform a tag checkout to ensure tag is available - name: Verify tag Exist uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - with: - ref: ${{ steps.pre_process_release_name.outputs.release_name }} + with: + ref: ${{ steps.validate_release_version.outputs.release_version }} fetch-depth: 1 outputs: - release_name: ${{ steps.pre_process_release_name.outputs.release_name }} + release_version: ${{ steps.validate_release_version.outputs.release_version }} build: runs-on: ubuntu-22.04 needs: validate env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} # Use the output from the pre_process_release job outputs: tarSha: ${{steps.hashes.outputs.tarSha}} zipSha: ${{steps.hashes.outputs.zipSha}} @@ -61,22 +61,22 @@ jobs: - name: Checkout tag uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 with: - ref: ${{ env.RELEASE_NAME }} - + ref: ${{ env.RELEASE_VERSION }} + - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 with: distribution: temurin java-version: 21 - + - name: Setup gradle uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 with: cache-disabled: true - + - name: Assemble release run: - ./gradlew -Prelease.releaseVersion=${{env.RELEASE_NAME}} -Pversion=${{env.RELEASE_NAME}} assemble + ./gradlew -Prelease.releaseVersion=${{env.RELEASE_VERSION}} -Pversion=${{env.RELEASE_VERSION}} assemble - name: Hashes id: hashes @@ -86,35 +86,35 @@ jobs: echo "tarSha=$(shasum -a 256 besu*.tar.gz)" echo "zipSha=$(shasum -a 256 besu*.zip)" >> $GITHUB_OUTPUT echo "tarSha=$(shasum -a 256 besu*.tar.gz)" >> $GITHUB_OUTPUT - shasum -a 256 besu-${{env.RELEASE_NAME}}.tar.gz > besu-${{env.RELEASE_NAME}}.tar.gz.sha256 - shasum -a 256 besu-${{env.RELEASE_NAME}}.zip > besu-${{env.RELEASE_NAME}}.zip.sha256 - + shasum -a 256 besu-${{env.RELEASE_VERSION}}.tar.gz > besu-${{env.RELEASE_VERSION}}.tar.gz.sha256 + shasum -a 256 besu-${{env.RELEASE_VERSION}}.zip > besu-${{env.RELEASE_VERSION}}.zip.sha256 + - name: Upload tarball uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 with: - path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.tar.gz' - name: besu-${{ env.RELEASE_NAME }}.tar.gz + path: 'build/distributions/besu-${{ env.RELEASE_VERSION }}.tar.gz' + name: besu-${{ env.RELEASE_VERSION }}.tar.gz compression-level: 0 - + - name: upload zipfile uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 with: - path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.zip' - name: besu-${{ env.RELEASE_NAME }}.zip + path: 'build/distributions/besu-${{ env.RELEASE_VERSION }}.zip' + name: besu-${{ env.RELEASE_VERSION }}.zip compression-level: 0 - name: upload checksum zip uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 with: - path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.zip.sha256' - name: besu-${{ env.RELEASE_NAME }}.zip.sha256 + path: 'build/distributions/besu-${{ env.RELEASE_VERSION }}.zip.sha256' + name: besu-${{ env.RELEASE_VERSION }}.zip.sha256 compression-level: 0 - name: upload checksum tar.gz uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 with: - path: 'build/distributions/besu-${{ env.RELEASE_NAME }}.tar.gz.sha256' - name: besu-${{ env.RELEASE_NAME }}.tar.gz.sha256 + path: 'build/distributions/besu-${{ env.RELEASE_VERSION }}.tar.gz.sha256' + name: besu-${{ env.RELEASE_VERSION }}.tar.gz.sha256 compression-level: 0 test-windows: @@ -127,7 +127,7 @@ jobs: with: distribution: temurin java-version: 21 - + - name: Download zip uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe with: @@ -153,7 +153,7 @@ jobs: with: distribution: temurin java-version: 21 - + - name: Download tar.gz uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe with: @@ -172,20 +172,20 @@ jobs: runs-on: ubuntu-22.04 needs: [test-linux, test-windows] env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} # Use the output from the pre_process_release job steps: - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 with: - ref: ${{ env.RELEASE_NAME }} + ref: ${{ env.RELEASE_VERSION }} - name: hadoLint run: docker run --rm -i hadolint/hadolint < docker/Dockerfile - + docker-publish: needs: [validate, docker-lint] env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} # Use the output from the pre_process_release job strategy: fail-fast: false matrix: @@ -198,7 +198,7 @@ jobs: id: prep run: | platform=${{ matrix.platform }} - if [ "$platform" = 'ubuntu-22.04' ]; then + if [ "$platform" = 'ubuntu-22.04' ]; then echo "PLATFORM_PAIR=linux-amd64" >> $GITHUB_OUTPUT echo "ARCH=amd64" >> $GITHUB_OUTPUT else @@ -209,7 +209,7 @@ jobs: - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 with: - ref: ${{ env.RELEASE_NAME }} + ref: ${{ env.RELEASE_VERSION }} - name: short sha id: shortSha @@ -225,7 +225,7 @@ jobs: uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1 with: cache-disabled: true - + - name: install goss run: | mkdir -p docker/reports @@ -244,23 +244,23 @@ jobs: architecture: ${{ steps.prep.outputs.ARCH }} with: cache-disabled: true - arguments: testDocker -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_NAME}} -Prelease.releaseVersion=${{ env.RELEASE_NAME }} + arguments: testDocker -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_VERSION}} -Prelease.releaseVersion=${{ env.RELEASE_VERSION }} - name: publish env: architecture: ${{ steps.prep.outputs.ARCH }} - run: ./gradlew --no-daemon dockerUpload -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_NAME}} -Prelease.releaseVersion=${{ env.RELEASE_NAME }} + run: ./gradlew --no-daemon dockerUpload -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_VERSION}} -Prelease.releaseVersion=${{ env.RELEASE_VERSION }} docker-manifest: needs: [validate, docker-publish] runs-on: ubuntu-22.04 env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} steps: - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 with: - ref: ${{ env.RELEASE_NAME }} + ref: ${{ env.RELEASE_VERSION }} - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 @@ -281,24 +281,48 @@ jobs: password: ${{ secrets.DOCKER_PASSWORD_RW }} - name: multi-arch docker - run: ./gradlew manifestDocker -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_NAME}} -Prelease.releaseVersion=${{ env.RELEASE_NAME }} + run: ./gradlew manifestDocker -PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }} -Pversion=${{env.RELEASE_VERSION}} -Prelease.releaseVersion=${{ env.RELEASE_VERSION }} docker-verify: needs: [validate,docker-manifest] env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} # Use the output from the pre_process_release job - runs-on: ubuntu-22.04 - permissions: - contents: read - actions: write + CONTAINER_NAME: besu-check + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} + runs-on: ${{ matrix.combination.runner }} + timeout-minutes: 4 + strategy: + matrix: + combination: + - tag: ${{ needs.validate.outputs.release_version }} + platform: '' + runner: ubuntu-22.04 + - tag: ${{ needs.validate.outputs.release_version }}-amd64 + platform: 'linux/amd64' + runner: ubuntu-22.04 + - tag: ${{ needs.validate.outputs.release_version }}-arm64 + platform: '' + runner: besu-arm64 steps: - name: Checkout uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + sparse-checkout: '.github/workflows/BesuContainerVerify.sh' + + - name: Start container + run: | + PLATFORM_OPT="" + [[ x${{ matrix.combination.platform }} != 'x' ]] && PLATFORM_OPT="--platform ${{ matrix.combination.platform }}" + docker run -d $PLATFORM_OPT --name ${{ env.CONTAINER_NAME }} ${{ secrets.DOCKER_ORG }}/besu:${{ matrix.combination.tag }} - - name: Trigger container verify - run: echo '{"version":"${{ env.RELEASE_NAME }}","verify-latest-version":"false"}' | gh workflow run container-verify.yml --json + - name: Verify besu container + run: bash .github/workflows/BesuContainerVerify.sh env: - GH_TOKEN: ${{ github.token }} + TAG: ${{ matrix.combination.tag }} + VERSION: ${{ env.RELEASE_VERSION }} + CHECK_LATEST: false + + - name: Stop container + run: docker stop ${{ env.CONTAINER_NAME }} release-draft: runs-on: ubuntu-22.04 @@ -306,42 +330,42 @@ jobs: permissions: contents: write env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} steps: - name: Checkout Repo uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 with: - ref: ${{ env.RELEASE_NAME }} + ref: ${{ env.RELEASE_VERSION }} - name: Download Besu artifacts uses: actions/download-artifact@eaceaf801fd36c7dee90939fad912460b18a1ffe with: - pattern: besu-${{env.RELEASE_NAME}}* + pattern: besu-${{env.RELEASE_VERSION}}* merge-multiple: true - name: Draft release notes run: | - echo "## ${{env.RELEASE_NAME}}" > draft-release-notes.md + echo "## ${{env.RELEASE_VERSION}}" > draft-release-notes.md echo "## Upcoming Breaking Changes" >> draft-release-notes.md echo "## Breaking Changes" >> draft-release-notes.md echo "## Additions and Improvements" >> draft-release-notes.md echo "## Bug fixes" >> draft-release-notes.md - echo "`$(cat besu-${{env.RELEASE_NAME}}.zip.sha256)`" >> draft-release-notes.md - echo "`$(cat besu-${{env.RELEASE_NAME}}.tar.gz.sha256)`" >> draft-release-notes.md - cat besu-${{env.RELEASE_NAME}}.zip.sha256 >> draft-release-notes.md - cat besu-${{env.RELEASE_NAME}}.tar.gz.sha256 >> draft-release-notes.md + echo "`$(cat besu-${{env.RELEASE_VERSION}}.zip.sha256)`" >> draft-release-notes.md + echo "`$(cat besu-${{env.RELEASE_VERSION}}.tar.gz.sha256)`" >> draft-release-notes.md + cat besu-${{env.RELEASE_VERSION}}.zip.sha256 >> draft-release-notes.md + cat besu-${{env.RELEASE_VERSION}}.tar.gz.sha256 >> draft-release-notes.md - name: Draft release run: | gh release create \ --draft \ - --title=${{env.RELEASE_NAME}} \ + --title=${{env.RELEASE_VERSION}} \ --notes-file draft-release-notes.md \ - --verify-tag ${{env.RELEASE_NAME}} \ - besu-${{env.RELEASE_NAME}}.tar.gz \ - besu-${{env.RELEASE_NAME}}.zip \ - besu-${{env.RELEASE_NAME}}.zip.sha256 \ - besu-${{env.RELEASE_NAME}}.tar.gz.sha256 + --verify-tag ${{env.RELEASE_VERSION}} \ + besu-${{env.RELEASE_VERSION}}.tar.gz \ + besu-${{env.RELEASE_VERSION}}.zip \ + besu-${{env.RELEASE_VERSION}}.zip.sha256 \ + besu-${{env.RELEASE_VERSION}}.tar.gz.sha256 env: GH_TOKEN: ${{ github.token }} @@ -349,12 +373,12 @@ jobs: runs-on: ubuntu-22.04 needs: [validate, test-linux, test-windows] env: - RELEASE_NAME: ${{ needs.validate.outputs.release_name }} + RELEASE_VERSION: ${{ needs.validate.outputs.release_version }} steps: - name: checkout uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 with: - ref: ${{ env.RELEASE_NAME }} + ref: ${{ env.RELEASE_VERSION }} - name: Set up Java uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 @@ -371,4 +395,4 @@ jobs: env: ARTIFACTORY_USER: ${{ secrets.BESU_ARTIFACTORY_USER }} ARTIFACTORY_KEY: ${{ secrets.BESU_ARTIFACTORY_TOKEN }} - run: ./gradlew -Prelease.releaseVersion=${{ env.RELEASE_NAME }} -Pversion=${{env.RELEASE_NAME}} artifactoryPublish + run: ./gradlew -Prelease.releaseVersion=${{ env.RELEASE_VERSION }} -Pversion=${{env.RELEASE_VERSION}} artifactoryPublish From 63b9ec9daa80b17972596e2f39b6f6eff095889d Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Wed, 2 Oct 2024 20:52:46 -0600 Subject: [PATCH 13/14] ReturnDataLoad is an EOF only operation (#7670) Update ReturnDataLoadOperation to fail when called from legacy. Signed-off-by: Danno Ferrin --- .../besu/evm/operation/ReturnDataLoadOperation.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/ReturnDataLoadOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/ReturnDataLoadOperation.java index 7d8a5e4209..3c04b7b939 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/operation/ReturnDataLoadOperation.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/operation/ReturnDataLoadOperation.java @@ -16,6 +16,7 @@ package org.hyperledger.besu.evm.operation; import static org.hyperledger.besu.evm.internal.Words.clampedToInt; +import org.hyperledger.besu.evm.Code; import org.hyperledger.besu.evm.EVM; import org.hyperledger.besu.evm.frame.MessageFrame; import org.hyperledger.besu.evm.gascalculator.GasCalculator; @@ -37,6 +38,11 @@ public class ReturnDataLoadOperation extends AbstractOperation { @Override public OperationResult execute(final MessageFrame frame, final EVM evm) { + Code code = frame.getCode(); + if (code.getEofVersion() == 0) { + return InvalidOperation.INVALID_RESULT; + } + final int offset = clampedToInt(frame.popStackItem()); Bytes returnData = frame.getReturnData(); int retunDataSize = returnData.size(); From e63f4730c6473bad15a8e01004ae3cf5882ebc19 Mon Sep 17 00:00:00 2001 From: Matilda Clerke Date: Fri, 4 Oct 2024 08:55:24 +1000 Subject: [PATCH 14/14] 7311: Make changes as discussed in walkthrough meeting Remove DefaultPeerSelector, make EthPeers implement PeerSelector interface, and add PeerTask.getPeerRequirementFilter Signed-off-by: Matilda Clerke --- .../besu/ethereum/eth/manager/EthPeers.java | 17 ++- .../manager/peertask/DefaultPeerSelector.java | 102 ------------- .../eth/manager/peertask/PeerSelector.java | 30 +--- .../eth/manager/peertask/PeerTask.java | 16 ++- .../manager/peertask/PeerTaskExecutor.java | 17 ++- .../peertask/DefaultPeerSelectorTest.java | 135 ------------------ .../peertask/PeerTaskExecutorTest.java | 21 ++- 7 files changed, 50 insertions(+), 288 deletions(-) delete mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java delete mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index d1a54d4d3d..1b413251be 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -17,6 +17,8 @@ package org.hyperledger.besu.ethereum.eth.manager; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthPeer.DisconnectCallback; +import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerSelector; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; import org.hyperledger.besu.ethereum.eth.sync.ChainHeadTracker; import org.hyperledger.besu.ethereum.eth.sync.SnapServerChecker; @@ -26,6 +28,7 @@ import org.hyperledger.besu.ethereum.forkid.ForkId; import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; import org.hyperledger.besu.ethereum.p2p.peers.Peer; +import org.hyperledger.besu.ethereum.p2p.peers.PeerId; import org.hyperledger.besu.ethereum.p2p.rlpx.RlpxAgent; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage; @@ -61,7 +64,7 @@ import org.apache.tuweni.bytes.Bytes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class EthPeers { +public class EthPeers implements PeerSelector { private static final Logger LOG = LoggerFactory.getLogger(EthPeers.class); public static final Comparator TOTAL_DIFFICULTY = Comparator.comparing((final EthPeer p) -> p.chainState().getEstimatedTotalDifficulty()); @@ -465,6 +468,18 @@ public class EthPeers { this.trailingPeerRequirementsSupplier = tprSupplier; } + // Part of the PeerSelector interface, to be split apart later + @Override + public EthPeer getPeer(final Predicate filter) { + return streamBestPeers().filter(filter).findFirst().orElseThrow(NoAvailablePeersException::new); + } + + // Part of the PeerSelector interface, to be split apart later + @Override + public Optional getPeerByPeerId(final PeerId peerId) { + return Optional.ofNullable(activeConnections.get(peerId.getId())); + } + @FunctionalInterface public interface ConnectCallback { void onPeerConnected(EthPeer newPeer); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java deleted file mode 100644 index 41d2e9b700..0000000000 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java +++ /dev/null @@ -1,102 +0,0 @@ -/* - * Copyright contributors to Hyperledger Besu. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.ethereum.eth.manager.peertask; - -import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; -import org.hyperledger.besu.ethereum.p2p.peers.PeerId; -import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; - -import java.util.Collection; -import java.util.Comparator; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Predicate; -import java.util.function.Supplier; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * This is a simple PeerSelector implementation that can be used the default implementation in most - * situations - */ -public class DefaultPeerSelector implements PeerSelector { - private static final Logger LOG = LoggerFactory.getLogger(DefaultPeerSelector.class); - - private final Supplier protocolSpecSupplier; - private final Map ethPeersByPeerId = new ConcurrentHashMap<>(); - - public DefaultPeerSelector(final Supplier protocolSpecSupplier) { - this.protocolSpecSupplier = protocolSpecSupplier; - } - - /** - * Gets the highest reputation peer matching the supplied filter - * - * @param filter a filter to match prospective peers with - * @return the highest reputation peer matching the supplies filter - * @throws NoAvailablePeerException If there are no suitable peers - */ - private EthPeer getPeer(final Predicate filter) throws NoAvailablePeerException { - LOG.trace("Finding peer from pool of {} peers", ethPeersByPeerId.size()); - return ethPeersByPeerId.values().stream() - .filter(filter) - .max(Comparator.naturalOrder()) - .orElseThrow(NoAvailablePeerException::new); - } - - @Override - public EthPeer getPeer( - final Collection usedEthPeers, - final long requiredPeerHeight, - final SubProtocol requiredSubProtocol) - throws NoAvailablePeerException { - return getPeer( - (candidatePeer) -> - isPeerUnused(candidatePeer, usedEthPeers) - && (protocolSpecSupplier.get().isPoS() - || isPeerHeightHighEnough(candidatePeer, requiredPeerHeight)) - && isPeerProtocolSuitable(candidatePeer, requiredSubProtocol)); - } - - @Override - public Optional getPeerByPeerId(final PeerId peerId) { - return Optional.ofNullable(ethPeersByPeerId.get(peerId)); - } - - @Override - public void addPeer(final EthPeer ethPeer) { - ethPeersByPeerId.put(ethPeer.getConnection().getPeer(), ethPeer); - } - - @Override - public void removePeer(final PeerId peerId) { - ethPeersByPeerId.remove(peerId); - } - - private boolean isPeerUnused(final EthPeer ethPeer, final Collection usedEthPeers) { - return !usedEthPeers.contains(ethPeer); - } - - private boolean isPeerHeightHighEnough(final EthPeer ethPeer, final long requiredHeight) { - return ethPeer.chainState().getEstimatedHeight() >= requiredHeight; - } - - private boolean isPeerProtocolSuitable(final EthPeer ethPeer, final SubProtocol protocol) { - return ethPeer.getProtocolName().equals(protocol.getName()); - } -} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java index 93d98a193b..0801f9f00e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java @@ -16,28 +16,20 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.p2p.peers.PeerId; -import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; -import java.util.Collection; import java.util.Optional; +import java.util.function.Predicate; /** Selects the EthPeers for the PeerTaskExecutor */ public interface PeerSelector { /** - * Gets a peer with the requiredPeerHeight (if not PoS), and with the requiredSubProtocol, and - * which is not in the supplied collection of usedEthPeers + * Gets a peer matching the supplied filter * - * @param usedEthPeers a collection of EthPeers to be excluded from selection because they have - * already been used - * @param requiredPeerHeight the minimum peer height required of the selected peer - * @param requiredSubProtocol the SubProtocol required of the peer + * @param filter a Predicate\ matching desirable peers * @return a peer matching the supplied conditions - * @throws NoAvailablePeerException If there are no suitable peers */ - EthPeer getPeer( - Collection usedEthPeers, long requiredPeerHeight, SubProtocol requiredSubProtocol) - throws NoAvailablePeerException; + EthPeer getPeer(final Predicate filter); /** * Attempts to get the EthPeer identified by peerId @@ -47,18 +39,4 @@ public interface PeerSelector { * PeerSelector, or empty otherwise */ Optional getPeerByPeerId(PeerId peerId); - - /** - * Add the supplied EthPeer to the PeerSelector - * - * @param ethPeer the EthPeer to be added to the PeerSelector - */ - void addPeer(EthPeer ethPeer); - - /** - * Remove the EthPeer identified by peerId from the PeerSelector - * - * @param peerId the PeerId of the EthPeer to be removed from the PeerSelector - */ - void removePeer(PeerId peerId); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java index 36bd03531b..1c5ee76c9a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java @@ -14,10 +14,12 @@ */ package org.hyperledger.besu.ethereum.eth.manager.peertask; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import java.util.Collection; +import java.util.function.Predicate; /** * Represents a task to be executed on an EthPeer by the PeerTaskExecutor @@ -32,13 +34,6 @@ public interface PeerTask { */ SubProtocol getSubProtocol(); - /** - * Gets the minimum required block number for a peer to have to successfully execute this task - * - * @return the minimum required block number for a peer to have to successfully execute this task - */ - long getRequiredBlockNumber(); - /** * Gets the request data to send to the EthPeer * @@ -61,4 +56,11 @@ public interface PeerTask { * @return the Collection of behaviors this task is expected to exhibit in the PeetTaskExecutor */ Collection getPeerTaskBehaviors(); + + /** + * Gets a Predicate that checks if an EthPeer is suitable for this PeerTask + * + * @return a Predicate that checks if an EthPeer is suitable for this PeerTask + */ + Predicate getPeerRequirementFilter(); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java index 10c882e7e5..2d9a6edfce 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java @@ -15,6 +15,8 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.metrics.BesuMetricCategory; @@ -37,15 +39,18 @@ public class PeerTaskExecutor { public static final int NO_RETRIES = 1; private final PeerSelector peerSelector; private final PeerTaskRequestSender requestSender; + private final EthScheduler ethScheduler; private final LabelledMetric requestTimer; public PeerTaskExecutor( final PeerSelector peerSelector, final PeerTaskRequestSender requestSender, + final EthScheduler ethScheduler, final MetricsSystem metricsSystem) { this.peerSelector = peerSelector; this.requestSender = requestSender; + this.ethScheduler = ethScheduler; requestTimer = metricsSystem.createLabelledTimer( BesuMetricCategory.PEERS, @@ -66,10 +71,12 @@ public class PeerTaskExecutor { try { peer = peerSelector.getPeer( - usedEthPeers, peerTask.getRequiredBlockNumber(), peerTask.getSubProtocol()); + (candidatePeer) -> + peerTask.getPeerRequirementFilter().test(candidatePeer) + && !usedEthPeers.contains(candidatePeer)); usedEthPeers.add(peer); executorResult = executeAgainstPeer(peerTask, peer); - } catch (NoAvailablePeerException e) { + } catch (NoAvailablePeersException e) { executorResult = new PeerTaskExecutorResult<>( Optional.empty(), PeerTaskExecutorResponseCode.NO_PEER_AVAILABLE); @@ -81,7 +88,8 @@ public class PeerTaskExecutor { } public CompletableFuture> executeAsync(final PeerTask peerTask) { - return CompletableFuture.supplyAsync(() -> execute(peerTask)); + return ethScheduler.scheduleSyncWorkerTask( + () -> CompletableFuture.completedFuture(execute(peerTask))); } public PeerTaskExecutorResult executeAgainstPeer( @@ -138,7 +146,8 @@ public class PeerTaskExecutor { public CompletableFuture> executeAgainstPeerAsync( final PeerTask peerTask, final EthPeer peer) { - return CompletableFuture.supplyAsync(() -> executeAgainstPeer(peerTask, peer)); + return ethScheduler.scheduleSyncWorkerTask( + () -> CompletableFuture.completedFuture(executeAgainstPeer(peerTask, peer))); } private boolean sleepBetweenRetries() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java deleted file mode 100644 index add2b1e612..0000000000 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright contributors to Hyperledger Besu. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.ethereum.eth.manager.peertask; - -import org.hyperledger.besu.ethereum.eth.EthProtocol; -import org.hyperledger.besu.ethereum.eth.SnapProtocol; -import org.hyperledger.besu.ethereum.eth.manager.ChainState; -import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.PeerReputation; -import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; -import org.hyperledger.besu.ethereum.p2p.peers.Peer; -import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; -import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; - -import java.util.HashSet; -import java.util.Set; - -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; - -public class DefaultPeerSelectorTest { - - public DefaultPeerSelector peerSelector; - - @BeforeEach - public void beforeTest() { - ProtocolSpec protocolSpec = Mockito.mock(ProtocolSpec.class); - Mockito.when(protocolSpec.isPoS()).thenReturn(false); - peerSelector = new DefaultPeerSelector(() -> protocolSpec); - } - - @Test - public void testGetPeer() throws NoAvailablePeerException { - EthPeer expectedPeer = createTestPeer(10, EthProtocol.get(), 5); - peerSelector.addPeer(expectedPeer); - EthPeer excludedForLowChainHeightPeer = createTestPeer(5, EthProtocol.get(), 50); - peerSelector.addPeer(excludedForLowChainHeightPeer); - EthPeer excludedForWrongProtocolPeer = createTestPeer(10, SnapProtocol.get(), 50); - peerSelector.addPeer(excludedForWrongProtocolPeer); - EthPeer excludedForLowReputationPeer = createTestPeer(10, EthProtocol.get(), 1); - peerSelector.addPeer(excludedForLowReputationPeer); - EthPeer excludedForBeingAlreadyUsedPeer = createTestPeer(10, EthProtocol.get(), 50); - peerSelector.addPeer(excludedForBeingAlreadyUsedPeer); - - Set usedEthPeers = new HashSet<>(); - usedEthPeers.add(excludedForBeingAlreadyUsedPeer); - - EthPeer result = peerSelector.getPeer(usedEthPeers, 10, EthProtocol.get()); - - Assertions.assertSame(expectedPeer, result); - } - - @Test - public void testGetPeerButNoPeerMatchesFilter() { - EthPeer expectedPeer = createTestPeer(10, EthProtocol.get(), 5); - peerSelector.addPeer(expectedPeer); - EthPeer excludedForLowChainHeightPeer = createTestPeer(5, EthProtocol.get(), 50); - peerSelector.addPeer(excludedForLowChainHeightPeer); - EthPeer excludedForWrongProtocolPeer = createTestPeer(10, SnapProtocol.get(), 50); - peerSelector.addPeer(excludedForWrongProtocolPeer); - EthPeer excludedForLowReputationPeer = createTestPeer(10, EthProtocol.get(), 1); - peerSelector.addPeer(excludedForLowReputationPeer); - EthPeer excludedForBeingAlreadyUsedPeer = createTestPeer(10, EthProtocol.get(), 50); - peerSelector.addPeer(excludedForBeingAlreadyUsedPeer); - - Set usedEthPeers = new HashSet<>(); - usedEthPeers.add(excludedForBeingAlreadyUsedPeer); - - Assertions.assertThrows( - NoAvailablePeerException.class, - () -> peerSelector.getPeer(usedEthPeers, 10, new MockSubProtocol())); - } - - private EthPeer createTestPeer( - final long chainHeight, final SubProtocol protocol, final int reputation) { - EthPeer ethPeer = Mockito.mock(EthPeer.class); - PeerConnection peerConnection = Mockito.mock(PeerConnection.class); - Peer peer = Mockito.mock(Peer.class); - ChainState chainState = Mockito.mock(ChainState.class); - PeerReputation peerReputation = Mockito.mock(PeerReputation.class); - - Mockito.when(ethPeer.getConnection()).thenReturn(peerConnection); - Mockito.when(peerConnection.getPeer()).thenReturn(peer); - Mockito.when(ethPeer.getProtocolName()).thenReturn(protocol.getName()); - Mockito.when(ethPeer.chainState()).thenReturn(chainState); - Mockito.when(chainState.getEstimatedHeight()).thenReturn(chainHeight); - Mockito.when(ethPeer.getReputation()).thenReturn(peerReputation); - Mockito.when(peerReputation.getScore()).thenReturn(reputation); - - Mockito.when(ethPeer.compareTo(Mockito.any(EthPeer.class))) - .thenAnswer( - (invocationOnMock) -> { - EthPeer otherPeer = invocationOnMock.getArgument(0, EthPeer.class); - return Integer.compare(reputation, otherPeer.getReputation().getScore()); - }); - return ethPeer; - } - - private static class MockSubProtocol implements SubProtocol { - - @Override - public String getName() { - return "Mock"; - } - - @Override - public int messageSpace(final int protocolVersion) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isValidMessageCode(final int protocolVersion, final int code) { - throw new UnsupportedOperationException(); - } - - @Override - public String messageName(final int protocolVersion, final int code) { - throw new UnsupportedOperationException(); - } - } -} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java index 15b1747bc7..98afe58b11 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java @@ -15,16 +15,17 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; +import java.util.function.Predicate; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -37,6 +38,7 @@ import org.mockito.MockitoAnnotations; public class PeerTaskExecutorTest { private @Mock PeerSelector peerSelector; private @Mock PeerTaskRequestSender requestSender; + private @Mock EthScheduler ethScheduler; private @Mock PeerTask peerTask; private @Mock SubProtocol subprotocol; private @Mock MessageData requestMessageData; @@ -49,7 +51,8 @@ public class PeerTaskExecutorTest { @BeforeEach public void beforeTest() { mockCloser = MockitoAnnotations.openMocks(this); - peerTaskExecutor = new PeerTaskExecutor(peerSelector, requestSender, new NoOpMetricsSystem()); + peerTaskExecutor = + new PeerTaskExecutor(peerSelector, requestSender, ethScheduler, new NoOpMetricsSystem()); } @AfterEach @@ -201,14 +204,10 @@ public class PeerTaskExecutorTest { NoAvailablePeerException { Object responseObject = new Object(); - Mockito.when( - peerSelector.getPeer( - Mockito.any(Collection.class), Mockito.eq(10L), Mockito.eq(subprotocol))) - .thenReturn(ethPeer); + Mockito.when(peerSelector.getPeer(Mockito.any(Predicate.class))).thenReturn(ethPeer); Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()).thenReturn(Collections.emptyList()); - Mockito.when(peerTask.getRequiredBlockNumber()).thenReturn(10L); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) @@ -232,22 +231,18 @@ public class PeerTaskExecutorTest { ExecutionException, InterruptedException, TimeoutException, - InvalidPeerTaskResponseException, - NoAvailablePeerException { + InvalidPeerTaskResponseException { Object responseObject = new Object(); int requestMessageDataCode = 123; EthPeer peer2 = Mockito.mock(EthPeer.class); - Mockito.when( - peerSelector.getPeer( - Mockito.any(Collection.class), Mockito.eq(10L), Mockito.eq(subprotocol))) + Mockito.when(peerSelector.getPeer(Mockito.any(Predicate.class))) .thenReturn(ethPeer) .thenReturn(peer2); Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()) .thenReturn(List.of(PeerTaskRetryBehavior.RETRY_WITH_OTHER_PEERS)); - Mockito.when(peerTask.getRequiredBlockNumber()).thenReturn(10L); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenThrow(new TimeoutException());