Skip to content

Commit

Permalink
reintroduce checking of block height for certain tasks when we are no…
Browse files Browse the repository at this point in the history
…t PoS (Revert PR#3911) (hyperledger#5083)

* reintroduce checking of block height for certain tasks when we are not PoS (Revert PR#3911)

Signed-off-by: Stefan <[email protected]>
  • Loading branch information
pinges authored and eum602 committed Nov 3, 2023
1 parent 7479cfa commit ce0ce5d
Show file tree
Hide file tree
Showing 44 changed files with 244 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolFactory;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.p2p.config.SubProtocolConfiguration;
import org.hyperledger.besu.ethereum.storage.StorageProvider;
import org.hyperledger.besu.ethereum.storage.keyvalue.KeyValueSegmentIdentifier;
Expand Down Expand Up @@ -548,9 +549,12 @@ public BesuController build() {
}
}
final int maxMessageSize = ethereumWireProtocolConfiguration.getMaxMessageSize();
final Supplier<ProtocolSpec> currentProtocolSpecSupplier =
() -> protocolSchedule.getByBlockHeader(blockchain.getChainHeadHeader());
final EthPeers ethPeers =
new EthPeers(
getSupportedProtocol(),
currentProtocolSpecSupplier,
clock,
metricsSystem,
maxPeers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ public void respondToEth65GetHeadersUsingIstanbul99()
EthPeers peers =
new EthPeers(
Istanbul99Protocol.NAME,
() -> protocolSchedule.getByBlockHeader(blockchain.getChainHeadHeader()),
TestClock.fixed(),
new NoOpMetricsSystem(),
25,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ public class EthGetTransactionReceiptTest {
null,
Optional.of(PoWHasher.ETHASH_LIGHT),
null,
Optional.empty());
Optional.empty(),
true);
private final ProtocolSpec statusTransactionTypeSpec =
new ProtocolSpec(
"status",
Expand All @@ -140,7 +141,8 @@ public class EthGetTransactionReceiptTest {
null,
Optional.of(PoWHasher.ETHASH_LIGHT),
null,
Optional.empty());
Optional.empty(),
true);

@SuppressWarnings("unchecked")
private final ProtocolSchedule protocolSchedule = mock(ProtocolSchedule.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,8 @@ static ProtocolSpecBuilder parisDefinition(
.difficultyCalculator(MainnetDifficultyCalculators.PROOF_OF_STAKE_DIFFICULTY)
.blockHeaderValidatorBuilder(MainnetBlockHeaderValidator::mergeBlockHeaderValidator)
.blockReward(Wei.ZERO)
.name("ParisFork");
.name("ParisFork")
.isPoS(true);
}

static ProtocolSpecBuilder shanghaiDefinition(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public class ProtocolSpec {

private final Optional<WithdrawalsProcessor> withdrawalsProcessor;

private final boolean isPoS;
/**
* Creates a new protocol specification instance.
*
Expand Down Expand Up @@ -108,6 +109,7 @@ public class ProtocolSpec {
* @param powHasher the proof-of-work hasher
* @param withdrawalsValidator the withdrawals validator to use
* @param withdrawalsProcessor the Withdrawals processor to use
* @param isPoS indicates whether the current spec is PoS
*/
public ProtocolSpec(
final String name,
Expand All @@ -134,7 +136,8 @@ public ProtocolSpec(
final BadBlockManager badBlockManager,
final Optional<PoWHasher> powHasher,
final WithdrawalsValidator withdrawalsValidator,
final Optional<WithdrawalsProcessor> withdrawalsProcessor) {
final Optional<WithdrawalsProcessor> withdrawalsProcessor,
final boolean isPoS) {
this.name = name;
this.evm = evm;
this.transactionValidator = transactionValidator;
Expand All @@ -160,6 +163,7 @@ public ProtocolSpec(
this.powHasher = powHasher;
this.withdrawalsValidator = withdrawalsValidator;
this.withdrawalsProcessor = withdrawalsProcessor;
this.isPoS = isPoS;
}

/**
Expand Down Expand Up @@ -367,4 +371,13 @@ public WithdrawalsValidator getWithdrawalsValidator() {
public Optional<WithdrawalsProcessor> getWithdrawalsProcessor() {
return withdrawalsProcessor;
}

/**
* Returns true if the network is running Proof of Stake
*
* @return true if the network is running Proof of Stake
*/
public boolean isPoS() {
return isPoS;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public class ProtocolSpecBuilder {
private FeeMarket feeMarket = FeeMarket.legacy();
private BadBlockManager badBlockManager;
private PoWHasher powHasher = PoWHasher.ETHASH_LIGHT;
private boolean isPoS = false;

public ProtocolSpecBuilder gasCalculator(final Supplier<GasCalculator> gasCalculatorBuilder) {
this.gasCalculatorBuilder = gasCalculatorBuilder;
Expand Down Expand Up @@ -257,6 +258,11 @@ public ProtocolSpecBuilder withdrawalsProcessor(final WithdrawalsProcessor withd
return this;
}

public ProtocolSpecBuilder isPoS(final boolean isPoS) {
this.isPoS = isPoS;
return this;
}

public ProtocolSpec build(final HeaderBasedProtocolSchedule protocolSchedule) {
checkNotNull(gasCalculatorBuilder, "Missing gasCalculator");
checkNotNull(gasLimitCalculator, "Missing gasLimitCalculator");
Expand Down Expand Up @@ -363,7 +369,8 @@ public ProtocolSpec build(final HeaderBasedProtocolSchedule protocolSchedule) {
badBlockManager,
Optional.ofNullable(powHasher),
withdrawalsValidator,
Optional.ofNullable(withdrawalsProcessor));
Optional.ofNullable(withdrawalsProcessor),
isPoS);
}

private PrivateTransactionProcessor createPrivateTransactionProcessor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.hyperledger.besu.ethereum.eth.manager.EthPeer.DisconnectCallback;
import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection;
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage;
import org.hyperledger.besu.metrics.BesuMetricCategory;
Expand All @@ -36,6 +37,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -67,26 +69,37 @@ public class EthPeers {
private final Subscribers<ConnectCallback> connectCallbacks = Subscribers.create();
private final Subscribers<DisconnectCallback> disconnectCallbacks = Subscribers.create();
private final Collection<PendingPeerRequest> pendingRequests = new CopyOnWriteArrayList<>();
private final Supplier<ProtocolSpec> currentProtocolSpecSupplier;

private Comparator<EthPeer> bestPeerComparator;

public EthPeers(
final String protocolName,
final Supplier<ProtocolSpec> currentProtocolSpecSupplier,
final Clock clock,
final MetricsSystem metricsSystem,
final int maxPeers,
final int maxMessageSize) {
this(protocolName, clock, metricsSystem, maxPeers, maxMessageSize, Collections.emptyList());
this(
protocolName,
currentProtocolSpecSupplier,
clock,
metricsSystem,
maxPeers,
maxMessageSize,
Collections.emptyList());
}

public EthPeers(
final String protocolName,
final Supplier<ProtocolSpec> currentProtocolSpecSupplier,
final Clock clock,
final MetricsSystem metricsSystem,
final int maxPeers,
final int maxMessageSize,
final List<NodeMessagePermissioningProvider> permissioningProviders) {
this.protocolName = protocolName;
this.currentProtocolSpecSupplier = currentProtocolSpecSupplier;
this.clock = clock;
this.permissioningProviders = permissioningProviders;
this.maxPeers = maxPeers;
Expand Down Expand Up @@ -148,8 +161,16 @@ public EthPeer peer(final PeerConnection peerConnection) {

public PendingPeerRequest executePeerRequest(
final PeerRequest request, final long minimumBlockNumber, final Optional<EthPeer> peer) {
final long actualMinBlockNumber;
if (minimumBlockNumber > 0 && currentProtocolSpecSupplier.get().isPoS()) {
// if on PoS do not enforce a min block number, since the estimated chain height of the remote
// peer is not updated anymore.
actualMinBlockNumber = 0;
} else {
actualMinBlockNumber = minimumBlockNumber;
}
final PendingPeerRequest pendingPeerRequest =
new PendingPeerRequest(this, request, minimumBlockNumber, peer);
new PendingPeerRequest(this, request, actualMinBlockNumber, peer);
synchronized (this) {
if (!pendingPeerRequest.attemptExecution()) {
pendingRequests.add(pendingPeerRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ protected final void executeTask() {
});
}

public PendingPeerRequest sendRequestToPeer(final PeerRequest request) {
return sendRequestToPeer(request, 0L);
}

public PendingPeerRequest sendRequestToPeer(
final PeerRequest request, final long minimumBlockNumber) {
return ethContext.getEthPeers().executePeerRequest(request, minimumBlockNumber, assignedPeer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ private CompletableFuture<PeerTaskResult<List<BlockHeader>>> downloadHeader() {
hash.map(
value ->
GetHeadersFromPeerByHashTask.forSingleHash(
protocolSchedule, ethContext, value, metricsSystem))
protocolSchedule, ethContext, value, blockNumber, metricsSystem))
.orElseGet(
() ->
GetHeadersFromPeerByNumberTask.forSingleNumber(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,14 @@ public static GetBodiesFromPeerTask forHeaders(
protected PendingPeerRequest sendRequest() {
final List<Hash> blockHashes =
headers.stream().map(BlockHeader::getHash).collect(Collectors.toList());
final long minimumRequiredBlockNumber = headers.get(headers.size() - 1).getNumber();

return sendRequestToPeer(
peer -> {
LOG.debug("Requesting {} bodies from peer {}.", blockHashes.size(), peer);
return peer.getBodies(blockHashes);
});
},
minimumRequiredBlockNumber);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,20 @@ public class GetHeadersFromPeerByHashTask extends AbstractGetHeadersFromPeerTask
private static final Logger LOG = LoggerFactory.getLogger(GetHeadersFromPeerByHashTask.class);

private final Hash referenceHash;
private final long minimumRequiredBlockNumber;

@VisibleForTesting
GetHeadersFromPeerByHashTask(
final ProtocolSchedule protocolSchedule,
final EthContext ethContext,
final Hash referenceHash,
final long minimumRequiredBlockNumber,
final int count,
final int skip,
final boolean reverse,
final MetricsSystem metricsSystem) {
super(protocolSchedule, ethContext, count, skip, reverse, metricsSystem);
this.minimumRequiredBlockNumber = minimumRequiredBlockNumber;
checkNotNull(referenceHash);
this.referenceHash = referenceHash;
}
Expand All @@ -51,40 +54,65 @@ public static AbstractGetHeadersFromPeerTask startingAtHash(
final ProtocolSchedule protocolSchedule,
final EthContext ethContext,
final Hash firstHash,
final long firstBlockNumber,
final int segmentLength,
final MetricsSystem metricsSystem) {
return new GetHeadersFromPeerByHashTask(
protocolSchedule, ethContext, firstHash, segmentLength, 0, false, metricsSystem);
protocolSchedule,
ethContext,
firstHash,
firstBlockNumber,
segmentLength,
0,
false,
metricsSystem);
}

public static AbstractGetHeadersFromPeerTask startingAtHash(
final ProtocolSchedule protocolSchedule,
final EthContext ethContext,
final Hash firstHash,
final long firstBlockNumber,
final int segmentLength,
final int skip,
final MetricsSystem metricsSystem) {
return new GetHeadersFromPeerByHashTask(
protocolSchedule, ethContext, firstHash, segmentLength, skip, false, metricsSystem);
protocolSchedule,
ethContext,
firstHash,
firstBlockNumber,
segmentLength,
skip,
false,
metricsSystem);
}

public static AbstractGetHeadersFromPeerTask endingAtHash(
final ProtocolSchedule protocolSchedule,
final EthContext ethContext,
final Hash lastHash,
final long lastBlockNumber,
final int segmentLength,
final MetricsSystem metricsSystem) {
return new GetHeadersFromPeerByHashTask(
protocolSchedule, ethContext, lastHash, segmentLength, 0, true, metricsSystem);
protocolSchedule,
ethContext,
lastHash,
lastBlockNumber,
segmentLength,
0,
true,
metricsSystem);
}

public static AbstractGetHeadersFromPeerTask forSingleHash(
final ProtocolSchedule protocolSchedule,
final EthContext ethContext,
final Hash hash,
final long minimumRequiredBlockNumber,
final MetricsSystem metricsSystem) {
return new GetHeadersFromPeerByHashTask(
protocolSchedule, ethContext, hash, 1, 0, false, metricsSystem);
protocolSchedule, ethContext, hash, minimumRequiredBlockNumber, 1, 0, false, metricsSystem);
}

@Override
Expand All @@ -97,7 +125,8 @@ protected PendingPeerRequest sendRequest() {
referenceHash.slice(0, 6),
peer);
return peer.getHeadersByHash(referenceHash, count, skip, reverse);
});
},
minimumRequiredBlockNumber);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ protected PendingPeerRequest sendRequest() {
LOG.debug(
"Requesting {} headers (blockNumber {}) from peer {}.", count, blockNumber, peer);
return peer.getHeadersByNumber(blockNumber, count, skip, reverse);
});
},
blockNumber);
}

@Override
Expand Down
Loading

0 comments on commit ce0ce5d

Please sign in to comment.