Skip to content

Commit

Permalink
Remove duplication between Qbft protocol schedule and Ibft protocol s…
Browse files Browse the repository at this point in the history 
…chedule (hyperledger#2751)

Signed-off-by: Jason Frame <jasonwframe@gmail.com>
  • Loading branch information
@jframe
jframe authored Sep 23, 2021
1 parent fd961d5 commit 2541b15
Show file tree
Hide file tree
Showing 12 changed files with 181 additions and 248 deletions.
6 changes: 2 additions & 4 deletions besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.hyperledger.besu.consensus.common.bft.BftExecutors;
import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
import org.hyperledger.besu.consensus.common.bft.BftProcessor;
import org.hyperledger.besu.consensus.common.bft.BftProtocolSchedule;
import org.hyperledger.besu.consensus.common.bft.BlockTimer;
import org.hyperledger.besu.consensus.common.bft.EthSynchronizerUpdater;
import org.hyperledger.besu.consensus.common.bft.EventMultiplexer;
Expand All @@ -41,9 +40,9 @@
import org.hyperledger.besu.consensus.common.bft.statemachine.FutureMessageBuffer;
import org.hyperledger.besu.consensus.common.validator.ValidatorProvider;
import org.hyperledger.besu.consensus.common.validator.blockbased.BlockValidatorProvider;
import org.hyperledger.besu.consensus.ibft.IbftBlockHeaderValidationRulesetFactory;
import org.hyperledger.besu.consensus.ibft.IbftExtraDataCodec;
import org.hyperledger.besu.consensus.ibft.IbftGossip;
import org.hyperledger.besu.consensus.ibft.IbftProtocolSchedule;
import org.hyperledger.besu.consensus.ibft.jsonrpc.IbftJsonRpcMethods;
import org.hyperledger.besu.consensus.ibft.payload.MessageFactory;
import org.hyperledger.besu.consensus.ibft.protocol.IbftSubProtocol;
Expand Down Expand Up @@ -230,11 +229,10 @@ protected PluginServiceFactory createAdditionalPluginServices(

@Override
protected ProtocolSchedule createProtocolSchedule() {
return BftProtocolSchedule.create(
return IbftProtocolSchedule.create(
genesisConfig.getConfigOptions(genesisConfigOverrides),
privacyParameters,
isRevertReasonEnabled,
IbftBlockHeaderValidationRulesetFactory::blockHeaderValidator,
bftExtraDataCodec().get());
}

Expand Down
5 changes: 0 additions & 5 deletions besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.hyperledger.besu.consensus.common.bft.statemachine.FutureMessageBuffer;
import org.hyperledger.besu.consensus.common.validator.ValidatorProvider;
import org.hyperledger.besu.consensus.common.validator.blockbased.BlockValidatorProvider;
import org.hyperledger.besu.consensus.qbft.QbftBlockHeaderValidationRulesetFactory;
import org.hyperledger.besu.consensus.qbft.QbftContext;
import org.hyperledger.besu.consensus.qbft.QbftExtraDataCodec;
import org.hyperledger.besu.consensus.qbft.QbftGossip;
Expand Down Expand Up @@ -255,14 +254,10 @@ protected PluginServiceFactory createAdditionalPluginServices(

@Override
protected ProtocolSchedule createProtocolSchedule() {
final QbftBlockHeaderValidationRulesetFactory qbftBlockHeaderValidationRulesetFactory =
new QbftBlockHeaderValidationRulesetFactory();

return QbftProtocolSchedule.create(
genesisConfig.getConfigOptions(genesisConfigOverrides),
privacyParameters,
isRevertReasonEnabled,
qbftBlockHeaderValidationRulesetFactory::blockHeaderValidator,
bftExtraDataCodec().get());
}

Expand Down
57 changes: 21 additions & 36 deletions ...ensus/common/bft/BftProtocolSchedule.java → ...s/common/bft/BaseBftProtocolSchedule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,19 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;

/** Defines the protocol behaviours for a blockchain using a BFT consensus mechanism. */
public class BftProtocolSchedule {
public abstract class BaseBftProtocolSchedule {

private static final BigInteger DEFAULT_CHAIN_ID = BigInteger.ONE;

public static ProtocolSchedule create(
public ProtocolSchedule createProtocolSchedule(
final GenesisConfigOptions config,
final PrivacyParameters privacyParameters,
final boolean isRevertReasonEnabled,
final Function<Integer, BlockHeaderValidator.Builder> blockHeaderRuleset,
final BftExtraDataCodec bftExtraDataCodec) {
final Map<Long, Function<ProtocolSpecBuilder, ProtocolSpecBuilder>> specMap = new HashMap<>();

Expand All @@ -56,9 +56,9 @@ public static ProtocolSchedule create(
config.getBftConfigOptions(),
builder,
config.isQuorum(),
blockHeaderRuleset,
createGenesisBlockHeaderRuleset(config),
bftExtraDataCodec,
config.getBftConfigOptions().getBlockRewardWei()));
Optional.of(config.getBftConfigOptions().getBlockRewardWei())));

final Supplier<List<? extends BftFork>> forks;
if (config.isIbft2()) {
Expand All @@ -67,8 +67,8 @@ public static ProtocolSchedule create(
forks = () -> config.getTransitions().getQbftForks();
}

forks.get().stream()
.filter(fork -> fork.getBlockRewardWei().isPresent())
forks
.get()
.forEach(
fork ->
specMap.put(
Expand All @@ -78,9 +78,9 @@ public static ProtocolSchedule create(
config.getBftConfigOptions(),
builder,
config.isQuorum(),
blockHeaderRuleset,
createForkBlockHeaderRuleset(config, fork),
bftExtraDataCodec,
fork.getBlockRewardWei().get())));
fork.getBlockRewardWei())));

final ProtocolSpecAdapters specAdapters = new ProtocolSpecAdapters(specMap);

Expand All @@ -94,55 +94,40 @@ public static ProtocolSchedule create(
.createProtocolSchedule();
}

public static ProtocolSchedule create(
final GenesisConfigOptions config,
final boolean isRevertReasonEnabled,
final Function<Integer, BlockHeaderValidator.Builder> blockHeaderRuleset,
final BftExtraDataCodec bftExtraDataCodec) {
return create(
config,
PrivacyParameters.DEFAULT,
isRevertReasonEnabled,
blockHeaderRuleset,
bftExtraDataCodec);
}
protected abstract Supplier<BlockHeaderValidator.Builder> createForkBlockHeaderRuleset(
final GenesisConfigOptions config, BftFork fork);

public static ProtocolSchedule create(
final GenesisConfigOptions config,
final Function<Integer, BlockHeaderValidator.Builder> blockHeaderRuleset,
final BftExtraDataCodec bftExtraDataCodec) {
return create(config, PrivacyParameters.DEFAULT, false, blockHeaderRuleset, bftExtraDataCodec);
}
protected abstract Supplier<BlockHeaderValidator.Builder> createGenesisBlockHeaderRuleset(
final GenesisConfigOptions config);

private static ProtocolSpecBuilder applyBftChanges(
private ProtocolSpecBuilder applyBftChanges(
final BftConfigOptions configOptions,
final ProtocolSpecBuilder builder,
final boolean goQuorumMode,
final Function<Integer, BlockHeaderValidator.Builder> blockHeaderRuleset,
final Supplier<BlockHeaderValidator.Builder> blockHeaderRuleset,
final BftExtraDataCodec bftExtraDataCodec,
final BigInteger blockReward) {
final Optional<BigInteger> blockReward) {

if (configOptions.getEpochLength() <= 0) {
throw new IllegalArgumentException("Epoch length in config must be greater than zero");
}

if (blockReward.signum() < 0) {
if (blockReward.isPresent() && blockReward.get().signum() < 0) {
throw new IllegalArgumentException("Bft Block reward in config cannot be negative");
}

builder
.blockHeaderValidatorBuilder(
feeMarket -> blockHeaderRuleset.apply(configOptions.getBlockPeriodSeconds()))
.ommerHeaderValidatorBuilder(
feeMarket -> blockHeaderRuleset.apply(configOptions.getBlockPeriodSeconds()))
.blockHeaderValidatorBuilder(feeMarket -> blockHeaderRuleset.get())
.ommerHeaderValidatorBuilder(feeMarket -> blockHeaderRuleset.get())
.blockBodyValidatorBuilder(MainnetBlockBodyValidator::new)
.blockValidatorBuilder(MainnetProtocolSpecs.blockValidatorBuilder(goQuorumMode))
.blockImporterBuilder(MainnetBlockImporter::new)
.difficultyCalculator((time, parent, protocolContext) -> BigInteger.ONE)
.blockReward(Wei.of(blockReward))
.skipZeroBlockRewards(true)
.blockHeaderFunctions(BftBlockHeaderFunctions.forOnChainBlock(bftExtraDataCodec));

blockReward.ifPresent(bigInteger -> builder.blockReward(Wei.of(bigInteger)));

if (configOptions.getMiningBeneficiary().isPresent()) {
final Address miningBeneficiary;
try {
Expand Down
69 changes: 29 additions & 40 deletions ...s/common/bft/BftProtocolScheduleTest.java → ...mmon/bft/BaseBftProtocolScheduleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
*
* SPDX-License-Identifier: Apache-2.0
*/

package org.hyperledger.besu.consensus.common.bft;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand All @@ -27,6 +26,7 @@
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.PrivacyParameters;
import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
import org.hyperledger.besu.ethereum.mainnet.MutableProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
Expand All @@ -35,18 +35,15 @@
import java.math.BigInteger;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;

import org.junit.Test;

public class BftProtocolScheduleTest {
public class BaseBftProtocolScheduleTest {

private final GenesisConfigOptions genesisConfig = mock(GenesisConfigOptions.class);
private final BftExtraDataCodec bftExtraDataCodec = mock(BftExtraDataCodec.class);

private static BlockHeaderValidator.Builder arbitraryRulesetBuilder(final Integer blockSeconds) {
return new BlockHeaderValidator.Builder();
}

@Test
public void ensureBlockRewardAndMiningBeneficiaryInProtocolSpecMatchConfig() {
final BigInteger arbitraryBlockReward = BigInteger.valueOf(5);
Expand All @@ -59,9 +56,7 @@ public void ensureBlockRewardAndMiningBeneficiaryInProtocolSpecMatchConfig() {

when(genesisConfig.getBftConfigOptions()).thenReturn(configOptions);

final ProtocolSchedule schedule =
BftProtocolSchedule.create(
genesisConfig, BftProtocolScheduleTest::arbitraryRulesetBuilder, bftExtraDataCodec);
final ProtocolSchedule schedule = createProtocolSchedule();
final ProtocolSpec spec = schedule.getByBlockNumber(1);

spec.getBlockReward();
Expand All @@ -80,12 +75,7 @@ public void illegalMiningBeneficiaryStringThrowsException() {
when(configOptions.getEpochLength()).thenReturn(3000L);
when(configOptions.getBlockRewardWei()).thenReturn(BigInteger.ZERO);
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);
assertThatThrownBy(
() ->
BftProtocolSchedule.create(
genesisConfig,
BftProtocolScheduleTest::arbitraryRulesetBuilder,
bftExtraDataCodec))
assertThatThrownBy(this::createProtocolSchedule)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Mining beneficiary in config is not a valid ethereum address");
}
Expand All @@ -100,9 +90,7 @@ public void missingMiningBeneficiaryInConfigWillPayCoinbaseInHeader() {
when(genesisConfig.getBftConfigOptions()).thenReturn(configOptions);
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

final ProtocolSchedule schedule =
BftProtocolSchedule.create(
genesisConfig, BftProtocolScheduleTest::arbitraryRulesetBuilder, bftExtraDataCodec);
final ProtocolSchedule schedule = createProtocolSchedule();
final ProtocolSpec spec = schedule.getByBlockNumber(1);

final Address headerCoinbase = Address.fromHexString("0x123");
Expand All @@ -124,12 +112,7 @@ public void negativeBlockRewardThrowsException() {
when(genesisConfig.getBftConfigOptions()).thenReturn(configOptions);
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

assertThatThrownBy(
() ->
BftProtocolSchedule.create(
genesisConfig,
BftProtocolScheduleTest::arbitraryRulesetBuilder,
bftExtraDataCodec))
assertThatThrownBy(this::createProtocolSchedule)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Bft Block reward in config cannot be negative");
}
Expand All @@ -144,12 +127,7 @@ public void zeroEpochLengthThrowsException() {
when(genesisConfig.getBftConfigOptions()).thenReturn(configOptions);
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

assertThatThrownBy(
() ->
BftProtocolSchedule.create(
genesisConfig,
BftProtocolScheduleTest::arbitraryRulesetBuilder,
bftExtraDataCodec))
assertThatThrownBy(this::createProtocolSchedule)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Epoch length in config must be greater than zero");
}
Expand All @@ -164,12 +142,7 @@ public void negativeEpochLengthThrowsException() {
when(genesisConfig.getBftConfigOptions()).thenReturn(configOptions);
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

assertThatThrownBy(
() ->
BftProtocolSchedule.create(
genesisConfig,
BftProtocolScheduleTest::arbitraryRulesetBuilder,
bftExtraDataCodec))
assertThatThrownBy(this::createProtocolSchedule)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Epoch length in config must be greater than zero");
}
Expand All @@ -194,15 +167,31 @@ public void blockRewardSpecifiedInTransitionCreatesNewMilestone() {
when(transitions.getIbftForks()).thenReturn(List.of(fork));
when(genesisConfig.getTransitions()).thenReturn(transitions);

final MutableProtocolSchedule schedule =
(MutableProtocolSchedule)
BftProtocolSchedule.create(
genesisConfig, BftProtocolScheduleTest::arbitraryRulesetBuilder, bftExtraDataCodec);
final MutableProtocolSchedule schedule = (MutableProtocolSchedule) createProtocolSchedule();

assertThat(schedule.streamMilestoneBlocks().count()).isEqualTo(2);
assertThat(schedule.getByBlockNumber(0).getBlockReward())
.isEqualTo(Wei.of(arbitraryBlockReward));
assertThat(schedule.getByBlockNumber(transitionBlock).getBlockReward())
.isEqualTo(Wei.of(forkBlockReward));
}

private ProtocolSchedule createProtocolSchedule() {
final BaseBftProtocolSchedule bftProtocolSchedule =
new BaseBftProtocolSchedule() {
@Override
protected Supplier<BlockHeaderValidator.Builder> createForkBlockHeaderRuleset(
final GenesisConfigOptions config, final BftFork fork) {
return BlockHeaderValidator.Builder::new;
}

@Override
protected Supplier<BlockHeaderValidator.Builder> createGenesisBlockHeaderRuleset(
final GenesisConfigOptions config) {
return BlockHeaderValidator.Builder::new;
}
};
return bftProtocolSchedule.createProtocolSchedule(
genesisConfig, PrivacyParameters.DEFAULT, false, bftExtraDataCodec);
}
}
8 changes: 2 additions & 6 deletions ...integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.hyperledger.besu.consensus.common.bft.BftExecutors;
import org.hyperledger.besu.consensus.common.bft.BftExtraData;
import org.hyperledger.besu.consensus.common.bft.BftHelpers;
import org.hyperledger.besu.consensus.common.bft.BftProtocolSchedule;
import org.hyperledger.besu.consensus.common.bft.BlockTimer;
import org.hyperledger.besu.consensus.common.bft.EventMultiplexer;
import org.hyperledger.besu.consensus.common.bft.Gossiper;
Expand All @@ -48,9 +47,9 @@
import org.hyperledger.besu.consensus.common.bft.statemachine.FutureMessageBuffer;
import org.hyperledger.besu.consensus.common.validator.ValidatorProvider;
import org.hyperledger.besu.consensus.common.validator.blockbased.BlockValidatorProvider;
import org.hyperledger.besu.consensus.ibft.IbftBlockHeaderValidationRulesetFactory;
import org.hyperledger.besu.consensus.ibft.IbftExtraDataCodec;
import org.hyperledger.besu.consensus.ibft.IbftGossip;
import org.hyperledger.besu.consensus.ibft.IbftProtocolSchedule;
import org.hyperledger.besu.consensus.ibft.payload.MessageFactory;
import org.hyperledger.besu.consensus.ibft.statemachine.IbftBlockHeightManagerFactory;
import org.hyperledger.besu.consensus.ibft.statemachine.IbftController;
Expand Down Expand Up @@ -294,10 +293,7 @@ private static ControllerAndState createControllerAndFinalState(
genesisConfigOptions.byzantiumBlock(0);

final ProtocolSchedule protocolSchedule =
BftProtocolSchedule.create(
genesisConfigOptions,
IbftBlockHeaderValidationRulesetFactory::blockHeaderValidator,
IBFT_EXTRA_DATA_ENCODER);
IbftProtocolSchedule.create(genesisConfigOptions, IBFT_EXTRA_DATA_ENCODER);

/////////////////////////////////////////////////////////////////////////////////////
// From here down is BASICALLY taken from IbftBesuController
Expand Down
Loading

0 comments on commit 2541b15

Please sign in to comment.