From 6e0a8748c9ecf103b2dff69fbdff8541cb90fca9 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Thu, 12 Nov 2020 16:57:13 +1300 Subject: [PATCH] Add Quorum Permissioning gate to Account and Node permissioning (#1545) * Create Quorum QIP-714 gate Signed-off-by: Lucas Saldanha --- .../node/configuration/BesuNodeFactory.java | 2 +- .../PermissionedNodeBuilder.java | 2 +- ...tPermissioningIbftStallAcceptanceTest.java | 4 +- .../org/hyperledger/besu/RunnerBuilder.java | 18 ++- .../org/hyperledger/besu/cli/BesuCommand.java | 27 ++++- .../besu/config/JsonGenesisConfigOptions.java | 2 +- .../besu/config/GenesisConfigOptionsTest.java | 17 +++ .../config/JsonGenesisConfigOptionsTest.java | 53 --------- .../NodePermissioningControllerFactory.java | 21 +++- .../PermissioningConfiguration.java | 12 +- .../QuorumPermissioningConfiguration.java | 44 +++++++ .../permissioning/QuorumQip714Gate.java | 72 ++++++++++++ .../AccountPermissioningController.java | 14 ++- ...AccountPermissioningControllerFactory.java | 22 +++- .../node/NodePermissioningController.java | 14 ++- .../permissioning/QuorumQip714GateTest.java | 109 ++++++++++++++++++ ...untPermissioningControllerFactoryTest.java | 35 +++--- .../AccountPermissioningControllerTest.java | 55 ++++++++- ...odePermissioningControllerFactoryTest.java | 46 +++++--- .../node/NodePermissioningControllerTest.java | 62 +++++++++- 20 files changed, 522 insertions(+), 109 deletions(-) create mode 100644 ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumPermissioningConfiguration.java create mode 100644 ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714Gate.java create mode 100644 ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714GateTest.java 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 a98c7f374c..7f0e837b67 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 @@ -299,7 +299,7 @@ public class BesuNodeFactory { config.setAccountAllowlist(accountAllowList); config.setAccountPermissioningConfigFilePath(configFile.getAbsolutePath()); final PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(Optional.of(config), Optional.empty()); + new PermissioningConfiguration(Optional.of(config), Optional.empty(), Optional.empty()); return create( new BesuNodeConfigurationBuilder() .name(name) diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/permissioning/PermissionedNodeBuilder.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/permissioning/PermissionedNodeBuilder.java index d1a4403cde..2e81875660 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/permissioning/PermissionedNodeBuilder.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/permissioning/PermissionedNodeBuilder.java @@ -186,7 +186,7 @@ public class PermissionedNodeBuilder { } final PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(localPermConfig, smartContractPermConfig); + new PermissioningConfiguration(localPermConfig, smartContractPermConfig, Optional.empty()); final BesuNodeConfigurationBuilder builder = new BesuNodeConfigurationBuilder(); builder diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningIbftStallAcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningIbftStallAcceptanceTest.java index 977130f3d8..851ab40667 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningIbftStallAcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningIbftStallAcceptanceTest.java @@ -61,7 +61,9 @@ public class NodeSmartContractPermissioningIbftStallAcceptanceTest Address.fromHexString(CONTRACT_ADDRESS)); final PermissioningConfiguration permissioningConfiguration = new PermissioningConfiguration( - Optional.empty(), Optional.of(smartContractPermissioningConfiguration)); + Optional.empty(), + Optional.of(smartContractPermissioningConfiguration), + Optional.empty()); // Set permissioning configurations on nodes bootnode.setPermissioningConfiguration(permissioningConfiguration); diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 0bb216c743..b4f28667d5 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -388,7 +388,7 @@ public class RunnerBuilder { final Bytes localNodeId = nodeKey.getPublicKey().getEncodedBytes(); final Optional nodePermissioningController = buildNodePermissioningController( - bootnodes, synchronizer, transactionSimulator, localNodeId); + bootnodes, synchronizer, transactionSimulator, localNodeId, context.getBlockchain()); final PeerPermissions peerPermissions = nodePermissioningController @@ -474,7 +474,10 @@ public class RunnerBuilder { final Optional accountPermissioningController = buildAccountPermissioningController( - permissioningConfiguration, besuController, transactionSimulator); + permissioningConfiguration, + besuController, + transactionSimulator, + context.getBlockchain()); final Optional accountLocalConfigPermissioningController = @@ -645,7 +648,8 @@ public class RunnerBuilder { final List bootnodesAsEnodeURLs, final Synchronizer synchronizer, final TransactionSimulator transactionSimulator, - final Bytes localNodeId) { + final Bytes localNodeId, + final Blockchain blockchain) { final Collection fixedNodes = getFixedNodes(bootnodesAsEnodeURLs, staticNodes); if (permissioningConfiguration.isPresent()) { @@ -658,7 +662,8 @@ public class RunnerBuilder { fixedNodes, localNodeId, transactionSimulator, - metricsSystem); + metricsSystem, + blockchain); return Optional.of(nodePermissioningController); } else { @@ -669,12 +674,13 @@ public class RunnerBuilder { private Optional buildAccountPermissioningController( final Optional permissioningConfiguration, final BesuController besuController, - final TransactionSimulator transactionSimulator) { + final TransactionSimulator transactionSimulator, + final Blockchain blockchain) { if (permissioningConfiguration.isPresent()) { final Optional accountPermissioningController = AccountPermissioningControllerFactory.create( - permissioningConfiguration.get(), transactionSimulator, metricsSystem); + permissioningConfiguration.get(), transactionSimulator, metricsSystem, blockchain); accountPermissioningController.ifPresent( permissioningController -> 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 f131255575..43d4d66ea1 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -26,6 +26,7 @@ import static org.hyperledger.besu.ethereum.api.graphql.GraphQLConfiguration.DEF import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration.DEFAULT_JSON_RPC_PORT; import static org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis.DEFAULT_JSON_RPC_APIS; import static org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration.DEFAULT_WEBSOCKET_PORT; +import static org.hyperledger.besu.ethereum.permissioning.QuorumPermissioningConfiguration.QIP714_DEFAULT_BLOCK; import static org.hyperledger.besu.metrics.BesuMetricCategory.DEFAULT_METRIC_CATEGORIES; import static org.hyperledger.besu.metrics.prometheus.MetricsConfiguration.DEFAULT_METRICS_PORT; import static org.hyperledger.besu.metrics.prometheus.MetricsConfiguration.DEFAULT_METRICS_PUSH_PORT; @@ -70,6 +71,7 @@ import org.hyperledger.besu.cli.util.CommandLineUtils; import org.hyperledger.besu.cli.util.ConfigOptionSearchAndRunHandler; import org.hyperledger.besu.cli.util.VersionProvider; import org.hyperledger.besu.config.GenesisConfigFile; +import org.hyperledger.besu.config.GenesisConfigOptions; import org.hyperledger.besu.config.experimental.ExperimentalEIPs; import org.hyperledger.besu.controller.BesuController; import org.hyperledger.besu.controller.BesuControllerBuilder; @@ -105,6 +107,7 @@ import org.hyperledger.besu.ethereum.p2p.peers.StaticNodesParser; import org.hyperledger.besu.ethereum.permissioning.LocalPermissioningConfiguration; import org.hyperledger.besu.ethereum.permissioning.PermissioningConfiguration; import org.hyperledger.besu.ethereum.permissioning.PermissioningConfigurationBuilder; +import org.hyperledger.besu.ethereum.permissioning.QuorumPermissioningConfiguration; import org.hyperledger.besu.ethereum.permissioning.SmartContractPermissioningConfiguration; import org.hyperledger.besu.ethereum.privacy.storage.keyvalue.PrivacyKeyValueStorageProvider; import org.hyperledger.besu.ethereum.privacy.storage.keyvalue.PrivacyKeyValueStorageProviderBuilder; @@ -159,6 +162,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.OptionalLong; import java.util.Set; import java.util.TreeMap; import java.util.function.Function; @@ -1782,6 +1786,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { final SmartContractPermissioningConfiguration smartContractPermissioningConfiguration = SmartContractPermissioningConfiguration.createDefault(); + if (permissionsNodesContractEnabled) { if (permissionsNodesContractAddress == null) { throw new ParameterException( @@ -1821,11 +1826,31 @@ public class BesuCommand implements DefaultCommandValues, Runnable { final PermissioningConfiguration permissioningConfiguration = new PermissioningConfiguration( localPermissioningConfigurationOptional, - Optional.of(smartContractPermissioningConfiguration)); + Optional.of(smartContractPermissioningConfiguration), + Optional.of(quorumPermissioningConfig())); return Optional.of(permissioningConfiguration); } + private QuorumPermissioningConfiguration quorumPermissioningConfig() { + final GenesisConfigOptions genesisConfigOptions; + try { + final GenesisConfigFile genesisConfigFile = GenesisConfigFile.fromConfig(genesisConfig()); + genesisConfigOptions = genesisConfigFile.getConfigOptions(genesisConfigOverrides); + } catch (Exception e) { + return QuorumPermissioningConfiguration.disabled(); + } + + final boolean isQuorumMode = genesisConfigOptions.isQuorum(); + final OptionalLong qip714BlockNumber = genesisConfigOptions.getQip714BlockNumber(); + if (isQuorumMode) { + return QuorumPermissioningConfiguration.enabled( + qip714BlockNumber.orElse(QIP714_DEFAULT_BLOCK)); + } else { + return QuorumPermissioningConfiguration.disabled(); + } + } + private boolean localPermissionsEnabled() { return permissionsAccountsEnabled || permissionsNodesEnabled; } 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 260aaca908..98e5aa4e99 100644 --- a/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java +++ b/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java @@ -302,7 +302,7 @@ public class JsonGenesisConfigOptions implements GenesisConfigOptions { @Override public boolean isQuorum() { - return getOptionalBoolean("isQuorum").orElse(false); + return getOptionalBoolean("isquorum").orElse(false); } @Override 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 4b9596dbc2..b930d54f51 100644 --- a/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java +++ b/config/src/test/java/org/hyperledger/besu/config/GenesisConfigOptionsTest.java @@ -226,6 +226,23 @@ public class GenesisConfigOptionsTest { assertThat(config.getHomesteadBlockNumber()).isEmpty(); } + @Test + public void isQuorumShouldDefaultToFalse() { + final GenesisConfigOptions config = GenesisConfigFile.fromConfig("{}").getConfigOptions(); + + assertThat(config.isQuorum()).isFalse(); + assertThat(config.getQip714BlockNumber()).isEmpty(); + } + + @Test + public void isQuorumConfigParsedCorrectly() { + final GenesisConfigOptions config = + fromConfigOptions(Map.of("isQuorum", true, "qip714block", 99999L)); + + assertThat(config.isQuorum()).isTrue(); + assertThat(config.getQip714BlockNumber()).hasValue(99999L); + } + private GenesisConfigOptions fromConfigOptions(final Map configOptions) { final ObjectNode rootNode = JsonUtil.createEmptyObjectNode(); final ObjectNode options = JsonUtil.objectNodeFromMap(configOptions); diff --git a/config/src/test/java/org/hyperledger/besu/config/JsonGenesisConfigOptionsTest.java b/config/src/test/java/org/hyperledger/besu/config/JsonGenesisConfigOptionsTest.java index bb2d0a8b42..da96485923 100644 --- a/config/src/test/java/org/hyperledger/besu/config/JsonGenesisConfigOptionsTest.java +++ b/config/src/test/java/org/hyperledger/besu/config/JsonGenesisConfigOptionsTest.java @@ -18,7 +18,6 @@ import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Map; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -165,56 +164,4 @@ public class JsonGenesisConfigOptionsTest { assertThat(configOptions.getIbft2ConfigOptions().getBlockRewardWei()).isEqualTo(12); } - - @Test - public void isQuorumShouldDefaultToFalse() { - final ObjectNode configNode = loadCompleteDataSet(); - - final JsonGenesisConfigOptions configOptions = - JsonGenesisConfigOptions.fromJsonObject(configNode); - - assertThat(configOptions.isQuorum()).isFalse(); - assertThat(configOptions.getQip714BlockNumber()).isEmpty(); - } - - @Test - public void isQuorumConfigParsedCorrectly() { - final ObjectNode configNode = loadGenesisWithQuorumConfig(); - - final JsonGenesisConfigOptions configOptions = - JsonGenesisConfigOptions.fromJsonObject(configNode); - - assertThat(configOptions.isQuorum()).isTrue(); - assertThat(configOptions.getQip714BlockNumber()).hasValue(99999L); - } - - @Test - public void whenDisabledQuorumOptionsAreNotAddedToMap() { - final ObjectNode configNode = loadCompleteDataSet(); - - final Map map = JsonGenesisConfigOptions.fromJsonObject(configNode).asMap(); - - assertThat(map).doesNotContainKey("isQuorum").doesNotContainKey("qip714block"); - } - - @Test - public void whenEnabledQuorumOptionsAreAddedToMap() { - final ObjectNode configNode = loadGenesisWithQuorumConfig(); - - final Map map = JsonGenesisConfigOptions.fromJsonObject(configNode).asMap(); - - assertThat(map).containsEntry("isQuorum", true).containsEntry("qip714block", 99999L); - } - - private ObjectNode loadGenesisWithQuorumConfig() { - try { - final String configText = - Resources.toString( - Resources.getResource("valid_config_with_quorum_config.json"), - StandardCharsets.UTF_8); - return JsonUtil.objectNodeFromString(configText); - } catch (final IOException e) { - throw new RuntimeException("Failed to load resource", e); - } - } } diff --git a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/NodePermissioningControllerFactory.java b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/NodePermissioningControllerFactory.java index 02bfd8c8ed..4f5b8db74b 100644 --- a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/NodePermissioningControllerFactory.java +++ b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/NodePermissioningControllerFactory.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.ethereum.permissioning; +import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURL; @@ -42,7 +43,8 @@ public class NodePermissioningControllerFactory { final Collection fixedNodes, final Bytes localNodeId, final TransactionSimulator transactionSimulator, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final Blockchain blockchain) { final Optional syncStatusProviderOptional; @@ -81,8 +83,21 @@ public class NodePermissioningControllerFactory { syncStatusProviderOptional = Optional.empty(); } - NodePermissioningController nodePermissioningController = - new NodePermissioningController(syncStatusProviderOptional, providers); + final Optional quorumQip714Gate = + permissioningConfiguration + .getQuorumPermissioningConfig() + .flatMap( + config -> { + if (config.isEnabled()) { + return Optional.of( + QuorumQip714Gate.getInstance(config.getQip714Block(), blockchain)); + } else { + return Optional.empty(); + } + }); + + final NodePermissioningController nodePermissioningController = + new NodePermissioningController(syncStatusProviderOptional, providers, quorumQip714Gate); permissioningConfiguration .getSmartContractConfig() diff --git a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/PermissioningConfiguration.java b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/PermissioningConfiguration.java index e08ec6edbd..63c6b9de95 100644 --- a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/PermissioningConfiguration.java +++ b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/PermissioningConfiguration.java @@ -17,14 +17,18 @@ package org.hyperledger.besu.ethereum.permissioning; import java.util.Optional; public class PermissioningConfiguration { + private final Optional localConfig; private final Optional smartContractConfig; + private final Optional quorumPermissioningConfig; public PermissioningConfiguration( final Optional localConfig, - final Optional smartContractConfig) { + final Optional smartContractConfig, + final Optional quorumPermissioningConfig) { this.localConfig = localConfig; this.smartContractConfig = smartContractConfig; + this.quorumPermissioningConfig = quorumPermissioningConfig; } public Optional getLocalConfig() { @@ -35,7 +39,11 @@ public class PermissioningConfiguration { return smartContractConfig; } + public Optional getQuorumPermissioningConfig() { + return quorumPermissioningConfig; + } + public static PermissioningConfiguration createDefault() { - return new PermissioningConfiguration(Optional.empty(), Optional.empty()); + return new PermissioningConfiguration(Optional.empty(), Optional.empty(), Optional.empty()); } } diff --git a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumPermissioningConfiguration.java b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumPermissioningConfiguration.java new file mode 100644 index 0000000000..fcc1359113 --- /dev/null +++ b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumPermissioningConfiguration.java @@ -0,0 +1,44 @@ +/* + * Copyright ConsenSys AG. + * + * 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.permissioning; + +public class QuorumPermissioningConfiguration { + + public static final long QIP714_DEFAULT_BLOCK = 0; + + private final long qip714Block; + private final boolean enabled; + + public QuorumPermissioningConfiguration(final long qip714Block, final boolean enabled) { + this.qip714Block = qip714Block; + this.enabled = enabled; + } + + public static QuorumPermissioningConfiguration enabled(final long qip714Block) { + return new QuorumPermissioningConfiguration(qip714Block, true); + } + + public static QuorumPermissioningConfiguration disabled() { + return new QuorumPermissioningConfiguration(QIP714_DEFAULT_BLOCK, false); + } + + public long getQip714Block() { + return qip714Block; + } + + public boolean isEnabled() { + return enabled; + } +} diff --git a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714Gate.java b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714Gate.java new file mode 100644 index 0000000000..fbf9dc7a67 --- /dev/null +++ b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714Gate.java @@ -0,0 +1,72 @@ +/* + * Copyright ConsenSys AG. + * + * 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.permissioning; + +import org.hyperledger.besu.ethereum.chain.BlockAddedEvent; +import org.hyperledger.besu.ethereum.chain.Blockchain; + +import java.util.concurrent.atomic.AtomicLong; + +import com.google.common.annotations.VisibleForTesting; + +public class QuorumQip714Gate { + + private static QuorumQip714Gate SINGLE_INSTANCE = null; + + private final long qip714Block; + private final AtomicLong latestBlock = new AtomicLong(0L); + + @VisibleForTesting + QuorumQip714Gate(final long qip714Block, final Blockchain blockchain) { + this.qip714Block = qip714Block; + + blockchain.observeBlockAdded(this::checkChainHeight); + } + + // this is only called during start-up, synchronized access won't hurt performance + public static synchronized QuorumQip714Gate getInstance( + final long qip714Block, final Blockchain blockchain) { + if (SINGLE_INSTANCE == null) { + SINGLE_INSTANCE = new QuorumQip714Gate(qip714Block, blockchain); + } else { + if (SINGLE_INSTANCE.qip714Block != qip714Block) { + throw new IllegalStateException( + "Tried to create Quorum QIP-714 gate with different block config from already instantiated gate block config"); + } + } + + return SINGLE_INSTANCE; + } + + public boolean shouldCheckPermissions() { + if (qip714Block == 0) { + return true; + } else { + return latestBlock.get() >= qip714Block; + } + } + + @VisibleForTesting + void checkChainHeight(final BlockAddedEvent event) { + if (event.isNewCanonicalHead()) { + latestBlock.set(event.getBlock().getHeader().getNumber()); + } + } + + @VisibleForTesting + long getLatestBlock() { + return latestBlock.get(); + } +} diff --git a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningController.java b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningController.java index 9ab7bb531c..f2df92aaa9 100644 --- a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningController.java +++ b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningController.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.core.Hash; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.permissioning.AccountLocalConfigPermissioningController; +import org.hyperledger.besu.ethereum.permissioning.QuorumQip714Gate; import org.hyperledger.besu.ethereum.permissioning.TransactionSmartContractPermissioningController; import java.util.Optional; @@ -34,21 +35,32 @@ public class AccountPermissioningController { accountLocalConfigPermissioningController; private final Optional transactionSmartContractPermissioningController; + private final Optional quorumQip714Gate; public AccountPermissioningController( final Optional accountLocalConfigPermissioningController, final Optional - transactionSmartContractPermissioningController) { + transactionSmartContractPermissioningController, + final Optional quorumQip714Gate) { this.accountLocalConfigPermissioningController = accountLocalConfigPermissioningController; this.transactionSmartContractPermissioningController = transactionSmartContractPermissioningController; + this.quorumQip714Gate = quorumQip714Gate; } public boolean isPermitted( final Transaction transaction, final boolean includeLocalCheck, final boolean includeOnChainCheck) { + final boolean checkPermissions = + quorumQip714Gate.map(QuorumQip714Gate::shouldCheckPermissions).orElse(true); + if (!checkPermissions) { + LOG.trace("Skipping account permissioning check due to qip714block config"); + + return true; + } + final Hash transactionHash = transaction.getHash(); final Address sender = transaction.getSender(); diff --git a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactory.java b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactory.java index e45f74c282..7d4e137634 100644 --- a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactory.java +++ b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactory.java @@ -15,12 +15,14 @@ package org.hyperledger.besu.ethereum.permissioning.account; import org.hyperledger.besu.crypto.SECP256K1; +import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.Wei; import org.hyperledger.besu.ethereum.permissioning.AccountLocalConfigPermissioningController; import org.hyperledger.besu.ethereum.permissioning.LocalPermissioningConfiguration; import org.hyperledger.besu.ethereum.permissioning.PermissioningConfiguration; +import org.hyperledger.besu.ethereum.permissioning.QuorumQip714Gate; import org.hyperledger.besu.ethereum.permissioning.SmartContractPermissioningConfiguration; import org.hyperledger.besu.ethereum.permissioning.TransactionSmartContractPermissioningController; import org.hyperledger.besu.ethereum.transaction.TransactionSimulator; @@ -39,7 +41,8 @@ public class AccountPermissioningControllerFactory { public static Optional create( final PermissioningConfiguration permissioningConfiguration, final TransactionSimulator transactionSimulator, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final Blockchain blockchain) { if (permissioningConfiguration == null) { return Optional.empty(); @@ -56,10 +59,25 @@ public class AccountPermissioningControllerFactory { if (accountLocalConfigPermissioningController.isPresent() || transactionSmartContractPermissioningController.isPresent()) { + + final Optional quorumQip714Gate = + permissioningConfiguration + .getQuorumPermissioningConfig() + .flatMap( + config -> { + if (config.isEnabled()) { + return Optional.of( + QuorumQip714Gate.getInstance(config.getQip714Block(), blockchain)); + } else { + return Optional.empty(); + } + }); + final AccountPermissioningController controller = new AccountPermissioningController( accountLocalConfigPermissioningController, - transactionSmartContractPermissioningController); + transactionSmartContractPermissioningController, + quorumQip714Gate); return Optional.of(controller); } else { diff --git a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningController.java b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningController.java index fb95b7431e..2a4ebf7845 100644 --- a/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningController.java +++ b/ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningController.java @@ -16,6 +16,7 @@ package org.hyperledger.besu.ethereum.permissioning.node; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURL; import org.hyperledger.besu.ethereum.permissioning.NodeLocalConfigPermissioningController; +import org.hyperledger.besu.ethereum.permissioning.QuorumQip714Gate; import org.hyperledger.besu.ethereum.permissioning.node.provider.SyncStatusNodePermissioningProvider; import org.hyperledger.besu.util.Subscribers; @@ -33,16 +34,27 @@ public class NodePermissioningController { private Optional insufficientPeersPermissioningProvider = Optional.empty(); private final List providers; + private final Optional quorumQip714Gate; private final Subscribers permissioningUpdateSubscribers = Subscribers.create(); public NodePermissioningController( final Optional syncStatusNodePermissioningProvider, - final List providers) { + final List providers, + final Optional quorumQip714Gate) { this.providers = providers; this.syncStatusNodePermissioningProvider = syncStatusNodePermissioningProvider; + this.quorumQip714Gate = quorumQip714Gate; } public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinationEnode) { + final boolean checkPermissions = + quorumQip714Gate.map(QuorumQip714Gate::shouldCheckPermissions).orElse(true); + if (!checkPermissions) { + LOG.trace("Skipping node permissioning check due to qip714block config"); + + return true; + } + LOG.trace("Node permissioning: Checking {} -> {}", sourceEnode, destinationEnode); if (syncStatusNodePermissioningProvider diff --git a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714GateTest.java b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714GateTest.java new file mode 100644 index 0000000000..6e8155ff3e --- /dev/null +++ b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/QuorumQip714GateTest.java @@ -0,0 +1,109 @@ +package org.hyperledger.besu.ethereum.permissioning; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import org.hyperledger.besu.ethereum.chain.BlockAddedEvent; +import org.hyperledger.besu.ethereum.chain.Blockchain; +import org.hyperledger.besu.ethereum.core.Block; +import org.hyperledger.besu.ethereum.core.BlockDataGenerator; +import org.hyperledger.besu.ethereum.core.BlockDataGenerator.BlockOptions; + +import java.util.Collections; + +import org.junit.Before; +import org.junit.Test; + +/* + * Copyright ConsenSys AG. + * + * 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 + */ +public class QuorumQip714GateTest { + + private Blockchain blockchain; + private QuorumQip714Gate gate; + + @Before + public void before() { + blockchain = mock(Blockchain.class); + } + + @Test + public void gateShouldSubscribeAsBlockAddedObserver() { + gate = new QuorumQip714Gate(100, blockchain); + + verify(blockchain).observeBlockAdded(any()); + } + + @Test + public void whenTargetBlockIsZeroCheckPermissionsReturnTrue() { + gate = new QuorumQip714Gate(0, blockchain); + + assertThat(gate.shouldCheckPermissions()).isTrue(); + } + + @Test + public void whenBelowTargetBlockCheckPermissionsReturnFalse() { + gate = new QuorumQip714Gate(99, blockchain); + + updateChainHead(55); + + assertThat(gate.shouldCheckPermissions()).isFalse(); + } + + @Test + public void whenAboveTargetBlockCheckPermissionsReturnTrue() { + gate = new QuorumQip714Gate(99, blockchain); + + updateChainHead(100); + + assertThat(gate.shouldCheckPermissions()).isTrue(); + } + + @Test + public void latestBlockCheckShouldKeepUpToChainHeight() { + gate = new QuorumQip714Gate(0, blockchain); + assertThat(gate.getLatestBlock()).isEqualTo(0); + + updateChainHead(1); + assertThat(gate.getLatestBlock()).isEqualTo(1); + + updateChainHead(3); + assertThat(gate.getLatestBlock()).isEqualTo(3); + + updateChainHead(2); + assertThat(gate.getLatestBlock()).isEqualTo(2); + } + + @Test + public void getInstanceForbidInstancesWithDifferentQip714BlockNumber() { + // creating singleton with qip714block = 1 + QuorumQip714Gate.getInstance(1L, blockchain); + + // creating new instance with qip714block != 1 should fail + assertThatThrownBy(() -> QuorumQip714Gate.getInstance(2L, blockchain)) + .isInstanceOf(IllegalStateException.class) + .hasMessage( + "Tried to create Quorum QIP-714 gate with different block config from already instantiated gate block config"); + } + + private void updateChainHead(final int height) { + final Block block = new BlockDataGenerator().block(new BlockOptions().setBlockNumber(height)); + gate.checkChainHeight( + BlockAddedEvent.createForHeadAdvancement( + block, Collections.emptyList(), Collections.emptyList())); + } +} diff --git a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactoryTest.java b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactoryTest.java index 3d31fc6596..7c862e3cad 100644 --- a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactoryTest.java +++ b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerFactoryTest.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.ThrowableAssert.catchThrowable; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.permissioning.LocalPermissioningConfiguration; import org.hyperledger.besu.ethereum.permissioning.PermissioningConfiguration; @@ -43,13 +44,15 @@ import org.mockito.junit.MockitoJUnitRunner; public class AccountPermissioningControllerFactoryTest { @Mock private TransactionSimulator transactionSimulator; + @Mock private Blockchain blockchain; private final MetricsSystem metricsSystem = new NoOpMetricsSystem(); @Test public void createWithNullPermissioningConfigShouldReturnEmpty() { Optional controller = - AccountPermissioningControllerFactory.create(null, transactionSimulator, metricsSystem); + AccountPermissioningControllerFactory.create( + null, transactionSimulator, metricsSystem, blockchain); Assertions.assertThat(controller).isEmpty(); } @@ -60,11 +63,12 @@ public class AccountPermissioningControllerFactoryTest { assertThat(localConfig.isAccountAllowlistEnabled()).isFalse(); PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(Optional.of(localConfig), Optional.empty()); + new PermissioningConfiguration( + Optional.of(localConfig), Optional.empty(), Optional.empty()); Optional controller = AccountPermissioningControllerFactory.create( - permissioningConfiguration, transactionSimulator, metricsSystem); + permissioningConfiguration, transactionSimulator, metricsSystem, blockchain); Assertions.assertThat(controller).isEmpty(); } @@ -75,11 +79,12 @@ public class AccountPermissioningControllerFactoryTest { assertThat(localConfig.isAccountAllowlistEnabled()).isTrue(); PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(Optional.of(localConfig), Optional.empty()); + new PermissioningConfiguration( + Optional.of(localConfig), Optional.empty(), Optional.empty()); Optional controller = AccountPermissioningControllerFactory.create( - permissioningConfiguration, transactionSimulator, metricsSystem); + permissioningConfiguration, transactionSimulator, metricsSystem, blockchain); Assertions.assertThat(controller).isNotEmpty(); assertThat(controller.get().getAccountLocalConfigPermissioningController()).isNotEmpty(); @@ -93,11 +98,12 @@ public class AccountPermissioningControllerFactoryTest { assertThat(onchainConfig.isSmartContractAccountAllowlistEnabled()).isFalse(); PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(Optional.empty(), Optional.of(onchainConfig)); + new PermissioningConfiguration( + Optional.empty(), Optional.of(onchainConfig), Optional.empty()); Optional controller = AccountPermissioningControllerFactory.create( - permissioningConfiguration, transactionSimulator, metricsSystem); + permissioningConfiguration, transactionSimulator, metricsSystem, blockchain); Assertions.assertThat(controller).isEmpty(); } @@ -108,11 +114,12 @@ public class AccountPermissioningControllerFactoryTest { assertThat(onchainConfig.isSmartContractAccountAllowlistEnabled()).isTrue(); PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(Optional.empty(), Optional.of(onchainConfig)); + new PermissioningConfiguration( + Optional.empty(), Optional.of(onchainConfig), Optional.empty()); Optional controller = AccountPermissioningControllerFactory.create( - permissioningConfiguration, transactionSimulator, metricsSystem); + permissioningConfiguration, transactionSimulator, metricsSystem, blockchain); Assertions.assertThat(controller).isNotEmpty(); assertThat(controller.get().getAccountLocalConfigPermissioningController()).isEmpty(); @@ -125,7 +132,8 @@ public class AccountPermissioningControllerFactoryTest { assertThat(onchainConfig.isSmartContractAccountAllowlistEnabled()).isTrue(); PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(Optional.empty(), Optional.of(onchainConfig)); + new PermissioningConfiguration( + Optional.empty(), Optional.of(onchainConfig), Optional.empty()); when(transactionSimulator.processAtHead(any())).thenThrow(new RuntimeException()); @@ -133,7 +141,7 @@ public class AccountPermissioningControllerFactoryTest { catchThrowable( () -> AccountPermissioningControllerFactory.create( - permissioningConfiguration, transactionSimulator, metricsSystem)); + permissioningConfiguration, transactionSimulator, metricsSystem, blockchain)); assertThat(thrown) .isInstanceOf(IllegalStateException.class) @@ -149,11 +157,12 @@ public class AccountPermissioningControllerFactoryTest { assertThat(onchainConfig.isSmartContractAccountAllowlistEnabled()).isTrue(); PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration(Optional.of(localConfig), Optional.of(onchainConfig)); + new PermissioningConfiguration( + Optional.of(localConfig), Optional.of(onchainConfig), Optional.empty()); Optional controller = AccountPermissioningControllerFactory.create( - permissioningConfiguration, transactionSimulator, metricsSystem); + permissioningConfiguration, transactionSimulator, metricsSystem, blockchain); Assertions.assertThat(controller).isNotEmpty(); assertThat(controller.get().getAccountLocalConfigPermissioningController()).isNotEmpty(); diff --git a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerTest.java b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerTest.java index a89235d57e..8ef6123610 100644 --- a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerTest.java +++ b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/account/AccountPermissioningControllerTest.java @@ -18,11 +18,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.permissioning.AccountLocalConfigPermissioningController; +import org.hyperledger.besu.ethereum.permissioning.QuorumQip714Gate; import org.hyperledger.besu.ethereum.permissioning.TransactionSmartContractPermissioningController; import java.util.Optional; @@ -40,12 +41,15 @@ public class AccountPermissioningControllerTest { @Mock private AccountLocalConfigPermissioningController localConfigController; @Mock private TransactionSmartContractPermissioningController smartContractController; + @Mock private QuorumQip714Gate quorumQip714Gate; @Before public void before() { permissioningController = new AccountPermissioningController( - Optional.of(localConfigController), Optional.of(smartContractController)); + Optional.of(localConfigController), + Optional.of(smartContractController), + Optional.empty()); } @Test @@ -57,7 +61,7 @@ public class AccountPermissioningControllerTest { assertThat(isPermitted).isTrue(); verify(localConfigController).isPermitted(any()); - verifyZeroInteractions(smartContractController); + verifyNoInteractions(smartContractController); } @Test @@ -85,4 +89,49 @@ public class AccountPermissioningControllerTest { verify(localConfigController).isPermitted(any()); verify(smartContractController).isPermitted(any()); } + + @Test + public void whenQuorumQip714GateIsEmptyShouldDelegateToProviders() { + this.permissioningController = + new AccountPermissioningController( + Optional.of(localConfigController), + Optional.of(smartContractController), + Optional.empty()); + + permissioningController.isPermitted(mock(Transaction.class), true, false); + + verify(localConfigController).isPermitted(any()); + } + + @Test + public void whenQuorumQip714GateIsNotActiveShouldBypassProviders() { + this.permissioningController = + new AccountPermissioningController( + Optional.of(localConfigController), + Optional.of(smartContractController), + Optional.of(quorumQip714Gate)); + + when(quorumQip714Gate.shouldCheckPermissions()).thenReturn(false); + + boolean isPermitted = permissioningController.isPermitted(mock(Transaction.class), true, false); + assertThat(isPermitted).isTrue(); + + verifyNoInteractions(localConfigController); + verifyNoInteractions(smartContractController); + } + + @Test + public void whenQuorumQip714GateIsActiveActiveShouldDelegateToProviders() { + this.permissioningController = + new AccountPermissioningController( + Optional.of(localConfigController), + Optional.of(smartContractController), + Optional.of(quorumQip714Gate)); + + when(quorumQip714Gate.shouldCheckPermissions()).thenReturn(true); + + permissioningController.isPermitted(mock(Transaction.class), true, false); + + verify(localConfigController).isPermitted(any()); + } } diff --git a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java index 73f9da2112..7095d12716 100644 --- a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java +++ b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURL; @@ -47,6 +48,7 @@ public class NodePermissioningControllerFactoryTest { @Mock private Synchronizer synchronizer; @Mock private TransactionSimulator transactionSimulator; + @Mock private Blockchain blockchain; private final String enode = "enode://5f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:1111"; @@ -63,7 +65,7 @@ public class NodePermissioningControllerFactoryTest { @Test public void testCreateWithNeitherPermissioningEnabled() { - config = new PermissioningConfiguration(Optional.empty(), Optional.empty()); + config = new PermissioningConfiguration(Optional.empty(), Optional.empty(), Optional.empty()); NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = factory.create( @@ -72,7 +74,8 @@ public class NodePermissioningControllerFactoryTest { bootnodes, selfEnode.getNodeId(), transactionSimulator, - new NoOpMetricsSystem()); + new NoOpMetricsSystem(), + blockchain); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(0); @@ -87,7 +90,9 @@ public class NodePermissioningControllerFactoryTest { smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled(true); config = new PermissioningConfiguration( - Optional.empty(), Optional.of(smartContractPermissioningConfiguration)); + Optional.empty(), + Optional.of(smartContractPermissioningConfiguration), + Optional.empty()); NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = @@ -97,7 +102,8 @@ public class NodePermissioningControllerFactoryTest { bootnodes, selfEnode.getNodeId(), transactionSimulator, - new NoOpMetricsSystem()); + new NoOpMetricsSystem(), + blockchain); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(1); @@ -113,7 +119,8 @@ public class NodePermissioningControllerFactoryTest { localPermissioningConfig.setNodeAllowlist(Collections.emptyList()); localPermissioningConfig.setNodePermissioningConfigFilePath("fake-file-path"); config = - new PermissioningConfiguration(Optional.of(localPermissioningConfig), Optional.empty()); + new PermissioningConfiguration( + Optional.of(localPermissioningConfig), Optional.empty(), Optional.empty()); NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = @@ -123,7 +130,8 @@ public class NodePermissioningControllerFactoryTest { bootnodes, selfEnode.getNodeId(), transactionSimulator, - new NoOpMetricsSystem()); + new NoOpMetricsSystem(), + blockchain); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(1); @@ -147,7 +155,8 @@ public class NodePermissioningControllerFactoryTest { config = new PermissioningConfiguration( Optional.of(localPermissioningConfig), - Optional.of(smartContractPermissioningConfiguration)); + Optional.of(smartContractPermissioningConfiguration), + Optional.empty()); NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = @@ -157,7 +166,8 @@ public class NodePermissioningControllerFactoryTest { fixedNodes, selfEnode.getNodeId(), transactionSimulator, - new NoOpMetricsSystem()); + new NoOpMetricsSystem(), + blockchain); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(1); @@ -180,7 +190,8 @@ public class NodePermissioningControllerFactoryTest { config = new PermissioningConfiguration( Optional.of(localPermissioningConfig), - Optional.of(smartContractPermissioningConfiguration)); + Optional.of(smartContractPermissioningConfiguration), + Optional.empty()); NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = @@ -190,7 +201,8 @@ public class NodePermissioningControllerFactoryTest { bootnodes, selfEnode.getNodeId(), transactionSimulator, - new NoOpMetricsSystem()); + new NoOpMetricsSystem(), + blockchain); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(2); @@ -216,7 +228,9 @@ public class NodePermissioningControllerFactoryTest { smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled(true); config = new PermissioningConfiguration( - Optional.empty(), Optional.of(smartContractPermissioningConfiguration)); + Optional.empty(), + Optional.of(smartContractPermissioningConfiguration), + Optional.empty()); NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = @@ -226,7 +240,8 @@ public class NodePermissioningControllerFactoryTest { fixedNodes, selfEnode.getNodeId(), transactionSimulator, - new NoOpMetricsSystem()); + new NoOpMetricsSystem(), + blockchain); assertThat(controller.getSyncStatusNodePermissioningProvider()).isPresent(); } @@ -239,7 +254,9 @@ public class NodePermissioningControllerFactoryTest { smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled(true); config = new PermissioningConfiguration( - Optional.empty(), Optional.of(smartContractPermissioningConfiguration)); + Optional.empty(), + Optional.of(smartContractPermissioningConfiguration), + Optional.empty()); when(transactionSimulator.processAtHead(any())).thenThrow(new RuntimeException()); @@ -253,7 +270,8 @@ public class NodePermissioningControllerFactoryTest { bootnodes, selfEnode.getNodeId(), transactionSimulator, - new NoOpMetricsSystem())); + new NoOpMetricsSystem(), + blockchain)); assertThat(thrown) .isInstanceOf(IllegalStateException.class) diff --git a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerTest.java b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerTest.java index a6c6f5b7a7..a12f2f065e 100644 --- a/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerTest.java +++ b/ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerTest.java @@ -21,13 +21,16 @@ import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURL; import org.hyperledger.besu.ethereum.permissioning.NodeLocalConfigPermissioningController; +import org.hyperledger.besu.ethereum.permissioning.QuorumQip714Gate; import org.hyperledger.besu.ethereum.permissioning.node.provider.SyncStatusNodePermissioningProvider; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Optional; @@ -51,6 +54,7 @@ public class NodePermissioningControllerTest { Optional syncStatusNodePermissioningProviderOptional; @Mock private NodeLocalConfigPermissioningController localConfigNodePermissioningProvider; @Mock private NodePermissioningProvider otherPermissioningProvider; + @Mock private QuorumQip714Gate quorumQip714Gate; private NodePermissioningController controller; @@ -60,7 +64,7 @@ public class NodePermissioningControllerTest { List emptyProviders = new ArrayList<>(); this.controller = new NodePermissioningController( - syncStatusNodePermissioningProviderOptional, emptyProviders); + syncStatusNodePermissioningProviderOptional, emptyProviders, Optional.empty()); } @Test @@ -74,7 +78,8 @@ public class NodePermissioningControllerTest { public void whenNoSyncStatusProviderWeShouldDelegateToLocalConfigNodePermissioningProvider() { List providers = new ArrayList<>(); providers.add(localConfigNodePermissioningProvider); - this.controller = new NodePermissioningController(Optional.empty(), providers); + this.controller = + new NodePermissioningController(Optional.empty(), providers, Optional.empty()); controller.isPermitted(enode1, enode2); @@ -86,7 +91,8 @@ public class NodePermissioningControllerTest { whenInSyncWeShouldDelegateToAnyOtherNodePermissioningProviderAndIsPermittedIfAllPermitted() { List providers = getNodePermissioningProviders(); this.controller = - new NodePermissioningController(syncStatusNodePermissioningProviderOptional, providers); + new NodePermissioningController( + syncStatusNodePermissioningProviderOptional, providers, Optional.empty()); when(syncStatusNodePermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(true); when(syncStatusNodePermissioningProvider.hasReachedSync()).thenReturn(true); @@ -113,7 +119,8 @@ public class NodePermissioningControllerTest { List providers = getNodePermissioningProviders(); this.controller = - new NodePermissioningController(syncStatusNodePermissioningProviderOptional, providers); + new NodePermissioningController( + syncStatusNodePermissioningProviderOptional, providers, Optional.empty()); when(syncStatusNodePermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(true); when(syncStatusNodePermissioningProvider.hasReachedSync()).thenReturn(true); @@ -132,7 +139,8 @@ public class NodePermissioningControllerTest { final List providers = getNodePermissioningProviders(); this.controller = - new NodePermissioningController(syncStatusNodePermissioningProviderOptional, providers); + new NodePermissioningController( + syncStatusNodePermissioningProviderOptional, providers, Optional.empty()); final ContextualNodePermissioningProvider insufficientPeersPermissioningProvider = mock(ContextualNodePermissioningProvider.class); @@ -154,7 +162,8 @@ public class NodePermissioningControllerTest { final List providers = getNodePermissioningProviders(); this.controller = - new NodePermissioningController(syncStatusNodePermissioningProviderOptional, providers); + new NodePermissioningController( + syncStatusNodePermissioningProviderOptional, providers, Optional.empty()); final ContextualNodePermissioningProvider insufficientPeersPermissioningProvider = mock(ContextualNodePermissioningProvider.class); @@ -174,4 +183,45 @@ public class NodePermissioningControllerTest { verify(insufficientPeersPermissioningProvider, times(1)).isPermitted(any(), any()); providers.forEach(p -> verify(p, times(1)).isPermitted(any(), any())); } + + @Test + public void whenQuorumQip714GateIsEmptyShouldDelegateToProviders() { + this.controller = + new NodePermissioningController( + syncStatusNodePermissioningProviderOptional, Collections.emptyList(), Optional.empty()); + + controller.isPermitted(enode1, enode2); + + verify(syncStatusNodePermissioningProvider, atLeast(1)).isPermitted(eq(enode1), eq(enode2)); + } + + @Test + public void whenQuorumQip714GateIsNotActiveShouldBypassProviders() { + this.controller = + new NodePermissioningController( + syncStatusNodePermissioningProviderOptional, + Collections.emptyList(), + Optional.of(quorumQip714Gate)); + + when(quorumQip714Gate.shouldCheckPermissions()).thenReturn(false); + + assertThat(controller.isPermitted(enode1, enode2)).isTrue(); + + verifyNoInteractions(syncStatusNodePermissioningProvider); + } + + @Test + public void whenQuorumQip714GateIsActiveShouldDelegateToProviders() { + this.controller = + new NodePermissioningController( + syncStatusNodePermissioningProviderOptional, + Collections.emptyList(), + Optional.of(quorumQip714Gate)); + + when(quorumQip714Gate.shouldCheckPermissions()).thenReturn(true); + + controller.isPermitted(enode1, enode2); + + verify(syncStatusNodePermissioningProvider, atLeast(1)).isPermitted(eq(enode1), eq(enode2)); + } }