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

* 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>

* Unit tests missing mock

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Unit tests missing mock

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Refactor and reduce code duplication

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matt Whitehead <matthew.whitehead@kaleido.io>
pull/6832/head
Matt Whitehead 8 months ago committed by GitHub
parent f2c2512ef6
commit 5bc81ae181
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 1
      CHANGELOG.md
  2. 4
      consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java
  3. 23
      consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java
  4. 121
      consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java
  5. 4
      consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRoundTest.java
  6. 44
      consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/PkiQbftBlockCreator.java
  7. 6
      consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/blockcreation/QbftBlockCreatorFactory.java
  8. 31
      consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/blockcreation/PkiQbftBlockCreatorTest.java
  9. 6
      consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/messagewrappers/ProposalTest.java
  10. 20
      ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java
  11. 2
      ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockCreator.java

@ -33,6 +33,7 @@
- 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)
- In JSON-RPC return optional `v` fields for type 1 and type 2 transactions [#6762](https://github.com/hyperledger/besu/pull/6762)
- Fix Shanghai/QBFT block import bug when syncing new nodes [#6765](https://github.com/hyperledger/besu/pull/6765)
### Download Links

@ -30,7 +30,6 @@ 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.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 @@ public abstract class BaseBftProtocolScheduleBuilder {
.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));
}

@ -19,6 +19,7 @@ import org.hyperledger.besu.consensus.common.ForksSchedule;
import org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions;
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.datatypes.Address;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.blockcreation.AbstractBlockCreator;
@ -29,7 +30,10 @@ import org.hyperledger.besu.ethereum.core.SealableBlockHeader;
import org.hyperledger.besu.ethereum.eth.manager.EthScheduler;
import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
import java.util.Collections;
import java.util.Optional;
/** The Bft block creator. */
@ -77,12 +81,31 @@ public class BftBlockCreator extends AbstractBlockCreator {
this.bftExtraDataCodec = bftExtraDataCodec;
}
@Override
public BlockCreationResult createBlock(final long timestamp) {
ProtocolSpec protocolSpec =
((BftProtocolSchedule) protocolSchedule)
.getByBlockNumberOrTimestamp(parentHeader.getNumber() + 1, timestamp);
if (protocolSpec.getWithdrawalsValidator() instanceof WithdrawalsValidator.AllowedWithdrawals) {
return createEmptyWithdrawalsBlock(timestamp);
} else {
return createBlock(Optional.empty(), Optional.empty(), timestamp);
}
}
private static MiningBeneficiaryCalculator miningBeneficiaryCalculator(
final Address localAddress, final ForksSchedule<? extends BftConfigOptions> forksSchedule) {
return blockNum ->
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 =

@ -213,4 +213,125 @@ public class BftBlockCreatorTest {
new BftBlockHashing(bftExtraDataEncoder)
.calculateDataHashForCommittedSeal(header, extraData));
}
@Test
public void testBlockCreationResultsInEmptyWithdrawalsListShanghaiOnwards() {
// Construct a parent block.
final BlockHeaderTestFixture blockHeaderBuilder = new BlockHeaderTestFixture();
blockHeaderBuilder.gasLimit(5000); // required to pass validation rule checks.
final BlockHeader parentHeader = blockHeaderBuilder.buildHeader();
final Optional<BlockHeader> optionalHeader = Optional.of(parentHeader);
// Construct a blockchain and world state
final MutableBlockchain blockchain = mock(MutableBlockchain.class);
when(blockchain.getChainHeadHash()).thenReturn(parentHeader.getHash());
when(blockchain.getBlockHeader(any())).thenReturn(optionalHeader);
final BlockHeader blockHeader = mock(BlockHeader.class);
when(blockHeader.getBaseFee()).thenReturn(Optional.empty());
when(blockchain.getChainHeadHeader()).thenReturn(blockHeader);
final List<Address> initialValidatorList = Lists.newArrayList();
for (int i = 0; i < 4; i++) {
initialValidatorList.add(AddressHelpers.ofValue(i));
}
final IbftExtraDataCodec bftExtraDataEncoder = new IbftExtraDataCodec();
final BaseBftProtocolScheduleBuilder bftProtocolSchedule =
new BaseBftProtocolScheduleBuilder() {
@Override
public BlockHeaderValidator.Builder createBlockHeaderRuleset(
final BftConfigOptions config, final FeeMarket feeMarket) {
return IbftBlockHeaderValidationRulesetFactory.blockHeaderValidator(
5, Optional.empty());
}
};
final GenesisConfigOptions configOptions =
GenesisConfigFile.fromConfig("{\"config\": {\"shanghaiTime\":0}}").getConfigOptions();
final ForksSchedule<BftConfigOptions> forksSchedule =
new ForksSchedule<>(List.of(new ForkSpec<>(0, configOptions.getBftConfigOptions())));
final ProtocolSchedule protocolSchedule =
bftProtocolSchedule.createProtocolSchedule(
configOptions,
forksSchedule,
PrivacyParameters.DEFAULT,
false,
bftExtraDataEncoder,
EvmConfiguration.DEFAULT,
MiningParameters.MINING_DISABLED,
new BadBlockManager());
final ProtocolContext protContext =
new ProtocolContext(
blockchain,
createInMemoryWorldStateArchive(),
setupContextWithBftExtraDataEncoder(initialValidatorList, bftExtraDataEncoder),
new BadBlockManager());
final TransactionPoolConfiguration poolConf =
ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build();
final GasPricePendingTransactionsSorter pendingTransactions =
new GasPricePendingTransactionsSorter(
poolConf,
TestClock.system(ZoneId.systemDefault()),
metricsSystem,
blockchain::getChainHeadHeader);
final EthContext ethContext = mock(EthContext.class, RETURNS_DEEP_STUBS);
when(ethContext.getEthPeers().subscribeConnect(any())).thenReturn(1L);
final TransactionPool transactionPool =
new TransactionPool(
() -> pendingTransactions,
protocolSchedule,
protContext,
mock(TransactionBroadcaster.class),
ethContext,
new TransactionPoolMetrics(metricsSystem),
poolConf);
transactionPool.setEnabled();
final MiningParameters miningParameters =
ImmutableMiningParameters.builder()
.mutableInitValues(
MutableInitValues.builder()
.extraData(
bftExtraDataEncoder.encode(
new BftExtraData(
Bytes.wrap(new byte[32]),
Collections.emptyList(),
Optional.empty(),
0,
initialValidatorList)))
.minTransactionGasPrice(Wei.ZERO)
.coinbase(AddressHelpers.ofValue(1))
.build())
.build();
final BftBlockCreator blockCreator =
new BftBlockCreator(
miningParameters,
forksSchedule,
initialValidatorList.get(0),
parent ->
bftExtraDataEncoder.encode(
new BftExtraData(
Bytes.wrap(new byte[32]),
Collections.emptyList(),
Optional.empty(),
0,
initialValidatorList)),
transactionPool,
protContext,
protocolSchedule,
parentHeader,
bftExtraDataEncoder,
new DeterministicEthScheduler());
final Block block = blockCreator.createBlock(parentHeader.getTimestamp() + 1).getBlock();
// Test that a BFT block contains an empty withdrawals list (not an optional empty)
assertThat(block.getBody().getWithdrawals().isPresent()).isTrue();
}
}

@ -59,6 +59,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
import org.hyperledger.besu.ethereum.core.BlockImporter;
import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
import org.hyperledger.besu.util.Subscribers;
@ -273,6 +274,9 @@ public class IbftRoundTest {
@Test
public void localNodeProposesToNetworkOfTwoValidatorsImportsOnReceptionOfCommitFromPeer() {
lenient()
.when(protocolSpec.getWithdrawalsValidator())
.thenReturn(new WithdrawalsValidator.AllowedWithdrawals());
final RoundState roundState = new RoundState(roundIdentifier, 2, messageValidator);
final IbftRound round =
new IbftRound(

@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument;
import org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions;
import org.hyperledger.besu.consensus.common.bft.BftExtraData;
import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
import org.hyperledger.besu.consensus.common.bft.BftProtocolSchedule;
import org.hyperledger.besu.consensus.qbft.pki.PkiBlockCreationConfiguration;
import org.hyperledger.besu.consensus.qbft.pki.PkiQbftBlockHeaderFunctions;
import org.hyperledger.besu.consensus.qbft.pki.PkiQbftExtraData;
@ -29,6 +30,9 @@ import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockHeaderBuilder;
import org.hyperledger.besu.ethereum.core.Transaction;
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.pki.cms.CmsCreator;
import java.util.Collections;
@ -48,6 +52,8 @@ public class PkiQbftBlockCreator implements BlockCreator {
private final BlockCreator blockCreator;
private final PkiQbftExtraDataCodec pkiQbftExtraDataCodec;
private final CmsCreator cmsCreator;
private final BlockHeader parentHeader;
private final ProtocolSchedule protocolSchedule;
/**
* Instantiates a new Pki qbft block creator.
@ -55,17 +61,24 @@ public class PkiQbftBlockCreator implements BlockCreator {
* @param blockCreator the block creator
* @param pkiBlockCreationConfiguration the pki block creation configuration
* @param pkiQbftExtraDataCodec the pki qbft extra data codec
* @param parentHeader the block header of the parent block
* @param protocolSchedule the protocol schedule (the type of block can vary based on the current
* protocol spec)
*/
public PkiQbftBlockCreator(
final BlockCreator blockCreator,
final PkiBlockCreationConfiguration pkiBlockCreationConfiguration,
final BftExtraDataCodec pkiQbftExtraDataCodec) {
final BftExtraDataCodec pkiQbftExtraDataCodec,
final BlockHeader parentHeader,
final ProtocolSchedule protocolSchedule) {
this(
blockCreator,
new CmsCreator(
pkiBlockCreationConfiguration.getKeyStore(),
pkiBlockCreationConfiguration.getCertificateAlias()),
pkiQbftExtraDataCodec);
pkiQbftExtraDataCodec,
parentHeader,
protocolSchedule);
}
/**
@ -74,14 +87,21 @@ public class PkiQbftBlockCreator implements BlockCreator {
* @param blockCreator the block creator
* @param cmsCreator the cms creator
* @param bftExtraDataCodec the bft extra data codec
* @param parentHeader the block header of the parent block
* @param protocolSchedule the protocol schedule (the type of block can vary based on the current
* protocol spec)
*/
@VisibleForTesting
public PkiQbftBlockCreator(
final BlockCreator blockCreator,
final CmsCreator cmsCreator,
final BftExtraDataCodec bftExtraDataCodec) {
final BftExtraDataCodec bftExtraDataCodec,
final BlockHeader parentHeader,
final ProtocolSchedule protocolSchedule) {
this.blockCreator = blockCreator;
this.cmsCreator = cmsCreator;
this.protocolSchedule = protocolSchedule;
this.parentHeader = parentHeader;
checkArgument(
bftExtraDataCodec instanceof PkiQbftExtraDataCodec,
@ -91,7 +111,16 @@ public class PkiQbftBlockCreator implements BlockCreator {
@Override
public BlockCreationResult createBlock(final long timestamp) {
final BlockCreationResult blockCreationResult = blockCreator.createBlock(timestamp);
ProtocolSpec protocolSpec =
((BftProtocolSchedule) protocolSchedule)
.getByBlockNumberOrTimestamp(parentHeader.getNumber() + 1, timestamp);
final BlockCreationResult blockCreationResult;
if (protocolSpec.getWithdrawalsValidator() instanceof WithdrawalsValidator.AllowedWithdrawals) {
blockCreationResult = blockCreator.createEmptyWithdrawalsBlock(timestamp);
} else {
blockCreationResult = blockCreator.createBlock(timestamp);
}
return replaceCmsInBlock(blockCreationResult);
}
@ -114,6 +143,13 @@ public class PkiQbftBlockCreator implements BlockCreator {
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();

@ -77,7 +77,11 @@ public class QbftBlockCreatorFactory extends BftBlockCreatorFactory<QbftConfigOp
return blockCreator;
} else {
return new PkiQbftBlockCreator(
blockCreator, qbftContext.getPkiBlockCreationConfiguration().get(), bftExtraDataCodec);
blockCreator,
qbftContext.getPkiBlockCreationConfiguration().get(),
bftExtraDataCodec,
parentHeader,
protocolSchedule);
}
}

@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hyperledger.besu.consensus.common.bft.BftExtraDataFixture.createExtraData;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@ -25,6 +26,7 @@ import static org.mockito.Mockito.when;
import org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions;
import org.hyperledger.besu.consensus.common.bft.BftExtraData;
import org.hyperledger.besu.consensus.common.bft.BftProtocolSchedule;
import org.hyperledger.besu.consensus.qbft.QbftExtraDataCodec;
import org.hyperledger.besu.consensus.qbft.pki.PkiQbftBlockHeaderFunctions;
import org.hyperledger.besu.consensus.qbft.pki.PkiQbftExtraData;
@ -37,6 +39,8 @@ import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockBody;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
import org.hyperledger.besu.pki.cms.CmsCreator;
import java.util.Collections;
@ -53,21 +57,38 @@ public class PkiQbftBlockCreatorTest {
private CmsCreator cmsCreator;
private PkiQbftBlockCreator pkiQbftBlockCreator;
private BlockHeaderTestFixture blockHeaderBuilder;
private BlockHeader blockHeader;
private BftProtocolSchedule protocolSchedule;
private ProtocolSpec protocolSpec;
@BeforeEach
public void before() {
blockCreator = mock(BlockCreator.class);
cmsCreator = mock(CmsCreator.class);
blockHeader = mock(BlockHeader.class);
protocolSchedule = mock(BftProtocolSchedule.class);
protocolSpec = mock(ProtocolSpec.class);
pkiQbftBlockCreator = new PkiQbftBlockCreator(blockCreator, cmsCreator, extraDataCodec);
pkiQbftBlockCreator =
new PkiQbftBlockCreator(
blockCreator, cmsCreator, extraDataCodec, blockHeader, protocolSchedule);
blockHeaderBuilder = new BlockHeaderTestFixture();
when(protocolSchedule.getByBlockNumberOrTimestamp(anyLong(), anyLong()))
.thenReturn(protocolSpec);
}
@Test
public void createProposalBehaviourWithNonPkiCodecFails() {
assertThatThrownBy(
() -> new PkiQbftBlockCreator(blockCreator, cmsCreator, new QbftExtraDataCodec()))
() ->
new PkiQbftBlockCreator(
blockCreator,
cmsCreator,
new QbftExtraDataCodec(),
blockHeader,
protocolSchedule))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("PkiQbftBlockCreator must use PkiQbftExtraDataCodec");
}
@ -75,6 +96,8 @@ public class PkiQbftBlockCreatorTest {
@Test
public void cmsInProposedBlockHasValueCreatedByCmsCreator() {
createBlockBeingProposed();
when(protocolSpec.getWithdrawalsValidator())
.thenReturn(new WithdrawalsValidator.AllowedWithdrawals());
final Bytes cms = Bytes.random(32);
when(cmsCreator.create(any(Bytes.class))).thenReturn(cms);
@ -89,6 +112,8 @@ public class PkiQbftBlockCreatorTest {
@Test
public void cmsIsCreatedWithCorrectHashingFunction() {
when(protocolSpec.getWithdrawalsValidator())
.thenReturn(new WithdrawalsValidator.ProhibitedWithdrawals());
final Block block = createBlockBeingProposed();
final Hash expectedHashForCmsCreation =
PkiQbftBlockHeaderFunctions.forCmsSignature(extraDataCodec).hash(block.getHeader());
@ -124,6 +149,8 @@ public class PkiQbftBlockCreatorTest {
new BlockBody(Collections.emptyList(), Collections.emptyList()));
when(blockCreator.createBlock(eq(1L)))
.thenReturn(new BlockCreationResult(block, new TransactionSelectionResults()));
when(blockCreator.createEmptyWithdrawalsBlock(anyLong()))
.thenReturn(new BlockCreationResult(block, new TransactionSelectionResults()));
return block;
}

@ -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() {

@ -159,6 +159,26 @@ public abstract class AbstractBlockCreator implements AsyncBlockCreator {
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,

@ -44,6 +44,8 @@ public interface BlockCreator {
BlockCreationResult createBlock(final long timestamp);
BlockCreationResult createEmptyWithdrawalsBlock(final long timestamp);
BlockCreationResult createBlock(
final List<Transaction> transactions, final List<BlockHeader> ommers, final long timestamp);

Loading…
Cancel
Save