From 81840ff9bfd9f986619c534c31848583f85b1f8f Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 13 Feb 2024 18:59:01 +1000 Subject: [PATCH 01/26] commit building version Signed-off-by: stefan.pingel@consensys.net --- .../org/hyperledger/besu/RunnerBuilder.java | 5 +- .../controller/BesuControllerBuilder.java | 11 +- .../bft/protocol/BftProtocolManager.java | 2 +- .../besu/ethereum/eth/manager/EthPeer.java | 9 + .../besu/ethereum/eth/manager/EthPeers.java | 211 +++++++++++++++--- .../eth/manager/EthProtocolManager.java | 15 +- .../ethereum/eth/manager/PeerRequest.java | 4 + .../eth/manager/PendingPeerRequest.java | 1 + .../snap/GetAccountRangeFromPeerTask.java | 28 ++- .../manager/snap/GetBytecodeFromPeerTask.java | 18 +- .../snap/GetStorageRangeFromPeerTask.java | 30 ++- .../manager/snap/GetTrieNodeFromPeerTask.java | 18 +- .../RetryingGetAccountRangeFromPeerTask.java | 5 + .../snap/RetryingGetBytecodeFromPeerTask.java | 5 + .../RetryingGetStorageRangeFromPeerTask.java | 5 + .../snap/RetryingGetTrieNodeFromPeerTask.java | 5 + .../eth/manager/snap/SnapProtocolManager.java | 2 +- .../task/AbstractRetryingPeerTask.java | 19 +- .../AbstractRetryingSwitchingPeerTask.java | 20 +- .../ethereum/eth/sync/ChainHeadTracker.java | 80 +++---- .../manager/EthProtocolManagerTestUtil.java | 17 +- .../ethtaskutils/AbstractMessageTaskTest.java | 7 +- .../AbstractBlockPropagationManagerTest.java | 11 +- .../ethereum/eth/transactions/TestNode.java | 6 +- .../TransactionPoolFactoryTest.java | 5 +- .../ethereum/p2p/network/NetworkRunner.java | 19 +- .../ethereum/p2p/network/ProtocolManager.java | 2 +- .../ethereum/retesteth/RetestethContext.java | 7 +- 28 files changed, 426 insertions(+), 141 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 8d0a693e27f..30a164cd361 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -634,6 +634,9 @@ public Runner build() { .flatMap(protocolManager -> protocolManager.getSupportedCapabilities().stream()) .collect(Collectors.toSet()); + final EthPeers ethPeers = besuController.getEthPeers(); + ethPeers.setProtocolManagers(protocolManagers); + final RlpxConfiguration rlpxConfiguration = RlpxConfiguration.create() .setBindHost(p2pListenInterface) @@ -673,7 +676,6 @@ public Runner build() { final NetworkBuilder inactiveNetwork = caps -> new NoopP2PNetwork(); final NetworkBuilder activeNetwork = caps -> { - final EthPeers ethPeers = besuController.getEthPeers(); return DefaultP2PNetwork.builder() .vertx(vertx) .nodeKey(nodeKey) @@ -700,6 +702,7 @@ public Runner build() { .subProtocols(subProtocols) .network(p2pEnabled ? activeNetwork : inactiveNetwork) .metricsSystem(metricsSystem) + .ethPeersShouldConnect(ethPeers::shouldTryToConnect) .build(); besuController.getEthPeers().setRlpxAgent(networkRunner.getRlpxAgent()); diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 539c3d62ab2..085ac277c1d 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -74,6 +74,7 @@ import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolFactory; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; import org.hyperledger.besu.ethereum.p2p.config.NetworkingConfiguration; @@ -659,6 +660,12 @@ public BesuController build() { final int maxMessageSize = ethereumWireProtocolConfiguration.getMaxMessageSize(); final Supplier currentProtocolSpecSupplier = () -> protocolSchedule.getByBlockHeader(blockchain.getChainHeadHeader()); + final ForkIdManager forkIdManager = + new ForkIdManager( + blockchain, + genesisConfig.getForkBlockNumbers(), + genesisConfig.getForkTimestamps(), + ethereumWireProtocolConfiguration.isLegacyEth64ForkIdEnabled()); final EthPeers ethPeers = new EthPeers( getSupportedProtocol(), @@ -670,7 +677,9 @@ public BesuController build() { nodeKey.getPublicKey().getEncodedBytes(), maxPeers, maxRemotelyInitiatedPeers, - randomPeerPriority); + randomPeerPriority, + syncConfig.getSyncMode(), + forkIdManager); final EthMessages ethMessages = new EthMessages(); final EthMessages snapMessages = new EthMessages(); diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java index 0ef72f95edf..9cc4a10b0b6 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java @@ -109,7 +109,7 @@ public void handleNewConnection(final PeerConnection peerConnection) { } @Override - public boolean shouldConnect(final Peer peer, final boolean incoming) { + public boolean shouldTryToConnect(final Peer peer, final boolean incoming) { return false; // for now the EthProtocolManager takes care of this } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index 7ac7e98dd61..f1aee2fecdf 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -103,6 +103,7 @@ protected boolean removeEldestEntry(final Map.Entry eldest) { private final PeerReputation reputation = new PeerReputation(); private final Map validationStatus = new ConcurrentHashMap<>(); private final Bytes id; + private boolean isServingSnap = false; private static final Map roundMessages; @@ -393,6 +394,14 @@ public RequestManager.ResponseStream getSnapTrieNode( requestManagers.get(SnapProtocol.NAME).get(SnapV1.GET_TRIE_NODES), getTrieNodes); } + public void setIsServingSnap(final boolean isServingSnap) { + this.isServingSnap = isServingSnap; + } + + public boolean isServingSnap() { + return isServingSnap; + } + private RequestManager.ResponseStream sendRequest( final RequestManager requestManager, final MessageData messageData) throws PeerNotConnected { lastRequestTimestamp = clock.millis(); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 43189c06abc..71d7b08b5b9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -14,13 +14,21 @@ */ package org.hyperledger.besu.ethereum.eth.manager; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthPeer.DisconnectCallback; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; +import org.hyperledger.besu.ethereum.eth.sync.SyncMode; +import org.hyperledger.besu.ethereum.forkid.ForkId; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; +import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; +import org.hyperledger.besu.ethereum.p2p.network.ProtocolManager; import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.RlpxAgent; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage; +import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput; import org.hyperledger.besu.metrics.BesuMetricCategory; import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.plugin.services.metrics.Counter; @@ -35,6 +43,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Predicate; @@ -72,7 +81,7 @@ public class EthPeers { public static final int NODE_ID_LENGTH = 64; public static final int USEFULL_PEER_SCORE_THRESHOLD = 102; - private final Map completeConnections = new ConcurrentHashMap<>(); + private final Map activeConnections = new ConcurrentHashMap<>(); private final Cache incompleteConnections = CacheBuilder.newBuilder() @@ -92,12 +101,16 @@ public class EthPeers { private final Boolean randomPeerPriority; private final Bytes nodeIdMask = Bytes.random(NODE_ID_LENGTH); private final Supplier currentProtocolSpecSupplier; + private final SyncMode syncMode; + private final ForkIdManager forkIdManager; + private final int snapServerTargetNumber; private Comparator bestPeerComparator; private final Bytes localNodeId; private RlpxAgent rlpxAgent; private final Counter connectedPeersCounter; + private List protocolManagers; public EthPeers( final String protocolName, @@ -109,7 +122,9 @@ public EthPeers( final Bytes localNodeId, final int peerUpperBound, final int maxRemotelyInitiatedConnections, - final Boolean randomPeerPriority) { + final Boolean randomPeerPriority, + final SyncMode syncMode, + final ForkIdManager forkIdManager) { this.protocolName = protocolName; this.currentProtocolSpecSupplier = currentProtocolSpecSupplier; this.clock = clock; @@ -121,6 +136,14 @@ public EthPeers( this.maxRemotelyInitiatedConnections = maxRemotelyInitiatedConnections; this.randomPeerPriority = randomPeerPriority; LOG.trace("MaxPeers: {}, Max Remote: {}", peerUpperBound, maxRemotelyInitiatedConnections); + this.syncMode = syncMode; + this.forkIdManager = forkIdManager; + if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { + snapServerTargetNumber = + peerUpperBound / 2; // hardcoded for now. 50% of peers should be snap servers + } else { + snapServerTargetNumber = 0; + } metricsSystem.createIntegerGauge( BesuMetricCategory.ETHEREUM, "peer_count", @@ -146,7 +169,7 @@ public void registerNewConnection( final PeerConnection newConnection, final List peerValidators) { final Bytes id = newConnection.getPeer().getId(); synchronized (this) { - EthPeer ethPeer = completeConnections.get(id); + EthPeer ethPeer = activeConnections.get(id); if (ethPeer == null) { final Optional peerInList = incompleteConnections.asMap().values().stream() @@ -186,7 +209,7 @@ private boolean registerDisconnect( boolean removed = false; if (peer != null && peer.getConnection().equals(connection)) { if (!peerHasIncompleteConnection(id)) { - removed = completeConnections.remove(id, peer); + removed = activeConnections.remove(id, peer); disconnectCallbacks.forEach(callback -> callback.onDisconnect(peer)); peer.handleDisconnect(); abortPendingRequestsAssignedToDisconnectedPeers(); @@ -217,7 +240,7 @@ private void abortPendingRequestsAssignedToDisconnectedPeers() { public EthPeer peer(final PeerConnection connection) { final EthPeer ethPeer = incompleteConnections.getIfPresent(connection); - return ethPeer != null ? ethPeer : completeConnections.get(connection.getPeer().getId()); + return ethPeer != null ? ethPeer : activeConnections.get(connection.getPeer().getId()); } public PendingPeerRequest executePeerRequest( @@ -280,7 +303,7 @@ public void subscribeDisconnect(final DisconnectCallback callback) { public int peerCount() { removeDisconnectedPeers(); - return completeConnections.size(); + return activeConnections.size(); } public int getMaxPeers() { @@ -288,11 +311,11 @@ public int getMaxPeers() { } public Stream streamAllPeers() { - return completeConnections.values().stream(); + return activeConnections.values().stream(); } private void removeDisconnectedPeers() { - completeConnections + activeConnections .values() .forEach( ep -> { @@ -341,36 +364,60 @@ public void setRlpxAgent(final RlpxAgent rlpxAgent) { } public Stream getAllActiveConnections() { - return completeConnections.values().stream() + return activeConnections.values().stream() .map(EthPeer::getConnection) .filter(c -> !c.isDisconnected()); } public Stream getAllConnections() { return Stream.concat( - completeConnections.values().stream().map(EthPeer::getConnection), + activeConnections.values().stream().map(EthPeer::getConnection), incompleteConnections.asMap().keySet().stream()) .distinct() .filter(c -> !c.isDisconnected()); } - public boolean shouldConnect(final Peer peer, final boolean inbound) { + public boolean shouldTryToConnect(final Peer peer, final boolean inbound) { + + if (peer.getForkId().isPresent()) { + final ForkId forkId = peer.getForkId().get(); + if (!forkIdManager.peerCheck(forkId)) { + LOG.atDebug() + .setMessage("Wrong fork id, not trying to connect to peer {}") + .addArgument(peer::getId) + .log(); + + return false; + } + } + final Bytes id = peer.getId(); - if (peerCount() >= peerUpperBound && !canExceedPeerLimits(id)) { + if (alreadyConnectedOrConnecting(inbound, id)) { return false; } - final EthPeer ethPeer = completeConnections.get(id); + + if (peerCount() < getMaxPeers() || canExceedPeerLimits(id)) { + return true; + } + + // ask the protocol managers whether they want to connect to this peer and if none of them want + // to connect, then we don't connect + return protocolManagers.stream().anyMatch(p -> p.shouldTryToConnect(peer, inbound)); + } + + private boolean alreadyConnectedOrConnecting(final boolean inbound, final Bytes id) { + final EthPeer ethPeer = activeConnections.get(id); if (ethPeer != null && !ethPeer.isDisconnected()) { - return false; + return true; } final List incompleteConnections = getIncompleteConnections(id); if (!incompleteConnections.isEmpty()) { if (incompleteConnections.stream() .anyMatch(c -> !c.isDisconnected() && (!inbound || (inbound && c.inboundInitiated())))) { - return false; + return true; } } - return true; + return false; } public void disconnectWorstUselessPeer() { @@ -390,6 +437,10 @@ public void disconnectWorstUselessPeer() { }); } + public void setProtocolManagers(final List protocolManagers) { + this.protocolManagers = protocolManagers; + } + @FunctionalInterface public interface ConnectCallback { void onPeerConnected(EthPeer newPeer); @@ -397,21 +448,83 @@ public interface ConnectCallback { @Override public String toString() { - if (completeConnections.isEmpty()) { + if (activeConnections.isEmpty()) { return "0 EthPeers {}"; } final String connectionsList = - completeConnections.values().stream() + activeConnections.values().stream() .sorted() .map(EthPeer::toString) .collect(Collectors.joining(", \n")); - return completeConnections.size() + " EthPeers {\n" + connectionsList + '}'; + return activeConnections.size() + " EthPeers {\n" + connectionsList + '}'; } private void ethPeerStatusExchanged(final EthPeer peer) { - if (addPeerToEthPeers(peer)) { - connectedPeersCounter.inc(); - connectCallbacks.forEach(cb -> cb.onPeerConnected(peer)); + // We have a connection to a peer that is on the right chain and is willing to connect to us. + // Find out what the EthPeer block hight is and whether it can serve snap data (if we are doing + // snap sync) + CompletableFuture future = + CompletableFuture.runAsync( + () -> { + // Call your function here + try { + final RequestManager.ResponseStream responseStream = + peer.getHeadersByHash(peer.chainState().getBestBlock().getHash(), 1, 0, false); + responseStream.then( + (closed, msg, p) -> { + if (closed) { + throw new RuntimeException("Peer disconnected"); + } + final List blockHeaders = + new BytesValueRLPInput(msg.getData(), false) + .readList( + rlp -> + BlockHeader.readFrom(rlp, new MainnetBlockHeaderFunctions())); + if (blockHeaders.isEmpty()) { + // empty response, disconnect + p.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); + throw new RuntimeException("Empty response"); + } + final BlockHeader header = blockHeaders.get(0); + p.chainState().updateHeightEstimate(header.getNumber()); + // make sure that the enforceTrailingPeerLimit() from ChainHeadTracker + // equivalent is done here, instead of after adding to EthPeers + // TODO: once we have the height we could do a SNAP request here to figure out + // whether the peer is serving snap data + }); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + + CompletableFuture isServingSnapFuture; + if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { + isServingSnapFuture = + CompletableFuture.runAsync( + () -> { + checkIsSnapServer(peer); + }); + } else { + isServingSnapFuture = CompletableFuture.completedFuture(null); + } + + // join the two futures + CompletableFuture.allOf(future, isServingSnapFuture) + .thenRun( + () -> { + if (!peer.getConnection().isDisconnected() && addPeerToEthPeers(peer)) { + connectedPeersCounter.inc(); + connectCallbacks.forEach(cb -> cb.onPeerConnected(peer)); + } + }); + } + + private void checkIsSnapServer(final EthPeer peer) { + if (peer.getAgreedCapabilities().contains(SnapProtocol.SNAP1)) { + // could try and retrieve some SNAP data here to check that (e.g. GetByteCodes for a small + // contract) + LOG.info("XXXXXXX" + peer.getConnection().getPeerInfo().getClientId()); + peer.setIsServingSnap(peer.getConnection().getPeerInfo().getClientId().contains("Geth")); } } @@ -467,7 +580,7 @@ private void enforceRemoteConnectionLimits() { } private Stream getActivePrioritizedPeers() { - return completeConnections.values().stream() + return activeConnections.values().stream() .filter(p -> !p.isDisconnected()) .sorted(this::comparePeerPriorities); } @@ -501,7 +614,7 @@ private boolean shouldLimitRemoteConnections() { } private long countUntrustedRemotelyInitiatedConnections() { - return completeConnections.values().stream() + return activeConnections.values().stream() .map(ep -> ep.getConnection()) .filter(c -> c.inboundInitiated()) .filter(c -> !c.isDisconnected()) @@ -523,21 +636,26 @@ private void onCacheRemoval( private boolean addPeerToEthPeers(final EthPeer peer) { // We have a connection to a peer that is on the right chain and is willing to connect to us. - // Figure out whether we want to keep this peer and add it to the EthPeers connections. - if (completeConnections.containsValue(peer)) { + // Figure out whether we want to keep this peer to add it to the active connections. + final PeerConnection connection = peer.getConnection(); + if (activeConnections.containsValue(peer)) { + connection.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); return false; } - final PeerConnection connection = peer.getConnection(); final Bytes id = peer.getId(); if (!randomPeerPriority) { // Disconnect if too many peers - if (!canExceedPeerLimits(id) && peerCount() >= peerUpperBound) { - LOG.trace( - "Too many peers. Disconnect connection: {}, max connections {}", - connection, - peerUpperBound); - connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); - return false; + if (peerCount() >= peerUpperBound) { + if (needMoreSnapServers()) { + disconnectNonSnapServerPeerOrLeastUseful(); + } else if (!canExceedPeerLimits(id)) { + LOG.trace( + "Too many peers. Disconnect connection: {}, max connections {}", + connection, + peerUpperBound); + connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); + return false; + } } // Disconnect if too many remotely-initiated connections if (connection.inboundInitiated() @@ -550,7 +668,7 @@ && remoteConnectionLimitReached()) { connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); return false; } - final boolean added = (completeConnections.putIfAbsent(id, peer) == null); + final boolean added = (activeConnections.putIfAbsent(id, peer) == null); if (added) { LOG.trace("Added peer {} with connection {} to completeConnections", id, connection); } else { @@ -559,10 +677,29 @@ && remoteConnectionLimitReached()) { return added; } else { // randomPeerPriority! Add the peer and if there are too many connections fix it - completeConnections.putIfAbsent(id, peer); + // TODO: random peer priority does not care yet about snap server peers -> check later + activeConnections.putIfAbsent(id, peer); enforceRemoteConnectionLimits(); enforceConnectionLimits(); - return completeConnections.containsKey(id); + return activeConnections.containsKey(id); } } + + private boolean needMoreSnapServers() { + return activeConnections.values().stream().filter(EthPeer::isServingSnap).count() + < snapServerTargetNumber; + } + + private void disconnectNonSnapServerPeerOrLeastUseful() { + activeConnections.values().stream() + .filter(p -> !p.isServingSnap()) + .min(MOST_USEFUL_PEER) + .ifPresentOrElse( + p -> p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS), + () -> + activeConnections.values().stream() + .min(MOST_USEFUL_PEER) + .ifPresent( + p -> p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS))); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index de5962575c3..15d69922558 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -396,20 +396,7 @@ public void handleNewConnection(final PeerConnection connection) { } @Override - public boolean shouldConnect(final Peer peer, final boolean incoming) { - if (peer.getForkId().map(forkIdManager::peerCheck).orElse(true)) { - LOG.atDebug() - .setMessage("ForkId OK or not available for peer {}") - .addArgument(peer::getLoggableId) - .log(); - if (ethPeers.shouldConnect(peer, incoming)) { - return true; - } - } - LOG.atDebug() - .setMessage("ForkId check failed for peer {}") - .addArgument(peer::getLoggableId) - .log(); + public boolean shouldTryToConnect(final Peer peer, final boolean incoming) { return false; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerRequest.java index 09cbdb969d2..5daad56b9be 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerRequest.java @@ -19,4 +19,8 @@ public interface PeerRequest { ResponseStream sendRequest(EthPeer peer) throws PeerNotConnected; + + default boolean isEthPeerSuitable(final EthPeer ethPeer) { + return true; + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PendingPeerRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PendingPeerRequest.java index 120b32bdfe3..71da1e4d2d9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PendingPeerRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PendingPeerRequest.java @@ -86,6 +86,7 @@ private Optional getPeerToUse() { : ethPeers .streamAvailablePeers() .filter(peer -> peer.chainState().getEstimatedHeight() >= minimumBlockNumber) + .filter(request::isEthPeerSuitable) .min(EthPeers.LEAST_TO_MOST_BUSY); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java index c52758eeb94..a19adb320dd 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java @@ -17,10 +17,13 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.PeerRequest; import org.hyperledger.besu.ethereum.eth.manager.PendingPeerRequest; +import org.hyperledger.besu.ethereum.eth.manager.RequestManager; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerRequestTask; import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage; import org.hyperledger.besu.ethereum.eth.messages.snap.SnapV1; +import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -64,14 +67,23 @@ public static GetAccountRangeFromPeerTask forAccountRange( @Override protected PendingPeerRequest sendRequest() { return sendRequestToPeer( - peer -> { - LOG.trace( - "Requesting account range [{} ,{}] for state root {} from peer {} .", - startKeyHash, - endKeyHash, - blockHeader.getStateRoot(), - peer); - return peer.getSnapAccountRange(blockHeader.getStateRoot(), startKeyHash, endKeyHash); + new PeerRequest() { + @Override + public RequestManager.ResponseStream sendRequest(final EthPeer peer) + throws PeerConnection.PeerNotConnected { + LOG.trace( + "Requesting account range [{} ,{}] for state root {} from peer {} .", + startKeyHash, + endKeyHash, + blockHeader.getStateRoot(), + peer); + return peer.getSnapAccountRange(blockHeader.getStateRoot(), startKeyHash, endKeyHash); + } + + @Override + public boolean isEthPeerSuitable(final EthPeer ethPeer) { + return ethPeer.isServingSnap(); + } }, blockHeader.getNumber()); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java index 357157484a0..db7b7ff8cd6 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java @@ -21,10 +21,13 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.PeerRequest; import org.hyperledger.besu.ethereum.eth.manager.PendingPeerRequest; +import org.hyperledger.besu.ethereum.eth.manager.RequestManager; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerRequestTask; import org.hyperledger.besu.ethereum.eth.messages.snap.ByteCodesMessage; import org.hyperledger.besu.ethereum.eth.messages.snap.SnapV1; +import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -66,9 +69,18 @@ public static GetBytecodeFromPeerTask forBytecode( @Override protected PendingPeerRequest sendRequest() { return sendRequestToPeer( - peer -> { - LOG.trace("Requesting {} Bytecodes from {} .", codeHashes.size(), peer); - return peer.getSnapBytecode(blockHeader.getStateRoot(), codeHashes); + new PeerRequest() { + @Override + public RequestManager.ResponseStream sendRequest(final EthPeer peer) + throws PeerConnection.PeerNotConnected { + LOG.trace("Requesting {} Bytecodes from {} .", codeHashes.size(), peer); + return peer.getSnapBytecode(blockHeader.getStateRoot(), codeHashes); + } + + @Override + public boolean isEthPeerSuitable(final EthPeer ethPeer) { + return ethPeer.isServingSnap(); + } }, blockHeader.getNumber()); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java index 48b3426ae42..89bb4bed797 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java @@ -17,10 +17,13 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.PeerRequest; import org.hyperledger.besu.ethereum.eth.manager.PendingPeerRequest; +import org.hyperledger.besu.ethereum.eth.manager.RequestManager; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerRequestTask; import org.hyperledger.besu.ethereum.eth.messages.snap.SnapV1; import org.hyperledger.besu.ethereum.eth.messages.snap.StorageRangeMessage; +import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -69,15 +72,24 @@ public static GetStorageRangeFromPeerTask forStorageRange( @Override protected PendingPeerRequest sendRequest() { return sendRequestToPeer( - peer -> { - LOG.trace( - "Requesting storage range [{} ,{}] for {} accounts from peer {} .", - startKeyHash, - endKeyHash, - accountHashes.size(), - peer); - return peer.getSnapStorageRange( - blockHeader.getStateRoot(), accountHashes, startKeyHash, endKeyHash); + new PeerRequest() { + @Override + public RequestManager.ResponseStream sendRequest(final EthPeer peer) + throws PeerConnection.PeerNotConnected { + LOG.trace( + "Requesting storage range [{} ,{}] for {} accounts from peer {} .", + startKeyHash, + endKeyHash, + accountHashes.size(), + peer); + return peer.getSnapStorageRange( + blockHeader.getStateRoot(), accountHashes, startKeyHash, endKeyHash); + } + + @Override + public boolean isEthPeerSuitable(final EthPeer ethPeer) { + return ethPeer.isServingSnap(); + } }, blockHeader.getNumber()); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java index 56c7d9a2357..6f23fb868c9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java @@ -20,10 +20,13 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.PeerRequest; import org.hyperledger.besu.ethereum.eth.manager.PendingPeerRequest; +import org.hyperledger.besu.ethereum.eth.manager.RequestManager; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerRequestTask; import org.hyperledger.besu.ethereum.eth.messages.snap.SnapV1; import org.hyperledger.besu.ethereum.eth.messages.snap.TrieNodesMessage; +import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -72,9 +75,18 @@ public static GetTrieNodeFromPeerTask forTrieNodes( @Override protected PendingPeerRequest sendRequest() { return sendRequestToPeer( - peer -> { - LOG.trace("Requesting {} trie nodes from peer {}", paths.size(), peer); - return peer.getSnapTrieNode(blockHeader.getStateRoot(), paths); + new PeerRequest() { + @Override + public RequestManager.ResponseStream sendRequest(final EthPeer peer) + throws PeerConnection.PeerNotConnected { + LOG.trace("Requesting {} trie nodes from peer {}", paths.size(), peer); + return peer.getSnapTrieNode(blockHeader.getStateRoot(), paths); + } + + @Override + public boolean isEthPeerSuitable(final EthPeer ethPeer) { + return ethPeer.isServingSnap(); + } }, blockHeader.getNumber()); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index 7512bdd03a9..beb7784bf59 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -75,4 +75,9 @@ protected CompletableFuture executePeerTas return peerResult.getResult(); }); } + + @Override + protected boolean isSuitablePeer(final EthPeer peer) { + return peer.isServingSnap(); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java index 7d23ec944d3..172fb8f3515 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java @@ -69,4 +69,9 @@ protected CompletableFuture> executePeerTask( return peerResult.getResult(); }); } + + @Override + protected boolean isSuitablePeer(final EthPeer peer) { + return peer.isServingSnap(); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java index 7a095de9292..1126e312bd8 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java @@ -79,4 +79,9 @@ protected CompletableFuture executePeerTask( return peerResult.getResult(); }); } + + @Override + protected boolean isSuitablePeer(final EthPeer peer) { + return peer.isServingSnap(); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java index ebfd8856898..1517522ff5c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java @@ -68,4 +68,9 @@ protected CompletableFuture> executePeerTask( return peerResult.getResult(); }); } + + @Override + protected boolean isSuitablePeer(final EthPeer peer) { + return peer.isServingSnap(); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java index de2cb280c8c..ba14daf157c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java @@ -139,7 +139,7 @@ public void processMessage(final Capability cap, final Message message) { public void handleNewConnection(final PeerConnection connection) {} @Override - public boolean shouldConnect(final Peer peer, final boolean incoming) { + public boolean shouldTryToConnect(final Peer peer, final boolean incoming) { return false; // EthManager is taking care of this for now } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index cf48d69847d..46026183d1e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -67,8 +67,19 @@ protected AbstractRetryingPeerTask( this.metricsSystem = metricsSystem; } - public void assignPeer(final EthPeer peer) { - assignedPeer = Optional.of(peer); + /** + * Assign the peer to be used for the task. + * + * @param peer The peer to assign to the task. + * @return True if the peer was assigned, false otherwise. + */ + public boolean assignPeer(final EthPeer peer) { + if (isSuitablePeer(peer)) { + assignedPeer = Optional.of(peer); + return true; + } else { + return false; + } } public Optional getAssignedPeer() { @@ -164,4 +175,8 @@ public int getRetryCount() { public int getMaxRetries() { return maxRetries; } + + protected boolean isSuitablePeer(final EthPeer peer) { + return true; + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java index 7db355646f0..b02ef867424 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java @@ -49,9 +49,12 @@ protected AbstractRetryingSwitchingPeerTask( } @Override - public void assignPeer(final EthPeer peer) { - super.assignPeer(peer); - triedPeers.add(peer); + public boolean assignPeer(final EthPeer peer) { + if (super.assignPeer(peer)) { + triedPeers.add(peer); + return true; + } + return false; } protected abstract CompletableFuture executeTaskOnCurrentPeer(final EthPeer peer); @@ -61,9 +64,9 @@ protected CompletableFuture executePeerTask(final Optional assignedP final Optional maybePeer = assignedPeer - .filter(u -> getRetryCount() == 1) // first try with the assigned peer if present - .map(Optional::of) - .orElseGet(this::selectNextPeer); // otherwise, select a new one from the pool + .filter(u -> getRetryCount() == 1) + .or(this::selectNextPeer); // first try with the assigned peer if present, otherwise + // select a new one from the pool if (maybePeer.isEmpty()) { LOG.atTrace() @@ -101,7 +104,7 @@ protected CompletableFuture executePeerTask(final Optional assignedP @Override protected void handleTaskError(final Throwable error) { if (isPeerFailure(error)) { - getAssignedPeer().ifPresent(peer -> failedPeers.add(peer)); + getAssignedPeer().ifPresent(failedPeers::add); } super.handleTaskError(error); } @@ -124,10 +127,11 @@ private Optional selectNextPeer() { return maybeNextPeer; } - private Stream remainingPeersToTry() { + protected Stream remainingPeersToTry() { return getEthContext() .getEthPeers() .streamBestPeers() + .filter(this::isSuitablePeer) .filter(peer -> !triedPeers.contains(peer)); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java index eb89ca12acc..fcbde5ad988 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java @@ -20,23 +20,22 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthPeers.ConnectCallback; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; -import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason; import org.hyperledger.besu.plugin.services.MetricsSystem; +import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.function.Supplier; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - public class ChainHeadTracker implements ConnectCallback { - private static final Logger LOG = LoggerFactory.getLogger(ChainHeadTracker.class); + // private static final Logger LOG = LoggerFactory.getLogger(ChainHeadTracker.class); private final EthContext ethContext; private final ProtocolSchedule protocolSchedule; - private final TrailingPeerLimiter trailingPeerLimiter; + // private final TrailingPeerLimiter trailingPeerLimiter; private final MetricsSystem metricsSystem; public ChainHeadTracker( @@ -46,7 +45,7 @@ public ChainHeadTracker( final MetricsSystem metricsSystem) { this.ethContext = ethContext; this.protocolSchedule = protocolSchedule; - this.trailingPeerLimiter = trailingPeerLimiter; + // this.trailingPeerLimiter = trailingPeerLimiter; this.metricsSystem = metricsSystem; } @@ -66,42 +65,47 @@ public static void trackChainHeadForPeers( @Override public void onPeerConnected(final EthPeer peer) { - LOG.atDebug() - .setMessage("Requesting chain head info from {}...") - .addArgument(peer::getLoggableId) - .log(); - GetHeadersFromPeerByHashTask.forSingleHash( + // LOG.atDebug() + // .setMessage("Requesting chain head info from {}...") + // .addArgument(peer::getLoggableId) + // .log(); + // getBestHeaderFromPeerCompletableFuture(peer) + // .whenComplete( + // (peerResult, error) -> { + // if (peerResult != null && !peerResult.getResult().isEmpty()) { + // final BlockHeader chainHeadHeader = peerResult.getResult().get(0); + // peer.chainState().update(chainHeadHeader); + // trailingPeerLimiter.enforceTrailingPeerLimit(); + // LOG.atDebug() + // .setMessage("Retrieved chain head info {} from {}...") + // .addArgument( + // () -> + // chainHeadHeader.getNumber() + // + " (" + // + chainHeadHeader.getBlockHash() + // + ")") + // .addArgument(peer::getLoggableId) + // .log(); + // } else { + // LOG.atDebug() + // .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") + // .addArgument(peer::getLoggableId) + // .addArgument(error) + // .log(); + // peer.disconnect(DisconnectReason.USELESS_PEER); + // } + // }); + } + + public CompletableFuture>> + getBestHeaderFromPeerCompletableFuture(final EthPeer peer) { + return GetHeadersFromPeerByHashTask.forSingleHash( protocolSchedule, ethContext, Hash.wrap(peer.chainState().getBestBlock().getHash()), 0, metricsSystem) .assignPeer(peer) - .run() - .whenComplete( - (peerResult, error) -> { - if (peerResult != null && !peerResult.getResult().isEmpty()) { - final BlockHeader chainHeadHeader = peerResult.getResult().get(0); - peer.chainState().update(chainHeadHeader); - trailingPeerLimiter.enforceTrailingPeerLimit(); - LOG.atDebug() - .setMessage("Retrieved chain head info {} from {}...") - .addArgument( - () -> - chainHeadHeader.getNumber() - + " (" - + chainHeadHeader.getBlockHash() - + ")") - .addArgument(peer::getLoggableId) - .log(); - } else { - LOG.atDebug() - .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") - .addArgument(peer::getLoggableId) - .addArgument(error) - .log(); - peer.disconnect(DisconnectReason.USELESS_PEER); - } - }); + .run(); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java index 8fc4e614c76..229d1e09764 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java @@ -29,6 +29,7 @@ import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.manager.snap.SnapProtocolManager; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; +import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.forkid.ForkIdManager; @@ -86,7 +87,9 @@ public static EthProtocolManager create( Bytes.random(64), 25, 25, - false); + false, + SyncMode.X_SNAP, + new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthMessages messages = new EthMessages(); final EthScheduler ethScheduler = new DeterministicEthScheduler(TimeoutPolicy.NEVER_TIMEOUT); final EthContext ethContext = new EthContext(peers, messages, ethScheduler); @@ -205,7 +208,9 @@ public static EthProtocolManager create( Bytes.random(64), 25, 25, - false); + false, + SyncMode.X_SNAP, + new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthMessages messages = new EthMessages(); return create( @@ -239,7 +244,9 @@ public static EthProtocolManager create( Bytes.random(64), 25, 25, - false); + false, + SyncMode.X_SNAP, + new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthMessages messages = new EthMessages(); return create( @@ -269,7 +276,9 @@ public static EthProtocolManager create( Bytes.random(64), 25, 25, - false); + false, + SyncMode.X_SNAP, + new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthMessages messages = new EthMessages(); return create( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java index 868030334c8..fe6f81d0c8b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java @@ -36,11 +36,13 @@ import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; +import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.BlobCache; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolFactory; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.worldstate.DataStorageFormat; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; @@ -118,7 +120,10 @@ public void setupTest() { Bytes.random(64), MAX_PEERS, MAX_PEERS, - false)); + false, + SyncMode.X_SNAP, + new ForkIdManager( + blockchain, Collections.emptyList(), Collections.emptyList(), false))); final EthMessages ethMessages = new EthMessages(); final EthScheduler ethScheduler = diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java index f36958b2fb1..ac608e46995 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java @@ -55,6 +55,7 @@ import org.hyperledger.besu.ethereum.eth.sync.BlockPropagationManager.ProcessingBlocksManager; import org.hyperledger.besu.ethereum.eth.sync.state.PendingBlocksManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; @@ -630,7 +631,10 @@ public void shouldNotImportBlocksThatAreAlreadyBeingImported() { Bytes.random(64), 25, 25, - false), + false, + SyncMode.X_SNAP, + new ForkIdManager( + blockchain, Collections.emptyList(), Collections.emptyList(), false)), new EthMessages(), ethScheduler); final BlockPropagationManager blockPropagationManager = @@ -768,7 +772,10 @@ public Object answer(final InvocationOnMock invocation) throws Throwable { Bytes.random(64), 25, 25, - false), + false, + SyncMode.X_SNAP, + new ForkIdManager( + blockchain, Collections.emptyList(), Collections.emptyList(), false)), new EthMessages(), ethScheduler); final BlockPropagationManager blockPropagationManager = diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java index c02e2d78934..08d03c7b84f 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java @@ -40,8 +40,10 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ScheduleBasedBlockHeaderFunctions; import org.hyperledger.besu.ethereum.p2p.config.DiscoveryConfiguration; @@ -147,7 +149,9 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod Bytes.random(64), 25, 25, - false); + false, + SyncMode.X_SNAP, + new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthScheduler scheduler = new EthScheduler(1, 1, 1, metricsSystem); final EthContext ethContext = new EthContext(ethPeers, ethMessages, scheduler); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java index 79d88a82d21..13a5bcd6efe 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java @@ -38,6 +38,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.layered.LayeredPendingTransactions; @@ -110,7 +111,9 @@ public void setup() { Bytes.random(64), 25, 25, - false); + false, + SyncMode.X_SNAP, + new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); when(ethContext.getEthMessages()).thenReturn(ethMessages); when(ethContext.getEthPeers()).thenReturn(ethPeers); diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/NetworkRunner.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/NetworkRunner.java index 5811bd65f99..cd34d28989c 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/NetworkRunner.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/NetworkRunner.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.ethereum.p2p.network; +import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.RlpxAgent; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; @@ -31,6 +32,7 @@ import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiFunction; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -47,15 +49,18 @@ public class NetworkRunner implements AutoCloseable { private final Map subProtocols; private final List protocolManagers; private final LabelledMetric inboundMessageCounter; + private final BiFunction ethPeersShouldConnect; private NetworkRunner( final P2PNetwork network, final Map subProtocols, final List protocolManagers, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final BiFunction ethPeersShouldConnect) { this.network = network; this.protocolManagers = protocolManagers; this.subProtocols = subProtocols; + this.ethPeersShouldConnect = ethPeersShouldConnect; inboundMessageCounter = metricsSystem.createLabelledCounter( BesuMetricCategory.NETWORK, @@ -155,8 +160,7 @@ private void setupHandlers() { protocolManager.handleNewConnection(connection); }); - network.subscribeConnectRequest( - (peer, incoming) -> protocolManager.shouldConnect(peer, incoming)); + network.subscribeConnectRequest(ethPeersShouldConnect::apply); network.subscribeDisconnect( (connection, disconnectReason, initiatedByPeer) -> { @@ -183,6 +187,7 @@ public static class Builder { List protocolManagers = new ArrayList<>(); List subProtocols = new ArrayList<>(); MetricsSystem metricsSystem; + private BiFunction ethPeersShouldConnect; public NetworkRunner build() { final Map subProtocolMap = new HashMap<>(); @@ -200,7 +205,8 @@ public NetworkRunner build() { } } final P2PNetwork network = networkProvider.build(caps); - return new NetworkRunner(network, subProtocolMap, protocolManagers, metricsSystem); + return new NetworkRunner( + network, subProtocolMap, protocolManagers, metricsSystem, ethPeersShouldConnect); } public Builder protocolManagers(final List protocolManagers) { @@ -227,6 +233,11 @@ public Builder metricsSystem(final MetricsSystem metricsSystem) { this.metricsSystem = metricsSystem; return this; } + + public Builder ethPeersShouldConnect(final BiFunction shouldConnect) { + this.ethPeersShouldConnect = shouldConnect; + return this; + } } @FunctionalInterface diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java index f87847a488d..f43d35bdb9f 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java @@ -66,7 +66,7 @@ public interface ProtocolManager extends AutoCloseable { * @param incoming true if the connection is incoming * @return true, if the ProtocolManager wants to connect to the peer, false otherwise */ - boolean shouldConnect(Peer peer, final boolean incoming); + boolean shouldTryToConnect(Peer peer, final boolean incoming); /** * Handles peer disconnects. * diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java index 98054b27dbf..755b00f2a05 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java @@ -41,12 +41,14 @@ import org.hyperledger.besu.ethereum.eth.manager.EthMessages; import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.BlobCache; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolFactory; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.EpochCalculator; import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; @@ -71,6 +73,7 @@ import org.hyperledger.besu.util.number.Fraction; import java.util.Collections; +import java.util.List; import java.util.Optional; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; @@ -231,7 +234,9 @@ private boolean buildContext( localNodeKey, MAX_PEERS, MAX_PEERS, - false); + false, + SyncMode.X_SNAP, + new ForkIdManager(blockchain, List.of(), List.of(), false)); final SyncState syncState = new SyncState(blockchain, ethPeers); ethScheduler = new EthScheduler(1, 1, 1, 1, metricsSystem); From 7f2fd43af4a0ddcc48be13b075233f885240580e Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Fri, 23 Feb 2024 14:30:31 +1000 Subject: [PATCH 02/26] EthPeer add isServingSnap to be able to make sure that we have enough snap servers connected when we are snap syncing Signed-off-by: stefan.pingel@consensys.net --- .../org/hyperledger/besu/RunnerBuilder.java | 2 +- .../cache/TransactionLogBloomCacher.java | 2 +- .../besu/ethereum/eth/manager/EthPeer.java | 24 +++--- .../besu/ethereum/eth/manager/EthPeers.java | 69 +++++----------- .../eth/manager/EthProtocolManager.java | 20 ++--- .../snap/GetAccountRangeFromPeerTask.java | 11 ++- .../manager/snap/GetBytecodeFromPeerTask.java | 9 +++ .../snap/GetStorageRangeFromPeerTask.java | 9 +++ .../manager/snap/GetTrieNodeFromPeerTask.java | 9 +++ .../ethereum/eth/sync/ChainHeadTracker.java | 78 ++++++++++--------- .../p2p/network/DefaultP2PNetwork.java | 1 - 11 files changed, 123 insertions(+), 111 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 30a164cd361..8ac07d161b5 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -635,7 +635,7 @@ public Runner build() { .collect(Collectors.toSet()); final EthPeers ethPeers = besuController.getEthPeers(); - ethPeers.setProtocolManagers(protocolManagers); + // ethPeers.setProtocolManagers(protocolManagers); final RlpxConfiguration rlpxConfiguration = RlpxConfiguration.create() diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/cache/TransactionLogBloomCacher.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/cache/TransactionLogBloomCacher.java index 31132a4655e..69a9edc82f5 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/cache/TransactionLogBloomCacher.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/cache/TransactionLogBloomCacher.java @@ -159,7 +159,7 @@ void cacheLogsBloomForBlockHeader( return; } final long blockNumber = blockHeader.getNumber(); - LOG.atDebug() + LOG.atTrace() .setMessage("Caching logs bloom for block {}") .addArgument(() -> "0x" + Long.toHexString(blockNumber)) .log(); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index f1aee2fecdf..185787eae40 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -432,13 +432,12 @@ Optional dispatch(final EthMessage ethMessage, final String prot Optional requestManager = getRequestManager(protocolName, messageCode); requestManager.ifPresentOrElse( localRequestManager -> localRequestManager.dispatchResponse(ethMessage), - () -> { - LOG.trace( - "Message {} not expected has just been received for protocol {}, peer {} ", - messageCode, - protocolName, - this); - }); + () -> + LOG.debug( + "Message {} not expected has just been received for protocol {}, peer {} ", + messageCode, + protocolName, + this)); return requestManager; } @@ -569,9 +568,9 @@ public String getProtocolName() { } /** - * Return A read-only snapshot of this peer's current {@code chainState} } + * Return A read-only snapshot of this peer's current {@code chainState} * - * @return A read-only snapshot of this peer's current {@code chainState} } + * @return A read-only snapshot of this peer's current {@code chainState} */ public ChainHeadEstimate chainStateSnapshot() { return chainHeadState.getSnapshot(); @@ -616,14 +615,17 @@ public boolean hasSupportForMessage(final int messageCode) { @Override public String toString() { return String.format( - "PeerId: %s... %s, validated? %s, disconnected? %s, client: %s, %s, %s", + "PeerId: %s... %s, validated? %s, disconnected? %s, client: %s, %s, %s, isServingSNAP %s, has height %s, connected for %s ms", getLoggableId(), reputation, isFullyValidated(), isDisconnected(), connection.getPeerInfo().getClientId(), connection, - connection.getPeer().getEnodeURLString()); + connection.getPeer().getEnodeURLString(), + isServingSnap, + chainHeadState.getEstimatedHeight(), + System.currentTimeMillis() - connection.getInitiatedAt()); } @Nonnull diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index fd6e680b7d3..66494e4aa7c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -14,21 +14,18 @@ */ package org.hyperledger.besu.ethereum.eth.manager; -import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthPeer.DisconnectCallback; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; +import org.hyperledger.besu.ethereum.eth.sync.ChainHeadTracker; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.forkid.ForkId; import org.hyperledger.besu.ethereum.forkid.ForkIdManager; -import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; -import org.hyperledger.besu.ethereum.p2p.network.ProtocolManager; import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.RlpxAgent; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage; -import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput; import org.hyperledger.besu.metrics.BesuMetricCategory; import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.plugin.services.metrics.Counter; @@ -110,7 +107,8 @@ public class EthPeers { private RlpxAgent rlpxAgent; private final Counter connectedPeersCounter; - private List protocolManagers; + // private List protocolManagers; + private ChainHeadTracker tracker; public EthPeers( final String protocolName, @@ -396,13 +394,15 @@ public boolean shouldTryToConnect(final Peer peer, final boolean inbound) { return false; } - if (peerCount() < getMaxPeers() || canExceedPeerLimits(id)) { + if (peerCount() < getMaxPeers() || needMoreSnapServers() || canExceedPeerLimits(id)) { return true; + } else { + return false; } // ask the protocol managers whether they want to connect to this peer and if none of them want // to connect, then we don't connect - return protocolManagers.stream().anyMatch(p -> p.shouldTryToConnect(peer, inbound)); + // return protocolManagers.stream().anyMatch(p -> p.shouldTryToConnect(peer, inbound)); } private boolean alreadyConnectedOrConnecting(final boolean inbound, final Bytes id) { @@ -437,8 +437,12 @@ public void disconnectWorstUselessPeer() { }); } - public void setProtocolManagers(final List protocolManagers) { - this.protocolManagers = protocolManagers; + // public void setProtocolManagers(final List protocolManagers) { + // this.protocolManagers = protocolManagers; + // } + + public void subscribeStatusExchanged(final ChainHeadTracker tracker) { + this.tracker = tracker; } @FunctionalInterface @@ -461,41 +465,10 @@ public String toString() { private void ethPeerStatusExchanged(final EthPeer peer) { // We have a connection to a peer that is on the right chain and is willing to connect to us. - // Find out what the EthPeer block hight is and whether it can serve snap data (if we are doing + // Find out what the EthPeer block height is and whether it can serve snap data (if we are doing // snap sync) - CompletableFuture future = - CompletableFuture.runAsync( - () -> { - // Call your function here - try { - final RequestManager.ResponseStream responseStream = - peer.getHeadersByHash(peer.chainState().getBestBlock().getHash(), 1, 0, false); - responseStream.then( - (closed, msg, p) -> { - if (closed) { - throw new RuntimeException("Peer disconnected"); - } - final List blockHeaders = - new BytesValueRLPInput(msg.getData(), false) - .readList( - rlp -> - BlockHeader.readFrom(rlp, new MainnetBlockHeaderFunctions())); - if (blockHeaders.isEmpty()) { - // empty response, disconnect - p.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); - throw new RuntimeException("Empty response"); - } - final BlockHeader header = blockHeaders.get(0); - p.chainState().updateHeightEstimate(header.getNumber()); - // make sure that the enforceTrailingPeerLimit() from ChainHeadTracker - // equivalent is done here, instead of after adding to EthPeers - // TODO: once we have the height we could do a SNAP request here to figure out - // whether the peer is serving snap data - }); - } catch (Exception e) { - throw new RuntimeException(e); - } - }); + LOG.debug("Peer {} status exchanged", peer); + CompletableFuture future = tracker.onPeerConnected(peer); CompletableFuture isServingSnapFuture; if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { @@ -523,7 +496,7 @@ private void checkIsSnapServer(final EthPeer peer) { if (peer.getAgreedCapabilities().contains(SnapProtocol.SNAP1)) { // could try and retrieve some SNAP data here to check that (e.g. GetByteCodes for a small // contract) - LOG.info("XXXXXXX" + peer.getConnection().getPeerInfo().getClientId()); + // LOG.info("XXXXXXX" + peer.getConnection().getPeerInfo().getClientId()); peer.setIsServingSnap(peer.getConnection().getPeerInfo().getClientId().contains("Geth")); } } @@ -639,7 +612,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { // Figure out whether we want to keep this peer to add it to the active connections. final PeerConnection connection = peer.getConnection(); if (activeConnections.containsValue(peer)) { - connection.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); + // connection.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); return false; } final Bytes id = peer.getId(); @@ -661,7 +634,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { if (connection.inboundInitiated() && !canExceedPeerLimits(id) && remoteConnectionLimitReached()) { - LOG.trace( + LOG.info( "Too many remotely-initiated connections. Disconnect incoming connection: {}, maxRemote={}", connection, maxRemotelyInitiatedConnections); @@ -670,9 +643,9 @@ && remoteConnectionLimitReached()) { } final boolean added = (activeConnections.putIfAbsent(id, peer) == null); if (added) { - LOG.trace("Added peer {} with connection {} to completeConnections", id, connection); + LOG.debug("Added peer {} with connection {} to activeConnections", id, connection); } else { - LOG.trace("Did not add peer {} with connection {} to completeConnections", id, connection); + LOG.debug("Did not add peer {} with connection {} to activeConnections", id, connection); } return added; } else { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index 15d69922558..49797546b4e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -405,16 +405,16 @@ public void handleDisconnect( final PeerConnection connection, final DisconnectReason reason, final boolean initiatedByPeer) { - if (ethPeers.registerDisconnect(connection)) { - LOG.atDebug() - .setMessage("Disconnect - {} - {} - {} - {} peers left") - .addArgument(initiatedByPeer ? "Inbound" : "Outbound") - .addArgument(reason::toString) - .addArgument(() -> connection.getPeer().getLoggableId()) - .addArgument(ethPeers::peerCount) - .log(); - LOG.atTrace().setMessage("{}").addArgument(ethPeers::toString).log(); - } + final boolean wasActiveConnection = ethPeers.registerDisconnect(connection); + LOG.atDebug() + .setMessage("Disconnect - active Connection: {} - {} - {} - {} - {} peers left") + .addArgument(wasActiveConnection) + .addArgument(initiatedByPeer ? "Inbound" : "Outbound") + .addArgument(reason::toString) + .addArgument(() -> connection.getPeer().getLoggableId()) + .addArgument(ethPeers::peerCount) + .log(); + LOG.atTrace().setMessage("{}").addArgument(ethPeers::toString).log(); } private void handleStatusMessage(final EthPeer peer, final Message message) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java index a19adb320dd..8e095af5c35 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java @@ -71,12 +71,21 @@ protected PendingPeerRequest sendRequest() { @Override public RequestManager.ResponseStream sendRequest(final EthPeer peer) throws PeerConnection.PeerNotConnected { - LOG.trace( + LOG.debug( "Requesting account range [{} ,{}] for state root {} from peer {} .", startKeyHash, endKeyHash, blockHeader.getStateRoot(), peer); + if (!peer.isServingSnap()) { + LOG.debug( + "EthPeer that is not serving snap called in {}, {}", + GetAccountRangeFromPeerTask.class, + peer); + throw new RuntimeException( + "EthPeer that is not serving snap called in " + + GetAccountRangeFromPeerTask.class); + } return peer.getSnapAccountRange(blockHeader.getStateRoot(), startKeyHash, endKeyHash); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java index db7b7ff8cd6..d3e299571ff 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java @@ -74,6 +74,15 @@ protected PendingPeerRequest sendRequest() { public RequestManager.ResponseStream sendRequest(final EthPeer peer) throws PeerConnection.PeerNotConnected { LOG.trace("Requesting {} Bytecodes from {} .", codeHashes.size(), peer); + if (!peer.isServingSnap()) { + LOG.debug( + "EthPeer that is not serving snap called in {}, {}", + GetAccountRangeFromPeerTask.class, + peer); + throw new RuntimeException( + "EthPeer that is not serving snap called in " + + GetAccountRangeFromPeerTask.class); + } return peer.getSnapBytecode(blockHeader.getStateRoot(), codeHashes); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java index 89bb4bed797..e2492c2d140 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java @@ -82,6 +82,15 @@ public RequestManager.ResponseStream sendRequest(final EthPeer peer) endKeyHash, accountHashes.size(), peer); + if (!peer.isServingSnap()) { + LOG.debug( + "EthPeer that is not serving snap called in {}, {}", + GetAccountRangeFromPeerTask.class, + peer); + throw new RuntimeException( + "EthPeer that is not serving snap called in " + + GetAccountRangeFromPeerTask.class); + } return peer.getSnapStorageRange( blockHeader.getStateRoot(), accountHashes, startKeyHash, endKeyHash); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java index 6f23fb868c9..5b0f28923ab 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java @@ -80,6 +80,15 @@ protected PendingPeerRequest sendRequest() { public RequestManager.ResponseStream sendRequest(final EthPeer peer) throws PeerConnection.PeerNotConnected { LOG.trace("Requesting {} trie nodes from peer {}", paths.size(), peer); + if (!peer.isServingSnap()) { + LOG.debug( + "EthPeer that is not serving snap called in {}, {}", + GetAccountRangeFromPeerTask.class, + peer); + throw new RuntimeException( + "EthPeer that is not serving snap called in " + + GetAccountRangeFromPeerTask.class); + } return peer.getSnapTrieNode(blockHeader.getStateRoot(), paths); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java index fcbde5ad988..b8a55cfcedb 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java @@ -19,23 +19,26 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.EthPeers.ConnectCallback; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage; import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.function.Supplier; -public class ChainHeadTracker implements ConnectCallback { +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; - // private static final Logger LOG = LoggerFactory.getLogger(ChainHeadTracker.class); +public class ChainHeadTracker { + + private static final Logger LOG = LoggerFactory.getLogger(ChainHeadTracker.class); private final EthContext ethContext; private final ProtocolSchedule protocolSchedule; - // private final TrailingPeerLimiter trailingPeerLimiter; + private final TrailingPeerLimiter trailingPeerLimiter; private final MetricsSystem metricsSystem; public ChainHeadTracker( @@ -45,7 +48,7 @@ public ChainHeadTracker( final MetricsSystem metricsSystem) { this.ethContext = ethContext; this.protocolSchedule = protocolSchedule; - // this.trailingPeerLimiter = trailingPeerLimiter; + this.trailingPeerLimiter = trailingPeerLimiter; this.metricsSystem = metricsSystem; } @@ -59,42 +62,41 @@ public static void trackChainHeadForPeers( new TrailingPeerLimiter(ethContext.getEthPeers(), trailingPeerRequirementsCalculator); final ChainHeadTracker tracker = new ChainHeadTracker(ethContext, protocolSchedule, trailingPeerLimiter, metricsSystem); - ethContext.getEthPeers().subscribeConnect(tracker); + ethContext.getEthPeers().subscribeStatusExchanged(tracker); blockchain.observeBlockAdded(trailingPeerLimiter); } - @Override - public void onPeerConnected(final EthPeer peer) { - // LOG.atDebug() - // .setMessage("Requesting chain head info from {}...") - // .addArgument(peer::getLoggableId) - // .log(); - // getBestHeaderFromPeerCompletableFuture(peer) - // .whenComplete( - // (peerResult, error) -> { - // if (peerResult != null && !peerResult.getResult().isEmpty()) { - // final BlockHeader chainHeadHeader = peerResult.getResult().get(0); - // peer.chainState().update(chainHeadHeader); - // trailingPeerLimiter.enforceTrailingPeerLimit(); - // LOG.atDebug() - // .setMessage("Retrieved chain head info {} from {}...") - // .addArgument( - // () -> - // chainHeadHeader.getNumber() - // + " (" - // + chainHeadHeader.getBlockHash() - // + ")") - // .addArgument(peer::getLoggableId) - // .log(); - // } else { - // LOG.atDebug() - // .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") - // .addArgument(peer::getLoggableId) - // .addArgument(error) - // .log(); - // peer.disconnect(DisconnectReason.USELESS_PEER); - // } - // }); + public CompletableFuture onPeerConnected(final EthPeer peer) { + LOG.atDebug() + .setMessage("Requesting chain head info from {}...") + .addArgument(peer::getLoggableId) + .log(); + final CompletableFuture>> + bestHeaderFromPeerCompletableFuture = getBestHeaderFromPeerCompletableFuture(peer); + final CompletableFuture future = new CompletableFuture<>(); + bestHeaderFromPeerCompletableFuture.whenComplete( + (peerResult, error) -> { + if (peerResult != null && !peerResult.getResult().isEmpty()) { + final BlockHeader chainHeadHeader = peerResult.getResult().get(0); + peer.chainState().update(chainHeadHeader); + trailingPeerLimiter.enforceTrailingPeerLimit(); + LOG.atDebug() + .setMessage("Retrieved chain head info {} from {}...") + .addArgument( + () -> chainHeadHeader.getNumber() + " (" + chainHeadHeader.getBlockHash() + ")") + .addArgument(peer::getLoggableId) + .log(); + } else { + LOG.atDebug() + .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") + .addArgument(peer::getLoggableId) + .addArgument(error) + .log(); + peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); + } + future.complete(null); + }); + return future; } public CompletableFuture>> diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java index eee530268db..d4a0e3daaba 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java @@ -425,7 +425,6 @@ public void subscribeConnect(final ConnectCallback callback) { public void subscribeConnectRequest(final ShouldConnectCallback callback) { rlpxAgent.subscribeConnectRequest(callback); } - ; @Override public void subscribeDisconnect(final DisconnectCallback callback) { From dcbb895bbc82dcc6d131df731f075491e52fb407 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Mon, 26 Feb 2024 17:54:55 +1000 Subject: [PATCH 03/26] add real check for snap server using a snap task Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/manager/EthPeers.java | 115 +++++++++++++----- .../ethereum/eth/sync/ChainHeadTracker.java | 9 +- .../eth/sync/DefaultSynchronizer.java | 2 + .../ethereum/eth/sync/SnapServerChecker.java | 86 +++++++++++++ .../eth/sync/ChainHeadTrackerTest.java | 6 +- 5 files changed, 182 insertions(+), 36 deletions(-) create mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 66494e4aa7c..e16f0c8e6c1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -14,10 +14,12 @@ */ package org.hyperledger.besu.ethereum.eth.manager; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthPeer.DisconnectCallback; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; import org.hyperledger.besu.ethereum.eth.sync.ChainHeadTracker; +import org.hyperledger.besu.ethereum.eth.sync.SnapServerChecker; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.forkid.ForkId; import org.hyperledger.besu.ethereum.forkid.ForkIdManager; @@ -43,6 +45,9 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -109,6 +114,7 @@ public class EthPeers { private final Counter connectedPeersCounter; // private List protocolManagers; private ChainHeadTracker tracker; + private SnapServerChecker snapServerChecker; public EthPeers( final String protocolName, @@ -146,7 +152,17 @@ public EthPeers( BesuMetricCategory.ETHEREUM, "peer_count", "The current number of peers connected", - () -> (int) streamAvailablePeers().filter(p -> p.readyForRequests()).count()); + () -> (int) streamAvailablePeers().filter(EthPeer::readyForRequests).count()); + metricsSystem.createIntegerGauge( + BesuMetricCategory.ETHEREUM, + "peer_count_snap_server", + "The current number of peers connected that serve snap data", + () -> + (int) + streamAvailablePeers() + .filter(EthPeer::readyForRequests) + .filter(EthPeer::isServingSnap) + .count()); metricsSystem.createIntegerGauge( BesuMetricCategory.PEERS, "pending_peer_requests_current", @@ -441,10 +457,14 @@ public void disconnectWorstUselessPeer() { // this.protocolManagers = protocolManagers; // } - public void subscribeStatusExchanged(final ChainHeadTracker tracker) { + public void setChainHeadTracker(final ChainHeadTracker tracker) { this.tracker = tracker; } + public void setSnapServerChecker(final SnapServerChecker checker) { + this.snapServerChecker = checker; + } + @FunctionalInterface public interface ConnectCallback { void onPeerConnected(EthPeer newPeer); @@ -468,36 +488,73 @@ private void ethPeerStatusExchanged(final EthPeer peer) { // Find out what the EthPeer block height is and whether it can serve snap data (if we are doing // snap sync) LOG.debug("Peer {} status exchanged", peer); - CompletableFuture future = tracker.onPeerConnected(peer); - - CompletableFuture isServingSnapFuture; - if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { - isServingSnapFuture = - CompletableFuture.runAsync( - () -> { - checkIsSnapServer(peer); - }); - } else { - isServingSnapFuture = CompletableFuture.completedFuture(null); - } - - // join the two futures - CompletableFuture.allOf(future, isServingSnapFuture) - .thenRun( - () -> { - if (!peer.getConnection().isDisconnected() && addPeerToEthPeers(peer)) { - connectedPeersCounter.inc(); - connectCallbacks.forEach(cb -> cb.onPeerConnected(peer)); - } - }); - } - - private void checkIsSnapServer(final EthPeer peer) { + CompletableFuture future = tracker.getBestHeaderFromPeer(peer); + + future.whenComplete( + (peersHeadBlockHeader, error) -> { + if (peersHeadBlockHeader == null) { + LOG.debug( + "Failed to retrieve chain head info. Disconnecting {}... {}", + peer.getLoggableId(), + error); + peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); + } else { + CompletableFuture isServingSnapFuture; + if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { + isServingSnapFuture = + CompletableFuture.runAsync( + () -> { + try { + checkIsSnapServer(peer, peersHeadBlockHeader); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } else { + isServingSnapFuture = CompletableFuture.completedFuture(null); + } + isServingSnapFuture.thenRun( + // TODO: might be a good idea not to connect to syncing peers if we are syncing + // ourselves (initial sync) + () -> { + if (!peer.getConnection().isDisconnected() && addPeerToEthPeers(peer)) { + connectedPeersCounter.inc(); + connectCallbacks.forEach(cb -> cb.onPeerConnected(peer)); + } + }); + } + }); + } + + private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBlockHeader) + throws ExecutionException, InterruptedException, TimeoutException { if (peer.getAgreedCapabilities().contains(SnapProtocol.SNAP1)) { // could try and retrieve some SNAP data here to check that (e.g. GetByteCodes for a small // contract) - // LOG.info("XXXXXXX" + peer.getConnection().getPeerInfo().getClientId()); - peer.setIsServingSnap(peer.getConnection().getPeerInfo().getClientId().contains("Geth")); + if (snapServerChecker != null) { + // set that peer is a snap server for the test + peer.setIsServingSnap(true); + Boolean isServer; + try { + isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(10L, TimeUnit.SECONDS); + } catch (Exception e) { + LOG.debug("XXXXXX Error checking if peer is a snap server: {}", e); + peer.setIsServingSnap(false); + return; + } + peer.setIsServingSnap(isServer); + final boolean simpleCheck = + peer.getConnection().getPeerInfo().getClientId().contains("Geth"); + if (simpleCheck && !isServer) { + LOG.debug( + "YYYYYYYYYY Found a peer that is Geth but not a snap server: {}", + peer.getLoggableId()); + } else if (!simpleCheck && isServer) { + LOG.debug( + "ZZZZZZZZZZ Found a peer that is NOT Geth but is a snap server: {}", + peer.getLoggableId()); + } + } } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java index b8a55cfcedb..2d34c27d562 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java @@ -62,24 +62,25 @@ public static void trackChainHeadForPeers( new TrailingPeerLimiter(ethContext.getEthPeers(), trailingPeerRequirementsCalculator); final ChainHeadTracker tracker = new ChainHeadTracker(ethContext, protocolSchedule, trailingPeerLimiter, metricsSystem); - ethContext.getEthPeers().subscribeStatusExchanged(tracker); + ethContext.getEthPeers().setChainHeadTracker(tracker); blockchain.observeBlockAdded(trailingPeerLimiter); } - public CompletableFuture onPeerConnected(final EthPeer peer) { + public CompletableFuture getBestHeaderFromPeer(final EthPeer peer) { LOG.atDebug() .setMessage("Requesting chain head info from {}...") .addArgument(peer::getLoggableId) .log(); final CompletableFuture>> bestHeaderFromPeerCompletableFuture = getBestHeaderFromPeerCompletableFuture(peer); - final CompletableFuture future = new CompletableFuture<>(); + final CompletableFuture future = new CompletableFuture<>(); bestHeaderFromPeerCompletableFuture.whenComplete( (peerResult, error) -> { if (peerResult != null && !peerResult.getResult().isEmpty()) { final BlockHeader chainHeadHeader = peerResult.getResult().get(0); peer.chainState().update(chainHeadHeader); trailingPeerLimiter.enforceTrailingPeerLimit(); + future.complete(chainHeadHeader); LOG.atDebug() .setMessage("Retrieved chain head info {} from {}...") .addArgument( @@ -93,8 +94,8 @@ public CompletableFuture onPeerConnected(final EthPeer peer) { .addArgument(error) .log(); peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); + future.complete(null); } - future.complete(null); }); return future; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index 00647142009..83ee8d4724a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -101,6 +101,8 @@ public DefaultSynchronizer( this::calculateTrailingPeerRequirements, metricsSystem); + SnapServerChecker.createSnapServerChecker(ethContext, metricsSystem); + this.blockPropagationManager = terminationCondition.shouldStopDownload() ? Optional.empty() diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java new file mode 100644 index 00000000000..dd4a7025f21 --- /dev/null +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java @@ -0,0 +1,86 @@ +/* + * Copyright ConsenSys AG. + * + * 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.ethereum.eth.sync; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.snap.GetAccountRangeFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask; +import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage; +import org.hyperledger.besu.plugin.services.MetricsSystem; + +import java.util.concurrent.CompletableFuture; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SnapServerChecker { + + private static final Logger LOG = LoggerFactory.getLogger(SnapServerChecker.class); + + private final EthContext ethContext; + private final MetricsSystem metricsSystem; + + public SnapServerChecker(final EthContext ethContext, final MetricsSystem metricsSystem) { + this.ethContext = ethContext; + this.metricsSystem = metricsSystem; + } + + public static void createSnapServerChecker( + final EthContext ethContext, final MetricsSystem metricsSystem) { + final SnapServerChecker checker = new SnapServerChecker(ethContext, metricsSystem); + ethContext.getEthPeers().setSnapServerChecker(checker); + } + + public CompletableFuture check(final EthPeer peer, final BlockHeader header) { + LOG.atDebug() + .setMessage("Checking whether peer {} is a snap server ...") + .addArgument(peer::getLoggableId) + .log(); + final CompletableFuture> + bestHeaderFromPeerCompletableFuture = getAccountRangeFromPeer(peer, header); + final CompletableFuture future = new CompletableFuture<>(); + bestHeaderFromPeerCompletableFuture.whenComplete( + (peerResult, error) -> { + if (peerResult != null) { + if (!peerResult.getResult().accounts().isEmpty() + || !peerResult.getResult().proofs().isEmpty()) { + LOG.atDebug() + .setMessage("Peer {} is a snap server ...") + .addArgument(peer::getLoggableId) + .log(); + future.complete(true); + } else { + LOG.atDebug() + .setMessage("Peer {} is not a snap server ...") + .addArgument(peer::getLoggableId) + .log(); + future.complete(false); + } + } + }); + return future; + } + + public CompletableFuture> + getAccountRangeFromPeer(final EthPeer peer, final BlockHeader header) { + return GetAccountRangeFromPeerTask.forAccountRange( + ethContext, Hash.ZERO, Hash.ZERO, header, metricsSystem) + .assignPeer(peer) + .run(); + } +} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java index 4cde3fd7edc..3cedf3a0fb2 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTrackerTest.java @@ -92,7 +92,7 @@ public void shouldRequestHeaderChainHeadWhenNewPeerConnects( blockchainSetupUtil.getBlockchain(), blockchainSetupUtil.getWorldArchive(), blockchainSetupUtil.getTransactionPool()); - chainHeadTracker.onPeerConnected(respondingPeer.getEthPeer()); + chainHeadTracker.getBestHeaderFromPeer(respondingPeer.getEthPeer()); Assertions.assertThat(chainHeadState().getEstimatedHeight()).isZero(); @@ -112,7 +112,7 @@ public void shouldIgnoreHeadersIfChainHeadHasAlreadyBeenUpdatedWhileWaiting( blockchainSetupUtil.getBlockchain(), blockchainSetupUtil.getWorldArchive(), blockchainSetupUtil.getTransactionPool()); - chainHeadTracker.onPeerConnected(respondingPeer.getEthPeer()); + chainHeadTracker.getBestHeaderFromPeer(respondingPeer.getEthPeer()); // Change the hash of the current known head respondingPeer.getEthPeer().chainState().statusReceived(Hash.EMPTY_TRIE_HASH, Difficulty.ONE); @@ -131,7 +131,7 @@ public void shouldCheckTrialingPeerLimits(final DataStorageFormat storageFormat) blockchainSetupUtil.getBlockchain(), blockchainSetupUtil.getWorldArchive(), blockchainSetupUtil.getTransactionPool()); - chainHeadTracker.onPeerConnected(respondingPeer.getEthPeer()); + chainHeadTracker.getBestHeaderFromPeer(respondingPeer.getEthPeer()); Assertions.assertThat(chainHeadState().getEstimatedHeight()).isZero(); From d3455c9c1f7175000281801c6b987a3e509af9cb Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 29 Feb 2024 17:28:56 +1000 Subject: [PATCH 04/26] some cleanup Signed-off-by: stefan.pingel@consensys.net --- besu/VERSION_METADATA.json | 1 + .../org/hyperledger/besu/RunnerBuilder.java | 5 +- .../controller/BesuControllerBuilder.java | 10 ++- ...onsensusScheduleBesuControllerBuilder.java | 7 +- .../MergeBesuControllerBuilder.java | 7 +- .../TransitionBesuControllerBuilder.java | 7 +- .../bft/protocol/BftProtocolManager.java | 6 -- .../besu/ethereum/eth/manager/EthPeers.java | 19 ++--- .../eth/manager/EthProtocolManager.java | 74 +++++++++---------- .../eth/manager/snap/SnapProtocolManager.java | 6 -- .../AbstractRetryingSwitchingPeerTask.java | 5 +- .../ethereum/eth/sync/SnapServerChecker.java | 13 +++- .../ethereum/p2p/network/ProtocolManager.java | 9 --- 13 files changed, 76 insertions(+), 93 deletions(-) create mode 100644 besu/VERSION_METADATA.json diff --git a/besu/VERSION_METADATA.json b/besu/VERSION_METADATA.json new file mode 100644 index 00000000000..4dc8c82358b --- /dev/null +++ b/besu/VERSION_METADATA.json @@ -0,0 +1 @@ +{"besuVersion":"24.2.0-dev-5449acc7"} \ No newline at end of file diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 8ac07d161b5..cf3fe013110 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -634,9 +634,6 @@ public Runner build() { .flatMap(protocolManager -> protocolManager.getSupportedCapabilities().stream()) .collect(Collectors.toSet()); - final EthPeers ethPeers = besuController.getEthPeers(); - // ethPeers.setProtocolManagers(protocolManagers); - final RlpxConfiguration rlpxConfiguration = RlpxConfiguration.create() .setBindHost(p2pListenInterface) @@ -670,6 +667,8 @@ public Runner build() { .map(nodePerms -> PeerPermissions.combine(nodePerms, bannedNodes)) .orElse(bannedNodes); + final EthPeers ethPeers = besuController.getEthPeers(); + LOG.info("Detecting NAT service."); final boolean fallbackEnabled = natMethod == NatMethod.AUTO || natMethodFallbackEnabled; final NatService natService = new NatService(buildNatManager(natMethod), fallbackEnabled); diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 0b696bb9faf..bb16031e66f 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -740,7 +740,8 @@ public BesuController build() { ethMessages, scheduler, peerValidators, - Optional.empty()); + Optional.empty(), + forkIdManager); final Optional maybeSnapProtocolManager = createSnapProtocolManager(peerValidators, ethPeers, snapMessages, worldStateArchive); @@ -1029,6 +1030,7 @@ protected String getSupportedProtocol() { * @param scheduler the scheduler * @param peerValidators the peer validators * @param mergePeerFilter the merge peer filter + * @param forkIdManager the fork id manager * @return the eth protocol manager */ protected EthProtocolManager createEthProtocolManager( @@ -1041,7 +1043,8 @@ protected EthProtocolManager createEthProtocolManager( final EthMessages ethMessages, final EthScheduler scheduler, final List peerValidators, - final Optional mergePeerFilter) { + final Optional mergePeerFilter, + final ForkIdManager forkIdManager) { return new EthProtocolManager( protocolContext.getBlockchain(), networkId, @@ -1055,8 +1058,7 @@ protected EthProtocolManager createEthProtocolManager( mergePeerFilter, synchronizerConfiguration, scheduler, - genesisConfig.getForkBlockNumbers(), - genesisConfig.getForkTimestamps()); + forkIdManager); } /** diff --git a/besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java index 0754f96ff11..94fc64fe274 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilder.java @@ -53,6 +53,7 @@ import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.p2p.config.SubProtocolConfiguration; import org.hyperledger.besu.ethereum.storage.StorageProvider; @@ -252,7 +253,8 @@ protected EthProtocolManager createEthProtocolManager( final EthMessages ethMessages, final EthScheduler scheduler, final List peerValidators, - final Optional mergePeerFilter) { + final Optional mergePeerFilter, + final ForkIdManager forkIdManager) { return besuControllerBuilderSchedule .get(0L) .createEthProtocolManager( @@ -265,7 +267,8 @@ protected EthProtocolManager createEthProtocolManager( ethMessages, scheduler, peerValidators, - mergePeerFilter); + mergePeerFilter, + forkIdManager); } @Override diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java index 62bce6ac69b..c71f2a7d048 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java @@ -42,6 +42,7 @@ import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ScheduleBasedBlockHeaderFunctions; import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; @@ -95,7 +96,8 @@ protected EthProtocolManager createEthProtocolManager( final EthMessages ethMessages, final EthScheduler scheduler, final List peerValidators, - final Optional mergePeerFilter) { + final Optional mergePeerFilter, + final ForkIdManager forkIdManager) { var mergeContext = protocolContext.getConsensusContext(MergeContext.class); @@ -128,7 +130,8 @@ protected EthProtocolManager createEthProtocolManager( ethMessages, scheduler, peerValidators, - filterToUse); + filterToUse, + forkIdManager); return ethProtocolManager; } diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java index 65aadd46c63..61837ee2544 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java @@ -50,6 +50,7 @@ import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; +import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.storage.StorageProvider; import org.hyperledger.besu.ethereum.trie.forest.pruner.Pruner; @@ -160,7 +161,8 @@ protected EthProtocolManager createEthProtocolManager( final EthMessages ethMessages, final EthScheduler scheduler, final List peerValidators, - final Optional mergePeerFilter) { + final Optional mergePeerFilter, + final ForkIdManager forkIdManager) { return mergeBesuControllerBuilder.createEthProtocolManager( protocolContext, synchronizerConfiguration, @@ -171,7 +173,8 @@ protected EthProtocolManager createEthProtocolManager( ethMessages, scheduler, peerValidators, - mergePeerFilter); + mergePeerFilter, + forkIdManager); } @Override diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java index 9cc4a10b0b6..0c0dd5f67df 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/protocol/BftProtocolManager.java @@ -20,7 +20,6 @@ import org.hyperledger.besu.consensus.common.bft.network.PeerConnectionTracker; import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.ethereum.p2p.network.ProtocolManager; -import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Message; @@ -108,11 +107,6 @@ public void handleNewConnection(final PeerConnection peerConnection) { peers.add(peerConnection); } - @Override - public boolean shouldTryToConnect(final Peer peer, final boolean incoming) { - return false; // for now the EthProtocolManager takes care of this - } - @Override public void handleDisconnect( final PeerConnection peerConnection, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index e16f0c8e6c1..36f91e5b031 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -415,10 +415,6 @@ public boolean shouldTryToConnect(final Peer peer, final boolean inbound) { } else { return false; } - - // ask the protocol managers whether they want to connect to this peer and if none of them want - // to connect, then we don't connect - // return protocolManagers.stream().anyMatch(p -> p.shouldTryToConnect(peer, inbound)); } private boolean alreadyConnectedOrConnecting(final boolean inbound, final Bytes id) { @@ -453,10 +449,6 @@ public void disconnectWorstUselessPeer() { }); } - // public void setProtocolManagers(final List protocolManagers) { - // this.protocolManagers = protocolManagers; - // } - public void setChainHeadTracker(final ChainHeadTracker tracker) { this.tracker = tracker; } @@ -532,25 +524,28 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl // could try and retrieve some SNAP data here to check that (e.g. GetByteCodes for a small // contract) if (snapServerChecker != null) { - // set that peer is a snap server for the test + // set that peer is a snap server for doing the test peer.setIsServingSnap(true); Boolean isServer; try { isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(10L, TimeUnit.SECONDS); } catch (Exception e) { - LOG.debug("XXXXXX Error checking if peer is a snap server: {}", e); + // TODO: change LOG to debug? + LOG.info("XXXXXX Error checking if peer is a snap server: {}", e); peer.setIsServingSnap(false); return; } peer.setIsServingSnap(isServer); + + // TODO: remove the following code. Just here for testing final boolean simpleCheck = peer.getConnection().getPeerInfo().getClientId().contains("Geth"); if (simpleCheck && !isServer) { - LOG.debug( + LOG.info( "YYYYYYYYYY Found a peer that is Geth but not a snap server: {}", peer.getLoggableId()); } else if (!simpleCheck && isServer) { - LOG.debug( + LOG.info( "ZZZZZZZZZZ Found a peer that is NOT Geth but is a snap server: {}", peer.getLoggableId()); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index 49797546b4e..df48076748f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -34,7 +34,6 @@ import org.hyperledger.besu.ethereum.forkid.ForkId; import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.p2p.network.ProtocolManager; -import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection.PeerNotConnected; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; @@ -161,40 +160,40 @@ public EthProtocolManager( ethereumWireProtocolConfiguration.isLegacyEth64ForkIdEnabled())); } - public EthProtocolManager( - final Blockchain blockchain, - final BigInteger networkId, - final WorldStateArchive worldStateArchive, - final TransactionPool transactionPool, - final EthProtocolConfiguration ethereumWireProtocolConfiguration, - final EthPeers ethPeers, - final EthMessages ethMessages, - final EthContext ethContext, - final List peerValidators, - final Optional mergePeerFilter, - final SynchronizerConfiguration synchronizerConfiguration, - final EthScheduler scheduler, - final List blockNumberForks, - final List timestampForks) { - this( - blockchain, - networkId, - worldStateArchive, - transactionPool, - ethereumWireProtocolConfiguration, - ethPeers, - ethMessages, - ethContext, - peerValidators, - mergePeerFilter, - synchronizerConfiguration, - scheduler, - new ForkIdManager( - blockchain, - blockNumberForks, - timestampForks, - ethereumWireProtocolConfiguration.isLegacyEth64ForkIdEnabled())); - } + // public EthProtocolManager( + // final Blockchain blockchain, + // final BigInteger networkId, + // final WorldStateArchive worldStateArchive, + // final TransactionPool transactionPool, + // final EthProtocolConfiguration ethereumWireProtocolConfiguration, + // final EthPeers ethPeers, + // final EthMessages ethMessages, + // final EthContext ethContext, + // final List peerValidators, + // final Optional mergePeerFilter, + // final SynchronizerConfiguration synchronizerConfiguration, + // final EthScheduler scheduler, + // final List blockNumberForks, + // final List timestampForks) { + // this( + // blockchain, + // networkId, + // worldStateArchive, + // transactionPool, + // ethereumWireProtocolConfiguration, + // ethPeers, + // ethMessages, + // ethContext, + // peerValidators, + // mergePeerFilter, + // synchronizerConfiguration, + // scheduler, + // new ForkIdManager( + // blockchain, + // blockNumberForks, + // timestampForks, + // ethereumWireProtocolConfiguration.isLegacyEth64ForkIdEnabled())); + // } public EthContext ethContext() { return ethContext; @@ -395,11 +394,6 @@ public void handleNewConnection(final PeerConnection connection) { LOG.atTrace().setMessage("{}").addArgument(ethPeers::toString).log(); } - @Override - public boolean shouldTryToConnect(final Peer peer, final boolean incoming) { - return false; - } - @Override public void handleDisconnect( final PeerConnection connection, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java index ba14daf157c..60af3d15f88 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java @@ -21,7 +21,6 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; import org.hyperledger.besu.ethereum.p2p.network.ProtocolManager; -import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.AbstractSnapMessageData; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; @@ -138,11 +137,6 @@ public void processMessage(final Capability cap, final Message message) { @Override public void handleNewConnection(final PeerConnection connection) {} - @Override - public boolean shouldTryToConnect(final Peer peer, final boolean incoming) { - return false; // EthManager is taking care of this for now - } - @Override public void handleDisconnect( final PeerConnection connection, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java index b02ef867424..fbcef632ea1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java @@ -64,9 +64,8 @@ protected CompletableFuture executePeerTask(final Optional assignedP final Optional maybePeer = assignedPeer - .filter(u -> getRetryCount() == 1) - .or(this::selectNextPeer); // first try with the assigned peer if present, otherwise - // select a new one from the pool + .filter(u -> getRetryCount() == 1) // first try with the assigned peer if present + .or(this::selectNextPeer); // otherwise select a new one from the pool if (maybePeer.isEmpty()) { LOG.atTrace() diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java index dd4a7025f21..9c3c3b9dddd 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java @@ -52,16 +52,21 @@ public CompletableFuture check(final EthPeer peer, final BlockHeader he .addArgument(peer::getLoggableId) .log(); final CompletableFuture> - bestHeaderFromPeerCompletableFuture = getAccountRangeFromPeer(peer, header); + snapServerCheckCompletableFuture = getAccountRangeFromPeer(peer, header); final CompletableFuture future = new CompletableFuture<>(); - bestHeaderFromPeerCompletableFuture.whenComplete( + snapServerCheckCompletableFuture.whenComplete( (peerResult, error) -> { if (peerResult != null) { if (!peerResult.getResult().accounts().isEmpty() || !peerResult.getResult().proofs().isEmpty()) { - LOG.atDebug() - .setMessage("Peer {} is a snap server ...") + // LOG.atDebug() + // .setMessage("Peer {} is a snap server ...") + // .addArgument(peer::getLoggableId) + // .log(); + LOG.atInfo() + .setMessage("Peer {} is a snap server! getAccountRangeResult: {}") .addArgument(peer::getLoggableId) + .addArgument(peerResult.getResult()) .log(); future.complete(true); } else { diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java index f43d35bdb9f..c61e6d907cd 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/ProtocolManager.java @@ -14,7 +14,6 @@ */ package org.hyperledger.besu.ethereum.p2p.network; -import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Message; @@ -59,14 +58,6 @@ public interface ProtocolManager extends AutoCloseable { */ void handleNewConnection(PeerConnection peerConnection); - /** - * Call this to find out whether we should try to connect to a certain peer - * - * @param peer the peer that we are trying to connect to - * @param incoming true if the connection is incoming - * @return true, if the ProtocolManager wants to connect to the peer, false otherwise - */ - boolean shouldTryToConnect(Peer peer, final boolean incoming); /** * Handles peer disconnects. * From 01f9101abb932ba11427fc464781ea0c9e2652b5 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 29 Feb 2024 17:31:01 +1000 Subject: [PATCH 05/26] add log for snap server check Signed-off-by: stefan.pingel@consensys.net --- .../java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 36f91e5b031..37d08c99f90 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -536,6 +536,7 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl return; } peer.setIsServingSnap(isServer); + LOG.info("Peer {} is a snap server: {}", peer.getLoggableId(), isServer); // TODO: remove the following code. Just here for testing final boolean simpleCheck = From 5d7f1f1316d333f6f5e22a78fb8e1afc234de13c Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Fri, 1 Mar 2024 16:34:49 +1000 Subject: [PATCH 06/26] change the way we handle the max incoming connections Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/manager/EthPeers.java | 84 +++++++++++-------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 37d08c99f90..ee78258e413 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -45,9 +45,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -228,7 +226,7 @@ private boolean registerDisconnect( peer.handleDisconnect(); abortPendingRequestsAssignedToDisconnectedPeers(); if (peer.getReputation().getScore() > USEFULL_PEER_SCORE_THRESHOLD) { - LOG.debug("Disconnected USEFULL peer {}", peer); + LOG.debug("Disconnected USEFUL peer {}", peer); } else { LOG.debug("Disconnected EthPeer {}", peer.getLoggableId()); } @@ -292,7 +290,7 @@ public void dispatchMessage(final EthPeer peer, final EthMessage ethMessage) { @VisibleForTesting void reattemptPendingPeerRequests() { synchronized (this) { - final List peers = streamAvailablePeers().collect(Collectors.toList()); + final List peers = streamAvailablePeers().toList(); final Iterator iterator = pendingRequests.iterator(); while (iterator.hasNext() && peers.stream().anyMatch(EthPeer::hasAvailableRequestCapacity)) { final PendingPeerRequest request = iterator.next(); @@ -410,11 +408,7 @@ public boolean shouldTryToConnect(final Peer peer, final boolean inbound) { return false; } - if (peerCount() < getMaxPeers() || needMoreSnapServers() || canExceedPeerLimits(id)) { - return true; - } else { - return false; - } + return peerCount() < getMaxPeers() || needMoreSnapServers() || canExceedPeerLimits(id); } private boolean alreadyConnectedOrConnecting(final boolean inbound, final Bytes id) { @@ -424,18 +418,16 @@ private boolean alreadyConnectedOrConnecting(final boolean inbound, final Bytes } final List incompleteConnections = getIncompleteConnections(id); if (!incompleteConnections.isEmpty()) { - if (incompleteConnections.stream() - .anyMatch(c -> !c.isDisconnected() && (!inbound || (inbound && c.inboundInitiated())))) { - return true; - } + return incompleteConnections.stream() + .anyMatch(c -> !c.isDisconnected() && (!inbound || (inbound && c.inboundInitiated()))); } return false; } public void disconnectWorstUselessPeer() { streamAvailablePeers() - .sorted(getBestChainComparator()) - .findFirst() + .filter(p -> !canExceedPeerLimits(p.getId())) + .min(getBestChainComparator()) .ifPresent( peer -> { LOG.atDebug() @@ -449,6 +441,24 @@ public void disconnectWorstUselessPeer() { }); } + public void disconnectWorstIncomingUselessPeer() { + streamAvailablePeers() + .filter(p -> p.getConnection().inboundInitiated()) + .filter(p -> !canExceedPeerLimits(p.getId())) + .min(getBestChainComparator()) + .ifPresent( + peer -> { + LOG.atDebug() + .setMessage( + "disconnecting peer {}. Waiting for better peers. Current {} of max {}") + .addArgument(peer::getLoggableId) + .addArgument(this::peerCount) + .addArgument(this::getMaxPeers) + .log(); + peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); + }); + } + public void setChainHeadTracker(final ChainHeadTracker tracker) { this.tracker = tracker; } @@ -518,8 +528,7 @@ private void ethPeerStatusExchanged(final EthPeer peer) { }); } - private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBlockHeader) - throws ExecutionException, InterruptedException, TimeoutException { + private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBlockHeader) { if (peer.getAgreedCapabilities().contains(SnapProtocol.SNAP1)) { // could try and retrieve some SNAP data here to check that (e.g. GetByteCodes for a small // contract) @@ -531,7 +540,7 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(10L, TimeUnit.SECONDS); } catch (Exception e) { // TODO: change LOG to debug? - LOG.info("XXXXXX Error checking if peer is a snap server: {}", e); + LOG.info("XXXXXX Error checking if peer is a snap server.", e); peer.setIsServingSnap(false); return; } @@ -641,8 +650,8 @@ private boolean shouldLimitRemoteConnections() { private long countUntrustedRemotelyInitiatedConnections() { return activeConnections.values().stream() - .map(ep -> ep.getConnection()) - .filter(c -> c.inboundInitiated()) + .map(EthPeer::getConnection) + .filter(PeerConnection::inboundInitiated) .filter(c -> !c.isDisconnected()) .filter(conn -> !canExceedPeerLimits(conn.getPeer().getId())) .count(); @@ -652,10 +661,16 @@ private void onCacheRemoval( final RemovalNotification removalNotification) { if (removalNotification.wasEvicted()) { final PeerConnection peerConnectionRemoved = removalNotification.getKey(); - final PeerConnection peerConnectionOfEthPeer = removalNotification.getValue().getConnection(); - if (!peerConnectionRemoved.equals(peerConnectionOfEthPeer)) { - // If this connection is not the connection of the EthPeer by now we can disconnect - peerConnectionRemoved.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); + final EthPeer peer = removalNotification.getValue(); + if (peer == null) { + return; + } + final PeerConnection peerConnectionOfEthPeer = peer.getConnection(); + if (peerConnectionRemoved != null) { + if (!peerConnectionRemoved.equals(peerConnectionOfEthPeer)) { + // If this connection is not the connection of the EthPeer by now we can disconnect + peerConnectionRemoved.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); + } } } } @@ -665,7 +680,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { // Figure out whether we want to keep this peer to add it to the active connections. final PeerConnection connection = peer.getConnection(); if (activeConnections.containsValue(peer)) { - // connection.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); + // connection.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); return false; } final Bytes id = peer.getId(); @@ -674,6 +689,12 @@ private boolean addPeerToEthPeers(final EthPeer peer) { if (peerCount() >= peerUpperBound) { if (needMoreSnapServers()) { disconnectNonSnapServerPeerOrLeastUseful(); + } else if (!remoteConnectionLimitReached() && !peer.getConnection().inboundInitiated()) { + if (peer.isServingSnap()) { + disconnectWorstIncomingUselessPeer(); + } else { + disconnectNonSnapServerPeerOrLeastUseful(); + } } else if (!canExceedPeerLimits(id)) { LOG.trace( "Too many peers. Disconnect connection: {}, max connections {}", @@ -683,17 +704,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { return false; } } - // Disconnect if too many remotely-initiated connections - if (connection.inboundInitiated() - && !canExceedPeerLimits(id) - && remoteConnectionLimitReached()) { - LOG.info( - "Too many remotely-initiated connections. Disconnect incoming connection: {}, maxRemote={}", - connection, - maxRemotelyInitiatedConnections); - connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); - return false; - } + final boolean added = (activeConnections.putIfAbsent(id, peer) == null); if (added) { LOG.debug("Added peer {} with connection {} to activeConnections", id, connection); @@ -719,6 +730,7 @@ private boolean needMoreSnapServers() { private void disconnectNonSnapServerPeerOrLeastUseful() { activeConnections.values().stream() .filter(p -> !p.isServingSnap()) + .filter(p -> !canExceedPeerLimits(p.getId())) .min(MOST_USEFUL_PEER) .ifPresentOrElse( p -> p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS), From 80534d39e24b1d40d9fc2502853b1d3e77aab161 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 5 Mar 2024 18:36:01 +1000 Subject: [PATCH 07/26] fix unit tests Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/forkid/ForkIdManager.java | 6 +++- .../besu/ethereum/eth/manager/EthPeer.java | 10 +++--- .../besu/ethereum/eth/manager/EthPeers.java | 36 ++++++++++--------- .../ethereum/eth/manager/EthPeersTest.java | 36 ++++++++++++++++--- .../manager/EthProtocolManagerTestUtil.java | 31 ++++++++++++++++ .../eth/manager/RespondingEthPeer.java | 22 ++++++++++-- .../ethereum/eth/transactions/TestNode.java | 18 ++++++++++ .../TransactionPoolFactoryTest.java | 4 +++ 8 files changed, 134 insertions(+), 29 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java index bae8cfc339e..859fb918a4d 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java @@ -58,7 +58,11 @@ public ForkIdManager( checkNotNull(blockchain); checkNotNull(blockNumberForks); this.chainHeadSupplier = blockchain::getChainHeadHeader; - this.genesisHash = blockchain.getGenesisBlock().getHash(); + try { + this.genesisHash = blockchain.getGenesisBlock().getHash(); + } catch (Exception e) { + throw new RuntimeException(e); + } this.blockNumbersForkIds = new ArrayList<>(); this.timestampsForkIds = new ArrayList<>(); this.legacyEth64 = legacyEth64; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index 185787eae40..ab547b03ab4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -581,10 +581,12 @@ public void registerHeight(final Hash blockHash, final long height) { } public int outstandingRequests() { - return requestManagers.values().stream() - .flatMap(m -> m.values().stream()) - .mapToInt(RequestManager::outstandingRequests) - .sum(); + final int sum = + requestManagers.values().stream() + .flatMap(m -> m.values().stream()) + .mapToInt(RequestManager::outstandingRequests) + .sum(); + return sum; } public long getLastRequestTimestamp() { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index ee78258e413..56da6fce312 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -443,20 +443,20 @@ public void disconnectWorstUselessPeer() { public void disconnectWorstIncomingUselessPeer() { streamAvailablePeers() - .filter(p -> p.getConnection().inboundInitiated()) - .filter(p -> !canExceedPeerLimits(p.getId())) - .min(getBestChainComparator()) - .ifPresent( - peer -> { - LOG.atDebug() - .setMessage( - "disconnecting peer {}. Waiting for better peers. Current {} of max {}") - .addArgument(peer::getLoggableId) - .addArgument(this::peerCount) - .addArgument(this::getMaxPeers) - .log(); - peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); - }); + .filter(p -> p.getConnection().inboundInitiated()) + .filter(p -> !canExceedPeerLimits(p.getId())) + .min(getBestChainComparator()) + .ifPresent( + peer -> { + LOG.atDebug() + .setMessage( + "disconnecting peer {}. Waiting for better peers. Current {} of max {}") + .addArgument(peer::getLoggableId) + .addArgument(this::peerCount) + .addArgument(this::getMaxPeers) + .log(); + peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); + }); } public void setChainHeadTracker(final ChainHeadTracker tracker) { @@ -490,24 +490,26 @@ private void ethPeerStatusExchanged(final EthPeer peer) { // Find out what the EthPeer block height is and whether it can serve snap data (if we are doing // snap sync) LOG.debug("Peer {} status exchanged", peer); + assert tracker != null : "ChainHeadTracker must be set before EthPeers can be used"; CompletableFuture future = tracker.getBestHeaderFromPeer(peer); future.whenComplete( - (peersHeadBlockHeader, error) -> { - if (peersHeadBlockHeader == null) { + (peerHeadBlockHeader, error) -> { + if (peerHeadBlockHeader == null) { LOG.debug( "Failed to retrieve chain head info. Disconnecting {}... {}", peer.getLoggableId(), error); peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); } else { + peer.chainState().updateHeightEstimate(peerHeadBlockHeader.getNumber()); CompletableFuture isServingSnapFuture; if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { isServingSnapFuture = CompletableFuture.runAsync( () -> { try { - checkIsSnapServer(peer, peersHeadBlockHeader); + checkIsSnapServer(peer, peerHeadBlockHeader); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java index 59c02941428..2b5445a95c2 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java @@ -21,15 +21,18 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; import org.hyperledger.besu.ethereum.eth.manager.exceptions.PeerDisconnectedException; import org.hyperledger.besu.ethereum.eth.messages.NodeDataMessage; +import org.hyperledger.besu.ethereum.eth.sync.ChainHeadTracker; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection.PeerNotConnected; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason; @@ -37,6 +40,7 @@ import java.util.Optional; import java.util.OptionalLong; import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; import org.junit.jupiter.api.BeforeEach; @@ -56,6 +60,11 @@ public void setup() throws Exception { when(peerRequest.sendRequest(any())).thenReturn(responseStream); ethProtocolManager = EthProtocolManagerTestUtil.create(); ethPeers = ethProtocolManager.ethContext().getEthPeers(); + final ChainHeadTracker mock = mock(ChainHeadTracker.class); + final BlockHeader blockHeader = mock(BlockHeader.class); + when(mock.getBestHeaderFromPeer(any())) + .thenReturn(CompletableFuture.completedFuture(blockHeader)); + ethPeers.setChainHeadTracker(mock); } @Test @@ -112,6 +121,9 @@ public void comparesPeersWithTdAndNoHeight() { @Test public void shouldExecutePeerRequestImmediatelyWhenPeerIsAvailable() throws Exception { final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + + when(peerRequest.isEthPeerSuitable(peer.getEthPeer())).thenReturn(true); + final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 10, Optional.empty()); @@ -127,6 +139,8 @@ public void shouldUseLeastBusyPeerForRequest() throws Exception { EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); useRequestSlot(workingPeer.getEthPeer()); + when(peerRequest.isEthPeerSuitable(any())).thenReturn(true); + final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 10, Optional.empty()); @@ -147,6 +161,8 @@ public void shouldUseLeastRecentlyUsedPeerWhenBothHaveSameNumberOfOutstandingReq assertThat(leastRecentlyUsedPeer.getEthPeer().outstandingRequests()) .isEqualTo(mostRecentlyUsedPeer.getEthPeer().outstandingRequests()); + when(peerRequest.isEthPeerSuitable(any())).thenReturn(true); + final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 10, Optional.empty()); @@ -180,10 +196,13 @@ public void shouldFailWhenAllPeersWithSufficientHeightHaveDisconnected() throws EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); useAllAvailableCapacity(suitablePeer.getEthPeer()); + when(peerRequest.isEthPeerSuitable(suitablePeer.getEthPeer())).thenReturn(true); + final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 200, Optional.empty()); - verifyNoInteractions(peerRequest); + verify(peerRequest, times(0)).sendRequest(suitablePeer.getEthPeer()); + assertNotDone(pendingRequest); suitablePeer.disconnect(DisconnectReason.TOO_MANY_PEERS); @@ -194,6 +213,8 @@ public void shouldFailWhenAllPeersWithSufficientHeightHaveDisconnected() throws public void shouldFailWithPeerNotConnectedIfPeerRequestThrows() throws Exception { final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); when(peerRequest.sendRequest(peer.getEthPeer())).thenThrow(new PeerNotConnected("Oh dear")); + when(peerRequest.isEthPeerSuitable(any())).thenReturn(true); + final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 100, Optional.empty()); @@ -205,9 +226,11 @@ public void shouldDelayExecutionUntilPeerHasCapacity() throws Exception { final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); useAllAvailableCapacity(peer.getEthPeer()); + when(peerRequest.isEthPeerSuitable(any())).thenReturn(true); + final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 100, Optional.empty()); - verifyNoInteractions(peerRequest); + verify(peerRequest, times(0)).sendRequest(peer.getEthPeer()); freeUpCapacity(peer.getEthPeer()); @@ -221,11 +244,12 @@ public void shouldDelayExecutionUntilPeerWithSufficientHeightHasCapacity() throw EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 10); final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + when(peerRequest.isEthPeerSuitable(peer.getEthPeer())).thenReturn(true); useAllAvailableCapacity(peer.getEthPeer()); final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 100, Optional.empty()); - verifyNoInteractions(peerRequest); + verify(peerRequest, times(0)).sendRequest(peer.getEthPeer()); freeUpCapacity(peer.getEthPeer()); @@ -238,15 +262,17 @@ public void shouldNotExecuteAbortedRequest() throws Exception { final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); useAllAvailableCapacity(peer.getEthPeer()); + when(peerRequest.isEthPeerSuitable(peer.getEthPeer())).thenReturn(true); + final PendingPeerRequest pendingRequest = ethPeers.executePeerRequest(peerRequest, 100, Optional.empty()); - verifyNoInteractions(peerRequest); + verify(peerRequest, times(0)).sendRequest(peer.getEthPeer()); pendingRequest.abort(); freeUpCapacity(peer.getEthPeer()); - verifyNoInteractions(peerRequest); + verify(peerRequest, times(0)).sendRequest(peer.getEthPeer()); assertRequestFailure(pendingRequest, CancellationException.class); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java index 496a3e7f30c..b4da01d8a95 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java @@ -16,12 +16,15 @@ import static com.google.common.base.Preconditions.checkArgument; import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import org.hyperledger.besu.config.GenesisConfigFile; +import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.chain.ChainHead; import org.hyperledger.besu.ethereum.chain.GenesisState; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.ProtocolScheduleFixture; @@ -29,6 +32,7 @@ import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.manager.snap.SnapProtocolManager; import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator; +import org.hyperledger.besu.ethereum.eth.sync.ChainHeadTracker; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; @@ -47,8 +51,10 @@ import java.util.Collections; import java.util.Optional; import java.util.OptionalLong; +import java.util.concurrent.CompletableFuture; import org.apache.tuweni.bytes.Bytes; +import org.mockito.Mockito; public class EthProtocolManagerTestUtil { @@ -90,6 +96,10 @@ public static EthProtocolManager create( false, SyncMode.X_SNAP, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); + + final ChainHeadTracker chainHeadTrackerMock = getChainHeadTrackerMock(); + peers.setChainHeadTracker(chainHeadTrackerMock); + final EthMessages messages = new EthMessages(); final EthScheduler ethScheduler = new DeterministicEthScheduler(TimeoutPolicy.NEVER_TIMEOUT); final EthContext ethContext = new EthContext(peers, messages, ethScheduler); @@ -142,6 +152,8 @@ public static EthProtocolManager create( final EthContext ethContext, final ForkIdManager forkIdManager) { + ethPeers.setChainHeadTracker(getChainHeadTrackerMock()); + final BigInteger networkId = BigInteger.ONE; return new EthProtocolManager( blockchain, @@ -213,6 +225,10 @@ public static EthProtocolManager create( new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthMessages messages = new EthMessages(); + final ChainHeadTracker chtMock = getChainHeadTrackerMock(); + + peers.setChainHeadTracker(chtMock); + return create( blockchain, ethScheduler, @@ -224,6 +240,17 @@ public static EthProtocolManager create( new EthContext(peers, messages, ethScheduler)); } + public static ChainHeadTracker getChainHeadTrackerMock() { + final ChainHeadTracker chtMock = mock(ChainHeadTracker.class); + final BlockHeader blockHeaderMock = mock(BlockHeader.class); + Mockito.lenient() + .when(chtMock.getBestHeaderFromPeer(any())) + .thenReturn(CompletableFuture.completedFuture(blockHeaderMock)); + Mockito.lenient().when(blockHeaderMock.getNumber()).thenReturn(0L); + Mockito.lenient().when(blockHeaderMock.getStateRoot()).thenReturn(Hash.ZERO); + return chtMock; + } + public static EthProtocolManager create( final ProtocolSchedule protocolSchedule, final Blockchain blockchain, @@ -279,6 +306,10 @@ public static EthProtocolManager create( false, SyncMode.X_SNAP, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); + + final ChainHeadTracker chainHeadTrackerMock = getChainHeadTrackerMock(); + peers.setChainHeadTracker(chainHeadTrackerMock); + final EthMessages messages = new EthMessages(); return create( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java index 7427545984b..c5bac5835f2 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java @@ -121,7 +121,8 @@ private static RespondingEthPeer create( final Hash chainHeadHash, final Difficulty totalDifficulty, final OptionalLong estimatedHeight, - final List peerValidators) { + final List peerValidators, + final boolean isServingSnap) { final EthPeers ethPeers = ethProtocolManager.ethContext().getEthPeers(); final Set caps = new HashSet<>(Collections.singletonList(EthProtocol.ETH63)); @@ -130,10 +131,20 @@ private static RespondingEthPeer create( new MockPeerConnection( caps, (cap, msg, conn) -> outgoingMessages.add(new OutgoingMessage(cap, msg))); ethPeers.registerNewConnection(peerConnection, peerValidators); + final int before = ethPeers.peerCount(); final EthPeer peer = ethPeers.peer(peerConnection); peer.registerStatusReceived(chainHeadHash, totalDifficulty, 63, peerConnection); estimatedHeight.ifPresent(height -> peer.chainState().update(chainHeadHash, height)); peer.registerStatusSent(peerConnection); + while (ethPeers.peerCount() + <= before) { // this is needed to make sure that the peer is added to the active connections + try { + Thread.sleep(100L); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + peer.setIsServingSnap(isServingSnap); return new RespondingEthPeer( ethProtocolManager, snapProtocolManager, peerConnection, peer, outgoingMessages); @@ -396,6 +407,7 @@ public static class Builder { private Difficulty totalDifficulty = Difficulty.of(1000L); private OptionalLong estimatedHeight = OptionalLong.of(1000L); private final List peerValidators = new ArrayList<>(); + private boolean isServingSnap = false; public RespondingEthPeer build() { checkNotNull(ethProtocolManager, "Must configure EthProtocolManager"); @@ -406,7 +418,8 @@ public RespondingEthPeer build() { chainHeadHash, totalDifficulty, estimatedHeight, - peerValidators); + peerValidators, + isServingSnap); } public Builder ethProtocolManager(final EthProtocolManager ethProtocolManager) { @@ -444,6 +457,11 @@ public Builder estimatedHeight(final long estimatedHeight) { return this; } + public Builder isServingSnap(final boolean isServingSnap) { + this.isServingSnap = isServingSnap; + return this; + } + public Builder peerValidators(final List peerValidators) { checkNotNull(peerValidators); this.peerValidators.addAll(peerValidators); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java index d176a497da8..4c9035367ee 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java @@ -18,6 +18,7 @@ import static java.util.Objects.requireNonNull; import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain; import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryWorldStateArchive; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -30,6 +31,7 @@ import org.hyperledger.besu.ethereum.chain.BadBlockManager; import org.hyperledger.besu.ethereum.chain.GenesisState; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions; import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider; import org.hyperledger.besu.ethereum.core.MiningParameters; @@ -42,6 +44,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.sync.ChainHeadTracker; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; @@ -79,6 +82,7 @@ import io.vertx.core.Vertx; import org.apache.tuweni.bytes.Bytes; +import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -158,6 +162,9 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod SyncMode.X_SNAP, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); + final ChainHeadTracker mockCHT = getChainHeadTracker(); + ethPeers.setChainHeadTracker(mockCHT); + final EthScheduler scheduler = new EthScheduler(1, 1, 1, metricsSystem); final EthContext ethContext = new EthContext(ethPeers, ethMessages, scheduler); @@ -193,6 +200,7 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod NetworkRunner.builder() .subProtocols(EthProtocol.get()) .protocolManagers(singletonList(ethProtocolManager)) + .ethPeersShouldConnect((p, d) -> true) .network( capabilities -> DefaultP2PNetwork.builder() @@ -221,6 +229,16 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod selfPeer = DefaultPeer.fromEnodeURL(network.getLocalEnode().get()); } + private static ChainHeadTracker getChainHeadTracker() { + final ChainHeadTracker mockCHT = mock(ChainHeadTracker.class); + final BlockHeader mockBlockHeader = mock(BlockHeader.class); + Mockito.lenient().when(mockBlockHeader.getNumber()).thenReturn(0L); + Mockito.lenient() + .when(mockCHT.getBestHeaderFromPeer(any())) + .thenReturn(CompletableFuture.completedFuture(mockBlockHeader)); + return mockCHT; + } + public Bytes id() { return nodeKey.getPublicKey().getEncodedBytes(); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java index d0af265493a..74999ddaac7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java @@ -31,6 +31,7 @@ import org.hyperledger.besu.ethereum.chain.BadBlockManager; import org.hyperledger.besu.ethereum.chain.BlockAddedObserver; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; @@ -99,6 +100,9 @@ public class TransactionPoolFactoryTest { @BeforeEach public void setup() { when(blockchain.getBlockHashByNumber(anyLong())).thenReturn(Optional.of(mock(Hash.class))); + final Block mockBlock = mock(Block.class); + when(mockBlock.getHash()).thenReturn(Hash.ZERO); + when(blockchain.getGenesisBlock()).thenReturn(mockBlock); when(context.getBlockchain()).thenReturn(blockchain); final NodeMessagePermissioningProvider nmpp = (destinationEnode, code) -> true; From ea43fd0a812247ba8246821351a2efcfb450ccdd Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 7 Mar 2024 14:17:44 +1000 Subject: [PATCH 08/26] fix acceptance test Signed-off-by: stefan.pingel@consensys.net --- .../acceptance/pubsub/NewPendingTransactionAcceptanceTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/pubsub/NewPendingTransactionAcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/pubsub/NewPendingTransactionAcceptanceTest.java index 4ae94924b25..29677dedf18 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/pubsub/NewPendingTransactionAcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/pubsub/NewPendingTransactionAcceptanceTest.java @@ -44,6 +44,8 @@ public void setUp() throws Exception { accountOne = accounts.createAccount("account-one"); minerWebSocket = new WebSocket(vertx, minerNode.getConfiguration()); archiveWebSocket = new WebSocket(vertx, archiveNode.getConfiguration()); + // to make sure that the transaction pool is active: + archiveNode.verify(eth.syncingStatus(false)); } @AfterEach From de6899f972c123b8fa2e68d6d96edc3c8f1c2b13 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 12 Apr 2024 10:57:46 +1000 Subject: [PATCH 09/26] extra logging from #6924 Signed-off-by: Sally MacFarlane --- .../eth/sync/snapsync/RequestDataStep.java | 354 ++++++++++-------- 1 file changed, 190 insertions(+), 164 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java index f866f4c41ba..86a31d6a7a6 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java @@ -64,12 +64,12 @@ public class RequestDataStep { private final WorldStateProofProvider worldStateProofProvider; public RequestDataStep( - final EthContext ethContext, - final WorldStateStorageCoordinator worldStateStorageCoordinator, - final SnapSyncProcessState fastSyncState, - final SnapWorldDownloadState downloadState, - final SnapSyncConfiguration snapSyncConfiguration, - final MetricsSystem metricsSystem) { + final EthContext ethContext, + final WorldStateStorageCoordinator worldStateStorageCoordinator, + final SnapSyncProcessState fastSyncState, + final SnapWorldDownloadState downloadState, + final SnapSyncConfiguration snapSyncConfiguration, + final MetricsSystem metricsSystem) { this.worldStateStorageCoordinator = worldStateStorageCoordinator; this.fastSyncState = fastSyncState; this.downloadState = downloadState; @@ -80,165 +80,191 @@ public RequestDataStep( } public CompletableFuture> requestAccount( - final Task requestTask) { + final Task requestTask) { final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final AccountRangeDataRequest accountDataRequest = - (AccountRangeDataRequest) requestTask.getData(); + (AccountRangeDataRequest) requestTask.getData(); final EthTask getAccountTask = - RetryingGetAccountRangeFromPeerTask.forAccountRange( - ethContext, - accountDataRequest.getStartKeyHash(), - accountDataRequest.getEndKeyHash(), - blockHeader, - metricsSystem); + RetryingGetAccountRangeFromPeerTask.forAccountRange( + ethContext, + accountDataRequest.getStartKeyHash(), + accountDataRequest.getEndKeyHash(), + blockHeader, + metricsSystem); downloadState.addOutstandingTask(getAccountTask); return getAccountTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getAccountTask); - accountDataRequest.setRootHash(blockHeader.getStateRoot()); - accountDataRequest.addResponse( - worldStateProofProvider, response.accounts(), response.proofs()); - } - return requestTask; - }); + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getAccountTask); + accountDataRequest.setRootHash(blockHeader.getStateRoot()); + accountDataRequest.addResponse( + worldStateProofProvider, response.accounts(), response.proofs()); + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling account download accounts ({} - {}) task: {}") + .addArgument(accountDataRequest.getStartKeyHash()) + .addArgument(accountDataRequest.getEndKeyHash()) + .addArgument(error) + .log(); + } + return requestTask; + }); } public CompletableFuture>> requestStorage( - final List> requestTasks) { + final List> requestTasks) { final List accountHashes = - requestTasks.stream() - .map(Task::getData) - .map(StorageRangeDataRequest.class::cast) - .map(StorageRangeDataRequest::getAccountHash) - .collect(Collectors.toList()); + requestTasks.stream() + .map(Task::getData) + .map(StorageRangeDataRequest.class::cast) + .map(StorageRangeDataRequest::getAccountHash) + .collect(Collectors.toList()); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final Bytes32 minRange = - requestTasks.size() == 1 - ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getStartKeyHash() - : RangeManager.MIN_RANGE; + requestTasks.size() == 1 + ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getStartKeyHash() + : RangeManager.MIN_RANGE; final Bytes32 maxRange = - requestTasks.size() == 1 - ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getEndKeyHash() - : RangeManager.MAX_RANGE; + requestTasks.size() == 1 + ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getEndKeyHash() + : RangeManager.MAX_RANGE; final EthTask getStorageRangeTask = - RetryingGetStorageRangeFromPeerTask.forStorageRange( - ethContext, accountHashes, minRange, maxRange, blockHeader, metricsSystem); + RetryingGetStorageRangeFromPeerTask.forStorageRange( + ethContext, accountHashes, minRange, maxRange, blockHeader, metricsSystem); downloadState.addOutstandingTask(getStorageRangeTask); return getStorageRangeTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getStorageRangeTask); - final ArrayDeque> slots = new ArrayDeque<>(); - // Check if we have an empty range + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getStorageRangeTask); + final ArrayDeque> slots = new ArrayDeque<>(); + // Check if we have an empty range - /* - * Checks if the response represents an "empty range". - * - * An "empty range" is defined as a response where at least one proof exists - * and either no slots are present, or the first slot is empty - */ - try { - final boolean isEmptyRange = - (response.slots().isEmpty() || response.slots().get(0).isEmpty()) - && !response.proofs().isEmpty(); - if (isEmptyRange) { // empty range detected - slots.add(new TreeMap<>()); - } else { - slots.addAll(response.slots()); - } - for (int i = 0; i < slots.size(); i++) { - final StorageRangeDataRequest request = - (StorageRangeDataRequest) requestTasks.get(i).getData(); - request.setRootHash(blockHeader.getStateRoot()); - request.addResponse( - downloadState, - worldStateProofProvider, - slots.get(i), - i < slots.size() - 1 ? new ArrayDeque<>() : response.proofs()); - } - } catch (final Exception e) { - LOG.error("Error while processing storage range response", e); - } - } - return requestTasks; - }); + /* + * Checks if the response represents an "empty range". + * + * An "empty range" is defined as a response where at least one proof exists + * and either no slots are present, or the first slot is empty + */ + try { + final boolean isEmptyRange = + (response.slots().isEmpty() || response.slots().get(0).isEmpty()) + && !response.proofs().isEmpty(); + if (isEmptyRange) { // empty range detected + slots.add(new TreeMap<>()); + } else { + slots.addAll(response.slots()); + } + for (int i = 0; i < slots.size(); i++) { + final StorageRangeDataRequest request = + (StorageRangeDataRequest) requestTasks.get(i).getData(); + request.setRootHash(blockHeader.getStateRoot()); + request.addResponse( + downloadState, + worldStateProofProvider, + slots.get(i), + i < slots.size() - 1 ? new ArrayDeque<>() : response.proofs()); + } + } catch (final Exception e) { + LOG.error("Error while processing storage range response", e); + } + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling storage range request task: {}") + .addArgument(error) + .log(); + } + return requestTasks; + }); } public CompletableFuture>> requestCode( - final List> requestTasks) { + final List> requestTasks) { final List codeHashes = - requestTasks.stream() - .map(Task::getData) - .map(BytecodeRequest.class::cast) - .map(BytecodeRequest::getCodeHash) - .distinct() - .collect(Collectors.toList()); + requestTasks.stream() + .map(Task::getData) + .map(BytecodeRequest.class::cast) + .map(BytecodeRequest::getCodeHash) + .distinct() + .collect(Collectors.toList()); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final EthTask> getByteCodeTask = - RetryingGetBytecodeFromPeerTask.forByteCode( - ethContext, codeHashes, blockHeader, metricsSystem); + RetryingGetBytecodeFromPeerTask.forByteCode( + ethContext, codeHashes, blockHeader, metricsSystem); downloadState.addOutstandingTask(getByteCodeTask); return getByteCodeTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getByteCodeTask); - for (Task requestTask : requestTasks) { - final BytecodeRequest request = (BytecodeRequest) requestTask.getData(); - request.setRootHash(blockHeader.getStateRoot()); - if (response.containsKey(request.getCodeHash())) { - request.setCode(response.get(request.getCodeHash())); - } - } - } - return requestTasks; - }); + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getByteCodeTask); + for (Task requestTask : requestTasks) { + final BytecodeRequest request = (BytecodeRequest) requestTask.getData(); + request.setRootHash(blockHeader.getStateRoot()); + if (response.containsKey(request.getCodeHash())) { + request.setCode(response.get(request.getCodeHash())); + } + } + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling code request task: {}") + .addArgument(error) + .log(); + } + return requestTasks; + }); } public CompletableFuture>> requestTrieNodeByPath( - final List> requestTasks) { + final List> requestTasks) { final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final Map> message = new HashMap<>(); requestTasks.stream() - .map(Task::getData) - .map(TrieNodeHealingRequest.class::cast) - .map(TrieNodeHealingRequest::getTrieNodePath) - .forEach( - path -> { - final List bytes = - message.computeIfAbsent(path.get(0), k -> Lists.newArrayList()); - if (path.size() > 1) { - bytes.add(path.get(1)); - } - }); + .map(Task::getData) + .map(TrieNodeHealingRequest.class::cast) + .map(TrieNodeHealingRequest::getTrieNodePath) + .forEach( + path -> { + final List bytes = + message.computeIfAbsent(path.get(0), k -> Lists.newArrayList()); + if (path.size() > 1) { + bytes.add(path.get(1)); + } + }); final EthTask> getTrieNodeFromPeerTask = - RetryingGetTrieNodeFromPeerTask.forTrieNodes( - ethContext, message, blockHeader, metricsSystem); + RetryingGetTrieNodeFromPeerTask.forTrieNodes( + ethContext, message, blockHeader, metricsSystem); downloadState.addOutstandingTask(getTrieNodeFromPeerTask); return getTrieNodeFromPeerTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getTrieNodeFromPeerTask); - for (final Task task : requestTasks) { - final TrieNodeHealingRequest request = (TrieNodeHealingRequest) task.getData(); - final Bytes matchingData = response.get(request.getPathId()); - if (matchingData != null) { - request.setData(matchingData); - } - } - } - return requestTasks; - }); + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getTrieNodeFromPeerTask); + for (final Task task : requestTasks) { + final TrieNodeHealingRequest request = (TrieNodeHealingRequest) task.getData(); + final Bytes matchingData = response.get(request.getPathId()); + if (matchingData != null) { + request.setData(matchingData); + } + } + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling trie node request task: {}") + .addArgument(error) + .log(); + } + return requestTasks; + }); } /** @@ -249,34 +275,34 @@ public CompletableFuture>> requestTrieNodeByPath( * @return data request with local accounts */ public CompletableFuture> requestLocalFlatAccounts( - final Task requestTask) { + final Task requestTask) { final AccountFlatDatabaseHealingRangeRequest accountDataRequest = - (AccountFlatDatabaseHealingRangeRequest) requestTask.getData(); + (AccountFlatDatabaseHealingRangeRequest) requestTask.getData(); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); // retrieve accounts from flat database final TreeMap accounts = new TreeMap<>(); worldStateStorageCoordinator.applyOnMatchingFlatMode( - FlatDbMode.FULL, - onBonsai -> { - accounts.putAll( - onBonsai.streamFlatAccounts( - accountDataRequest.getStartKeyHash(), - accountDataRequest.getEndKeyHash(), - snapSyncConfiguration.getLocalFlatAccountCountToHealPerRequest())); - }); + FlatDbMode.FULL, + onBonsai -> { + accounts.putAll( + onBonsai.streamFlatAccounts( + accountDataRequest.getStartKeyHash(), + accountDataRequest.getEndKeyHash(), + snapSyncConfiguration.getLocalFlatAccountCountToHealPerRequest())); + }); final List proofs = new ArrayList<>(); if (!accounts.isEmpty()) { // generate range proof if accounts are present proofs.addAll( - worldStateProofProvider.getAccountProofRelatedNodes( - blockHeader.getStateRoot(), accounts.firstKey())); + worldStateProofProvider.getAccountProofRelatedNodes( + blockHeader.getStateRoot(), accounts.firstKey())); proofs.addAll( - worldStateProofProvider.getAccountProofRelatedNodes( - blockHeader.getStateRoot(), accounts.lastKey())); + worldStateProofProvider.getAccountProofRelatedNodes( + blockHeader.getStateRoot(), accounts.lastKey())); } accountDataRequest.setRootHash(blockHeader.getStateRoot()); @@ -293,10 +319,10 @@ public CompletableFuture> requestLocalFlatAccounts( * @return data request with local slots */ public CompletableFuture> requestLocalFlatStorages( - final Task requestTask) { + final Task requestTask) { final StorageFlatDatabaseHealingRangeRequest storageDataRequest = - (StorageFlatDatabaseHealingRangeRequest) requestTask.getData(); + (StorageFlatDatabaseHealingRangeRequest) requestTask.getData(); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); storageDataRequest.setRootHash(blockHeader.getStateRoot()); @@ -304,32 +330,32 @@ public CompletableFuture> requestLocalFlatStorages( // retrieve slots from flat database final TreeMap slots = new TreeMap<>(); worldStateStorageCoordinator.applyOnMatchingFlatMode( - FlatDbMode.FULL, - onBonsai -> { - slots.putAll( - onBonsai.streamFlatStorages( - storageDataRequest.getAccountHash(), - storageDataRequest.getStartKeyHash(), - storageDataRequest.getEndKeyHash(), - snapSyncConfiguration.getLocalFlatStorageCountToHealPerRequest())); - }); + FlatDbMode.FULL, + onBonsai -> { + slots.putAll( + onBonsai.streamFlatStorages( + storageDataRequest.getAccountHash(), + storageDataRequest.getStartKeyHash(), + storageDataRequest.getEndKeyHash(), + snapSyncConfiguration.getLocalFlatStorageCountToHealPerRequest())); + }); final List proofs = new ArrayList<>(); if (!slots.isEmpty()) { // generate range proof if slots are present proofs.addAll( - worldStateProofProvider.getStorageProofRelatedNodes( - storageDataRequest.getStorageRoot(), - storageDataRequest.getAccountHash(), - slots.firstKey())); + worldStateProofProvider.getStorageProofRelatedNodes( + storageDataRequest.getStorageRoot(), + storageDataRequest.getAccountHash(), + slots.firstKey())); proofs.addAll( - worldStateProofProvider.getStorageProofRelatedNodes( - storageDataRequest.getStorageRoot(), - storageDataRequest.getAccountHash(), - slots.lastKey())); + worldStateProofProvider.getStorageProofRelatedNodes( + storageDataRequest.getStorageRoot(), + storageDataRequest.getAccountHash(), + slots.lastKey())); } storageDataRequest.addLocalData(worldStateProofProvider, slots, new ArrayDeque<>(proofs)); return CompletableFuture.completedFuture(requestTask); } -} +} \ No newline at end of file From 272c1bbd81c77ddf283066f7e13b8971734f5654 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 12 Apr 2024 11:02:33 +1000 Subject: [PATCH 10/26] formatting Signed-off-by: Sally MacFarlane --- .../eth/manager/EthProtocolManager.java | 35 -- .../eth/sync/snapsync/RequestDataStep.java | 380 +++++++++--------- 2 files changed, 190 insertions(+), 225 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index 0cf45946ac6..af5ad64bc6a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -160,41 +160,6 @@ public EthProtocolManager( ethereumWireProtocolConfiguration.isLegacyEth64ForkIdEnabled())); } - // public EthProtocolManager( - // final Blockchain blockchain, - // final BigInteger networkId, - // final WorldStateArchive worldStateArchive, - // final TransactionPool transactionPool, - // final EthProtocolConfiguration ethereumWireProtocolConfiguration, - // final EthPeers ethPeers, - // final EthMessages ethMessages, - // final EthContext ethContext, - // final List peerValidators, - // final Optional mergePeerFilter, - // final SynchronizerConfiguration synchronizerConfiguration, - // final EthScheduler scheduler, - // final List blockNumberForks, - // final List timestampForks) { - // this( - // blockchain, - // networkId, - // worldStateArchive, - // transactionPool, - // ethereumWireProtocolConfiguration, - // ethPeers, - // ethMessages, - // ethContext, - // peerValidators, - // mergePeerFilter, - // synchronizerConfiguration, - // scheduler, - // new ForkIdManager( - // blockchain, - // blockNumberForks, - // timestampForks, - // ethereumWireProtocolConfiguration.isLegacyEth64ForkIdEnabled())); - // } - public EthContext ethContext() { return ethContext; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java index 86a31d6a7a6..ea91c84396f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java @@ -64,12 +64,12 @@ public class RequestDataStep { private final WorldStateProofProvider worldStateProofProvider; public RequestDataStep( - final EthContext ethContext, - final WorldStateStorageCoordinator worldStateStorageCoordinator, - final SnapSyncProcessState fastSyncState, - final SnapWorldDownloadState downloadState, - final SnapSyncConfiguration snapSyncConfiguration, - final MetricsSystem metricsSystem) { + final EthContext ethContext, + final WorldStateStorageCoordinator worldStateStorageCoordinator, + final SnapSyncProcessState fastSyncState, + final SnapWorldDownloadState downloadState, + final SnapSyncConfiguration snapSyncConfiguration, + final MetricsSystem metricsSystem) { this.worldStateStorageCoordinator = worldStateStorageCoordinator; this.fastSyncState = fastSyncState; this.downloadState = downloadState; @@ -80,191 +80,191 @@ public RequestDataStep( } public CompletableFuture> requestAccount( - final Task requestTask) { + final Task requestTask) { final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final AccountRangeDataRequest accountDataRequest = - (AccountRangeDataRequest) requestTask.getData(); + (AccountRangeDataRequest) requestTask.getData(); final EthTask getAccountTask = - RetryingGetAccountRangeFromPeerTask.forAccountRange( - ethContext, - accountDataRequest.getStartKeyHash(), - accountDataRequest.getEndKeyHash(), - blockHeader, - metricsSystem); + RetryingGetAccountRangeFromPeerTask.forAccountRange( + ethContext, + accountDataRequest.getStartKeyHash(), + accountDataRequest.getEndKeyHash(), + blockHeader, + metricsSystem); downloadState.addOutstandingTask(getAccountTask); return getAccountTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getAccountTask); - accountDataRequest.setRootHash(blockHeader.getStateRoot()); - accountDataRequest.addResponse( - worldStateProofProvider, response.accounts(), response.proofs()); - } - if (error != null) { - LOG.atWarn() - .setMessage("Error handling account download accounts ({} - {}) task: {}") - .addArgument(accountDataRequest.getStartKeyHash()) - .addArgument(accountDataRequest.getEndKeyHash()) - .addArgument(error) - .log(); - } - return requestTask; - }); + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getAccountTask); + accountDataRequest.setRootHash(blockHeader.getStateRoot()); + accountDataRequest.addResponse( + worldStateProofProvider, response.accounts(), response.proofs()); + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling account download accounts ({} - {}) task: {}") + .addArgument(accountDataRequest.getStartKeyHash()) + .addArgument(accountDataRequest.getEndKeyHash()) + .addArgument(error) + .log(); + } + return requestTask; + }); } public CompletableFuture>> requestStorage( - final List> requestTasks) { + final List> requestTasks) { final List accountHashes = - requestTasks.stream() - .map(Task::getData) - .map(StorageRangeDataRequest.class::cast) - .map(StorageRangeDataRequest::getAccountHash) - .collect(Collectors.toList()); + requestTasks.stream() + .map(Task::getData) + .map(StorageRangeDataRequest.class::cast) + .map(StorageRangeDataRequest::getAccountHash) + .collect(Collectors.toList()); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final Bytes32 minRange = - requestTasks.size() == 1 - ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getStartKeyHash() - : RangeManager.MIN_RANGE; + requestTasks.size() == 1 + ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getStartKeyHash() + : RangeManager.MIN_RANGE; final Bytes32 maxRange = - requestTasks.size() == 1 - ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getEndKeyHash() - : RangeManager.MAX_RANGE; + requestTasks.size() == 1 + ? ((StorageRangeDataRequest) requestTasks.get(0).getData()).getEndKeyHash() + : RangeManager.MAX_RANGE; final EthTask getStorageRangeTask = - RetryingGetStorageRangeFromPeerTask.forStorageRange( - ethContext, accountHashes, minRange, maxRange, blockHeader, metricsSystem); + RetryingGetStorageRangeFromPeerTask.forStorageRange( + ethContext, accountHashes, minRange, maxRange, blockHeader, metricsSystem); downloadState.addOutstandingTask(getStorageRangeTask); return getStorageRangeTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getStorageRangeTask); - final ArrayDeque> slots = new ArrayDeque<>(); - // Check if we have an empty range + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getStorageRangeTask); + final ArrayDeque> slots = new ArrayDeque<>(); + // Check if we have an empty range - /* - * Checks if the response represents an "empty range". - * - * An "empty range" is defined as a response where at least one proof exists - * and either no slots are present, or the first slot is empty - */ - try { - final boolean isEmptyRange = - (response.slots().isEmpty() || response.slots().get(0).isEmpty()) - && !response.proofs().isEmpty(); - if (isEmptyRange) { // empty range detected - slots.add(new TreeMap<>()); - } else { - slots.addAll(response.slots()); - } - for (int i = 0; i < slots.size(); i++) { - final StorageRangeDataRequest request = - (StorageRangeDataRequest) requestTasks.get(i).getData(); - request.setRootHash(blockHeader.getStateRoot()); - request.addResponse( - downloadState, - worldStateProofProvider, - slots.get(i), - i < slots.size() - 1 ? new ArrayDeque<>() : response.proofs()); - } - } catch (final Exception e) { - LOG.error("Error while processing storage range response", e); - } - } - if (error != null) { - LOG.atWarn() - .setMessage("Error handling storage range request task: {}") - .addArgument(error) - .log(); - } - return requestTasks; - }); + /* + * Checks if the response represents an "empty range". + * + * An "empty range" is defined as a response where at least one proof exists + * and either no slots are present, or the first slot is empty + */ + try { + final boolean isEmptyRange = + (response.slots().isEmpty() || response.slots().get(0).isEmpty()) + && !response.proofs().isEmpty(); + if (isEmptyRange) { // empty range detected + slots.add(new TreeMap<>()); + } else { + slots.addAll(response.slots()); + } + for (int i = 0; i < slots.size(); i++) { + final StorageRangeDataRequest request = + (StorageRangeDataRequest) requestTasks.get(i).getData(); + request.setRootHash(blockHeader.getStateRoot()); + request.addResponse( + downloadState, + worldStateProofProvider, + slots.get(i), + i < slots.size() - 1 ? new ArrayDeque<>() : response.proofs()); + } + } catch (final Exception e) { + LOG.error("Error while processing storage range response", e); + } + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling storage range request task: {}") + .addArgument(error) + .log(); + } + return requestTasks; + }); } public CompletableFuture>> requestCode( - final List> requestTasks) { + final List> requestTasks) { final List codeHashes = - requestTasks.stream() - .map(Task::getData) - .map(BytecodeRequest.class::cast) - .map(BytecodeRequest::getCodeHash) - .distinct() - .collect(Collectors.toList()); + requestTasks.stream() + .map(Task::getData) + .map(BytecodeRequest.class::cast) + .map(BytecodeRequest::getCodeHash) + .distinct() + .collect(Collectors.toList()); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final EthTask> getByteCodeTask = - RetryingGetBytecodeFromPeerTask.forByteCode( - ethContext, codeHashes, blockHeader, metricsSystem); + RetryingGetBytecodeFromPeerTask.forByteCode( + ethContext, codeHashes, blockHeader, metricsSystem); downloadState.addOutstandingTask(getByteCodeTask); return getByteCodeTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getByteCodeTask); - for (Task requestTask : requestTasks) { - final BytecodeRequest request = (BytecodeRequest) requestTask.getData(); - request.setRootHash(blockHeader.getStateRoot()); - if (response.containsKey(request.getCodeHash())) { - request.setCode(response.get(request.getCodeHash())); - } - } - } - if (error != null) { - LOG.atWarn() - .setMessage("Error handling code request task: {}") - .addArgument(error) - .log(); - } - return requestTasks; - }); + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getByteCodeTask); + for (Task requestTask : requestTasks) { + final BytecodeRequest request = (BytecodeRequest) requestTask.getData(); + request.setRootHash(blockHeader.getStateRoot()); + if (response.containsKey(request.getCodeHash())) { + request.setCode(response.get(request.getCodeHash())); + } + } + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling code request task: {}") + .addArgument(error) + .log(); + } + return requestTasks; + }); } public CompletableFuture>> requestTrieNodeByPath( - final List> requestTasks) { + final List> requestTasks) { final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final Map> message = new HashMap<>(); requestTasks.stream() - .map(Task::getData) - .map(TrieNodeHealingRequest.class::cast) - .map(TrieNodeHealingRequest::getTrieNodePath) - .forEach( - path -> { - final List bytes = - message.computeIfAbsent(path.get(0), k -> Lists.newArrayList()); - if (path.size() > 1) { - bytes.add(path.get(1)); - } - }); + .map(Task::getData) + .map(TrieNodeHealingRequest.class::cast) + .map(TrieNodeHealingRequest::getTrieNodePath) + .forEach( + path -> { + final List bytes = + message.computeIfAbsent(path.get(0), k -> Lists.newArrayList()); + if (path.size() > 1) { + bytes.add(path.get(1)); + } + }); final EthTask> getTrieNodeFromPeerTask = - RetryingGetTrieNodeFromPeerTask.forTrieNodes( - ethContext, message, blockHeader, metricsSystem); + RetryingGetTrieNodeFromPeerTask.forTrieNodes( + ethContext, message, blockHeader, metricsSystem); downloadState.addOutstandingTask(getTrieNodeFromPeerTask); return getTrieNodeFromPeerTask - .run() - .handle( - (response, error) -> { - if (response != null) { - downloadState.removeOutstandingTask(getTrieNodeFromPeerTask); - for (final Task task : requestTasks) { - final TrieNodeHealingRequest request = (TrieNodeHealingRequest) task.getData(); - final Bytes matchingData = response.get(request.getPathId()); - if (matchingData != null) { - request.setData(matchingData); - } - } - } - if (error != null) { - LOG.atWarn() - .setMessage("Error handling trie node request task: {}") - .addArgument(error) - .log(); - } - return requestTasks; - }); + .run() + .handle( + (response, error) -> { + if (response != null) { + downloadState.removeOutstandingTask(getTrieNodeFromPeerTask); + for (final Task task : requestTasks) { + final TrieNodeHealingRequest request = (TrieNodeHealingRequest) task.getData(); + final Bytes matchingData = response.get(request.getPathId()); + if (matchingData != null) { + request.setData(matchingData); + } + } + } + if (error != null) { + LOG.atWarn() + .setMessage("Error handling trie node request task: {}") + .addArgument(error) + .log(); + } + return requestTasks; + }); } /** @@ -275,34 +275,34 @@ public CompletableFuture>> requestTrieNodeByPath( * @return data request with local accounts */ public CompletableFuture> requestLocalFlatAccounts( - final Task requestTask) { + final Task requestTask) { final AccountFlatDatabaseHealingRangeRequest accountDataRequest = - (AccountFlatDatabaseHealingRangeRequest) requestTask.getData(); + (AccountFlatDatabaseHealingRangeRequest) requestTask.getData(); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); // retrieve accounts from flat database final TreeMap accounts = new TreeMap<>(); worldStateStorageCoordinator.applyOnMatchingFlatMode( - FlatDbMode.FULL, - onBonsai -> { - accounts.putAll( - onBonsai.streamFlatAccounts( - accountDataRequest.getStartKeyHash(), - accountDataRequest.getEndKeyHash(), - snapSyncConfiguration.getLocalFlatAccountCountToHealPerRequest())); - }); + FlatDbMode.FULL, + onBonsai -> { + accounts.putAll( + onBonsai.streamFlatAccounts( + accountDataRequest.getStartKeyHash(), + accountDataRequest.getEndKeyHash(), + snapSyncConfiguration.getLocalFlatAccountCountToHealPerRequest())); + }); final List proofs = new ArrayList<>(); if (!accounts.isEmpty()) { // generate range proof if accounts are present proofs.addAll( - worldStateProofProvider.getAccountProofRelatedNodes( - blockHeader.getStateRoot(), accounts.firstKey())); + worldStateProofProvider.getAccountProofRelatedNodes( + blockHeader.getStateRoot(), accounts.firstKey())); proofs.addAll( - worldStateProofProvider.getAccountProofRelatedNodes( - blockHeader.getStateRoot(), accounts.lastKey())); + worldStateProofProvider.getAccountProofRelatedNodes( + blockHeader.getStateRoot(), accounts.lastKey())); } accountDataRequest.setRootHash(blockHeader.getStateRoot()); @@ -319,10 +319,10 @@ public CompletableFuture> requestLocalFlatAccounts( * @return data request with local slots */ public CompletableFuture> requestLocalFlatStorages( - final Task requestTask) { + final Task requestTask) { final StorageFlatDatabaseHealingRangeRequest storageDataRequest = - (StorageFlatDatabaseHealingRangeRequest) requestTask.getData(); + (StorageFlatDatabaseHealingRangeRequest) requestTask.getData(); final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); storageDataRequest.setRootHash(blockHeader.getStateRoot()); @@ -330,32 +330,32 @@ public CompletableFuture> requestLocalFlatStorages( // retrieve slots from flat database final TreeMap slots = new TreeMap<>(); worldStateStorageCoordinator.applyOnMatchingFlatMode( - FlatDbMode.FULL, - onBonsai -> { - slots.putAll( - onBonsai.streamFlatStorages( - storageDataRequest.getAccountHash(), - storageDataRequest.getStartKeyHash(), - storageDataRequest.getEndKeyHash(), - snapSyncConfiguration.getLocalFlatStorageCountToHealPerRequest())); - }); + FlatDbMode.FULL, + onBonsai -> { + slots.putAll( + onBonsai.streamFlatStorages( + storageDataRequest.getAccountHash(), + storageDataRequest.getStartKeyHash(), + storageDataRequest.getEndKeyHash(), + snapSyncConfiguration.getLocalFlatStorageCountToHealPerRequest())); + }); final List proofs = new ArrayList<>(); if (!slots.isEmpty()) { // generate range proof if slots are present proofs.addAll( - worldStateProofProvider.getStorageProofRelatedNodes( - storageDataRequest.getStorageRoot(), - storageDataRequest.getAccountHash(), - slots.firstKey())); + worldStateProofProvider.getStorageProofRelatedNodes( + storageDataRequest.getStorageRoot(), + storageDataRequest.getAccountHash(), + slots.firstKey())); proofs.addAll( - worldStateProofProvider.getStorageProofRelatedNodes( - storageDataRequest.getStorageRoot(), - storageDataRequest.getAccountHash(), - slots.lastKey())); + worldStateProofProvider.getStorageProofRelatedNodes( + storageDataRequest.getStorageRoot(), + storageDataRequest.getAccountHash(), + slots.lastKey())); } storageDataRequest.addLocalData(worldStateProofProvider, slots, new ArrayDeque<>(proofs)); return CompletableFuture.completedFuture(requestTask); } -} \ No newline at end of file +} From ae030e745445ae54f78ba896ceb102328d5b0407 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 12 Apr 2024 11:03:23 +1000 Subject: [PATCH 11/26] delete version metadata Signed-off-by: Sally MacFarlane --- besu/VERSION_METADATA.json | 1 - 1 file changed, 1 deletion(-) delete mode 100644 besu/VERSION_METADATA.json diff --git a/besu/VERSION_METADATA.json b/besu/VERSION_METADATA.json deleted file mode 100644 index 4dc8c82358b..00000000000 --- a/besu/VERSION_METADATA.json +++ /dev/null @@ -1 +0,0 @@ -{"besuVersion":"24.2.0-dev-5449acc7"} \ No newline at end of file From 68934cc67696185a36061b21972e12d69746af0f Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 12 Apr 2024 17:41:26 +1000 Subject: [PATCH 12/26] reuse ethPeers Signed-off-by: Sally MacFarlane --- besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java | 2 +- 1 file changed, 1 insertion(+), 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 e0e51bb01c7..045f8459cb9 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -705,7 +705,7 @@ public Runner build() { .ethPeersShouldConnect(ethPeers::shouldTryToConnect) .build(); - besuController.getEthPeers().setRlpxAgent(networkRunner.getRlpxAgent()); + ethPeers.setRlpxAgent(networkRunner.getRlpxAgent()); final P2PNetwork network = networkRunner.getNetwork(); // ForkId in Ethereum Node Record needs updating when we transition to a new protocol spec From a95e4f4ea748b8988f730b6c38200cb3bf554eea Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 12 Apr 2024 20:52:04 +1000 Subject: [PATCH 13/26] make a switching peer task Signed-off-by: Sally MacFarlane --- .../RetryingGetAccountRangeFromPeerTask.java | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index beb7784bf59..b40bd71fc1f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -17,7 +17,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingSwitchingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -28,7 +28,7 @@ import org.apache.tuweni.bytes.Bytes32; public class RetryingGetAccountRangeFromPeerTask - extends AbstractRetryingPeerTask { + extends AbstractRetryingSwitchingPeerTask { private final EthContext ethContext; private final Bytes32 startKeyHash; @@ -43,7 +43,7 @@ private RetryingGetAccountRangeFromPeerTask( final BlockHeader blockHeader, final MetricsSystem metricsSystem) { super( - ethContext, 4, data -> data.accounts().isEmpty() && data.proofs().isEmpty(), metricsSystem); + ethContext, metricsSystem, data -> data.accounts().isEmpty() && data.proofs().isEmpty(), 4); this.ethContext = ethContext; this.startKeyHash = startKeyHash; this.endKeyHash = endKeyHash; @@ -61,6 +61,21 @@ public static EthTask forAccountRange( ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem); } + @Override + protected CompletableFuture executeTaskOnCurrentPeer( + final EthPeer peer) { + final GetAccountRangeFromPeerTask task = + GetAccountRangeFromPeerTask.forAccountRange( + ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem); + task.assignPeer(peer); + return executeSubTask(task::run) + .thenApply( + peerResult -> { + result.complete(peerResult.getResult()); + return peerResult.getResult(); + }); + } + @Override protected CompletableFuture executePeerTask( final Optional assignedPeer) { From 83ef98f3af44c2b80aa53cf110acca4794b1ad71 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 15 Apr 2024 12:15:33 +1000 Subject: [PATCH 14/26] add client info to disconnect message Signed-off-by: Sally MacFarlane --- .../besu/ethereum/eth/manager/EthProtocolManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index af5ad64bc6a..3537d9720fc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -369,11 +369,12 @@ public void handleDisconnect( final boolean initiatedByPeer) { final boolean wasActiveConnection = ethPeers.registerDisconnect(connection); LOG.atDebug() - .setMessage("Disconnect - active Connection: {} - {} - {} - {} - {} peers left") + .setMessage("Disconnect - active Connection? {} - {} - {} - {} {} - {} peers left") .addArgument(wasActiveConnection) .addArgument(initiatedByPeer ? "Inbound" : "Outbound") .addArgument(reason::toString) .addArgument(() -> connection.getPeer().getLoggableId()) + .addArgument(() -> connection.getPeerInfo().getClientId()) .addArgument(ethPeers::peerCount) .log(); LOG.atTrace().setMessage("{}").addArgument(ethPeers::toString).log(); From 5af6bbb8d8a19a347975ae5f36a3fdf0ee8d9de0 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 15 Apr 2024 13:19:59 +1000 Subject: [PATCH 15/26] check for non-deprecated sync modes Signed-off-by: Sally MacFarlane --- .../besu/ethereum/eth/manager/EthPeers.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 92d7c8fbaee..5e6d54d526e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -140,7 +140,10 @@ public EthPeers( LOG.trace("MaxPeers: {}, Max Remote: {}", peerUpperBound, maxRemotelyInitiatedConnections); this.syncMode = syncMode; this.forkIdManager = forkIdManager; - if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { + if (syncMode == SyncMode.X_CHECKPOINT + || syncMode == SyncMode.X_SNAP + || syncMode == SyncMode.CHECKPOINT + || syncMode == SyncMode.X_SNAP) { snapServerTargetNumber = peerUpperBound / 2; // hardcoded for now. 50% of peers should be snap servers } else { @@ -515,11 +518,15 @@ private void ethPeerStatusExchanged(final EthPeer peer) { "Failed to retrieve chain head info. Disconnecting {}... {}", peer.getLoggableId(), error); - peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); + peer.disconnect( + DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_STATE); } else { peer.chainState().updateHeightEstimate(peerHeadBlockHeader.getNumber()); CompletableFuture isServingSnapFuture; - if (syncMode == SyncMode.X_CHECKPOINT || syncMode == SyncMode.X_SNAP) { + if (syncMode == SyncMode.X_CHECKPOINT + || syncMode == SyncMode.X_SNAP + || syncMode == SyncMode.CHECKPOINT + || syncMode == SyncMode.SNAP) { isServingSnapFuture = CompletableFuture.runAsync( () -> { From 87690b9da2a6763a908d51e8af60c54ad57f158b Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 15 Apr 2024 18:30:43 +1000 Subject: [PATCH 16/26] logging Signed-off-by: Sally MacFarlane --- .../hyperledger/besu/ethereum/eth/manager/EthPeers.java | 8 +++++--- .../besu/ethereum/eth/sync/SnapServerChecker.java | 4 ---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 5e6d54d526e..27450faad1d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -569,18 +569,20 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl return; } peer.setIsServingSnap(isServer); - LOG.info("Peer {} is a snap server: {}", peer.getLoggableId(), isServer); + LOG.info("Peer {} snap server? {}", peer.getLoggableId(), isServer); // TODO: remove the following code. Just here for testing final boolean simpleCheck = peer.getConnection().getPeerInfo().getClientId().contains("Geth"); if (simpleCheck && !isServer) { LOG.info( - "YYYYYYYYYY Found a peer that is Geth but not a snap server: {}", + "YYYYYYYYYY Found a peer {} that is Geth but not a snap server: {}", + peer.getConnection().getPeerInfo().getClientId(), peer.getLoggableId()); } else if (!simpleCheck && isServer) { LOG.info( - "ZZZZZZZZZZ Found a peer that is NOT Geth but is a snap server: {}", + "ZZZZZZZZZZ Found a peer {} that is NOT Geth but is a snap server: {}", + peer.getConnection().getPeerInfo().getClientId(), peer.getLoggableId()); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java index 9c3c3b9dddd..62dfecf6865 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java @@ -59,10 +59,6 @@ public CompletableFuture check(final EthPeer peer, final BlockHeader he if (peerResult != null) { if (!peerResult.getResult().accounts().isEmpty() || !peerResult.getResult().proofs().isEmpty()) { - // LOG.atDebug() - // .setMessage("Peer {} is a snap server ...") - // .addArgument(peer::getLoggableId) - // .log(); LOG.atInfo() .setMessage("Peer {} is a snap server! getAccountRangeResult: {}") .addArgument(peer::getLoggableId) From c3c5bb376b1d7cdc8659c9c828639eb905956f5d Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Mon, 15 Apr 2024 18:31:48 +1000 Subject: [PATCH 17/26] formatting Signed-off-by: Sally MacFarlane --- .../org/hyperledger/besu/ethereum/eth/manager/EthPeers.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 27450faad1d..8b45c70bfdf 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -582,7 +582,7 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl } else if (!simpleCheck && isServer) { LOG.info( "ZZZZZZZZZZ Found a peer {} that is NOT Geth but is a snap server: {}", - peer.getConnection().getPeerInfo().getClientId(), + peer.getConnection().getPeerInfo().getClientId(), peer.getLoggableId()); } } From b14834730e8f2fdd6073f4dfc277f6dcfb418c07 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 13 Jun 2024 13:15:25 +1000 Subject: [PATCH 18/26] fix after merge Signed-off-by: stefan.pingel@consensys.net --- .../hyperledger/besu/controller/BesuControllerBuilder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 6d8015629f2..b18d0cb6c61 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -606,8 +606,8 @@ public BesuController build() { final ForkIdManager forkIdManager = new ForkIdManager( blockchain, - genesisConfig.getForkBlockNumbers(), - genesisConfig.getForkTimestamps(), + genesisConfigOptions.getForkBlockNumbers(), + genesisConfigOptions.getForkBlockTimestamps(), ethereumWireProtocolConfiguration.isLegacyEth64ForkIdEnabled()); final EthPeers ethPeers = new EthPeers( From 12c8350187e21059b098b5b721b6ae1cd540f422 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 18 Jun 2024 13:13:36 +1000 Subject: [PATCH 19/26] stop prefering snap servers after sync Signed-off-by: stefan.pingel@consensys.net --- .../controller/BesuControllerBuilder.java | 7 +++ .../besu/ethereum/eth/manager/EthPeers.java | 47 +++++-------------- .../ethereum/eth/sync/ChainHeadTracker.java | 2 +- .../eth/sync/DefaultSynchronizer.java | 4 +- .../ethereum/eth/sync/SnapServerChecker.java | 2 +- .../besu/ethereum/eth/sync/SyncMode.java | 4 ++ .../rlpx/wire/messages/DisconnectMessage.java | 4 +- 7 files changed, 30 insertions(+), 40 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index b18d0cb6c61..7054625c5ac 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -705,6 +705,13 @@ public BesuController build() { ethProtocolManager, pivotBlockSelector); + if (SyncMode.isSnapSync(syncConfig.getSyncMode()) || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { + synchronizer.subscribeInSync( (b)-> ethPeers.snapSyncServerPeersNeeded(!b)); + ethPeers.snapSyncServerPeersNeeded(true); + } else { + ethPeers.snapSyncServerPeersNeeded(false); + } + protocolContext.setSynchronizer(Optional.of(synchronizer)); final Optional maybeSnapProtocolManager = diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 8b45c70bfdf..57f72fefd86 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -113,6 +113,7 @@ public class EthPeers { // private List protocolManagers; private ChainHeadTracker tracker; private SnapServerChecker snapServerChecker; + private boolean snapSyncServerPeersNeeded = false; public EthPeers( final String protocolName, @@ -140,15 +141,8 @@ public EthPeers( LOG.trace("MaxPeers: {}, Max Remote: {}", peerUpperBound, maxRemotelyInitiatedConnections); this.syncMode = syncMode; this.forkIdManager = forkIdManager; - if (syncMode == SyncMode.X_CHECKPOINT - || syncMode == SyncMode.X_SNAP - || syncMode == SyncMode.CHECKPOINT - || syncMode == SyncMode.X_SNAP) { - snapServerTargetNumber = - peerUpperBound / 2; // hardcoded for now. 50% of peers should be snap servers - } else { - snapServerTargetNumber = 0; - } + snapServerTargetNumber = + peerUpperBound / 2; // 50% of peers should be snap servers while snap syncing metricsSystem.createIntegerGauge( BesuMetricCategory.ETHEREUM, "peer_count", @@ -485,6 +479,10 @@ public void setSnapServerChecker(final SnapServerChecker checker) { this.snapServerChecker = checker; } + public void snapSyncServerPeersNeeded(final boolean b) { + snapSyncServerPeersNeeded = b; + } + @FunctionalInterface public interface ConnectCallback { void onPeerConnected(EthPeer newPeer); @@ -519,14 +517,12 @@ private void ethPeerStatusExchanged(final EthPeer peer) { peer.getLoggableId(), error); peer.disconnect( - DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_STATE); + DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD); } else { peer.chainState().updateHeightEstimate(peerHeadBlockHeader.getNumber()); CompletableFuture isServingSnapFuture; - if (syncMode == SyncMode.X_CHECKPOINT - || syncMode == SyncMode.X_SNAP - || syncMode == SyncMode.CHECKPOINT - || syncMode == SyncMode.SNAP) { + if (SyncMode.isCheckpointSync(syncMode) || SyncMode.isSnapSync(syncMode)) { + // even if we have finished the snap sync, we still want to know if the peer is a snap server isServingSnapFuture = CompletableFuture.runAsync( () -> { @@ -540,8 +536,6 @@ private void ethPeerStatusExchanged(final EthPeer peer) { isServingSnapFuture = CompletableFuture.completedFuture(null); } isServingSnapFuture.thenRun( - // TODO: might be a good idea not to connect to syncing peers if we are syncing - // ourselves (initial sync) () -> { if (!peer.getConnection().isDisconnected() && addPeerToEthPeers(peer)) { connectedPeersCounter.inc(); @@ -554,8 +548,6 @@ private void ethPeerStatusExchanged(final EthPeer peer) { private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBlockHeader) { if (peer.getAgreedCapabilities().contains(SnapProtocol.SNAP1)) { - // could try and retrieve some SNAP data here to check that (e.g. GetByteCodes for a small - // contract) if (snapServerChecker != null) { // set that peer is a snap server for doing the test peer.setIsServingSnap(true); @@ -564,27 +556,12 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(10L, TimeUnit.SECONDS); } catch (Exception e) { // TODO: change LOG to debug? - LOG.info("XXXXXX Error checking if peer is a snap server.", e); + LOG.info("XXXXXX Error checking if peer is a snap server. {}", e.getStackTrace()); peer.setIsServingSnap(false); return; } peer.setIsServingSnap(isServer); LOG.info("Peer {} snap server? {}", peer.getLoggableId(), isServer); - - // TODO: remove the following code. Just here for testing - final boolean simpleCheck = - peer.getConnection().getPeerInfo().getClientId().contains("Geth"); - if (simpleCheck && !isServer) { - LOG.info( - "YYYYYYYYYY Found a peer {} that is Geth but not a snap server: {}", - peer.getConnection().getPeerInfo().getClientId(), - peer.getLoggableId()); - } else if (!simpleCheck && isServer) { - LOG.info( - "ZZZZZZZZZZ Found a peer {} that is NOT Geth but is a snap server: {}", - peer.getConnection().getPeerInfo().getClientId(), - peer.getLoggableId()); - } } } } @@ -749,7 +726,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { } private boolean needMoreSnapServers() { - return activeConnections.values().stream().filter(EthPeer::isServingSnap).count() + return snapSyncServerPeersNeeded && activeConnections.values().stream().filter(EthPeer::isServingSnap).count() < snapServerTargetNumber; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java index 5fd60ff4598..b3e28dfb807 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java @@ -94,7 +94,7 @@ public CompletableFuture getBestHeaderFromPeer(final EthPeer peer) .addArgument(error) .log(); peer.disconnect( - DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_STATE); + DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD); future.complete(null); } }); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index 8ff87222911..dfc959814cb 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -98,7 +98,9 @@ public DefaultSynchronizer( this::calculateTrailingPeerRequirements, metricsSystem); - SnapServerChecker.createSnapServerChecker(ethContext, metricsSystem); + if (SyncMode.isSnapSync(syncConfig.getSyncMode()) || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { + SnapServerChecker.createAndSetSnapServerChecker(ethContext, metricsSystem); + } this.blockPropagationManager = terminationCondition.shouldStopDownload() diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java index 62dfecf6865..3dcce26b7fc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java @@ -40,7 +40,7 @@ public SnapServerChecker(final EthContext ethContext, final MetricsSystem metric this.metricsSystem = metricsSystem; } - public static void createSnapServerChecker( + public static void createAndSetSnapServerChecker( final EthContext ethContext, final MetricsSystem metricsSystem) { final SnapServerChecker checker = new SnapServerChecker(ethContext, metricsSystem); ethContext.getEthPeers().setSnapServerChecker(checker); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SyncMode.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SyncMode.java index b5d0c2570bb..4f3affbe555 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SyncMode.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SyncMode.java @@ -54,4 +54,8 @@ public static boolean isFullSync(final SyncMode syncMode) { public static boolean isCheckpointSync(final SyncMode syncMode) { return X_CHECKPOINT.equals(syncMode) || CHECKPOINT.equals(syncMode); } + + public static boolean isSnapSync(final SyncMode syncMode) { + return X_SNAP.equals(syncMode) || SNAP.equals(syncMode); + } } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java index 66337903358..5bba36a5b33 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java @@ -129,8 +129,8 @@ public enum DisconnectReason { USELESS_PEER_NO_SHARED_CAPABILITIES((byte) 0x03, "No shared capabilities"), USELESS_PEER_WORLD_STATE_NOT_AVAILABLE((byte) 0x03, "World state not available"), USELESS_PEER_MISMATCHED_PIVOT_BLOCK((byte) 0x03, "Mismatched pivot block"), - USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_STATE( - (byte) 0x03, "Failed to retrieve header for chain state"), + USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD( + (byte) 0x03, "Failed to retrieve the chain head header"), USELESS_PEER_BY_REPUTATION((byte) 0x03, "Lowest reputation score"), USELESS_PEER_BY_CHAIN_COMPARATOR((byte) 0x03, "Lowest by chain height comparator"), TOO_MANY_PEERS((byte) 0x04), From 135497902f27bd3c26e4daeb8f49d65cdc5ed535 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 18 Jun 2024 14:15:05 +1000 Subject: [PATCH 20/26] fix compile Signed-off-by: stefan.pingel@consensys.net --- .../besu/controller/BesuControllerBuilder.java | 5 +++-- .../besu/ethereum/eth/manager/EthPeers.java | 15 ++++++++++----- .../ethereum/eth/sync/DefaultSynchronizer.java | 3 ++- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 7054625c5ac..3556bff98c9 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -705,8 +705,9 @@ public BesuController build() { ethProtocolManager, pivotBlockSelector); - if (SyncMode.isSnapSync(syncConfig.getSyncMode()) || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { - synchronizer.subscribeInSync( (b)-> ethPeers.snapSyncServerPeersNeeded(!b)); + if (SyncMode.isSnapSync(syncConfig.getSyncMode()) + || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { + synchronizer.subscribeInSync((b) -> ethPeers.snapSyncServerPeersNeeded(!b)); ethPeers.snapSyncServerPeersNeeded(true); } else { ethPeers.snapSyncServerPeersNeeded(false); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 57f72fefd86..549419e7f49 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -142,7 +142,7 @@ public EthPeers( this.syncMode = syncMode; this.forkIdManager = forkIdManager; snapServerTargetNumber = - peerUpperBound / 2; // 50% of peers should be snap servers while snap syncing + peerUpperBound / 2; // 50% of peers should be snap servers while snap syncing metricsSystem.createIntegerGauge( BesuMetricCategory.ETHEREUM, "peer_count", @@ -522,7 +522,8 @@ private void ethPeerStatusExchanged(final EthPeer peer) { peer.chainState().updateHeightEstimate(peerHeadBlockHeader.getNumber()); CompletableFuture isServingSnapFuture; if (SyncMode.isCheckpointSync(syncMode) || SyncMode.isSnapSync(syncMode)) { - // even if we have finished the snap sync, we still want to know if the peer is a snap server + // even if we have finished the snap sync, we still want to know if the peer is a snap + // server isServingSnapFuture = CompletableFuture.runAsync( () -> { @@ -556,7 +557,10 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(10L, TimeUnit.SECONDS); } catch (Exception e) { // TODO: change LOG to debug? - LOG.info("XXXXXX Error checking if peer is a snap server. {}", e.getStackTrace()); + LOG.atInfo() + .setMessage("XXXXXX Error checking if peer is a snap server. {}") + .addArgument(e.getStackTrace()) + .log(); peer.setIsServingSnap(false); return; } @@ -726,8 +730,9 @@ private boolean addPeerToEthPeers(final EthPeer peer) { } private boolean needMoreSnapServers() { - return snapSyncServerPeersNeeded && activeConnections.values().stream().filter(EthPeer::isServingSnap).count() - < snapServerTargetNumber; + return snapSyncServerPeersNeeded + && activeConnections.values().stream().filter(EthPeer::isServingSnap).count() + < snapServerTargetNumber; } private void disconnectNonSnapServerPeerOrLeastUseful() { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index dfc959814cb..bea32e3ae4d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -98,7 +98,8 @@ public DefaultSynchronizer( this::calculateTrailingPeerRequirements, metricsSystem); - if (SyncMode.isSnapSync(syncConfig.getSyncMode()) || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { + if (SyncMode.isSnapSync(syncConfig.getSyncMode()) + || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { SnapServerChecker.createAndSetSnapServerChecker(ethContext, metricsSystem); } From 048765f46e1eea98d4f833f993a3dd15a16104cc Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 20 Jun 2024 12:10:27 +1000 Subject: [PATCH 21/26] add tests and some clean-up Signed-off-by: stefan.pingel@consensys.net --- .../controller/BesuControllerBuilder.java | 6 +- .../besu/ethereum/eth/manager/EthPeers.java | 115 ++++++++---------- .../ethereum/eth/manager/EthPeersTest.java | 53 ++++++++ .../manager/EthProtocolManagerTestUtil.java | 23 +++- .../eth/manager/RespondingEthPeer.java | 29 +++-- 5 files changed, 149 insertions(+), 77 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 3556bff98c9..94da1026c15 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -707,10 +707,10 @@ public BesuController build() { if (SyncMode.isSnapSync(syncConfig.getSyncMode()) || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { - synchronizer.subscribeInSync((b) -> ethPeers.snapSyncServerPeersNeeded(!b)); - ethPeers.snapSyncServerPeersNeeded(true); + synchronizer.subscribeInSync((b) -> ethPeers.snapServerPeersNeeded(!b)); + ethPeers.snapServerPeersNeeded(true); } else { - ethPeers.snapSyncServerPeersNeeded(false); + ethPeers.snapServerPeersNeeded(false); } protocolContext.setSynchronizer(Optional.of(synchronizer)); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 549419e7f49..2366ac03adc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -104,6 +104,7 @@ public class EthPeers { private final SyncMode syncMode; private final ForkIdManager forkIdManager; private final int snapServerTargetNumber; + private final boolean shouldLimitRemoteConnections; private Comparator bestPeerComparator; private final Bytes localNodeId; @@ -113,7 +114,7 @@ public class EthPeers { // private List protocolManagers; private ChainHeadTracker tracker; private SnapServerChecker snapServerChecker; - private boolean snapSyncServerPeersNeeded = false; + private boolean snapServerPeersNeeded = false; public EthPeers( final String protocolName, @@ -141,8 +142,10 @@ public EthPeers( LOG.trace("MaxPeers: {}, Max Remote: {}", peerUpperBound, maxRemotelyInitiatedConnections); this.syncMode = syncMode; this.forkIdManager = forkIdManager; - snapServerTargetNumber = + this.snapServerTargetNumber = peerUpperBound / 2; // 50% of peers should be snap servers while snap syncing + this.shouldLimitRemoteConnections = maxRemotelyInitiatedConnections < peerUpperBound; + metricsSystem.createIntegerGauge( BesuMetricCategory.ETHEREUM, "peer_count", @@ -453,24 +456,6 @@ public void disconnectWorstUselessPeer() { }); } - public void disconnectWorstIncomingUselessPeer() { - streamAvailablePeers() - .filter(p -> p.getConnection().inboundInitiated()) - .filter(p -> !canExceedPeerLimits(p.getId())) - .min(getBestChainComparator()) - .ifPresent( - peer -> { - LOG.atDebug() - .setMessage( - "disconnecting peer {}. Waiting for better peers. Current {} of max {}") - .addArgument(peer::getLoggableId) - .addArgument(this::peerCount) - .addArgument(this::getMaxPeers) - .log(); - peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER); - }); - } - public void setChainHeadTracker(final ChainHeadTracker tracker) { this.tracker = tracker; } @@ -479,8 +464,8 @@ public void setSnapServerChecker(final SnapServerChecker checker) { this.snapServerChecker = checker; } - public void snapSyncServerPeersNeeded(final boolean b) { - snapSyncServerPeersNeeded = b; + public void snapServerPeersNeeded(final boolean b) { + this.snapServerPeersNeeded = b; } @FunctionalInterface @@ -602,7 +587,7 @@ private int compareByMaskedNodeId(final PeerConnection a, final PeerConnection b } private void enforceRemoteConnectionLimits() { - if (!shouldLimitRemoteConnections() || peerCount() < maxRemotelyInitiatedConnections) { + if (!shouldLimitRemoteConnections || peerCount() < maxRemotelyInitiatedConnections) { // Nothing to do return; } @@ -646,13 +631,9 @@ private void enforceConnectionLimits() { }); } - private boolean remoteConnectionLimitReached() { - return shouldLimitRemoteConnections() - && countUntrustedRemotelyInitiatedConnections() >= maxRemotelyInitiatedConnections; - } - - private boolean shouldLimitRemoteConnections() { - return maxRemotelyInitiatedConnections < peerUpperBound; + private boolean inboundInitiatedConnectionLimitExceeded() { + return shouldLimitRemoteConnections + && countUntrustedRemotelyInitiatedConnections() > maxRemotelyInitiatedConnections; } private long countUntrustedRemotelyInitiatedConnections() { @@ -682,9 +663,9 @@ private void onCacheRemoval( } } - private boolean addPeerToEthPeers(final EthPeer peer) { + boolean addPeerToEthPeers(final EthPeer peer) { // We have a connection to a peer that is on the right chain and is willing to connect to us. - // Figure out whether we want to keep this peer to add it to the active connections. + // Figure out whether we want to add it to the active connections. final PeerConnection connection = peer.getConnection(); if (activeConnections.containsValue(peer)) { // connection.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); @@ -692,21 +673,44 @@ private boolean addPeerToEthPeers(final EthPeer peer) { } final Bytes id = peer.getId(); if (!randomPeerPriority) { - // Disconnect if too many peers + if (peerCount() >= peerUpperBound) { - if (needMoreSnapServers()) { - disconnectNonSnapServerPeerOrLeastUseful(); - } else if (!remoteConnectionLimitReached() && !peer.getConnection().inboundInitiated()) { - if (peer.isServingSnap()) { - disconnectWorstIncomingUselessPeer(); - } else { - disconnectNonSnapServerPeerOrLeastUseful(); - } - } else if (!canExceedPeerLimits(id)) { - LOG.trace( - "Too many peers. Disconnect connection: {}, max connections {}", - connection, - peerUpperBound); + final long numSnapServers = numberOfSnapServers(); + final boolean inboundLimitExceeded = inboundInitiatedConnectionLimitExceeded(); + // three reasons why we would disconnect an existing peer to accommodate the new peer + if (canExceedPeerLimits(id) + || (snapServerPeersNeeded + && numSnapServers < snapServerTargetNumber + && peer.isServingSnap()) + || (inboundLimitExceeded && !peer.getConnection().inboundInitiated())) { + + final boolean filterOutSnapServers = + snapServerPeersNeeded && (numSnapServers <= snapServerTargetNumber); + + // find and disconnect the least useful peer we can disconnect + activeConnections.values().stream() + .filter(p -> !canExceedPeerLimits(p.getId())) + .filter(filterOutSnapServers ? p -> !p.isServingSnap() : p -> true) + .filter(inboundLimitExceeded ? p -> p.getConnection().inboundInitiated() : p -> true) + .min(MOST_USEFUL_PEER) + .ifPresentOrElse( + pe -> pe.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS), + () -> + activeConnections.values().stream() + .filter(p -> !canExceedPeerLimits(p.getId())) + .min(MOST_USEFUL_PEER) + .ifPresent( + p -> { + p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); + LOG.atTrace().setMessage("Disconnecting peer {} to be replaced by prioritised peer {}").addArgument(p.getLoggableId()).addArgument(peer.getLoggableId()).log(); + }) + ); + } else { + LOG.atTrace() + .setMessage("Too many peers. Disconnect connection: {}, max connections {}") + .addArgument(connection) + .addArgument(peerUpperBound) + .log(); connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); return false; } @@ -719,6 +723,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { LOG.debug("Did not add peer {} with connection {} to activeConnections", id, connection); } return added; + } else { // randomPeerPriority! Add the peer and if there are too many connections fix it // TODO: random peer priority does not care yet about snap server peers -> check later @@ -730,22 +735,10 @@ private boolean addPeerToEthPeers(final EthPeer peer) { } private boolean needMoreSnapServers() { - return snapSyncServerPeersNeeded - && activeConnections.values().stream().filter(EthPeer::isServingSnap).count() - < snapServerTargetNumber; + return snapServerPeersNeeded && numberOfSnapServers() < snapServerTargetNumber; } - private void disconnectNonSnapServerPeerOrLeastUseful() { - activeConnections.values().stream() - .filter(p -> !p.isServingSnap()) - .filter(p -> !canExceedPeerLimits(p.getId())) - .min(MOST_USEFUL_PEER) - .ifPresentOrElse( - p -> p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS), - () -> - activeConnections.values().stream() - .min(MOST_USEFUL_PEER) - .ifPresent( - p -> p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS))); + private long numberOfSnapServers() { + return activeConnections.values().stream().filter(EthPeer::isServingSnap).count(); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java index 2b5445a95c2..74d1a75b9e7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java @@ -375,6 +375,59 @@ public void toString_hasExpectedInfo() { assertThat(ethPeers.toString()).contains(peerA.getLoggableId()); } + @Test + public void snapServersPreferredWhileSyncing() { + + ethPeers.snapServerPeersNeeded(true); + + while (ethPeers.peerCount() < ethPeers.getMaxPeers()) { + final EthPeer ethPeer = + EthProtocolManagerTestUtil.createPeer( + ethProtocolManager, Difficulty.of(50), 20, false, false) + .getEthPeer(); + assertThat(ethPeers.addPeerToEthPeers(ethPeer)).isTrue(); + } + + final EthPeer nonSnapServingPeer = + EthProtocolManagerTestUtil.createPeer( + ethProtocolManager, Difficulty.of(50), 20, false, false) + .getEthPeer(); + + assertThat(ethPeers.addPeerToEthPeers(nonSnapServingPeer)).isFalse(); + assertThat(nonSnapServingPeer.getConnection().isDisconnected()).isTrue(); + + final EthPeer snapServingPeer = + EthProtocolManagerTestUtil.createPeer( + ethProtocolManager, Difficulty.of(50), 20, true, false) + .getEthPeer(); + + assertThat(ethPeers.addPeerToEthPeers(snapServingPeer)).isTrue(); + assertThat(ethPeers.peerCount()).isEqualTo(ethPeers.getMaxPeers()); + } + + @Test + public void snapServersNotPreferedWhenInSync() { + + ethPeers.snapServerPeersNeeded(false); + + while (ethPeers.peerCount() < ethPeers.getMaxPeers()) { + final EthPeer ethPeer = + EthProtocolManagerTestUtil.createPeer( + ethProtocolManager, Difficulty.of(50), 20, false, false) + .getEthPeer(); + assertThat(ethPeers.addPeerToEthPeers(ethPeer)).isTrue(); + } + + final EthPeer snapServingPeer = + EthProtocolManagerTestUtil.createPeer( + ethProtocolManager, Difficulty.of(50), 20, true, false) + .getEthPeer(); + + assertThat(ethPeers.addPeerToEthPeers(snapServingPeer)).isFalse(); + assertThat(snapServingPeer.getConnection().isDisconnected()).isTrue(); + assertThat(ethPeers.peerCount()).isEqualTo(ethPeers.getMaxPeers()); + } + private void freeUpCapacity(final EthPeer ethPeer) { ethPeers.dispatchMessage(ethPeer, new EthMessage(ethPeer, NodeDataMessage.create(emptyList()))); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java index b4da01d8a95..1f73221b209 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java @@ -292,7 +292,7 @@ public static EthProtocolManager create( final ProtocolSchedule protocolSchedule, final Blockchain blockchain, final EthScheduler ethScheduler) { - final EthPeers peers = + final EthPeers ethPeers = new EthPeers( EthProtocol.NAME, () -> protocolSchedule.getByBlockHeader(blockchain.getChainHeadHeader()), @@ -308,7 +308,7 @@ public static EthProtocolManager create( new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final ChainHeadTracker chainHeadTrackerMock = getChainHeadTrackerMock(); - peers.setChainHeadTracker(chainHeadTrackerMock); + ethPeers.setChainHeadTracker(chainHeadTrackerMock); final EthMessages messages = new EthMessages(); @@ -318,9 +318,9 @@ public static EthProtocolManager create( BlockchainSetupUtil.forTesting(DataStorageFormat.FOREST).getWorldArchive(), mock(TransactionPool.class), EthProtocolConfiguration.defaultConfig(), - peers, + ethPeers, messages, - new EthContext(peers, messages, ethScheduler)); + new EthContext(ethPeers, messages, ethScheduler)); } public static EthProtocolManager create() { @@ -486,4 +486,19 @@ public static RespondingEthPeer createPeer( .estimatedHeight(blockchain.getChainHeadBlockNumber()) .build(); } + + public static RespondingEthPeer createPeer( + final EthProtocolManager ethProtocolManager, + final Difficulty td, + final int estimatedHeight, + final boolean isServingSnap, + final boolean addToEthPeers) { + return RespondingEthPeer.builder() + .ethProtocolManager(ethProtocolManager) + .totalDifficulty(td) + .estimatedHeight(estimatedHeight) + .isServingSnap(isServingSnap) + .addToEthPeers(addToEthPeers) + .build(); + } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java index c5bac5835f2..9ff37de8f6a 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java @@ -122,7 +122,8 @@ private static RespondingEthPeer create( final Difficulty totalDifficulty, final OptionalLong estimatedHeight, final List peerValidators, - final boolean isServingSnap) { + final boolean isServingSnap, + final boolean addToEthPeers) { final EthPeers ethPeers = ethProtocolManager.ethContext().getEthPeers(); final Set caps = new HashSet<>(Collections.singletonList(EthProtocol.ETH63)); @@ -135,13 +136,16 @@ private static RespondingEthPeer create( final EthPeer peer = ethPeers.peer(peerConnection); peer.registerStatusReceived(chainHeadHash, totalDifficulty, 63, peerConnection); estimatedHeight.ifPresent(height -> peer.chainState().update(chainHeadHash, height)); - peer.registerStatusSent(peerConnection); - while (ethPeers.peerCount() - <= before) { // this is needed to make sure that the peer is added to the active connections - try { - Thread.sleep(100L); - } catch (InterruptedException e) { - throw new RuntimeException(e); + if (addToEthPeers) { + peer.registerStatusSent(peerConnection); + while (ethPeers.peerCount() + <= before) { // this is needed to make sure that the peer is added to the active + // connections + try { + Thread.sleep(100L); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } } } peer.setIsServingSnap(isServingSnap); @@ -408,6 +412,7 @@ public static class Builder { private OptionalLong estimatedHeight = OptionalLong.of(1000L); private final List peerValidators = new ArrayList<>(); private boolean isServingSnap = false; + private boolean addToEthPeers = true; public RespondingEthPeer build() { checkNotNull(ethProtocolManager, "Must configure EthProtocolManager"); @@ -419,7 +424,8 @@ public RespondingEthPeer build() { totalDifficulty, estimatedHeight, peerValidators, - isServingSnap); + isServingSnap, + addToEthPeers); } public Builder ethProtocolManager(final EthProtocolManager ethProtocolManager) { @@ -472,6 +478,11 @@ public Builder peerValidators(final PeerValidator... peerValidators) { peerValidators(Arrays.asList(peerValidators)); return this; } + + public Builder addToEthPeers(final boolean addToEthPeers) { + this.addToEthPeers = addToEthPeers; + return this; + } } static class OutgoingMessage { From de7d55b7a5d102faf9988ee0c4697e9b0439a8d1 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 20 Jun 2024 16:24:19 +1000 Subject: [PATCH 22/26] make constant Signed-off-by: stefan.pingel@consensys.net --- .../ethereum/eth/sync/fastsync/SyncTargetManager.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java index c19e06f993b..60444508d2f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java @@ -41,6 +41,8 @@ public class SyncTargetManager extends AbstractSyncTargetManager { private static final Logger LOG = LoggerFactory.getLogger(SyncTargetManager.class); + private static final int LOG_DEBUG_REPEAT_DELAY = 15; + private static final int LOG_INFO_REPEAT_DELAY = 120; private final WorldStateStorageCoordinator worldStateStorageCoordinator; private final ProtocolSchedule protocolSchedule; @@ -50,8 +52,6 @@ public class SyncTargetManager extends AbstractSyncTargetManager { private final FastSyncState fastSyncState; private final AtomicBoolean logDebug = new AtomicBoolean(true); private final AtomicBoolean logInfo = new AtomicBoolean(true); - private final int logDebugRepeatDelay = 15; - private final int logInfoRepeatDelay = 120; public SyncTargetManager( final SynchronizerConfiguration config, @@ -82,14 +82,14 @@ protected CompletableFuture> selectBestAvailableSyncTarget() { "Unable to find sync target. Currently checking %d peers for usefulness. Pivot block: %d", ethContext.getEthPeers().peerCount(), pivotBlockHeader.getNumber()), logDebug, - logDebugRepeatDelay); + LOG_DEBUG_REPEAT_DELAY); throttledLog( LOG::info, String.format( "Unable to find sync target. Currently checking %d peers for usefulness.", ethContext.getEthPeers().peerCount()), logInfo, - logInfoRepeatDelay); + LOG_INFO_REPEAT_DELAY); return completedFuture(Optional.empty()); } else { final EthPeer bestPeer = maybeBestPeer.get(); From 0099f1137b39248d810c78e70c4e50f3563168b4 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 20 Jun 2024 16:48:05 +1000 Subject: [PATCH 23/26] add newline Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/sync/fastsync/SyncTargetManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java index 3260be2a8ea..cdb824cce15 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java @@ -42,6 +42,7 @@ public class SyncTargetManager extends AbstractSyncTargetManager { private static final Logger LOG = LoggerFactory.getLogger(SyncTargetManager.class); + private static final int LOG_DEBUG_REPEAT_DELAY = 15; private static final int LOG_INFO_REPEAT_DELAY = 120; private static final int SECONDS_PER_REQUEST = 6; // 5s per request + 1s wait between retries From f1f20736ecfb16045e8c84e78552d2fea20dd5f7 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 20 Jun 2024 16:49:08 +1000 Subject: [PATCH 24/26] spotless Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/manager/EthPeers.java | 12 ++++++++---- .../eth/sync/fastsync/SyncTargetManager.java | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 32146ac51e6..b2a707c63f5 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -704,10 +704,14 @@ boolean addPeerToEthPeers(final EthPeer peer) { .min(MOST_USEFUL_PEER) .ifPresent( p -> { - p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); - LOG.atTrace().setMessage("Disconnecting peer {} to be replaced by prioritised peer {}").addArgument(p.getLoggableId()).addArgument(peer.getLoggableId()).log(); - }) - ); + p.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); + LOG.atTrace() + .setMessage( + "Disconnecting peer {} to be replaced by prioritised peer {}") + .addArgument(p.getLoggableId()) + .addArgument(peer.getLoggableId()) + .log(); + })); } else { LOG.atTrace() .setMessage("Too many peers. Disconnect connection: {}, max connections {}") diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java index cdb824cce15..2acfa54d04d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java @@ -85,14 +85,14 @@ protected CompletableFuture> selectBestAvailableSyncTarget() { "Unable to find sync target. Currently checking %d peers for usefulness. Pivot block: %d", ethContext.getEthPeers().peerCount(), pivotBlockHeader.getNumber()), logDebug, - LOG_DEBUG_REPEAT_DELAY); + LOG_DEBUG_REPEAT_DELAY); throttledLog( LOG::info, String.format( "Unable to find sync target. Currently checking %d peers for usefulness.", ethContext.getEthPeers().peerCount()), logInfo, - LOG_INFO_REPEAT_DELAY); + LOG_INFO_REPEAT_DELAY); return completedFuture(Optional.empty()); } else { final EthPeer bestPeer = maybeBestPeer.get(); From 13807b13c311cd60af5fa0dd9800dabbff005a7e Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 25 Jun 2024 17:48:37 +1000 Subject: [PATCH 25/26] fix tests, fix trailing peers, and clean up Signed-off-by: stefan.pingel@consensys.net --- .../org/hyperledger/besu/RunnerBuilder.java | 4 +- .../controller/BesuControllerBuilder.java | 7 +- .../TransitionBesuControllerBuilder.java | 20 ++-- .../besu/ethereum/eth/manager/EthPeers.java | 100 ++++++++++++------ .../snap/GetAccountRangeFromPeerTask.java | 22 ++-- .../manager/snap/GetBytecodeFromPeerTask.java | 15 ++- .../snap/GetStorageRangeFromPeerTask.java | 22 ++-- .../manager/snap/GetTrieNodeFromPeerTask.java | 6 +- .../RetryingGetAccountRangeFromPeerTask.java | 26 ++--- .../ethereum/eth/sync/ChainHeadTracker.java | 5 +- .../eth/sync/DefaultSynchronizer.java | 2 +- .../ethereum/eth/sync/SnapServerChecker.java | 15 ++- .../ethereum/eth/manager/EthPeersTest.java | 2 +- .../eth/manager/EthProtocolManagerTest.java | 9 +- .../manager/EthProtocolManagerTestUtil.java | 8 +- .../eth/manager/RespondingEthPeer.java | 1 + .../ethtaskutils/AbstractMessageTaskTest.java | 2 +- .../AbstractBlockPropagationManagerTest.java | 4 +- .../fastsync/PivotBlockRetrieverTest.java | 6 +- .../ethereum/eth/transactions/TestNode.java | 6 +- .../TransactionPoolFactoryTest.java | 2 +- .../rlpx/wire/messages/DisconnectMessage.java | 4 +- .../ethereum/retesteth/RetestethContext.java | 2 +- 23 files changed, 163 insertions(+), 127 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index b79f6e9ef03..d62843049e5 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -710,8 +710,8 @@ public Runner build() { .blockchain(context.getBlockchain()) .blockNumberForks(besuController.getGenesisConfigOptions().getForkBlockNumbers()) .timestampForks(besuController.getGenesisConfigOptions().getForkBlockTimestamps()) - .allConnectionsSupplier(ethPeers::getAllConnections) - .allActiveConnectionsSupplier(ethPeers::getAllActiveConnections) + .allConnectionsSupplier(ethPeers::streamAllConnections) + .allActiveConnectionsSupplier(ethPeers::streamAllActiveConnections) .maxPeers(ethPeers.getMaxPeers()) .build(); }; diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 94da1026c15..3c33c318859 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -45,7 +45,6 @@ import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; -import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.eth.EthProtocol; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.SnapProtocol; @@ -695,7 +694,7 @@ public BesuController build() { createPivotSelector( protocolSchedule, protocolContext, ethContext, syncState, metricsSystem); - final Synchronizer synchronizer = + final DefaultSynchronizer synchronizer = createSynchronizer( protocolSchedule, worldStateStorageCoordinator, @@ -705,6 +704,8 @@ public BesuController build() { ethProtocolManager, pivotBlockSelector); + ethPeers.setTrailingPeerRequirementsSupplier(synchronizer::calculateTrailingPeerRequirements); + if (SyncMode.isSnapSync(syncConfig.getSyncMode()) || SyncMode.isCheckpointSync(syncConfig.getSyncMode())) { synchronizer.subscribeInSync((b) -> ethPeers.snapServerPeersNeeded(!b)); @@ -825,7 +826,7 @@ private TrieLogPruner createTrieLogPruner( * @param pivotBlockSelector the pivot block selector * @return the synchronizer */ - protected Synchronizer createSynchronizer( + protected DefaultSynchronizer createSynchronizer( final ProtocolSchedule protocolSchedule, final WorldStateStorageCoordinator worldStateStorageCoordinator, final ProtocolContext protocolContext, diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java index 33f25343093..4f084f3c138 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java @@ -33,7 +33,6 @@ import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; -import org.hyperledger.besu.ethereum.core.Synchronizer; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthMessages; @@ -215,7 +214,7 @@ protected PluginServiceFactory createAdditionalPluginServices( } @Override - protected Synchronizer createSynchronizer( + protected DefaultSynchronizer createSynchronizer( final ProtocolSchedule protocolSchedule, final WorldStateStorageCoordinator worldStateStorageCoordinator, final ProtocolContext protocolContext, @@ -225,15 +224,14 @@ protected Synchronizer createSynchronizer( final PivotBlockSelector pivotBlockSelector) { DefaultSynchronizer sync = - (DefaultSynchronizer) - super.createSynchronizer( - protocolSchedule, - worldStateStorageCoordinator, - protocolContext, - ethContext, - syncState, - ethProtocolManager, - pivotBlockSelector); + super.createSynchronizer( + protocolSchedule, + worldStateStorageCoordinator, + protocolContext, + ethContext, + syncState, + ethProtocolManager, + pivotBlockSelector); if (genesisConfigOptions.getTerminalTotalDifficulty().isPresent()) { LOG.info( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index b2a707c63f5..851c075290e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.ethereum.eth.sync.ChainHeadTracker; import org.hyperledger.besu.ethereum.eth.sync.SnapServerChecker; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; +import org.hyperledger.besu.ethereum.eth.sync.TrailingPeerRequirements; import org.hyperledger.besu.ethereum.forkid.ForkId; import org.hyperledger.besu.ethereum.forkid.ForkIdManager; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; @@ -115,6 +116,8 @@ public class EthPeers { private ChainHeadTracker tracker; private SnapServerChecker snapServerChecker; private boolean snapServerPeersNeeded = false; + private Supplier trailingPeerRequirementsSupplier = + () -> TrailingPeerRequirements.UNRESTRICTED; public EthPeers( final String protocolName, @@ -150,17 +153,12 @@ public EthPeers( BesuMetricCategory.ETHEREUM, "peer_count", "The current number of peers connected", - () -> (int) streamAvailablePeers().filter(EthPeer::readyForRequests).count()); + activeConnections::size); metricsSystem.createIntegerGauge( BesuMetricCategory.ETHEREUM, "peer_count_snap_server", "The current number of peers connected that serve snap data", - () -> - (int) - streamAvailablePeers() - .filter(EthPeer::readyForRequests) - .filter(EthPeer::isServingSnap) - .count()); + () -> (int) streamAvailablePeers().filter(EthPeer::isServingSnap).count()); metricsSystem.createIntegerGauge( BesuMetricCategory.PEERS, "pending_peer_requests_current", @@ -348,9 +346,7 @@ private void removeDisconnectedPeers() { } public Stream streamAvailablePeers() { - return streamAllPeers() - .filter(EthPeer::readyForRequests) - .filter(peer -> !peer.isDisconnected()); + return streamAllPeers().filter(peer -> !peer.isDisconnected()); } public Stream streamBestPeers() { @@ -385,13 +381,13 @@ public void setRlpxAgent(final RlpxAgent rlpxAgent) { this.rlpxAgent = rlpxAgent; } - public Stream getAllActiveConnections() { + public Stream streamAllActiveConnections() { return activeConnections.values().stream() .map(EthPeer::getConnection) .filter(c -> !c.isDisconnected()); } - public Stream getAllConnections() { + public Stream streamAllConnections() { return Stream.concat( activeConnections.values().stream().map(EthPeer::getConnection), incompleteConnections.asMap().keySet().stream()) @@ -416,7 +412,7 @@ public boolean shouldTryToConnect(final Peer peer, final boolean inbound) { final Bytes id = peer.getId(); if (alreadyConnectedOrConnecting(inbound, id)) { LOG.atTrace() - .setMessage("not connecting to peer {} - too many peers") + .setMessage("not connecting to peer {} - already connected") .addArgument(peer.getLoggableId()) .log(); return false; @@ -428,18 +424,11 @@ public boolean shouldTryToConnect(final Peer peer, final boolean inbound) { private boolean alreadyConnectedOrConnecting(final boolean inbound, final Bytes id) { final EthPeer ethPeer = activeConnections.get(id); if (ethPeer != null && !ethPeer.isDisconnected()) { - LOG.atTrace() - .setMessage("not connecting to peer {} - already disconnected") - .addArgument(ethPeer.getLoggableId()) - .log(); return true; } final List incompleteConnections = getIncompleteConnections(id); - if (!incompleteConnections.isEmpty()) { - return incompleteConnections.stream() - .anyMatch(c -> !c.isDisconnected() && (!inbound || (inbound && c.inboundInitiated()))); - } - return false; + return incompleteConnections.stream() + .anyMatch(c -> !c.isDisconnected() && (!inbound || (inbound && c.inboundInitiated()))); } public void disconnectWorstUselessPeer() { @@ -471,6 +460,11 @@ public void snapServerPeersNeeded(final boolean b) { this.snapServerPeersNeeded = b; } + public void setTrailingPeerRequirementsSupplier( + final Supplier tprSupplier) { + this.trailingPeerRequirementsSupplier = tprSupplier; + } + @FunctionalInterface public interface ConnectCallback { void onPeerConnected(EthPeer newPeer); @@ -507,6 +501,28 @@ private void ethPeerStatusExchanged(final EthPeer peer) { peer.disconnect( DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD); } else { + + // we can check trailing peers now + final TrailingPeerRequirements trailingPeerRequirements = + trailingPeerRequirementsSupplier.get(); + if (trailingPeerRequirements != null) { + if (peer.chainState().getEstimatedHeight() + < trailingPeerRequirements.getMinimumHeightToBeUpToDate()) { + if (!(getNumTrailingPeers(trailingPeerRequirements.getMinimumHeightToBeUpToDate()) + < trailingPeerRequirements.getMaxTrailingPeers())) { + LOG.atTrace() + .setMessage( + "Adding trailing peer {} would exceed max trailing peers {}. Disconnecting...") + .addArgument(peer.getLoggableId()) + .addArgument(trailingPeerRequirements.getMaxTrailingPeers()) + .log(); + peer.disconnect( + DisconnectMessage.DisconnectReason.USELESS_PEER_EXCEEDS_TRAILING_PEERS); + return; + } + } + } + peer.chainState().updateHeightEstimate(peerHeadBlockHeader.getNumber()); CompletableFuture isServingSnapFuture; if (SyncMode.isCheckpointSync(syncMode) || SyncMode.isSnapSync(syncMode)) { @@ -542,18 +558,21 @@ private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBl peer.setIsServingSnap(true); Boolean isServer; try { - isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(10L, TimeUnit.SECONDS); + isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(6L, TimeUnit.SECONDS); } catch (Exception e) { - // TODO: change LOG to debug? - LOG.atInfo() - .setMessage("XXXXXX Error checking if peer is a snap server. {}") - .addArgument(e.getStackTrace()) + LOG.atTrace() + .setMessage("Error checking if peer {} is a snap server. Setting to false.") + .addArgument(peer.getLoggableId()) .log(); peer.setIsServingSnap(false); return; } peer.setIsServingSnap(isServer); - LOG.info("Peer {} snap server? {}", peer.getLoggableId(), isServer); + LOG.atTrace() + .setMessage("{}: peer {}") + .addArgument(isServer ? "Is a snap server" : "Is NOT a snap server") + .addArgument(peer.getLoggableId()) + .log(); } } } @@ -671,9 +690,9 @@ boolean addPeerToEthPeers(final EthPeer peer) { // Figure out whether we want to add it to the active connections. final PeerConnection connection = peer.getConnection(); if (activeConnections.containsValue(peer)) { - // connection.disconnect(DisconnectMessage.DisconnectReason.ALREADY_CONNECTED); return false; } + final Bytes id = peer.getId(); if (!randomPeerPriority) { @@ -697,9 +716,16 @@ boolean addPeerToEthPeers(final EthPeer peer) { .filter(inboundLimitExceeded ? p -> p.getConnection().inboundInitiated() : p -> true) .min(MOST_USEFUL_PEER) .ifPresentOrElse( - pe -> pe.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS), - () -> - activeConnections.values().stream() + pe -> { + pe.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); + LOG.atTrace() + .setMessage("Disconnecting peer {} to be replaced by prioritised peer {}") + .addArgument(pe.getLoggableId()) + .addArgument(peer.getLoggableId()) + .log(); + }, + () -> // disconnect the least useful peer + activeConnections.values().stream() .filter(p -> !canExceedPeerLimits(p.getId())) .min(MOST_USEFUL_PEER) .ifPresent( @@ -714,7 +740,9 @@ boolean addPeerToEthPeers(final EthPeer peer) { })); } else { LOG.atTrace() - .setMessage("Too many peers. Disconnect connection: {}, max connections {}") + .setMessage( + "Too many peers. Disconnect peer {} with connection: {}, max connections {}") + .addArgument(peer.getLoggableId()) .addArgument(connection) .addArgument(peerUpperBound) .log(); @@ -749,6 +777,12 @@ boolean addPeerToEthPeers(final EthPeer peer) { } } + private long getNumTrailingPeers(final long minimumHeightToBeUpToDate) { + return streamAvailablePeers() + .filter(p -> p.chainState().getEstimatedHeight() < minimumHeightToBeUpToDate) + .count(); + } + private boolean needMoreSnapServers() { return snapServerPeersNeeded && numberOfSnapServers() < snapServerTargetNumber; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java index c16d9e8c936..8e67ff4aeb2 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetAccountRangeFromPeerTask.java @@ -71,17 +71,19 @@ protected PendingPeerRequest sendRequest() { @Override public RequestManager.ResponseStream sendRequest(final EthPeer peer) throws PeerConnection.PeerNotConnected { - LOG.debug( - "Requesting account range [{} ,{}] for state root {} from peer {} .", - startKeyHash, - endKeyHash, - blockHeader.getStateRoot(), - peer); + LOG.atTrace() + .setMessage("Requesting account range [{} ,{}] for state root {} from peer {} .") + .addArgument(startKeyHash) + .addArgument(endKeyHash) + .addArgument(blockHeader) + .addArgument(peer) + .log(); if (!peer.isServingSnap()) { - LOG.debug( - "EthPeer that is not serving snap called in {}, {}", - GetAccountRangeFromPeerTask.class, - peer); + LOG.atDebug() + .setMessage("EthPeer that is not serving snap called in {}, peer: {}") + .addArgument(GetAccountRangeFromPeerTask.class) + .addArgument(peer) + .log(); throw new RuntimeException( "EthPeer that is not serving snap called in " + GetAccountRangeFromPeerTask.class); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java index 4f1665d0007..1c8c1d12d74 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetBytecodeFromPeerTask.java @@ -73,12 +73,17 @@ protected PendingPeerRequest sendRequest() { @Override public RequestManager.ResponseStream sendRequest(final EthPeer peer) throws PeerConnection.PeerNotConnected { - LOG.trace("Requesting {} Bytecodes from {} .", codeHashes.size(), peer); + LOG.atTrace() + .setMessage("Requesting {} Bytecodes from {} .") + .addArgument(codeHashes.size()) + .addArgument(peer) + .log(); if (!peer.isServingSnap()) { - LOG.debug( - "EthPeer that is not serving snap called in {}, {}", - GetAccountRangeFromPeerTask.class, - peer); + LOG.atDebug() + .setMessage("EthPeer that is not serving snap called in {}, peer: {}") + .addArgument(GetAccountRangeFromPeerTask.class) + .addArgument(peer) + .log(); throw new RuntimeException( "EthPeer that is not serving snap called in " + GetAccountRangeFromPeerTask.class); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java index 8d0ffd1d27f..cb3e4c83ea9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetStorageRangeFromPeerTask.java @@ -76,17 +76,19 @@ protected PendingPeerRequest sendRequest() { @Override public RequestManager.ResponseStream sendRequest(final EthPeer peer) throws PeerConnection.PeerNotConnected { - LOG.trace( - "Requesting storage range [{} ,{}] for {} accounts from peer {} .", - startKeyHash, - endKeyHash, - accountHashes.size(), - peer); + LOG.atTrace() + .setMessage("Requesting storage range [{} ,{}] for {} accounts from peer {} .") + .addArgument(startKeyHash) + .addArgument(endKeyHash) + .addArgument(accountHashes.size()) + .addArgument(peer) + .log(); if (!peer.isServingSnap()) { - LOG.debug( - "EthPeer that is not serving snap called in {}, {}", - GetAccountRangeFromPeerTask.class, - peer); + LOG.atDebug() + .setMessage("EthPeer that is not serving snap called in {}, peer: {}") + .addArgument(GetAccountRangeFromPeerTask.class) + .addArgument(peer) + .log(); throw new RuntimeException( "EthPeer that is not serving snap called in " + GetAccountRangeFromPeerTask.class); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java index 08ff27b3bd1..a060ca79d0b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/GetTrieNodeFromPeerTask.java @@ -79,7 +79,11 @@ protected PendingPeerRequest sendRequest() { @Override public RequestManager.ResponseStream sendRequest(final EthPeer peer) throws PeerConnection.PeerNotConnected { - LOG.trace("Requesting {} trie nodes from peer {}", paths.size(), peer); + LOG.atTrace() + .setMessage("Requesting {} trie nodes from peer {}") + .addArgument(paths.size()) + .addArgument(peer) + .log(); if (!peer.isServingSnap()) { LOG.debug( "EthPeer that is not serving snap called in {}, {}", diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index 7dbae79fdb2..0624a90589c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -17,7 +17,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingSwitchingPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -28,7 +28,9 @@ import org.apache.tuweni.bytes.Bytes32; public class RetryingGetAccountRangeFromPeerTask - extends AbstractRetryingSwitchingPeerTask { + extends AbstractRetryingPeerTask { + + public static final int MAX_RETRIES = 4; private final EthContext ethContext; private final Bytes32 startKeyHash; @@ -43,7 +45,10 @@ private RetryingGetAccountRangeFromPeerTask( final BlockHeader blockHeader, final MetricsSystem metricsSystem) { super( - ethContext, metricsSystem, data -> data.accounts().isEmpty() && data.proofs().isEmpty(), 4); + ethContext, + MAX_RETRIES, + data -> data.accounts().isEmpty() && data.proofs().isEmpty(), + metricsSystem); this.ethContext = ethContext; this.startKeyHash = startKeyHash; this.endKeyHash = endKeyHash; @@ -61,21 +66,6 @@ public static EthTask forAccountRange( ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem); } - @Override - protected CompletableFuture executeTaskOnCurrentPeer( - final EthPeer peer) { - final GetAccountRangeFromPeerTask task = - GetAccountRangeFromPeerTask.forAccountRange( - ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem); - task.assignPeer(peer); - return executeSubTask(task::run) - .thenApply( - peerResult -> { - result.complete(peerResult.getResult()); - return peerResult.getResult(); - }); - } - @Override protected CompletableFuture executePeerTask( final Optional assignedPeer) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java index b3e28dfb807..67681acf6a7 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/ChainHeadTracker.java @@ -38,7 +38,6 @@ public class ChainHeadTracker { private final EthContext ethContext; private final ProtocolSchedule protocolSchedule; - private final TrailingPeerLimiter trailingPeerLimiter; private final MetricsSystem metricsSystem; public ChainHeadTracker( @@ -48,7 +47,6 @@ public ChainHeadTracker( final MetricsSystem metricsSystem) { this.ethContext = ethContext; this.protocolSchedule = protocolSchedule; - this.trailingPeerLimiter = trailingPeerLimiter; this.metricsSystem = metricsSystem; } @@ -79,7 +77,6 @@ public CompletableFuture getBestHeaderFromPeer(final EthPeer peer) if (peerResult != null && !peerResult.getResult().isEmpty()) { final BlockHeader chainHeadHeader = peerResult.getResult().get(0); peer.chainState().update(chainHeadHeader); - trailingPeerLimiter.enforceTrailingPeerLimit(); future.complete(chainHeadHeader); LOG.atDebug() .setMessage("Retrieved chain head info {} from {}...") @@ -91,7 +88,7 @@ public CompletableFuture getBestHeaderFromPeer(final EthPeer peer) LOG.atDebug() .setMessage("Failed to retrieve chain head info. Disconnecting {}... {}") .addArgument(peer::getLoggableId) - .addArgument(error) + .addArgument(error != null ? error : "Empty Response") .log(); peer.disconnect( DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java index bea32e3ae4d..cd4867427f8 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java @@ -191,7 +191,7 @@ public DefaultSynchronizer( () -> getSyncStatus().isPresent() ? 0 : 1); } - private TrailingPeerRequirements calculateTrailingPeerRequirements() { + public TrailingPeerRequirements calculateTrailingPeerRequirements() { return fastSyncDownloader .flatMap(FastSyncDownloader::calculateTrailingPeerRequirements) .orElse( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java index 3dcce26b7fc..c7fa141837b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java @@ -46,28 +46,27 @@ public static void createAndSetSnapServerChecker( ethContext.getEthPeers().setSnapServerChecker(checker); } - public CompletableFuture check(final EthPeer peer, final BlockHeader header) { - LOG.atDebug() + public CompletableFuture check(final EthPeer peer, final BlockHeader peersHeadHeader) { + LOG.atTrace() .setMessage("Checking whether peer {} is a snap server ...") .addArgument(peer::getLoggableId) .log(); final CompletableFuture> - snapServerCheckCompletableFuture = getAccountRangeFromPeer(peer, header); + snapServerCheckCompletableFuture = getAccountRangeFromPeer(peer, peersHeadHeader); final CompletableFuture future = new CompletableFuture<>(); snapServerCheckCompletableFuture.whenComplete( (peerResult, error) -> { if (peerResult != null) { if (!peerResult.getResult().accounts().isEmpty() || !peerResult.getResult().proofs().isEmpty()) { - LOG.atInfo() - .setMessage("Peer {} is a snap server! getAccountRangeResult: {}") + LOG.atTrace() + .setMessage("Peer {} is a snap server.") .addArgument(peer::getLoggableId) - .addArgument(peerResult.getResult()) .log(); future.complete(true); } else { - LOG.atDebug() - .setMessage("Peer {} is not a snap server ...") + LOG.atTrace() + .setMessage("Peer {} is not a snap server.") .addArgument(peer::getLoggableId) .log(); future.complete(false); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java index 74d1a75b9e7..f6c635aa51f 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthPeersTest.java @@ -406,7 +406,7 @@ public void snapServersPreferredWhileSyncing() { } @Test - public void snapServersNotPreferedWhenInSync() { + public void snapServersNotPreferredWhenInSync() { ethPeers.snapServerPeersNeeded(false); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java index ff5d655cc61..93a80be2fd2 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java @@ -528,7 +528,7 @@ public void respondToGetHeadersReversedWithSkip() private MockPeerConnection setupPeer( final EthProtocolManager ethManager, final PeerSendHandler onSend) { - final MockPeerConnection peer = setupPeerWithoutStatusExchange(ethManager, onSend); + final MockPeerConnection peerConnection = setupPeerWithoutStatusExchange(ethManager, onSend); final StatusMessage statusMessage = StatusMessage.create( EthProtocolVersion.V63, @@ -536,8 +536,11 @@ private MockPeerConnection setupPeer( blockchain.getChainHead().getTotalDifficulty(), blockchain.getChainHeadHash(), blockchain.getBlockHeader(BlockHeader.GENESIS_BLOCK_NUMBER).get().getHash()); - ethManager.processMessage(EthProtocol.ETH63, new DefaultMessage(peer, statusMessage)); - return peer; + ethManager.processMessage(EthProtocol.ETH63, new DefaultMessage(peerConnection, statusMessage)); + final EthPeers ethPeers = ethManager.ethContext().getEthPeers(); + final EthPeer ethPeer = ethPeers.peer(peerConnection); + ethPeers.addPeerToEthPeers(ethPeer); + return peerConnection; } private MockPeerConnection setupPeerWithoutStatusExchange( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java index 1f73221b209..0b0bd1e3eb7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java @@ -94,7 +94,7 @@ public static EthProtocolManager create( 25, 25, false, - SyncMode.X_SNAP, + SyncMode.FAST, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final ChainHeadTracker chainHeadTrackerMock = getChainHeadTrackerMock(); @@ -221,7 +221,7 @@ public static EthProtocolManager create( 25, 25, false, - SyncMode.X_SNAP, + SyncMode.FAST, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthMessages messages = new EthMessages(); @@ -272,7 +272,7 @@ public static EthProtocolManager create( 25, 25, false, - SyncMode.X_SNAP, + SyncMode.FAST, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final EthMessages messages = new EthMessages(); @@ -304,7 +304,7 @@ public static EthProtocolManager create( 25, 25, false, - SyncMode.X_SNAP, + SyncMode.FAST, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final ChainHeadTracker chainHeadTrackerMock = getChainHeadTrackerMock(); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java index 9ff37de8f6a..58f4893432c 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/RespondingEthPeer.java @@ -138,6 +138,7 @@ private static RespondingEthPeer create( estimatedHeight.ifPresent(height -> peer.chainState().update(chainHeadHash, height)); if (addToEthPeers) { peer.registerStatusSent(peerConnection); + ethPeers.addPeerToEthPeers(peer); while (ethPeers.peerCount() <= before) { // this is needed to make sure that the peer is added to the active // connections diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java index b021203759b..6dbea259cc2 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java @@ -124,7 +124,7 @@ public void setupTest() { MAX_PEERS, MAX_PEERS, false, - SyncMode.X_SNAP, + SyncMode.FAST, new ForkIdManager( blockchain, Collections.emptyList(), Collections.emptyList(), false))); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java index 9a961573d29..dcc0239f616 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java @@ -631,7 +631,7 @@ public void shouldNotImportBlocksThatAreAlreadyBeingImported() { 25, 25, false, - SyncMode.X_SNAP, + SyncMode.SNAP, new ForkIdManager( blockchain, Collections.emptyList(), Collections.emptyList(), false)), new EthMessages(), @@ -772,7 +772,7 @@ public Object answer(final InvocationOnMock invocation) throws Throwable { 25, 25, false, - SyncMode.X_SNAP, + SyncMode.SNAP, new ForkIdManager( blockchain, Collections.emptyList(), Collections.emptyList(), false)), new EthMessages(), diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java index 847ef535192..41317fdab38 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetrieverTest.java @@ -288,15 +288,15 @@ public void shouldRecoverFromUnresponsivePeer(final DataStorageFormat storageFor final CompletableFuture future = pivotBlockRetriever.downloadPivotBlockHeader(); peerA.respond(responder); - peerB.respondTimes(emptyResponder, 2); + peerB.respondTimes(emptyResponder, 4); // PeerA should have responded, while peerB is being retried, peerC shouldn't have been queried // yet assertThat(future).isNotCompleted(); assertThat(peerC.hasOutstandingRequests()).isFalse(); - // After exhausting retries for peerB, we should try peerC - peerB.respondTimes(emptyResponder, 2); + // After exhausting retries (max retries is 5) for peerB, we should try peerC + peerB.respondTimes(emptyResponder, 1); peerC.respond(responder); assertThat(future) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java index 5925be32572..8e9bda89c05 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java @@ -160,7 +160,7 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod 25, 25, false, - SyncMode.X_SNAP, + SyncMode.SNAP, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); final ChainHeadTracker mockCHT = getChainHeadTracker(); @@ -213,8 +213,8 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod .blockchain(blockchain) .blockNumberForks(Collections.emptyList()) .timestampForks(Collections.emptyList()) - .allConnectionsSupplier(ethPeers::getAllConnections) - .allActiveConnectionsSupplier(ethPeers::getAllActiveConnections) + .allConnectionsSupplier(ethPeers::streamAllConnections) + .allActiveConnectionsSupplier(ethPeers::streamAllActiveConnections) .build()) .metricsSystem(new NoOpMetricsSystem()) .build(); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java index 3456421d16b..ef8d0701782 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java @@ -122,7 +122,7 @@ public void setup() { 25, 25, false, - SyncMode.X_SNAP, + SyncMode.SNAP, new ForkIdManager(blockchain, Collections.emptyList(), Collections.emptyList(), false)); when(ethContext.getEthMessages()).thenReturn(ethMessages); when(ethContext.getEthPeers()).thenReturn(ethPeers); diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java index d23a4fd56fa..d951ecc5d9e 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java @@ -129,11 +129,11 @@ public enum DisconnectReason { USELESS_PEER_NO_SHARED_CAPABILITIES((byte) 0x03, "No shared capabilities"), USELESS_PEER_WORLD_STATE_NOT_AVAILABLE((byte) 0x03, "World state not available"), USELESS_PEER_MISMATCHED_PIVOT_BLOCK((byte) 0x03, "Mismatched pivot block"), - USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD( - (byte) 0x03, "Failed to retrieve the chain head header"), + USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD((byte) 0x03, "Failed to retrieve chain head header"), USELESS_PEER_CANNOT_CONFIRM_PIVOT_BLOCK((byte) 0x03, "Peer failed to confirm pivot block"), USELESS_PEER_BY_REPUTATION((byte) 0x03, "Lowest reputation score"), USELESS_PEER_BY_CHAIN_COMPARATOR((byte) 0x03, "Lowest by chain height comparator"), + USELESS_PEER_EXCEEDS_TRAILING_PEERS((byte) 0x03, "Adding peer would exceed max trailing peers"), TOO_MANY_PEERS((byte) 0x04), ALREADY_CONNECTED((byte) 0x05), INCOMPATIBLE_P2P_PROTOCOL_VERSION((byte) 0x06), diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java index a62eeca4f5c..fd39ef7f44c 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java @@ -243,7 +243,7 @@ private boolean buildContext( MAX_PEERS, MAX_PEERS, false, - SyncMode.X_SNAP, + SyncMode.FAST, new ForkIdManager(blockchain, List.of(), List.of(), false)); final SyncState syncState = new SyncState(blockchain, ethPeers); From 2a84c29f2292ef7c302b7103b5ace08b2e76c796 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Wed, 3 Jul 2024 11:54:27 +1000 Subject: [PATCH 26/26] review change Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index 412e77823e4..e7f1556b5a2 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -78,6 +78,7 @@ public boolean assignPeer(final EthPeer peer) { assignedPeer = Optional.of(peer); return true; } else { + assignedPeer = Optional.empty(); return false; } }