Skip to content

Commit f6e7880

Browse files
Gabriel-TrintinaliaGabriel-Trintinalia
committed
Revert "feat: remove chain head check if peer supports eth/69 (#8725)"
This reverts commit ed1086d. Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
1 parent e200e11 commit f6e7880

File tree

4 files changed

+66
-93
lines changed

4 files changed

+66
-93
lines changed

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242

4343
import java.time.Clock;
4444
import java.util.Collections;
45-
import java.util.Comparator;
4645
import java.util.HashMap;
4746
import java.util.LinkedHashMap;
4847
import java.util.List;
@@ -632,12 +631,6 @@ public boolean hasSupportForMessage(final int messageCode) {
632631
.anyMatch(cap -> EthProtocol.get().isValidMessageCode(cap.getVersion(), messageCode));
633632
}
634633

635-
public Capability getMaxAgreedCapability() {
636-
return getAgreedCapabilities().stream()
637-
.max(Comparator.comparingInt(Capability::getVersion))
638-
.orElseThrow(() -> new IllegalStateException("No capabilities available"));
639-
}
640-
641634
@Override
642635
public String toString() {
643636
return String.format(

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java‎

Lines changed: 60 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
3030
import org.hyperledger.besu.ethereum.p2p.peers.Peer;
3131
import org.hyperledger.besu.ethereum.p2p.peers.PeerId;
32+
import org.hyperledger.besu.ethereum.p2p.rlpx.ConnectCallback;
3233
import org.hyperledger.besu.ethereum.p2p.rlpx.RlpxAgent;
3334
import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection;
3435
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.PeerClientName;
@@ -556,95 +557,80 @@ public String toString() {
556557

557558
private void ethPeerStatusExchanged(final EthPeer peer) {
558559
// We have a connection to a peer that is on the right chain and is willing to connect to us.
560+
// Find out what the EthPeer block height is and whether it can serve snap data (if we are doing
561+
// snap sync)
559562
LOG.debug("Peer {} status exchanged", peer);
560563
assert tracker != null : "ChainHeadTracker must be set before EthPeers can be used";
564+
CompletableFuture<BlockHeader> future = tracker.getBestHeaderFromPeer(peer);
565+
566+
future.whenComplete(
567+
(peerHeadBlockHeader, error) -> {
568+
if (peerHeadBlockHeader == null) {
569+
LOG.debug(
570+
"Failed to retrieve chain head info. Disconnecting {}... {}",
571+
peer.getLoggableId(),
572+
error);
573+
peer.disconnect(
574+
DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD);
575+
} else {
576+
577+
// we can check trailing peers now
578+
final TrailingPeerRequirements trailingPeerRequirements =
579+
trailingPeerRequirementsSupplier.get();
580+
if (trailingPeerRequirements != null) {
581+
if (peer.chainState().getEstimatedHeight()
582+
< trailingPeerRequirements.getMinimumHeightToBeUpToDate()) {
583+
if (!(getNumTrailingPeers(trailingPeerRequirements.getMinimumHeightToBeUpToDate())
584+
< trailingPeerRequirements.getMaxTrailingPeers())) {
585+
LOG.atTrace()
586+
.setMessage(
587+
"Adding trailing peer {} would exceed max trailing peers {}. Disconnecting...")
588+
.addArgument(peer.getLoggableId())
589+
.addArgument(trailingPeerRequirements.getMaxTrailingPeers())
590+
.log();
591+
peer.disconnect(
592+
DisconnectMessage.DisconnectReason.USELESS_PEER_EXCEEDS_TRAILING_PEERS);
593+
return;
594+
}
595+
}
596+
}
561597

562-
// Handle chain head tracking based on protocol compatibility.
563-
if (EthProtocol.isEth69Compatible(peer.getMaxAgreedCapability())) {
564-
// For Eth69-compatible peers, the chain head information is included in the status message.
565-
// Skip fetching the chain head separately and directly check trailing peer requirements.
566-
checkTrailingPeerRequirements(peer);
567-
} else {
568-
// For non-Eth69-compatible peers, fetch the chain head before checking trailing requirements.
569-
CompletableFuture<BlockHeader> future = tracker.getBestHeaderFromPeer(peer);
570-
future.whenComplete(
571-
(peerHeadBlockHeader, error) -> {
572-
// If we successfully retrieved the chain head, check trailing peer requirements and
573-
// update
574-
// the peer's estimated height.
575-
if (peerHeadBlockHeader != null) {
576-
checkTrailingPeerRequirements(peer);
577-
peer.chainState().updateHeightEstimate(peerHeadBlockHeader.getNumber());
598+
peer.chainState().updateHeightEstimate(peerHeadBlockHeader.getNumber());
599+
CompletableFuture<Void> isServingSnapFuture;
600+
if (syncMode == SyncMode.SNAP || syncMode == SyncMode.CHECKPOINT) {
601+
// even if we have finished the snap sync, we still want to know if the peer is a snap
602+
// server
603+
isServingSnapFuture =
604+
CompletableFuture.runAsync(
605+
() -> {
606+
try {
607+
checkIsSnapServer(peer, peerHeadBlockHeader);
608+
} catch (Exception e) {
609+
throw new RuntimeException(e);
610+
}
611+
});
578612
} else {
579-
LOG.debug(
580-
"Failed to retrieve chain head info. Disconnecting {}... {}",
581-
peer.getLoggableId(),
582-
error);
583-
peer.disconnect(
584-
DisconnectMessage.DisconnectReason.USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_HEAD);
613+
isServingSnapFuture = CompletableFuture.completedFuture(null);
585614
}
586-
});
587-
}
588-
589-
// If we are doing snap sync, check if the peer is a snap server
590-
checkSnapServer(peer);
591-
}
592-
593-
private void checkTrailingPeerRequirements(final EthPeer peer) {
594-
// we can check trailing peers now
595-
final TrailingPeerRequirements trailingPeerRequirements =
596-
trailingPeerRequirementsSupplier.get();
597-
if (trailingPeerRequirements != null) {
598-
if (peer.chainState().getEstimatedHeight()
599-
< trailingPeerRequirements.getMinimumHeightToBeUpToDate()) {
600-
if (!(getNumTrailingPeers(trailingPeerRequirements.getMinimumHeightToBeUpToDate())
601-
< trailingPeerRequirements.getMaxTrailingPeers())) {
602-
LOG.atTrace()
603-
.setMessage(
604-
"Adding trailing peer {} would exceed max trailing peers {}. Disconnecting...")
605-
.addArgument(peer.getLoggableId())
606-
.addArgument(trailingPeerRequirements.getMaxTrailingPeers())
607-
.log();
608-
peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER_EXCEEDS_TRAILING_PEERS);
609-
}
610-
}
611-
}
612-
}
613-
614-
private void checkSnapServer(final EthPeer peer) {
615-
CompletableFuture<Void> isServingSnapFuture;
616-
if (syncMode == SyncMode.SNAP || syncMode == SyncMode.CHECKPOINT) {
617-
// even if we have finished the snap sync, we still want to know if the peer is a snap
618-
// server
619-
isServingSnapFuture =
620-
CompletableFuture.runAsync(
621-
() -> {
622-
try {
623-
checkIsSnapServer(peer);
624-
} catch (Exception e) {
625-
throw new RuntimeException(e);
626-
}
627-
});
628-
} else {
629-
isServingSnapFuture = CompletableFuture.completedFuture(null);
630-
}
631-
isServingSnapFuture.thenRun(
632-
() -> {
633-
if (!peer.getConnection().isDisconnected() && addPeerToEthPeers(peer)) {
634-
connectedPeersCounter.inc();
635-
connectCallbacks.forEach(cb -> cb.onPeerConnected(peer));
615+
isServingSnapFuture.thenRun(
616+
() -> {
617+
if (!peer.getConnection().isDisconnected() && addPeerToEthPeers(peer)) {
618+
connectedPeersCounter.inc();
619+
connectCallbacks.forEach(cb -> cb.onPeerConnected(peer));
620+
}
621+
});
636622
}
637623
});
638624
}
639625

640-
private void checkIsSnapServer(final EthPeer peer) {
626+
private void checkIsSnapServer(final EthPeer peer, final BlockHeader peersHeadBlockHeader) {
641627
if (peer.getAgreedCapabilities().contains(SnapProtocol.SNAP1)) {
642628
if (snapServerChecker != null) {
643629
// set that peer is a snap server for doing the test
644630
peer.setIsServingSnap(true);
645631
Boolean isServer;
646632
try {
647-
isServer = snapServerChecker.check(peer).get(6L, TimeUnit.SECONDS);
633+
isServer = snapServerChecker.check(peer, peersHeadBlockHeader).get(6L, TimeUnit.SECONDS);
648634
} catch (Exception e) {
649635
LOG.atTrace()
650636
.setMessage("Error checking if peer {} is a snap server. Setting to false.")

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,7 @@ public DefaultSynchronizer(
106106

107107
if (syncConfig.getSyncMode() == SyncMode.SNAP
108108
|| syncConfig.getSyncMode() == SyncMode.CHECKPOINT) {
109-
SnapServerChecker.createAndSetSnapServerChecker(
110-
ethContext, metricsSystem, protocolContext.getBlockchain());
109+
SnapServerChecker.createAndSetSnapServerChecker(ethContext, metricsSystem);
111110
}
112111

113112
this.blockPropagationManager =

‎ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/SnapServerChecker.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package org.hyperledger.besu.ethereum.eth.sync;
1616

1717
import org.hyperledger.besu.datatypes.Hash;
18-
import org.hyperledger.besu.ethereum.chain.Blockchain;
1918
import org.hyperledger.besu.ethereum.core.BlockHeader;
2019
import org.hyperledger.besu.ethereum.eth.manager.EthContext;
2120
import org.hyperledger.besu.ethereum.eth.manager.EthPeer;
@@ -35,29 +34,25 @@ public class SnapServerChecker {
3534

3635
private final EthContext ethContext;
3736
private final MetricsSystem metricsSystem;
38-
private final Blockchain blockchain;
3937

40-
private SnapServerChecker(
41-
final EthContext ethContext, final MetricsSystem metricsSystem, final Blockchain blockchain) {
38+
public SnapServerChecker(final EthContext ethContext, final MetricsSystem metricsSystem) {
4239
this.ethContext = ethContext;
4340
this.metricsSystem = metricsSystem;
44-
this.blockchain = blockchain;
4541
}
4642

4743
public static void createAndSetSnapServerChecker(
48-
final EthContext ethContext, final MetricsSystem metricsSystem, final Blockchain blockchain) {
49-
final SnapServerChecker checker = new SnapServerChecker(ethContext, metricsSystem, blockchain);
44+
final EthContext ethContext, final MetricsSystem metricsSystem) {
45+
final SnapServerChecker checker = new SnapServerChecker(ethContext, metricsSystem);
5046
ethContext.getEthPeers().setSnapServerChecker(checker);
5147
}
5248

53-
public CompletableFuture<Boolean> check(final EthPeer peer) {
49+
public CompletableFuture<Boolean> check(final EthPeer peer, final BlockHeader peersHeadHeader) {
5450
LOG.atTrace()
5551
.setMessage("Checking whether peer {} is a snap server ...")
5652
.addArgument(peer::getLoggableId)
5753
.log();
5854
final CompletableFuture<AbstractPeerTask.PeerTaskResult<AccountRangeMessage.AccountRangeData>>
59-
snapServerCheckCompletableFuture =
60-
getAccountRangeFromPeer(peer, blockchain.getGenesisBlockHeader());
55+
snapServerCheckCompletableFuture = getAccountRangeFromPeer(peer, peersHeadHeader);
6156
final CompletableFuture<Boolean> future = new CompletableFuture<>();
6257
snapServerCheckCompletableFuture.whenComplete(
6358
(peerResult, error) -> {

0 commit comments

Comments
 (0)