From f0606eff90b790773d762a94703870f6fa392ff7 Mon Sep 17 00:00:00 2001 From: Matthew Whitehead Date: Tue, 25 Jun 2024 09:19:49 +0100 Subject: [PATCH 1/6] Add bootnodes to the maintained peer list Signed-off-by: Matthew Whitehead --- besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 1f9c48cccc5..fcb5ee637af 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -790,7 +790,8 @@ public Runner build() { LOG.debug("added ethash observer: {}", stratumServer.get()); } - sanitizePeers(network, staticNodes) + // Make sure Besu maintains connections to static nodes and bootnodes, including retries periodically + Stream.concat(sanitizePeers(network, staticNodes), sanitizePeers(network, bootnodes)) .map(DefaultPeer::fromEnodeURL) .forEach(peerNetwork::addMaintainedConnectionPeer); From a1aaad691d154617b86ca62d0c7bd034b665a3b7 Mon Sep 17 00:00:00 2001 From: Matthew Whitehead Date: Tue, 25 Jun 2024 16:09:46 +0100 Subject: [PATCH 2/6] Update unit tests Signed-off-by: Matthew Whitehead --- ...SmartContractPermissioningAcceptanceTest.java | 4 ++-- ...artContractPermissioningV2AcceptanceTest.java | 3 --- .../java/org/hyperledger/besu/RunnerBuilder.java | 16 ++++++++++++---- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningAcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningAcceptanceTest.java index 8f17c78adde..47eda46d52f 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningAcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningAcceptanceTest.java @@ -37,9 +37,10 @@ public void setUp() { permissionedCluster.start(bootnode, forbiddenNode, allowedNode, permissionedNode); // updating permissioning smart contract with allowed nodes - + permissionedNode.verify(nodeIsForbidden(bootnode)); permissionedNode.execute(allowNode(bootnode)); permissionedNode.verify(nodeIsAllowed(bootnode)); + permissionedNode.verify(admin.hasPeer(bootnode)); permissionedNode.execute(allowNode(allowedNode)); permissionedNode.verify(nodeIsAllowed(allowedNode)); @@ -47,7 +48,6 @@ public void setUp() { permissionedNode.execute(allowNode(permissionedNode)); permissionedNode.verify(nodeIsAllowed(permissionedNode)); - permissionedNode.verify(admin.addPeer(bootnode)); permissionedNode.verify(admin.addPeer(allowedNode)); allowedNode.verify(eth.syncingStatus(false)); diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java index 843d14004fc..6ee9f3f7596 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java @@ -60,7 +60,6 @@ public void permissionedNodeShouldPeerOnlyWithAllowedNodes() { @Test public void permissionedNodeShouldDisconnectFromNodeNotPermittedAnymore() { - permissionedNode.verify(admin.addPeer(bootnode)); permissionedNode.verify(admin.addPeer(allowedNode)); permissionedNode.verify(net.awaitPeerCount(2)); @@ -72,7 +71,6 @@ public void permissionedNodeShouldDisconnectFromNodeNotPermittedAnymore() { @Test public void permissionedNodeShouldConnectToNewlyPermittedNode() { - permissionedNode.verify(admin.addPeer(bootnode)); permissionedNode.verify(admin.addPeer(allowedNode)); permissionedNode.verify(net.awaitPeerCount(2)); @@ -87,7 +85,6 @@ public void permissionedNodeShouldConnectToNewlyPermittedNode() { @Test public void permissioningUpdatesPropagateThroughNetwork() { - permissionedNode.verify(admin.addPeer(bootnode)); permissionedNode.verify(admin.addPeer(allowedNode)); permissionedNode.verify(net.awaitPeerCount(2)); diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index fcb5ee637af..8aa1a0de9d7 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -790,10 +790,18 @@ public Runner build() { LOG.debug("added ethash observer: {}", stratumServer.get()); } - // Make sure Besu maintains connections to static nodes and bootnodes, including retries periodically - Stream.concat(sanitizePeers(network, staticNodes), sanitizePeers(network, bootnodes)) - .map(DefaultPeer::fromEnodeURL) - .forEach(peerNetwork::addMaintainedConnectionPeer); + if (besuController.getGenesisConfigOptions().isPoa()) { + // In a permissioned chain Besu should maintain connections to both static nodes and + // bootnodes, which includes retries periodically + Stream.concat(sanitizePeers(network, staticNodes), sanitizePeers(network, bootnodes)) + .map(DefaultPeer::fromEnodeURL) + .forEach(peerNetwork::addMaintainedConnectionPeer); + } else { + // In a public chain only maintain connections to static nodes + sanitizePeers(network, staticNodes) + .map(DefaultPeer::fromEnodeURL) + .forEach(peerNetwork::addMaintainedConnectionPeer); + } final Optional nodeLocalConfigPermissioningController = nodePermissioningController.flatMap(NodePermissioningController::localConfigController); From 6a24607c97a2d9da2eb9e0353de5f18f254b2417 Mon Sep 17 00:00:00 2001 From: Matthew Whitehead Date: Tue, 25 Jun 2024 17:05:43 +0100 Subject: [PATCH 3/6] Add entry in changelog Signed-off-by: Matthew Whitehead --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07cdf586349..4eb0b5166d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Update Docker base image to Ubuntu 24.04 [#7251](https://github.com/hyperledger/besu/pull/7251) - Add LUKSO as predefined network name [#7223](https://github.com/hyperledger/besu/pull/7223) - Refactored how code, initcode, and max stack size are configured in forks. [#7245](https://github.com/hyperledger/besu/pull/7245) +- Nodes in a permissioned chain maintain (and retry) connections to bootnodes [#7257](https://github.com/hyperledger/besu/pull/7257) ### Bug fixes - Validation errors ignored in accounts-allowlist and empty list [#7138](https://github.com/hyperledger/besu/issues/7138) From e57be5816d7ea1230a3c90e865514a71b7623826 Mon Sep 17 00:00:00 2001 From: Matthew Whitehead Date: Tue, 25 Jun 2024 17:42:09 +0100 Subject: [PATCH 4/6] Tweak unit test Signed-off-by: Matthew Whitehead --- .../NodeSmartContractPermissioningV2AcceptanceTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java index 6ee9f3f7596..95256c0cd90 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java @@ -60,6 +60,7 @@ public void permissionedNodeShouldPeerOnlyWithAllowedNodes() { @Test public void permissionedNodeShouldDisconnectFromNodeNotPermittedAnymore() { + permissionedNode.verify(admin.hasPeer(bootnode)); permissionedNode.verify(admin.addPeer(allowedNode)); permissionedNode.verify(net.awaitPeerCount(2)); @@ -71,6 +72,7 @@ public void permissionedNodeShouldDisconnectFromNodeNotPermittedAnymore() { @Test public void permissionedNodeShouldConnectToNewlyPermittedNode() { + permissionedNode.verify(admin.hasPeer(bootnode)); permissionedNode.verify(admin.addPeer(allowedNode)); permissionedNode.verify(net.awaitPeerCount(2)); @@ -85,6 +87,7 @@ public void permissionedNodeShouldConnectToNewlyPermittedNode() { @Test public void permissioningUpdatesPropagateThroughNetwork() { + permissionedNode.verify(admin.hasPeer(bootnode)); permissionedNode.verify(admin.addPeer(allowedNode)); permissionedNode.verify(net.awaitPeerCount(2)); From 31711972053442d02721ba616141df0a2133290d Mon Sep 17 00:00:00 2001 From: Matthew Whitehead Date: Wed, 26 Jun 2024 08:44:55 +0100 Subject: [PATCH 5/6] Refactor to keep common steps the same for both cases Signed-off-by: Matthew Whitehead --- .../java/org/hyperledger/besu/RunnerBuilder.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 8aa1a0de9d7..2e7de9cecb8 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -790,18 +790,19 @@ public Runner build() { LOG.debug("added ethash observer: {}", stratumServer.get()); } + final Stream maintainedPeers; if (besuController.getGenesisConfigOptions().isPoa()) { // In a permissioned chain Besu should maintain connections to both static nodes and // bootnodes, which includes retries periodically - Stream.concat(sanitizePeers(network, staticNodes), sanitizePeers(network, bootnodes)) - .map(DefaultPeer::fromEnodeURL) - .forEach(peerNetwork::addMaintainedConnectionPeer); + maintainedPeers = + Stream.concat(sanitizePeers(network, staticNodes), sanitizePeers(network, bootnodes)); } else { // In a public chain only maintain connections to static nodes - sanitizePeers(network, staticNodes) - .map(DefaultPeer::fromEnodeURL) - .forEach(peerNetwork::addMaintainedConnectionPeer); + maintainedPeers = sanitizePeers(network, staticNodes); } + maintainedPeers + .map(DefaultPeer::fromEnodeURL) + .forEach(peerNetwork::addMaintainedConnectionPeer); final Optional nodeLocalConfigPermissioningController = nodePermissioningController.flatMap(NodePermissioningController::localConfigController); From ab1409a29a832ef1446ac68f340802c21eb48b16 Mon Sep 17 00:00:00 2001 From: Matthew Whitehead Date: Thu, 27 Jun 2024 11:11:16 +0100 Subject: [PATCH 6/6] Add debug log, call sanitizePeers() only once Signed-off-by: Matthew Whitehead --- besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 2e7de9cecb8..e9f9ef6268c 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -795,7 +795,10 @@ public Runner build() { // In a permissioned chain Besu should maintain connections to both static nodes and // bootnodes, which includes retries periodically maintainedPeers = - Stream.concat(sanitizePeers(network, staticNodes), sanitizePeers(network, bootnodes)); + sanitizePeers( + network, + Stream.concat(staticNodes.stream(), bootnodes.stream()).collect(Collectors.toList())); + LOG.debug("Added bootnodes to the maintained peer list"); } else { // In a public chain only maintain connections to static nodes maintainedPeers = sanitizePeers(network, staticNodes);