Ignore min-block-occupancy-ratio option when on PoS (#5491)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
This commit is contained in:
Fabio Di Fabio
2023-05-26 10:25:19 +02:00
committed by GitHub
parent df5599172e
commit 610812c841
10 changed files with 245 additions and 25 deletions

View File

@@ -16,6 +16,7 @@
### Breaking Changes
- Add request content length limit for the JSON-RPC API (5MB) [#5467](https://github.com/hyperledger/besu/pull/5467)
- `min-block-occupancy-ratio` options is now ignored on PoS networks [#5491](https://github.com/hyperledger/besu/pull/5491)
### Additions and Improvements
- Set the retention policy for RocksDB log files to maintain only the logs from the last week [#5428](https://github.com/hyperledger/besu/pull/5428)
@@ -27,6 +28,7 @@
- Early access - layered transaction pool implementation [#5290](https://github.com/hyperledger/besu/pull/5290)
- New RPC method `debug_getRawReceipts` [#5476](https://github.com/hyperledger/besu/pull/5476)
- Add TrieLogFactory plugin support [#5440](https://github.com/hyperledger/besu/pull/5440)
- Ignore `min-block-occupancy-ratio` option when on PoS networks, since in some cases, it prevents to have full blocks even if enough transactions are present [#5491](https://github.com/hyperledger/besu/pull/5491)
### Bug Fixes
- Fix eth_feeHistory response for the case in which blockCount is higher than highestBlock requested. [#5397](https://github.com/hyperledger/besu/pull/5397)

View File

@@ -36,6 +36,14 @@ import org.apache.tuweni.bytes.Bytes32;
/** The Merge block creator. */
class MergeBlockCreator extends AbstractBlockCreator {
/**
* On PoS you do not need to compete with other nodes for block production, since you have an
* allocated slot for that, so in this case make sense to always try to fill the block, if there
* are enough pending transactions, until the remaining gas is less than the minimum needed for
* the smaller transaction. So for PoS the min-block-occupancy-ratio option is set to always try
* to fill 100% of the block.
*/
private static final double TRY_FILL_BLOCK = 1.0;
/**
* Instantiates a new Merge block creator.
@@ -48,7 +56,6 @@ class MergeBlockCreator extends AbstractBlockCreator {
* @param protocolSchedule the protocol schedule
* @param minTransactionGasPrice the min transaction gas price
* @param miningBeneficiary the mining beneficiary
* @param minBlockOccupancyRatio the min block occupancy ratio
* @param parentHeader the parent header
*/
public MergeBlockCreator(
@@ -60,7 +67,6 @@ class MergeBlockCreator extends AbstractBlockCreator {
final ProtocolSchedule protocolSchedule,
final Wei minTransactionGasPrice,
final Address miningBeneficiary,
final Double minBlockOccupancyRatio,
final BlockHeader parentHeader) {
super(
miningBeneficiary,
@@ -71,7 +77,7 @@ class MergeBlockCreator extends AbstractBlockCreator {
protocolContext,
protocolSchedule,
minTransactionGasPrice,
minBlockOccupancyRatio,
TRY_FILL_BLOCK,
parentHeader);
}

View File

@@ -136,7 +136,6 @@ public class MergeCoordinator implements MergeMiningCoordinator, BadChainListene
protocolSchedule,
this.miningParameters.getMinTransactionGasPrice(),
address.or(miningParameters::getCoinbase).orElse(Address.ZERO),
this.miningParameters.getMinBlockOccupancyRatio(),
parentHeader);
this.backwardSyncContext.subscribeBadChainListener(this);

View File

@@ -254,7 +254,6 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
protocolSchedule,
this.miningParameters.getMinTransactionGasPrice(),
address.or(miningParameters::getCoinbase).orElse(Address.ZERO),
this.miningParameters.getMinBlockOccupancyRatio(),
parentHeader));
doCallRealMethod()

View File

@@ -307,6 +307,9 @@ public class BlockTransactionSelector {
if (blockOccupancyAboveThreshold()) {
LOG.trace("Block occupancy above threshold, completing operation");
return TransactionSelectionResult.COMPLETE_OPERATION;
} else if (blockFull()) {
LOG.trace("Block full, completing operation");
return TransactionSelectionResult.COMPLETE_OPERATION;
} else {
return TransactionSelectionResult.CONTINUE;
}
@@ -458,15 +461,34 @@ public class BlockTransactionSelector {
}
private boolean blockOccupancyAboveThreshold() {
final double gasAvailable = processableBlockHeader.getGasLimit();
final double gasUsed = transactionSelectionResult.getCumulativeGasUsed();
final double occupancyRatio = gasUsed / gasAvailable;
final long gasAvailable = processableBlockHeader.getGasLimit();
final long gasUsed = transactionSelectionResult.getCumulativeGasUsed();
final long gasRemaining = gasAvailable - gasUsed;
final double occupancyRatio = (double) gasUsed / (double) gasAvailable;
LOG.trace(
"Min block occupancy ratio {}, gas used {}, available {}, used/available {}",
"Min block occupancy ratio {}, gas used {}, available {}, remaining {}, used/available {}",
minBlockOccupancyRatio,
gasUsed,
gasAvailable,
gasRemaining,
occupancyRatio);
return occupancyRatio >= minBlockOccupancyRatio;
}
private boolean blockFull() {
final long gasAvailable = processableBlockHeader.getGasLimit();
final long gasUsed = transactionSelectionResult.getCumulativeGasUsed();
final long gasRemaining = gasAvailable - gasUsed;
if (gasRemaining < gasCalculator.getMinimumTransactionCost()) {
LOG.trace(
"Block full, remaining gas {} is less than minimum transaction gas cost {}",
gasRemaining,
gasCalculator.getMinimumTransactionCost());
return true;
}
return false;
}
}

View File

@@ -52,6 +52,7 @@ import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
import org.hyperledger.besu.ethereum.referencetests.ReferenceTestBlockchain;
import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.hyperledger.besu.evm.gascalculator.LondonGasCalculator;
import org.hyperledger.besu.evm.internal.EvmConfiguration;
import org.hyperledger.besu.evm.worldstate.WorldState;
@@ -78,6 +79,8 @@ import org.mockito.junit.MockitoJUnitRunner;
@RunWith(MockitoJUnitRunner.class)
public abstract class AbstractBlockTransactionSelectorTest {
protected static final double MIN_OCCUPANCY_80_PERCENT = 0.8;
protected static final double MIN_OCCUPANCY_100_PERCENT = 1;
protected static final KeyPair keyPair =
SignatureAlgorithmFactory.getInstance().generateKeyPair();
protected final MetricsSystem metricsSystem = new NoOpMetricsSystem();
@@ -131,7 +134,12 @@ public abstract class AbstractBlockTransactionSelectorTest {
final BlockTransactionSelector selector =
createBlockSelector(
mainnetTransactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
mainnetTransactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();
@@ -155,7 +163,12 @@ public abstract class AbstractBlockTransactionSelectorTest {
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();
@@ -187,7 +200,12 @@ public abstract class AbstractBlockTransactionSelectorTest {
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();
@@ -215,7 +233,12 @@ public abstract class AbstractBlockTransactionSelectorTest {
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();
@@ -306,12 +329,17 @@ public abstract class AbstractBlockTransactionSelectorTest {
@Test
public void transactionTooLargeForBlockDoesNotPreventMoreBeingAddedIfBlockOccupancyNotReached() {
final ProcessableBlockHeader blockHeader = createBlock(300);
final ProcessableBlockHeader blockHeader = createBlock(300_000);
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final TransactionTestFixture txTestFixture = new TransactionTestFixture();
// Add 3 transactions to the Pending Transactions, 79% of block, 100% of block and 10% of block
@@ -351,7 +379,12 @@ public abstract class AbstractBlockTransactionSelectorTest {
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final TransactionTestFixture txTestFixture = new TransactionTestFixture();
// Add 4 transactions to the Pending Transactions 15% (ok), 79% (ok), 25% (too large), 10%
@@ -393,6 +426,106 @@ public abstract class AbstractBlockTransactionSelectorTest {
assertThat(results.getTransactions().contains(txs[2])).isFalse();
}
@Test
public void transactionSelectionStopsWhenBlockIsFull() {
final ProcessableBlockHeader blockHeader = createBlock(3_000_000);
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_100_PERCENT);
final long minTxGasCost = getGasCalculator().getMinimumTransactionCost();
// Add 4 transactions to the Pending Transactions
// 0) 90% of block (selected)
// 1) 90% of block (skipped since too large)
// 2) enough gas to only leave space for a transaction with the min gas cost (selected)
// 3) min gas cost (selected and 100% block gas used)
// 4) min gas cost (not selected since selection stopped after tx 3)
// NOTE - PendingTransactions outputs these in nonce order
final long gasLimit0 = (long) (blockHeader.getGasLimit() * 0.9);
final long gasLimit1 = (long) (blockHeader.getGasLimit() * 0.9);
final long gasLimit2 = blockHeader.getGasLimit() - gasLimit0 - minTxGasCost;
final long gasLimit3 = minTxGasCost;
final long gasLimit4 = minTxGasCost;
final TransactionTestFixture txTestFixture = new TransactionTestFixture();
final List<Transaction> transactionsToInject = Lists.newArrayList();
transactionsToInject.add(txTestFixture.gasLimit(gasLimit0).nonce(0).createTransaction(keyPair));
transactionsToInject.add(txTestFixture.gasLimit(gasLimit1).nonce(1).createTransaction(keyPair));
transactionsToInject.add(txTestFixture.gasLimit(gasLimit2).nonce(2).createTransaction(keyPair));
transactionsToInject.add(txTestFixture.gasLimit(gasLimit3).nonce(3).createTransaction(keyPair));
transactionsToInject.add(txTestFixture.gasLimit(gasLimit4).nonce(4).createTransaction(keyPair));
for (final Transaction tx : transactionsToInject) {
pendingTransactions.addRemoteTransaction(tx, Optional.empty());
ensureTransactionIsValid(tx);
}
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();
assertThat(results.getTransactions())
.containsExactly(
transactionsToInject.get(0), transactionsToInject.get(2), transactionsToInject.get(3));
assertThat(results.getCumulativeGasUsed()).isEqualTo(blockHeader.getGasLimit());
}
@Test
public void transactionSelectionStopsWhenRemainingGasIsNotEnoughForAnyMoreTransaction() {
final ProcessableBlockHeader blockHeader = createBlock(3_000_000);
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_100_PERCENT);
final long minTxGasCost = getGasCalculator().getMinimumTransactionCost();
// Add 4 transactions to the Pending Transactions
// 0) 90% of block (selected)
// 1) 90% of block (skipped since too large)
// 2) do not fill the block, but leaves less gas than the min for a tx (selected)
// 3) min gas cost (skipped since not enough gas remaining)
// NOTE - PendingTransactions outputs these in nonce order
final long gasLimit0 = (long) (blockHeader.getGasLimit() * 0.9);
final long gasLimit1 = (long) (blockHeader.getGasLimit() * 0.9);
final long gasLimit2 = blockHeader.getGasLimit() - gasLimit0 - (minTxGasCost - 1);
final long gasLimit3 = minTxGasCost;
final TransactionTestFixture txTestFixture = new TransactionTestFixture();
final List<Transaction> transactionsToInject = Lists.newArrayList();
transactionsToInject.add(txTestFixture.gasLimit(gasLimit0).nonce(0).createTransaction(keyPair));
transactionsToInject.add(txTestFixture.gasLimit(gasLimit1).nonce(1).createTransaction(keyPair));
transactionsToInject.add(txTestFixture.gasLimit(gasLimit2).nonce(2).createTransaction(keyPair));
transactionsToInject.add(txTestFixture.gasLimit(gasLimit3).nonce(3).createTransaction(keyPair));
for (final Transaction tx : transactionsToInject) {
pendingTransactions.addRemoteTransaction(tx, Optional.empty());
ensureTransactionIsValid(tx);
}
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();
assertThat(results.getTransactions())
.containsExactly(transactionsToInject.get(0), transactionsToInject.get(2));
assertThat(blockHeader.getGasLimit() - results.getCumulativeGasUsed()).isLessThan(minTxGasCost);
}
@Test
public void shouldDiscardTransactionsThatFailValidation() {
final ProcessableBlockHeader blockHeader = createBlock(300);
@@ -400,7 +533,12 @@ public abstract class AbstractBlockTransactionSelectorTest {
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final TransactionTestFixture txTestFixture = new TransactionTestFixture();
final Transaction validTransaction =
@@ -436,7 +574,12 @@ public abstract class AbstractBlockTransactionSelectorTest {
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();
@@ -451,7 +594,8 @@ public abstract class AbstractBlockTransactionSelectorTest {
final ProcessableBlockHeader blockHeader,
final Wei minGasPrice,
final Address miningBeneficiary,
final Wei dataGasPrice) {
final Wei dataGasPrice,
final double minBlockOccupancyRatio) {
final BlockTransactionSelector selector =
new BlockTransactionSelector(
transactionProcessor,
@@ -461,16 +605,18 @@ public abstract class AbstractBlockTransactionSelectorTest {
blockHeader,
this::createReceipt,
minGasPrice,
0.8,
minBlockOccupancyRatio,
this::isCancelled,
miningBeneficiary,
dataGasPrice,
getFeeMarket(),
new LondonGasCalculator(),
getGasCalculator(),
GasLimitCalculator.constant());
return selector;
}
protected abstract GasCalculator getGasCalculator();
protected abstract FeeMarket getFeeMarket();
private Transaction createTransaction(final int transactionNumber) {

View File

@@ -22,6 +22,8 @@ import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolCo
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
import org.hyperledger.besu.ethereum.eth.transactions.sorter.GasPricePendingTransactionsSorter;
import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
import org.hyperledger.besu.evm.gascalculator.BerlinGasCalculator;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.hyperledger.besu.testutil.TestClock;
import java.time.ZoneId;
@@ -53,4 +55,9 @@ public class LegacyFeeMarketBlockTransactionSelectorTest
protected FeeMarket getFeeMarket() {
return FeeMarket.legacy();
}
@Override
protected GasCalculator getGasCalculator() {
return new BerlinGasCalculator();
}
}

View File

@@ -29,6 +29,8 @@ import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolCo
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions;
import org.hyperledger.besu.ethereum.eth.transactions.sorter.BaseFeePendingTransactionsSorter;
import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.hyperledger.besu.evm.gascalculator.LondonGasCalculator;
import org.hyperledger.besu.plugin.data.TransactionType;
import org.hyperledger.besu.testutil.TestClock;
@@ -54,6 +56,11 @@ public class LondonFeeMarketBlockTransactionSelectorTest
LondonFeeMarketBlockTransactionSelectorTest::mockBlockHeader);
}
@Override
protected GasCalculator getGasCalculator() {
return new LondonGasCalculator();
}
private static BlockHeader mockBlockHeader() {
final BlockHeader blockHeader = mock(BlockHeader.class);
when(blockHeader.getBaseFee()).thenReturn(Optional.of(Wei.ONE));
@@ -72,7 +79,12 @@ public class LondonFeeMarketBlockTransactionSelectorTest
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.of(6), miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.of(6),
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
// tx is willing to pay max 6 wei for gas, but current network condition (baseFee == 1)
// result in it paying 2 wei, that is below the minimum accepted by the node, so it is skipped
@@ -93,7 +105,12 @@ public class LondonFeeMarketBlockTransactionSelectorTest
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.of(6), miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.of(6),
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
// tx is willing to pay max 6 wei for gas, and current network condition (baseFee == 5)
// result in it paying the max, that is >= the minimum accepted by the node, so it is selected
@@ -116,7 +133,12 @@ public class LondonFeeMarketBlockTransactionSelectorTest
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.of(6), miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.of(6),
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
// tx is willing to pay max 6 wei for gas, but current network condition (baseFee == 1)
// result in it paying 2 wei, that is below the minimum accepted by the node, but since it is
@@ -168,7 +190,12 @@ public class LondonFeeMarketBlockTransactionSelectorTest
final Address miningBeneficiary = AddressHelpers.ofValue(1);
final BlockTransactionSelector selector =
createBlockSelector(
transactionProcessor, blockHeader, Wei.ZERO, miningBeneficiary, Wei.ZERO);
transactionProcessor,
blockHeader,
Wei.ZERO,
miningBeneficiary,
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT);
final BlockTransactionSelector.TransactionSelectionResults results =
selector.buildTransactionListForBlock();

View File

@@ -489,4 +489,9 @@ public class FrontierGasCalculator implements GasCalculator {
public long getMaximumTransactionCost(final int size) {
return TX_BASE_COST + TX_DATA_NON_ZERO_COST * size;
}
@Override
public long getMinimumTransactionCost() {
return TX_BASE_COST;
}
}

View File

@@ -488,6 +488,13 @@ public interface GasCalculator {
// what would be the gas for a PMT with hash of all non-zeros
long getMaximumTransactionCost(int size);
/**
* Minimum gas cost of a transaction.
*
* @return the minimum gas cost
*/
long getMinimumTransactionCost();
/**
* Returns the cost of a loading from Transient Storage
*