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.

Sign up for GitHub

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

Rename BftForkSpec to ForkSpec and make it generic #3087

Merged
merged 3 commits into from
Nov 19, 2021
Merged

Rename BftForkSpec to ForkSpec and make it generic #3087

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
517ab44
Rename BftForkSpec to ForkSpec and make it generic
jframe Nov 19, 2021
7b120ab
rename var
jframe Nov 19, 2021
4c02299
fix some usages that didn't get updated
jframe Nov 19, 2021
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
Next Next commit
Rename BftForkSpec to ForkSpec and make it generic 
Signed-off-by: Jason Frame <jasonwframe@gmail.com>
  • Loading branch information
@jframe
jframe committed Nov 19, 2021
commit 517ab4458dff5b41d9fb0bf29c930f35e2c7d590
22 changes: 10 additions & 12 deletions ...esu/consensus/common/bft/BftForkSpec.java → ...edger/besu/consensus/common/ForkSpec.java
View file
Open in desktop
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);
}
}
6 changes: 3 additions & 3 deletions ...mmon/src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolSchedule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ public ProtocolSchedule createProtocolSchedule(
bftForkSpec.getBlock(),
builder ->
applyBftChanges(
bftForkSpec.getConfigOptions(),
bftForkSpec.getValue(),
builder,
config.isQuorum(),
createBlockHeaderRuleset(bftForkSpec.getConfigOptions()),
createBlockHeaderRuleset(bftForkSpec.getValue()),
bftExtraDataCodec,
Optional.of(bftForkSpec.getConfigOptions().getBlockRewardWei()))));
Optional.of(bftForkSpec.getValue().getBlockRewardWei()))));

final ProtocolSpecAdapters specAdapters = new ProtocolSpecAdapters(specMap);

Expand Down
23 changes: 11 additions & 12 deletions ...nsus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftForksSchedule.java
View file
Open in desktop
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);
}
}
5 changes: 1 addition & 4 deletions consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java
View file
Open in desktop
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()).getSpec().getBlockPeriodSeconds();
final long minimumTimeBetweenBlocksMillis = blockPeriodSeconds * 1000L;
final long expiryTime = chainHeadHeader.getTimestamp() * 1_000 + minimumTimeBetweenBlocksMillis;

Expand Down
28 changes: 10 additions & 18 deletions .../src/test/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleTest.java
View file
Open in desktop
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
27 changes: 13 additions & 14 deletions .../common/src/test/java/org/hyperledger/besu/consensus/common/bft/BftForksScheduleTest.java
View file
Open in desktop
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
13 changes: 7 additions & 6 deletions consensus/common/src/test/java/org/hyperledger/besu/consensus/common/bft/BlockTimerTest.java
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions ...sus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/IbftForksSchedulesFactory.java
View file
Open in desktop
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