diff --git a/CHANGELOG.md b/CHANGELOG.md index c1fbd4d4f4e..a641f74fdf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,13 @@ - Upgrade netty to 4.2.10-Final - Fixes `setsockopt() failed: Protocol not available` [#9783](https://github.com/hyperledger/besu/pull/9783) - Allow nonce to be max value when `isAllowFutureNonce` is true [#9759](https://github.com/hyperledger/besu/pull/9759) +<<<<<<< bft-time-based-forks +- BFT forks that change block period on time-based forks don't take effect [9681](https://github.com/hyperledger/besu/issues/9681) + +## 26.1.0-RC1 +======= ## 26.1.0 +>>>>>>> main ### Breaking Changes - Remove experimental CLI flag `--Xenable-extra-debug-tracers`. Call tracer (`callTracer`) is now always available for `debug_trace*` methods. diff --git a/app/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java b/app/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java index 29d8298cf90..93374af51b0 100644 --- a/app/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java +++ b/app/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java @@ -114,7 +114,7 @@ protected MiningCoordinator createMiningCoordinator( o -> miningConfiguration.setBlockPeriodSeconds( forksSchedule - .getFork(o.getHeader().getNumber() + 1) + .getFork(o.getHeader().getNumber() + 1, o.getHeader().getTimestamp()) .getValue() .getBlockPeriodSeconds())); diff --git a/app/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/app/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java index 5c702d3a490..9f0504bd6c0 100644 --- a/app/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java +++ b/app/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java @@ -250,7 +250,7 @@ protected MiningCoordinator createMiningCoordinator( o -> miningConfiguration.setBlockPeriodSeconds( forksSchedule - .getFork(o.getHeader().getNumber() + 1) + .getFork(o.getHeader().getNumber() + 1, o.getHeader().getTimestamp()) .getValue() .getBlockPeriodSeconds())); diff --git a/app/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/app/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java index 88cabd5a0a4..7fbf58d0bae 100644 --- a/app/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java +++ b/app/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java @@ -335,12 +335,12 @@ protected MiningCoordinator createMiningCoordinator( o -> { miningConfiguration.setBlockPeriodSeconds( qbftForksSchedule - .getFork(o.getHeader().getNumber() + 1) + .getFork(o.getHeader().getNumber() + 1, o.getHeader().getTimestamp()) .getValue() .getBlockPeriodSeconds()); miningConfiguration.setEmptyBlockPeriodSeconds( qbftForksSchedule - .getFork(o.getHeader().getNumber() + 1) + .getFork(o.getHeader().getNumber() + 1, o.getHeader().getTimestamp()) .getValue() .getEmptyBlockPeriodSeconds()); }); diff --git a/app/src/test/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilderTest.java b/app/src/test/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilderTest.java index fc966dffa26..b45814afea2 100644 --- a/app/src/test/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilderTest.java +++ b/app/src/test/java/org/hyperledger/besu/controller/ConsensusScheduleBesuControllerBuilderTest.java @@ -32,6 +32,7 @@ import org.hyperledger.besu.ethereum.blockcreation.MiningCoordinator; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MiningConfiguration; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; @@ -129,7 +130,9 @@ public void createsMigratingMiningCoordinator() { when(besuControllerBuilder2.createMiningCoordinator(any(), any(), any(), any(), any(), any())) .thenReturn(miningCoordinator2); final ProtocolContext mockProtocolContext = mock(ProtocolContext.class); - when(mockProtocolContext.getBlockchain()).thenReturn(mock(MutableBlockchain.class)); + final MutableBlockchain blockchain = mock(MutableBlockchain.class); + when(mockProtocolContext.getBlockchain()).thenReturn(blockchain); + when(blockchain.getChainHeadHeader()).thenReturn(mock(BlockHeader.class)); final ConsensusScheduleBesuControllerBuilder builder = new ConsensusScheduleBesuControllerBuilder(consensusSchedule); @@ -150,19 +153,31 @@ public void createsMigratingMiningCoordinator() { (softly) -> { softly .assertThat( - migratingMiningCoordinator.getMiningCoordinatorSchedule().getFork(0L).getValue()) + migratingMiningCoordinator + .getMiningCoordinatorSchedule() + .getFork(0L, 0) + .getValue()) .isSameAs(miningCoordinator1); softly .assertThat( - migratingMiningCoordinator.getMiningCoordinatorSchedule().getFork(4L).getValue()) + migratingMiningCoordinator + .getMiningCoordinatorSchedule() + .getFork(4L, 0) + .getValue()) .isSameAs(miningCoordinator1); softly .assertThat( - migratingMiningCoordinator.getMiningCoordinatorSchedule().getFork(5L).getValue()) + migratingMiningCoordinator + .getMiningCoordinatorSchedule() + .getFork(5L, 0) + .getValue()) .isSameAs(miningCoordinator2); softly .assertThat( - migratingMiningCoordinator.getMiningCoordinatorSchedule().getFork(6L).getValue()) + migratingMiningCoordinator + .getMiningCoordinatorSchedule() + .getFork(6L, 0) + .getValue()) .isSameAs(miningCoordinator2); }); } @@ -198,11 +213,11 @@ public void createsMigratingContext() { expectedConsensusContextSpecs.add(new ForkSpec<>(10L, context2)); assertThat(contextSchedule.getForks()).isEqualTo(expectedConsensusContextSpecs); - assertThat(contextSchedule.getFork(0).getValue()).isSameAs(context1); - assertThat(contextSchedule.getFork(1).getValue()).isSameAs(context1); - assertThat(contextSchedule.getFork(9).getValue()).isSameAs(context1); - assertThat(contextSchedule.getFork(10).getValue()).isSameAs(context2); - assertThat(contextSchedule.getFork(11).getValue()).isSameAs(context2); + assertThat(contextSchedule.getFork(0, 0).getValue()).isSameAs(context1); + assertThat(contextSchedule.getFork(1, 0).getValue()).isSameAs(context1); + assertThat(contextSchedule.getFork(9, 0).getValue()).isSameAs(context1); + assertThat(contextSchedule.getFork(10, 0).getValue()).isSameAs(context2); + assertThat(contextSchedule.getFork(11, 0).getValue()).isSameAs(context2); } @Test diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMiner.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMiner.java index f84e3426512..3b1beadcb58 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMiner.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMiner.java @@ -78,7 +78,10 @@ protected boolean mineBlock() throws InterruptedException { @Override protected boolean shouldImportBlock(final Block block) throws InterruptedException { - if (forksSchedule.getFork(block.getHeader().getNumber()).getValue().getCreateEmptyBlocks()) { + if (forksSchedule + .getFork(block.getHeader().getNumber(), block.getHeader().getTimestamp()) + .getValue() + .getCreateEmptyBlocks()) { return true; } diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockScheduler.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockScheduler.java index 5ffa78e09f1..3f4cfb3a4fa 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockScheduler.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockScheduler.java @@ -53,7 +53,7 @@ public CliqueBlockScheduler( parentHeader -> (long) forksSchedule - .getFork(parentHeader.getNumber() + 1) + .getFork(parentHeader.getNumber() + 1, parentHeader.getTimestamp()) .getValue() .getBlockPeriodSeconds(), 0L, diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForkSpec.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForkSpec.java index 0af223e0afe..4fc5dfac98c 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForkSpec.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForkSpec.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.consensus.common; +import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; + import java.util.Comparator; import java.util.Objects; @@ -28,6 +30,8 @@ public class ForkSpec { public static final Comparator> COMPARATOR = Comparator.comparing(ForkSpec::getBlock); private final long block; + private ScheduledProtocolSpec.ScheduleType forkType = + ScheduledProtocolSpec.ScheduleType.BLOCK; // Default type private final C value; /** @@ -50,6 +54,24 @@ public long getBlock() { return block; } + /** + * Gets the fork type (block number or timestamp). + * + * @param forkType the fork type + */ + public void setForkType(final ScheduledProtocolSpec.ScheduleType forkType) { + this.forkType = forkType; + } + + /** + * Gets the fork type (block number or timestamp). + * + * @return the fork type + */ + public ScheduledProtocolSpec.ScheduleType getForkType() { + return forkType; + } + /** * Gets value. * diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForksSchedule.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForksSchedule.java index 3ab50b6cacb..78f18b863d6 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForksSchedule.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/ForksSchedule.java @@ -14,6 +14,9 @@ */ package org.hyperledger.besu.consensus.common; +import org.hyperledger.besu.consensus.common.bft.BftProtocolSchedule; +import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; + import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -42,15 +45,33 @@ public ForksSchedule(final Collection> forks) { this.forks.addAll(forks); } + /** + * Apply the protocol schedule to the forks to assert their fork type - block or timestamp + * + * @param protocolSchedule the protocol schedule + */ + public void applyMilestoneTypes(final BftProtocolSchedule protocolSchedule) { + forks.forEach( + f -> + f.setForkType( + protocolSchedule.getSpecTypeBlockNumberOrTimestamp(f.getBlock(), f.getBlock()))); + } + /** * Gets fork. * * @param blockNumber the block number + * @param blockTimestamp the block timestamp * @return the fork */ - public ForkSpec getFork(final long blockNumber) { + public ForkSpec getFork(final long blockNumber, final long blockTimestamp) { for (final ForkSpec f : forks) { - if (blockNumber >= f.getBlock()) { + if (f.getForkType() == ScheduledProtocolSpec.ScheduleType.BLOCK + && blockNumber >= f.getBlock()) { + return f; + } + if (f.getForkType() == ScheduledProtocolSpec.ScheduleType.TIME + && blockTimestamp >= f.getBlock()) { return f; } } diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java index b5166bad69e..830ba5e9efe 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinator.java @@ -53,8 +53,13 @@ public MigratingMiningCoordinator( final Blockchain blockchain) { this.miningCoordinatorSchedule = miningCoordinatorSchedule; this.blockchain = blockchain; + + final BlockHeader chainHead = blockchain.getChainHeadHeader(); + this.activeMiningCoordinator = - this.miningCoordinatorSchedule.getFork(blockchain.getChainHeadBlockNumber() + 1).getValue(); + this.miningCoordinatorSchedule + .getFork(chainHead.getNumber() + 1, chainHead.getTimestamp()) + .getValue(); } @Override @@ -133,8 +138,9 @@ public void changeTargetGasLimit(final Long targetGasLimit) { @Override public void onBlockAdded(final BlockAddedEvent event) { final long currentBlock = event.getHeader().getNumber(); + final long parentTimestamp = event.getHeader().getTimestamp(); final MiningCoordinator nextMiningCoordinator = - miningCoordinatorSchedule.getFork(currentBlock + 1).getValue(); + miningCoordinatorSchedule.getFork(currentBlock + 1, parentTimestamp).getValue(); if (activeMiningCoordinator != nextMiningCoordinator) { LOG.trace( diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingProtocolContext.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingProtocolContext.java index fe44a03b1ca..c1d672eb795 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingProtocolContext.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/MigratingProtocolContext.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; import org.hyperledger.besu.plugin.ServiceManager; +import org.hyperledger.besu.plugin.data.BlockHeader; /** The Migrating protocol context. */ public class MigratingProtocolContext extends ProtocolContext { @@ -48,13 +49,18 @@ public MigratingProtocolContext( @Override public C getConsensusContext(final Class klass) { - final long chainHeadBlockNumber = getBlockchain().getChainHeadBlockNumber(); - return consensusContextSchedule.getFork(chainHeadBlockNumber + 1).getValue().as(klass); + final BlockHeader chainHead = getBlockchain().getChainHeadHeader(); + return consensusContextSchedule + .getFork(chainHead.getNumber() + 1, chainHead.getTimestamp()) + .getValue() + .as(klass); } @Override public C getConsensusContext( final Class klass, final long blockNumber) { - return consensusContextSchedule.getFork(blockNumber).getValue().as(klass); + // Block number will be either an actual block number or a timestamp, so we pass it in for both + // getFork() args + return consensusContextSchedule.getFork(blockNumber, blockNumber).getValue().as(klass); } } 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 c0a407a29c0..28edcc95b25 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 @@ -101,7 +101,13 @@ public BftProtocolSchedule createProtocolSchedule( balConfiguration, metricsSystem) .createProtocolSchedule(); - return new BftProtocolSchedule((DefaultProtocolSchedule) protocolSchedule); + final BftProtocolSchedule bftSchedule = + new BftProtocolSchedule((DefaultProtocolSchedule) protocolSchedule); + + // Once we have the schedule we can update the fork schedule with the type of each milestone + forksSchedule.applyMilestoneTypes(bftSchedule); + + return bftSchedule; } /** 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 ab6dc6eb66a..bd72904333b 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 @@ -68,6 +68,38 @@ public ProtocolSpec getByBlockNumberOrTimestamp(final long number, final long ti return theSpec; } + /** + * Look up type of spec by block number or timestamp + * + * @param number block number + * @param timestamp block timestamp + * @return the protocol spec type for that block number or timestamp + */ + public ScheduledProtocolSpec.ScheduleType getSpecTypeBlockNumberOrTimestamp( + 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 + ScheduledProtocolSpec.ScheduleType specType = null; + for (final ScheduledProtocolSpec s : protocolSpecs) { + if ((s instanceof ScheduledProtocolSpec.BlockNumberProtocolSpec) + && (number >= s.fork().milestone())) { + specType = ScheduledProtocolSpec.ScheduleType.BLOCK; + break; + } else if ((s instanceof ScheduledProtocolSpec.TimestampProtocolSpec) + && (timestamp >= s.fork().milestone())) { + specType = ScheduledProtocolSpec.ScheduleType.TIME; + break; + } + } + return specType; + } + /** * return the ordered list of scheduled protocol specs * diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java index 91f3c394d74..b756347177a 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java @@ -88,10 +88,30 @@ public synchronized void startTimer( cancelTimer(); final long expiryTime; + int currentBlockPeriodSeconds = + forksSchedule + .getFork(round.getSequenceNumber(), headerTimestamp.get()) + .getValue() + .getBlockPeriodSeconds(); + final int nextBlockPeriodSeconds = + forksSchedule + .getFork(round.getSequenceNumber(), headerTimestamp.get() + currentBlockPeriodSeconds) + .getValue() + .getBlockPeriodSeconds(); + + // If the block period seconds change between the current block and the next one we need to + // produce this block on the longer of the two values, + // otherwise block validation will fail (blocks produced too close together) + if (nextBlockPeriodSeconds > currentBlockPeriodSeconds) { + currentBlockPeriodSeconds = nextBlockPeriodSeconds; + } // Experimental option for test scenarios only. Not for production use. final long blockPeriodMilliseconds = - forksSchedule.getFork(round.getSequenceNumber()).getValue().getBlockPeriodMilliseconds(); + forksSchedule + .getFork(round.getSequenceNumber(), headerTimestamp.get()) + .getValue() + .getBlockPeriodMilliseconds(); if (blockPeriodMilliseconds > 0) { // Experimental mode for setting < 1 second block periods e.g. for CI/CD pipelines // running tests against Besu @@ -101,13 +121,16 @@ public synchronized void startTimer( blockPeriodMilliseconds); } else { // absolute time when the timer is supposed to expire - final int currentBlockPeriodSeconds = - forksSchedule.getFork(round.getSequenceNumber()).getValue().getBlockPeriodSeconds(); final long minimumTimeBetweenBlocksMillis = currentBlockPeriodSeconds * 1000L; expiryTime = headerTimestamp.get() * 1_000 + minimumTimeBetweenBlocksMillis; } - setBlockTimes(round); + final int emptyBlockPeriodSeconds = + forksSchedule + .getFork(round.getSequenceNumber(), headerTimestamp.get()) + .getValue() + .getEmptyBlockPeriodSeconds(); + setBlockTimes(currentBlockPeriodSeconds, emptyBlockPeriodSeconds); startTimer(round, expiryTime); } @@ -168,11 +191,10 @@ private synchronized void startTimer( } } - private synchronized void setBlockTimes(final ConsensusRoundIdentifier round) { - final BftConfigOptions currentConfigOptions = - forksSchedule.getFork(round.getSequenceNumber()).getValue(); - this.blockPeriodSeconds = currentConfigOptions.getBlockPeriodSeconds(); - this.emptyBlockPeriodSeconds = currentConfigOptions.getEmptyBlockPeriodSeconds(); + private synchronized void setBlockTimes( + final int blockPeriodSeconds, final int emptyBlockPeriodSeconds) { + this.blockPeriodSeconds = blockPeriodSeconds; + this.emptyBlockPeriodSeconds = emptyBlockPeriodSeconds; } /** diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleFactoryTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleFactoryTest.java index 89e7330c4c4..4bbecebfe89 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleFactoryTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleFactoryTest.java @@ -77,9 +77,9 @@ public void createsScheduleUsingSpecCreator() { final ForksSchedule schedule = ForksScheduleFactory.create(genesisConfigOptions, List.of(fork1, fork2), specCreator); - assertThat(schedule.getFork(0)).isEqualTo(genesisForkSpec); - assertThat(schedule.getFork(1)).isEqualTo(new ForkSpec<>(1, configOptions1)); - assertThat(schedule.getFork(2)).isEqualTo(new ForkSpec<>(2, configOptions2)); + assertThat(schedule.getFork(0, 0)).isEqualTo(genesisForkSpec); + assertThat(schedule.getFork(1, 0)).isEqualTo(new ForkSpec<>(1, configOptions1)); + assertThat(schedule.getFork(2, 0)).isEqualTo(new ForkSpec<>(2, configOptions2)); } private MutableBftConfigOptions createBftConfigOptions( diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleTest.java index a70d20da8d2..88c435aa29c 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/ForksScheduleTest.java @@ -36,8 +36,8 @@ public void retrievesGenesisFork() { final ForksSchedule schedule = new ForksSchedule<>(List.of(forkSpec1, genesisForkSpec)); - assertThat(schedule.getFork(0)).isEqualTo(genesisForkSpec); - assertThat(schedule.getFork(1)).isEqualTo(genesisForkSpec); + assertThat(schedule.getFork(0, 0)).isEqualTo(genesisForkSpec); + assertThat(schedule.getFork(1, 0)).isEqualTo(genesisForkSpec); } @Test @@ -56,13 +56,14 @@ public void retrievesLatestFork() { final ForksSchedule schedule = new ForksSchedule<>(List.of(genesisForkSpec, forkSpec1, forkSpec2, forkSpec3, forkSpec4)); - assertThat(schedule.getFork(0)).isEqualTo(genesisForkSpec); - assertThat(schedule.getFork(1)).isEqualTo(forkSpec1); - assertThat(schedule.getFork(2)).isEqualTo(forkSpec2); - assertThat(schedule.getFork(3)).isEqualTo(forkSpec3); - assertThat(schedule.getFork(3).getValue().getMiningBeneficiary()).isEqualTo(miningBeneficiary3); - assertThat(schedule.getFork(4)).isEqualTo(forkSpec4); - assertThat(schedule.getFork(4).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(schedule.getFork(0, 0)).isEqualTo(genesisForkSpec); + assertThat(schedule.getFork(1, 0)).isEqualTo(forkSpec1); + assertThat(schedule.getFork(2, 0)).isEqualTo(forkSpec2); + assertThat(schedule.getFork(3, 0)).isEqualTo(forkSpec3); + assertThat(schedule.getFork(3, 0).getValue().getMiningBeneficiary()) + .isEqualTo(miningBeneficiary3); + assertThat(schedule.getFork(4, 0)).isEqualTo(forkSpec4); + assertThat(schedule.getFork(4, 0).getValue().getMiningBeneficiary()).isEmpty(); } private ForkSpec createForkSpecWithMiningBeneficiary( diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinatorTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinatorTest.java index 8c18a934d0f..c886d577bd2 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinatorTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingMiningCoordinatorTest.java @@ -75,6 +75,7 @@ private ForksSchedule createCoordinatorSchedule( @Test public void startShouldRegisterThisCoordinatorAsObserver() { + when(blockchain.getChainHeadHeader()).thenReturn(blockHeader); final MigratingMiningCoordinator coordinator = new MigratingMiningCoordinator(coordinatorSchedule, blockchain); @@ -86,6 +87,7 @@ public void startShouldRegisterThisCoordinatorAsObserver() { @Test public void startShouldUnregisterDelegateCoordinatorAsObserver() { final BftMiningCoordinator delegateCoordinator = createDelegateCoordinator(); + when(blockchain.getChainHeadHeader()).thenReturn(blockHeader); lenient().when(blockchain.observeBlockAdded(delegateCoordinator)).thenReturn(1L); final MigratingMiningCoordinator coordinator = new MigratingMiningCoordinator( @@ -109,6 +111,7 @@ private BftMiningCoordinator createDelegateCoordinator() { @Test public void stopShouldUnregisterThisCoordinatorAsObserver() { + when(blockchain.getChainHeadHeader()).thenReturn(blockHeader); final MigratingMiningCoordinator coordinator = new MigratingMiningCoordinator(coordinatorSchedule, blockchain); when(blockchain.observeBlockAdded(coordinator)).thenReturn(1L); @@ -121,6 +124,7 @@ public void stopShouldUnregisterThisCoordinatorAsObserver() { @Test public void onBlockAddedShouldNotDelegateWhenDelegateIsNoop() { + when(blockchain.getChainHeadHeader()).thenReturn(blockHeader); MiningCoordinator mockMiningCoordinator = mock(MiningCoordinator.class); coordinatorSchedule = createCoordinatorSchedule(mockMiningCoordinator, coordinator2); when(blockHeader.getNumber()).thenReturn(GENESIS_BLOCK_NUMBER); @@ -192,7 +196,8 @@ private void verifyDelegation( final long blockHeight, final MiningCoordinator expectedActiveCoordinator, final MiningCoordinator expectedInactiveCoordinator) { - when(blockchain.getChainHeadBlockNumber()).thenReturn(blockHeight); + when(blockchain.getChainHeadHeader()).thenReturn(blockHeader); + when(blockHeader.getNumber()).thenReturn(blockHeight); methodUnderTest.accept(new MigratingMiningCoordinator(coordinatorSchedule, blockchain)); @@ -212,7 +217,8 @@ private void verifyDelegationForAwaitStop( final MiningCoordinator expectedActiveCoordinator, final MiningCoordinator expectedInactiveCoordinator) throws InterruptedException { - when(blockchain.getChainHeadBlockNumber()).thenReturn(blockHeight); + when(blockchain.getChainHeadHeader()).thenReturn(blockHeader); + when(blockHeader.getNumber()).thenReturn(blockHeight); new MigratingMiningCoordinator(coordinatorSchedule, blockchain).awaitStop(); @@ -231,7 +237,7 @@ private void verifyDelegationForOnBlockAdded( final long blockHeight, final BftMiningCoordinator expectedActiveCoordinator, final BftMiningCoordinator expectedInactiveCoordinator) { - when(blockchain.getChainHeadBlockNumber()).thenReturn(blockHeight); + when(blockchain.getChainHeadHeader()).thenReturn(blockHeader); when(blockHeader.getNumber()).thenReturn(blockHeight); new MigratingMiningCoordinator(coordinatorSchedule, blockchain).onBlockAdded(blockEvent); diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingProtocolContextTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingProtocolContextTest.java index 3e80ab0376f..8510f12930e 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingProtocolContextTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/MigratingProtocolContextTest.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.ethereum.ConsensusContext; import org.hyperledger.besu.ethereum.chain.BadBlockManager; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; import org.hyperledger.besu.plugin.ServiceManager; @@ -34,8 +35,11 @@ public class MigratingProtocolContextTest { @Test public void returnsContextForSpecificChainHeight() { final MutableBlockchain blockchain = Mockito.mock(MutableBlockchain.class); + final BlockHeader header = Mockito.mock(BlockHeader.class); final WorldStateArchive worldStateArchive = Mockito.mock(WorldStateArchive.class); + when(blockchain.getChainHeadHeader()).thenReturn(header); + final ConsensusContext context1 = Mockito.mock(ConsensusContext.class); when(context1.as(any())).thenReturn(context1); @@ -56,11 +60,11 @@ public void returnsContextForSpecificChainHeight() { assertThat(migratingProtocolContext.getConsensusContext(ConsensusContext.class)) .isSameAs(context1); - when(blockchain.getChainHeadBlockNumber()).thenReturn(2L); + when(header.getNumber()).thenReturn(2L); assertThat(migratingProtocolContext.getConsensusContext(ConsensusContext.class)) .isSameAs(context1); - when(blockchain.getChainHeadBlockNumber()).thenReturn(10L); + when(header.getNumber()).thenReturn(10L); assertThat(migratingProtocolContext.getConsensusContext(ConsensusContext.class)) .isSameAs(context2); } diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilderTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilderTest.java index 443879a9261..ee5ee5a43cc 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilderTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilderTest.java @@ -37,6 +37,7 @@ import org.hyperledger.besu.ethereum.mainnet.DefaultProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; +import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator; import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket; import org.hyperledger.besu.evm.internal.EvmConfiguration; @@ -45,6 +46,7 @@ import java.math.BigInteger; import java.util.List; import java.util.Optional; +import java.util.OptionalLong; import org.junit.jupiter.api.Test; @@ -246,6 +248,11 @@ public void blockRewardSpecifiedInTransitionCreatesNewMilestone() { } private ProtocolSchedule createProtocolSchedule(final List> forks) { + return createProtocolSchedule(new ForksSchedule<>(forks)); + } + + private ProtocolSchedule createProtocolSchedule( + final ForksSchedule forkSchedule) { final BaseBftProtocolScheduleBuilder bftProtocolSchedule = new BaseBftProtocolScheduleBuilder() { @Override @@ -254,17 +261,58 @@ protected BlockHeaderValidator.Builder createBlockHeaderRuleset( return new BlockHeaderValidator.Builder(); } }; - return bftProtocolSchedule.createProtocolSchedule( - genesisConfig, - new ForksSchedule<>(forks), - false, - bftExtraDataCodec, - EvmConfiguration.DEFAULT, - MiningConfiguration.MINING_DISABLED, - new BadBlockManager(), - false, - BalConfiguration.DEFAULT, - new NoOpMetricsSystem()); + final BftProtocolSchedule protocolSchedule = + bftProtocolSchedule.createProtocolSchedule( + genesisConfig, + forkSchedule, + false, + bftExtraDataCodec, + EvmConfiguration.DEFAULT, + MiningConfiguration.MINING_DISABLED, + new BadBlockManager(), + false, + BalConfiguration.DEFAULT, + new NoOpMetricsSystem()); + + return protocolSchedule; + } + + @Test + public void ensureBlockPeriodInProtocolSpecMatchConfig() { + final BftConfigOptions configBlockPeriod2 = createBftConfigBlockPeriod(2); + when(genesisConfig.getBftConfigOptions()).thenReturn(configBlockPeriod2); + when(genesisConfig.getLondonBlockNumber()).thenReturn(OptionalLong.of(50)); + + // All forks after block 100 should be time based forks and retrieval from the fork + // schedule should be based on block timestamp not block number + when(genesisConfig.getShanghaiTime()).thenReturn(OptionalLong.of(100)); + TransitionsConfigOptions transitions = TransitionsConfigOptions.DEFAULT; + when(genesisConfig.getTransitions()).thenReturn(transitions); + + final BftConfigOptions configBlockPeriod3 = createBftConfigBlockPeriod(3); + final BftConfigOptions configBlockPeriod4 = createBftConfigBlockPeriod(4); + + ForksSchedule forkSchedule = + new ForksSchedule<>( + List.of( + new ForkSpec<>(0, configBlockPeriod2), + new ForkSpec<>(123456, configBlockPeriod3), + new ForkSpec<>(999999, configBlockPeriod4))); + + createProtocolSchedule(forkSchedule); + + // Creating a protocol schedule updates the fork schedule with fork types + assertThat(forkSchedule.getFork(0, 0).getForkType()) + .isEqualTo(ScheduledProtocolSpec.ScheduleType.BLOCK); + assertThat(forkSchedule.getFork(10, 0).getForkType()) + .isEqualTo(ScheduledProtocolSpec.ScheduleType.BLOCK); + assertThat(forkSchedule.getFork(10, 0).getValue().getBlockPeriodSeconds()).isEqualTo(2); + assertThat(forkSchedule.getFork(10, 123456).getForkType()) + .isEqualTo(ScheduledProtocolSpec.ScheduleType.TIME); + assertThat(forkSchedule.getFork(10, 123456).getValue().getBlockPeriodSeconds()).isEqualTo(3); + assertThat(forkSchedule.getFork(10, 999999).getForkType()) + .isEqualTo(ScheduledProtocolSpec.ScheduleType.TIME); + assertThat(forkSchedule.getFork(10, 999999).getValue().getBlockPeriodSeconds()).isEqualTo(4); } private BftConfigOptions createBftConfig(final BigInteger blockReward) { @@ -281,6 +329,15 @@ private BftConfigOptions createBftConfig( return bftConfig; } + private BftConfigOptions createBftConfigBlockPeriod(final int blockPeriod) { + final BftConfigOptions bftConfig = mock(JsonBftConfigOptions.class); + when(bftConfig.getBlockPeriodSeconds()).thenReturn(blockPeriod); + when(bftConfig.getBlockRewardWei()).thenReturn(BigInteger.ONE); + when(bftConfig.getEpochLength()).thenReturn(3000L); + + return bftConfig; + } + private BlockHeader blockHeader(final long number) { return new BlockHeaderTestFixture().number(number).buildHeader(); } diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseForksSchedulesFactoryTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseForksSchedulesFactoryTest.java index 14b3bea7f8a..1c83d78b183 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseForksSchedulesFactoryTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BaseForksSchedulesFactoryTest.java @@ -24,6 +24,7 @@ import org.hyperledger.besu.consensus.common.ForkSpec; import org.hyperledger.besu.consensus.common.ForksSchedule; import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; import java.util.Map; import java.util.Optional; @@ -42,9 +43,9 @@ public void createsScheduleForJustGenesisConfig() { final GenesisConfigOptions genesisConfigOptions = createGenesisConfig(configOptions); final ForksSchedule forksSchedule = createForkSchedule(genesisConfigOptions); - assertThat(forksSchedule.getFork(0)).usingRecursiveComparison().isEqualTo(expectedForkSpec); - assertThat(forksSchedule.getFork(1)).usingRecursiveComparison().isEqualTo(expectedForkSpec); - assertThat(forksSchedule.getFork(2)).usingRecursiveComparison().isEqualTo(expectedForkSpec); + assertThat(forksSchedule.getFork(0, 0)).usingRecursiveComparison().isEqualTo(expectedForkSpec); + assertThat(forksSchedule.getFork(1, 0)).usingRecursiveComparison().isEqualTo(expectedForkSpec); + assertThat(forksSchedule.getFork(2, 0)).usingRecursiveComparison().isEqualTo(expectedForkSpec); } @Test @@ -68,10 +69,10 @@ public void createsScheduleThatChangesMiningBeneficiary_beneficiaryInitiallyEmpt createGenesisConfig(qbftConfigOptions, forkWithBeneficiary, forkWithNoBeneficiary); final ForksSchedule forksSchedule = createForkSchedule(genesisConfigOptions); - assertThat(forksSchedule.getFork(0).getValue().getMiningBeneficiary()).isEmpty(); - assertThat(forksSchedule.getFork(1).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(0, 0).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(forksSchedule.getFork(1, 0).getValue().getMiningBeneficiary()) .contains(beneficiaryAddress); - assertThat(forksSchedule.getFork(2).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(forksSchedule.getFork(2, 0).getValue().getMiningBeneficiary()).isEmpty(); } @Test @@ -97,10 +98,10 @@ public void createsScheduleThatChangesMiningBeneficiary_beneficiaryInitiallyNonE createGenesisConfig(qbftConfigOptions, forkWithBeneficiary, forkWithNoBeneficiary); final ForksSchedule forksSchedule = createForkSchedule(genesisConfigOptions); - assertThat(forksSchedule.getFork(0).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(0, 0).getValue().getMiningBeneficiary()) .contains(beneficiaryAddress); - assertThat(forksSchedule.getFork(1).getValue().getMiningBeneficiary()).isEmpty(); - assertThat(forksSchedule.getFork(2).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(1, 0).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(forksSchedule.getFork(2, 0).getValue().getMiningBeneficiary()) .contains(beneficiaryAddress2); } @@ -135,19 +136,19 @@ public void createsScheduleThatChangesMiningBeneficiary_beneficiaryInitiallyNonE final GenesisConfigOptions genesisConfigOptions = createGenesisConfig(qbftConfigOptions, forks); final ForksSchedule forksSchedule = createForkSchedule(genesisConfigOptions); - assertThat(forksSchedule.getFork(0).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(0, 0).getValue().getMiningBeneficiary()) .contains(initialBeneficiaryAddress); - assertThat(forksSchedule.getFork(1).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(1, 0).getValue().getMiningBeneficiary()) .contains(initialBeneficiaryAddress); - assertThat(forksSchedule.getFork(2).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(2, 0).getValue().getMiningBeneficiary()) .contains(initialBeneficiaryAddress); - assertThat(forksSchedule.getFork(3).getValue().getMiningBeneficiary()).isEmpty(); - assertThat(forksSchedule.getFork(4).getValue().getMiningBeneficiary()).isEmpty(); - assertThat(forksSchedule.getFork(5).getValue().getMiningBeneficiary()).isEmpty(); - assertThat(forksSchedule.getFork(6).getValue().getMiningBeneficiary()).isEmpty(); - assertThat(forksSchedule.getFork(7).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(3, 0).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(forksSchedule.getFork(4, 0).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(forksSchedule.getFork(5, 0).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(forksSchedule.getFork(6, 0).getValue().getMiningBeneficiary()).isEmpty(); + assertThat(forksSchedule.getFork(7, 0).getValue().getMiningBeneficiary()) .contains(beneficiaryAddress2); - assertThat(forksSchedule.getFork(8).getValue().getMiningBeneficiary()) + assertThat(forksSchedule.getFork(8, 0).getValue().getMiningBeneficiary()) .contains(beneficiaryAddress2); } @@ -168,6 +169,46 @@ public void createsScheduleWithInvalidMiningBeneficiary_shouldThrow() { "Mining beneficiary in transition config is not a valid ethereum address"); } + @Test + public void createsScheduleThatChangesBlockPeriodAtTimestamp() { + final C qbftConfigOptions = createBftOptions(o -> o.setBlockPeriodSeconds(2)); + + ObjectNode[] forks = { + // Change block period 2->3 seconds + JsonUtil.objectNodeFromMap( + Map.of(BftFork.FORK_BLOCK_KEY, 1234567, BftFork.BLOCK_PERIOD_SECONDS_KEY, 3)), + // Change block period 3->4 seconds + JsonUtil.objectNodeFromMap( + Map.of(BftFork.FORK_BLOCK_KEY, 1234568, BftFork.BLOCK_PERIOD_SECONDS_KEY, 4)) + }; + + final GenesisConfigOptions genesisConfigOptions = createGenesisConfig(qbftConfigOptions, forks); + final ForksSchedule forksSchedule = createForkSchedule(genesisConfigOptions); + + for (ForkSpec f : forksSchedule.getForks()) { + f.setForkType(ScheduledProtocolSpec.ScheduleType.BLOCK); + } + + // Should ignore block timestamp if forks are block type + assertThat(forksSchedule.getFork(0, 1234566).getValue().getBlockPeriodSeconds()).isEqualTo(2); + assertThat(forksSchedule.getFork(0, 1234567).getValue().getBlockPeriodSeconds()).isEqualTo(2); + assertThat(forksSchedule.getFork(0, 1234568).getValue().getBlockPeriodSeconds()).isEqualTo(2); + + for (ForkSpec f : forksSchedule.getForks()) { + f.setForkType(ScheduledProtocolSpec.ScheduleType.TIME); + } + + // Should ignore block timestamp if forks are block type + assertThat(forksSchedule.getFork(1234566, 0).getValue().getBlockPeriodSeconds()).isEqualTo(2); + assertThat(forksSchedule.getFork(1234567, 0).getValue().getBlockPeriodSeconds()).isEqualTo(2); + assertThat(forksSchedule.getFork(1234568, 0).getValue().getBlockPeriodSeconds()).isEqualTo(2); + + // Should reflect changes based on timestamp + assertThat(forksSchedule.getFork(0, 1234566).getValue().getBlockPeriodSeconds()).isEqualTo(2); + assertThat(forksSchedule.getFork(0, 1234567).getValue().getBlockPeriodSeconds()).isEqualTo(3); + assertThat(forksSchedule.getFork(0, 1234568).getValue().getBlockPeriodSeconds()).isEqualTo(4); + } + protected abstract C createBftOptions(final Consumer optionModifier); protected C createBftOptions() { diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BlockTimerTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BlockTimerTest.java index da8a739aa4a..e27d5acd8a1 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BlockTimerTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BlockTimerTest.java @@ -80,7 +80,7 @@ public void startTimerSchedulesCorrectlyWhenExpiryIsInTheFuture() { final long BLOCK_TIME_STAMP = 500L; final long EXPECTED_DELAY = 10_000L; - when(mockForksSchedule.getFork(anyLong())) + when(mockForksSchedule.getFork(anyLong(), anyLong())) .thenReturn( new ForkSpec<>( 0, @@ -115,7 +115,7 @@ public void aBlockTimerExpiryEventIsAddedToTheQueueOnExpiry() throws Interrupted final long BLOCK_TIME_STAMP = 300; final long EXPECTED_DELAY = 500; - when(mockForksSchedule.getFork(anyLong())) + when(mockForksSchedule.getFork(anyLong(), anyLong())) .thenReturn( new ForkSpec<>( 0, @@ -165,7 +165,7 @@ public void eventIsImmediatelyAddedToTheQueueIfAbsoluteExpiryIsEqualToNow() { final long NOW_MILLIS = 515_000L; final long BLOCK_TIME_STAMP = 500; - when(mockForksSchedule.getFork(anyLong())) + when(mockForksSchedule.getFork(anyLong(), anyLong())) .thenReturn( new ForkSpec<>( 0, @@ -201,7 +201,7 @@ public void eventIsImmediatelyAddedToTheQueueIfAbsoluteExpiryIsInThePast() { final long NOW_MILLIS = 520_000L; final long BLOCK_TIME_STAMP = 500L; - when(mockForksSchedule.getFork(anyLong())) + when(mockForksSchedule.getFork(anyLong(), anyLong())) .thenReturn( new ForkSpec<>( 0, @@ -237,7 +237,7 @@ public void startTimerCancelsExistingTimer() { final long NOW_MILLIS = 500_000L; final long BLOCK_TIME_STAMP = 500L; - when(mockForksSchedule.getFork(anyLong())) + when(mockForksSchedule.getFork(anyLong(), anyLong())) .thenReturn( new ForkSpec<>( 0, @@ -271,7 +271,7 @@ public void runningFollowsTheStateOfTheTimer() { final long NOW_MILLIS = 500_000L; final long BLOCK_TIME_STAMP = 500L; - when(mockForksSchedule.getFork(anyLong())) + when(mockForksSchedule.getFork(anyLong(), anyLong())) .thenReturn( new ForkSpec<>( 0, @@ -313,7 +313,7 @@ public void checkBlockTimerEmptyAndNonEmptyPeriodSecods() { bftExecutors.scheduleTask(any(Runnable.class), anyLong(), any())) .thenReturn(mockedFuture); - when(mockForksSchedule.getFork(anyLong())) + when(mockForksSchedule.getFork(anyLong(), anyLong())) .thenReturn( new ForkSpec<>( 0, diff --git a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/IbftForksSchedulesFactoryTest.java b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/IbftForksSchedulesFactoryTest.java index 235ba89d323..a2ad857beec 100644 --- a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/IbftForksSchedulesFactoryTest.java +++ b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/IbftForksSchedulesFactoryTest.java @@ -56,7 +56,7 @@ public void createsScheduleWithForkThatOverridesGenesisValues() { final ForksSchedule forksSchedule = IbftForksSchedulesFactory.create(createGenesisConfig(configOptions, fork)); - assertThat(forksSchedule.getFork(0)) + assertThat(forksSchedule.getFork(0, 0)) .usingRecursiveComparison() .isEqualTo(new ForkSpec<>(0, configOptions)); @@ -69,8 +69,8 @@ public void createsScheduleWithForkThatOverridesGenesisValues() { new JsonBftConfigOptions(JsonUtil.objectNodeFromMap(forkOptions))); final ForkSpec expectedFork = new ForkSpec<>(1, expectedForkConfig); - assertThat(forksSchedule.getFork(1)).usingRecursiveComparison().isEqualTo(expectedFork); - assertThat(forksSchedule.getFork(2)).usingRecursiveComparison().isEqualTo(expectedFork); + assertThat(forksSchedule.getFork(1, 0)).usingRecursiveComparison().isEqualTo(expectedFork); + assertThat(forksSchedule.getFork(2, 0)).usingRecursiveComparison().isEqualTo(expectedFork); } @Override diff --git a/consensus/qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftBlockHeightManagerFactory.java b/consensus/qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftBlockHeightManagerFactory.java index b212909feb3..6f3a4932dd3 100644 --- a/consensus/qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftBlockHeightManagerFactory.java +++ b/consensus/qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftBlockHeightManagerFactory.java @@ -124,8 +124,7 @@ private BaseQbftBlockHeightManager createFullBlockHeightManager( finalState.getClock(), messageValidatorFactory, messageFactory, - validatorProvider, - true); + validatorProvider); } else { roundChangeManager = new RoundChangeManager( diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactory.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactory.java index fc46b6c32fb..5e6de291b69 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactory.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactory.java @@ -69,7 +69,10 @@ public QbftBlockCreatorFactory( @Override public Bytes createExtraData(final int round, final BlockHeader parentHeader) { - if (forksSchedule.getFork(parentHeader.getNumber() + 1L).getValue().isValidatorContractMode()) { + if (forksSchedule + .getFork(parentHeader.getNumber() + 1L, parentHeader.getTimestamp()) + .getValue() + .isValidatorContractMode()) { // vote and validators will come from contract instead of block final BftExtraData extraData = new BftExtraData( diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java index 40d139e7d2e..a2e1821c73c 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java @@ -63,27 +63,32 @@ public Collection
getValidatorsAtHead() { @Override public Collection
getValidatorsAfterBlock(final BlockHeader parentHeader) { final long nextBlock = parentHeader.getNumber() + 1; - return resolveValidatorProvider(nextBlock).getValidatorsAfterBlock(parentHeader); + return resolveValidatorProvider(nextBlock, parentHeader.getTimestamp()) + .getValidatorsAfterBlock(parentHeader); } @Override public Collection
getValidatorsForBlock(final BlockHeader header) { - return resolveValidatorProvider(header.getNumber()).getValidatorsForBlock(header); + return resolveValidatorProvider(header.getNumber(), header.getTimestamp()) + .getValidatorsForBlock(header); } @Override public Optional getVoteProviderAtHead() { - return resolveValidatorProvider(blockchain.getChainHeadHeader().getNumber()) + return resolveValidatorProvider( + blockchain.getChainHeadHeader().getNumber(), + blockchain.getChainHeadHeader().getTimestamp()) .getVoteProviderAtHead(); } @Override public Optional getVoteProviderAfterBlock(final BlockHeader header) { - return resolveValidatorProvider(header.getNumber() + 1).getVoteProviderAtHead(); + return resolveValidatorProvider(header.getNumber() + 1, header.getTimestamp()) + .getVoteProviderAtHead(); } - private ValidatorProvider resolveValidatorProvider(final long block) { - final ForkSpec fork = forksSchedule.getFork(block); + private ValidatorProvider resolveValidatorProvider(final long block, final long blockTimestamp) { + final ForkSpec fork = forksSchedule.getFork(block, blockTimestamp); return fork.getValue().isValidatorContractMode() ? transactionValidatorProvider : blockValidatorProvider; diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java index 569461a8647..1561059753f 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java @@ -67,13 +67,15 @@ public Collection
getValidatorsAfterBlock(final BlockHeader parentHeade // For the validator contract we determine the validators from the previous block but the // address from block about to be created final long nextBlock = parentHeader.getNumber() + 1; - final Address contractAddress = resolveContractAddress(nextBlock); + final long blockTimestamp = parentHeader.getTimestamp(); + final Address contractAddress = resolveContractAddress(nextBlock, blockTimestamp); return getValidatorsFromContract(afterBlockValidatorCache, parentHeader, contractAddress); } @Override public Collection
getValidatorsForBlock(final BlockHeader header) { - final Address contractAddress = resolveContractAddress(header.getNumber()); + final Address contractAddress = + resolveContractAddress(header.getNumber(), header.getTimestamp()); return getValidatorsFromContract(forBlockValidatorCache, header, contractAddress); } @@ -99,9 +101,9 @@ public Optional getVoteProviderAtHead() { return Optional.empty(); } - private Address resolveContractAddress(final long blockNumber) { + private Address resolveContractAddress(final long blockNumber, final long blockTimestamp) { return forksSchedule - .getFork(blockNumber) + .getFork(blockNumber, blockTimestamp) .getValue() .getValidatorContractAddress() .map(Address::fromHexString) diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLogger.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLogger.java index 83c18262c66..27bc82fd823 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLogger.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLogger.java @@ -63,9 +63,9 @@ public ValidatorModeTransitionLogger(final ForksSchedule fork */ public void logTransitionChange(final BlockHeader parentHeader) { final ForkSpec currentForkSpec = - forksSchedule.getFork(parentHeader.getNumber()); + forksSchedule.getFork(parentHeader.getNumber(), parentHeader.getTimestamp()); final ForkSpec nextForkSpec = - forksSchedule.getFork(parentHeader.getNumber() + 1L); + forksSchedule.getFork(parentHeader.getNumber() + 1L, parentHeader.getTimestamp()); final QbftConfigOptions currentConfigOptions = currentForkSpec.getValue(); final QbftConfigOptions nextConfigOptions = nextForkSpec.getValue(); diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactoryTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactoryTest.java index f3682e2533d..e2980ad0b64 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactoryTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactoryTest.java @@ -63,7 +63,7 @@ public void setUp() { qbftConfigOptions.setValidatorContractAddress(Optional.of("1")); final ForkSpec spec = new ForkSpec<>(0, qbftConfigOptions); final ForksSchedule forksSchedule = mock(ForksSchedule.class); - when(forksSchedule.getFork(anyLong())).thenReturn(spec); + when(forksSchedule.getFork(anyLong(), anyLong())).thenReturn(spec); qbftBlockCreatorFactory = new QbftBlockCreatorFactory( diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/QbftForksSchedulesFactoryTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/QbftForksSchedulesFactoryTest.java index 6213b6817d6..d38f7b65750 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/QbftForksSchedulesFactoryTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/QbftForksSchedulesFactoryTest.java @@ -74,7 +74,7 @@ public void createsScheduleWithForkThatOverridesGenesisValues() { final ForksSchedule forksSchedule = QbftForksSchedulesFactory.create(createGenesisConfig(configOptions, fork)); - assertThat(forksSchedule.getFork(0)) + assertThat(forksSchedule.getFork(0, 0)) .usingRecursiveComparison() .isEqualTo(new ForkSpec<>(0, configOptions)); @@ -89,8 +89,8 @@ public void createsScheduleWithForkThatOverridesGenesisValues() { new JsonQbftConfigOptions(JsonUtil.objectNodeFromMap(forkOptions))); final ForkSpec expectedFork = new ForkSpec<>(1, expectedForkConfig); - assertThat(forksSchedule.getFork(1)).usingRecursiveComparison().isEqualTo(expectedFork); - assertThat(forksSchedule.getFork(2)).usingRecursiveComparison().isEqualTo(expectedFork); + assertThat(forksSchedule.getFork(1, 0)).usingRecursiveComparison().isEqualTo(expectedFork); + assertThat(forksSchedule.getFork(2, 0)).usingRecursiveComparison().isEqualTo(expectedFork); } @Test @@ -136,7 +136,7 @@ public void switchingToBlockHeaderRemovesValidatorContractAddress() { final ForksSchedule forksSchedule = QbftForksSchedulesFactory.create(createGenesisConfig(configOptions, fork)); - assertThat(forksSchedule.getFork(1).getValue().getValidatorContractAddress()).isEmpty(); + assertThat(forksSchedule.getFork(1, 0).getValue().getValidatorContractAddress()).isEmpty(); } @Test diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLoggerTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLoggerTest.java index dabf26749a5..afb41b01960 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLoggerTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ValidatorModeTransitionLoggerTest.java @@ -53,8 +53,8 @@ public void doNotLogMessageWhenTransitioningFromBlockHeaderToBlockHeader() { final ForkSpec forkSpecB = new ForkSpec<>(1, createQbftConfigOptionsForBlockHeader()); - when(forksSchedule.getFork(0)).thenReturn(forkSpecA); - when(forksSchedule.getFork(1)).thenReturn(forkSpecB); + when(forksSchedule.getFork(0, 0)).thenReturn(forkSpecA); + when(forksSchedule.getFork(1, 0)).thenReturn(forkSpecB); qbftTransitionNotifier.logTransitionChange(blockHeader(0)); @@ -68,8 +68,8 @@ public void doNotLogMessageWhenTransitioningFromContractToContractWithSameAddres final ForkSpec contractForkSpecB = new ForkSpec<>(1, createQbftConfigOptionsForContract("0x0")); - when(forksSchedule.getFork(0)).thenReturn(contractForkSpecA); - when(forksSchedule.getFork(1)).thenReturn(contractForkSpecB); + when(forksSchedule.getFork(0, 0)).thenReturn(contractForkSpecA); + when(forksSchedule.getFork(1, 0)).thenReturn(contractForkSpecB); qbftTransitionNotifier.logTransitionChange(blockHeader(0)); @@ -83,8 +83,8 @@ public void logMessageWhenTransitioningFromContractToContractWithDifferentAddres final ForkSpec contractForkSpecB = new ForkSpec<>(1, createQbftConfigOptionsForContract("0x1")); - when(forksSchedule.getFork(0)).thenReturn(contractForkSpecA); - when(forksSchedule.getFork(1)).thenReturn(contractForkSpecB); + when(forksSchedule.getFork(0, 0)).thenReturn(contractForkSpecA); + when(forksSchedule.getFork(1, 0)).thenReturn(contractForkSpecB); qbftTransitionNotifier.logTransitionChange(blockHeader(0)); @@ -100,8 +100,8 @@ public void logMessageWhenTransitioningFromContractToBlockHeader() { final ForkSpec blockForkSpec = new ForkSpec<>(1, createQbftConfigOptionsForBlockHeader()); - when(forksSchedule.getFork(0)).thenReturn(contractForkSpec); - when(forksSchedule.getFork(1)).thenReturn(blockForkSpec); + when(forksSchedule.getFork(0, 0)).thenReturn(contractForkSpec); + when(forksSchedule.getFork(1, 0)).thenReturn(blockForkSpec); qbftTransitionNotifier.logTransitionChange(blockHeader(0)); @@ -117,8 +117,8 @@ public void logMessageWhenTransitioningFromBlockHeaderToContract() { final ForkSpec contractForkSpec = new ForkSpec<>(1, createQbftConfigOptionsForContract("0x0")); - when(forksSchedule.getFork(0)).thenReturn(blockForkSpec); - when(forksSchedule.getFork(1)).thenReturn(contractForkSpec); + when(forksSchedule.getFork(0, 0)).thenReturn(blockForkSpec); + when(forksSchedule.getFork(1, 0)).thenReturn(contractForkSpec); qbftTransitionNotifier.logTransitionChange(blockHeader(0)); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java index 5b2fb59bf7f..58c4aaa54bc 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ScheduledProtocolSpec.java @@ -21,6 +21,12 @@ * Knows how to query the timestamp or block number of a given block header */ public interface ScheduledProtocolSpec { + + enum ScheduleType { + BLOCK, + TIME + } + boolean isOnOrAfterMilestoneBoundary( org.hyperledger.besu.plugin.data.ProcessableBlockHeader header);