Skip to content

Commit

Permalink
Improve blob size transaction selector
Browse files Browse the repository at this point in the history
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
  • Loading branch information
fab-10 committed Jul 16, 2024
1 parent 782553e commit b17a880
Show file tree
Hide file tree
Showing 19 changed files with 546 additions and 74 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- `privacy-nonce-always-increments` option enables private transactions to always increment the nonce, even if the transaction is invalid [#6593](https://github.com/hyperledger/besu/pull/6593)
- Add `block-test` subcommand to the evmtool which runs blockchain reference tests [#7310](https://github.com/hyperledger/besu/pull/7310)
- Implement gnark-crypto for eip-2537 [#7316](https://github.com/hyperledger/besu/pull/7316)
- Improve blob size transaction selector [#7312](https://github.com/hyperledger/besu/pull/7312)

### Bug fixes
- Fix `eth_call` deserialization to correctly ignore unknown fields in the transaction object. [#7323](https://github.com/hyperledger/besu/pull/7323)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.hyperledger.besu.ethereum.GasLimitCalculator;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.AbstractTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.BlobPriceTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.BlobSizeTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.BlockSizeTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.MinPriorityFeePerGasTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.PriceTransactionSelector;
Expand Down Expand Up @@ -146,6 +147,7 @@ private List<AbstractTransactionSelector> createTransactionSelectors(
final BlockSelectionContext context) {
return List.of(
new BlockSizeTransactionSelector(context),
new BlobSizeTransactionSelector(context),
new PriceTransactionSelector(context),
new BlobPriceTransactionSelector(context),
new MinPriorityFeePerGasTransactionSelector(context),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Copyright contributors to Hyperledger Besu.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.ethereum.blockcreation.txselection.selectors;

import org.hyperledger.besu.ethereum.blockcreation.txselection.BlockSelectionContext;
import org.hyperledger.besu.ethereum.blockcreation.txselection.TransactionEvaluationContext;
import org.hyperledger.besu.ethereum.blockcreation.txselection.TransactionSelectionResults;
import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
import org.hyperledger.besu.plugin.data.TransactionSelectionResult;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This class extends AbstractTransactionSelector and provides a specific implementation for
* evaluating transactions based on blobs size. It checks if a transaction supports blobs, and if
* so, checks that there is enough remaining data gas in the block to fit the blobs of the tx.
*/
public class BlobSizeTransactionSelector extends AbstractTransactionSelector {
private static final Logger LOG = LoggerFactory.getLogger(BlobSizeTransactionSelector.class);

public BlobSizeTransactionSelector(final BlockSelectionContext context) {
super(context);
}

/**
* Evaluates a transaction considering other transactions in the same block. If the tx does not
* support blobs, no check is performed, and SELECTED is returned, otherwise SELECTED is returned
* only if there is enough remaining blob gas to fit the blobs of the tx, otherwise a specific not
* selected result is returned, depending on the fact that the block already contains the max
* number of blobs or not.
*
* @param evaluationContext The current selection session data.
* @param transactionSelectionResults The results of other transaction evaluations in the same
* block.
* @return The result of the transaction selection.
*/
@Override
public TransactionSelectionResult evaluateTransactionPreProcessing(
final TransactionEvaluationContext evaluationContext,
final TransactionSelectionResults transactionSelectionResults) {

final var tx = evaluationContext.getTransaction();
if (tx.getType().supportsBlob()) {

final var remainingBlobGas =
context.gasLimitCalculator().currentBlobGasLimit()
- transactionSelectionResults.getCumulativeBlobGasUsed();

if (remainingBlobGas == 0) {
LOG.atTrace()
.setMessage("The block already contains the max number of allowed blobs")
.addArgument(evaluationContext.getPendingTransaction()::toTraceLog)
.log();
return TransactionSelectionResult.BLOBS_FULL;
}

final long requestedBlobGas = context.gasCalculator().blobGasCost(tx.getBlobCount());

if (requestedBlobGas > remainingBlobGas) {
LOG.atTrace()
.setMessage(
"There is not enough blob gas available to fit the blobs of the transaction {} in the block."
+ " Available {} / Requested {}")
.addArgument(evaluationContext.getPendingTransaction()::toTraceLog)
.addArgument(remainingBlobGas)
.addArgument(requestedBlobGas)
.log();
return TransactionSelectionResult.TX_TOO_LARGE_FOR_REMAINING_BLOB_GAS;
}
}
return TransactionSelectionResult.SELECTED;
}

@Override
public TransactionSelectionResult evaluateTransactionPostProcessing(
final TransactionEvaluationContext evaluationContext,
final TransactionSelectionResults blockTransactionResults,
final TransactionProcessingResult processingResult) {
// All necessary checks were done in the pre-processing method, so nothing to do here.
return TransactionSelectionResult.SELECTED;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,8 @@ public TransactionSelectionResult evaluateTransactionPostProcessing(
private boolean transactionTooLargeForBlock(
final Transaction transaction,
final TransactionSelectionResults transactionSelectionResults) {
final long blobGasUsed = context.gasCalculator().blobGasCost(transaction.getBlobCount());

if (blobGasUsed
> context.gasLimitCalculator().currentBlobGasLimit()
- transactionSelectionResults.getCumulativeBlobGasUsed()) {
return true;
}

return transaction.getGasLimit() + blobGasUsed
return transaction.getGasLimit()
> context.processableBlockHeader().getGasLimit()
- transactionSelectionResults.getCumulativeGasUsed();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@
import static org.mockito.Mockito.when;

import org.hyperledger.besu.config.GenesisConfigFile;
import org.hyperledger.besu.config.GenesisConfigOptions;
import org.hyperledger.besu.crypto.KeyPair;
import org.hyperledger.besu.crypto.SECPPrivateKey;
import org.hyperledger.besu.crypto.SignatureAlgorithm;
import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.BLSPublicKey;
Expand Down Expand Up @@ -73,13 +74,18 @@
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.TransactionValidationParams;
import org.hyperledger.besu.ethereum.mainnet.TransactionValidator;
import org.hyperledger.besu.ethereum.mainnet.TransactionValidatorFactory;
import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
import org.hyperledger.besu.ethereum.mainnet.WithdrawalsProcessor;
import org.hyperledger.besu.ethereum.mainnet.feemarket.CancunFeeMarket;
import org.hyperledger.besu.ethereum.mainnet.requests.DepositRequestProcessor;
import org.hyperledger.besu.ethereum.mainnet.requests.DepositRequestValidator;
import org.hyperledger.besu.ethereum.mainnet.requests.ProcessRequestContext;
import org.hyperledger.besu.ethereum.mainnet.requests.RequestProcessorCoordinator;
import org.hyperledger.besu.ethereum.mainnet.requests.RequestsValidatorCoordinator;
import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
import org.hyperledger.besu.evm.account.Account;
import org.hyperledger.besu.evm.internal.EvmConfiguration;
import org.hyperledger.besu.evm.log.Log;
import org.hyperledger.besu.evm.log.LogTopic;
Expand All @@ -89,20 +95,31 @@
import java.math.BigInteger;
import java.time.Clock;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt64;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
abstract class AbstractBlockCreatorTest {
private static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
Suppliers.memoize(SignatureAlgorithmFactory::getInstance);
private static final SECPPrivateKey PRIVATE_KEY1 =
SIGNATURE_ALGORITHM
.get()
.createPrivateKey(
Bytes32.fromHexString(
"8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63"));
private static final KeyPair KEYS1 =
new KeyPair(PRIVATE_KEY1, SIGNATURE_ALGORITHM.get().createPublicKey(PRIVATE_KEY1));

@Mock private WithdrawalsProcessor withdrawalsProcessor;
protected EthScheduler ethScheduler = new DeterministicEthScheduler();

Expand Down Expand Up @@ -301,10 +318,8 @@ void withNoProcessorAndWithdrawals_WithdrawalsAreNotProcessed() {
assertThat(blockCreationResult.getBlock().getBody().getWithdrawals()).isEmpty();
}

@Disabled
@Test
public void computesGasUsageFromIncludedTransactions() {
final KeyPair senderKeys = SignatureAlgorithmFactory.getInstance().generateKeyPair();
final AbstractBlockCreator blockCreator = blockCreatorWithBlobGasSupport();
BlobTestFixture blobTestFixture = new BlobTestFixture();
BlobsWithCommitments bwc = blobTestFixture.createBlobsWithCommitments(6);
Expand All @@ -313,13 +328,14 @@ public void computesGasUsageFromIncludedTransactions() {
ttf.to(Optional.of(Address.ZERO))
.type(TransactionType.BLOB)
.chainId(Optional.of(BigInteger.valueOf(42)))
.gasLimit(21000)
.maxFeePerGas(Optional.of(Wei.of(15)))
.maxFeePerBlobGas(Optional.of(Wei.of(128)))
.maxPriorityFeePerGas(Optional.of(Wei.of(1)))
.versionedHashes(Optional.of(bwc.getVersionedHashes()))
.createTransaction(senderKeys);
.blobsWithCommitments(Optional.of(bwc))
.createTransaction(KEYS1);

ttf.blobsWithCommitments(Optional.of(bwc));
final BlockCreationResult blockCreationResult =
blockCreator.createBlock(
Optional.of(List.of(fullOfBlobs)),
Expand All @@ -336,13 +352,17 @@ public void computesGasUsageFromIncludedTransactions() {
}

private AbstractBlockCreator blockCreatorWithBlobGasSupport() {
final var alwaysValidTransactionValidatorFactory = mock(TransactionValidatorFactory.class);
when(alwaysValidTransactionValidatorFactory.get())
.thenReturn(new AlwaysValidTransactionValidator());
final ProtocolSpecAdapters protocolSpecAdapters =
ProtocolSpecAdapters.create(
0,
specBuilder -> {
specBuilder.feeMarket(new CancunFeeMarket(0, Optional.empty()));
specBuilder.isReplayProtectionSupported(true);
specBuilder.withdrawalsProcessor(withdrawalsProcessor);
specBuilder.transactionValidatorFactoryBuilder(
(evm, gasLimitCalculator, feeMarket) -> alwaysValidTransactionValidatorFactory);
return specBuilder;
});
return createBlockCreator(protocolSpecAdapters);
Expand All @@ -356,16 +376,19 @@ private AbstractBlockCreator blockCreatorWithWithdrawalsProcessor() {
}

private AbstractBlockCreator blockCreatorWithoutWithdrawalsProcessor() {
return createBlockCreator(new ProtocolSpecAdapters(Map.of()));
final ProtocolSpecAdapters protocolSpecAdapters =
ProtocolSpecAdapters.create(0, specBuilder -> specBuilder.withdrawalsProcessor(null));
return createBlockCreator(protocolSpecAdapters);
}

private AbstractBlockCreator createBlockCreator(final ProtocolSpecAdapters protocolSpecAdapters) {
final GenesisConfigOptions genesisConfigOptions = GenesisConfigFile.DEFAULT.getConfigOptions();

final var genesisConfigFile = GenesisConfigFile.fromResource("/block-creation-genesis.json");
final ExecutionContextTestFixture executionContextTestFixture =
ExecutionContextTestFixture.builder()
ExecutionContextTestFixture.builder(genesisConfigFile)
.protocolSchedule(
new ProtocolScheduleBuilder(
genesisConfigOptions,
genesisConfigFile.getConfigOptions(),
BigInteger.valueOf(42),
protocolSpecAdapters,
PrivacyParameters.DEFAULT,
Expand Down Expand Up @@ -452,4 +475,24 @@ protected BlockHeader createFinalBlockHeader(final SealableBlockHeader sealableB
.buildBlockHeader();
}
}

static class AlwaysValidTransactionValidator implements TransactionValidator {

@Override
public ValidationResult<TransactionInvalidReason> validate(
final Transaction transaction,
final Optional<Wei> baseFee,
final Optional<Wei> blobBaseFee,
final TransactionValidationParams transactionValidationParams) {
return ValidationResult.valid();
}

@Override
public ValidationResult<TransactionInvalidReason> validateForSender(
final Transaction transaction,
final Account sender,
final TransactionValidationParams validationParams) {
return ValidationResult.valid();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@
import java.time.ZoneId;
import java.util.function.Function;

import org.junit.jupiter.api.Disabled;

@Disabled(
"disabled since it's flaky with a timeout see https://github.com/hyperledger/besu/issues/6850")
public class LegacyFeeMarketBlockTransactionSelectorTest
extends AbstractBlockTransactionSelectorTest {

Expand Down
Loading

0 comments on commit b17a880

Please sign in to comment.