From 0f91a674ca4e24bd6decf61c61a433876c4affe6 Mon Sep 17 00:00:00 2001 From: Matt Whitehead Date: Mon, 29 Jan 2024 04:51:42 +0000 Subject: [PATCH 1/5] Write a DEBUG log entry to make it clear of a BFT node is a validator or not (#6470) Signed-off-by: Matthew Whitehead Co-authored-by: Sally MacFarlane --- .../common/bft/statemachine/BftFinalState.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/statemachine/BftFinalState.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/statemachine/BftFinalState.java index 13f6959779..0a65acc865 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/statemachine/BftFinalState.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/statemachine/BftFinalState.java @@ -28,8 +28,14 @@ import org.hyperledger.besu.datatypes.Address; import java.time.Clock; import java.util.Collection; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** This is the full data set, or context, required for many of the aspects of BFT workflows. */ public class BftFinalState { + + private static final Logger LOG = LoggerFactory.getLogger(BftFinalState.class); + private final ValidatorProvider validatorProvider; private final NodeKey nodeKey; private final Address localAddress; @@ -126,7 +132,9 @@ public class BftFinalState { * @return the boolean */ public boolean isLocalNodeValidator() { - return getValidators().contains(localAddress); + final boolean isValidator = getValidators().contains(localAddress); + LOG.debug(isValidator ? "Local node is a validator" : "Local node is a non-validator"); + return isValidator; } /** From f125aed72f2d522ef5c052bfce9faf95ad429b3b Mon Sep 17 00:00:00 2001 From: Thabokani <149070269+Thabokani@users.noreply.github.com> Date: Mon, 29 Jan 2024 16:00:25 +0800 Subject: [PATCH 2/5] Fix typos (#6481) * Fix typos Signed-off-by: Thabokani <149070269+Thabokani@users.noreply.github.com> * update plugin-api/build.gradle Signed-off-by: Thabokani <149070269+Thabokani@users.noreply.github.com> --------- Signed-off-by: Thabokani <149070269+Thabokani@users.noreply.github.com> --- .../java/org/hyperledger/besu/collections/undo/UndoScalar.java | 2 +- plugin-api/build.gradle | 2 +- .../besu/plugin/services/trielogs/TrieLogAccumulator.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/evm/src/main/java/org/hyperledger/besu/collections/undo/UndoScalar.java b/evm/src/main/java/org/hyperledger/besu/collections/undo/UndoScalar.java index 0e3356c379..55e3a84e0b 100644 --- a/evm/src/main/java/org/hyperledger/besu/collections/undo/UndoScalar.java +++ b/evm/src/main/java/org/hyperledger/besu/collections/undo/UndoScalar.java @@ -61,7 +61,7 @@ public class UndoScalar implements Undoable { } /** - * Has this scalar had any change since the inital value + * Has this scalar had any change since the initial value * * @return true if there are any changes to undo */ diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index c72ad93433..01ed8833da 100644 --- a/plugin-api/build.gradle +++ b/plugin-api/build.gradle @@ -69,7 +69,7 @@ Calculated : ${currentHash} tasks.register('checkAPIChanges', FileStateChecker) { description = "Checks that the API for the Plugin-API project does not change without deliberate thought" files = sourceSets.main.allJava.files - knownHash = 'ZsovOR0oPfomcLP4b+HjikWzM0Tx6sCwi68mf5qwZf4=' + knownHash = 'VpNy2KuAtEUc9hPguNivbjwy2YM3vIF444RCREJojqY=' } check.dependsOn('checkAPIChanges') diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogAccumulator.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogAccumulator.java index 6984ca48a1..384a327166 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogAccumulator.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/trielogs/TrieLogAccumulator.java @@ -23,7 +23,7 @@ import java.util.Map; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt256; -/** Accumulator interface tor provding trie updates for creating TrieLogs. */ +/** Accumulator interface for providing trie updates for creating TrieLogs. */ public interface TrieLogAccumulator { /** From 98ab26df2f50823c473fd6d63b7d603dd6681051 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Mon, 29 Jan 2024 08:41:31 -0800 Subject: [PATCH 3/5] filter empty ipv4 and ipv6 ping packet source addresses (#6474) * add empty ipv4 and ipv6 to the filtered ping packet source addresses Signed-off-by: garyschulte --- .../p2p/discovery/PeerDiscoveryAgent.java | 2 +- .../p2p/discovery/PeerDiscoveryAgentTest.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java index a4af09481c..5d05cc6531 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java @@ -75,7 +75,7 @@ public abstract class PeerDiscoveryAgent { // clients ignore that, so we add in a little extra padding. private static final int MAX_PACKET_SIZE_BYTES = 1600; private static final List PING_PACKET_SOURCE_IGNORED = - List.of("127.0.0.1", "255.255.255.255"); + List.of("127.0.0.1", "255.255.255.255", "0.0.0.0", "::", "0:0:0:0:0:0:0:0"); protected final List bootstrapPeers; private final List peerRequirements = new CopyOnWriteArrayList<>(); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java index aae83ffdc8..269eb92b08 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java @@ -839,15 +839,31 @@ public class PeerDiscoveryAgentTest { @Test public void assertHostCorrectlyRevertsOnIgnoredPacketFrom() { final String sourceHost = "UDP_SOURCE_ORIGIN_HOST"; + final String emptyIPv4Host = "0.0.0.0"; + final String emptyIPv6Host = "::"; final String localHost = "127.0.0.1"; final String broadcastDefaultHost = "255.255.255.255"; final String routableHost = "50.50.50.50"; Endpoint source = new Endpoint(sourceHost, 30303, Optional.empty()); + Endpoint emptyIPv4 = new Endpoint(emptyIPv4Host, 30303, Optional.empty()); + Endpoint emptyIPv6 = new Endpoint(emptyIPv6Host, 30303, Optional.empty()); Endpoint endpointLocal = new Endpoint(localHost, 30303, Optional.empty()); Endpoint endpointBroadcast = new Endpoint(broadcastDefaultHost, 30303, Optional.empty()); Endpoint endpointRoutable = new Endpoint(routableHost, 30303, Optional.empty()); + Packet mockEmptyIPv4 = + when(mock(Packet.class).getPacketData(any())) + .thenReturn( + Optional.of( + PingPacketData.create(Optional.of(emptyIPv4), endpointLocal, UInt64.ONE))) + .getMock(); + Packet mockEmptyIPv6 = + when(mock(Packet.class).getPacketData(any())) + .thenReturn( + Optional.of( + PingPacketData.create(Optional.of(emptyIPv6), endpointLocal, UInt64.ONE))) + .getMock(); Packet mockLocal = when(mock(Packet.class).getPacketData(any())) .thenReturn( @@ -869,6 +885,10 @@ public class PeerDiscoveryAgentTest { Optional.of(endpointRoutable), endpointLocal, UInt64.ONE))) .getMock(); + // assert a pingpacketdata with empty ipv4 address reverts to the udp source host + assertThat(PeerDiscoveryAgent.deriveHost(source, mockEmptyIPv4)).isEqualTo(sourceHost); + // assert a pingpacketdata with empty ipv6 address reverts to the udp source host + assertThat(PeerDiscoveryAgent.deriveHost(source, mockEmptyIPv6)).isEqualTo(sourceHost); // assert a pingpacketdata from address of 127.0.0.1 reverts to the udp source host assertThat(PeerDiscoveryAgent.deriveHost(source, mockLocal)).isEqualTo(sourceHost); // assert that 255.255.255.255 reverts to the udp source host From 9084bde9d8df82a4818dc96678cdbe89b526f7c8 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Tue, 30 Jan 2024 12:08:06 +1100 Subject: [PATCH 4/5] Move permission options to itw own class (#6490) Signed-off-by: Gabriel-Trintinalia --- .../org/hyperledger/besu/cli/BesuCommand.java | 176 +------ .../options/stable/PermissionsOptions.java | 206 ++++++++ .../hyperledger/besu/cli/BesuCommandTest.java | 412 ---------------- .../cli/options/PermissionsOptionsTest.java | 453 ++++++++++++++++++ 4 files changed, 671 insertions(+), 576 deletions(-) create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java create mode 100644 besu/src/test/java/org/hyperledger/besu/cli/options/PermissionsOptionsTest.java 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 68c3a6a5ab..8e59b4c23d 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -56,6 +56,7 @@ import org.hyperledger.besu.cli.options.stable.JsonRpcHttpOptions; import org.hyperledger.besu.cli.options.stable.LoggingLevelOption; import org.hyperledger.besu.cli.options.stable.NodePrivateKeyFileOption; import org.hyperledger.besu.cli.options.stable.P2PTLSConfigOptions; +import org.hyperledger.besu.cli.options.stable.PermissionsOptions; import org.hyperledger.besu.cli.options.stable.RpcWebsocketOptions; import org.hyperledger.besu.cli.options.unstable.ChainPruningOptions; import org.hyperledger.besu.cli.options.unstable.DnsOptions; @@ -130,8 +131,6 @@ import org.hyperledger.besu.ethereum.p2p.peers.StaticNodesParser; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.TLSConfiguration; 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.SmartContractPermissioningConfiguration; import org.hyperledger.besu.ethereum.privacy.storage.keyvalue.PrivacyKeyValueStorageProvider; import org.hyperledger.besu.ethereum.privacy.storage.keyvalue.PrivacyKeyValueStorageProviderBuilder; import org.hyperledger.besu.ethereum.storage.StorageProvider; @@ -825,62 +824,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { // Permission Option Group @CommandLine.ArgGroup(validate = false, heading = "@|bold Permissions Options|@%n") - PermissionsOptionGroup permissionsOptionGroup = new PermissionsOptionGroup(); - - static class PermissionsOptionGroup { - @Option( - names = {"--permissions-nodes-config-file-enabled"}, - description = "Enable node level permissions (default: ${DEFAULT-VALUE})") - private final Boolean permissionsNodesEnabled = false; - - @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. - @CommandLine.Option( - names = {"--permissions-nodes-config-file"}, - description = - "Node permissioning config TOML file (default: a file named \"permissions_config.toml\" in the Besu data folder)") - private String nodePermissionsConfigFile = null; - - @Option( - names = {"--permissions-accounts-config-file-enabled"}, - description = "Enable account level permissions (default: ${DEFAULT-VALUE})") - private final Boolean permissionsAccountsEnabled = false; - - @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. - @CommandLine.Option( - names = {"--permissions-accounts-config-file"}, - description = - "Account permissioning config TOML file (default: a file named \"permissions_config.toml\" in the Besu data folder)") - private String accountPermissionsConfigFile = null; - - @Option( - names = {"--permissions-nodes-contract-address"}, - description = "Address of the node permissioning smart contract", - arity = "1") - private final Address permissionsNodesContractAddress = null; - - @Option( - names = {"--permissions-nodes-contract-version"}, - description = "Version of the EEA Node Permissioning interface (default: ${DEFAULT-VALUE})") - private final Integer permissionsNodesContractVersion = 1; - - @Option( - names = {"--permissions-nodes-contract-enabled"}, - description = - "Enable node level permissions via smart contract (default: ${DEFAULT-VALUE})") - private final Boolean permissionsNodesContractEnabled = false; - - @Option( - names = {"--permissions-accounts-contract-address"}, - description = "Address of the account permissioning smart contract", - arity = "1") - private final Address permissionsAccountsContractAddress = null; - - @Option( - names = {"--permissions-accounts-contract-enabled"}, - description = - "Enable account level permissions via smart contract (default: ${DEFAULT-VALUE})") - private final Boolean permissionsAccountsContractEnabled = false; - } + PermissionsOptions permissionsOptions = new PermissionsOptions(); @Option( names = {"--revert-reason-enabled"}, @@ -1852,6 +1796,16 @@ public class BesuCommand implements DefaultCommandValues, Runnable { logger.info("Security Module: {}", securityModuleName); } + private Optional permissioningConfiguration() throws Exception { + return permissionsOptions.permissioningConfiguration( + jsonRpcHttpOptions, + rpcWebsocketOptions, + getEnodeDnsConfiguration(), + dataDir(), + logger, + commandLine); + } + private JsonRpcIpcConfiguration jsonRpcIpcConfiguration( final Boolean enabled, final Path ipcPath, final List rpcIpcApis) { final Path actualPath; @@ -2091,106 +2045,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .build(); } - private Optional permissioningConfiguration() throws Exception { - if (!(localPermissionsEnabled() || contractPermissionsEnabled())) { - if (jsonRpcHttpOptions.getRpcHttpApis().contains(RpcApis.PERM.name()) - || rpcWebsocketOptions.getRpcWsApis().contains(RpcApis.PERM.name())) { - logger.warn( - "Permissions are disabled. Cannot enable PERM APIs when not using Permissions."); - } - return Optional.empty(); - } - - final Optional localPermissioningConfigurationOptional; - if (localPermissionsEnabled()) { - final Optional nodePermissioningConfigFile = - Optional.ofNullable(permissionsOptionGroup.nodePermissionsConfigFile); - final Optional accountPermissioningConfigFile = - Optional.ofNullable(permissionsOptionGroup.accountPermissionsConfigFile); - - final LocalPermissioningConfiguration localPermissioningConfiguration = - PermissioningConfigurationBuilder.permissioningConfiguration( - permissionsOptionGroup.permissionsNodesEnabled, - getEnodeDnsConfiguration(), - nodePermissioningConfigFile.orElse(getDefaultPermissioningFilePath()), - permissionsOptionGroup.permissionsAccountsEnabled, - accountPermissioningConfigFile.orElse(getDefaultPermissioningFilePath())); - - localPermissioningConfigurationOptional = Optional.of(localPermissioningConfiguration); - } else { - if (permissionsOptionGroup.nodePermissionsConfigFile != null - && !permissionsOptionGroup.permissionsNodesEnabled) { - logger.warn( - "Node permissioning config file set {} but no permissions enabled", - permissionsOptionGroup.nodePermissionsConfigFile); - } - - if (permissionsOptionGroup.accountPermissionsConfigFile != null - && !permissionsOptionGroup.permissionsAccountsEnabled) { - logger.warn( - "Account permissioning config file set {} but no permissions enabled", - permissionsOptionGroup.accountPermissionsConfigFile); - } - localPermissioningConfigurationOptional = Optional.empty(); - } - - final SmartContractPermissioningConfiguration smartContractPermissioningConfiguration = - SmartContractPermissioningConfiguration.createDefault(); - - if (Boolean.TRUE.equals(permissionsOptionGroup.permissionsNodesContractEnabled)) { - if (permissionsOptionGroup.permissionsNodesContractAddress == null) { - throw new ParameterException( - this.commandLine, - "No node permissioning contract address specified. Cannot enable smart contract based node permissioning."); - } else { - smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled( - permissionsOptionGroup.permissionsNodesContractEnabled); - smartContractPermissioningConfiguration.setNodeSmartContractAddress( - permissionsOptionGroup.permissionsNodesContractAddress); - smartContractPermissioningConfiguration.setNodeSmartContractInterfaceVersion( - permissionsOptionGroup.permissionsNodesContractVersion); - } - } else if (permissionsOptionGroup.permissionsNodesContractAddress != null) { - logger.warn( - "Node permissioning smart contract address set {} but smart contract node permissioning is disabled.", - permissionsOptionGroup.permissionsNodesContractAddress); - } - - if (Boolean.TRUE.equals(permissionsOptionGroup.permissionsAccountsContractEnabled)) { - if (permissionsOptionGroup.permissionsAccountsContractAddress == null) { - throw new ParameterException( - this.commandLine, - "No account permissioning contract address specified. Cannot enable smart contract based account permissioning."); - } else { - smartContractPermissioningConfiguration.setSmartContractAccountAllowlistEnabled( - permissionsOptionGroup.permissionsAccountsContractEnabled); - smartContractPermissioningConfiguration.setAccountSmartContractAddress( - permissionsOptionGroup.permissionsAccountsContractAddress); - } - } else if (permissionsOptionGroup.permissionsAccountsContractAddress != null) { - logger.warn( - "Account permissioning smart contract address set {} but smart contract account permissioning is disabled.", - permissionsOptionGroup.permissionsAccountsContractAddress); - } - - final PermissioningConfiguration permissioningConfiguration = - new PermissioningConfiguration( - localPermissioningConfigurationOptional, - Optional.of(smartContractPermissioningConfiguration)); - - return Optional.of(permissioningConfiguration); - } - - private boolean localPermissionsEnabled() { - return permissionsOptionGroup.permissionsAccountsEnabled - || permissionsOptionGroup.permissionsNodesEnabled; - } - - private boolean contractPermissionsEnabled() { - return permissionsOptionGroup.permissionsNodesContractEnabled - || permissionsOptionGroup.permissionsAccountsContractEnabled; - } - private PrivacyParameters privacyParameters() { CommandLineUtils.checkOptionDependencies( @@ -2661,12 +2515,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable { .orElseGet(() -> KeyPairUtil.getDefaultKeyFile(dataDir())); } - private String getDefaultPermissioningFilePath() { - return dataDir() - + System.getProperty("file.separator") - + DefaultCommandValues.PERMISSIONING_CONFIG_LOCATION; - } - /** * Metrics System used by Besu * diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java new file mode 100644 index 0000000000..3f16432750 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PermissionsOptions.java @@ -0,0 +1,206 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.cli.options.stable; + +import org.hyperledger.besu.cli.DefaultCommandValues; +import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis; +import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration; +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.SmartContractPermissioningConfiguration; + +import java.nio.file.Path; +import java.util.Optional; + +import org.slf4j.Logger; +import picocli.CommandLine; + +/** Handles configuration options for permissions in Besu. */ +public class PermissionsOptions { + @CommandLine.Option( + names = {"--permissions-nodes-config-file-enabled"}, + description = "Enable node level permissions (default: ${DEFAULT-VALUE})") + private final Boolean permissionsNodesEnabled = false; + + @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. + @CommandLine.Option( + names = {"--permissions-nodes-config-file"}, + description = + "Node permissioning config TOML file (default: a file named \"permissions_config.toml\" in the Besu data folder)") + private String nodePermissionsConfigFile = null; + + @CommandLine.Option( + names = {"--permissions-accounts-config-file-enabled"}, + description = "Enable account level permissions (default: ${DEFAULT-VALUE})") + private final Boolean permissionsAccountsEnabled = false; + + @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings. + @CommandLine.Option( + names = {"--permissions-accounts-config-file"}, + description = + "Account permissioning config TOML file (default: a file named \"permissions_config.toml\" in the Besu data folder)") + private String accountPermissionsConfigFile = null; + + @CommandLine.Option( + names = {"--permissions-nodes-contract-address"}, + description = "Address of the node permissioning smart contract", + arity = "1") + private final Address permissionsNodesContractAddress = null; + + @CommandLine.Option( + names = {"--permissions-nodes-contract-version"}, + description = "Version of the EEA Node Permissioning interface (default: ${DEFAULT-VALUE})") + private final Integer permissionsNodesContractVersion = 1; + + @CommandLine.Option( + names = {"--permissions-nodes-contract-enabled"}, + description = "Enable node level permissions via smart contract (default: ${DEFAULT-VALUE})") + private final Boolean permissionsNodesContractEnabled = false; + + @CommandLine.Option( + names = {"--permissions-accounts-contract-address"}, + description = "Address of the account permissioning smart contract", + arity = "1") + private final Address permissionsAccountsContractAddress = null; + + @CommandLine.Option( + names = {"--permissions-accounts-contract-enabled"}, + description = + "Enable account level permissions via smart contract (default: ${DEFAULT-VALUE})") + private final Boolean permissionsAccountsContractEnabled = false; + + /** + * Creates a PermissioningConfiguration based on the provided options. + * + * @param jsonRpcHttpOptions The JSON-RPC HTTP options + * @param rpcWebsocketOptions The RPC websocket options + * @param enodeDnsConfiguration The enode DNS configuration + * @param dataPath The data path + * @param logger The logger + * @param commandLine The command line + * @return An Optional PermissioningConfiguration instance + * @throws Exception If an error occurs while creating the configuration + */ + public Optional permissioningConfiguration( + final JsonRpcHttpOptions jsonRpcHttpOptions, + final RpcWebsocketOptions rpcWebsocketOptions, + final EnodeDnsConfiguration enodeDnsConfiguration, + final Path dataPath, + final Logger logger, + final CommandLine commandLine) + throws Exception { + if (!(localPermissionsEnabled() || contractPermissionsEnabled())) { + if (jsonRpcHttpOptions.getRpcHttpApis().contains(RpcApis.PERM.name()) + || rpcWebsocketOptions.getRpcWsApis().contains(RpcApis.PERM.name())) { + logger.warn( + "Permissions are disabled. Cannot enable PERM APIs when not using Permissions."); + } + return Optional.empty(); + } + + final Optional localPermissioningConfigurationOptional; + if (localPermissionsEnabled()) { + final Optional nodePermissioningConfigFile = + Optional.ofNullable(nodePermissionsConfigFile); + final Optional accountPermissioningConfigFile = + Optional.ofNullable(accountPermissionsConfigFile); + + final LocalPermissioningConfiguration localPermissioningConfiguration = + PermissioningConfigurationBuilder.permissioningConfiguration( + permissionsNodesEnabled, + enodeDnsConfiguration, + nodePermissioningConfigFile.orElse(getDefaultPermissioningFilePath(dataPath)), + permissionsAccountsEnabled, + accountPermissioningConfigFile.orElse(getDefaultPermissioningFilePath(dataPath))); + + localPermissioningConfigurationOptional = Optional.of(localPermissioningConfiguration); + } else { + if (nodePermissionsConfigFile != null && !permissionsNodesEnabled) { + logger.warn( + "Node permissioning config file set {} but no permissions enabled", + nodePermissionsConfigFile); + } + + if (accountPermissionsConfigFile != null && !permissionsAccountsEnabled) { + logger.warn( + "Account permissioning config file set {} but no permissions enabled", + accountPermissionsConfigFile); + } + localPermissioningConfigurationOptional = Optional.empty(); + } + + final SmartContractPermissioningConfiguration smartContractPermissioningConfiguration = + SmartContractPermissioningConfiguration.createDefault(); + + if (Boolean.TRUE.equals(permissionsNodesContractEnabled)) { + if (permissionsNodesContractAddress == null) { + throw new CommandLine.ParameterException( + commandLine, + "No node permissioning contract address specified. Cannot enable smart contract based node permissioning."); + } else { + smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled( + permissionsNodesContractEnabled); + smartContractPermissioningConfiguration.setNodeSmartContractAddress( + permissionsNodesContractAddress); + smartContractPermissioningConfiguration.setNodeSmartContractInterfaceVersion( + permissionsNodesContractVersion); + } + } else if (permissionsNodesContractAddress != null) { + logger.warn( + "Node permissioning smart contract address set {} but smart contract node permissioning is disabled.", + permissionsNodesContractAddress); + } + + if (Boolean.TRUE.equals(permissionsAccountsContractEnabled)) { + if (permissionsAccountsContractAddress == null) { + throw new CommandLine.ParameterException( + commandLine, + "No account permissioning contract address specified. Cannot enable smart contract based account permissioning."); + } else { + smartContractPermissioningConfiguration.setSmartContractAccountAllowlistEnabled( + permissionsAccountsContractEnabled); + smartContractPermissioningConfiguration.setAccountSmartContractAddress( + permissionsAccountsContractAddress); + } + } else if (permissionsAccountsContractAddress != null) { + logger.warn( + "Account permissioning smart contract address set {} but smart contract account permissioning is disabled.", + permissionsAccountsContractAddress); + } + + final PermissioningConfiguration permissioningConfiguration = + new PermissioningConfiguration( + localPermissioningConfigurationOptional, + Optional.of(smartContractPermissioningConfiguration)); + + return Optional.of(permissioningConfiguration); + } + + private boolean localPermissionsEnabled() { + return permissionsAccountsEnabled || permissionsNodesEnabled; + } + + private boolean contractPermissionsEnabled() { + return permissionsNodesContractEnabled || permissionsAccountsContractEnabled; + } + + private String getDefaultPermissioningFilePath(final Path dataPath) { + return dataPath + + System.getProperty("file.separator") + + DefaultCommandValues.PERMISSIONING_CONFIG_LOCATION; + } +} diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index c69d886109..3850233d52 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -51,7 +51,6 @@ import org.hyperledger.besu.cli.config.EthNetworkConfig; import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.config.MergeConfigOptions; import org.hyperledger.besu.crypto.SignatureAlgorithmFactory; -import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.GasLimitCalculator; @@ -67,9 +66,6 @@ import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; -import org.hyperledger.besu.ethereum.permissioning.LocalPermissioningConfiguration; -import org.hyperledger.besu.ethereum.permissioning.PermissioningConfiguration; -import org.hyperledger.besu.ethereum.permissioning.SmartContractPermissioningConfiguration; import org.hyperledger.besu.ethereum.trie.forest.pruner.PrunerConfiguration; import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration; import org.hyperledger.besu.evm.precompile.AbstractAltBnPrecompiledContract; @@ -96,7 +92,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -104,7 +99,6 @@ import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; -import com.google.common.collect.Lists; import com.google.common.io.Resources; import io.vertx.core.json.JsonObject; import org.apache.commons.io.FileUtils; @@ -129,7 +123,6 @@ public class BesuCommandTest extends CommandTestAbstract { private static final String ENCLAVE_PUBLIC_KEY = "A1aVtMxLCUHmBVHXoZzzBgPbW/wj5axDpW9X8l91SGo="; private static final String VALID_NODE_ID = "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"; - private static final String PERMISSIONING_CONFIG_TOML = "/permissioning_config.toml"; private static final JsonRpcConfiguration DEFAULT_JSON_RPC_CONFIGURATION; private static final GraphQLConfiguration DEFAULT_GRAPH_QL_CONFIGURATION; private static final WebSocketConfiguration DEFAULT_WEB_SOCKET_CONFIGURATION; @@ -349,374 +342,6 @@ public class BesuCommandTest extends CommandTestAbstract { assertThat(commandOutput.toString(UTF_8)).isEmpty(); } - @Test - public void nodePermissionsSmartContractWithoutOptionMustError() { - parseCommand("--permissions-nodes-contract-address"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)) - .startsWith("Missing required parameter for option '--permissions-nodes-contract-address'"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissionsEnabledWithoutContractAddressMustError() { - parseCommand("--permissions-nodes-contract-enabled"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)) - .contains("No node permissioning contract address specified"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissionsEnabledWithInvalidContractAddressMustError() { - parseCommand( - "--permissions-nodes-contract-enabled", - "--permissions-nodes-contract-address", - "invalid-smart-contract-address"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissionsEnabledWithTooShortContractAddressMustError() { - parseCommand( - "--permissions-nodes-contract-enabled", "--permissions-nodes-contract-address", "0x1234"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissionsSmartContractMustUseOption() { - - final String smartContractAddress = "0x0000000000000000000000000000000000001234"; - - parseCommand( - "--permissions-nodes-contract-enabled", - "--permissions-nodes-contract-address", - smartContractAddress); - final SmartContractPermissioningConfiguration smartContractPermissioningConfiguration = - new SmartContractPermissioningConfiguration(); - smartContractPermissioningConfiguration.setNodeSmartContractAddress( - Address.fromHexString(smartContractAddress)); - smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled(true); - - verify(mockRunnerBuilder) - .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); - verify(mockRunnerBuilder).build(); - - final PermissioningConfiguration config = - permissioningConfigurationArgumentCaptor.getValue().get(); - assertThat(config.getSmartContractConfig().get()) - .usingRecursiveComparison() - .isEqualTo(smartContractPermissioningConfiguration); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissionsContractVersionDefaultValue() { - final SmartContractPermissioningConfiguration expectedConfig = - new SmartContractPermissioningConfiguration(); - expectedConfig.setNodeSmartContractAddress( - Address.fromHexString("0x0000000000000000000000000000000000001234")); - expectedConfig.setSmartContractNodeAllowlistEnabled(true); - expectedConfig.setNodeSmartContractInterfaceVersion(1); - - parseCommand( - "--permissions-nodes-contract-enabled", - "--permissions-nodes-contract-address", - "0x0000000000000000000000000000000000001234"); - - verify(mockRunnerBuilder) - .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); - verify(mockRunnerBuilder).build(); - - final PermissioningConfiguration config = - permissioningConfigurationArgumentCaptor.getValue().get(); - assertThat(config.getSmartContractConfig().get()) - .usingRecursiveComparison() - .isEqualTo(expectedConfig); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissionsContractVersionSetsValue() { - final SmartContractPermissioningConfiguration expectedConfig = - new SmartContractPermissioningConfiguration(); - expectedConfig.setNodeSmartContractAddress( - Address.fromHexString("0x0000000000000000000000000000000000001234")); - expectedConfig.setSmartContractNodeAllowlistEnabled(true); - expectedConfig.setNodeSmartContractInterfaceVersion(2); - - parseCommand( - "--permissions-nodes-contract-enabled", - "--permissions-nodes-contract-address", - "0x0000000000000000000000000000000000001234", - "--permissions-nodes-contract-version", - "2"); - - verify(mockRunnerBuilder) - .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); - verify(mockRunnerBuilder).build(); - - final PermissioningConfiguration config = - permissioningConfigurationArgumentCaptor.getValue().get(); - assertThat(config.getSmartContractConfig().get()) - .usingRecursiveComparison() - .isEqualTo(expectedConfig); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissionsSmartContractWithoutOptionMustError() { - parseCommand("--permissions-accounts-contract-address"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)) - .startsWith( - "Missing required parameter for option '--permissions-accounts-contract-address'"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissionsEnabledWithoutContractAddressMustError() { - parseCommand("--permissions-accounts-contract-enabled"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)) - .contains("No account permissioning contract address specified"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissionsEnabledWithInvalidContractAddressMustError() { - parseCommand( - "--permissions-accounts-contract-enabled", - "--permissions-accounts-contract-address", - "invalid-smart-contract-address"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissionsEnabledWithTooShortContractAddressMustError() { - parseCommand( - "--permissions-accounts-contract-enabled", - "--permissions-accounts-contract-address", - "0x1234"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissionsSmartContractMustUseOption() { - final String smartContractAddress = "0x0000000000000000000000000000000000001234"; - - parseCommand( - "--permissions-accounts-contract-enabled", - "--permissions-accounts-contract-address", - smartContractAddress); - final SmartContractPermissioningConfiguration smartContractPermissioningConfiguration = - new SmartContractPermissioningConfiguration(); - smartContractPermissioningConfiguration.setAccountSmartContractAddress( - Address.fromHexString(smartContractAddress)); - smartContractPermissioningConfiguration.setSmartContractAccountAllowlistEnabled(true); - - verify(mockRunnerBuilder) - .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); - final PermissioningConfiguration permissioningConfiguration = - permissioningConfigurationArgumentCaptor.getValue().get(); - assertThat(permissioningConfiguration.getSmartContractConfig()).isPresent(); - - final SmartContractPermissioningConfiguration effectiveSmartContractConfig = - permissioningConfiguration.getSmartContractConfig().get(); - assertThat(effectiveSmartContractConfig.isSmartContractAccountAllowlistEnabled()).isTrue(); - assertThat(effectiveSmartContractConfig.getAccountSmartContractAddress()) - .isEqualTo(Address.fromHexString(smartContractAddress)); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissioningTomlPathWithoutOptionMustDisplayUsage() { - parseCommand("--permissions-nodes-config-file"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)) - .startsWith("Missing required parameter for option '--permissions-nodes-config-file'"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissioningTomlPathWithoutOptionMustDisplayUsage() { - parseCommand("--permissions-accounts-config-file"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)) - .startsWith("Missing required parameter for option '--permissions-accounts-config-file'"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissioningEnabledWithNonexistentConfigFileMustError() { - parseCommand( - "--permissions-nodes-config-file-enabled", - "--permissions-nodes-config-file", - "file-does-not-exist"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)).contains("Configuration file does not exist"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissioningEnabledWithNonexistentConfigFileMustError() { - parseCommand( - "--permissions-accounts-config-file-enabled", - "--permissions-accounts-config-file", - "file-does-not-exist"); - - Mockito.verifyNoInteractions(mockRunnerBuilder); - - assertThat(commandErrorOutput.toString(UTF_8)).contains("Configuration file does not exist"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissioningTomlFileWithNoPermissionsEnabledMustNotError() throws IOException { - - final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); - final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); - parseCommand("--permissions-nodes-config-file", permToml.toString()); - - verify(mockRunnerBuilder).build(); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissioningTomlFileWithNoPermissionsEnabledMustNotError() - throws IOException { - - final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); - final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); - parseCommand("--permissions-accounts-config-file", permToml.toString()); - - verify(mockRunnerBuilder).build(); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void defaultPermissionsTomlFileWithNoPermissionsEnabledMustNotError() { - parseCommand("--p2p-enabled", "false"); - - verify(mockRunnerBuilder).build(); - - assertThat(commandErrorOutput.toString(UTF_8)).doesNotContain("no permissions enabled"); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void nodePermissioningTomlPathMustUseOption() throws IOException { - final List allowedNodes = - Lists.newArrayList( - EnodeURLImpl.fromString( - "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.9:4567"), - EnodeURLImpl.fromString( - "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.169.0.9:4568")); - - final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); - final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); - - final String allowedNodesString = - allowedNodes.stream().map(Object::toString).collect(Collectors.joining(",")); - parseCommand( - "--permissions-nodes-config-file-enabled", - "--permissions-nodes-config-file", - permToml.toString(), - "--bootnodes", - allowedNodesString); - final LocalPermissioningConfiguration localPermissioningConfiguration = - LocalPermissioningConfiguration.createDefault(); - localPermissioningConfiguration.setNodePermissioningConfigFilePath(permToml.toString()); - localPermissioningConfiguration.setNodeAllowlist(allowedNodes); - - verify(mockRunnerBuilder) - .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); - verify(mockRunnerBuilder).build(); - - final PermissioningConfiguration config = - permissioningConfigurationArgumentCaptor.getValue().get(); - assertThat(config.getLocalConfig().get()) - .usingRecursiveComparison() - .isEqualTo(localPermissioningConfiguration); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - - @Test - public void accountPermissioningTomlPathMustUseOption() throws IOException { - - final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); - final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); - - parseCommand( - "--permissions-accounts-config-file-enabled", - "--permissions-accounts-config-file", - permToml.toString()); - final LocalPermissioningConfiguration localPermissioningConfiguration = - LocalPermissioningConfiguration.createDefault(); - localPermissioningConfiguration.setAccountPermissioningConfigFilePath(permToml.toString()); - localPermissioningConfiguration.setAccountAllowlist( - Collections.singletonList("0x0000000000000000000000000000000000000009")); - - verify(mockRunnerBuilder) - .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); - final PermissioningConfiguration permissioningConfiguration = - permissioningConfigurationArgumentCaptor.getValue().get(); - assertThat(permissioningConfiguration.getLocalConfig()).isPresent(); - - final LocalPermissioningConfiguration effectiveLocalPermissioningConfig = - permissioningConfiguration.getLocalConfig().get(); - assertThat(effectiveLocalPermissioningConfig.isAccountAllowlistEnabled()).isTrue(); - assertThat(effectiveLocalPermissioningConfig.getAccountPermissioningConfigFilePath()) - .isEqualTo(permToml.toString()); - - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); - } - @Test public void tomlThatConfiguresEverythingExceptPermissioningToml() throws IOException { // Load a TOML that configures literally everything (except permissioning TOML config) @@ -3166,43 +2791,6 @@ public class BesuCommandTest extends CommandTestAbstract { assertThat(commandOutput.toString(UTF_8)).isEmpty(); } - @Test - public void errorIsRaisedIfStaticNodesAreNotAllowed(final @TempDir Path testFolder) - throws IOException { - final Path staticNodesFile = testFolder.resolve("static-nodes.json"); - final Path permissioningConfig = testFolder.resolve("permissioning.json"); - - final EnodeURL staticNodeURI = - EnodeURLImpl.builder() - .nodeId( - "50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa") - .ipAddress("127.0.0.1") - .useDefaultPorts() - .build(); - - final EnodeURL allowedNode = - EnodeURLImpl.builder() - .nodeId( - "50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa") - .useDefaultPorts() - .ipAddress("127.0.0.1") - .listeningPort(30304) - .build(); - - Files.write(staticNodesFile, ("[\"" + staticNodeURI.toString() + "\"]").getBytes(UTF_8)); - Files.write( - permissioningConfig, - ("nodes-allowlist=[\"" + allowedNode.toString() + "\"]").getBytes(UTF_8)); - - parseCommand( - "--data-path=" + testFolder, - "--bootnodes", - "--permissions-nodes-config-file-enabled=true", - "--permissions-nodes-config-file=" + permissioningConfig); - assertThat(commandErrorOutput.toString(UTF_8)) - .contains(staticNodeURI.toString(), "not in nodes-allowlist"); - } - @Test public void tomlThatHasInvalidOptions() throws IOException { final URL configFile = this.getClass().getResource("/complete_config.toml"); diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/PermissionsOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/PermissionsOptionsTest.java new file mode 100644 index 0000000000..e93f712c6f --- /dev/null +++ b/besu/src/test/java/org/hyperledger/besu/cli/options/PermissionsOptionsTest.java @@ -0,0 +1,453 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.cli.options; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; + +import org.hyperledger.besu.cli.CommandTestAbstract; +import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; +import org.hyperledger.besu.ethereum.permissioning.LocalPermissioningConfiguration; +import org.hyperledger.besu.ethereum.permissioning.PermissioningConfiguration; +import org.hyperledger.besu.ethereum.permissioning.SmartContractPermissioningConfiguration; +import org.hyperledger.besu.plugin.data.EnodeURL; + +import java.io.IOException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import com.google.common.collect.Lists; +import com.google.common.io.Resources; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +public class PermissionsOptionsTest extends CommandTestAbstract { + private static final String PERMISSIONING_CONFIG_TOML = "/permissioning_config.toml"; + + @Test + public void errorIsRaisedIfStaticNodesAreNotAllowed(final @TempDir Path testFolder) + throws IOException { + final Path staticNodesFile = testFolder.resolve("static-nodes.json"); + final Path permissioningConfig = testFolder.resolve("permissioning.json"); + + final EnodeURL staticNodeURI = + EnodeURLImpl.builder() + .nodeId( + "50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa") + .ipAddress("127.0.0.1") + .useDefaultPorts() + .build(); + + final EnodeURL allowedNode = + EnodeURLImpl.builder() + .nodeId( + "50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa") + .useDefaultPorts() + .ipAddress("127.0.0.1") + .listeningPort(30304) + .build(); + + Files.write(staticNodesFile, ("[\"" + staticNodeURI.toString() + "\"]").getBytes(UTF_8)); + Files.write( + permissioningConfig, + ("nodes-allowlist=[\"" + allowedNode.toString() + "\"]").getBytes(UTF_8)); + + parseCommand( + "--data-path=" + testFolder, + "--bootnodes", + "--permissions-nodes-config-file-enabled=true", + "--permissions-nodes-config-file=" + permissioningConfig); + assertThat(commandErrorOutput.toString(UTF_8)) + .contains(staticNodeURI.toString(), "not in nodes-allowlist"); + } + + @Test + public void nodePermissionsSmartContractWithoutOptionMustError() { + parseCommand("--permissions-nodes-contract-address"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)) + .startsWith("Missing required parameter for option '--permissions-nodes-contract-address'"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissionsEnabledWithoutContractAddressMustError() { + parseCommand("--permissions-nodes-contract-enabled"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)) + .contains("No node permissioning contract address specified"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissionsEnabledWithInvalidContractAddressMustError() { + parseCommand( + "--permissions-nodes-contract-enabled", + "--permissions-nodes-contract-address", + "invalid-smart-contract-address"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissionsEnabledWithTooShortContractAddressMustError() { + parseCommand( + "--permissions-nodes-contract-enabled", "--permissions-nodes-contract-address", "0x1234"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissionsSmartContractMustUseOption() { + + final String smartContractAddress = "0x0000000000000000000000000000000000001234"; + + parseCommand( + "--permissions-nodes-contract-enabled", + "--permissions-nodes-contract-address", + smartContractAddress); + final SmartContractPermissioningConfiguration smartContractPermissioningConfiguration = + new SmartContractPermissioningConfiguration(); + smartContractPermissioningConfiguration.setNodeSmartContractAddress( + Address.fromHexString(smartContractAddress)); + smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled(true); + + verify(mockRunnerBuilder) + .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + final PermissioningConfiguration config = + permissioningConfigurationArgumentCaptor.getValue().get(); + assertThat(config.getSmartContractConfig().get()) + .usingRecursiveComparison() + .isEqualTo(smartContractPermissioningConfiguration); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissionsContractVersionDefaultValue() { + final SmartContractPermissioningConfiguration expectedConfig = + new SmartContractPermissioningConfiguration(); + expectedConfig.setNodeSmartContractAddress( + Address.fromHexString("0x0000000000000000000000000000000000001234")); + expectedConfig.setSmartContractNodeAllowlistEnabled(true); + expectedConfig.setNodeSmartContractInterfaceVersion(1); + + parseCommand( + "--permissions-nodes-contract-enabled", + "--permissions-nodes-contract-address", + "0x0000000000000000000000000000000000001234"); + + verify(mockRunnerBuilder) + .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + final PermissioningConfiguration config = + permissioningConfigurationArgumentCaptor.getValue().get(); + assertThat(config.getSmartContractConfig().get()) + .usingRecursiveComparison() + .isEqualTo(expectedConfig); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissionsContractVersionSetsValue() { + final SmartContractPermissioningConfiguration expectedConfig = + new SmartContractPermissioningConfiguration(); + expectedConfig.setNodeSmartContractAddress( + Address.fromHexString("0x0000000000000000000000000000000000001234")); + expectedConfig.setSmartContractNodeAllowlistEnabled(true); + expectedConfig.setNodeSmartContractInterfaceVersion(2); + + parseCommand( + "--permissions-nodes-contract-enabled", + "--permissions-nodes-contract-address", + "0x0000000000000000000000000000000000001234", + "--permissions-nodes-contract-version", + "2"); + + verify(mockRunnerBuilder) + .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + final PermissioningConfiguration config = + permissioningConfigurationArgumentCaptor.getValue().get(); + assertThat(config.getSmartContractConfig().get()) + .usingRecursiveComparison() + .isEqualTo(expectedConfig); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissionsSmartContractWithoutOptionMustError() { + parseCommand("--permissions-accounts-contract-address"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)) + .startsWith( + "Missing required parameter for option '--permissions-accounts-contract-address'"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissionsEnabledWithoutContractAddressMustError() { + parseCommand("--permissions-accounts-contract-enabled"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)) + .contains("No account permissioning contract address specified"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissionsEnabledWithInvalidContractAddressMustError() { + parseCommand( + "--permissions-accounts-contract-enabled", + "--permissions-accounts-contract-address", + "invalid-smart-contract-address"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissionsEnabledWithTooShortContractAddressMustError() { + parseCommand( + "--permissions-accounts-contract-enabled", + "--permissions-accounts-contract-address", + "0x1234"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)).contains("Invalid value"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissionsSmartContractMustUseOption() { + final String smartContractAddress = "0x0000000000000000000000000000000000001234"; + + parseCommand( + "--permissions-accounts-contract-enabled", + "--permissions-accounts-contract-address", + smartContractAddress); + final SmartContractPermissioningConfiguration smartContractPermissioningConfiguration = + new SmartContractPermissioningConfiguration(); + smartContractPermissioningConfiguration.setAccountSmartContractAddress( + Address.fromHexString(smartContractAddress)); + smartContractPermissioningConfiguration.setSmartContractAccountAllowlistEnabled(true); + + verify(mockRunnerBuilder) + .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); + final PermissioningConfiguration permissioningConfiguration = + permissioningConfigurationArgumentCaptor.getValue().get(); + assertThat(permissioningConfiguration.getSmartContractConfig()).isPresent(); + + final SmartContractPermissioningConfiguration effectiveSmartContractConfig = + permissioningConfiguration.getSmartContractConfig().get(); + assertThat(effectiveSmartContractConfig.isSmartContractAccountAllowlistEnabled()).isTrue(); + assertThat(effectiveSmartContractConfig.getAccountSmartContractAddress()) + .isEqualTo(Address.fromHexString(smartContractAddress)); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissioningTomlPathWithoutOptionMustDisplayUsage() { + parseCommand("--permissions-nodes-config-file"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)) + .startsWith("Missing required parameter for option '--permissions-nodes-config-file'"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissioningTomlPathWithoutOptionMustDisplayUsage() { + parseCommand("--permissions-accounts-config-file"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)) + .startsWith("Missing required parameter for option '--permissions-accounts-config-file'"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissioningEnabledWithNonexistentConfigFileMustError() { + parseCommand( + "--permissions-nodes-config-file-enabled", + "--permissions-nodes-config-file", + "file-does-not-exist"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)).contains("Configuration file does not exist"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissioningEnabledWithNonexistentConfigFileMustError() { + parseCommand( + "--permissions-accounts-config-file-enabled", + "--permissions-accounts-config-file", + "file-does-not-exist"); + + Mockito.verifyNoInteractions(mockRunnerBuilder); + + assertThat(commandErrorOutput.toString(UTF_8)).contains("Configuration file does not exist"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissioningTomlFileWithNoPermissionsEnabledMustNotError() throws IOException { + + final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); + final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); + parseCommand("--permissions-nodes-config-file", permToml.toString()); + + verify(mockRunnerBuilder).build(); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissioningTomlFileWithNoPermissionsEnabledMustNotError() + throws IOException { + + final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); + final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); + parseCommand("--permissions-accounts-config-file", permToml.toString()); + + verify(mockRunnerBuilder).build(); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void defaultPermissionsTomlFileWithNoPermissionsEnabledMustNotError() { + parseCommand("--p2p-enabled", "false"); + + verify(mockRunnerBuilder).build(); + + assertThat(commandErrorOutput.toString(UTF_8)).doesNotContain("no permissions enabled"); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void nodePermissioningTomlPathMustUseOption() throws IOException { + final List allowedNodes = + Lists.newArrayList( + EnodeURLImpl.fromString( + "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.9:4567"), + EnodeURLImpl.fromString( + "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.169.0.9:4568")); + + final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); + final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); + + final String allowedNodesString = + allowedNodes.stream().map(Object::toString).collect(Collectors.joining(",")); + parseCommand( + "--permissions-nodes-config-file-enabled", + "--permissions-nodes-config-file", + permToml.toString(), + "--bootnodes", + allowedNodesString); + final LocalPermissioningConfiguration localPermissioningConfiguration = + LocalPermissioningConfiguration.createDefault(); + localPermissioningConfiguration.setNodePermissioningConfigFilePath(permToml.toString()); + localPermissioningConfiguration.setNodeAllowlist(allowedNodes); + + verify(mockRunnerBuilder) + .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); + verify(mockRunnerBuilder).build(); + + final PermissioningConfiguration config = + permissioningConfigurationArgumentCaptor.getValue().get(); + assertThat(config.getLocalConfig().get()) + .usingRecursiveComparison() + .isEqualTo(localPermissioningConfiguration); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void accountPermissioningTomlPathMustUseOption() throws IOException { + + final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG_TOML); + final Path permToml = createTempFile("toml", Resources.toByteArray(configFile)); + + parseCommand( + "--permissions-accounts-config-file-enabled", + "--permissions-accounts-config-file", + permToml.toString()); + final LocalPermissioningConfiguration localPermissioningConfiguration = + LocalPermissioningConfiguration.createDefault(); + localPermissioningConfiguration.setAccountPermissioningConfigFilePath(permToml.toString()); + localPermissioningConfiguration.setAccountAllowlist( + Collections.singletonList("0x0000000000000000000000000000000000000009")); + + verify(mockRunnerBuilder) + .permissioningConfiguration(permissioningConfigurationArgumentCaptor.capture()); + final PermissioningConfiguration permissioningConfiguration = + permissioningConfigurationArgumentCaptor.getValue().get(); + assertThat(permissioningConfiguration.getLocalConfig()).isPresent(); + + final LocalPermissioningConfiguration effectiveLocalPermissioningConfig = + permissioningConfiguration.getLocalConfig().get(); + assertThat(effectiveLocalPermissioningConfig.isAccountAllowlistEnabled()).isTrue(); + assertThat(effectiveLocalPermissioningConfig.getAccountPermissioningConfigFilePath()) + .isEqualTo(permToml.toString()); + + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + } +} From 7afe3a9ae30bd165c83d7234fce5367bdf943e46 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Tue, 30 Jan 2024 12:50:50 +1100 Subject: [PATCH 5/5] Remove `--engine-jwt-enabled` deprecated option (#6491) Signed-off-by: Gabriel-Trintinalia --- CHANGELOG.md | 4 +++- .../main/java/org/hyperledger/besu/cli/BesuCommand.java | 7 ------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba9a4e7a2c..33919806b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,9 @@ ### Breaking Changes - The `trace-filter` method in JSON-RPC API now has a default block range limit of 1000, adjustable with `--rpc-max-trace-filter-range` (thanks @alyokaz) [#6446](https://github.com/hyperledger/besu/pull/6446) - Requesting the Ethereum Node Record (ENR) to acquire the fork id from bonded peers is now enabled by default, so the following change has been made [#5628](https://github.com/hyperledger/besu/pull/5628): - - `--Xfilter-on-enr-fork-id` has been removed. To disable the feature use `--filter-on-enr-fork-id=false`. +- `--Xfilter-on-enr-fork-id` has been removed. To disable the feature use `--filter-on-enr-fork-id=false`. +- `--engine-jwt-enabled` has been removed. Use `--engine-jwt-disabled` instead. [#6491](https://github.com/hyperledger/besu/pull/6491) + ### Deprecations 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 8e59b4c23d..7cbb60f13c 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -610,13 +610,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable { description = "Path to file containing shared secret key for JWT signature verification") private final Path engineJwtKeyFile = null; - @Option( - names = {"--engine-jwt-enabled"}, - description = "deprecated option, engine jwt auth is enabled by default", - hidden = true) - @SuppressWarnings({"FieldCanBeFinal", "UnusedVariable"}) - private final Boolean deprecatedIsEngineAuthEnabled = true; - @Option( names = {"--engine-jwt-disabled"}, description = "Disable authentication for Engine APIs (default: ${DEFAULT-VALUE})")