From 99260fae96cf9d6248a6a112298e943ac323e8cb Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 16 Jul 2024 23:50:09 +0200 Subject: [PATCH] Improve blob size transaction selector (#7312) Signed-off-by: Fabio Di Fabio --- CHANGELOG.md | 1 + .../txselection/BlockTransactionSelector.java | 2 + .../BlobSizeTransactionSelector.java | 95 +++++++++ .../BlockSizeTransactionSelector.java | 9 +- .../BlobSizeTransactionSelectorTest.java | 188 ++++++++++++++++++ plugin-api/build.gradle | 2 +- .../data/TransactionSelectionResult.java | 12 ++ 7 files changed, 300 insertions(+), 9 deletions(-) create mode 100644 ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelector.java create mode 100644 ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelectorTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index bee4620060c..e0269472a5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java index ab311c3fb38..c106b7aff16 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java @@ -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; @@ -146,6 +147,7 @@ private List createTransactionSelectors( final BlockSelectionContext context) { return List.of( new BlockSizeTransactionSelector(context), + new BlobSizeTransactionSelector(context), new PriceTransactionSelector(context), new BlobPriceTransactionSelector(context), new MinPriorityFeePerGasTransactionSelector(context), diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelector.java new file mode 100644 index 00000000000..9df006722e9 --- /dev/null +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelector.java @@ -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; + } +} diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlockSizeTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlockSizeTransactionSelector.java index 7f0b13e8b99..793f3e93842 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlockSizeTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlockSizeTransactionSelector.java @@ -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(); } diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelectorTest.java new file mode 100644 index 00000000000..f52a5dfb462 --- /dev/null +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/BlobSizeTransactionSelectorTest.java @@ -0,0 +1,188 @@ +/* + * 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 static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; +import static org.mockito.Answers.RETURNS_DEEP_STUBS; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import org.hyperledger.besu.crypto.KeyPair; +import org.hyperledger.besu.crypto.SignatureAlgorithm; +import org.hyperledger.besu.crypto.SignatureAlgorithmFactory; +import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.datatypes.Blob; +import org.hyperledger.besu.datatypes.BlobsWithCommitments; +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.datatypes.KZGCommitment; +import org.hyperledger.besu.datatypes.KZGProof; +import org.hyperledger.besu.datatypes.TransactionType; +import org.hyperledger.besu.datatypes.VersionedHash; +import org.hyperledger.besu.datatypes.Wei; +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.core.Transaction; +import org.hyperledger.besu.ethereum.core.TransactionTestFixture; +import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction; +import org.hyperledger.besu.evm.gascalculator.CancunGasCalculator; +import org.hyperledger.besu.plugin.data.TransactionSelectionResult; + +import java.util.Optional; +import java.util.stream.IntStream; + +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; +import org.apache.tuweni.bytes.Bytes; +import org.apache.tuweni.bytes.Bytes48; +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) +class BlobSizeTransactionSelectorTest { + private static final Supplier SIGNATURE_ALGORITHM = + Suppliers.memoize(SignatureAlgorithmFactory::getInstance); + private static final KeyPair KEYS = SIGNATURE_ALGORITHM.get().generateKeyPair(); + + private static final long BLOB_GAS_PER_BLOB = CancunGasCalculator.BLOB_GAS_PER_BLOB; + private static final int MAX_BLOBS = 6; + private static final long MAX_BLOB_GAS = BLOB_GAS_PER_BLOB * MAX_BLOBS; + + @Mock(answer = RETURNS_DEEP_STUBS) + BlockSelectionContext blockSelectionContext; + + @Mock TransactionSelectionResults selectionResults; + + @Test + void notBlobTransactionsAreSelectedWithoutAnyCheck() { + final var selector = new BlobSizeTransactionSelector(blockSelectionContext); + + final var nonBlobTx = createEIP1559PendingTransaction(); + + final var txEvaluationContext = new TransactionEvaluationContext(nonBlobTx, null, null, null); + + final var result = + selector.evaluateTransactionPreProcessing(txEvaluationContext, selectionResults); + assertThat(result).isEqualTo(TransactionSelectionResult.SELECTED); + verifyNoInteractions(selectionResults); + } + + @Test + void firstBlobTransactionIsSelected() { + when(blockSelectionContext.gasLimitCalculator().currentBlobGasLimit()).thenReturn(MAX_BLOB_GAS); + when(blockSelectionContext.gasCalculator().blobGasCost(anyInt())) + .thenAnswer(iom -> BLOB_GAS_PER_BLOB * iom.getArgument(0, Integer.class)); + + final var selector = new BlobSizeTransactionSelector(blockSelectionContext); + + final var firstBlobTx = createBlobPendingTransaction(MAX_BLOBS); + + final var txEvaluationContext = new TransactionEvaluationContext(firstBlobTx, null, null, null); + + when(selectionResults.getCumulativeBlobGasUsed()).thenReturn(0L); + + final var result = + selector.evaluateTransactionPreProcessing(txEvaluationContext, selectionResults); + assertThat(result).isEqualTo(TransactionSelectionResult.SELECTED); + verify(selectionResults).getCumulativeBlobGasUsed(); + } + + @Test + void returnsBlobsFullWhenMaxNumberOfBlobsAlreadyPresent() { + when(blockSelectionContext.gasLimitCalculator().currentBlobGasLimit()).thenReturn(MAX_BLOB_GAS); + + final var selector = new BlobSizeTransactionSelector(blockSelectionContext); + + final var firstBlobTx = createBlobPendingTransaction(1); + + final var txEvaluationContext = new TransactionEvaluationContext(firstBlobTx, null, null, null); + + when(selectionResults.getCumulativeBlobGasUsed()).thenReturn(MAX_BLOB_GAS); + + final var result = + selector.evaluateTransactionPreProcessing(txEvaluationContext, selectionResults); + assertThat(result).isEqualTo(TransactionSelectionResult.BLOBS_FULL); + verify(selectionResults).getCumulativeBlobGasUsed(); + } + + @Test + void returnsTooLargeForRemainingBlobGas() { + when(blockSelectionContext.gasLimitCalculator().currentBlobGasLimit()).thenReturn(MAX_BLOB_GAS); + when(blockSelectionContext.gasCalculator().blobGasCost(anyInt())) + .thenAnswer(iom -> BLOB_GAS_PER_BLOB * iom.getArgument(0, Integer.class)); + + final var selector = new BlobSizeTransactionSelector(blockSelectionContext); + + final var firstBlobTx = createBlobPendingTransaction(MAX_BLOBS); + + final var txEvaluationContext = new TransactionEvaluationContext(firstBlobTx, null, null, null); + + when(selectionResults.getCumulativeBlobGasUsed()).thenReturn(MAX_BLOB_GAS - 1); + + final var result = + selector.evaluateTransactionPreProcessing(txEvaluationContext, selectionResults); + assertThat(result).isEqualTo(TransactionSelectionResult.TX_TOO_LARGE_FOR_REMAINING_BLOB_GAS); + verify(selectionResults).getCumulativeBlobGasUsed(); + } + + private PendingTransaction createEIP1559PendingTransaction() { + return PendingTransaction.newPendingTransaction( + createTransaction(TransactionType.EIP1559, 0), false, false); + } + + private PendingTransaction createBlobPendingTransaction(final int blobCount) { + return PendingTransaction.newPendingTransaction( + createTransaction(TransactionType.BLOB, blobCount), false, false); + } + + private Transaction createTransaction(final TransactionType type, final int blobCount) { + + var tx = + new TransactionTestFixture() + .to(Optional.of(Address.fromHexString("0x634316eA0EE79c701c6F67C53A4C54cBAfd2316d"))) + .nonce(0) + .type(type); + tx.maxFeePerGas(Optional.of(Wei.of(1000))).maxPriorityFeePerGas(Optional.of(Wei.of(100))); + if (type.supportsBlob()) { + if (blobCount > 0) { + tx.maxFeePerBlobGas(Optional.of(Wei.of(10))); + final var versionHashes = + IntStream.range(0, blobCount) + .mapToObj(i -> new VersionedHash((byte) 1, Hash.ZERO)) + .toList(); + final var kgzCommitments = + IntStream.range(0, blobCount) + .mapToObj(i -> new KZGCommitment(Bytes48.random())) + .toList(); + final var kzgProofs = + IntStream.range(0, blobCount).mapToObj(i -> new KZGProof(Bytes48.random())).toList(); + final var blobs = + IntStream.range(0, blobCount).mapToObj(i -> new Blob(Bytes.random(32 * 4096))).toList(); + tx.versionedHashes(Optional.of(versionHashes)); + final var blobsWithCommitments = + new BlobsWithCommitments(kgzCommitments, blobs, kzgProofs, versionHashes); + tx.blobsWithCommitments(Optional.of(blobsWithCommitments)); + } else { + fail("At least 1 blob is required for blob tx type"); + } + } + return tx.createTransaction(KEYS); + } +} diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index dff6cc90986..1ea78e7f3a5 100644 --- a/plugin-api/build.gradle +++ b/plugin-api/build.gradle @@ -70,7 +70,7 @@ Calculated : ${currentHash} tasks.register('checkAPIChanges', FileStateChecker) { description = "Checks that the API for the Plugin-API project does not change without deliberate thought" files = sourceSets.main.allJava.files - knownHash = 'MOOKIka8Q1D64m0lU29Leq+zf1vcgeLoDVukBVQXhnY=' + knownHash = 'vWxI4pduLCvzqU73k5kJsoeK9t7QTqOBC9IxksFuzBs=' } check.dependsOn('checkAPIChanges') diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java index 839c4120d08..b59fb269670 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java @@ -53,6 +53,7 @@ protected interface Status { private enum BaseStatus implements Status { SELECTED, BLOCK_FULL(true, false), + BLOBS_FULL(false, false), BLOCK_OCCUPANCY_ABOVE_THRESHOLD(true, false), BLOCK_SELECTION_TIMEOUT(true, false), TX_EVALUATION_TOO_LONG(true, true), @@ -96,6 +97,10 @@ public boolean discard() { public static final TransactionSelectionResult BLOCK_FULL = new TransactionSelectionResult(BaseStatus.BLOCK_FULL); + /** The block already contains the max number of allowed blobs. */ + public static final TransactionSelectionResult BLOBS_FULL = + new TransactionSelectionResult(BaseStatus.BLOBS_FULL); + /** There was no more time to add transaction to the block */ public static final TransactionSelectionResult BLOCK_SELECTION_TIMEOUT = new TransactionSelectionResult(BaseStatus.BLOCK_SELECTION_TIMEOUT); @@ -118,6 +123,13 @@ public boolean discard() { public static final TransactionSelectionResult TX_TOO_LARGE_FOR_REMAINING_GAS = TransactionSelectionResult.invalidTransient("TX_TOO_LARGE_FOR_REMAINING_GAS"); + /** + * The transaction has not been selected since there is not enough remaining blob gas in the block + * to fit the blobs of the tx, but selection should continue. + */ + public static final TransactionSelectionResult TX_TOO_LARGE_FOR_REMAINING_BLOB_GAS = + TransactionSelectionResult.invalidTransient("TX_TOO_LARGE_FOR_REMAINING_BLOB_GAS"); + /** * The transaction has not been selected since its current price is below the configured min * price, but the selection should continue.