Skip to content

Commit

Permalink
Don't cache protocol spec in block creator (hyperledger#4982)
Browse files Browse the repository at this point in the history 
* Don't cache protocol spec in block creator

Don't cache the protocol spec in the block creator.  With the new
shanghaiTimestamp the correct spec may be a function of the timestamp
not just the block number.  So every time we are asked to build a block
re-query the spec.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
  • Loading branch information
@shemnon
shemnon authored Jan 23, 2023
1 parent 1abe277 commit 9034d31
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .../merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private ProtocolSpec getByBlockHeaderFromTransitionUtils(
protocolContext
.getBlockchain()
.getTotalDifficultyByHash(blockHeader.getParentHash())
.orElseThrow();
.orElse(Difficulty.ZERO);
Difficulty thisDifficulty = parentDifficulty.add(blockHeader.getDifficulty());
Difficulty terminalDifficulty = mergeContext.getTerminalTotalDifficulty();
debugLambda(
Expand Down
40 changes: 25 additions & 15 deletions ...ation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.hyperledger.besu.ethereum.mainnet.AbstractBlockProcessor;
import org.hyperledger.besu.ethereum.mainnet.BodyValidation;
import org.hyperledger.besu.ethereum.mainnet.DifficultyCalculator;
import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionProcessor;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
Expand Down Expand Up @@ -79,7 +80,6 @@ public interface ExtraDataCalculator {
private final Wei minTransactionGasPrice;
private final Double minBlockOccupancyRatio;
protected final BlockHeader parentHeader;
protected final ProtocolSpec protocolSpec;

private final AtomicBoolean isCancelled = new AtomicBoolean(false);

Expand All @@ -104,7 +104,6 @@ protected AbstractBlockCreator(
this.minTransactionGasPrice = minTransactionGasPrice;
this.minBlockOccupancyRatio = minBlockOccupancyRatio;
this.parentHeader = parentHeader;
this.protocolSpec = protocolSchedule.getByBlockNumber(parentHeader.getNumber() + 1);
blockHeaderFunctions = ScheduleBasedBlockHeaderFunctions.create(protocolSchedule);
}

Expand Down Expand Up @@ -154,8 +153,16 @@ protected BlockCreationResult createBlock(
boolean rewardCoinbase) {

try (final MutableWorldState disposableWorldState = duplicateWorldStateAtParent()) {
final ProtocolSpec newProtocolSpec =
protocolSchedule.getByBlockHeader(
BlockHeaderBuilder.fromHeader(parentHeader)
.number(parentHeader.getNumber() + 1)
.timestamp(timestamp)
.blockHeaderFunctions(new MainnetBlockHeaderFunctions())
.buildBlockHeader());

final ProcessableBlockHeader processableBlockHeader =
createPendingBlockHeader(timestamp, maybePrevRandao);
createPendingBlockHeader(timestamp, maybePrevRandao, newProtocolSpec);
final Address miningBeneficiary =
miningBeneficiaryCalculator.getMiningBeneficiary(processableBlockHeader.getNumber());

Expand All @@ -164,16 +171,16 @@ protected BlockCreationResult createBlock(
final List<BlockHeader> ommers = maybeOmmers.orElse(selectOmmers());

throwIfStopped();

final BlockTransactionSelector.TransactionSelectionResults transactionResults =
selectTransactions(
processableBlockHeader, disposableWorldState, maybeTransactions, miningBeneficiary);
processableBlockHeader,
disposableWorldState,
maybeTransactions,
miningBeneficiary,
newProtocolSpec);

throwIfStopped();

final ProtocolSpec newProtocolSpec =
protocolSchedule.getByBlockHeader(processableBlockHeader);

final Optional<WithdrawalsProcessor> maybeWithdrawalsProcessor =
newProtocolSpec.getWithdrawalsProcessor();
final boolean withdrawalsCanBeProcessed =
Expand All @@ -193,7 +200,8 @@ protected BlockCreationResult createBlock(
ommers,
miningBeneficiary,
newProtocolSpec.getBlockReward(),
newProtocolSpec.isSkipZeroBlockRewards())) {
newProtocolSpec.isSkipZeroBlockRewards(),
newProtocolSpec)) {
LOG.trace("Failed to apply mining reward, exiting.");
throw new RuntimeException("Failed to apply mining reward.");
}
Expand Down Expand Up @@ -228,9 +236,7 @@ protected BlockCreationResult createBlock(
return new BlockCreationResult(block, transactionResults);
} catch (final SecurityModuleException ex) {
throw new IllegalStateException("Failed to create block signature", ex);
} catch (final CancellationException ex) {
throw ex;
} catch (final StorageException ex) {
} catch (final CancellationException | StorageException ex) {
throw ex;
} catch (final Exception ex) {
throw new IllegalStateException(
Expand All @@ -242,7 +248,8 @@ private BlockTransactionSelector.TransactionSelectionResults selectTransactions(
final ProcessableBlockHeader processableBlockHeader,
final MutableWorldState disposableWorldState,
final Optional<List<Transaction>> transactions,
final Address miningBeneficiary)
final Address miningBeneficiary,
final ProtocolSpec protocolSpec)
throws RuntimeException {
final MainnetTransactionProcessor transactionProcessor = protocolSpec.getTransactionProcessor();

Expand Down Expand Up @@ -300,7 +307,9 @@ private List<BlockHeader> selectOmmers() {
}

private ProcessableBlockHeader createPendingBlockHeader(
final long timestamp, final Optional<Bytes32> maybePrevRandao) {
final long timestamp,
final Optional<Bytes32> maybePrevRandao,
final ProtocolSpec protocolSpec) {
final long newBlockNumber = parentHeader.getNumber() + 1;
long gasLimit =
protocolSpec
Expand Down Expand Up @@ -363,7 +372,8 @@ boolean rewardBeneficiary(
final List<BlockHeader> ommers,
final Address miningBeneficiary,
final Wei blockReward,
final boolean skipZeroBlockRewards) {
final boolean skipZeroBlockRewards,
final ProtocolSpec protocolSpec) {

// TODO(tmm): Added to make this work, should come from blockProcessor.
final int MAX_GENERATION = 6;
Expand Down
83 changes: 47 additions & 36 deletions ...eation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.hyperledger.besu.ethereum.mainnet.EpochCalculator;
import org.hyperledger.besu.ethereum.mainnet.PoWHasher;
import org.hyperledger.besu.ethereum.mainnet.PoWSolver;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolScheduleBuilder;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecAdapters;
import org.hyperledger.besu.ethereum.mainnet.ValidationTestUtils;
Expand All @@ -54,7 +55,7 @@
import org.apache.tuweni.bytes.Bytes;
import org.junit.jupiter.api.Test;

public class PoWBlockCreatorTest extends AbstractBlockCreatorTest {
class PoWBlockCreatorTest extends AbstractBlockCreatorTest {

private final Address BLOCK_1_COINBASE =
Address.fromHexString("0x05a56e2d52c817161883f50c441c3228cfe54d9f");
Expand All @@ -68,7 +69,7 @@ public class PoWBlockCreatorTest extends AbstractBlockCreatorTest {
private final MetricsSystem metricsSystem = new NoOpMetricsSystem();

@Test
public void createMainnetBlock1() throws IOException {
void createMainnetBlock1() throws IOException {
final GenesisConfigOptions genesisConfigOptions = GenesisConfigFile.DEFAULT.getConfigOptions();
final ExecutionContextTestFixture executionContextTestFixture =
ExecutionContextTestFixture.builder()
Expand Down Expand Up @@ -104,7 +105,7 @@ public void createMainnetBlock1() throws IOException {
final PoWBlockCreator blockCreator =
new PoWBlockCreator(
BLOCK_1_COINBASE,
() -> Optional.empty(),
Optional::empty,
parent -> BLOCK_1_EXTRA_DATA,
pendingTransactions,
executionContextTestFixture.getProtocolContext(),
Expand All @@ -115,20 +116,20 @@ public void createMainnetBlock1() throws IOException {
executionContextTestFixture.getBlockchain().getChainHeadHeader());

// A Hashrate should not exist in the block creator prior to creating a block
assertThat(blockCreator.getHashesPerSecond().isPresent()).isFalse();
assertThat(blockCreator.getHashesPerSecond()).isNotPresent();

final BlockCreationResult blockResult = blockCreator.createBlock(BLOCK_1_TIMESTAMP);
final Block actualBlock = blockResult.getBlock();
final Block expectedBlock = ValidationTestUtils.readBlock(1);

assertThat(actualBlock).isEqualTo(expectedBlock);
assertThat(blockCreator.getHashesPerSecond().isPresent()).isTrue();
assertThat(blockCreator.getHashesPerSecond()).isPresent();
assertThat(blockResult.getTransactionSelectionResults())
.isEqualTo(new TransactionSelectionResults());
}

@Test
public void createMainnetBlock1_fixedDifficulty1() {
void createMainnetBlock1_fixedDifficulty1() {
final GenesisConfigOptions genesisConfigOptions =
GenesisConfigFile.fromConfig("{\"config\": {\"ethash\": {\"fixeddifficulty\":1}}}")
.getConfigOptions();
Expand Down Expand Up @@ -166,7 +167,7 @@ public void createMainnetBlock1_fixedDifficulty1() {
final PoWBlockCreator blockCreator =
new PoWBlockCreator(
BLOCK_1_COINBASE,
() -> Optional.empty(),
Optional::empty,
parent -> BLOCK_1_EXTRA_DATA,
pendingTransactions,
executionContextTestFixture.getProtocolContext(),
Expand All @@ -176,29 +177,28 @@ public void createMainnetBlock1_fixedDifficulty1() {
0.8,
executionContextTestFixture.getBlockchain().getChainHeadHeader());

blockCreator.createBlock(BLOCK_1_TIMESTAMP);
assertThat(blockCreator.createBlock(BLOCK_1_TIMESTAMP)).isNotNull();
// If we weren't setting difficulty to 2^256-1 a difficulty of 1 would have caused a
// IllegalArgumentException at the previous line, as 2^256 is 33 bytes.
}

@Test
public void rewardBeneficiary_zeroReward_skipZeroRewardsFalse() {
void rewardBeneficiary_zeroReward_skipZeroRewardsFalse() {
final GenesisConfigOptions genesisConfigOptions =
GenesisConfigFile.fromConfig("{\"config\": {\"ethash\": {\"fixeddifficulty\":1}}}")
.getConfigOptions();
ProtocolSchedule protocolSchedule =
new ProtocolScheduleBuilder(
genesisConfigOptions,
BigInteger.valueOf(42),
ProtocolSpecAdapters.create(0, Function.identity()),
PrivacyParameters.DEFAULT,
false,
genesisConfigOptions.isQuorum(),
EvmConfiguration.DEFAULT)
.createProtocolSchedule();
final ExecutionContextTestFixture executionContextTestFixture =
ExecutionContextTestFixture.builder()
.protocolSchedule(
new ProtocolScheduleBuilder(
genesisConfigOptions,
BigInteger.valueOf(42),
ProtocolSpecAdapters.create(0, Function.identity()),
PrivacyParameters.DEFAULT,
false,
genesisConfigOptions.isQuorum(),
EvmConfiguration.DEFAULT)
.createProtocolSchedule())
.build();
ExecutionContextTestFixture.builder().protocolSchedule(protocolSchedule).build();

final PoWSolver solver =
new PoWSolver(
Expand Down Expand Up @@ -245,30 +245,35 @@ public void rewardBeneficiary_zeroReward_skipZeroRewardsFalse() {
.buildProcessableBlockHeader();

blockCreator.rewardBeneficiary(
mutableWorldState, header, Collections.emptyList(), BLOCK_1_COINBASE, Wei.ZERO, false);
mutableWorldState,
header,
Collections.emptyList(),
BLOCK_1_COINBASE,
Wei.ZERO,
false,
protocolSchedule.getByBlockHeader(header));

assertThat(mutableWorldState.get(BLOCK_1_COINBASE)).isNotNull();
assertThat(mutableWorldState.get(BLOCK_1_COINBASE).getBalance()).isEqualTo(Wei.ZERO);
}

@Test
public void rewardBeneficiary_zeroReward_skipZeroRewardsTrue() {
void rewardBeneficiary_zeroReward_skipZeroRewardsTrue() {
final GenesisConfigOptions genesisConfigOptions =
GenesisConfigFile.fromConfig("{\"config\": {\"ethash\": {\"fixeddifficulty\":1}}}")
.getConfigOptions();
ProtocolSchedule protocolSchedule =
new ProtocolScheduleBuilder(
genesisConfigOptions,
BigInteger.valueOf(42),
ProtocolSpecAdapters.create(0, Function.identity()),
PrivacyParameters.DEFAULT,
false,
genesisConfigOptions.isQuorum(),
EvmConfiguration.DEFAULT)
.createProtocolSchedule();
final ExecutionContextTestFixture executionContextTestFixture =
ExecutionContextTestFixture.builder()
.protocolSchedule(
new ProtocolScheduleBuilder(
genesisConfigOptions,
BigInteger.valueOf(42),
ProtocolSpecAdapters.create(0, Function.identity()),
PrivacyParameters.DEFAULT,
false,
genesisConfigOptions.isQuorum(),
EvmConfiguration.DEFAULT)
.createProtocolSchedule())
.build();
ExecutionContextTestFixture.builder().protocolSchedule(protocolSchedule).build();

final PoWSolver solver =
new PoWSolver(
Expand Down Expand Up @@ -315,7 +320,13 @@ public void rewardBeneficiary_zeroReward_skipZeroRewardsTrue() {
.buildProcessableBlockHeader();

blockCreator.rewardBeneficiary(
mutableWorldState, header, Collections.emptyList(), BLOCK_1_COINBASE, Wei.ZERO, true);
mutableWorldState,
header,
Collections.emptyList(),
BLOCK_1_COINBASE,
Wei.ZERO,
true,
protocolSchedule.getByBlockHeader(header));

assertThat(mutableWorldState.get(BLOCK_1_COINBASE)).isNull();
}
Expand Down

0 comments on commit 9034d31

Please sign in to comment.