From 7cd7178e8f8da74d8dd44bd6a51322bd2451de00 Mon Sep 17 00:00:00 2001 From: Matthew Whitehead Date: Wed, 10 Jan 2024 08:44:58 +0000 Subject: [PATCH] Refactored version of QBFT/shanghai Signed-off-by: Matthew Whitehead --- .../bft/BaseBftProtocolScheduleBuilder.java | 4 +++ .../common/bft/BftProtocolSchedule.java | 29 +++++++++++++++++++ .../statemachine/QbftBlockHeightManager.java | 9 ++++-- .../qbft/statemachine/QbftRoundFactory.java | 22 ++++++++++---- .../FutureRoundProposalMessageValidator.java | 3 +- .../validation/MessageValidatorFactory.java | 9 ++++-- .../QbftBlockHeightManagerTest.java | 17 ++++++----- 7 files changed, 74 insertions(+), 19 deletions(-) diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java index c17652f4a43..7b93fd44a45 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java @@ -28,6 +28,7 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolScheduleBuilder; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecAdapters; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecBuilder; +import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator; import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket; import org.hyperledger.besu.evm.internal.EvmConfiguration; @@ -116,6 +117,9 @@ private ProtocolSpecBuilder applyBftChanges( .skipZeroBlockRewards(true) .blockHeaderFunctions(BftBlockHeaderFunctions.forOnchainBlock(bftExtraDataCodec)) .blockReward(Wei.of(configOptions.getBlockRewardWei())) + .withdrawalsValidator( + new WithdrawalsValidator + .ProhibitedWithdrawals()) // QBFT/IBFT doesn't support withdrawals .miningBeneficiaryCalculator( header -> configOptions.getMiningBeneficiary().orElseGet(header::getCoinbase)); } diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftProtocolSchedule.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftProtocolSchedule.java index eca88b95841..5a50764eb7b 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftProtocolSchedule.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftProtocolSchedule.java @@ -61,6 +61,35 @@ public ProtocolSpec getByBlockNumber(final long number) { return null; } + /** + * Look up ProtocolSpec by block number and timestamp + * + * @param number block number + * @param timestamp block timestamp + * @return the protocol spec for that block number and timestamp + */ + public ProtocolSpec getByBlockNumberAndTimestamp(final long number, final long timestamp) { + checkArgument(number >= 0, "number must be non-negative"); + checkArgument( + !protocolSpecs.isEmpty(), "At least 1 milestone must be provided to the protocol schedule"); + checkArgument( + protocolSpecs.last().fork().milestone() == 0, + "There must be a milestone starting from block 0"); + // protocolSpecs is sorted in descending block order, so the first one we find that's lower than + // the requested level will be the most appropriate spec + ProtocolSpec theSpec = null; + for (final ScheduledProtocolSpec s : protocolSpecs) { + if (number >= s.fork().milestone()) { + theSpec = s.spec(); + break; + } else if (timestamp >= s.fork().milestone()) { + theSpec = s.spec(); + break; + } + } + return theSpec; + } + /** * return the ordered list of scheduled protocol specs * diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java index 45e97ed6795..6532d116a1e 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java @@ -104,7 +104,7 @@ public QbftBlockHeightManager( new RoundState( roundIdentifier, finalState.getQuorum(), - messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader)); + messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader, 0L)); final long nextBlockHeight = parentHeader.getNumber() + 1; final ConsensusRoundIdentifier roundIdentifier = @@ -280,15 +280,18 @@ public void handleRoundChangePayload(final RoundChange message) { private void startNewRound(final int roundNumber) { LOG.debug("Starting new round {}", roundNumber); + final long headerTimeStampSeconds = Math.round(clock.millis() / 1000D); // validate the current round if (futureRoundStateBuffer.containsKey(roundNumber)) { currentRound = Optional.of( roundFactory.createNewRoundWithState( - parentHeader, futureRoundStateBuffer.get(roundNumber))); + parentHeader, futureRoundStateBuffer.get(roundNumber), headerTimeStampSeconds)); futureRoundStateBuffer.keySet().removeIf(k -> k <= roundNumber); } else { - currentRound = Optional.of(roundFactory.createNewRound(parentHeader, roundNumber)); + currentRound = + Optional.of( + roundFactory.createNewRound(parentHeader, roundNumber, headerTimeStampSeconds)); } // discard roundChange messages from the current and previous rounds roundChangeManager.discardRoundsPriorTo(currentRound.get().getRoundIdentifier()); diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundFactory.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundFactory.java index 7f93a73d3b5..21efdbef098 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundFactory.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundFactory.java @@ -26,6 +26,7 @@ import org.hyperledger.besu.ethereum.blockcreation.BlockCreator; import org.hyperledger.besu.ethereum.chain.MinedBlockObserver; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.BlockImporter; import org.hyperledger.besu.util.Subscribers; /** The Qbft round factory. */ @@ -74,9 +75,12 @@ public QbftRoundFactory( * * @param parentHeader the parent header * @param round the round + * @param blockTimestamp the timestamp of the new round, and hence the timestamp of the protocol + * schedule to use to create the block * @return the qbft round */ - public QbftRound createNewRound(final BlockHeader parentHeader, final int round) { + public QbftRound createNewRound( + final BlockHeader parentHeader, final int round, final long blockTimestamp) { long nextBlockHeight = parentHeader.getNumber() + 1; final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(nextBlockHeight, round); @@ -85,9 +89,10 @@ public QbftRound createNewRound(final BlockHeader parentHeader, final int round) new RoundState( roundIdentifier, finalState.getQuorum(), - messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader)); + messageValidatorFactory.createMessageValidator( + roundIdentifier, parentHeader, blockTimestamp)); - return createNewRoundWithState(parentHeader, roundState); + return createNewRoundWithState(parentHeader, roundState, blockTimestamp); } /** @@ -95,10 +100,12 @@ public QbftRound createNewRound(final BlockHeader parentHeader, final int round) * * @param parentHeader the parent header * @param roundState the round state + * @param blockTimestamp the timestamp of the new round, and hence the timestamp of the protocol + * schedule to use to create the block * @return the qbft round */ public QbftRound createNewRoundWithState( - final BlockHeader parentHeader, final RoundState roundState) { + final BlockHeader parentHeader, final RoundState roundState, final long blockTimestamp) { final ConsensusRoundIdentifier roundIdentifier = roundState.getRoundIdentifier(); final BlockCreator blockCreator = blockCreatorFactory.create(parentHeader, 0); @@ -106,11 +113,16 @@ public QbftRound createNewRoundWithState( final QbftMessageTransmitter messageTransmitter = new QbftMessageTransmitter(messageFactory, finalState.getValidatorMulticaster()); + final BlockImporter theImporter = + protocolSchedule + .getByBlockNumberAndTimestamp(roundIdentifier.getSequenceNumber(), blockTimestamp) + .getBlockImporter(); + return new QbftRound( roundState, blockCreator, protocolContext, - protocolSchedule.getByBlockNumber(roundIdentifier.getSequenceNumber()).getBlockImporter(), + theImporter, minedBlockObservers, finalState.getNodeKey(), messageFactory, diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/FutureRoundProposalMessageValidator.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/FutureRoundProposalMessageValidator.java index 3c401bb203c..108482651c9 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/FutureRoundProposalMessageValidator.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/FutureRoundProposalMessageValidator.java @@ -52,7 +52,8 @@ public boolean validateProposalMessage(final Proposal msg) { new ConsensusRoundIdentifier(chainHeight, msg.getRoundIdentifier().getRoundNumber()); final MessageValidator messageValidator = - messageValidatorFactory.createMessageValidator(roundIdentifier, parentHeader); + messageValidatorFactory.createMessageValidator( + roundIdentifier, parentHeader, msg.getBlock().getHeader().getTimestamp()); return messageValidator.validateProposal(msg); } diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java index 2875d36ab3c..13bd1c83cab 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java @@ -95,14 +95,19 @@ public RoundChangeMessageValidator createRoundChangeMessageValidator( * * @param roundIdentifier the round identifier * @param parentHeader the parent header + * @param headerTimestamp the timestamp of the header * @return the message validator */ public MessageValidator createMessageValidator( - final ConsensusRoundIdentifier roundIdentifier, final BlockHeader parentHeader) { + final ConsensusRoundIdentifier roundIdentifier, + final BlockHeader parentHeader, + final long headerTimestamp) { final Collection
validatorsForHeight = getValidatorsAfterBlock(parentHeader); final BlockValidator blockValidator = - protocolSchedule.getByBlockNumber(roundIdentifier.getSequenceNumber()).getBlockValidator(); + protocolSchedule + .getByBlockNumberAndTimestamp(roundIdentifier.getSequenceNumber(), headerTimestamp) + .getBlockValidator(); final ProposalValidator proposalValidator = new ProposalValidator( diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java index 92471799cfd..43902017c0e 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java @@ -152,7 +152,8 @@ public void setup() { when(futureRoundProposalMessageValidator.validateProposalMessage(any())).thenReturn(true); when(messageValidatorFactory.createFutureRoundProposalMessageValidator(anyLong(), any())) .thenReturn(futureRoundProposalMessageValidator); - when(messageValidatorFactory.createMessageValidator(any(), any())).thenReturn(messageValidator); + when(messageValidatorFactory.createMessageValidator(any(), any(), anyLong())) + .thenReturn(messageValidator); when(blockImporter.importBlock(any(), any(), any())).thenReturn(new BlockImportResult(false)); protocolContext = @@ -164,7 +165,7 @@ QbftContext.class, validators, new QbftExtraDataCodec()), Optional.empty()); // Ensure the created QbftRound has the valid ConsensusRoundIdentifier; - when(roundFactory.createNewRound(any(), anyInt())) + when(roundFactory.createNewRound(any(), anyInt(), anyLong())) .thenAnswer( invocation -> { final int round = invocation.getArgument(1); @@ -183,7 +184,7 @@ QbftContext.class, validators, new QbftExtraDataCodec()), bftExtraDataCodec); }); - when(roundFactory.createNewRoundWithState(any(), any())) + when(roundFactory.createNewRoundWithState(any(), any(), anyLong())) .thenAnswer( invocation -> { final RoundState providedRoundState = invocation.getArgument(1); @@ -323,13 +324,13 @@ public void onRoundChangeReceptionRoundChangeManagerIsInvokedAndNewRoundStarted( messageValidatorFactory, messageFactory); manager.handleBlockTimerExpiry(roundIdentifier); - verify(roundFactory).createNewRound(any(), eq(0)); + verify(roundFactory).createNewRound(any(), eq(0), anyLong()); manager.handleRoundChangePayload(roundChange); verify(roundChangeManager, times(1)).appendRoundChangeMessage(roundChange); verify(roundFactory, times(1)) - .createNewRound(any(), eq(futureRoundIdentifier.getRoundNumber())); + .createNewRound(any(), eq(futureRoundIdentifier.getRoundNumber()), anyLong()); } @Test @@ -344,10 +345,10 @@ public void onRoundTimerExpiryANewRoundIsCreatedWithAnIncrementedRoundNumber() { messageValidatorFactory, messageFactory); manager.handleBlockTimerExpiry(roundIdentifier); - verify(roundFactory).createNewRound(any(), eq(0)); + verify(roundFactory).createNewRound(any(), eq(0), anyLong()); manager.roundExpired(new RoundExpiry(roundIdentifier)); - verify(roundFactory).createNewRound(any(), eq(1)); + verify(roundFactory).createNewRound(any(), eq(1), anyLong()); } @Test @@ -542,6 +543,6 @@ public void illegalFutureRoundProposalDoesNotTriggerNewRound() { reset(roundFactory); // Discard the existing createNewRound invocation. manager.handleProposalPayload(futureRoundProposal); - verify(roundFactory, never()).createNewRound(any(), anyInt()); + verify(roundFactory, never()).createNewRound(any(), anyInt(), anyLong()); } }