From 067d279c28483977cf7f37d73868034b428bdac3 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Tue, 5 Apr 2022 20:44:45 -0700 Subject: [PATCH 1/5] initial check in of backward sync re-org below TTD fix Signed-off-by: garyschulte --- .../MergeBesuControllerBuilder.java | 30 ++++++++-- .../TransitionBesuControllerBuilder.java | 32 ++++++++-- .../TransitionControllerBuilderTest.java | 40 +++++++++++-- .../consensus/merge/PostMergeContext.java | 5 +- .../merge/TransitionBackwardSyncContext.java | 60 +++++++++++++++++++ .../merge/TransitionProtocolSchedule.java | 32 ++++++++++ .../besu/consensus/merge/TransitionUtils.java | 2 +- .../backwardsync/BackwardSyncContext.java | 6 +- .../sync/backwardsync/ForwardSyncPhase.java | 2 +- 9 files changed, 187 insertions(+), 22 deletions(-) create mode 100644 consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java index 7b3faa1f66f..2e578e09659 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java @@ -55,14 +55,13 @@ protected MiningCoordinator createMiningCoordinator( final MiningParameters miningParameters, final SyncState syncState, final EthProtocolManager ethProtocolManager) { - - this.syncState.set(syncState); - - return new MergeCoordinator( - protocolContext, + return createTransitionMiningCoordinator( protocolSchedule, - transactionPool.getPendingTransactions(), + protocolContext, + transactionPool, miningParameters, + syncState, + ethProtocolManager, new BackwardSyncContext( protocolContext, protocolSchedule, @@ -74,6 +73,25 @@ protected MiningCoordinator createMiningCoordinator( storageProvider)); } + protected MiningCoordinator createTransitionMiningCoordinator( + final ProtocolSchedule protocolSchedule, + final ProtocolContext protocolContext, + final TransactionPool transactionPool, + final MiningParameters miningParameters, + final SyncState syncState, + final EthProtocolManager ethProtocolManager, + final BackwardSyncContext backwardSyncContext) { + + this.syncState.set(syncState); + + return new MergeCoordinator( + protocolContext, + protocolSchedule, + transactionPool.getPendingTransactions(), + miningParameters, + backwardSyncContext); + } + @Override protected ProtocolSchedule createProtocolSchedule() { return MergeProtocolSchedule.create( diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java index d0a08ad5e71..e4e596bb48f 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java @@ -16,6 +16,7 @@ import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.consensus.merge.PostMergeContext; +import org.hyperledger.besu.consensus.merge.TransitionBackwardSyncContext; import org.hyperledger.besu.consensus.merge.TransitionContext; import org.hyperledger.besu.consensus.merge.TransitionProtocolSchedule; import org.hyperledger.besu.consensus.merge.blockcreation.TransitionCoordinator; @@ -32,6 +33,8 @@ import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; +import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext; +import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncLookupService; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; @@ -79,29 +82,46 @@ protected MiningCoordinator createMiningCoordinator( final EthProtocolManager ethProtocolManager) { // cast to transition schedule for explicit access to pre and post objects: - final TransitionProtocolSchedule tps = (TransitionProtocolSchedule) protocolSchedule; + final TransitionProtocolSchedule transitionProtocolSchedule = + (TransitionProtocolSchedule) protocolSchedule; // PoA consensus mines by default, get consensus-specific mining parameters for // TransitionCoordinator: MiningParameters transitionMiningParameters = preMergeBesuControllerBuilder.getMiningParameterOverrides(miningParameters); + // construct a transition backward sync context + BackwardSyncContext transitionBackwardsSyncContext = + new TransitionBackwardSyncContext( + protocolContext, + transitionProtocolSchedule, + metricsSystem, + ethProtocolManager.ethContext(), + syncState, + new BackwardSyncLookupService( + transitionProtocolSchedule, + ethProtocolManager.ethContext(), + metricsSystem, + protocolContext), + storageProvider); + final TransitionCoordinator composedCoordinator = new TransitionCoordinator( preMergeBesuControllerBuilder.createMiningCoordinator( - tps.getPreMergeSchedule(), + transitionProtocolSchedule.getPreMergeSchedule(), protocolContext, transactionPool, new MiningParameters.Builder(miningParameters).miningEnabled(false).build(), syncState, ethProtocolManager), - mergeBesuControllerBuilder.createMiningCoordinator( - tps.getPostMergeSchedule(), + mergeBesuControllerBuilder.createTransitionMiningCoordinator( + transitionProtocolSchedule, protocolContext, transactionPool, transitionMiningParameters, syncState, - ethProtocolManager)); + ethProtocolManager, + transitionBackwardsSyncContext)); initTransitionWatcher(protocolContext, composedCoordinator); return composedCoordinator; } @@ -147,7 +167,7 @@ private void initTransitionWatcher( .setBlockChoiceRule((newBlockHeader, currentBlockHeader) -> -1); } else if (composedCoordinator.isMiningBeforeMerge()) { - // if our merge state is set to pre-merge and we are mining, start mining + // if our merge state is set to mine pre-merge and we are mining, start mining composedCoordinator.getPreMergeObject().enable(); composedCoordinator.getPreMergeObject().start(); } diff --git a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java index 83e5699ddbf..829967a2ce0 100644 --- a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java +++ b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java @@ -16,21 +16,26 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import org.hyperledger.besu.consensus.clique.CliqueContext; +import org.hyperledger.besu.consensus.merge.MergeContext; import org.hyperledger.besu.consensus.merge.PostMergeContext; import org.hyperledger.besu.consensus.merge.TransitionProtocolSchedule; import org.hyperledger.besu.consensus.merge.blockcreation.TransitionCoordinator; import org.hyperledger.besu.crypto.NodeKeyUtils; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import java.util.Optional; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,28 +50,32 @@ @RunWith(MockitoJUnitRunner.class) public class TransitionControllerBuilderTest { - @Mock ProtocolSchedule protocolSchedule; - @Mock TransitionProtocolSchedule transitionProtocolSchedule; + @Mock ProtocolSchedule preMergeProtocolSchedule; + @Mock ProtocolSchedule postMergeProtocolSchedule; @Mock ProtocolContext protocolContext; @Mock TransactionPool transactionPool; @Mock SyncState syncState; @Mock EthProtocolManager ethProtocolManager; + @Mock PostMergeContext mergeContext; @Spy CliqueBesuControllerBuilder cliqueBuilder = new CliqueBesuControllerBuilder(); @Spy BesuControllerBuilder powBuilder = new MainnetBesuControllerBuilder(); @Spy MergeBesuControllerBuilder postMergeBuilder = new MergeBesuControllerBuilder(); @Spy MiningParameters miningParameters = new MiningParameters.Builder().build(); + TransitionProtocolSchedule transitionProtocolSchedule = spy(new TransitionProtocolSchedule( + preMergeProtocolSchedule, postMergeProtocolSchedule)); + @Before public void setup() { cliqueBuilder.nodeKey(NodeKeyUtils.generate()); when(protocolContext.getBlockchain()).thenReturn(mock(MutableBlockchain.class)); - when(transitionProtocolSchedule.getPostMergeSchedule()).thenReturn(protocolSchedule); - when(transitionProtocolSchedule.getPreMergeSchedule()).thenReturn(protocolSchedule); + when(transitionProtocolSchedule.getPostMergeSchedule()).thenReturn(postMergeProtocolSchedule); + when(transitionProtocolSchedule.getPreMergeSchedule()).thenReturn(preMergeProtocolSchedule); when(protocolContext.getConsensusContext(CliqueContext.class)) .thenReturn(mock(CliqueContext.class)); when(protocolContext.getConsensusContext(PostMergeContext.class)) - .thenReturn(mock(PostMergeContext.class)); + .thenReturn(mergeContext); } @Test @@ -90,6 +99,27 @@ public void assertPowMiningPreMerge() { assertThat(transCoordinator.isMiningBeforeMerge()).isTrue(); } + @Test + public void assertPreMergeScheduleForBlockWhenPostMergeIfNotFinalized() { + + } + + @Test + public void assertPostMergeScheduleForBlockWhenFinalized() { + + } + + @Test + public void assertPostMergeScheduleForAnyBlockWhenPostMergeAndFinalized() { + + //TODO: spy/plumb mergeContext in transitionprotocolschedule + var mockBlock = new BlockHeaderTestFixture().buildHeader(); + when(mergeContext.isPostMerge()).thenReturn(Boolean.TRUE); + when(mergeContext.getFinalized()).thenReturn(Optional.of(mockBlock)); + assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock)) + .isEqualTo(transitionProtocolSchedule.getPostMergeSchedule()); + } + TransitionCoordinator buildTransitionCoordinator( final BesuControllerBuilder preMerge, final MergeBesuControllerBuilder postMerge) { var builder = new TransitionBesuControllerBuilder(preMerge, postMerge); diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java index 22cdc2201a9..6dc8a652113 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java @@ -80,8 +80,9 @@ public PostMergeContext setTerminalTotalDifficulty(final Difficulty newTerminalT @Override public void setIsPostMerge(final Difficulty totalDifficulty) { - if (isPostMerge.get().orElse(Boolean.FALSE)) { - // we never switch back to a pre-merge once we have transitioned post-TTD. + if (isPostMerge.get().orElse(Boolean.FALSE) && lastFinalized.get() != null) { + // if we have finalized, we never switch back to a pre-merge once we have transitioned + // post-TTD. return; } final boolean newState = terminalTotalDifficulty.get().lessOrEqualThan(totalDifficulty); diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java new file mode 100644 index 00000000000..175d31e3f0b --- /dev/null +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java @@ -0,0 +1,60 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * 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.consensus.merge; + +import org.hyperledger.besu.ethereum.BlockValidator; +import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.core.Block; +import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext; +import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncLookupService; +import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; +import org.hyperledger.besu.ethereum.storage.StorageProvider; +import org.hyperledger.besu.plugin.services.MetricsSystem; + +public class TransitionBackwardSyncContext extends BackwardSyncContext { + + private final TransitionProtocolSchedule transitionProtocolSchedule; + + public TransitionBackwardSyncContext( + final ProtocolContext protocolContext, + final TransitionProtocolSchedule transitionProtocolSchedule, + final MetricsSystem metricsSystem, + final EthContext ethContext, + final SyncState syncState, + final BackwardSyncLookupService backwardSyncLookupService, + final StorageProvider storageProvider) { + super( + protocolContext, + transitionProtocolSchedule, + metricsSystem, + ethContext, + syncState, + backwardSyncLookupService, + storageProvider); + this.transitionProtocolSchedule = transitionProtocolSchedule; + } + + /** + * Choose the correct protocolSchedule and blockvalidator by block rather than number. + * This should be used in the merge transition, specifically when the chain has not + * yet finalized. + */ + @Override + public BlockValidator getBlockValidatorForBlock(final Block block) { + return transitionProtocolSchedule.getByBlockHeader(protocolContext, block.getHeader()) + .getBlockValidator(); + } +} diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java index 0a2dc5e01db..d9ec40a5f9d 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java @@ -14,6 +14,10 @@ */ package org.hyperledger.besu.consensus.merge; +import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.chain.MutableBlockchain; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.TransactionFilter; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; @@ -40,6 +44,34 @@ public ProtocolSchedule getPostMergeSchedule() { return getPostMergeObject(); } + public ProtocolSpec getByBlockHeader(final ProtocolContext protocolContext, final BlockHeader blockHeader) { + // if we do not have a finalized block we might return pre or post merge protocol schedule: + if (mergeContext.getFinalized().isEmpty()) { + + // if head is not post-merge, return pre-merge schedule: + if (!mergeContext.isPostMerge()) { + return getPreMergeSchedule().getByBlockNumber(blockHeader.getNumber()); + } + + // otherwise check to see if this block represents a re-org below TTD: + MutableBlockchain blockchain = protocolContext.getBlockchain(); + Difficulty parentDifficulty = + blockchain.getTotalDifficultyByHash(blockHeader.getParentHash()).orElseThrow(); + Difficulty thisDifficulty = parentDifficulty.add(blockHeader.getDifficulty()); + Difficulty terminalDifficulty = mergeContext.getTerminalTotalDifficulty(); + + // if this block is pre-merge + if (thisDifficulty.lessOrEqualThan(terminalDifficulty) + || TransitionUtils.isTerminalProofOfWorkBlock(blockHeader, protocolContext)) { + return getPreMergeSchedule() + .getByBlockNumber(blockHeader.getNumber()); + } + } + + // else return post-merge schedule + return getPostMergeSchedule().getByBlockNumber(blockHeader.getNumber()); + } + @Override public ProtocolSpec getByBlockNumber(final long number) { return dispatchFunctionAccordingToMergeState( diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java index e3ef8d5ffe7..79639aaf154 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java @@ -30,7 +30,7 @@ public abstract class TransitionUtils { private static final Logger LOG = LoggerFactory.getLogger(TransitionUtils.class); - private final MergeContext mergeContext = PostMergeContext.get(); + protected final MergeContext mergeContext = PostMergeContext.get(); private final SwitchingObject preMergeObject; private final SwitchingObject postMergeObject; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java index b1ac2a3f232..acf97118e03 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java @@ -46,7 +46,7 @@ public class BackwardSyncContext { public static final int BATCH_SIZE = 200; private static final int MAX_RETRIES = 100; - private final ProtocolContext protocolContext; + protected final ProtocolContext protocolContext; private final ProtocolSchedule protocolSchedule; private final EthContext ethContext; private final MetricsSystem metricsSystem; @@ -257,6 +257,10 @@ public BlockValidator getBlockValidator(final long blockNumber) { return protocolSchedule.getByBlockNumber(blockNumber).getBlockValidator(); } + public BlockValidator getBlockValidatorForBlock(final Block block) { + return getBlockValidator(block.getHeader().getNumber()); + } + public Optional findCorrectChainFromPivot(final long number) { return Optional.ofNullable(backwardChainMap.get(number)); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhase.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhase.java index 2a9debfd350..6bf708636bc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhase.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhase.java @@ -170,7 +170,7 @@ protected Void saveBlock(final Block block) { debugLambda(LOG, "Going to validate block {}", () -> block.getHeader().getHash().toHexString()); var optResult = context - .getBlockValidator(block.getHeader().getNumber()) + .getBlockValidatorForBlock(block) .validateAndProcessBlock( context.getProtocolContext(), block, From 7beeb30d89aa3b85cb8a9a27a054ad7de31d5e37 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Thu, 7 Apr 2022 14:16:49 -0700 Subject: [PATCH 2/5] pre-finalization transition protocol test coverage Signed-off-by: garyschulte --- .../TransitionControllerBuilderTest.java | 62 ++++++++++++++----- .../merge/TransitionProtocolSchedule.java | 8 ++- .../besu/consensus/merge/TransitionUtils.java | 10 ++- 3 files changed, 64 insertions(+), 16 deletions(-) diff --git a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java index 829967a2ce0..3f330bb11db 100644 --- a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java +++ b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java @@ -15,12 +15,13 @@ package org.hyperledger.besu.controller; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import org.hyperledger.besu.consensus.clique.CliqueContext; -import org.hyperledger.besu.consensus.merge.MergeContext; import org.hyperledger.besu.consensus.merge.PostMergeContext; import org.hyperledger.besu.consensus.merge.TransitionProtocolSchedule; import org.hyperledger.besu.consensus.merge.blockcreation.TransitionCoordinator; @@ -28,11 +29,13 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; +import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; import java.util.Optional; @@ -53,6 +56,7 @@ public class TransitionControllerBuilderTest { @Mock ProtocolSchedule preMergeProtocolSchedule; @Mock ProtocolSchedule postMergeProtocolSchedule; @Mock ProtocolContext protocolContext; + @Mock MutableBlockchain mockBlockchain; @Mock TransactionPool transactionPool; @Mock SyncState syncState; @Mock EthProtocolManager ethProtocolManager; @@ -63,13 +67,14 @@ public class TransitionControllerBuilderTest { @Spy MergeBesuControllerBuilder postMergeBuilder = new MergeBesuControllerBuilder(); @Spy MiningParameters miningParameters = new MiningParameters.Builder().build(); - TransitionProtocolSchedule transitionProtocolSchedule = spy(new TransitionProtocolSchedule( - preMergeProtocolSchedule, postMergeProtocolSchedule)); + TransitionProtocolSchedule transitionProtocolSchedule; @Before public void setup() { + transitionProtocolSchedule = spy(new TransitionProtocolSchedule( + preMergeProtocolSchedule, postMergeProtocolSchedule, mergeContext)); cliqueBuilder.nodeKey(NodeKeyUtils.generate()); - when(protocolContext.getBlockchain()).thenReturn(mock(MutableBlockchain.class)); + when(protocolContext.getBlockchain()).thenReturn(mockBlockchain); when(transitionProtocolSchedule.getPostMergeSchedule()).thenReturn(postMergeProtocolSchedule); when(transitionProtocolSchedule.getPreMergeSchedule()).thenReturn(preMergeProtocolSchedule); when(protocolContext.getConsensusContext(CliqueContext.class)) @@ -100,24 +105,53 @@ public void assertPowMiningPreMerge() { } @Test - public void assertPreMergeScheduleForBlockWhenPostMergeIfNotFinalized() { - - } - - @Test - public void assertPostMergeScheduleForBlockWhenFinalized() { - + public void assertPreMergeScheduleForNotPostMerge() { + var mockBlock = new BlockHeaderTestFixture().buildHeader(); + var preMergeProtocolSpec = mock(ProtocolSpec.class); + when(mergeContext.isPostMerge()).thenReturn(Boolean.FALSE); + when(mergeContext.getFinalized()).thenReturn(Optional.empty()); + when(preMergeProtocolSchedule.getByBlockNumber(anyLong())) + .thenReturn(preMergeProtocolSpec); + assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock)) + .isEqualTo(preMergeProtocolSpec); } @Test public void assertPostMergeScheduleForAnyBlockWhenPostMergeAndFinalized() { - - //TODO: spy/plumb mergeContext in transitionprotocolschedule var mockBlock = new BlockHeaderTestFixture().buildHeader(); + var postMergeProtocolSpec = mock(ProtocolSpec.class); when(mergeContext.isPostMerge()).thenReturn(Boolean.TRUE); when(mergeContext.getFinalized()).thenReturn(Optional.of(mockBlock)); + when(postMergeProtocolSchedule.getByBlockNumber(anyLong())) + .thenReturn(postMergeProtocolSpec); + assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock)) + .isEqualTo(postMergeProtocolSpec); + } + + + @Test + public void assertPreMergeScheduleForBelowTerminalBlockWhenPostMergeIfNotFinalized() { + var mockParentBlock = new BlockHeaderTestFixture() + .number(100) + .buildHeader(); + var mockBlock = new BlockHeaderTestFixture() + .number(101) + .difficulty(Difficulty.of(1L)) + .parentHash(mockParentBlock.getHash()) + .buildHeader(); + + var preMergeProtocolSpec = mock(ProtocolSpec.class); + + when(mergeContext.getTerminalTotalDifficulty()).thenReturn(Difficulty.of(1337L)); + when(mergeContext.isPostMerge()).thenReturn(Boolean.TRUE); + when(mergeContext.getFinalized()).thenReturn(Optional.empty()); + when(mockBlockchain.getTotalDifficultyByHash(any())) + .thenReturn(Optional.of(Difficulty.of(1335L))); + + when(preMergeProtocolSchedule.getByBlockNumber(anyLong())) + .thenReturn(preMergeProtocolSpec); assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock)) - .isEqualTo(transitionProtocolSchedule.getPostMergeSchedule()); + .isEqualTo(preMergeProtocolSpec); } TransitionCoordinator buildTransitionCoordinator( diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java index d9ec40a5f9d..28f0270b2d0 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java @@ -36,6 +36,13 @@ public TransitionProtocolSchedule( super(preMergeProtocolSchedule, postMergeProtocolSchedule); } + public TransitionProtocolSchedule( + final ProtocolSchedule preMergeProtocolSchedule, + final ProtocolSchedule postMergeProtocolSchedule, + final MergeContext mergeContext) { + super(preMergeProtocolSchedule, postMergeProtocolSchedule, mergeContext); + } + public ProtocolSchedule getPreMergeSchedule() { return getPreMergeObject(); } @@ -67,7 +74,6 @@ public ProtocolSpec getByBlockHeader(final ProtocolContext protocolContext, fina .getByBlockNumber(blockHeader.getNumber()); } } - // else return post-merge schedule return getPostMergeSchedule().getByBlockNumber(blockHeader.getNumber()); } diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java index 79639aaf154..0560711ae13 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionUtils.java @@ -30,14 +30,22 @@ public abstract class TransitionUtils { private static final Logger LOG = LoggerFactory.getLogger(TransitionUtils.class); - protected final MergeContext mergeContext = PostMergeContext.get(); + protected final MergeContext mergeContext; private final SwitchingObject preMergeObject; private final SwitchingObject postMergeObject; public TransitionUtils( final SwitchingObject preMergeObject, final SwitchingObject postMergeObject) { + this(preMergeObject, postMergeObject, PostMergeContext.get()); + } + + public TransitionUtils( + final SwitchingObject preMergeObject, + final SwitchingObject postMergeObject, + final MergeContext mergeContext) { this.preMergeObject = preMergeObject; this.postMergeObject = postMergeObject; + this.mergeContext = mergeContext; } protected void dispatchConsumerAccordingToMergeState(final Consumer consumer) { From 5b9230e477a67e9268ab3b1b459b3894410f9d5b Mon Sep 17 00:00:00 2001 From: garyschulte Date: Thu, 7 Apr 2022 14:35:37 -0700 Subject: [PATCH 3/5] spotless and fix unnecessary mockito stubbing Signed-off-by: garyschulte --- .../TransitionControllerBuilderTest.java | 35 ++++++++----------- .../merge/TransitionBackwardSyncContext.java | 8 ++--- .../merge/TransitionProtocolSchedule.java | 6 ++-- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java index 3f330bb11db..89a2d559617 100644 --- a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java +++ b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java @@ -71,16 +71,17 @@ public class TransitionControllerBuilderTest { @Before public void setup() { - transitionProtocolSchedule = spy(new TransitionProtocolSchedule( - preMergeProtocolSchedule, postMergeProtocolSchedule, mergeContext)); + transitionProtocolSchedule = + spy( + new TransitionProtocolSchedule( + preMergeProtocolSchedule, postMergeProtocolSchedule, mergeContext)); cliqueBuilder.nodeKey(NodeKeyUtils.generate()); when(protocolContext.getBlockchain()).thenReturn(mockBlockchain); when(transitionProtocolSchedule.getPostMergeSchedule()).thenReturn(postMergeProtocolSchedule); when(transitionProtocolSchedule.getPreMergeSchedule()).thenReturn(preMergeProtocolSchedule); when(protocolContext.getConsensusContext(CliqueContext.class)) .thenReturn(mock(CliqueContext.class)); - when(protocolContext.getConsensusContext(PostMergeContext.class)) - .thenReturn(mergeContext); + when(protocolContext.getConsensusContext(PostMergeContext.class)).thenReturn(mergeContext); } @Test @@ -110,8 +111,7 @@ public void assertPreMergeScheduleForNotPostMerge() { var preMergeProtocolSpec = mock(ProtocolSpec.class); when(mergeContext.isPostMerge()).thenReturn(Boolean.FALSE); when(mergeContext.getFinalized()).thenReturn(Optional.empty()); - when(preMergeProtocolSchedule.getByBlockNumber(anyLong())) - .thenReturn(preMergeProtocolSpec); + when(preMergeProtocolSchedule.getByBlockNumber(anyLong())).thenReturn(preMergeProtocolSpec); assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock)) .isEqualTo(preMergeProtocolSpec); } @@ -120,25 +120,21 @@ public void assertPreMergeScheduleForNotPostMerge() { public void assertPostMergeScheduleForAnyBlockWhenPostMergeAndFinalized() { var mockBlock = new BlockHeaderTestFixture().buildHeader(); var postMergeProtocolSpec = mock(ProtocolSpec.class); - when(mergeContext.isPostMerge()).thenReturn(Boolean.TRUE); when(mergeContext.getFinalized()).thenReturn(Optional.of(mockBlock)); - when(postMergeProtocolSchedule.getByBlockNumber(anyLong())) - .thenReturn(postMergeProtocolSpec); + when(postMergeProtocolSchedule.getByBlockNumber(anyLong())).thenReturn(postMergeProtocolSpec); assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock)) .isEqualTo(postMergeProtocolSpec); } - @Test public void assertPreMergeScheduleForBelowTerminalBlockWhenPostMergeIfNotFinalized() { - var mockParentBlock = new BlockHeaderTestFixture() - .number(100) - .buildHeader(); - var mockBlock = new BlockHeaderTestFixture() - .number(101) - .difficulty(Difficulty.of(1L)) - .parentHash(mockParentBlock.getHash()) - .buildHeader(); + var mockParentBlock = new BlockHeaderTestFixture().number(100).buildHeader(); + var mockBlock = + new BlockHeaderTestFixture() + .number(101) + .difficulty(Difficulty.of(1L)) + .parentHash(mockParentBlock.getHash()) + .buildHeader(); var preMergeProtocolSpec = mock(ProtocolSpec.class); @@ -148,8 +144,7 @@ public void assertPreMergeScheduleForBelowTerminalBlockWhenPostMergeIfNotFinaliz when(mockBlockchain.getTotalDifficultyByHash(any())) .thenReturn(Optional.of(Difficulty.of(1335L))); - when(preMergeProtocolSchedule.getByBlockNumber(anyLong())) - .thenReturn(preMergeProtocolSpec); + when(preMergeProtocolSchedule.getByBlockNumber(anyLong())).thenReturn(preMergeProtocolSpec); assertThat(transitionProtocolSchedule.getByBlockHeader(protocolContext, mockBlock)) .isEqualTo(preMergeProtocolSpec); } diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java index 175d31e3f0b..819644d1303 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionBackwardSyncContext.java @@ -48,13 +48,13 @@ public TransitionBackwardSyncContext( } /** - * Choose the correct protocolSchedule and blockvalidator by block rather than number. - * This should be used in the merge transition, specifically when the chain has not - * yet finalized. + * Choose the correct protocolSchedule and blockvalidator by block rather than number. This should + * be used in the merge transition, specifically when the chain has not yet finalized. */ @Override public BlockValidator getBlockValidatorForBlock(final Block block) { - return transitionProtocolSchedule.getByBlockHeader(protocolContext, block.getHeader()) + return transitionProtocolSchedule + .getByBlockHeader(protocolContext, block.getHeader()) .getBlockValidator(); } } diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java index 28f0270b2d0..95495c3a056 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java @@ -51,7 +51,8 @@ public ProtocolSchedule getPostMergeSchedule() { return getPostMergeObject(); } - public ProtocolSpec getByBlockHeader(final ProtocolContext protocolContext, final BlockHeader blockHeader) { + public ProtocolSpec getByBlockHeader( + final ProtocolContext protocolContext, final BlockHeader blockHeader) { // if we do not have a finalized block we might return pre or post merge protocol schedule: if (mergeContext.getFinalized().isEmpty()) { @@ -70,8 +71,7 @@ public ProtocolSpec getByBlockHeader(final ProtocolContext protocolContext, fina // if this block is pre-merge if (thisDifficulty.lessOrEqualThan(terminalDifficulty) || TransitionUtils.isTerminalProofOfWorkBlock(blockHeader, protocolContext)) { - return getPreMergeSchedule() - .getByBlockNumber(blockHeader.getNumber()); + return getPreMergeSchedule().getByBlockNumber(blockHeader.getNumber()); } } // else return post-merge schedule From a464aeec2c60982b7f66f77883e6c51e3e3843c2 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Thu, 7 Apr 2022 15:44:57 -0700 Subject: [PATCH 4/5] fix mock in ForwardSyncPhaseTest Signed-off-by: garyschulte --- .../ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java index c5f34d2a47d..940b686923e 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java @@ -114,7 +114,7 @@ public void setup() { EthContext ethContext = ethProtocolManager.ethContext(); when(context.getEthContext()).thenReturn(ethContext); - when(context.getBlockValidator(anyLong()).validateAndProcessBlock(any(), any(), any(), any())) + when(context.getBlockValidatorForBlock(any()).validateAndProcessBlock(any(), any(), any(), any())) .thenAnswer( invocation -> { final Object[] arguments = invocation.getArguments(); From 57f5dcb490fea6608c20c36e9508ece27ecf28a5 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Thu, 7 Apr 2022 16:06:49 -0700 Subject: [PATCH 5/5] spotless again Signed-off-by: garyschulte --- .../ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java index 940b686923e..7c58d57226b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncPhaseTest.java @@ -20,7 +20,6 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -114,7 +113,9 @@ public void setup() { EthContext ethContext = ethProtocolManager.ethContext(); when(context.getEthContext()).thenReturn(ethContext); - when(context.getBlockValidatorForBlock(any()).validateAndProcessBlock(any(), any(), any(), any())) + when(context + .getBlockValidatorForBlock(any()) + .validateAndProcessBlock(any(), any(), any(), any())) .thenAnswer( invocation -> { final Object[] arguments = invocation.getArguments();