[PAN-2573] include static nodes in permissioning logic (#1339)

* combine bootnodes and staticNodes and pass the combined collection when building permissioning config; renamed error code that specifically called out bootnodes
PegaSysEng · Apr 28, 2019 · a6b0215 · a6b0215
1 parent b577b36
commit a6b0215
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 30 deletions.
diff --git a/...antheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelist.java b/...antheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelist.java
@@ -70,9 +70,9 @@ public JsonRpcResponse response(final JsonRpcRequest req) {
               return new JsonRpcErrorResponse(req.getId(), JsonRpcError.WHITELIST_PERSIST_FAILURE);
             case ERROR_WHITELIST_FILE_SYNC:
               return new JsonRpcErrorResponse(req.getId(), JsonRpcError.WHITELIST_FILE_SYNC);
-            case ERROR_BOOTNODE_CANNOT_BE_REMOVED:
+            case ERROR_FIXED_NODE_CANNOT_BE_REMOVED:
               return new JsonRpcErrorResponse(
-                  req.getId(), JsonRpcError.NODE_WHITELIST_BOOTNODE_CANNOT_BE_REMOVED);
+                  req.getId(), JsonRpcError.NODE_WHITELIST_FIXED_NODE_CANNOT_BE_REMOVED);
             default:
               throw new Exception();
           }

diff --git a/.../src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java b/.../src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java
@@ -70,7 +70,8 @@ public enum JsonRpcError {
   NODE_WHITELIST_DUPLICATED_ENTRY(-32000, "Request contains duplicate nodes"),
   NODE_WHITELIST_EXISTING_ENTRY(-32000, "Cannot add an existing node to whitelist"),
   NODE_WHITELIST_MISSING_ENTRY(-32000, "Cannot remove an absent node from whitelist"),
-  NODE_WHITELIST_BOOTNODE_CANNOT_BE_REMOVED(-32000, "Cannot remove a bootnode from whitelist"),
+  NODE_WHITELIST_FIXED_NODE_CANNOT_BE_REMOVED(
+      -32000, "Cannot remove a fixed node (bootnode or static node) from whitelist"),
 
   // Permissioning/persistence errors
   WHITELIST_PERSIST_FAILURE(

diff --git a/...eon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelistTest.java b/...eon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelistTest.java
@@ -174,11 +174,11 @@ public void shouldReturnCantRemoveBootnodeWhenRemovingBootnode() {
     final JsonRpcRequest request = buildRequest(Lists.newArrayList(enode1));
     final JsonRpcResponse expected =
         new JsonRpcErrorResponse(
-            request.getId(), JsonRpcError.NODE_WHITELIST_BOOTNODE_CANNOT_BE_REMOVED);
+            request.getId(), JsonRpcError.NODE_WHITELIST_FIXED_NODE_CANNOT_BE_REMOVED);
 
     when(nodeLocalConfigPermissioningController.removeNodes(any()))
         .thenReturn(
-            new NodesWhitelistResult(WhitelistOperationResult.ERROR_BOOTNODE_CANNOT_BE_REMOVED));
+            new NodesWhitelistResult(WhitelistOperationResult.ERROR_FIXED_NODE_CANNOT_BE_REMOVED));
 
     final JsonRpcResponse actual = method.response(request);
 

diff --git a/.../tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java b/.../tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java
@@ -37,7 +37,7 @@ public class NodeLocalConfigPermissioningController implements NodePermissioning
   private static final Logger LOG = LogManager.getLogger();
 
   private LocalPermissioningConfiguration configuration;
-  private final List<EnodeURL> bootnodes;
+  private final List<EnodeURL> fixedNodes;
   private final EnodeURL selfEnode;
   private final List<EnodeURL> nodesWhitelist = new ArrayList<>();
   private final WhitelistPersistor whitelistPersistor;
@@ -46,22 +46,22 @@ public class NodeLocalConfigPermissioningController implements NodePermissioning
 
   public NodeLocalConfigPermissioningController(
       final LocalPermissioningConfiguration permissioningConfiguration,
-      final List<EnodeURL> bootnodes,
+      final List<EnodeURL> fixedNodes,
       final EnodeURL selfEnode) {
     this(
         permissioningConfiguration,
-        bootnodes,
+        fixedNodes,
         selfEnode,
         new WhitelistPersistor(permissioningConfiguration.getNodePermissioningConfigFilePath()));
   }
 
   public NodeLocalConfigPermissioningController(
       final LocalPermissioningConfiguration configuration,
-      final List<EnodeURL> bootnodes,
+      final List<EnodeURL> fixedNodes,
       final EnodeURL selfEnode,
       final WhitelistPersistor whitelistPersistor) {
     this.configuration = configuration;
-    this.bootnodes = bootnodes;
+    this.fixedNodes = fixedNodes;
     this.selfEnode = selfEnode;
     this.whitelistPersistor = whitelistPersistor;
     readNodesFromConfig(configuration);
@@ -115,9 +115,9 @@ public NodesWhitelistResult removeNodes(final List<String> enodeURLs) {
     final List<EnodeURL> peers =
         enodeURLs.stream().map(EnodeURL::fromString).collect(Collectors.toList());
 
-    boolean anyBootnode = peers.stream().anyMatch(bootnodes::contains);
+    boolean anyBootnode = peers.stream().anyMatch(fixedNodes::contains);
     if (anyBootnode) {
-      return new NodesWhitelistResult(WhitelistOperationResult.ERROR_BOOTNODE_CANNOT_BE_REMOVED);
+      return new NodesWhitelistResult(WhitelistOperationResult.ERROR_FIXED_NODE_CANNOT_BE_REMOVED);
     }
 
     for (EnodeURL peer : peers) {

diff --git a/...java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java b/...java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java
@@ -29,7 +29,7 @@ public class NodePermissioningControllerFactory {
   public NodePermissioningController create(
       final PermissioningConfiguration permissioningConfiguration,
       final Synchronizer synchronizer,
-      final Collection<EnodeURL> bootnodes,
+      final Collection<EnodeURL> fixedNodes,
       final EnodeURL selfEnode,
       final TransactionSimulator transactionSimulator) {
 
@@ -42,7 +42,7 @@ public NodePermissioningController create(
       if (localPermissioningConfiguration.isNodeWhitelistEnabled()) {
         NodeLocalConfigPermissioningController localProvider =
             new NodeLocalConfigPermissioningController(
-                localPermissioningConfiguration, new ArrayList<>(bootnodes), selfEnode);
+                localPermissioningConfiguration, new ArrayList<>(fixedNodes), selfEnode);
         providers.add(localProvider);
       }
     }
@@ -59,7 +59,7 @@ public NodePermissioningController create(
       }
 
       final SyncStatusNodePermissioningProvider syncStatusProvider =
-          new SyncStatusNodePermissioningProvider(synchronizer, bootnodes);
+          new SyncStatusNodePermissioningProvider(synchronizer, fixedNodes);
       syncStatusProviderOptional = Optional.of(syncStatusProvider);
     } else {
       syncStatusProviderOptional = Optional.empty();

diff --git a/.../src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistOperationResult.java b/.../src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistOperationResult.java
@@ -19,7 +19,7 @@ public enum WhitelistOperationResult {
   ERROR_EXISTING_ENTRY,
   ERROR_INVALID_ENTRY,
   ERROR_ABSENT_ENTRY,
-  ERROR_BOOTNODE_CANNOT_BE_REMOVED,
+  ERROR_FIXED_NODE_CANNOT_BE_REMOVED,
   ERROR_WHITELIST_PERSIST_FAIL,
   ERROR_WHITELIST_FILE_SYNC
 }
diff --git a/...ys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java b/...ys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java
@@ -27,18 +27,18 @@
 public class SyncStatusNodePermissioningProvider implements NodePermissioningProvider {
 
   private final Synchronizer synchronizer;
-  private final Collection<EnodeURL> bootnodes = new HashSet<>();
+  private final Collection<EnodeURL> fixedNodes = new HashSet<>();
   private OptionalLong syncStatusObserverId;
   private boolean hasReachedSync = false;
   private Optional<Runnable> hasReachedSyncCallback = Optional.empty();
 
   public SyncStatusNodePermissioningProvider(
-      final Synchronizer synchronizer, final Collection<EnodeURL> bootnodes) {
+      final Synchronizer synchronizer, final Collection<EnodeURL> fixedNodes) {
     checkNotNull(synchronizer);
     this.synchronizer = synchronizer;
     long id = this.synchronizer.observeSyncStatus(this::handleSyncStatusUpdate);
     this.syncStatusObserverId = OptionalLong.of(id);
-    this.bootnodes.addAll(bootnodes);
+    this.fixedNodes.addAll(fixedNodes);
   }
 
   private void handleSyncStatusUpdate(final SyncStatus syncStatus) {
@@ -74,7 +74,7 @@ private synchronized void runCallback() {
   }
 
   /**
-   * Before reaching a sync'd state, the node will only be allowed to talk to its bootnodes
+   * Before reaching a sync'd state, the node will only be allowed to talk to its fixedNodes
    * (outgoing connections). After reaching a sync'd state, it is expected that other providers will
    * check the permissions (most likely the smart contract based provider). That's why we always
    * return true after reaching a sync'd state.
@@ -89,7 +89,7 @@ public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinatio
     if (hasReachedSync) {
       return true;
     } else {
-      return bootnodes.contains(destinationEnode);
+      return fixedNodes.contains(destinationEnode);
     }
   }
 

diff --git a/...h/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java b/...h/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java
@@ -353,7 +353,7 @@ public void whenRemovingNodeDoesNotRemoveShouldNotifyWhitelistModifiedSubscriber
   @Test
   public void whenRemovingBootnodeShouldReturnRemoveBootnodeError() {
     NodesWhitelistResult expected =
-        new NodesWhitelistResult(WhitelistOperationResult.ERROR_BOOTNODE_CANNOT_BE_REMOVED);
+        new NodesWhitelistResult(WhitelistOperationResult.ERROR_FIXED_NODE_CANNOT_BE_REMOVED);
     bootnodesList.add(EnodeURL.fromString(enode1));
     controller.addNodes(Lists.newArrayList(enode1, enode2));
 

diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java
@@ -73,6 +73,7 @@
 
 import java.net.URI;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -81,6 +82,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import io.vertx.core.Vertx;
 
@@ -408,15 +410,19 @@ private Optional<NodePermissioningController> buildNodePermissioningController(
       final List<EnodeURL> bootnodesAsEnodeURLs,
       final Synchronizer synchronizer,
       final TransactionSimulator transactionSimulator) {
+    Collection<EnodeURL> fixedNodes = getFixedNodes(bootnodesAsEnodeURLs, staticNodes);
     return permissioningConfiguration.map(
         config ->
             new NodePermissioningControllerFactory()
-                .create(
-                    config,
-                    synchronizer,
-                    bootnodesAsEnodeURLs,
-                    getSelfEnode(),
-                    transactionSimulator));
+                .create(config, synchronizer, fixedNodes, getSelfEnode(), transactionSimulator));
+  }
+
+  @VisibleForTesting
+  public static Collection<EnodeURL> getFixedNodes(
+      final Collection<EnodeURL> someFixedNodes, final Collection<EnodeURL> moreFixedNodes) {
+    Collection<EnodeURL> fixedNodes = new ArrayList<>(someFixedNodes);
+    fixedNodes.addAll(moreFixedNodes);
+    return fixedNodes;
   }
 
   private FilterManager createFilterManager(

diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java
@@ -1219,8 +1219,8 @@ public PantheonExceptionHandler exceptionHandler() {
   }
 
   private Set<EnodeURL> loadStaticNodes() throws IOException {
-    final String staticNodesFilname = "static-nodes.json";
-    final Path staticNodesPath = dataDir().resolve(staticNodesFilname);
+    final String staticNodesFilename = "static-nodes.json";
+    final Path staticNodesPath = dataDir().resolve(staticNodesFilename);
 
     return StaticNodesParser.fromPath(staticNodesPath);
   }

diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java
@@ -52,6 +52,8 @@
 import java.io.IOException;
 import java.net.InetAddress;
 import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -78,6 +80,24 @@ public final class RunnerTest {
 
   @Rule public final TemporaryFolder temp = new TemporaryFolder();
 
+  @Test
+  public void getFixedNodes() {
+    EnodeURL staticNode =
+        EnodeURL.fromString(
+            "enode://8f4b88336cc40ef2516d8b27df812e007fb2384a61e93635f1899051311344f3dcdbb49a4fe49a79f66d2f589a9f282e8cc4f1d7381e8ef7e4fcc6b0db578c77@127.0.0.1:30301");
+    EnodeURL bootnode =
+        EnodeURL.fromString(
+            "enode://8f4b88336cc40ef2516d8b27df812e007fb2384a61e93635f1899051311344f3dcdbb49a4fe49a79f66d2f589a9f282e8cc4f1d7381e8ef7e4fcc6b0db578c77@127.0.0.1:30302");
+    final List<EnodeURL> bootnodes = new ArrayList<EnodeURL>();
+    bootnodes.add(bootnode);
+    Collection<EnodeURL> staticNodes = new ArrayList<EnodeURL>();
+    staticNodes.add(staticNode);
+    Collection<EnodeURL> fixedNodes = RunnerBuilder.getFixedNodes(bootnodes, staticNodes);
+    assertThat(fixedNodes).containsExactlyInAnyOrder(staticNode, bootnode);
+    // bootnodes should be unchanged
+    assertThat(bootnodes).containsExactly(bootnode);
+  }
+
   @Test
   public void fullSyncFromGenesis() throws Exception {
     syncFromGenesis(SyncMode.FULL);