Skip to content

Commit f20d064

Browse files
fab-10fab-10
authored
Fix data gas calculation during block import (#5023)
 Signed-off-by: Fabio Di Fabio <[email protected]>
1 parent 242768f commit f20d064

File tree

7 files changed

+63
-50
lines changed

7 files changed

+63
-50
lines changed

‎consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import static org.mockito.Mockito.doAnswer;
2727
import static org.mockito.Mockito.doCallRealMethod;
2828
import static org.mockito.Mockito.doReturn;
29-
import static org.mockito.Mockito.doThrow;
3029
import static org.mockito.Mockito.mock;
3130
import static org.mockito.Mockito.never;
3231
import static org.mockito.Mockito.spy;
@@ -365,7 +364,8 @@ public void shouldRetryBlockCreationOnRecoverableError()
365364
.getTransactions()
366365
.isEmpty()) {
367366
// this is called by the first empty block
368-
doThrow(new MerkleTrieException("lock")) // first fail
367+
doCallRealMethod() // first work
368+
.doThrow(new MerkleTrieException("lock")) // second fail
369369
.doCallRealMethod() // then work
370370
.when(blockchain)
371371
.getBlockHeader(any());

‎ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static org.hyperledger.besu.util.Slf4jLambdaHelper.traceLambda;
1818

1919
import org.hyperledger.besu.datatypes.Address;
20+
import org.hyperledger.besu.datatypes.DataGas;
2021
import org.hyperledger.besu.datatypes.Wei;
2122
import org.hyperledger.besu.ethereum.GasLimitCalculator;
2223
import org.hyperledger.besu.ethereum.chain.Blockchain;
@@ -246,7 +247,7 @@ private boolean transactionDataPriceBelowMin(final Transaction transaction) {
246247
.lessThan(
247248
feeMarket
248249
.getTransactionPriceCalculator()
249-
.dataPrice(transaction, processableBlockHeader))) {
250+
.dataPrice(processableBlockHeader.getExcessDataGas().orElse(DataGas.ZERO)))) {
250251
return true;
251252
}
252253
}

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/feemarket/TransactionPriceCalculator.java‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616

1717
import static org.hyperledger.besu.util.Slf4jLambdaHelper.traceLambda;
1818

19+
import org.hyperledger.besu.datatypes.DataGas;
1920
import org.hyperledger.besu.datatypes.Wei;
20-
import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
2121
import org.hyperledger.besu.ethereum.core.Transaction;
2222

2323
import java.math.BigInteger;
@@ -29,7 +29,7 @@
2929
public interface TransactionPriceCalculator {
3030
Wei price(Transaction transaction, Optional<Wei> maybeFee);
3131

32-
default Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader blockHeader) {
32+
default Wei dataPrice(final DataGas excessDataGas) {
3333
return Wei.ZERO;
3434
}
3535

@@ -38,11 +38,6 @@ class Frontier implements TransactionPriceCalculator {
3838
public Wei price(final Transaction transaction, final Optional<Wei> maybeFee) {
3939
return transaction.getGasPrice().orElse(Wei.ZERO);
4040
}
41-
42-
@Override
43-
public Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader blockHeader) {
44-
return Wei.ZERO;
45-
}
4641
}
4742

4843
class EIP1559 implements TransactionPriceCalculator {
@@ -73,17 +68,14 @@ public DataBlob(final int minDataGasPrice, final int dataGasPriceUpdateFraction)
7368
}
7469

7570
@Override
76-
public Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader blockHeader) {
77-
final var excessDataGas = blockHeader.getExcessDataGas().orElseThrow();
78-
71+
public Wei dataPrice(final DataGas excessDataGas) {
7972
final var dataGasPrice =
8073
Wei.of(
8174
fakeExponential(
8275
minDataGasPrice, excessDataGas.toBigInteger(), dataGasPriceUpdateFraction));
8376
traceLambda(
8477
LOG,
85-
"block #{} parentExcessDataGas: {} dataGasPrice: {}",
86-
blockHeader::getNumber,
78+
"parentExcessDataGas: {} dataGasPrice: {}",
8779
excessDataGas::toShortHexString,
8880
dataGasPrice::toHexString);
8981

@@ -92,14 +84,15 @@ public Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader
9284

9385
private BigInteger fakeExponential(
9486
final BigInteger factor, final BigInteger numerator, final BigInteger denominator) {
95-
BigInteger i = BigInteger.ONE;
87+
int i = 1;
9688
BigInteger output = BigInteger.ZERO;
9789
BigInteger numeratorAccumulator = factor.multiply(denominator);
98-
while (numeratorAccumulator.compareTo(BigInteger.ZERO) > 0) {
90+
while (numeratorAccumulator.signum() > 0) {
9991
output = output.add(numeratorAccumulator);
10092
numeratorAccumulator =
101-
(numeratorAccumulator.multiply(numerator)).divide(denominator.multiply(i));
102-
i = i.add(BigInteger.ONE);
93+
(numeratorAccumulator.multiply(numerator))
94+
.divide(denominator.multiply(BigInteger.valueOf(i)));
95+
++i;
10396
}
10497
return output.divide(denominator);
10598
}

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,7 @@ public BlockProcessingResult processBlock(
135135
}
136136
worldStateUpdater.commit();
137137

138-
final long dataGasUsed =
139-
protocolSpec.getGasCalculator().dataGasCost(transaction.getBlobCount());
140-
141-
currentGasUsed += transaction.getGasLimit() - result.getGasRemaining() - dataGasUsed;
138+
currentGasUsed += transaction.getGasLimit() - result.getGasRemaining();
142139
final TransactionReceipt transactionReceipt =
143140
transactionReceiptFactory.create(
144141
transaction.getType(), result, worldState, currentGasUsed);

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,7 @@ static ProtocolSpecBuilder cancunDefinition(
726726
final boolean quorumCompatibilityMode,
727727
final EvmConfiguration evmConfiguration) {
728728

729+
final int stackSizeLimit = configStackSizeLimit.orElse(MessageFrame.DEFAULT_MAX_STACK_SIZE);
729730
final int contractSizeLimit =
730731
configContractSizeLimit.orElse(SPURIOUS_DRAGON_CONTRACT_SIZE_LIMIT);
731732
final long londonForkBlockNumber = genesisConfigOptions.getLondonBlockNumber().orElse(0L);
@@ -765,6 +766,22 @@ static ProtocolSpecBuilder cancunDefinition(
765766
MaxCodeSizeRule.of(contractSizeLimit), EOFValidationCodeRule.of(1, false)),
766767
1,
767768
SPURIOUS_DRAGON_FORCE_DELETE_WHEN_EMPTY_ADDRESSES))
769+
// use Cancun fee market
770+
.transactionProcessorBuilder(
771+
(gasCalculator,
772+
transactionValidator,
773+
contractCreationProcessor,
774+
messageCallProcessor) ->
775+
new MainnetTransactionProcessor(
776+
gasCalculator,
777+
transactionValidator,
778+
contractCreationProcessor,
779+
messageCallProcessor,
780+
true,
781+
true,
782+
stackSizeLimit,
783+
cancunFeeMarket,
784+
CoinbaseFeePriceCalculator.eip1559()))
768785
// change to check for max data gas per block for EIP-4844
769786
.transactionValidatorBuilder(
770787
(gasCalculator, gasLimitCalculator) ->

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
import static org.hyperledger.besu.ethereum.mainnet.PrivateStateUtils.KEY_TRANSACTION_HASH;
2121

2222
import org.hyperledger.besu.datatypes.Address;
23+
import org.hyperledger.besu.datatypes.DataGas;
2324
import org.hyperledger.besu.datatypes.Hash;
2425
import org.hyperledger.besu.datatypes.Wei;
2526
import org.hyperledger.besu.ethereum.chain.Blockchain;
27+
import org.hyperledger.besu.ethereum.core.BlockHeader;
2628
import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
2729
import org.hyperledger.besu.ethereum.core.Transaction;
2830
import org.hyperledger.besu.ethereum.core.feemarket.CoinbaseFeePriceCalculator;
@@ -252,7 +254,7 @@ public TransactionProcessingResult processTransaction(
252254
}
253255

254256
public TransactionProcessingResult processTransaction(
255-
final Blockchain ignoredBlockchain,
257+
final Blockchain blockchain,
256258
final WorldUpdater worldState,
257259
final ProcessableBlockHeader blockHeader,
258260
final Transaction transaction,
@@ -288,16 +290,23 @@ public TransactionProcessingResult processTransaction(
288290

289291
final MutableAccount senderMutableAccount = sender.getMutable();
290292
final long previousNonce = senderMutableAccount.incrementNonce();
291-
final Wei transactionGasPrice =
292-
feeMarket.getTransactionPriceCalculator().price(transaction, blockHeader.getBaseFee());
293-
final Wei dataGasPrice =
294-
feeMarket.getTransactionPriceCalculator().dataPrice(transaction, blockHeader);
295293
LOG.trace(
296294
"Incremented sender {} nonce ({} -> {})",
297295
senderAddress,
298296
previousNonce,
299297
sender.getNonce());
300298

299+
final Wei transactionGasPrice =
300+
feeMarket.getTransactionPriceCalculator().price(transaction, blockHeader.getBaseFee());
301+
final Wei dataGasPrice =
302+
feeMarket
303+
.getTransactionPriceCalculator()
304+
.dataPrice(
305+
blockchain
306+
.getBlockHeader(blockHeader.getParentHash())
307+
.flatMap(BlockHeader::getExcessDataGas)
308+
.orElse(DataGas.ZERO));
309+
301310
final long dataGas = gasCalculator.dataGasCost(transaction.getBlobCount());
302311

303312
final Wei upfrontGasCost =
@@ -333,14 +342,13 @@ public TransactionProcessingResult processTransaction(
333342
final long accessListGas =
334343
gasCalculator.accessListGasCost(accessListEntries.size(), accessListStorageCount);
335344

336-
final long gasAvailable = transaction.getGasLimit() - intrinsicGas - accessListGas - dataGas;
345+
final long gasAvailable = transaction.getGasLimit() - intrinsicGas - accessListGas;
337346
LOG.trace(
338-
"Gas available for execution {} = {} - {} - {} - {} (limit - intrinsic - accessList - data)",
347+
"Gas available for execution {} = {} - {} - {} - {} (limit - intrinsic - accessList)",
339348
gasAvailable,
340349
transaction.getGasLimit(),
341350
intrinsicGas,
342-
accessListGas,
343-
dataGas);
351+
accessListGas);
344352

345353
final WorldUpdater worldUpdater = worldState.updater();
346354
final Deque<MessageFrame> messageFrameStack = new ArrayDeque<>();
@@ -434,35 +442,36 @@ public TransactionProcessingResult processTransaction(
434442
// after the other so that if it is the same account somehow, we end up with the right result)
435443
final long selfDestructRefund =
436444
gasCalculator.getSelfDestructRefundAmount() * initialFrame.getSelfDestructs().size();
437-
final long refundGas = initialFrame.getGasRefund() + selfDestructRefund;
438-
final long refunded = refunded(transaction, initialFrame.getRemainingGas(), refundGas);
439-
final Wei refundedWei = transactionGasPrice.multiply(refunded);
445+
final long baseRefundGas = initialFrame.getGasRefund() + selfDestructRefund;
446+
final long refundedGas = refunded(transaction, initialFrame.getRemainingGas(), baseRefundGas);
447+
final Wei refundedWei = transactionGasPrice.multiply(refundedGas);
440448
senderMutableAccount.incrementBalance(refundedWei);
441449

442450
final long gasUsedByTransaction = transaction.getGasLimit() - initialFrame.getRemainingGas();
443451

444452
if (!worldState.getClass().equals(GoQuorumMutablePrivateWorldStateUpdater.class)) {
445453
// if this is not a private GoQuorum transaction we have to update the coinbase
446454
final var coinbase = worldState.getOrCreate(miningBeneficiary).getMutable();
447-
final long coinbaseFee = transaction.getGasLimit() - refunded;
455+
final long usedGas = transaction.getGasLimit() - refundedGas;
456+
final CoinbaseFeePriceCalculator coinbaseCalculator;
448457
if (blockHeader.getBaseFee().isPresent()) {
449458
final Wei baseFee = blockHeader.getBaseFee().get();
450459
if (transactionGasPrice.compareTo(baseFee) < 0) {
451460
return TransactionProcessingResult.failed(
452461
gasUsedByTransaction,
453-
refunded,
462+
refundedGas,
454463
ValidationResult.invalid(
455464
TransactionInvalidReason.TRANSACTION_PRICE_TOO_LOW,
456465
"transaction price must be greater than base fee"),
457466
Optional.empty());
458467
}
468+
coinbaseCalculator = coinbaseFeePriceCalculator;
469+
} else {
470+
coinbaseCalculator = CoinbaseFeePriceCalculator.frontier();
459471
}
460-
final CoinbaseFeePriceCalculator coinbaseCalculator =
461-
blockHeader.getBaseFee().isPresent()
462-
? coinbaseFeePriceCalculator
463-
: CoinbaseFeePriceCalculator.frontier();
472+
464473
final Wei coinbaseWeiDelta =
465-
coinbaseCalculator.price(coinbaseFee, transactionGasPrice, blockHeader.getBaseFee());
474+
coinbaseCalculator.price(usedGas, transactionGasPrice, blockHeader.getBaseFee());
466475

467476
coinbase.incrementBalance(coinbaseWeiDelta);
468477
}
@@ -477,12 +486,12 @@ public TransactionProcessingResult processTransaction(
477486
return TransactionProcessingResult.successful(
478487
initialFrame.getLogs(),
479488
gasUsedByTransaction,
480-
refunded,
489+
refundedGas,
481490
initialFrame.getOutputData(),
482491
validationResult);
483492
} else {
484493
return TransactionProcessingResult.failed(
485-
gasUsedByTransaction, refunded, validationResult, initialFrame.getRevertReason());
494+
gasUsedByTransaction, refundedGas, validationResult, initialFrame.getRevertReason());
486495
}
487496
} catch (final RuntimeException re) {
488497
LOG.error("Critical Exception Processing Transaction", re);
@@ -515,7 +524,7 @@ private AbstractMessageProcessor getMessageProcessor(final MessageFrame.Type typ
515524

516525
protected long refunded(
517526
final Transaction transaction, final long gasRemaining, final long gasRefund) {
518-
// Integer truncation takes care of the the floor calculation needed after the divide.
527+
// Integer truncation takes care of the floor calculation needed after the divide.
519528
final long maxRefundAllowance =
520529
(transaction.getGasLimit() - gasRemaining) / gasCalculator.getMaxRefundQuotient();
521530
final long refundAllowance = Math.min(maxRefundAllowance, gasRefund);

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ public ValidationResult<TransactionInvalidReason> validate(
134134
if (!signatureResult.isValid()) {
135135
return signatureResult;
136136
}
137+
137138
if (transaction.getType().equals(TransactionType.BLOB)
138139
&& transaction.getBlobsWithCommitments().isPresent()) {
139140
final ValidationResult<TransactionInvalidReason> blobsResult =
@@ -202,11 +203,6 @@ private ValidationResult<TransactionInvalidReason> validateCostAndFee(
202203
}
203204
}
204205

205-
if (transaction.getNonce() == MAX_NONCE) {
206-
return ValidationResult.invalid(
207-
TransactionInvalidReason.NONCE_OVERFLOW, "Nonce must be less than 2^64-1");
208-
}
209-
210206
if (transaction.getType().supportsBlob()) {
211207
final long txTotalDataGas = gasCalculator.dataGasCost(transaction.getBlobCount());
212208
if (txTotalDataGas > gasLimitCalculator.currentDataGasLimit()) {

0 commit comments

Comments
 (0)