Ensure empty withdrawal lists are set in BFT blocks when the protocol spec is shanghai or higher

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
 

consensus/common/src/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));
   }

consensus/common/src/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 =

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());

consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRoundTest.java

@@ -139,6 +139,10 @@ public void setup() {
         .when(blockImporter.importBlock(any(), any(), any()))
         .thenReturn(new BlockImportResult(BlockImportResult.BlockImportStatus.IMPORTED));
 
+    lenient()
+        .when(protocolSchedule.getByBlockNumberOrTimestamp(anyLong(), anyLong()))
+        .thenReturn(protocolSpec);
+
     subscribers.subscribe(minedBlockObserver);
   }
 

consensus/qbft/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();

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());

consensus/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() {

consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundTest.java

@@ -147,6 +147,9 @@ QbftContext.class, emptyList(), new QbftExtraDataCodec()),
     when(blockImporter.importBlock(any(), any(), any()))
         .thenReturn(new BlockImportResult(BlockImportResult.BlockImportStatus.IMPORTED));
 
+    when(protocolSchedule.getByBlockNumberOrTimestamp(anyLong(), anyLong()))
+        .thenReturn(protocolSpec);
+
     subscribers.subscribe(minedBlockObserver);
   }
 

ethereum/blockcreation/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,

ethereum/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);