Decouple block creators (#7468)

* wip decoupled parent block header from block creators --------- Signed-off-by: Justin Florentine <[email protected]>
hyperledger · Sep 9, 2024 · 9a570d4 · 9a570d4
1 parent 2a52b02
commit 9a570d4
Show file tree

Hide file tree

Showing 32 changed files with 295 additions and 196 deletions.
diff --git a/...src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java b/...src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java
@@ -54,7 +54,6 @@ public class CliqueBlockCreator extends AbstractBlockCreator {
    * @param protocolContext the protocol context
    * @param protocolSchedule the protocol schedule
    * @param nodeKey the node key
-   * @param parentHeader the parent header
    * @param epochManager the epoch manager
    * @param ethScheduler the scheduler for asynchronous block creation tasks
    */
@@ -65,7 +64,6 @@ public CliqueBlockCreator(
       final ProtocolContext protocolContext,
       final ProtocolSchedule protocolSchedule,
       final NodeKey nodeKey,
-      final BlockHeader parentHeader,
       final EpochManager epochManager,
       final EthScheduler ethScheduler) {
     super(
@@ -75,7 +73,6 @@ public CliqueBlockCreator(
         transactionPool,
         protocolContext,
         protocolSchedule,
-        parentHeader,
         ethScheduler);
     this.nodeKey = nodeKey;
     this.epochManager = epochManager;
@@ -112,6 +109,8 @@ protected BlockHeader createFinalBlockHeader(final SealableBlockHeader sealableB
 
   private Optional<ValidatorVote> determineCliqueVote(
       final SealableBlockHeader sealableBlockHeader) {
+    BlockHeader parentHeader =
+        protocolContext.getBlockchain().getBlockHeader(sealableBlockHeader.getParentHash()).get();
     if (epochManager.isEpochBlock(sealableBlockHeader.getNumber())) {
       return Optional.empty();
     } else {

diff --git a/...rc/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java b/...rc/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java
@@ -103,7 +103,6 @@ public CliqueBlockMiner createMiner(
                 protocolContext,
                 protocolSchedule,
                 nodeKey,
-                header,
                 epochManager,
                 ethScheduler);
 

diff --git a/...test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/...test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java
@@ -156,11 +156,11 @@ public void proposerAddressCanBeExtractFromAConstructedBlock() {
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
-            blockchain.getChainHeadHeader(),
             epochManager,
             ethScheduler);
 
-    final Block createdBlock = blockCreator.createBlock(5L).getBlock();
+    final Block createdBlock =
+        blockCreator.createBlock(5L, blockchain.getChainHeadHeader()).getBlock();
 
     assertThat(CliqueHelpers.getProposerOfBlock(createdBlock.getHeader()))
         .isEqualTo(proposerAddress);
@@ -185,11 +185,11 @@ public void insertsValidVoteIntoConstructedBlock() {
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
-            blockchain.getChainHeadHeader(),
             epochManager,
             ethScheduler);
 
-    final Block createdBlock = blockCreator.createBlock(0L).getBlock();
+    final Block createdBlock =
+        blockCreator.createBlock(0L, blockchain.getChainHeadHeader()).getBlock();
     assertThat(createdBlock.getHeader().getNonce()).isEqualTo(CliqueBlockInterface.ADD_NONCE);
     assertThat(createdBlock.getHeader().getCoinbase()).isEqualTo(a1);
   }
@@ -219,11 +219,11 @@ public void insertsNoVoteWhenAtEpoch() {
             protocolContext,
             protocolSchedule,
             proposerNodeKey,
-            blockchain.getChainHeadHeader(),
             epochManager,
             ethScheduler);
 
-    final Block createdBlock = blockCreator.createBlock(0L).getBlock();
+    final Block createdBlock =
+        blockCreator.createBlock(0L, blockchain.getChainHeadHeader()).getBlock();
     assertThat(createdBlock.getHeader().getNonce()).isEqualTo(CliqueBlockInterface.DROP_NONCE);
     assertThat(createdBlock.getHeader().getCoinbase()).isEqualTo(Address.fromHexString("0"));
   }

diff --git a/...c/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMinerTest.java b/...c/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMinerTest.java
@@ -91,7 +91,7 @@ void doesNotMineBlockIfNoTransactionsWhenEmptyBlocksNotAllowed() throws Interrup
     final CliqueBlockCreator blockCreator = mock(CliqueBlockCreator.class);
     final Function<BlockHeader, CliqueBlockCreator> blockCreatorSupplier =
         (parentHeader) -> blockCreator;
-    when(blockCreator.createBlock(anyLong()))
+    when(blockCreator.createBlock(anyLong(), any()))
         .thenReturn(
             new BlockCreator.BlockCreationResult(
                 blockToCreate, new TransactionSelectionResults(), new BlockCreationTiming()));
@@ -147,7 +147,7 @@ void minesBlockIfHasTransactionsWhenEmptyBlocksNotAllowed() throws InterruptedEx
     final CliqueBlockCreator blockCreator = mock(CliqueBlockCreator.class);
     final Function<BlockHeader, CliqueBlockCreator> blockCreatorSupplier =
         (parentHeader) -> blockCreator;
-    when(blockCreator.createBlock(anyLong()))
+    when(blockCreator.createBlock(anyLong(), any()))
         .thenReturn(
             new BlockCreator.BlockCreationResult(
                 blockToCreate, new TransactionSelectionResults(), new BlockCreationTiming()));

diff --git a/...rc/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java b/...rc/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java
@@ -53,7 +53,6 @@ public class BftBlockCreator extends AbstractBlockCreator {
    * @param transactionPool the pending transactions
    * @param protocolContext the protocol context
    * @param protocolSchedule the protocol schedule
-   * @param parentHeader the parent header
    * @param bftExtraDataCodec the bft extra data codec
    * @param ethScheduler the scheduler for asynchronous block creation tasks
    */
@@ -65,7 +64,6 @@ public BftBlockCreator(
       final TransactionPool transactionPool,
       final ProtocolContext protocolContext,
       final ProtocolSchedule protocolSchedule,
-      final BlockHeader parentHeader,
       final BftExtraDataCodec bftExtraDataCodec,
       final EthScheduler ethScheduler) {
     super(
@@ -75,21 +73,20 @@ public BftBlockCreator(
         transactionPool,
         protocolContext,
         protocolSchedule,
-        parentHeader,
         ethScheduler);
     this.bftExtraDataCodec = bftExtraDataCodec;
   }
 
   @Override
-  public BlockCreationResult createBlock(final long timestamp) {
+  public BlockCreationResult createBlock(final long timestamp, final BlockHeader parentHeader) {
     ProtocolSpec protocolSpec =
         ((BftProtocolSchedule) protocolSchedule)
             .getByBlockNumberOrTimestamp(parentHeader.getNumber() + 1, timestamp);
 
     if (protocolSpec.getWithdrawalsValidator() instanceof WithdrawalsValidator.AllowedWithdrawals) {
-      return createEmptyWithdrawalsBlock(timestamp);
+      return createEmptyWithdrawalsBlock(timestamp, parentHeader);
     } else {
-      return createBlock(Optional.empty(), Optional.empty(), timestamp);
+      return createBlock(Optional.empty(), Optional.empty(), timestamp, parentHeader);
     }
   }
 
@@ -100,9 +97,14 @@ private static MiningBeneficiaryCalculator miningBeneficiaryCalculator(
   }
 
   @Override
-  public BlockCreationResult createEmptyWithdrawalsBlock(final long timestamp) {
+  public BlockCreationResult createEmptyWithdrawalsBlock(
+      final long timestamp, final BlockHeader parentHeader) {
     return createBlock(
-        Optional.empty(), Optional.empty(), Optional.of(Collections.emptyList()), timestamp);
+        Optional.empty(),
+        Optional.empty(),
+        Optional.of(Collections.emptyList()),
+        timestamp,
+        parentHeader);
   }
 
   @Override

diff --git a/.../java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java b/.../java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java
@@ -106,11 +106,10 @@ public BftBlockCreatorFactory(
   /**
    * Create block creator.
    *
-   * @param parentHeader the parent header
    * @param round the round
    * @return the block creator
    */
-  public BlockCreator create(final BlockHeader parentHeader, final int round) {
+  public BlockCreator create(final int round) {
     return new BftBlockCreator(
         miningParameters,
         forksSchedule,
@@ -119,7 +118,6 @@ public BlockCreator create(final BlockHeader parentHeader, final int round) {
         transactionPool,
         protocolContext,
         protocolSchedule,
-        parentHeader,
         bftExtraDataCodec,
         ethScheduler);
   }

diff --git a/...ft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContext.java b/...ft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContext.java
@@ -90,8 +90,8 @@ public Block createBlockForProposal(
       final BlockHeader parent, final int round, final long timestamp) {
     return finalState
         .getBlockCreatorFactory()
-        .create(parent, round)
-        .createBlock(timestamp)
+        .create(round)
+        .createBlock(timestamp, parent)
         .getBlock();
   }
 

diff --git a/...n-test/java/org/hyperledger/besu/consensus/ibft/tests/round/IbftRoundIntegrationTest.java b/...n-test/java/org/hyperledger/besu/consensus/ibft/tests/round/IbftRoundIntegrationTest.java
@@ -90,6 +90,7 @@ public class IbftRoundIntegrationTest {
   private MessageFactory throwingMessageFactory;
   private IbftMessageTransmitter transmitter;
   @Mock private StubValidatorMulticaster multicaster;
+  @Mock BlockHeader parentHeader;
 
   private Block proposedBlock;
 
@@ -145,7 +146,8 @@ public void signingFailsOnReceiptOfProposalUpdatesRoundButTransmitsNothing() {
             throwingMessageFactory,
             transmitter,
             roundTimer,
-            bftExtraDataEncoder);
+            bftExtraDataEncoder,
+            parentHeader);
 
     round.handleProposalMessage(
         peerMessageFactory.createProposal(roundIdentifier, proposedBlock, Optional.empty()));
@@ -172,7 +174,8 @@ public void failuresToSignStillAllowBlockToBeImported() {
             throwingMessageFactory,
             transmitter,
             roundTimer,
-            bftExtraDataEncoder);
+            bftExtraDataEncoder,
+            parentHeader);
 
     // inject a block first, then a prepare on it.
     round.handleProposalMessage(

diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java
@@ -66,6 +66,7 @@ public class IbftRound {
   private final MessageFactory messageFactory; // used only to create stored local msgs
   private final IbftMessageTransmitter transmitter;
   private final BftExtraDataCodec bftExtraDataCodec;
+  private final BlockHeader parentHeader;
 
   /**
    * Instantiates a new Ibft round.
@@ -80,6 +81,7 @@ public class IbftRound {
    * @param transmitter the transmitter
    * @param roundTimer the round timer
    * @param bftExtraDataCodec the bft extra data codec
+   * @param parentHeader the parent header
    */
   public IbftRound(
       final RoundState roundState,
@@ -91,7 +93,8 @@ public IbftRound(
       final MessageFactory messageFactory,
       final IbftMessageTransmitter transmitter,
       final RoundTimer roundTimer,
-      final BftExtraDataCodec bftExtraDataCodec) {
+      final BftExtraDataCodec bftExtraDataCodec,
+      final BlockHeader parentHeader) {
     this.roundState = roundState;
     this.blockCreator = blockCreator;
     this.protocolContext = protocolContext;
@@ -101,6 +104,7 @@ public IbftRound(
     this.messageFactory = messageFactory;
     this.transmitter = transmitter;
     this.bftExtraDataCodec = bftExtraDataCodec;
+    this.parentHeader = parentHeader;
     roundTimer.startTimer(getRoundIdentifier());
   }
 
@@ -119,7 +123,8 @@ public ConsensusRoundIdentifier getRoundIdentifier() {
    * @param headerTimeStampSeconds the header time stamp seconds
    */
   public void createAndSendProposalMessage(final long headerTimeStampSeconds) {
-    final Block block = blockCreator.createBlock(headerTimeStampSeconds).getBlock();
+    final Block block =
+        blockCreator.createBlock(headerTimeStampSeconds, this.parentHeader).getBlock();
     final BftExtraData extraData = bftExtraDataCodec.decode(block.getHeader());
     LOG.debug("Creating proposed block. round={}", roundState.getRoundIdentifier());
     LOG.trace(
@@ -142,7 +147,7 @@ public void startRoundWith(
     final Block blockToPublish;
     if (!bestBlockFromRoundChange.isPresent()) {
       LOG.debug("Sending proposal with new block. round={}", roundState.getRoundIdentifier());
-      blockToPublish = blockCreator.createBlock(headerTimestamp).getBlock();
+      blockToPublish = blockCreator.createBlock(headerTimestamp, this.parentHeader).getBlock();
     } else {
       LOG.debug(
           "Sending proposal from PreparedCertificate. round={}", roundState.getRoundIdentifier());

diff --git a/...ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRoundFactory.java b/...ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRoundFactory.java
@@ -100,7 +100,7 @@ public IbftRound createNewRound(final BlockHeader parentHeader, final int round)
   public IbftRound createNewRoundWithState(
       final BlockHeader parentHeader, final RoundState roundState) {
     final BlockCreator blockCreator =
-        blockCreatorFactory.create(parentHeader, roundState.getRoundIdentifier().getRoundNumber());
+        blockCreatorFactory.create(roundState.getRoundIdentifier().getRoundNumber());
 
     final IbftMessageTransmitter messageTransmitter =
         new IbftMessageTransmitter(messageFactory, finalState.getValidatorMulticaster());
@@ -115,6 +115,7 @@ public IbftRound createNewRoundWithState(
         messageFactory,
         messageTransmitter,
         finalState.getRoundTimer(),
-        bftExtraDataCodec);
+        bftExtraDataCodec,
+        parentHeader);
   }
 }
diff --git a/.../src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java b/.../src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java
@@ -191,12 +191,12 @@ public BlockHeaderValidator.Builder createBlockHeaderRuleset(
             transactionPool,
             protContext,
             protocolSchedule,
-            parentHeader,
             bftExtraDataEncoder,
             new DeterministicEthScheduler());
 
     final int secondsBetweenBlocks = 1;
-    final Block block = blockCreator.createBlock(parentHeader.getTimestamp() + 1).getBlock();
+    final Block block =
+        blockCreator.createBlock(parentHeader.getTimestamp() + 1, parentHeader).getBlock();
 
     final BlockHeaderValidator rules =
         IbftBlockHeaderValidationRulesetFactory.blockHeaderValidator(

diff --git a/...est/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java b/...est/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java
@@ -119,6 +119,7 @@ public class IbftBlockHeightManagerTest {
   @Mock private RoundTimer roundTimer;
   @Mock private FutureRoundProposalMessageValidator futureRoundProposalMessageValidator;
   @Mock private ValidatorMulticaster validatorMulticaster;
+  @Mock private BlockHeader parentHeader;
 
   @Captor private ArgumentCaptor<MessageData> sentMessageArgCaptor;
 
@@ -158,7 +159,7 @@ public void setup() {
     lenient().when(finalState.getQuorum()).thenReturn(3);
     when(finalState.getValidatorMulticaster()).thenReturn(validatorMulticaster);
     lenient()
-        .when(blockCreator.createBlock(anyLong()))
+        .when(blockCreator.createBlock(anyLong(), any()))
         .thenReturn(
             new BlockCreationResult(
                 createdBlock, new TransactionSelectionResults(), new BlockCreationTiming()));
@@ -210,7 +211,8 @@ public void setup() {
                   messageFactory,
                   messageTransmitter,
                   roundTimer,
-                  bftExtraDataCodec);
+                  bftExtraDataCodec,
+                  parentHeader);
             });
 
     lenient()
@@ -228,7 +230,8 @@ public void setup() {
                   messageFactory,
                   messageTransmitter,
                   roundTimer,
-                  bftExtraDataCodec);
+                  bftExtraDataCodec,
+                  parentHeader);
             });
   }