Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge: Backward sync re-org below TTD fix #3696

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
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;
Expand All @@ -25,11 +28,16 @@
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.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;

import org.junit.Before;
import org.junit.Test;
Expand All @@ -45,28 +53,35 @@
@RunWith(MockitoJUnitRunner.class)
public class TransitionControllerBuilderTest {

@Mock ProtocolSchedule protocolSchedule;
@Mock TransitionProtocolSchedule transitionProtocolSchedule;
@Mock ProtocolSchedule preMergeProtocolSchedule;
@Mock ProtocolSchedule postMergeProtocolSchedule;
@Mock ProtocolContext protocolContext;
@Mock MutableBlockchain mockBlockchain;
@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;

@Before
public void setup() {
transitionProtocolSchedule =
spy(
new TransitionProtocolSchedule(
preMergeProtocolSchedule, postMergeProtocolSchedule, mergeContext));
cliqueBuilder.nodeKey(NodeKeyUtils.generate());
when(protocolContext.getBlockchain()).thenReturn(mock(MutableBlockchain.class));
when(transitionProtocolSchedule.getPostMergeSchedule()).thenReturn(protocolSchedule);
when(transitionProtocolSchedule.getPreMergeSchedule()).thenReturn(protocolSchedule);
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(mock(PostMergeContext.class));
when(protocolContext.getConsensusContext(PostMergeContext.class)).thenReturn(mergeContext);
}

@Test
Expand All @@ -90,6 +105,50 @@ public void assertPowMiningPreMerge() {
assertThat(transCoordinator.isMiningBeforeMerge()).isTrue();
}

@Test
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() {
var mockBlock = new BlockHeaderTestFixture().buildHeader();
var postMergeProtocolSpec = mock(ProtocolSpec.class);
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(preMergeProtocolSpec);
}

TransitionCoordinator buildTransitionCoordinator(
final BesuControllerBuilder preMerge, final MergeBesuControllerBuilder postMerge) {
var builder = new TransitionBesuControllerBuilder(preMerge, postMerge);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,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();
}
Expand All @@ -40,6 +51,33 @@ 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(
Expand Down
Loading