Sync status permissioning check conditional on onchain permissioning enabled (#1269)

* only check sync status if onchain node permissioning config is enabled

Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>

ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/NodePermissioningControllerFactory.java

@@ -43,7 +43,7 @@ public class NodePermissioningControllerFactory {
       final TransactionSimulator transactionSimulator,
       final MetricsSystem metricsSystem) {
 
-    Optional<SyncStatusNodePermissioningProvider> syncStatusProviderOptional;
+    final Optional<SyncStatusNodePermissioningProvider> syncStatusProviderOptional;
 
     List<NodePermissioningProvider> providers = new ArrayList<>();
     if (permissioningConfiguration.getLocalConfig().isPresent()) {
@@ -60,17 +60,20 @@ public class NodePermissioningControllerFactory {
       }
     }
 
-    if (permissioningConfiguration.getSmartContractConfig().isPresent()) {
-      SmartContractPermissioningConfiguration smartContractPermissioningConfiguration =
-          permissioningConfiguration.getSmartContractConfig().get();
-      if (smartContractPermissioningConfiguration.isSmartContractNodeAllowlistEnabled()) {
-        NodeSmartContractPermissioningController smartContractProvider =
-            new NodeSmartContractPermissioningController(
-                smartContractPermissioningConfiguration.getNodeSmartContractAddress(),
-                transactionSimulator,
-                metricsSystem);
-        providers.add(smartContractProvider);
-      }
+    if (permissioningConfiguration.getSmartContractConfig().isPresent()
+        && permissioningConfiguration
+            .getSmartContractConfig()
+            .get()
+            .isSmartContractNodeAllowlistEnabled()) {
+      NodeSmartContractPermissioningController smartContractProvider =
+          new NodeSmartContractPermissioningController(
+              permissioningConfiguration
+                  .getSmartContractConfig()
+                  .get()
+                  .getNodeSmartContractAddress(),
+              transactionSimulator,
+              metricsSystem);
+      providers.add(smartContractProvider);
 
       if (fixedNodes.isEmpty()) {
         syncStatusProviderOptional = Optional.empty();
@@ -86,7 +89,14 @@ public class NodePermissioningControllerFactory {
     NodePermissioningController nodePermissioningController =
         new NodePermissioningController(syncStatusProviderOptional, providers);
 
-    validatePermissioningContract(nodePermissioningController);
+    permissioningConfiguration
+        .getSmartContractConfig()
+        .ifPresent(
+            config -> {
+              if (config.isSmartContractNodeAllowlistEnabled()) {
+                validatePermissioningContract(nodePermissioningController);
+              }
+            });
 
     return nodePermissioningController;
   }

ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/node/InsufficientPeersPermissioningProvider.java

@@ -37,7 +37,7 @@ public class InsufficientPeersPermissioningProvider implements ContextualNodePer
    * Creates the provider observing the provided p2p network
    *
    * @param p2pNetwork the p2p network to observe
-   * @param bootnodeEnodes the bootnodes that this node is configured to connection to
+   * @param bootnodeEnodes the bootnodes that this node is configured to connect to
    */
   public InsufficientPeersPermissioningProvider(
       final P2PNetwork p2pNetwork, final Collection<EnodeURL> bootnodeEnodes) {

ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java

@@ -127,6 +127,40 @@ public class NodePermissioningControllerFactoryTest {
     assertThat(controller.getSyncStatusNodePermissioningProvider()).isNotPresent();
   }
 
+  @Test
+  public void
+      testCreateWithLocalNodePermissioningEnabledAndSmartContractPresentButDisabledAndBootnode() {
+    smartContractPermissioningConfiguration = new SmartContractPermissioningConfiguration();
+    smartContractPermissioningConfiguration.setNodeSmartContractAddress(
+        Address.fromHexString("0x0000000000000000000000000000000000001234"));
+    smartContractPermissioningConfiguration.setSmartContractNodeAllowlistEnabled(false);
+    final Collection<EnodeURL> fixedNodes = Collections.singleton(selfEnode);
+    localPermissioningConfig = LocalPermissioningConfiguration.createDefault();
+    localPermissioningConfig.setNodeAllowlist(Collections.emptyList());
+    localPermissioningConfig.setNodePermissioningConfigFilePath("fake-file-path");
+    config =
+        new PermissioningConfiguration(
+            Optional.of(localPermissioningConfig),
+            Optional.of(smartContractPermissioningConfiguration));
+
+    NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory();
+    NodePermissioningController controller =
+        factory.create(
+            config,
+            synchronizer,
+            fixedNodes,
+            selfEnode.getNodeId(),
+            transactionSimulator,
+            new NoOpMetricsSystem());
+
+    List<NodePermissioningProvider> providers = controller.getProviders();
+    assertThat(providers.size()).isEqualTo(1);
+
+    NodePermissioningProvider p1 = providers.get(0);
+    assertThat(p1).isInstanceOf(NodeLocalConfigPermissioningController.class);
+    assertThat(controller.getSyncStatusNodePermissioningProvider()).isNotPresent();
+  }
+
   @Test
   public void testCreateWithLocalNodeAndSmartContractPermissioningEnabled() {
     localPermissioningConfig = LocalPermissioningConfiguration.createDefault();