diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 21cb787f52b..47ced1f5f5f 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -69,6 +69,7 @@ import org.hyperledger.besu.cli.options.TransactionPoolOptions; import org.hyperledger.besu.cli.options.storage.DataStorageOptions; import org.hyperledger.besu.cli.options.storage.DiffBasedSubStorageOptions; +import org.hyperledger.besu.cli.options.unstable.QBFTOptions; import org.hyperledger.besu.cli.presynctasks.PreSynchronizationTaskRunner; import org.hyperledger.besu.cli.presynctasks.PrivateDatabaseMigrationPreSyncTask; import org.hyperledger.besu.cli.subcommands.PasswordSubCommand; @@ -301,6 +302,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { private final EvmOptions unstableEvmOptions = EvmOptions.create(); private final IpcOptions unstableIpcOptions = IpcOptions.create(); private final ChainPruningOptions unstableChainPruningOptions = ChainPruningOptions.create(); + private final QBFTOptions unstableQbftOptions = QBFTOptions.create(); // stable CLI options final DataStorageOptions dataStorageOptions = DataStorageOptions.create(); @@ -1162,6 +1164,7 @@ private void handleUnstableOptions() { .put("EVM Options", unstableEvmOptions) .put("IPC Options", unstableIpcOptions) .put("Chain Data Pruning Options", unstableChainPruningOptions) + .put("QBFT Options", unstableQbftOptions) .build(); UnstableOptionsSubCommand.createUnstableOptions(commandLine, unstableOptions); @@ -1787,6 +1790,7 @@ public BesuControllerBuilder setupControllerBuilder() { .clock(Clock.systemUTC()) .isRevertReasonEnabled(isRevertReasonEnabled) .storageProvider(storageProvider) + .isEarlyRoundChangeEnabled(unstableQbftOptions.isEarlyRoundChangeEnabled()) .gasLimitCalculator( miningParametersSupplier.get().getTargetGasLimit().isPresent() ? new FrontierTargetingGasLimitCalculator() diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/QBFTOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/QBFTOptions.java new file mode 100644 index 00000000000..3852b8225ef --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/QBFTOptions.java @@ -0,0 +1,35 @@ +/* + * Copyright contributors to Besu. + * + * 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.cli.options.unstable; + +import picocli.CommandLine; + +public class QBFTOptions { + + public static QBFTOptions create() { + return new QBFTOptions(); + } + + @CommandLine.Option( + names = {"--Xqbft-enable-early-round-change"}, + description = + "Enable early round change upon receiving f+1 valid future Round Change messages from different validators (experimental)", + hidden = true) + private boolean enableEarlyRoundChange = false; + + public boolean isEarlyRoundChangeEnabled() { + return enableEarlyRoundChange; + } +} 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 258cf95ca35..b5df0235e34 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -219,6 +219,8 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides /** The transaction simulator */ protected TransactionSimulator transactionSimulator; + protected boolean isEarlyRoundChangeEnabled = false; + /** Instantiates a new Besu controller builder. */ protected BesuControllerBuilder() {} @@ -553,6 +555,11 @@ public BesuControllerBuilder isParallelTxProcessingEnabled( return this; } + public BesuControllerBuilder isEarlyRoundChangeEnabled(final boolean isEarlyRoundChangeEnabled) { + this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled; + return this; + } + /** * Build besu controller. * diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java index e8c08d8a473..a86f6a1566e 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java @@ -245,23 +245,28 @@ protected MiningCoordinator createMiningCoordinator( final MessageFactory messageFactory = new MessageFactory(nodeKey); - final BftEventHandler qbftController = - new QbftController( - blockchain, + QbftBlockHeightManagerFactory qbftBlockHeightManagerFactory = + new QbftBlockHeightManagerFactory( finalState, - new QbftBlockHeightManagerFactory( + new QbftRoundFactory( finalState, - new QbftRoundFactory( - finalState, - protocolContext, - bftProtocolSchedule, - minedBlockObservers, - messageValidatorFactory, - messageFactory, - bftExtraDataCodec().get()), + protocolContext, + bftProtocolSchedule, + minedBlockObservers, messageValidatorFactory, messageFactory, - new ValidatorModeTransitionLogger(qbftForksSchedule)), + bftExtraDataCodec().get()), + messageValidatorFactory, + messageFactory, + new ValidatorModeTransitionLogger(qbftForksSchedule)); + + qbftBlockHeightManagerFactory.isEarlyRoundChangeEnabled(isEarlyRoundChangeEnabled); + + final BftEventHandler qbftController = + new QbftController( + blockchain, + finalState, + qbftBlockHeightManagerFactory, gossiper, duplicateMessageTracker, futureMessageBuffer, diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 78f782d5e55..ac1de0b5252 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -279,6 +279,7 @@ public void initMocks() throws Exception { when(mockControllerBuilder.isRevertReasonEnabled(false)).thenReturn(mockControllerBuilder); when(mockControllerBuilder.isParallelTxProcessingEnabled(false)) .thenReturn(mockControllerBuilder); + when(mockControllerBuilder.isEarlyRoundChangeEnabled(false)).thenReturn(mockControllerBuilder); when(mockControllerBuilder.storageProvider(any())).thenReturn(mockControllerBuilder); when(mockControllerBuilder.gasLimitCalculator(any())).thenReturn(mockControllerBuilder); when(mockControllerBuilder.requiredBlocks(any())).thenReturn(mockControllerBuilder); diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java index 318701da7cd..6c52dd71914 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java @@ -43,6 +43,16 @@ public static int calculateRequiredValidatorQuorum(final int validatorCount) { return Util.fastDivCeiling(2 * validatorCount, 3); } + /** + * Calculate required future RC messages count quorum for a round change. + * + * @param validatorCount the validator count + * @return Required number of future round change messages to reach quorum for a round change. + */ + public static int calculateRequiredFutureRCQuorum(final int validatorCount) { + return (validatorCount - 1) / 3 + 1; + } + /** * Prepare message count for quorum. * diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/RoundTimer.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/RoundTimer.java index 0302943fc12..78cfedfa98b 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/RoundTimer.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/RoundTimer.java @@ -83,8 +83,7 @@ public synchronized void startTimer(final ConsensusRoundIdentifier round) { // Once we are up to round 2 start logging round expiries if (round.getRoundNumber() >= 2) { LOG.info( - "BFT round {} expired. Moved to round {} which will expire in {} seconds", - round.getRoundNumber() - 1, + "Moved to round {} which will expire in {} seconds", round.getRoundNumber(), (expiryTime / 1000)); } diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BftHelpersTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BftHelpersTest.java index 67e7cc283b9..7106d9f5dcb 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BftHelpersTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BftHelpersTest.java @@ -63,4 +63,39 @@ public void calculateRequiredValidatorQuorum15Validator() { public void calculateRequiredValidatorQuorum20Validator() { Assertions.assertThat(BftHelpers.calculateRequiredValidatorQuorum(20)).isEqualTo(14); } + + @Test + public void calculateRequiredFutureRCQuorum4Validator() { + Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(4)).isEqualTo(2); + } + + @Test + public void calculateRequiredFutureRCQuorum6Validator() { + Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(6)).isEqualTo(2); + } + + @Test + public void calculateRequiredFutureRCQuorum7Validator() { + Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(7)).isEqualTo(3); + } + + @Test + public void calculateRequiredFutureRCQuorum9Validator() { + Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(9)).isEqualTo(3); + } + + @Test + public void calculateRequiredFutureRCQuorum10Validator() { + Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(10)).isEqualTo(4); + } + + @Test + public void calculateRequiredFutureRCQuorum13Validator() { + Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(13)).isEqualTo(5); + } + + @Test + public void calculateRequiredFutureRCQuorum15Validator() { + Assertions.assertThat(BftHelpers.calculateRequiredFutureRCQuorum(15)).isEqualTo(5); + } } 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 757998b5ca6..0064902e8c3 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 @@ -68,6 +68,7 @@ public class QbftBlockHeightManager implements BaseQbftBlockHeightManager { private Optional latestPreparedCertificate = Optional.empty(); private Optional currentRound = Optional.empty(); + private boolean isEarlyRoundChangeEnabled = false; /** * Instantiates a new Qbft block height manager. @@ -115,6 +116,31 @@ public QbftBlockHeightManager( finalState.getBlockTimer().startTimer(roundIdentifier, parentHeader); } + /** + * Secondary constructor with early round change option + * + * @param isEarlyRoundChangeEnabled enable round change when f+1 RC messages are received + */ + public QbftBlockHeightManager( + final BlockHeader parentHeader, + final BftFinalState finalState, + final RoundChangeManager roundChangeManager, + final QbftRoundFactory qbftRoundFactory, + final Clock clock, + final MessageValidatorFactory messageValidatorFactory, + final MessageFactory messageFactory, + final boolean isEarlyRoundChangeEnabled) { + this( + parentHeader, + finalState, + roundChangeManager, + qbftRoundFactory, + clock, + messageValidatorFactory, + messageFactory); + this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled; + } + @Override public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifier) { if (currentRound.isPresent()) { @@ -227,23 +253,36 @@ public void roundExpired(final RoundExpiry expire) { return; } + doRoundChange(qbftRound.getRoundIdentifier().getRoundNumber() + 1); + } + + private synchronized void doRoundChange(final int newRoundNumber) { + + if (currentRound.isPresent() + && currentRound.get().getRoundIdentifier().getRoundNumber() >= newRoundNumber) { + return; + } LOG.debug( - "Round has expired, creating PreparedCertificate and notifying peers. round={}", - qbftRound.getRoundIdentifier()); + "Round has expired or changing based on RC quorum, creating PreparedCertificate and notifying peers. round={}", + currentRound.get().getRoundIdentifier()); final Optional preparedCertificate = - qbftRound.constructPreparedCertificate(); + currentRound.get().constructPreparedCertificate(); if (preparedCertificate.isPresent()) { latestPreparedCertificate = preparedCertificate; } - startNewRound(qbftRound.getRoundIdentifier().getRoundNumber() + 1); - qbftRound = currentRound.get(); + startNewRound(newRoundNumber); + if (currentRound.isEmpty()) { + LOG.info("Failed to start round "); + return; + } + QbftRound qbftRoundNew = currentRound.get(); try { final RoundChange localRoundChange = messageFactory.createRoundChange( - qbftRound.getRoundIdentifier(), latestPreparedCertificate); + qbftRoundNew.getRoundIdentifier(), latestPreparedCertificate); // Its possible the locally created RoundChange triggers the transmission of a NewRound // message - so it must be handled accordingly. @@ -252,7 +291,7 @@ public void roundExpired(final RoundExpiry expire) { LOG.warn("Failed to create signed RoundChange message.", e); } - transmitter.multicastRoundChange(qbftRound.getRoundIdentifier(), latestPreparedCertificate); + transmitter.multicastRoundChange(qbftRoundNew.getRoundIdentifier(), latestPreparedCertificate); } @Override @@ -333,24 +372,55 @@ public void handleRoundChangePayload(final RoundChange message) { final Optional> result = roundChangeManager.appendRoundChangeMessage(message); - if (result.isPresent()) { - LOG.debug( - "Received sufficient RoundChange messages to change round to targetRound={}", - targetRound); - if (messageAge == MessageAge.FUTURE_ROUND) { - startNewRound(targetRound.getRoundNumber()); - } + if (!isEarlyRoundChangeEnabled) { + if (result.isPresent()) { + LOG.debug( + "Received sufficient RoundChange messages to change round to targetRound={}", + targetRound); + if (messageAge == MessageAge.FUTURE_ROUND) { + startNewRound(targetRound.getRoundNumber()); + } - final RoundChangeArtifacts roundChangeMetadata = RoundChangeArtifacts.create(result.get()); + final RoundChangeArtifacts roundChangeMetadata = RoundChangeArtifacts.create(result.get()); - if (finalState.isLocalNodeProposerForRound(targetRound)) { - if (currentRound.isEmpty()) { - startNewRound(0); + if (finalState.isLocalNodeProposerForRound(targetRound)) { + if (currentRound.isEmpty()) { + startNewRound(0); + } + currentRound + .get() + .startRoundWith(roundChangeMetadata, TimeUnit.MILLISECONDS.toSeconds(clock.millis())); } + } + } else { + + if (currentRound.isEmpty()) { + startNewRound(0); + } + int currentRoundNumber = currentRound.get().getRoundIdentifier().getRoundNumber(); + // If this node is proposer for the current round, check if quorum is achieved for RC messages + // aiming this round + if (targetRound.getRoundNumber() == currentRoundNumber + && finalState.isLocalNodeProposerForRound(targetRound) + && result.isPresent()) { + + final RoundChangeArtifacts roundChangeMetadata = RoundChangeArtifacts.create(result.get()); + currentRound .get() .startRoundWith(roundChangeMetadata, TimeUnit.MILLISECONDS.toSeconds(clock.millis())); } + + // check if f+1 RC messages for future rounds are received + QbftRound qbftRound = currentRound.get(); + Optional nextHigherRound = + roundChangeManager.futureRCQuorumReceived(qbftRound.getRoundIdentifier()); + if (nextHigherRound.isPresent()) { + LOG.info( + "Received sufficient RoundChange messages to change round to targetRound={}", + nextHigherRound.get()); + doRoundChange(nextHigherRound.get()); + } } } diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerFactory.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerFactory.java index 889f83f98a7..b05fe8c45a3 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerFactory.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerFactory.java @@ -34,6 +34,7 @@ public class QbftBlockHeightManagerFactory { private final MessageValidatorFactory messageValidatorFactory; private final MessageFactory messageFactory; private final ValidatorModeTransitionLogger validatorModeTransitionLogger; + private boolean isEarlyRoundChangeEnabled = false; /** * Instantiates a new Qbft block height manager factory. @@ -75,22 +76,55 @@ public BaseQbftBlockHeightManager create(final BlockHeader parentHeader) { } } + public void isEarlyRoundChangeEnabled(final boolean isEarlyRoundChangeEnabled) { + this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled; + } + private BaseQbftBlockHeightManager createNoOpBlockHeightManager(final BlockHeader parentHeader) { return new NoOpBlockHeightManager(parentHeader); } private BaseQbftBlockHeightManager createFullBlockHeightManager(final BlockHeader parentHeader) { - return new QbftBlockHeightManager( - parentHeader, - finalState, - new RoundChangeManager( - BftHelpers.calculateRequiredValidatorQuorum(finalState.getValidators().size()), - messageValidatorFactory.createRoundChangeMessageValidator( - parentHeader.getNumber() + 1L, parentHeader), - finalState.getLocalAddress()), - roundFactory, - finalState.getClock(), - messageValidatorFactory, - messageFactory); + + QbftBlockHeightManager qbftBlockHeightManager; + RoundChangeManager roundChangeManager; + + if (isEarlyRoundChangeEnabled) { + roundChangeManager = + new RoundChangeManager( + BftHelpers.calculateRequiredValidatorQuorum(finalState.getValidators().size()), + BftHelpers.calculateRequiredFutureRCQuorum(finalState.getValidators().size()), + messageValidatorFactory.createRoundChangeMessageValidator( + parentHeader.getNumber() + 1L, parentHeader), + finalState.getLocalAddress()); + qbftBlockHeightManager = + new QbftBlockHeightManager( + parentHeader, + finalState, + roundChangeManager, + roundFactory, + finalState.getClock(), + messageValidatorFactory, + messageFactory, + true); + } else { + roundChangeManager = + new RoundChangeManager( + BftHelpers.calculateRequiredValidatorQuorum(finalState.getValidators().size()), + messageValidatorFactory.createRoundChangeMessageValidator( + parentHeader.getNumber() + 1L, parentHeader), + finalState.getLocalAddress()); + qbftBlockHeightManager = + new QbftBlockHeightManager( + parentHeader, + finalState, + roundChangeManager, + roundFactory, + finalState.getClock(), + messageValidatorFactory, + messageFactory); + } + + return qbftBlockHeightManager; } } diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeManager.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeManager.java index 89e2888395c..c260219a6f0 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeManager.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeManager.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; @@ -75,7 +76,7 @@ public void addMessage(final RoundChange msg) { * * @return the boolean */ - public boolean roundChangeReady() { + public boolean roundChangeQuorumReceived() { return receivedMessages.size() >= quorum && !actioned; } @@ -85,7 +86,7 @@ public boolean roundChangeReady() { * @return the collection */ public Collection createRoundChangeCertificate() { - if (roundChangeReady()) { + if (roundChangeQuorumReceived()) { actioned = true; return receivedMessages.values(); } else { @@ -104,6 +105,7 @@ public Collection createRoundChangeCertificate() { private final Map roundSummary = Maps.newHashMap(); private final long quorum; + private long rcQuorum; private final RoundChangeMessageValidator roundChangeMessageValidator; private final Address localAddress; @@ -123,6 +125,23 @@ public RoundChangeManager( this.localAddress = localAddress; } + /** + * Instantiates a new Round change manager. + * + * @param quorum the quorum + * @param rcQuorum quorum for round change messages + * @param roundChangeMessageValidator the round change message validator + * @param localAddress this node's address + */ + public RoundChangeManager( + final long quorum, + final long rcQuorum, + final RoundChangeMessageValidator roundChangeMessageValidator, + final Address localAddress) { + this(quorum, roundChangeMessageValidator, localAddress); + this.rcQuorum = rcQuorum; + } + /** * Store the latest round for a node, and if chain is stalled log a summary of which round each * address is on @@ -130,6 +149,10 @@ public RoundChangeManager( * @param message the round-change message that has just been received */ public void storeAndLogRoundChangeSummary(final RoundChange message) { + if (!isMessageValid(message)) { + LOG.info("RoundChange message is invalid ."); + return; + } roundSummary.put(message.getAuthor(), message.getRoundIdentifier()); if (roundChangeCache.keySet().stream() .findFirst() @@ -147,6 +170,32 @@ public void storeAndLogRoundChangeSummary(final RoundChange message) { } } + public Optional futureRCQuorumReceived( + final ConsensusRoundIdentifier currentRoundIdentifier) { + // Iterate through elements of round summary, identify ones with round number higher than + // current, + // tracking minimum of those and return the next higher round number if quorum is reached + + // Filter out entries with round number greater than current round + // and collect their round numbers + Map higherRounds = + roundSummary.entrySet().stream() + .filter(entry -> isAFutureRound(entry.getValue(), currentRoundIdentifier)) + .collect( + Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getRoundNumber())); + + LOG.debug("Higher rounds size ={} rcquorum = {}", higherRounds.size(), rcQuorum); + + // Check if we have at least f + 1 validators at higher rounds + if (higherRounds.size() >= rcQuorum) { + // Find the minimum round that is greater than the current round + return Optional.of(higherRounds.values().stream().min(Integer::compareTo).orElseThrow()); + } + + // If quorum is not reached, return empty Optional + return Optional.empty(); + } + /** * Adds the round message to this manager and return a certificate if it passes the threshold * @@ -163,7 +212,7 @@ public Optional> appendRoundChangeMessage(final RoundCha final RoundChangeStatus roundChangeStatus = storeRoundChangeMessage(msg); - if (roundChangeStatus.roundChangeReady()) { + if (roundChangeStatus.roundChangeQuorumReceived()) { return Optional.of(roundChangeStatus.createRoundChangeCertificate()); } @@ -198,4 +247,9 @@ private boolean isAnEarlierRound( final ConsensusRoundIdentifier left, final ConsensusRoundIdentifier right) { return left.getRoundNumber() < right.getRoundNumber(); } + + private boolean isAFutureRound( + final ConsensusRoundIdentifier left, final ConsensusRoundIdentifier right) { + return left.getRoundNumber() > right.getRoundNumber(); + } } 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 b6adcb73e62..317cfb98bc6 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 @@ -54,6 +54,7 @@ import org.hyperledger.besu.consensus.qbft.validation.FutureRoundProposalMessageValidator; import org.hyperledger.besu.consensus.qbft.validation.MessageValidator; import org.hyperledger.besu.consensus.qbft.validation.MessageValidatorFactory; +import org.hyperledger.besu.consensus.qbft.validation.RoundChangeMessageValidator; import org.hyperledger.besu.crypto.SignatureAlgorithmFactory; import org.hyperledger.besu.cryptoservices.NodeKey; import org.hyperledger.besu.cryptoservices.NodeKeyUtils; @@ -144,7 +145,7 @@ private void buildCreatedBlock() { @BeforeEach public void setup() { - for (int i = 0; i < 3; i++) { + for (int i = 0; i <= 3; i++) { final NodeKey nodeKey = NodeKeyUtils.generate(); validators.add(Util.publicKeyToAddress(nodeKey.getPublicKey())); validatorMessageFactory.add(new MessageFactory(nodeKey)); @@ -603,4 +604,42 @@ public void checkOnlyEmptyBlockPeriodSecondsIsInvokedForBlocksWithNoTransactions verify(blockTimer, times(0)).getEmptyBlockPeriodSeconds(); verify(blockTimer, times(0)).getBlockPeriodSeconds(); } + + @Test + public void roundChangeTriggeredUponReceivingFPlusOneRoundChanges() { + final ConsensusRoundIdentifier futureRoundIdentifier1 = createFrom(roundIdentifier, 0, +2); + final ConsensusRoundIdentifier futureRoundIdentifier2 = createFrom(roundIdentifier, 0, +3); + + final RoundChange roundChange1 = + validatorMessageFactory.get(0).createRoundChange(futureRoundIdentifier1, Optional.empty()); + final RoundChange roundChange2 = + validatorMessageFactory.get(1).createRoundChange(futureRoundIdentifier2, Optional.empty()); + + RoundChangeMessageValidator roundChangeMessageValidator = + mock(RoundChangeMessageValidator.class); + when(roundChangeMessageValidator.validate(any())).thenReturn(true); + + // Instantiate the real RoundChangeManager + final RoundChangeManager roundChangeManager = + new RoundChangeManager(3, 2, roundChangeMessageValidator, validators.get(2)); + + when(finalState.isLocalNodeProposerForRound(any())).thenReturn(false); + + final QbftBlockHeightManager manager = + new QbftBlockHeightManager( + headerTestFixture.buildHeader(), + finalState, + roundChangeManager, + roundFactory, + clock, + messageValidatorFactory, + validatorMessageFactory.get(2), + true); // Enable early round change + + manager.handleRoundChangePayload(roundChange1); + manager.handleRoundChangePayload(roundChange2); + + verify(roundFactory, times(1)) + .createNewRound(any(), eq(futureRoundIdentifier1.getRoundNumber())); + } }