Fix tracing, upfrontgascost and transaction pool issues for london(#2342)

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Co-authored-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java

@@ -526,7 +526,7 @@ public class Transaction implements org.hyperledger.besu.plugin.data.Transaction
    * @return the up-front cost for the gas the transaction can use.
    */
   public Wei getUpfrontGasCost() {
-    return getUpfrontGasCost(getGasPrice());
+    return getUpfrontGasCost((Wei.of(getMaxFeePerGas().orElse(getGasPrice()).getAsBigInteger())));
   }
 
   /**
@@ -536,8 +536,10 @@ public class Transaction implements org.hyperledger.besu.plugin.data.Transaction
    * @return the up-front cost for the gas the transaction can use.
    */
   public Wei getUpfrontGasCost(final Wei gasPrice) {
-    return Wei.of(getGasLimit())
+    if (gasPrice == null || gasPrice.isZero()) {
-        .multiply(Wei.of(getMaxFeePerGas().orElse(gasPrice).getAsBigInteger()));
+      return Wei.ZERO;
+    }
+    return Wei.of(getGasLimit()).multiply(gasPrice);
   }
 
   /**

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/FixedStack.java

@@ -66,7 +66,7 @@ public class FixedStack<T> {
 
   public T get(final int offset) {
     if (offset < 0 || offset >= size()) {
-      throw new IndexOutOfBoundsException();
+      throw new UnderflowException();
     }
 
     return entries[top - offset];

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/MessageFrame.java

@@ -29,6 +29,7 @@ import org.hyperledger.besu.ethereum.core.Wei;
 import org.hyperledger.besu.ethereum.core.WorldUpdater;
 import org.hyperledger.besu.ethereum.mainnet.AbstractMessageProcessor;
 import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater;
+import org.hyperledger.besu.ethereum.vm.FixedStack.UnderflowException;
 import org.hyperledger.besu.ethereum.vm.internal.MemoryEntry;
 
 import java.util.ArrayList;
@@ -450,7 +451,7 @@ public class MessageFrame {
    *
    * @param offset The item's position relative to the top of the stack
    * @return The item at the specified offset in the stack
-   * @throws IndexOutOfBoundsException if the offset is out of range
+   * @throws UnderflowException if the offset is out of range
    */
   public Bytes32 getStackItem(final int offset) {
     return stack.get(offset);
@@ -460,7 +461,7 @@ public class MessageFrame {
    * Removes the item at the top of the stack.
    *
    * @return the item at the top of the stack
-   * @throws IllegalStateException if the stack is empty
+   * @throws UnderflowException if the stack is empty
    */
   public Bytes32 popStackItem() {
     return stack.pop();

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/StandardJsonTracer.java

@@ -33,8 +33,6 @@ import org.apache.tuweni.units.bigints.UInt256;
 
 public class StandardJsonTracer implements OperationTracer {
 
-  private static final int EIP_150_DIVISOR = 64;
-
   private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
 
   private final PrintStream out;
@@ -88,18 +86,8 @@ public class StandardJsonTracer implements OperationTracer {
     final OperationResult executeResult = executeOperation.execute();
 
     traceLine.put("refund", messageFrame.getGasRefund().toLong());
-
+    traceLine.put(
-    if (AbstractCallOperation.class.isAssignableFrom(
+        "gasCost", executeResult.getGasCost().map(gas -> shortNumber(gas.asUInt256())).orElse(""));
-        messageFrame.getCurrentOperation().getClass())) {
-      final AbstractCallOperation callOperation =
-          (AbstractCallOperation) messageFrame.getCurrentOperation();
-      traceLine.put(
-          "gasCost", shortNumber((computeCallGas(callOperation, messageFrame).asUInt256())));
-    } else {
-      traceLine.put(
-          "gasCost",
-          executeResult.getGasCost().map(gas -> shortNumber(gas.asUInt256())).orElse(""));
-    }
 
     if (showMemory) {
       traceLine.put(
@@ -123,18 +111,7 @@ public class StandardJsonTracer implements OperationTracer {
                     .orElse(""));
     traceLine.put("opName", currentOp.getName());
     traceLine.put("error", error);
-    out.println(traceLine.toString());
+    out.println(traceLine);
-  }
-
-  private Gas computeCallGas(
-      final AbstractCallOperation callOperation, final MessageFrame messageFrame) {
-    // as part of EIP 150 the returned costGas is gas - base * 63 / 64.
-    // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md
-    final Gas callCost = callOperation.gas(messageFrame);
-    final Gas gasAvailable = callCost.minus(messageFrame.getGasCost().orElse(Gas.ZERO));
-    final Gas gas = gasAvailable.minus(gasAvailable.dividedBy(EIP_150_DIVISOR));
-    final Gas baseCost = callOperation.cost(messageFrame);
-    return ((gas.toLong() > 0 && gas.compareTo(callCost) < 0) ? gas : callCost).plus(baseCost);
   }
 
   @Override

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/OperandStackTest.java

@@ -56,13 +56,13 @@ public class OperandStackTest {
     assertThat(stack.pop()).isEqualTo(Bytes32.fromHexString("0x01"));
   }
 
-  @Test(expected = IndexOutOfBoundsException.class)
+  @Test(expected = UnderflowException.class)
   public void get_NegativeOffset() {
     final OperandStack stack = new OperandStack(1);
     stack.get(-1);
   }
 
-  @Test(expected = IndexOutOfBoundsException.class)
+  @Test(expected = UnderflowException.class)
   public void get_IndexGreaterThanSize() {
     final OperandStack stack = new OperandStack(1);
     stack.push(Bytes32.fromHexString("0x01"));
@@ -81,13 +81,13 @@ public class OperandStackTest {
     assertThat(stack.get(2)).isEqualTo(Bytes32.fromHexString("0x01"));
   }
 
-  @Test(expected = IndexOutOfBoundsException.class)
+  @Test(expected = UnderflowException.class)
   public void set_NegativeOffset() {
     final OperandStack stack = new OperandStack(1);
     stack.get(-1);
   }
 
-  @Test(expected = IndexOutOfBoundsException.class)
+  @Test(expected = UnderflowException.class)
   public void set_IndexGreaterThanSize() {
     final OperandStack stack = new OperandStack(1);
     stack.push(Bytes32.fromHexString("0x01"));

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java

@@ -41,6 +41,7 @@ import org.hyperledger.besu.ethereum.mainnet.TransactionValidationParams;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
 import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
 import org.hyperledger.besu.metrics.BesuMetricCategory;
+import org.hyperledger.besu.plugin.data.TransactionType;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
 import org.hyperledger.besu.plugin.services.metrics.Counter;
 import org.hyperledger.besu.plugin.services.metrics.LabelledMetric;
@@ -137,13 +138,13 @@ public class TransactionPool implements BlockAddedObserver {
 
   public ValidationResult<TransactionInvalidReason> addLocalTransaction(
       final Transaction transaction) {
-    if (!configuration.getTxFeeCap().isZero()
-        && minTransactionGasPrice(transaction).compareTo(configuration.getTxFeeCap()) > 0) {
-      return ValidationResult.invalid(TransactionInvalidReason.TX_FEECAP_EXCEEDED);
-    }
     final ValidationResult<TransactionInvalidReason> validationResult =
         validateTransaction(transaction);
     if (validationResult.isValid()) {
+      if (!configuration.getTxFeeCap().isZero()
+          && minTransactionGasPrice(transaction).compareTo(configuration.getTxFeeCap()) > 0) {
+        return ValidationResult.invalid(TransactionInvalidReason.TX_FEECAP_EXCEEDED);
+      }
       final TransactionAddedStatus transactionAddedStatus =
           pendingTransactions.addLocalTransaction(transaction);
       if (!transactionAddedStatus.equals(TransactionAddedStatus.ADDED)) {
@@ -244,6 +245,14 @@ public class TransactionPool implements BlockAddedObserver {
               "Transaction gas limit of %s exceeds block gas limit of %s",
               transaction.getGasLimit(), chainHeadBlockHeader.getGasLimit()));
     }
+    if (transaction.getType().equals(TransactionType.EIP1559)
+        && !eip1559
+            .map(eip1559 -> eip1559.isEIP1559(chainHeadBlockHeader.getNumber()))
+            .orElse(false)) {
+      return ValidationResult.invalid(
+          TransactionInvalidReason.INVALID_TRANSACTION_FORMAT,
+          String.format("EIP-1559 transaction are not allowed yet"));
+    }
 
     return protocolContext
         .getWorldStateArchive()
@@ -273,14 +282,9 @@ public class TransactionPool implements BlockAddedObserver {
   }
 
   private Wei minTransactionGasPrice(final Transaction transaction) {
-    // EIP-1559 enablement guard block
-    if (this.eip1559.isEmpty()) {
-      return frontierPriceCalculator.price(transaction, Optional.empty());
-    }
-
     final BlockHeader chainHeadBlockHeader = getChainHeadBlockHeader();
     // Compute transaction price using EIP-1559 rules if chain head is after fork
-    if (this.eip1559.get().isEIP1559(chainHeadBlockHeader.getNumber())) {
+    if (eip1559.isPresent() && eip1559.get().isEIP1559(chainHeadBlockHeader.getNumber())) {
       return BaseFee.minTransactionPriceInNextBlock(
           transaction, eip1559PriceCalculator, chainHeadBlockHeader::getBaseFee);
     } else { // Use frontier rules otherwise

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolTest.java

@@ -51,6 +51,7 @@ import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.core.TransactionReceipt;
 import org.hyperledger.besu.ethereum.core.TransactionTestFixture;
 import org.hyperledger.besu.ethereum.core.Wei;
+import org.hyperledger.besu.ethereum.core.fees.EIP1559;
 import org.hyperledger.besu.ethereum.eth.EthProtocol;
 import org.hyperledger.besu.ethereum.eth.manager.EthContext;
 import org.hyperledger.besu.ethereum.eth.manager.EthPeer;
@@ -66,6 +67,7 @@ import org.hyperledger.besu.ethereum.mainnet.TransactionValidationParams;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
 import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
 import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
+import org.hyperledger.besu.plugin.data.TransactionType;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
 import org.hyperledger.besu.testutil.TestClock;
 
@@ -704,6 +706,92 @@ public class TransactionPoolTest {
         .isTrue();
   }
 
+  @Test
+  public void shouldIgnoreEIP1559TransactionWhenNotAllowed() {
+    final EthProtocolManager ethProtocolManager = EthProtocolManagerTestUtil.create();
+    final EthContext ethContext = ethProtocolManager.ethContext();
+    final PeerTransactionTracker peerTransactionTracker = new PeerTransactionTracker();
+    final TransactionPool transactionPool =
+        new TransactionPool(
+            transactions,
+            protocolSchedule,
+            protocolContext,
+            batchAddedListener,
+            pendingBatchAddedListener,
+            syncState,
+            ethContext,
+            peerTransactionTracker,
+            peerPendingTransactionTracker,
+            Wei.ZERO,
+            metricsSystem,
+            Optional.empty(),
+            ImmutableTransactionPoolConfiguration.builder().txFeeCap(Wei.ONE).build());
+    when(transactionValidator.validate(any(Transaction.class), any(Optional.class)))
+        .thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            any(Transaction.class),
+            nullable(Account.class),
+            any(TransactionValidationParams.class)))
+        .thenReturn(valid());
+    final Transaction transaction =
+        new TransactionTestFixture()
+            .nonce(1)
+            .type(TransactionType.EIP1559)
+            .maxFeePerGas(Optional.of(Wei.ONE))
+            .maxPriorityFeePerGas(Optional.of(Wei.ONE))
+            .gasLimit(10)
+            .gasPrice(null)
+            .createTransaction(KEY_PAIR1);
+
+    final ValidationResult<TransactionInvalidReason> result =
+        transactionPool.addLocalTransaction(transaction);
+    assertThat(result.getInvalidReason())
+        .isEqualTo(TransactionInvalidReason.INVALID_TRANSACTION_FORMAT);
+  }
+
+  @Test
+  public void shouldIgnoreEIP1559TransactionBeforeTheFork() {
+    final EthProtocolManager ethProtocolManager = EthProtocolManagerTestUtil.create();
+    final EthContext ethContext = ethProtocolManager.ethContext();
+    final PeerTransactionTracker peerTransactionTracker = new PeerTransactionTracker();
+    final TransactionPool transactionPool =
+        new TransactionPool(
+            transactions,
+            protocolSchedule,
+            protocolContext,
+            batchAddedListener,
+            pendingBatchAddedListener,
+            syncState,
+            ethContext,
+            peerTransactionTracker,
+            peerPendingTransactionTracker,
+            Wei.ZERO,
+            metricsSystem,
+            Optional.of(new EIP1559(100)),
+            ImmutableTransactionPoolConfiguration.builder().txFeeCap(Wei.ONE).build());
+    when(transactionValidator.validate(any(Transaction.class), any(Optional.class)))
+        .thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            any(Transaction.class),
+            nullable(Account.class),
+            any(TransactionValidationParams.class)))
+        .thenReturn(valid());
+    final Transaction transaction =
+        new TransactionTestFixture()
+            .nonce(1)
+            .type(TransactionType.EIP1559)
+            .maxFeePerGas(Optional.of(Wei.ONE))
+            .maxPriorityFeePerGas(Optional.of(Wei.ONE))
+            .gasLimit(10)
+            .gasPrice(null)
+            .createTransaction(KEY_PAIR1);
+
+    final ValidationResult<TransactionInvalidReason> result =
+        transactionPool.addLocalTransaction(transaction);
+    assertThat(result.getInvalidReason())
+        .isEqualTo(TransactionInvalidReason.INVALID_TRANSACTION_FORMAT);
+  }
+
   @Test
   public void shouldRejectLocalTransactionIfFeeCapExceeded() {
     final EthProtocolManager ethProtocolManager = EthProtocolManagerTestUtil.create();
@@ -730,6 +818,14 @@ public class TransactionPoolTest {
     final Transaction transactionLocal =
         builder.nonce(1).gasPrice(twoEthers.add(Wei.of(1))).createTransaction(KEY_PAIR1);
 
+    when(transactionValidator.validate(any(Transaction.class), any(Optional.class)))
+        .thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            any(Transaction.class),
+            nullable(Account.class),
+            any(TransactionValidationParams.class)))
+        .thenReturn(valid());
+
     final ValidationResult<TransactionInvalidReason> result =
         transactionPool.addLocalTransaction(transactionLocal);
     assertThat(result.getInvalidReason()).isEqualTo(TransactionInvalidReason.TX_FEECAP_EXCEEDED);