Skip to content

Commit

Permalink
Rename BftForkSpec to ForkSpec and make it generic (hyperledger#3087)
Browse files Browse the repository at this point in the history
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
  • Loading branch information
jframe authored Nov 19, 2021
1 parent 5230af5 commit b1e2496
Show file tree
Hide file tree
Showing 21 changed files with 126 additions and 146 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,26 @@
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.consensus.common.bft;

import org.hyperledger.besu.config.BftConfigOptions;
package org.hyperledger.besu.consensus.common;

import java.util.Objects;

public class BftForkSpec<C extends BftConfigOptions> {
public class ForkSpec<C> {

private final long block;
private final C configOptions;
private final C value;

public BftForkSpec(final long block, final C configOptions) {
public ForkSpec(final long block, final C value) {
this.block = block;
this.configOptions = configOptions;
this.value = value;
}

public long getBlock() {
return block;
}

public C getConfigOptions() {
return configOptions;
public C getValue() {
return value;
}

@Override
Expand All @@ -44,12 +42,12 @@ public boolean equals(final Object o) {
if (o == null || getClass() != o.getClass()) {
return false;
}
final BftForkSpec<?> that = (BftForkSpec<?>) o;
return block == that.block && Objects.equals(configOptions, that.configOptions);
final ForkSpec<?> that = (ForkSpec<?>) o;
return block == that.block && Objects.equals(value, that.value);
}

@Override
public int hashCode() {
return Objects.hash(block, configOptions);
return Objects.hash(block, value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ public ProtocolSchedule createProtocolSchedule(
bftForksSchedule
.getForks()
.forEach(
bftForkSpec ->
forkSpec ->
specMap.put(
bftForkSpec.getBlock(),
forkSpec.getBlock(),
builder ->
applyBftChanges(
bftForkSpec.getConfigOptions(),
forkSpec.getValue(),
builder,
config.isQuorum(),
createBlockHeaderRuleset(bftForkSpec.getConfigOptions()),
createBlockHeaderRuleset(forkSpec.getValue()),
bftExtraDataCodec,
Optional.of(bftForkSpec.getConfigOptions().getBlockRewardWei()))));
Optional.of(forkSpec.getValue().getBlockRewardWei()))));

final ProtocolSpecAdapters specAdapters = new ProtocolSpecAdapters(specMap);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.hyperledger.besu.config.BftConfigOptions;
import org.hyperledger.besu.config.BftFork;
import org.hyperledger.besu.consensus.common.ForkSpec;

import java.util.Collection;
import java.util.Collections;
Expand All @@ -32,17 +33,16 @@

public class BftForksSchedule<C extends BftConfigOptions> {

private final NavigableSet<BftForkSpec<C>> forks =
private final NavigableSet<ForkSpec<C>> forks =
new TreeSet<>(
Comparator.comparing((Function<BftForkSpec<C>, Long>) BftForkSpec::getBlock).reversed());
Comparator.comparing((Function<ForkSpec<C>, Long>) ForkSpec::getBlock).reversed());

public interface BftSpecCreator<T extends BftConfigOptions, U extends BftFork> {
T create(BftForkSpec<T> lastSpec, U fork);
T create(ForkSpec<T> lastSpec, U fork);
}

@VisibleForTesting
public BftForksSchedule(
final BftForkSpec<C> genesisFork, final Collection<BftForkSpec<C>> forks) {
public BftForksSchedule(final ForkSpec<C> genesisFork, final Collection<ForkSpec<C>> forks) {
this.forks.add(genesisFork);
this.forks.addAll(forks);
}
Expand All @@ -56,24 +56,23 @@ public static <T extends BftConfigOptions, U extends BftFork> BftForksSchedule<T
forks.stream().map(BftFork::getForkBlock).distinct().count() == forks.size(),
"Duplicate transitions cannot be created for the same block");

final NavigableSet<BftForkSpec<T>> specs =
new TreeSet<>(Comparator.comparing(BftForkSpec::getBlock));
final BftForkSpec<T> initialForkSpec = new BftForkSpec<>(0, initial);
final NavigableSet<ForkSpec<T>> specs = new TreeSet<>(Comparator.comparing(ForkSpec::getBlock));
final ForkSpec<T> initialForkSpec = new ForkSpec<>(0, initial);
specs.add(initialForkSpec);

forks.stream()
.sorted(Comparator.comparing(BftFork::getForkBlock))
.forEachOrdered(
f -> {
final T spec = specCreator.create(specs.last(), f);
specs.add(new BftForkSpec<>(f.getForkBlock(), spec));
specs.add(new ForkSpec<>(f.getForkBlock(), spec));
});

return new BftForksSchedule<>(initialForkSpec, specs.tailSet(initialForkSpec, false));
}

public BftForkSpec<C> getFork(final long blockNumber) {
for (final BftForkSpec<C> f : forks) {
public ForkSpec<C> getFork(final long blockNumber) {
for (final ForkSpec<C> f : forks) {
if (blockNumber >= f.getBlock()) {
return f;
}
Expand All @@ -82,7 +81,7 @@ public BftForkSpec<C> getFork(final long blockNumber) {
return forks.first();
}

public Set<BftForkSpec<C>> getForks() {
public Set<ForkSpec<C>> getForks() {
return Collections.unmodifiableSet(forks);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,7 @@ public synchronized void startTimer(

// absolute time when the timer is supposed to expire
final int blockPeriodSeconds =
bftForksSchedule
.getFork(round.getSequenceNumber())
.getConfigOptions()
.getBlockPeriodSeconds();
bftForksSchedule.getFork(round.getSequenceNumber()).getValue().getBlockPeriodSeconds();
final long minimumTimeBetweenBlocksMillis = blockPeriodSeconds * 1000L;
final long expiryTime = chainHeadHeader.getTimestamp() * 1_000 + minimumTimeBetweenBlocksMillis;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.hyperledger.besu.config.GenesisConfigOptions;
import org.hyperledger.besu.config.JsonBftConfigOptions;
import org.hyperledger.besu.config.TransitionsConfigOptions;
import org.hyperledger.besu.consensus.common.ForkSpec;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.core.BlockHeader;
Expand Down Expand Up @@ -58,7 +59,7 @@ public void ensureBlockRewardAndMiningBeneficiaryInProtocolSpecMatchConfig() {
when(genesisConfig.getBftConfigOptions()).thenReturn(configOptions);

final ProtocolSchedule schedule =
createProtocolSchedule(new BftForkSpec<>(0, configOptions), Collections.emptyList());
createProtocolSchedule(new ForkSpec<>(0, configOptions), Collections.emptyList());
final ProtocolSpec spec = schedule.getByBlockNumber(1);

assertThat(spec.getBlockReward()).isEqualTo(Wei.of(arbitraryBlockReward));
Expand All @@ -76,9 +77,7 @@ public void illegalMiningBeneficiaryStringThrowsException() {
when(configOptions.getBlockRewardWei()).thenReturn(BigInteger.ZERO);
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);
assertThatThrownBy(
() ->
createProtocolSchedule(
new BftForkSpec<>(0, configOptions), Collections.emptyList()))
() -> createProtocolSchedule(new ForkSpec<>(0, configOptions), Collections.emptyList()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Mining beneficiary in config is not a valid ethereum address");
}
Expand All @@ -94,7 +93,7 @@ public void missingMiningBeneficiaryInConfigWillPayCoinbaseInHeader() {
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

final ProtocolSchedule schedule =
createProtocolSchedule(new BftForkSpec<>(0, configOptions), Collections.emptyList());
createProtocolSchedule(new ForkSpec<>(0, configOptions), Collections.emptyList());
final ProtocolSpec spec = schedule.getByBlockNumber(1);

final Address headerCoinbase = Address.fromHexString("0x123");
Expand All @@ -117,9 +116,7 @@ public void negativeBlockRewardThrowsException() {
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

assertThatThrownBy(
() ->
createProtocolSchedule(
new BftForkSpec<>(0, configOptions), Collections.emptyList()))
() -> createProtocolSchedule(new ForkSpec<>(0, configOptions), Collections.emptyList()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Bft Block reward in config cannot be negative");
}
Expand All @@ -135,9 +132,7 @@ public void zeroEpochLengthThrowsException() {
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

assertThatThrownBy(
() ->
createProtocolSchedule(
new BftForkSpec<>(0, configOptions), Collections.emptyList()))
() -> createProtocolSchedule(new ForkSpec<>(0, configOptions), Collections.emptyList()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Epoch length in config must be greater than zero");
}
Expand All @@ -153,9 +148,7 @@ public void negativeEpochLengthThrowsException() {
when(genesisConfig.getTransitions()).thenReturn(TransitionsConfigOptions.DEFAULT);

assertThatThrownBy(
() ->
createProtocolSchedule(
new BftForkSpec<>(0, configOptions), Collections.emptyList()))
() -> createProtocolSchedule(new ForkSpec<>(0, configOptions), Collections.emptyList()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Epoch length in config must be greater than zero");
}
Expand All @@ -179,8 +172,8 @@ public void blockRewardSpecifiedInTransitionCreatesNewMilestone() {

final ProtocolSchedule schedule =
createProtocolSchedule(
new BftForkSpec<>(0, configOptions),
List.of(new BftForkSpec<>(transitionBlock, blockRewardTransition)));
new ForkSpec<>(0, configOptions),
List.of(new ForkSpec<>(transitionBlock, blockRewardTransition)));

assertThat(schedule.streamMilestoneBlocks().count()).isEqualTo(2);
assertThat(schedule.getByBlockNumber(0).getBlockReward())
Expand All @@ -190,8 +183,7 @@ public void blockRewardSpecifiedInTransitionCreatesNewMilestone() {
}

private ProtocolSchedule createProtocolSchedule(
final BftForkSpec<BftConfigOptions> genesisFork,
final List<BftForkSpec<BftConfigOptions>> forks) {
final ForkSpec<BftConfigOptions> genesisFork, final List<ForkSpec<BftConfigOptions>> forks) {
final BaseBftProtocolSchedule bftProtocolSchedule =
new BaseBftProtocolSchedule() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.hyperledger.besu.config.BftFork;
import org.hyperledger.besu.config.JsonBftConfigOptions;
import org.hyperledger.besu.config.JsonUtil;
import org.hyperledger.besu.consensus.common.ForkSpec;
import org.hyperledger.besu.consensus.common.bft.BftForksSchedule.BftSpecCreator;

import java.util.List;
Expand All @@ -34,8 +35,8 @@ public class BftForksScheduleTest {

@Test
public void retrievesGenesisFork() {
final BftForkSpec<BftConfigOptions> genesisForkSpec =
new BftForkSpec<>(0, JsonBftConfigOptions.DEFAULT);
final ForkSpec<BftConfigOptions> genesisForkSpec =
new ForkSpec<>(0, JsonBftConfigOptions.DEFAULT);

final BftForksSchedule<BftConfigOptions> schedule =
new BftForksSchedule<>(genesisForkSpec, List.of());
Expand All @@ -45,10 +46,10 @@ public void retrievesGenesisFork() {

@Test
public void retrievesLatestFork() {
final BftForkSpec<BftConfigOptions> genesisForkSpec =
new BftForkSpec<>(0, JsonBftConfigOptions.DEFAULT);
final BftForkSpec<BftConfigOptions> forkSpec1 = createForkSpec(1, 10);
final BftForkSpec<BftConfigOptions> forkSpec2 = createForkSpec(2, 20);
final ForkSpec<BftConfigOptions> genesisForkSpec =
new ForkSpec<>(0, JsonBftConfigOptions.DEFAULT);
final ForkSpec<BftConfigOptions> forkSpec1 = createForkSpec(1, 10);
final ForkSpec<BftConfigOptions> forkSpec2 = createForkSpec(2, 20);

final BftForksSchedule<BftConfigOptions> schedule =
new BftForksSchedule<>(genesisForkSpec, List.of(forkSpec1, forkSpec2));
Expand Down Expand Up @@ -93,8 +94,7 @@ public void throwsErrorIfHasForksWithDuplicateBlock() {
@Test
public void createsScheduleUsingSpecCreator() {
final BftConfigOptions genesisConfigOptions = JsonBftConfigOptions.DEFAULT;
final BftForkSpec<BftConfigOptions> genesisForkSpec =
new BftForkSpec<>(0, genesisConfigOptions);
final ForkSpec<BftConfigOptions> genesisForkSpec = new ForkSpec<>(0, genesisConfigOptions);
final BftFork fork1 = createFork(1, 10);
final BftFork fork2 = createFork(2, 20);
final BftSpecCreator<BftConfigOptions, BftFork> specCreator =
Expand All @@ -103,20 +103,19 @@ public void createsScheduleUsingSpecCreator() {
final BftConfigOptions configOptions1 = createBftConfigOptions(10);
final BftConfigOptions configOptions2 = createBftConfigOptions(20);
when(specCreator.create(genesisForkSpec, fork1)).thenReturn(configOptions1);
when(specCreator.create(new BftForkSpec<>(1, configOptions1), fork2))
.thenReturn(configOptions2);
when(specCreator.create(new ForkSpec<>(1, configOptions1), fork2)).thenReturn(configOptions2);

final BftForksSchedule<BftConfigOptions> schedule =
BftForksSchedule.create(genesisConfigOptions, List.of(fork1, fork2), specCreator);
assertThat(schedule.getFork(0)).isEqualTo(genesisForkSpec);
assertThat(schedule.getFork(1)).isEqualTo(new BftForkSpec<>(1, configOptions1));
assertThat(schedule.getFork(2)).isEqualTo(new BftForkSpec<>(2, configOptions2));
assertThat(schedule.getFork(1)).isEqualTo(new ForkSpec<>(1, configOptions1));
assertThat(schedule.getFork(2)).isEqualTo(new ForkSpec<>(2, configOptions2));
}

private BftForkSpec<BftConfigOptions> createForkSpec(
private ForkSpec<BftConfigOptions> createForkSpec(
final long block, final int blockPeriodSeconds) {
final MutableBftConfigOptions bftConfigOptions = createBftConfigOptions(blockPeriodSeconds);
return new BftForkSpec<>(block, bftConfigOptions);
return new ForkSpec<>(block, bftConfigOptions);
}

private MutableBftConfigOptions createBftConfigOptions(final int blockPeriodSeconds) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import org.hyperledger.besu.config.BftConfigOptions;
import org.hyperledger.besu.config.JsonBftConfigOptions;
import org.hyperledger.besu.consensus.common.ForkSpec;
import org.hyperledger.besu.consensus.common.bft.events.BftEvent;
import org.hyperledger.besu.consensus.common.bft.events.BlockTimerExpiry;
import org.hyperledger.besu.ethereum.core.BlockHeader;
Expand Down Expand Up @@ -78,7 +79,7 @@ public void startTimerSchedulesCorrectlyWhenExpiryIsInTheFuture() {
final long EXPECTED_DELAY = 10_000L;

when(mockForksSchedule.getFork(anyLong()))
.thenReturn(new BftForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));
.thenReturn(new ForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));

final BlockTimer timer = new BlockTimer(mockQueue, mockForksSchedule, bftExecutors, mockClock);

Expand Down Expand Up @@ -107,7 +108,7 @@ public void aBlockTimerExpiryEventIsAddedToTheQueueOnExpiry() throws Interrupted
final long EXPECTED_DELAY = 500;

when(mockForksSchedule.getFork(anyLong()))
.thenReturn(new BftForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));
.thenReturn(new ForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));
when(mockClock.millis()).thenReturn(NOW_MILLIS);

final BlockHeader header =
Expand Down Expand Up @@ -151,7 +152,7 @@ public void eventIsImmediatelyAddedToTheQueueIfAbsoluteExpiryIsEqualToNow() {
final long BLOCK_TIME_STAMP = 500;

when(mockForksSchedule.getFork(anyLong()))
.thenReturn(new BftForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));
.thenReturn(new ForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));

final BlockTimer timer = new BlockTimer(mockQueue, mockForksSchedule, bftExecutors, mockClock);

Expand Down Expand Up @@ -181,7 +182,7 @@ public void eventIsImmediatelyAddedToTheQueueIfAbsoluteExpiryIsInThePast() {
final long BLOCK_TIME_STAMP = 500L;

when(mockForksSchedule.getFork(anyLong()))
.thenReturn(new BftForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));
.thenReturn(new ForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));

final BlockTimer timer = new BlockTimer(mockQueue, mockForksSchedule, bftExecutors, mockClock);

Expand Down Expand Up @@ -211,7 +212,7 @@ public void startTimerCancelsExistingTimer() {
final long BLOCK_TIME_STAMP = 500L;

when(mockForksSchedule.getFork(anyLong()))
.thenReturn(new BftForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));
.thenReturn(new ForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));

final BlockTimer timer = new BlockTimer(mockQueue, mockForksSchedule, bftExecutors, mockClock);

Expand Down Expand Up @@ -239,7 +240,7 @@ public void runningFollowsTheStateOfTheTimer() {
final long BLOCK_TIME_STAMP = 500L;

when(mockForksSchedule.getFork(anyLong()))
.thenReturn(new BftForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));
.thenReturn(new ForkSpec<>(0, createBftFork(MINIMAL_TIME_BETWEEN_BLOCKS_SECONDS)));

final BlockTimer timer = new BlockTimer(mockQueue, mockForksSchedule, bftExecutors, mockClock);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import org.hyperledger.besu.config.BftConfigOptions;
import org.hyperledger.besu.config.BftFork;
import org.hyperledger.besu.config.GenesisConfigOptions;
import org.hyperledger.besu.consensus.common.bft.BftForkSpec;
import org.hyperledger.besu.consensus.common.ForkSpec;
import org.hyperledger.besu.consensus.common.bft.BftForksSchedule;
import org.hyperledger.besu.consensus.common.bft.MutableBftConfigOptions;

Expand All @@ -32,9 +32,9 @@ public static BftForksSchedule<BftConfigOptions> create(
}

private static BftConfigOptions createBftConfigOptions(
final BftForkSpec<BftConfigOptions> lastSpec, final BftFork fork) {
final ForkSpec<BftConfigOptions> lastSpec, final BftFork fork) {
final MutableBftConfigOptions bftConfigOptions =
new MutableBftConfigOptions(lastSpec.getConfigOptions());
new MutableBftConfigOptions(lastSpec.getValue());

fork.getBlockPeriodSeconds().ifPresent(bftConfigOptions::setBlockPeriodSeconds);
fork.getBlockRewardWei().ifPresent(bftConfigOptions::setBlockRewardWei);
Expand Down
Loading

0 comments on commit b1e2496

Please sign in to comment.