Fix data gas calculation during block import (hyperledger#5023)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

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

@@ -26,7 +26,6 @@
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
@@ -365,7 +364,8 @@ public void shouldRetryBlockCreationOnRecoverableError()
                   .getTransactions()
                   .isEmpty()) {
                 // this is called by the first empty block
-                doThrow(new MerkleTrieException("lock")) // first fail
+                doCallRealMethod() // first work
+                    .doThrow(new MerkleTrieException("lock")) // second fail
                     .doCallRealMethod() // then work
                     .when(blockchain)
                     .getBlockHeader(any());

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

@@ -17,6 +17,7 @@
 import static org.hyperledger.besu.util.Slf4jLambdaHelper.traceLambda;
 
 import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.DataGas;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.GasLimitCalculator;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
@@ -246,7 +247,7 @@ private boolean transactionDataPriceBelowMin(final Transaction transaction) {
           .lessThan(
               feeMarket
                   .getTransactionPriceCalculator()
-                  .dataPrice(transaction, processableBlockHeader))) {
+                  .dataPrice(processableBlockHeader.getExcessDataGas().orElse(DataGas.ZERO)))) {
         return true;
       }
     }

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

@@ -16,8 +16,8 @@
 
 import static org.hyperledger.besu.util.Slf4jLambdaHelper.traceLambda;
 
+import org.hyperledger.besu.datatypes.DataGas;
 import org.hyperledger.besu.datatypes.Wei;
-import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.ethereum.core.Transaction;
 
 import java.math.BigInteger;
@@ -29,7 +29,7 @@
 public interface TransactionPriceCalculator {
   Wei price(Transaction transaction, Optional<Wei> maybeFee);
 
-  default Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader blockHeader) {
+  default Wei dataPrice(final DataGas excessDataGas) {
     return Wei.ZERO;
   }
 
@@ -38,11 +38,6 @@ class Frontier implements TransactionPriceCalculator {
     public Wei price(final Transaction transaction, final Optional<Wei> maybeFee) {
       return transaction.getGasPrice().orElse(Wei.ZERO);
     }
-
-    @Override
-    public Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader blockHeader) {
-      return Wei.ZERO;
-    }
   }
 
   class EIP1559 implements TransactionPriceCalculator {
@@ -73,17 +68,14 @@ public DataBlob(final int minDataGasPrice, final int dataGasPriceUpdateFraction)
     }
 
     @Override
-    public Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader blockHeader) {
-      final var excessDataGas = blockHeader.getExcessDataGas().orElseThrow();
-
+    public Wei dataPrice(final DataGas excessDataGas) {
       final var dataGasPrice =
           Wei.of(
               fakeExponential(
                   minDataGasPrice, excessDataGas.toBigInteger(), dataGasPriceUpdateFraction));
       traceLambda(
           LOG,
-          "block #{} parentExcessDataGas: {} dataGasPrice: {}",
-          blockHeader::getNumber,
+          "parentExcessDataGas: {} dataGasPrice: {}",
           excessDataGas::toShortHexString,
           dataGasPrice::toHexString);
 
@@ -92,14 +84,15 @@ public Wei dataPrice(final Transaction transaction, final ProcessableBlockHeader
 
     private BigInteger fakeExponential(
         final BigInteger factor, final BigInteger numerator, final BigInteger denominator) {
-      BigInteger i = BigInteger.ONE;
+      int i = 1;
       BigInteger output = BigInteger.ZERO;
       BigInteger numeratorAccumulator = factor.multiply(denominator);
-      while (numeratorAccumulator.compareTo(BigInteger.ZERO) > 0) {
+      while (numeratorAccumulator.signum() > 0) {
         output = output.add(numeratorAccumulator);
         numeratorAccumulator =
-            (numeratorAccumulator.multiply(numerator)).divide(denominator.multiply(i));
-        i = i.add(BigInteger.ONE);
+            (numeratorAccumulator.multiply(numerator))
+                .divide(denominator.multiply(BigInteger.valueOf(i)));
+        ++i;
       }
       return output.divide(denominator);
     }

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

@@ -135,10 +135,7 @@ public BlockProcessingResult processBlock(
       }
       worldStateUpdater.commit();
 
-      final long dataGasUsed =
-          protocolSpec.getGasCalculator().dataGasCost(transaction.getBlobCount());
-
-      currentGasUsed += transaction.getGasLimit() - result.getGasRemaining() - dataGasUsed;
+      currentGasUsed += transaction.getGasLimit() - result.getGasRemaining();
       final TransactionReceipt transactionReceipt =
           transactionReceiptFactory.create(
               transaction.getType(), result, worldState, currentGasUsed);

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

@@ -726,6 +726,7 @@ static ProtocolSpecBuilder cancunDefinition(
       final boolean quorumCompatibilityMode,
       final EvmConfiguration evmConfiguration) {
 
+    final int stackSizeLimit = configStackSizeLimit.orElse(MessageFrame.DEFAULT_MAX_STACK_SIZE);
     final int contractSizeLimit =
         configContractSizeLimit.orElse(SPURIOUS_DRAGON_CONTRACT_SIZE_LIMIT);
     final long londonForkBlockNumber = genesisConfigOptions.getLondonBlockNumber().orElse(0L);
@@ -765,6 +766,22 @@ static ProtocolSpecBuilder cancunDefinition(
                         MaxCodeSizeRule.of(contractSizeLimit), EOFValidationCodeRule.of(1, false)),
                     1,
                     SPURIOUS_DRAGON_FORCE_DELETE_WHEN_EMPTY_ADDRESSES))
+        // use Cancun fee market
+        .transactionProcessorBuilder(
+            (gasCalculator,
+                transactionValidator,
+                contractCreationProcessor,
+                messageCallProcessor) ->
+                new MainnetTransactionProcessor(
+                    gasCalculator,
+                    transactionValidator,
+                    contractCreationProcessor,
+                    messageCallProcessor,
+                    true,
+                    true,
+                    stackSizeLimit,
+                    cancunFeeMarket,
+                    CoinbaseFeePriceCalculator.eip1559()))
         // change to check for max data gas per block for EIP-4844
         .transactionValidatorBuilder(
             (gasCalculator, gasLimitCalculator) ->

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

@@ -20,9 +20,11 @@
 import static org.hyperledger.besu.ethereum.mainnet.PrivateStateUtils.KEY_TRANSACTION_HASH;
 
 import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.DataGas;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.core.feemarket.CoinbaseFeePriceCalculator;
@@ -252,7 +254,7 @@ public TransactionProcessingResult processTransaction(
   }
 
   public TransactionProcessingResult processTransaction(
-      final Blockchain ignoredBlockchain,
+      final Blockchain blockchain,
       final WorldUpdater worldState,
       final ProcessableBlockHeader blockHeader,
       final Transaction transaction,
@@ -288,16 +290,23 @@ public TransactionProcessingResult processTransaction(
 
       final MutableAccount senderMutableAccount = sender.getMutable();
       final long previousNonce = senderMutableAccount.incrementNonce();
-      final Wei transactionGasPrice =
-          feeMarket.getTransactionPriceCalculator().price(transaction, blockHeader.getBaseFee());
-      final Wei dataGasPrice =
-          feeMarket.getTransactionPriceCalculator().dataPrice(transaction, blockHeader);
       LOG.trace(
           "Incremented sender {} nonce ({} -> {})",
           senderAddress,
           previousNonce,
           sender.getNonce());
 
+      final Wei transactionGasPrice =
+          feeMarket.getTransactionPriceCalculator().price(transaction, blockHeader.getBaseFee());
+      final Wei dataGasPrice =
+          feeMarket
+              .getTransactionPriceCalculator()
+              .dataPrice(
+                  blockchain
+                      .getBlockHeader(blockHeader.getParentHash())
+                      .flatMap(BlockHeader::getExcessDataGas)
+                      .orElse(DataGas.ZERO));
+
       final long dataGas = gasCalculator.dataGasCost(transaction.getBlobCount());
 
       final Wei upfrontGasCost =
@@ -333,14 +342,13 @@ public TransactionProcessingResult processTransaction(
       final long accessListGas =
           gasCalculator.accessListGasCost(accessListEntries.size(), accessListStorageCount);
 
-      final long gasAvailable = transaction.getGasLimit() - intrinsicGas - accessListGas - dataGas;
+      final long gasAvailable = transaction.getGasLimit() - intrinsicGas - accessListGas;
       LOG.trace(
-          "Gas available for execution {} = {} - {} - {} - {} (limit - intrinsic - accessList - data)",
+          "Gas available for execution {} = {} - {} - {} - {} (limit - intrinsic - accessList)",
           gasAvailable,
           transaction.getGasLimit(),
           intrinsicGas,
-          accessListGas,
-          dataGas);
+          accessListGas);
 
       final WorldUpdater worldUpdater = worldState.updater();
       final Deque<MessageFrame> messageFrameStack = new ArrayDeque<>();
@@ -434,35 +442,36 @@ public TransactionProcessingResult processTransaction(
       // after the other so that if it is the same account somehow, we end up with the right result)
       final long selfDestructRefund =
           gasCalculator.getSelfDestructRefundAmount() * initialFrame.getSelfDestructs().size();
-      final long refundGas = initialFrame.getGasRefund() + selfDestructRefund;
-      final long refunded = refunded(transaction, initialFrame.getRemainingGas(), refundGas);
-      final Wei refundedWei = transactionGasPrice.multiply(refunded);
+      final long baseRefundGas = initialFrame.getGasRefund() + selfDestructRefund;
+      final long refundedGas = refunded(transaction, initialFrame.getRemainingGas(), baseRefundGas);
+      final Wei refundedWei = transactionGasPrice.multiply(refundedGas);
       senderMutableAccount.incrementBalance(refundedWei);
 
       final long gasUsedByTransaction = transaction.getGasLimit() - initialFrame.getRemainingGas();
 
       if (!worldState.getClass().equals(GoQuorumMutablePrivateWorldStateUpdater.class)) {
         // if this is not a private GoQuorum transaction we have to update the coinbase
         final var coinbase = worldState.getOrCreate(miningBeneficiary).getMutable();
-        final long coinbaseFee = transaction.getGasLimit() - refunded;
+        final long usedGas = transaction.getGasLimit() - refundedGas;
+        final CoinbaseFeePriceCalculator coinbaseCalculator;
         if (blockHeader.getBaseFee().isPresent()) {
           final Wei baseFee = blockHeader.getBaseFee().get();
           if (transactionGasPrice.compareTo(baseFee) < 0) {
             return TransactionProcessingResult.failed(
                 gasUsedByTransaction,
-                refunded,
+                refundedGas,
                 ValidationResult.invalid(
                     TransactionInvalidReason.TRANSACTION_PRICE_TOO_LOW,
                     "transaction price must be greater than base fee"),
                 Optional.empty());
           }
+          coinbaseCalculator = coinbaseFeePriceCalculator;
+        } else {
+          coinbaseCalculator = CoinbaseFeePriceCalculator.frontier();
         }
-        final CoinbaseFeePriceCalculator coinbaseCalculator =
-            blockHeader.getBaseFee().isPresent()
-                ? coinbaseFeePriceCalculator
-                : CoinbaseFeePriceCalculator.frontier();
+
         final Wei coinbaseWeiDelta =
-            coinbaseCalculator.price(coinbaseFee, transactionGasPrice, blockHeader.getBaseFee());
+            coinbaseCalculator.price(usedGas, transactionGasPrice, blockHeader.getBaseFee());
 
         coinbase.incrementBalance(coinbaseWeiDelta);
       }
@@ -477,12 +486,12 @@ public TransactionProcessingResult processTransaction(
         return TransactionProcessingResult.successful(
             initialFrame.getLogs(),
             gasUsedByTransaction,
-            refunded,
+            refundedGas,
             initialFrame.getOutputData(),
             validationResult);
       } else {
         return TransactionProcessingResult.failed(
-            gasUsedByTransaction, refunded, validationResult, initialFrame.getRevertReason());
+            gasUsedByTransaction, refundedGas, validationResult, initialFrame.getRevertReason());
       }
     } catch (final RuntimeException re) {
       LOG.error("Critical Exception Processing Transaction", re);
@@ -515,7 +524,7 @@ private AbstractMessageProcessor getMessageProcessor(final MessageFrame.Type typ
 
   protected long refunded(
       final Transaction transaction, final long gasRemaining, final long gasRefund) {
-    // Integer truncation takes care of the the floor calculation needed after the divide.
+    // Integer truncation takes care of the floor calculation needed after the divide.
     final long maxRefundAllowance =
         (transaction.getGasLimit() - gasRemaining) / gasCalculator.getMaxRefundQuotient();
     final long refundAllowance = Math.min(maxRefundAllowance, gasRefund);

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

@@ -134,6 +134,7 @@ public ValidationResult<TransactionInvalidReason> validate(
     if (!signatureResult.isValid()) {
       return signatureResult;
     }
+
     if (transaction.getType().equals(TransactionType.BLOB)
         && transaction.getBlobsWithCommitments().isPresent()) {
       final ValidationResult<TransactionInvalidReason> blobsResult =
@@ -202,11 +203,6 @@ private ValidationResult<TransactionInvalidReason> validateCostAndFee(
       }
     }
 
-    if (transaction.getNonce() == MAX_NONCE) {
-      return ValidationResult.invalid(
-          TransactionInvalidReason.NONCE_OVERFLOW, "Nonce must be less than 2^64-1");
-    }
-
     if (transaction.getType().supportsBlob()) {
       final long txTotalDataGas = gasCalculator.dataGasCost(transaction.getBlobCount());
       if (txTotalDataGas > gasLimitCalculator.currentDataGasLimit()) {