[PAN-2494] [PAN-2474] permissioning issues

ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java

@@ -207,25 +207,32 @@ private Collection<String> peerToEnodeURI(final Collection<EnodeURL> peers) {
     return peers.parallelStream().map(EnodeURL::toString).collect(Collectors.toList());
   }
 
+  private boolean checkSelfEnode(final EnodeURL node) {
+    return selfEnode.getNodeId().equals(node.getNodeId());
+  }
+
+  private boolean compareEnodes(final EnodeURL nodeA, final EnodeURL nodeB) {
+    boolean idsMatch = nodeA.getNodeId().equals(nodeB.getNodeId());
+    boolean hostsMatch = nodeA.getIp().equals(nodeB.getIp());
+    boolean listeningPortsMatch = nodeA.getListeningPort().equals(nodeB.getListeningPort());
+    boolean discoveryPortsMatch = true;
+    if (nodeA.getDiscoveryPort().isPresent() && nodeB.getDiscoveryPort().isPresent()) {
+      discoveryPortsMatch =
+          nodeA.getDiscoveryPort().getAsInt() == nodeB.getDiscoveryPort().getAsInt();
+    }
+
+    return idsMatch && hostsMatch && listeningPortsMatch && discoveryPortsMatch;
+  }
+
   public boolean isPermitted(final String enodeURL) {
     return isPermitted(new EnodeURL(enodeURL));
   }
 
   public boolean isPermitted(final EnodeURL node) {
-    return nodesWhitelist.stream()
-        .anyMatch(
-            p -> {
-              boolean idsMatch = node.getNodeId().equals(p.getNodeId());
-              boolean hostsMatch = node.getIp().equals(p.getIp());
-              boolean listeningPortsMatch = node.getListeningPort().equals(p.getListeningPort());
-              boolean discoveryPortsMatch = true;
-              if (node.getDiscoveryPort().isPresent() && p.getDiscoveryPort().isPresent()) {
-                discoveryPortsMatch =
-                    node.getDiscoveryPort().getAsInt() == p.getDiscoveryPort().getAsInt();
-              }
-
-              return idsMatch && hostsMatch && listeningPortsMatch && discoveryPortsMatch;
-            });
+    if (checkSelfEnode(node)) {
+      return true;
+    }
+    return nodesWhitelist.stream().anyMatch(p -> compareEnodes(p, node));
   }
 
   public List<String> getNodesWhitelist() {
@@ -315,6 +322,6 @@ public Optional<String> message() {
 
   @Override
   public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinationEnode) {
-    return isPermitted(sourceEnode) || isPermitted(destinationEnode);
+    return isPermitted(sourceEnode) && isPermitted(destinationEnode);
   }
 }

ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodePermissioningController.java

@@ -39,6 +39,12 @@ public NodePermissioningController(
   public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinationEnode) {
     LOG.trace("Checking node permission: {} -> {}", sourceEnode, destinationEnode);
 
+    if (syncStatusNodePermissioningProvider
+        .map(p -> !p.hasReachedSync() && p.isPermitted(sourceEnode, destinationEnode))
+        .orElse(false)) {
+      return true;
+    }
+
     if (syncStatusNodePermissioningProvider.isPresent()
         && !syncStatusNodePermissioningProvider.get().isPermitted(sourceEnode, destinationEnode)) {
       return false;

ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java

@@ -24,8 +24,6 @@
 import java.util.Optional;
 import java.util.OptionalLong;
 
-import com.google.common.annotations.VisibleForTesting;
-
 public class SyncStatusNodePermissioningProvider implements NodePermissioningProvider {
 
   private final Synchronizer synchronizer;
@@ -95,8 +93,7 @@ public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinatio
     }
   }
 
-  @VisibleForTesting
-  boolean hasReachedSync() {
+  public boolean hasReachedSync() {
     return hasReachedSync;
   }
 }

ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java

@@ -232,8 +232,6 @@ public void whenCheckingIfNodeIsPermittedOrderDoesNotMatter() {
     controller.addNodes(Arrays.asList(enode1));
     assertThat(controller.isPermitted(new EnodeURL(enode1), new EnodeURL(selfEnode))).isTrue();
     assertThat(controller.isPermitted(new EnodeURL(selfEnode), new EnodeURL(enode1))).isTrue();
-
-    assertThat(controller.isPermitted(new EnodeURL(selfEnode), new EnodeURL(selfEnode))).isFalse();
   }
 
   @Test

ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodePermissioningControllerTest.java

@@ -15,6 +15,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -64,7 +65,7 @@ public void before() {
   public void isPermittedShouldDelegateToSyncStatusProvider() {
     controller.isPermitted(enode1, enode2);
 
-    verify(syncStatusNodePermissioningProvider).isPermitted(eq(enode1), eq(enode2));
+    verify(syncStatusNodePermissioningProvider, atLeast(1)).isPermitted(eq(enode1), eq(enode2));
   }
 
   @Test
@@ -103,6 +104,7 @@ public void whenNoSyncStatusProviderWeShouldDelegateToLocalConfigNodePermissioni
         new NodePermissioningController(syncStatusNodePermissioningProviderOptional, providers);
 
     when(syncStatusNodePermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(true);
+    when(syncStatusNodePermissioningProvider.hasReachedSync()).thenReturn(true);
     when(localConfigNodePermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(true);
     when(otherPermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(true);
 
@@ -129,6 +131,7 @@ private List<NodePermissioningProvider> getNodePermissioningProviders() {
         new NodePermissioningController(syncStatusNodePermissioningProviderOptional, providers);
 
     when(syncStatusNodePermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(true);
+    when(syncStatusNodePermissioningProvider.hasReachedSync()).thenReturn(true);
     when(localConfigNodePermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(true);
     when(otherPermissioningProvider.isPermitted(eq(enode1), eq(enode2))).thenReturn(false);
 

util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java

@@ -64,7 +64,11 @@ public EnodeURL(
       final InetAddress address,
       final Integer listeningPort,
       final OptionalInt discoveryPort) {
-    this.nodeId = nodeId;
+    if (nodeId.startsWith("0x")) {
+      this.nodeId = nodeId.substring(2);
+    } else {
+      this.nodeId = nodeId;
+    }
     this.ip = address;
     this.listeningPort = listeningPort;
     this.discoveryPort = discoveryPort;