From 4f07e76a6c26b294bcc8f50deefefc78438636dd Mon Sep 17 00:00:00 2001 From: Matilda-Clerke Date: Tue, 24 Sep 2024 13:04:03 +1000 Subject: [PATCH 1/6] 7311: Add feature toggle for enabling use of the peertask system where available (#7633) Signed-off-by: Matilda Clerke --- .../options/unstable/SynchronizerOptions.java | 18 +++++++++++++++++- besu/src/test/resources/everything_config.toml | 1 + .../eth/sync/SynchronizerConfiguration.java | 18 ++++++++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java index 95bbe0a2b1f..816d9df00a1 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java @@ -314,6 +314,13 @@ public void parseBlockPropagationRange(final String arg) { description = "Snap sync enabled for BFT chains (default: ${DEFAULT-VALUE})") private Boolean snapsyncBftEnabled = SnapSyncConfiguration.DEFAULT_SNAP_SYNC_BFT_ENABLED; + @CommandLine.Option( + names = {"--Xpeertask-system-enabled"}, + hidden = true, + description = + "Temporary feature toggle to enable using the new peertask system (default: ${DEFAULT-VALUE})") + private final Boolean isPeerTaskSystemEnabled = false; + private SynchronizerOptions() {} /** @@ -334,6 +341,15 @@ public boolean isSnapSyncBftEnabled() { return snapsyncBftEnabled; } + /** + * Flag to indicate whether the peer task system should be used where available + * + * @return true if the peer task system should be used where available + */ + public boolean isPeerTaskSystemEnabled() { + return isPeerTaskSystemEnabled; + } + /** * Create synchronizer options. * @@ -420,7 +436,7 @@ public SynchronizerConfiguration.Builder toDomainObject() { .isSnapSyncBftEnabled(snapsyncBftEnabled) .build()); builder.checkpointPostMergeEnabled(checkpointPostMergeSyncEnabled); - + builder.isPeerTaskSystemEnabled(isPeerTaskSystemEnabled); return builder; } diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml index e3d7a3f28d3..d28152b47cd 100644 --- a/besu/src/test/resources/everything_config.toml +++ b/besu/src/test/resources/everything_config.toml @@ -226,6 +226,7 @@ Xsecp256k1-native-enabled=false Xaltbn128-native-enabled=false Xsnapsync-server-enabled=true Xbonsai-full-flat-db-enabled=true +Xpeertask-system-enabled=false # compatibility flags compatibility-eth64-forkid-enabled=false diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SynchronizerConfiguration.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SynchronizerConfiguration.java index d46da85dc48..d72f76c213c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SynchronizerConfiguration.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SynchronizerConfiguration.java @@ -85,6 +85,7 @@ public class SynchronizerConfiguration { private final int maxTrailingPeers; private final long worldStateMinMillisBeforeStalling; private final long propagationManagerGetBlockTimeoutMillis; + private final boolean isPeerTaskSystemEnabled; private SynchronizerConfiguration( final int syncPivotDistance, @@ -108,7 +109,8 @@ private SynchronizerConfiguration( final int computationParallelism, final int maxTrailingPeers, final long propagationManagerGetBlockTimeoutMillis, - final boolean checkpointPostMergeEnabled) { + final boolean checkpointPostMergeEnabled, + final boolean isPeerTaskSystemEnabled) { this.syncPivotDistance = syncPivotDistance; this.fastSyncFullValidationRate = fastSyncFullValidationRate; this.syncMinimumPeerCount = syncMinimumPeerCount; @@ -131,6 +133,7 @@ private SynchronizerConfiguration( this.maxTrailingPeers = maxTrailingPeers; this.propagationManagerGetBlockTimeoutMillis = propagationManagerGetBlockTimeoutMillis; this.checkpointPostMergeEnabled = checkpointPostMergeEnabled; + this.isPeerTaskSystemEnabled = isPeerTaskSystemEnabled; } public static Builder builder() { @@ -256,6 +259,10 @@ public long getPropagationManagerGetBlockTimeoutMillis() { return propagationManagerGetBlockTimeoutMillis; } + public boolean isPeerTaskSystemEnabled() { + return isPeerTaskSystemEnabled; + } + public static class Builder { private SyncMode syncMode = SyncMode.FULL; private int syncMinimumPeerCount = DEFAULT_SYNC_MINIMUM_PEERS; @@ -280,6 +287,7 @@ public static class Builder { DEFAULT_WORLD_STATE_MAX_REQUESTS_WITHOUT_PROGRESS; private long worldStateMinMillisBeforeStalling = DEFAULT_WORLD_STATE_MIN_MILLIS_BEFORE_STALLING; private int worldStateTaskCacheSize = DEFAULT_WORLD_STATE_TASK_CACHE_SIZE; + private boolean isPeerTaskSystemEnabled = false; private long propagationManagerGetBlockTimeoutMillis = DEFAULT_PROPAGATION_MANAGER_GET_BLOCK_TIMEOUT_MILLIS; @@ -406,6 +414,11 @@ public Builder checkpointPostMergeEnabled(final boolean checkpointPostMergeEnabl return this; } + public Builder isPeerTaskSystemEnabled(final boolean isPeerTaskSystemEnabled) { + this.isPeerTaskSystemEnabled = isPeerTaskSystemEnabled; + return this; + } + public SynchronizerConfiguration build() { return new SynchronizerConfiguration( syncPivotDistance, @@ -429,7 +442,8 @@ public SynchronizerConfiguration build() { computationParallelism, maxTrailingPeers, propagationManagerGetBlockTimeoutMillis, - checkpointPostMergeEnabled); + checkpointPostMergeEnabled, + isPeerTaskSystemEnabled); } } } From 04ba15aa922659a1516589612c6057301804c4d5 Mon Sep 17 00:00:00 2001 From: Glory Agatevure Date: Tue, 24 Sep 2024 06:06:16 +0100 Subject: [PATCH 2/6] Add consolidationRequestContract in jsonGenesisConfig (#7647) * Include consolidationRequestContract in jsonGenesisConfigOptions Signed-off-by: gconnect * Update changelog and ran spotlessApply Signed-off-by: gconnect * Rename consolidationRequestPredeployAddress to consolidationRequestContractAddress Signed-off-by: gconnect * Create request contract addresses class Signed-off-by: gconnect * Update method calls Signed-off-by: gconnect * Refactor RequestContractAddresses class and update method calls and test Signed-off-by: gconnect --------- Signed-off-by: gconnect Co-authored-by: Gabriel-Trintinalia --- CHANGELOG.md | 2 + .../besu/config/GenesisConfigOptions.java | 7 +++ .../besu/config/JsonGenesisConfigOptions.java | 11 ++++ .../besu/config/StubGenesisConfigOptions.java | 5 ++ .../besu/config/GenesisConfigOptionsTest.java | 27 ++++++++ .../mainnet/MainnetProtocolSpecs.java | 16 ++--- .../ConsolidationRequestProcessor.java | 9 ++- .../requests/MainnetRequestsValidator.java | 21 ++++--- .../requests/RequestContractAddresses.java | 61 +++++++++++++++++++ .../mainnet/PragueRequestsValidatorTest.java | 11 +++- 10 files changed, 148 insertions(+), 22 deletions(-) create mode 100644 ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/RequestContractAddresses.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 96e1511eead..5ce29955d12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog ## [Unreleased] +- Add configuration of Consolidation Request Contract Address via genesis configuration [#7647](https://github.com/hyperledger/besu/pull/7647) + ### Upcoming Breaking Changes - k8s (KUBERNETES) Nat method is now deprecated and will be removed in a future release diff --git a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java index d6323ee9bfb..07ddd0d7eac 100644 --- a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java @@ -539,4 +539,11 @@ default boolean isConsensusMigration() { * @return the deposit address */ Optional
getDepositContractAddress(); + + /** + * The consolidation request contract address + * + * @return the consolidation request contract address + */ + Optional
getConsolidationRequestContractAddress(); } diff --git a/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java index 67114b29bf3..83b1f48fb48 100644 --- a/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java @@ -52,6 +52,8 @@ public class JsonGenesisConfigOptions implements GenesisConfigOptions { private static final String WITHDRAWAL_REQUEST_CONTRACT_ADDRESS_KEY = "withdrawalrequestcontractaddress"; private static final String DEPOSIT_CONTRACT_ADDRESS_KEY = "depositcontractaddress"; + private static final String CONSOLIDATION_REQUEST_CONTRACT_ADDRESS_KEY = + "consolidationrequestcontractaddress"; private final ObjectNode configRoot; private final Map configOverrides = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); @@ -453,6 +455,13 @@ public Optional
getDepositContractAddress() { return inputAddress.map(Address::fromHexString); } + @Override + public Optional
getConsolidationRequestContractAddress() { + Optional inputAddress = + JsonUtil.getString(configRoot, CONSOLIDATION_REQUEST_CONTRACT_ADDRESS_KEY); + return inputAddress.map(Address::fromHexString); + } + @Override public Map asMap() { final ImmutableMap.Builder builder = ImmutableMap.builder(); @@ -504,6 +513,8 @@ public Map asMap() { getWithdrawalRequestContractAddress() .ifPresent(l -> builder.put("withdrawalRequestContractAddress", l)); getDepositContractAddress().ifPresent(l -> builder.put("depositContractAddress", l)); + getConsolidationRequestContractAddress() + .ifPresent(l -> builder.put("consolidationRequestContractAddress", l)); if (isClique()) { builder.put("clique", getCliqueConfigOptions().asMap()); diff --git a/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java index efe56a086d0..ee9584fd3a4 100644 --- a/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java @@ -467,6 +467,11 @@ public Optional
getDepositContractAddress() { return Optional.empty(); } + @Override + public Optional
getConsolidationRequestContractAddress() { + return Optional.empty(); + } + /** * Homestead block stub genesis config options. * diff --git a/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java b/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java index 219ea4fcf8a..69494a9c9c4 100644 --- a/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java +++ b/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java @@ -382,6 +382,33 @@ void asMapIncludesDepositContractAddress() { .containsValue(Address.ZERO); } + @Test + void shouldGetConsolidationRequestContractAddress() { + final GenesisConfigOptions config = + fromConfigOptions( + singletonMap( + "consolidationRequestContractAddress", + "0x00000000219ab540356cbb839cbe05303d7705fa")); + assertThat(config.getConsolidationRequestContractAddress()) + .hasValue(Address.fromHexString("0x00000000219ab540356cbb839cbe05303d7705fa")); + } + + @Test + void shouldNotHaveConsolidationRequestContractAddressWhenEmpty() { + final GenesisConfigOptions config = fromConfigOptions(emptyMap()); + assertThat(config.getConsolidationRequestContractAddress()).isEmpty(); + } + + @Test + void asMapIncludesConsolidationRequestContractAddress() { + final GenesisConfigOptions config = + fromConfigOptions(Map.of("consolidationRequestContractAddress", "0x0")); + + assertThat(config.asMap()) + .containsOnlyKeys("consolidationRequestContractAddress") + .containsValue(Address.ZERO); + } + private GenesisConfigOptions fromConfigOptions(final Map configOptions) { final ObjectNode rootNode = JsonUtil.createEmptyObjectNode(); final ObjectNode options = JsonUtil.objectNodeFromMap(configOptions); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java index e6d7a88cbdf..b5b2f678d67 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java @@ -14,10 +14,8 @@ */ package org.hyperledger.besu.ethereum.mainnet; -import static org.hyperledger.besu.ethereum.mainnet.requests.DepositRequestProcessor.DEFAULT_DEPOSIT_CONTRACT_ADDRESS; import static org.hyperledger.besu.ethereum.mainnet.requests.MainnetRequestsValidator.pragueRequestsProcessors; import static org.hyperledger.besu.ethereum.mainnet.requests.MainnetRequestsValidator.pragueRequestsValidator; -import static org.hyperledger.besu.ethereum.mainnet.requests.WithdrawalRequestProcessor.DEFAULT_WITHDRAWAL_REQUEST_CONTRACT_ADDRESS; import org.hyperledger.besu.config.GenesisConfigOptions; import org.hyperledger.besu.config.PowAlgorithm; @@ -41,6 +39,7 @@ import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket; import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket; import org.hyperledger.besu.ethereum.mainnet.parallelization.MainnetParallelBlockProcessor; +import org.hyperledger.besu.ethereum.mainnet.requests.RequestContractAddresses; import org.hyperledger.besu.ethereum.privacy.PrivateTransactionProcessor; import org.hyperledger.besu.ethereum.privacy.PrivateTransactionValidator; import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater; @@ -767,12 +766,8 @@ static ProtocolSpecBuilder pragueDefinition( final boolean isParallelTxProcessingEnabled, final MetricsSystem metricsSystem) { - final Address withdrawalRequestContractAddress = - genesisConfigOptions - .getWithdrawalRequestContractAddress() - .orElse(DEFAULT_WITHDRAWAL_REQUEST_CONTRACT_ADDRESS); - final Address depositContractAddress = - genesisConfigOptions.getDepositContractAddress().orElse(DEFAULT_DEPOSIT_CONTRACT_ADDRESS); + RequestContractAddresses requestContractAddresses = + RequestContractAddresses.fromGenesis(genesisConfigOptions); return cancunDefinition( chainId, @@ -794,10 +789,9 @@ static ProtocolSpecBuilder pragueDefinition( .precompileContractRegistryBuilder(MainnetPrecompiledContractRegistries::prague) // EIP-7002 Withdrawals / EIP-6610 Deposits / EIP-7685 Requests - .requestsValidator(pragueRequestsValidator(depositContractAddress)) + .requestsValidator(pragueRequestsValidator(requestContractAddresses)) // EIP-7002 Withdrawals / EIP-6610 Deposits / EIP-7685 Requests - .requestProcessorCoordinator( - pragueRequestsProcessors(withdrawalRequestContractAddress, depositContractAddress)) + .requestProcessorCoordinator(pragueRequestsProcessors(requestContractAddresses)) // change to accept EIP-7702 transactions .transactionValidatorFactoryBuilder( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/ConsolidationRequestProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/ConsolidationRequestProcessor.java index 0a48d8278c9..641720670fb 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/ConsolidationRequestProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/ConsolidationRequestProcessor.java @@ -22,13 +22,18 @@ public class ConsolidationRequestProcessor extends AbstractSystemCallRequestProcessor { - public static final Address CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS = + public static final Address CONSOLIDATION_REQUEST_CONTRACT_ADDRESS = Address.fromHexString("0x00b42dbF2194e931E80326D950320f7d9Dbeac02"); private static final int ADDRESS_BYTES = 20; private static final int PUBLIC_KEY_BYTES = 48; private static final int CONSOLIDATION_REQUEST_BYTES_SIZE = ADDRESS_BYTES + PUBLIC_KEY_BYTES + PUBLIC_KEY_BYTES; + private final Address consolidationRequestContractAddress; + + public ConsolidationRequestProcessor(final Address consolidationRequestContractAddress) { + this.consolidationRequestContractAddress = consolidationRequestContractAddress; + } /** * Gets the call address for consolidation requests. @@ -37,7 +42,7 @@ public class ConsolidationRequestProcessor */ @Override protected Address getCallAddress() { - return CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS; + return consolidationRequestContractAddress; } /** diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/MainnetRequestsValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/MainnetRequestsValidator.java index 6e61a0343c3..9c86d18f7ad 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/MainnetRequestsValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/MainnetRequestsValidator.java @@ -14,27 +14,34 @@ */ package org.hyperledger.besu.ethereum.mainnet.requests; -import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.RequestType; public class MainnetRequestsValidator { public static RequestsValidatorCoordinator pragueRequestsValidator( - final Address depositContractAddress) { + final RequestContractAddresses requestContractAddresses) { return new RequestsValidatorCoordinator.Builder() .addValidator(RequestType.WITHDRAWAL, new WithdrawalRequestValidator()) .addValidator(RequestType.CONSOLIDATION, new ConsolidationRequestValidator()) - .addValidator(RequestType.DEPOSIT, new DepositRequestValidator(depositContractAddress)) + .addValidator( + RequestType.DEPOSIT, + new DepositRequestValidator(requestContractAddresses.getDepositContractAddress())) .build(); } public static RequestProcessorCoordinator pragueRequestsProcessors( - final Address withdrawalRequestContractAddress, final Address depositContractAddress) { + final RequestContractAddresses requestContractAddresses) { return new RequestProcessorCoordinator.Builder() .addProcessor( RequestType.WITHDRAWAL, - new WithdrawalRequestProcessor(withdrawalRequestContractAddress)) - .addProcessor(RequestType.CONSOLIDATION, new ConsolidationRequestProcessor()) - .addProcessor(RequestType.DEPOSIT, new DepositRequestProcessor(depositContractAddress)) + new WithdrawalRequestProcessor( + requestContractAddresses.getWithdrawalRequestContractAddress())) + .addProcessor( + RequestType.CONSOLIDATION, + new ConsolidationRequestProcessor( + requestContractAddresses.getConsolidationRequestContractAddress())) + .addProcessor( + RequestType.DEPOSIT, + new DepositRequestProcessor(requestContractAddresses.getDepositContractAddress())) .build(); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/RequestContractAddresses.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/RequestContractAddresses.java new file mode 100644 index 00000000000..b75677dda79 --- /dev/null +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/requests/RequestContractAddresses.java @@ -0,0 +1,61 @@ +/* + * 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.mainnet.requests; + +import static org.hyperledger.besu.ethereum.mainnet.requests.ConsolidationRequestProcessor.CONSOLIDATION_REQUEST_CONTRACT_ADDRESS; +import static org.hyperledger.besu.ethereum.mainnet.requests.DepositRequestProcessor.DEFAULT_DEPOSIT_CONTRACT_ADDRESS; +import static org.hyperledger.besu.ethereum.mainnet.requests.WithdrawalRequestProcessor.DEFAULT_WITHDRAWAL_REQUEST_CONTRACT_ADDRESS; + +import org.hyperledger.besu.config.GenesisConfigOptions; +import org.hyperledger.besu.datatypes.Address; + +public class RequestContractAddresses { + private final Address withdrawalRequestContractAddress; + private final Address depositContractAddress; + private final Address consolidationRequestContractAddress; + + public RequestContractAddresses( + final Address withdrawalRequestContractAddress, + final Address depositContractAddress, + final Address consolidationRequestContractAddress) { + this.withdrawalRequestContractAddress = withdrawalRequestContractAddress; + this.depositContractAddress = depositContractAddress; + this.consolidationRequestContractAddress = consolidationRequestContractAddress; + } + + public static RequestContractAddresses fromGenesis( + final GenesisConfigOptions genesisConfigOptions) { + return new RequestContractAddresses( + genesisConfigOptions + .getWithdrawalRequestContractAddress() + .orElse(DEFAULT_WITHDRAWAL_REQUEST_CONTRACT_ADDRESS), + genesisConfigOptions.getDepositContractAddress().orElse(DEFAULT_DEPOSIT_CONTRACT_ADDRESS), + genesisConfigOptions + .getConsolidationRequestContractAddress() + .orElse(CONSOLIDATION_REQUEST_CONTRACT_ADDRESS)); + } + + public Address getWithdrawalRequestContractAddress() { + return withdrawalRequestContractAddress; + } + + public Address getDepositContractAddress() { + return depositContractAddress; + } + + public Address getConsolidationRequestContractAddress() { + return consolidationRequestContractAddress; + } +} diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PragueRequestsValidatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PragueRequestsValidatorTest.java index 6c325b70a86..6158ba44c37 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PragueRequestsValidatorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PragueRequestsValidatorTest.java @@ -17,8 +17,10 @@ import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode.NONE; +import static org.hyperledger.besu.ethereum.mainnet.requests.ConsolidationRequestProcessor.CONSOLIDATION_REQUEST_CONTRACT_ADDRESS; import static org.hyperledger.besu.ethereum.mainnet.requests.DepositRequestProcessor.DEFAULT_DEPOSIT_CONTRACT_ADDRESS; import static org.hyperledger.besu.ethereum.mainnet.requests.MainnetRequestsValidator.pragueRequestsValidator; +import static org.hyperledger.besu.ethereum.mainnet.requests.WithdrawalRequestProcessor.DEFAULT_WITHDRAWAL_REQUEST_CONTRACT_ADDRESS; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; @@ -31,6 +33,7 @@ import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; import org.hyperledger.besu.ethereum.core.Request; import org.hyperledger.besu.ethereum.core.WithdrawalRequest; +import org.hyperledger.besu.ethereum.mainnet.requests.RequestContractAddresses; import org.hyperledger.besu.ethereum.mainnet.requests.RequestsValidatorCoordinator; import org.hyperledger.besu.evm.log.LogsBloomFilter; @@ -52,9 +55,13 @@ class PragueRequestsValidatorTest { @Mock private ProtocolSchedule protocolSchedule; @Mock private ProtocolSpec protocolSpec; @Mock private WithdrawalsValidator withdrawalsValidator; + private final RequestContractAddresses requestContractAddresses = + new RequestContractAddresses( + DEFAULT_WITHDRAWAL_REQUEST_CONTRACT_ADDRESS, + DEFAULT_DEPOSIT_CONTRACT_ADDRESS, + CONSOLIDATION_REQUEST_CONTRACT_ADDRESS); - RequestsValidatorCoordinator requestValidator = - pragueRequestsValidator(DEFAULT_DEPOSIT_CONTRACT_ADDRESS); + RequestsValidatorCoordinator requestValidator = pragueRequestsValidator(requestContractAddresses); @BeforeEach public void setUp() { From 2aa3848950c86bbe720531539bdb990ab1fcaa7b Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 24 Sep 2024 11:49:17 +0200 Subject: [PATCH 3/6] Enable logging during unit tests (#7672) Signed-off-by: Fabio Di Fabio --- testutil/src/main/resources/{log4j2.xml => log4j2-test.xml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename testutil/src/main/resources/{log4j2.xml => log4j2-test.xml} (100%) diff --git a/testutil/src/main/resources/log4j2.xml b/testutil/src/main/resources/log4j2-test.xml similarity index 100% rename from testutil/src/main/resources/log4j2.xml rename to testutil/src/main/resources/log4j2-test.xml From e0518c6d94bf1ae4f0e3742a9e3e51aac7e15fb3 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Tue, 24 Sep 2024 20:50:19 +1000 Subject: [PATCH 4/6] Force besu to stop on plugin initialization errors (#7662) Signed-off-by: Gabriel-Trintinalia --- CHANGELOG.md | 1 + .../dsl/node/ThreadBesuNodeRunner.java | 3 +- .../acceptance/plugins/TestPicoCLIPlugin.java | 51 +++- .../services/BesuPluginContextImplTest.java | 266 +++++++++++++++--- .../org/hyperledger/besu/cli/BesuCommand.java | 6 +- .../besu/cli/DefaultCommandValues.java | 3 + .../stable/PluginsConfigurationOptions.java | 30 +- .../besu/services/BesuPluginContextImpl.java | 91 ++++-- .../besu/cli/PluginsOptionsTest.java | 51 +++- .../src/test/resources/everything_config.toml | 3 +- .../core/plugins/PluginConfiguration.java | 18 +- 11 files changed, 424 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce29955d12..bb867af945e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - k8s (KUBERNETES) Nat method is now deprecated and will be removed in a future release ### Breaking Changes +- Besu will now fail to start if any plugins encounter errors during initialization. To allow Besu to continue running despite plugin errors, use the `--plugin-continue-on-error` option. [#7662](https://github.com/hyperledger/besu/pull/7662) ### Additions and Improvements - Remove privacy test classes support [#7569](https://github.com/hyperledger/besu/pull/7569) 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 5d78f1460c4..90de0b0e952 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 @@ -503,8 +503,9 @@ public BesuPluginContextImpl providePluginContext( besuPluginContext.addService(PermissioningService.class, permissioningService); besuPluginContext.addService(PrivacyPluginService.class, new PrivacyPluginServiceImpl()); - besuPluginContext.registerPlugins( + besuPluginContext.initialize( new PluginConfiguration.Builder().pluginsDir(pluginsPath).build()); + besuPluginContext.registerPlugins(); commandLine.parseArgs(extraCLIOptions.toArray(new String[0])); // register built-in plugins diff --git a/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java b/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java index 0db5281537c..375fbd490ec 100644 --- a/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java +++ b/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java @@ -1,5 +1,5 @@ /* - * Copyright ConsenSys AG. + * 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 @@ -32,16 +32,25 @@ public class TestPicoCLIPlugin implements BesuPlugin { private static final Logger LOG = LoggerFactory.getLogger(TestPicoCLIPlugin.class); + private static final String UNSET = "UNSET"; + private static final String FAIL_REGISTER = "FAILREGISTER"; + private static final String FAIL_BEFORE_EXTERNAL_SERVICES = "FAILBEFOREEXTERNALSERVICES"; + private static final String FAIL_START = "FAILSTART"; + private static final String FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP = + "FAILAFTEREXTERNALSERVICEPOSTMAINLOOP"; + private static final String FAIL_STOP = "FAILSTOP"; + private static final String PLUGIN_LIFECYCLE_PREFIX = "pluginLifecycle."; + @Option( names = {"--Xplugin-test-option"}, hidden = true, - defaultValue = "UNSET") + defaultValue = UNSET) String testOption = System.getProperty("testPicoCLIPlugin.testOption"); @Option( names = {"--plugin-test-stable-option"}, hidden = true, - defaultValue = "UNSET") + defaultValue = UNSET) String stableOption = ""; private String state = "uninited"; @@ -52,7 +61,7 @@ public void register(final BesuContext context) { LOG.info("Registering. Test Option is '{}'", testOption); state = "registering"; - if ("FAILREGISTER".equals(testOption)) { + if (FAIL_REGISTER.equals(testOption)) { state = "failregister"; throw new RuntimeException("I was told to fail at registration"); } @@ -66,12 +75,26 @@ public void register(final BesuContext context) { state = "registered"; } + @Override + public void beforeExternalServices() { + LOG.info("Before external services. Test Option is '{}'", testOption); + state = "beforeExternalServices"; + + if (FAIL_BEFORE_EXTERNAL_SERVICES.equals(testOption)) { + state = "failbeforeExternalServices"; + throw new RuntimeException("I was told to fail before external services"); + } + + writeSignal("beforeExternalServices"); + state = "beforeExternalServicesFinished"; + } + @Override public void start() { LOG.info("Starting. Test Option is '{}'", testOption); state = "starting"; - if ("FAILSTART".equals(testOption)) { + if (FAIL_START.equals(testOption)) { state = "failstart"; throw new RuntimeException("I was told to fail at startup"); } @@ -80,12 +103,26 @@ public void start() { state = "started"; } + @Override + public void afterExternalServicePostMainLoop() { + LOG.info("After external services post main loop. Test Option is '{}'", testOption); + state = "afterExternalServicePostMainLoop"; + + if (FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP.equals(testOption)) { + state = "failafterExternalServicePostMainLoop"; + throw new RuntimeException("I was told to fail after external services post main loop"); + } + + writeSignal("afterExternalServicePostMainLoop"); + state = "afterExternalServicePostMainLoopFinished"; + } + @Override public void stop() { LOG.info("Stopping. Test Option is '{}'", testOption); state = "stopping"; - if ("FAILSTOP".equals(testOption)) { + if (FAIL_STOP.equals(testOption)) { state = "failstop"; throw new RuntimeException("I was told to fail at stop"); } @@ -103,7 +140,7 @@ public String getState() { @SuppressWarnings("ResultOfMethodCallIgnored") private void writeSignal(final String signal) { try { - final File callbackFile = new File(callbackDir, "pluginLifecycle." + signal); + final File callbackFile = new File(callbackDir, PLUGIN_LIFECYCLE_PREFIX + signal); if (!callbackFile.getParentFile().exists()) { callbackFile.getParentFile().mkdirs(); callbackFile.getParentFile().deleteOnExit(); diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index 7e277041776..a6d167665ff 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -1,5 +1,5 @@ /* - * Copyright ConsenSys AG. + * 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 @@ -40,8 +40,31 @@ public class BesuPluginContextImplTest { private static final Path DEFAULT_PLUGIN_DIRECTORY = Paths.get("."); + private static final String TEST_PICO_CLI_PLUGIN = "TestPicoCLIPlugin"; + private static final String TEST_PICO_CLI_PLUGIN_TEST_OPTION = "testPicoCLIPlugin.testOption"; + private static final String FAIL_REGISTER = "FAILREGISTER"; + private static final String FAIL_START = "FAILSTART"; + private static final String FAIL_STOP = "FAILSTOP"; + private static final String FAIL_BEFORE_EXTERNAL_SERVICES = "FAILBEFOREEXTERNALSERVICES"; + private static final String FAIL_BEFORE_MAIN_LOOP = "FAILBEFOREMAINLOOP"; + private static final String FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP = + "FAILAFTEREXTERNALSERVICEPOSTMAINLOOP"; + private static final String NON_EXISTENT_PLUGIN = "NonExistentPlugin"; + private static final String REGISTERED = "registered"; + private static final String STARTED = "started"; + private static final String STOPPED = "stopped"; + private static final String FAIL_START_STATE = "failstart"; + private static final String FAIL_STOP_STATE = "failstop"; + private BesuPluginContextImpl contextImpl; + private static final PluginConfiguration DEFAULT_CONFIGURATION = + PluginConfiguration.builder() + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .externalPluginsEnabled(true) + .continueOnPluginError(true) + .build(); + @BeforeAll public static void createFakePluginDir() throws IOException { if (System.getProperty("besu.plugins.dir") == null) { @@ -53,7 +76,7 @@ public static void createFakePluginDir() throws IOException { @AfterEach public void clearTestPluginState() { - System.clearProperty("testPicoCLIPlugin.testOption"); + System.clearProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION); } @BeforeEach @@ -64,31 +87,31 @@ void setup() { @Test public void verifyEverythingGoesSmoothly() { assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); final Optional testPluginOptional = findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); contextImpl.beforeExternalServices(); contextImpl.startPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("started"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(STARTED); contextImpl.stopPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("stopped"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(STOPPED); } @Test public void registrationErrorsHandledSmoothly() { - System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER"); + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_REGISTER); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.beforeExternalServices(); @@ -103,11 +126,11 @@ public void registrationErrorsHandledSmoothly() { @Test public void startErrorsHandledSmoothly() { - System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART"); + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_START); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -116,11 +139,11 @@ public void startErrorsHandledSmoothly() { findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); contextImpl.beforeExternalServices(); contextImpl.startPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("failstart"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(FAIL_START_STATE); assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.stopPlugins(); @@ -129,11 +152,11 @@ public void startErrorsHandledSmoothly() { @Test public void stopErrorsHandledSmoothly() { - System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP"); + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_STOP); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -142,22 +165,20 @@ public void stopErrorsHandledSmoothly() { findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); contextImpl.beforeExternalServices(); contextImpl.startPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("started"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(STARTED); contextImpl.stopPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("failstop"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(FAIL_STOP_STATE); } @Test public void lifecycleExceptions() throws Throwable { - final ThrowableAssert.ThrowingCallable registerPlugins = - () -> - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + final ThrowableAssert.ThrowingCallable registerPlugins = () -> contextImpl.registerPlugins(); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::startPlugins); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins); @@ -179,30 +200,27 @@ public void lifecycleExceptions() throws Throwable { @Test public void shouldRegisterAllPluginsWhenNoPluginsOption() { - final PluginConfiguration config = - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build(); - assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(config); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); final Optional testPluginOptional = findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); } @Test public void shouldRegisterOnlySpecifiedPluginWhenPluginsOptionIsSet() { - final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin"); - assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(config); + contextImpl.initialize(createConfigurationForSpecificPlugin(TEST_PICO_CLI_PLUGIN)); + contextImpl.registerPlugins(); final Optional requestedPlugin = findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(requestedPlugin).isPresent(); - assertThat(requestedPlugin.get().getState()).isEqualTo("registered"); + assertThat(requestedPlugin.get().getState()).isEqualTo(REGISTERED); final Optional nonRequestedPlugin = findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class); @@ -212,9 +230,9 @@ public void shouldRegisterOnlySpecifiedPluginWhenPluginsOptionIsSet() { @Test public void shouldNotRegisterUnspecifiedPluginsWhenPluginsOptionIsSet() { - final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(config); + contextImpl.initialize(createConfigurationForSpecificPlugin(TEST_PICO_CLI_PLUGIN)); + contextImpl.registerPlugins(); final Optional nonRequestedPlugin = findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class); @@ -223,13 +241,12 @@ public void shouldNotRegisterUnspecifiedPluginsWhenPluginsOptionIsSet() { @Test void shouldThrowExceptionIfExplicitlySpecifiedPluginNotFound() { - PluginConfiguration config = createConfigurationForSpecificPlugin("NonExistentPlugin"); - + contextImpl.initialize(createConfigurationForSpecificPlugin(NON_EXISTENT_PLUGIN)); String exceptionMessage = - assertThrows(NoSuchElementException.class, () -> contextImpl.registerPlugins(config)) + assertThrows(NoSuchElementException.class, () -> contextImpl.registerPlugins()) .getMessage(); final String expectedMessage = - "The following requested plugins were not found: NonExistentPlugin"; + "The following requested plugins were not found: " + NON_EXISTENT_PLUGIN; assertThat(exceptionMessage).isEqualTo(expectedMessage); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); } @@ -241,19 +258,180 @@ void shouldNotRegisterAnyPluginsIfExternalPluginsDisabled() { .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) .externalPluginsEnabled(false) .build(); - contextImpl.registerPlugins(config); + contextImpl.initialize(config); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isTrue(); } @Test void shouldRegisterPluginsIfExternalPluginsEnabled() { + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); + assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isFalse(); + } + + @Test + void shouldHaltOnRegisterErrorWhenFlagIsFalse() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_REGISTER); + PluginConfiguration config = PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) - .externalPluginsEnabled(true) + .continueOnPluginError(false) .build(); - contextImpl.registerPlugins(config); - assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isFalse(); + + contextImpl.initialize(config); + + String errorMessage = + String.format("Error registering plugin of type %s", TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.registerPlugins()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnRegisterErrorWhenFlagIsTrue() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_REGISTER); + + PluginConfiguration config = + PluginConfiguration.builder() + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + } + + @Test + void shouldHaltOnBeforeExternalServicesErrorWhenFlagIsFalse() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_BEFORE_EXTERNAL_SERVICES); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(false) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + + String errorMessage = + String.format( + "Error calling `beforeExternalServices` on plugin of type %s", + TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.beforeExternalServices()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnBeforeExternalServicesErrorWhenFlagIsTrue() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_BEFORE_EXTERNAL_SERVICES); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + } + + @Test + void shouldHaltOnBeforeMainLoopErrorWhenFlagIsFalse() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_START); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(false) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + + String errorMessage = + String.format("Error starting plugin of type %s", TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.startPlugins()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnBeforeMainLoopErrorWhenFlagIsTrue() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_BEFORE_MAIN_LOOP); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + contextImpl.startPlugins(); + + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + } + + @Test + void shouldHaltOnAfterExternalServicePostMainLoopErrorWhenFlagIsFalse() { + System.setProperty( + TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(false) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + contextImpl.startPlugins(); + + String errorMessage = + String.format( + "Error calling `afterExternalServicePostMainLoop` on plugin of type %s", + TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.afterExternalServicesMainLoop()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnAfterExternalServicePostMainLoopErrorWhenFlagIsTrue() { + System.setProperty( + TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + contextImpl.startPlugins(); + contextImpl.afterExternalServicesMainLoop(); + + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); } private PluginConfiguration createConfigurationForSpecificPlugin(final String pluginName) { 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 79879538bd5..e93518cad43 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -118,7 +118,6 @@ import org.hyperledger.besu.ethereum.core.MiningParametersMetrics; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.core.VersionMetadata; -import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; @@ -1080,9 +1079,8 @@ private IExecutionStrategy createExecuteTask(final IExecutionStrategy nextStep) private IExecutionStrategy createPluginRegistrationTask(final IExecutionStrategy nextStep) { return parseResult -> { - PluginConfiguration configuration = - PluginsConfigurationOptions.fromCommandLine(parseResult.commandSpec().commandLine()); - besuPluginContext.registerPlugins(configuration); + besuPluginContext.initialize(PluginsConfigurationOptions.fromCommandLine(commandLine)); + besuPluginContext.registerPlugins(); commandLine.setExecutionStrategy(nextStep); return commandLine.execute(parseResult.originalArgs().toArray(new String[0])); }; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java index ba05f455246..8f83c71a787 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java @@ -128,6 +128,9 @@ public interface DefaultCommandValues { /** The constant DEFAULT_PLUGINS_OPTION_NAME. */ String DEFAULT_PLUGINS_OPTION_NAME = "--plugins"; + /** The constant DEFAULT_CONTINUE_ON_PLUGIN_ERROR_OPTION_NAME. */ + String DEFAULT_CONTINUE_ON_PLUGIN_ERROR_OPTION_NAME = "--plugin-continue-on-error"; + /** The constant DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME. */ String DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME = "--Xplugins-external-enabled"; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index 47df831c577..25893fff895 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.cli.options.stable; +import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_CONTINUE_ON_PLUGIN_ERROR_OPTION_NAME; import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME; import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_OPTION_NAME; @@ -27,7 +28,7 @@ import picocli.CommandLine; -/** The Plugins Options options. */ +/** The Plugins options. */ public class PluginsConfigurationOptions implements CLIOptions { @CommandLine.Option( @@ -44,9 +45,17 @@ public class PluginsConfigurationOptions implements CLIOptions