Ensure empty withdrawal lists are set in BFT blocks when the protocol…

… schedule is shanghai or higher Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
hyperledger · Mar 19, 2024 · b8df9c8 · b8df9c8
1 parent 5c1e1e1
commit b8df9c8
Show file tree

Hide file tree

Showing 9 changed files with 114 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@
 - Fix txpool dump/restore race condition [#6665](https://github.com/hyperledger/besu/pull/6665)
 - Make block transaction selection max time aware of PoA transitions [#6676](https://github.com/hyperledger/besu/pull/6676)
 - Don't enable the BFT mining coordinator when running sub commands such as `blocks export` [#6675](https://github.com/hyperledger/besu/pull/6675)
+- Fix Shanghai/QBFT block import bug when syncing new nodes [#6765](https://github.com/hyperledger/besu/pull/6765)
 
 ### Download Links
 

diff --git a/...c/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java b/...c/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java
@@ -30,7 +30,6 @@
 import org.hyperledger.besu.ethereum.mainnet.ProtocolScheduleBuilder;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecAdapters;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecBuilder;
-import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
 import org.hyperledger.besu.evm.internal.EvmConfiguration;
 
@@ -125,9 +124,6 @@ private ProtocolSpecBuilder applyBftChanges(
         .skipZeroBlockRewards(true)
         .blockHeaderFunctions(BftBlockHeaderFunctions.forOnchainBlock(bftExtraDataCodec))
         .blockReward(Wei.of(configOptions.getBlockRewardWei()))
-        .withdrawalsValidator(
-            new WithdrawalsValidator
-                .ProhibitedWithdrawals()) // QBFT/IBFT doesn't support withdrawals
         .miningBeneficiaryCalculator(
             header -> configOptions.getMiningBeneficiary().orElseGet(header::getCoinbase));
   }

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
@@ -30,6 +30,7 @@
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 
+import java.util.Collections;
 import java.util.Optional;
 
 /** The Bft block creator. */
@@ -83,6 +84,12 @@ private static MiningBeneficiaryCalculator miningBeneficiaryCalculator(
         forksSchedule.getFork(blockNum).getValue().getMiningBeneficiary().orElse(localAddress);
   }
 
+  @Override
+  public BlockCreationResult createEmptyWithdrawalsBlock(final long timestamp) {
+    return createBlock(
+        Optional.empty(), Optional.empty(), Optional.of(Collections.emptyList()), timestamp);
+  }
+
   @Override
   protected BlockHeader createFinalBlockHeader(final SealableBlockHeader sealableBlockHeader) {
     final BlockHeaderBuilder builder =

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
@@ -21,6 +21,7 @@
 import org.hyperledger.besu.consensus.common.bft.BftExtraData;
 import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.BftHelpers;
+import org.hyperledger.besu.consensus.common.bft.BftProtocolSchedule;
 import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier;
 import org.hyperledger.besu.consensus.common.bft.RoundTimer;
 import org.hyperledger.besu.consensus.ibft.messagewrappers.Commit;
@@ -41,6 +42,8 @@
 import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
+import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
+import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
 
@@ -118,7 +121,20 @@ public ConsensusRoundIdentifier getRoundIdentifier() {
    * @param headerTimeStampSeconds the header time stamp seconds
    */
   public void createAndSendProposalMessage(final long headerTimeStampSeconds) {
-    final Block block = blockCreator.createBlock(headerTimeStampSeconds).getBlock();
+
+    // Determine if we're at a protocol spec that requires withdrawals (even if empty) to be present
+    ProtocolSpec protocolSpec =
+        ((BftProtocolSchedule) protocolSchedule)
+            .getByBlockNumberOrTimestamp(
+                roundState.getRoundIdentifier().getSequenceNumber(), headerTimeStampSeconds);
+
+    final Block block;
+    if (protocolSpec.getWithdrawalsValidator() instanceof WithdrawalsValidator.AllowedWithdrawals) {
+      block = blockCreator.createEmptyWithdrawalsBlock(headerTimeStampSeconds).getBlock();
+    } else {
+      block = blockCreator.createBlock(headerTimeStampSeconds).getBlock();
+    }
+
     final BftExtraData extraData = bftExtraDataCodec.decode(block.getHeader());
     LOG.debug("Creating proposed block. round={}", roundState.getRoundIdentifier());
     LOG.trace(
@@ -141,7 +157,20 @@ public void startRoundWith(
     final Block blockToPublish;
     if (!bestBlockFromRoundChange.isPresent()) {
       LOG.debug("Sending proposal with new block. round={}", roundState.getRoundIdentifier());
-      blockToPublish = blockCreator.createBlock(headerTimestamp).getBlock();
+
+      // Determine if we're at a protocol spec that requires withdrawals (even if empty) to be
+      // present
+      ProtocolSpec protocolSpec =
+          ((BftProtocolSchedule) protocolSchedule)
+              .getByBlockNumberOrTimestamp(
+                  roundState.getRoundIdentifier().getSequenceNumber(), headerTimestamp);
+
+      if (protocolSpec.getWithdrawalsValidator()
+          instanceof WithdrawalsValidator.AllowedWithdrawals) {
+        blockToPublish = blockCreator.createEmptyWithdrawalsBlock(headerTimestamp).getBlock();
+      } else {
+        blockToPublish = blockCreator.createBlock(headerTimestamp).getBlock();
+      }
     } else {
       LOG.debug(
           "Sending proposal from PreparedCertificate. round={}", roundState.getRoundIdentifier());

diff --git a/.../src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/PkiQbftBlockCreator.java b/.../src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/PkiQbftBlockCreator.java
@@ -114,6 +114,13 @@ public BlockCreationResult createBlock(
         timestamp);
   }
 
+  @Override
+  public BlockCreationResult createEmptyWithdrawalsBlock(final long timestamp) {
+    final BlockCreationResult blockCreationResult =
+        blockCreator.createEmptyWithdrawalsBlock(timestamp);
+    return replaceCmsInBlock(blockCreationResult);
+  }
+
   private BlockCreationResult replaceCmsInBlock(final BlockCreationResult blockCreationResult) {
     final Block block = blockCreationResult.getBlock();
     final BlockHeader blockHeader = block.getHeader();

diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java
@@ -23,6 +23,7 @@
 import org.hyperledger.besu.consensus.common.bft.BftExtraData;
 import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.BftHelpers;
+import org.hyperledger.besu.consensus.common.bft.BftProtocolSchedule;
 import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier;
 import org.hyperledger.besu.consensus.common.bft.RoundTimer;
 import org.hyperledger.besu.consensus.common.bft.payload.SignedData;
@@ -45,6 +46,8 @@
 import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
+import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
+import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
 
@@ -129,8 +132,19 @@ public ConsensusRoundIdentifier getRoundIdentifier() {
    */
   public void createAndSendProposalMessage(final long headerTimeStampSeconds) {
     LOG.debug("Creating proposed block. round={}", roundState.getRoundIdentifier());
-    final Block block = blockCreator.createBlock(headerTimeStampSeconds).getBlock();
 
+    // Determine if we're at a protocol spec that requires withdrawals (even if empty) to be present
+    ProtocolSpec protocolSpec =
+        ((BftProtocolSchedule) protocolSchedule)
+            .getByBlockNumberOrTimestamp(
+                roundState.getRoundIdentifier().getSequenceNumber(), headerTimeStampSeconds);
+
+    final Block block;
+    if (protocolSpec.getWithdrawalsValidator() instanceof WithdrawalsValidator.AllowedWithdrawals) {
+      block = blockCreator.createEmptyWithdrawalsBlock(headerTimeStampSeconds).getBlock();
+    } else {
+      block = blockCreator.createBlock(headerTimeStampSeconds).getBlock();
+    }
     LOG.trace("Creating proposed block blockHeader={}", block.getHeader());
     updateStateWithProposalAndTransmit(block, emptyList(), emptyList());
   }
@@ -148,8 +162,21 @@ public void startRoundWith(
 
     final Block blockToPublish;
     if (bestPreparedCertificate.isEmpty()) {
+
+      // Determine if we're at a protocol spec that requires withdrawals (even if empty) to be
+      // present
+      ProtocolSpec protocolSpec =
+          ((BftProtocolSchedule) protocolSchedule)
+              .getByBlockNumberOrTimestamp(
+                  roundState.getRoundIdentifier().getSequenceNumber(), headerTimestamp);
+
+      if (protocolSpec.getWithdrawalsValidator()
+          instanceof WithdrawalsValidator.AllowedWithdrawals) {
+        blockToPublish = blockCreator.createEmptyWithdrawalsBlock(headerTimestamp).getBlock();
+      } else {
+        blockToPublish = blockCreator.createBlock(headerTimestamp).getBlock();
+      }
       LOG.debug("Sending proposal with new block. round={}", roundState.getRoundIdentifier());
-      blockToPublish = blockCreator.createBlock(headerTimestamp).getBlock();
     } else {
       LOG.debug(
           "Sending proposal from PreparedCertificate. round={}", roundState.getRoundIdentifier());
@@ -175,12 +202,16 @@ protected void updateStateWithProposalAndTransmit(
       final List<SignedData<PreparePayload>> prepares) {
     final Proposal proposal;
     try {
+      // System.out.println("Creating a proposal containing the block. Withdrawals at this point: "
+      // + block.getBody().getWithdrawals());
       proposal = messageFactory.createProposal(getRoundIdentifier(), block, roundChanges, prepares);
     } catch (final SecurityModuleException e) {
       LOG.warn("Failed to create a signed Proposal, waiting for next round.", e);
       return;
     }
 
+    // System.out.println("Multicasting this proposal: " +
+    // proposal.getSignedPayload().getPayload().getProposedBlock());
     transmitter.multicastProposal(
         proposal.getRoundIdentifier(),
         proposal.getSignedPayload().getPayload().getProposedBlock(),
@@ -201,6 +232,8 @@ public void handleProposalMessage(final Proposal msg) {
         roundState.getRoundIdentifier(),
         msg.getAuthor());
     final Block block = msg.getSignedPayload().getPayload().getProposedBlock();
+    // System.out.println("Handling a proposal message: " +
+    // msg.getSignedPayload().getPayload().getProposedBlock());
     if (updateStateWithProposedBlock(msg)) {
       sendPrepare(block);
     }
@@ -258,6 +291,11 @@ private boolean updateStateWithProposedBlock(final Proposal msg) {
     final boolean wasPrepared = roundState.isPrepared();
     final boolean wasCommitted = roundState.isCommitted();
     final boolean blockAccepted = roundState.setProposedBlock(msg);
+    // System.out.println("Received QBFT block - withdrawals = " +
+    // msg.getBlock().getBody().getWithdrawals());
+    // System.out.println("Received QBFT block - withdrawals 2 = " +
+    // (roundState.getProposedBlock().isPresent() ? roundState.getProposedBlock().get() : " (not
+    // present yet)"));
 
     if (blockAccepted) {
       final Block block = roundState.getProposedBlock().get();

diff --git a/.../qbft/src/test/java/org/hyperledger/besu/consensus/qbft/messagewrappers/ProposalTest.java b/.../qbft/src/test/java/org/hyperledger/besu/consensus/qbft/messagewrappers/ProposalTest.java
@@ -51,7 +51,11 @@ public class ProposalTest {
   private static final Block BLOCK =
       new Block(
           new BlockHeaderTestFixture().extraData(bftExtraDataCodec.encode(extraData)).buildHeader(),
-          new BlockBody(Collections.emptyList(), Collections.emptyList()));
+          new BlockBody(
+              Collections.emptyList(),
+              Collections.emptyList(),
+              Optional.of(Collections.emptyList()),
+              Optional.empty()));
 
   @Test
   public void canRoundTripProposalMessage() {

diff --git a/...ation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java b/...ation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java
@@ -159,6 +159,26 @@ public BlockCreationResult createBlock(
         true);
   }
 
+  @Override
+  public BlockCreationResult createEmptyWithdrawalsBlock(final long timestamp) {
+    throw new UnsupportedOperationException("Only used by BFT block creators");
+  }
+
+  public BlockCreationResult createBlock(
+      final Optional<List<Transaction>> maybeTransactions,
+      final Optional<List<BlockHeader>> maybeOmmers,
+      final Optional<List<Withdrawal>> maybeWithdrawals,
+      final long timestamp) {
+    return createBlock(
+        maybeTransactions,
+        maybeOmmers,
+        maybeWithdrawals,
+        Optional.empty(),
+        Optional.empty(),
+        timestamp,
+        true);
+  }
+
   protected BlockCreationResult createBlock(
       final Optional<List<Transaction>> maybeTransactions,
       final Optional<List<BlockHeader>> maybeOmmers,
@@ -276,6 +296,7 @@ protected BlockCreationResult createBlock(
 
       final Optional<List<Withdrawal>> withdrawals =
           withdrawalsCanBeProcessed ? maybeWithdrawals : Optional.empty();
+      // System.out.println("MRW: Create block. Withdrawals is " + withdrawals);
       final BlockBody blockBody =
           new BlockBody(
               transactionResults.getSelectedTransactions(), ommers, withdrawals, maybeDeposits);

diff --git a/...blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockCreator.java b/...blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockCreator.java
@@ -44,6 +44,8 @@ public TransactionSelectionResults getTransactionSelectionResults() {
 
   BlockCreationResult createBlock(final long timestamp);
 
+  BlockCreationResult createEmptyWithdrawalsBlock(final long timestamp);
+
   BlockCreationResult createBlock(
       final List<Transaction> transactions, final List<BlockHeader> ommers, final long timestamp);