Move EIP-4762 contract account creation gas charge (#7869)

* Move EIP-4762 gas charging for contract account creation from code successfully created to transaction start Signed-off-by: Luis Pinto <luis.pinto@consensys.net> * fixup! Move EIP-4762 gas charging for contract account creation from code successfully created to transaction start compact code for debug printing witness gas schedule Signed-off-by: Luis Pinto <luis.pinto@consensys.net> * fixup! Move EIP-4762 gas charging for contract account creation from code successfully created to transaction start update verkle reference tests build Signed-off-by: Luis Pinto <luis.pinto@consensys.net> --------- Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
1 week ago · 445235dd2a
parent 09f92d22a7
commit 445235dd2a
4 changed files with 139 additions and 128 deletions
--- a/ethereum/referencetests/build.gradle
+++ b/ethereum/referencetests/build.gradle
@ -258,7 +258,7 @@ dependencies {
  }
  verkleRefTestImplemention dependencies.create('ethereum:execution-spec-tests') {
    version {
-      strictly 'verkle@v0.0.5'
+      strictly 'verkle@v0.0.8'
    }
    artifact {
      name = 'fixtures'
--- a/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/stateless/AccessEvents.java
+++ b/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/stateless/AccessEvents.java
@ -16,6 +16,12 @@ package org.hyperledger.besu.evm.gascalculator.stateless;

 public final class AccessEvents {

+  private static final long WITNESS_BRANCH_COST = 1900;
+  private static final long WITNESS_CHUNK_COST = 200;
+  private static final long SUBTREE_EDIT_COST = 3000;
+  private static final long CHUNK_EDIT_COST = 500;
+  private static final long CHUNK_FILL_COST = 6200;
+
  public static final short NONE = 0;
  public static final short BRANCH_READ = 1;
  public static final short BRANCH_WRITE = 2;
@ -44,4 +50,24 @@ public final class AccessEvents {
  public static boolean isLeafSet(final short accessEvents) {
    return (accessEvents & LEAF_SET) != 0;
  }
+
+  public static long getBranchReadCost() {
+    return WITNESS_BRANCH_COST;
+  }
+
+  public static long getLeafReadCost() {
+    return WITNESS_CHUNK_COST;
+  }
+
+  public static long getBranchWriteCost() {
+    return SUBTREE_EDIT_COST;
+  }
+
+  public static long getLeafResetCost() {
+    return CHUNK_EDIT_COST;
+  }
+
+  public static long getLeafSetCost() {
+    return CHUNK_FILL_COST;
+  }
 }
--- a/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/stateless/Eip4762AccessWitness.java
+++ b/evm/src/main/java/org/hyperledger/besu/evm/gascalculator/stateless/Eip4762AccessWitness.java
@ -40,11 +40,6 @@ public class Eip4762AccessWitness implements AccessWitness {

  private static final Logger LOG = LoggerFactory.getLogger(Eip4762AccessWitness.class);
  private static final TrieKeyAdapter TRIE_KEY_ADAPTER = new TrieKeyAdapter(new PedersenHasher());
-  private static final long WITNESS_BRANCH_COST = 1900;
-  private static final long WITNESS_CHUNK_COST = 200;
-  private static final long SUBTREE_EDIT_COST = 3000;
-  private static final long CHUNK_EDIT_COST = 500;
-  private static final long CHUNK_FILL_COST = 6200;

  private static final UInt256 zeroTreeIndex = UInt256.ZERO;
  private final Map<LeafAccessKey, Integer> leaves;
@ -289,89 +284,57 @@ public class Eip4762AccessWitness implements AccessWitness {
    final short accessEvents = touchAddress(address, treeIndex, subIndex, accessMode);
    long gas = 0;
    if (AccessEvents.isBranchRead(accessEvents)) {
-      gas = clampedAdd(gas, WITNESS_BRANCH_COST);
-      final long gasView = gas;
-      LOG.atDebug().log(
-          () ->
-              "touchAddressAndChargeGas WITNESS_BRANCH_COST "
-                  + address
-                  + " "
-                  + treeIndex
-                  + " "
-                  + subIndex
-                  + " "
-                  + AccessMode.toString(accessMode)
-                  + " "
-                  + gasView);
+      gas = clampedAdd(gas, AccessEvents.getBranchReadCost());
    }
    if (AccessEvents.isLeafRead(accessEvents)) {
-      gas = clampedAdd(gas, WITNESS_CHUNK_COST);
-      final long gasView = gas;
-      LOG.atDebug().log(
-          () ->
-              "touchAddressAndChargeGas WITNESS_CHUNK_COST "
-                  + address
-                  + " "
-                  + treeIndex
-                  + " "
-                  + subIndex
-                  + " "
-                  + AccessMode.toString(accessMode)
-                  + " "
-                  + gasView);
+      gas = clampedAdd(gas, AccessEvents.getLeafReadCost());
    }
    if (AccessEvents.isBranchWrite(accessEvents)) {
-      gas = clampedAdd(gas, SUBTREE_EDIT_COST);
-      final long gasView = gas;
-      LOG.atDebug().log(
-          () ->
-              "touchAddressAndChargeGas SUBTREE_EDIT_COST "
-                  + address
-                  + " "
-                  + treeIndex
-                  + " "
-                  + subIndex
-                  + " "
-                  + AccessMode.toString(accessMode)
-                  + " "
-                  + gasView);
+      gas = clampedAdd(gas, AccessEvents.getBranchWriteCost());
    }
    if (AccessEvents.isLeafReset(accessEvents)) {
-      gas = clampedAdd(gas, CHUNK_EDIT_COST);
-      final long gasView = gas;
-      LOG.atDebug().log(
-          () ->
-              "touchAddressAndChargeGas CHUNK_EDIT_COST "
-                  + address
-                  + " "
-                  + treeIndex
-                  + " "
-                  + subIndex
-                  + " "
-                  + AccessMode.toString(accessMode)
-                  + " "
-                  + gasView);
+      gas = clampedAdd(gas, AccessEvents.getLeafResetCost());
    }
    if (AccessEvents.isLeafSet(accessEvents)) {
-      gas = clampedAdd(gas, CHUNK_FILL_COST);
+      gas = clampedAdd(gas, AccessEvents.getLeafSetCost());
+    }
+
    final long gasView = gas;
    LOG.atDebug().log(
        () ->
-              "touchAddressAndChargeGas CHUNK_FILL_COST "
+            "touch witness "
                + address
                + " "
-                  + treeIndex
+                + treeIndex.toShortHexString()
                + " "
-                  + subIndex
-                  + " "
-                  + AccessMode.toString(accessMode)
-                  + " "
-                  + gasView);
-    }
+                + subIndex.toShortHexString()
+                + "\ntotal charges "
+                + gasView
+                + costSchedulePrettyPrint(accessEvents));

    return gas;
  }

+  private static String costSchedulePrettyPrint(final short accessEvents) {
+    String message = "";
+    if (AccessEvents.isBranchRead(accessEvents)) {
+      message += "\n\tWITNESS_BRANCH_COST " + AccessEvents.getBranchReadCost();
+    }
+    if (AccessEvents.isLeafRead(accessEvents)) {
+      message += "\n\tWITNESS_CHUNK_COST " + AccessEvents.getLeafReadCost();
+    }
+    if (AccessEvents.isBranchWrite(accessEvents)) {
+      message += "\n\tSUBTREE_EDIT_COST " + AccessEvents.getBranchWriteCost();
+    }
+    if (AccessEvents.isLeafReset(accessEvents)) {
+      message += "\n\tCHUNK_EDIT_COST " + AccessEvents.getLeafResetCost();
+    }
+    if (AccessEvents.isLeafSet(accessEvents)) {
+      message += "\n\tCHUNK_FILL_COST " + AccessEvents.getLeafSetCost();
+    }
+    return message;
+  }
+
  public short touchAddress(
      final Address addr, final UInt256 treeIndex, final UInt256 leafIndex, final int accessMode) {
    short accessEvents = AccessEvents.NONE;
--- a/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java
+++ b/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java
@ -28,6 +28,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Supplier;

 import org.apache.tuweni.bytes.Bytes;
 import org.slf4j.Logger;
@ -101,7 +102,17 @@ public class ContractCreationProcessor extends AbstractMessageProcessor {
      final MutableAccount contract = frame.getWorldUpdater().getOrCreate(contractAddress);
      long statelessGasCost =
          evm.getGasCalculator().proofOfAbsenceCost(frame, contract.getAddress());
+      if (handleInsufficientGas(
+          frame,
+          statelessGasCost,
+          () ->
+              String.format(
+                  "Not enough gas to cover proof of absence fee for %s: remaining gas = %d < %d = creation fee",
+                  frame.getContractAddress(), frame.getRemainingGas(), statelessGasCost))) {
+        return;
+      }
      frame.decrementRemainingGas(statelessGasCost);
+
      if (accountExists(contract)) {
        LOG.trace(
            "Contract creation error: account has already been created for address {}",
@ -111,6 +122,20 @@ public class ContractCreationProcessor extends AbstractMessageProcessor {
        operationTracer.traceAccountCreationResult(
            frame, Optional.of(ExceptionalHaltReason.ILLEGAL_STATE_CHANGE));
      } else {
+        final long accountCreationFee =
+            evm.getGasCalculator().completedCreateContractGasCost(frame);
+        if (handleInsufficientGas(
+            frame,
+            accountCreationFee,
+            () ->
+                String.format(
+                    "Not enough gas to pay the contract creation fee for %s: "
+                        + "remaining gas = %d < %d = creation fee",
+                    frame.getContractAddress(), frame.getRemainingGas(), accountCreationFee))) {
+          return;
+        }
+        frame.decrementRemainingGas(accountCreationFee);
+
        frame.addCreate(contractAddress);
        contract.incrementBalance(frame.getValue());
        contract.setNonce(initialContractNonce);
@ -132,47 +157,41 @@ public class ContractCreationProcessor extends AbstractMessageProcessor {

    final long depositFee = evm.getGasCalculator().codeDepositGasCost(frame, contractCode.size());

-    if (frame.getRemainingGas() < depositFee) {
-      LOG.trace(
-          "Not enough gas to pay the code deposit fee for {}: "
-              + "remaining gas = {} < {} = deposit fee",
-          frame.getContractAddress(),
-          frame.getRemainingGas(),
-          depositFee);
+    if (handleInsufficientGas(
+        frame,
+        depositFee,
+        () ->
+            String.format(
+                "Not enough gas to pay the code deposit fee for %s: "
+                    + "remaining gas = %d < %d = deposit fee",
+                frame.getContractAddress(), frame.getRemainingGas(), depositFee))) {
+
      if (requireCodeDepositToSucceed) {
-        LOG.trace("Contract creation error: insufficient funds for code deposit");
-        frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
-        frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
        operationTracer.traceAccountCreationResult(
            frame, Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
      } else {
        frame.setState(MessageFrame.State.COMPLETED_SUCCESS);
      }
-    } else {
+      return;
+    }
+
    final var invalidReason =
        contractValidationRules.stream()
            .map(rule -> rule.validate(contractCode, frame, evm))
            .filter(Optional::isPresent)
            .findFirst();
-      if (invalidReason.isEmpty()) {
+    if (invalidReason.isPresent()) {
+      final Optional<ExceptionalHaltReason> exceptionalHaltReason = invalidReason.get();
+      frame.setExceptionalHaltReason(exceptionalHaltReason);
+      frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
+      operationTracer.traceAccountCreationResult(frame, exceptionalHaltReason);
+      return;
+    }
+
    frame.decrementRemainingGas(depositFee);

-        final long statelessContractCompletionFee =
-            evm.getGasCalculator().completedCreateContractGasCost(frame);
-        if (frame.getRemainingGas() < statelessContractCompletionFee) {
-          LOG.trace(
-              "Not enough gas to pay the contract creation completion fee for {}: "
-                  + "remaining gas = {} < {} = deposit fee",
-              frame.getContractAddress(),
-              frame.getRemainingGas(),
-              statelessContractCompletionFee);
-          frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
-          frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
-        } else {
-          frame.decrementRemainingGas(statelessContractCompletionFee);
    // Finalize contract creation, setting the contract code.
-          final MutableAccount contract =
-              frame.getWorldUpdater().getOrCreate(frame.getContractAddress());
+    final MutableAccount contract = frame.getWorldUpdater().getOrCreate(frame.getContractAddress());
    contract.setCode(contractCode);
    LOG.trace(
        "Successful creation of contract {} with code of size {} (Gas remaining: {})",
@ -180,17 +199,20 @@ public class ContractCreationProcessor extends AbstractMessageProcessor {
        contractCode.size(),
        frame.getRemainingGas());
    frame.setState(MessageFrame.State.COMPLETED_SUCCESS);
-        }

    if (operationTracer.isExtendedTracing()) {
      operationTracer.traceAccountCreationResult(frame, Optional.empty());
    }
-      } else {
-        final Optional<ExceptionalHaltReason> exceptionalHaltReason = invalidReason.get();
-        frame.setExceptionalHaltReason(exceptionalHaltReason);
-        frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
-        operationTracer.traceAccountCreationResult(frame, exceptionalHaltReason);
  }
+
+  private static boolean handleInsufficientGas(
+      final MessageFrame frame, final long gasFee, final Supplier<String> message) {
+    if (frame.getRemainingGas() < gasFee) {
+      LOG.trace(message.get());
+      frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
+      frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
+      return true;
    }
+    return false;
  }
 }