EOF updates (#4859)

* EOF updates EOF changes tracking Breakout room #4 changes. Also, add more test coverage. Signed-off-by: Danno Ferrin <[email protected]>
hyperledger · Dec 23, 2022 · 986077e · 986077e
1 parent 642d477
commit 986077e
Show file tree

Hide file tree

Showing 19 changed files with 735 additions and 74 deletions.
diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java b/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java
@@ -37,6 +37,7 @@
 import org.hyperledger.besu.evm.account.EvmAccount;
 import org.hyperledger.besu.evm.account.MutableAccount;
 import org.hyperledger.besu.evm.code.CodeV0;
+import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
 import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.gascalculator.GasCalculator;
 import org.hyperledger.besu.evm.processor.AbstractMessageProcessor;
@@ -400,8 +401,13 @@ public TransactionProcessingResult processTransaction(
 
       messageFrameStack.addFirst(initialFrame);
 
-      while (!messageFrameStack.isEmpty()) {
-        process(messageFrameStack.peekFirst(), operationTracer);
+      if (initialFrame.getCode().isValid()) {
+        while (!messageFrameStack.isEmpty()) {
+          process(messageFrameStack.peekFirst(), operationTracer);
+        }
+      } else {
+        initialFrame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
+        initialFrame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.INVALID_CODE));
       }
 
       if (initialFrame.getState() == MessageFrame.State.COMPLETED_SUCCESS) {

diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java
@@ -221,10 +221,10 @@ public void run() {
       Log4j2ConfiguratorUtil.setAllLevels("", repeat == 0 ? Level.INFO : Level.OFF);
       int remainingIters = this.repeat;
       Log4j2ConfiguratorUtil.setLevel(
-          "org.hyperledger.besu.ethereum.mainnet.ProtocolScheduleBuilder", Level.OFF);
+          "org.hyperledger.besu.ethereum.mainnet.AbstractProtocolScheduleBuilder", Level.OFF);
       final ProtocolSpec protocolSpec = component.getProtocolSpec().apply(0);
       Log4j2ConfiguratorUtil.setLevel(
-          "org.hyperledger.besu.ethereum.mainnet.ProtocolScheduleBuilder", null);
+          "org.hyperledger.besu.ethereum.mainnet.AbstractProtocolScheduleBuilder", null);
       final Transaction tx =
           new Transaction(
               0,

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/code/CodeFactory.java b/evm/src/main/java/org/hyperledger/besu/evm/code/CodeFactory.java
@@ -71,7 +71,7 @@ public static Code createCode(
 
         final String stackValidationError = CodeV1.validateStack(layout);
         if (stackValidationError != null) {
-          return new CodeInvalid(codeHash, bytes, "EOF Code Invalid : " + codeValidationError);
+          return new CodeInvalid(codeHash, bytes, "EOF Code Invalid : " + stackValidationError);
         }
 
         return new CodeV1(codeHash, layout);

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1.java b/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1.java
@@ -549,7 +549,7 @@ public class CodeV1 implements Code {
     {3, 1, 1}, // 0xf0 - CREATE
     {7, 1, 1}, // 0xf1 - CALL
     {0, 1, 1}, // 0xf2 - CALLCODE
-    {2, 0, 1}, // 0xf3 - RETURN
+    {2, 0, -1}, // 0xf3 - RETURN
     {6, 1, 1}, // 0xf4 - DELEGATECALL
     {4, 1, 1}, // 0xf5 - CREATE2
     {0, 0, 0}, // 0xf6

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java b/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java
@@ -99,6 +99,12 @@ public static EOFLayout parseEOF(final Bytes container) {
     if (codeSectionCount < 0) {
       return invalidLayout(container, version, "Invalid Code section count");
     }
+    if (codeSectionCount > 1024) {
+      return invalidLayout(
+          container,
+          version,
+          "Too many code sections - 0x" + Integer.toHexString(codeSectionCount));
+    }
     int[] codeSectionSizes = new int[codeSectionCount];
     for (int i = 0; i < codeSectionCount; i++) {
       int size = readUnsignedShort(inputStream);
@@ -142,6 +148,24 @@ public static EOFLayout parseEOF(final Bytes container) {
       if (inputStream.read(code, 0, codeSectionSize) != codeSectionSize) {
         return invalidLayout(container, version, "Incomplete code section " + i);
       }
+      if (typeData[i][0] > 0x7f) {
+        return invalidLayout(
+            container,
+            version,
+            "Type data input stack too large - 0x" + Integer.toHexString(typeData[i][0]));
+      }
+      if (typeData[i][1] > 0x7f) {
+        return invalidLayout(
+            container,
+            version,
+            "Type data output stack too large - 0x" + Integer.toHexString(typeData[i][1]));
+      }
+      if (typeData[i][2] > 0x3ff) {
+        return invalidLayout(
+            container,
+            version,
+            "Type data max stack too large - 0x" + Integer.toHexString(typeData[i][2]));
+      }
       codeSections[i] =
           new CodeSection(Bytes.wrap(code), typeData[i][0], typeData[i][1], typeData[i][2]);
     }

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/ExceptionalHaltReason.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/ExceptionalHaltReason.java
@@ -32,8 +32,11 @@ public interface ExceptionalHaltReason {
   ExceptionalHaltReason INVALID_CODE = DefaultExceptionalHaltReason.INVALID_CODE;
   ExceptionalHaltReason PRECOMPILE_ERROR = DefaultExceptionalHaltReason.PRECOMPILE_ERROR;
   ExceptionalHaltReason CODE_SECTION_MISSING = DefaultExceptionalHaltReason.CODE_SECTION_MISSING;
-  ExceptionalHaltReason MISMATCHED_CODE_SECTION_OUTPUTS =
-      DefaultExceptionalHaltReason.MISMATCHED_CODE_SECTION_OUTPUTS;
+  ExceptionalHaltReason INCORRECT_CODE_SECTION_RETURN_OUTPUTS =
+      DefaultExceptionalHaltReason.INCORRECT_CODE_SECTION_RETURN_OUTPUTS;
+  ExceptionalHaltReason TOO_FEW_INPUTS_FOR_CODE_SECTION =
+      DefaultExceptionalHaltReason.TOO_FEW_INPUTS_FOR_CODE_SECTION;
+  ExceptionalHaltReason JUMPF_STACK_MISMATCH = DefaultExceptionalHaltReason.JUMPF_STACK_MISMATCH;
 
   String name();
 
@@ -53,8 +56,12 @@ enum DefaultExceptionalHaltReason implements ExceptionalHaltReason {
     INVALID_CODE("Code is invalid"),
     PRECOMPILE_ERROR("Precompile error"),
     CODE_SECTION_MISSING("No code section at requested index"),
-    MISMATCHED_CODE_SECTION_OUTPUTS("Jumped into a code section with unequal declared outputs"),
-    INSUFFICIENT_CODE_SECTION_RETURN_DATA("The stack for a return ");
+    INSUFFICIENT_CODE_SECTION_RETURN_DATA("The stack for a return "),
+    INCORRECT_CODE_SECTION_RETURN_OUTPUTS(
+        "The return of a code section does not have the correct number of outputs"),
+    TOO_FEW_INPUTS_FOR_CODE_SECTION("Not enough stack items for a function call"),
+    JUMPF_STACK_MISMATCH(
+        "The stack height for a JUMPF does not match the requirements of the target section");
 
     final String description;
 

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java
@@ -286,6 +286,7 @@ private MessageFrame(
     this.memory = new Memory();
     this.stack = new OperandStack(maxStackSize);
     this.returnStack = new ReturnStack();
+    returnStack.push(new ReturnStack.ReturnStackItem(0, 0, 0));
     this.output = Bytes.EMPTY;
     this.returnData = Bytes.EMPTY;
     this.logs = new ArrayList<>();
@@ -347,6 +348,15 @@ public void setPC(final int pc) {
     this.pc = pc;
   }
 
+  /**
+   * Set the code section index.
+   *
+   * @param section the code section index
+   */
+  public void setSection(final int section) {
+    this.section = section;
+  }
+
   /**
    * Return the current code section. Always zero for legacy code.
    *
@@ -362,6 +372,8 @@ public ExceptionalHaltReason callFunction(final int calledSection) {
       return ExceptionalHaltReason.CODE_SECTION_MISSING;
     } else if (stack.size() + info.getMaxStackHeight() > maxStackSize) {
       return ExceptionalHaltReason.TOO_MANY_STACK_ITEMS;
+    } else if (stack.size() < info.getInputs()) {
+      return ExceptionalHaltReason.TOO_FEW_INPUTS_FOR_CODE_SECTION;
     } else {
       returnStack.push(
           new ReturnStack.ReturnStackItem(section, pc + 2, stack.size() - info.getInputs()));
@@ -372,25 +384,32 @@ public ExceptionalHaltReason callFunction(final int calledSection) {
   }
 
   public ExceptionalHaltReason jumpFunction(final int section) {
-    CodeSection thisInfo = code.getCodeSection(this.section);
     CodeSection info = code.getCodeSection(section);
     if (info == null) {
       return ExceptionalHaltReason.CODE_SECTION_MISSING;
-    } else if (thisInfo.getOutputs() != info.getOutputs()) {
-      return ExceptionalHaltReason.MISMATCHED_CODE_SECTION_OUTPUTS;
+    } else if (stackSize() != peekReturnStack().getStackHeight() + info.getInputs()) {
+      return ExceptionalHaltReason.JUMPF_STACK_MISMATCH;
     } else {
       pc = -1; // will be +1ed at end of operations loop
       this.section = section;
       return null;
     }
   }
 
-  public void returnFunction() {
+  public ExceptionalHaltReason returnFunction() {
     CodeSection thisInfo = code.getCodeSection(this.section);
     var returnInfo = returnStack.pop();
-    stack.preserveTop(returnInfo.getStackHeight(), thisInfo.getOutputs());
-    this.pc = returnInfo.getPC();
-    this.section = returnInfo.getCodeSectionIndex();
+    if ((returnInfo.getStackHeight() + thisInfo.getOutputs()) != stack.size()) {
+      return ExceptionalHaltReason.INCORRECT_CODE_SECTION_RETURN_OUTPUTS;
+    } else if (returnStack.isEmpty()) {
+      setState(MessageFrame.State.CODE_SUCCESS);
+      setOutputData(Bytes.EMPTY);
+      return null;
+    } else {
+      this.pc = returnInfo.getPC();
+      this.section = returnInfo.getCodeSectionIndex();
+      return null;
+    }
   }
 
   /** Deducts the remaining gas. */
@@ -540,6 +559,33 @@ public int stackSize() {
     return stack.size();
   }
 
+  /**
+   * Return the current return stack size.
+   *
+   * @return The current return stack size
+   */
+  public int returnStackSize() {
+    return returnStack.size();
+  }
+
+  /**
+   * The top item of the return stack
+   *
+   * @return The top item of the return stack, or null if the stack is empty
+   */
+  public ReturnStack.ReturnStackItem peekReturnStack() {
+    return returnStack.peek();
+  }
+
+  /**
+   * Pushes a new return stack item onto the return stack
+   *
+   * @param returnStackItem item to be pushed
+   */
+  public void pushReturnStackItem(final ReturnStack.ReturnStackItem returnStackItem) {
+    returnStack.push(returnStackItem);
+  }
+
   /**
    * Returns whether the message frame is static or not.
    *

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/internal/FixedStack.java b/evm/src/main/java/org/hyperledger/besu/evm/internal/FixedStack.java
@@ -82,6 +82,14 @@ public T pop() {
     return removed;
   }
 
+  public T peek() {
+    if (top < 0) {
+      return null;
+    } else {
+      return entries[top];
+    }
+  }
+
   /**
    * Pops the specified number of operands from the stack.
    *

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/internal/ReturnStack.java b/evm/src/main/java/org/hyperledger/besu/evm/internal/ReturnStack.java
@@ -14,6 +14,8 @@
  */
 package org.hyperledger.besu.evm.internal;
 
+import java.util.Objects;
+
 public class ReturnStack extends FixedStack<ReturnStack.ReturnStackItem> {
 
   // Java17 convert to record
@@ -40,6 +42,33 @@ public int getPC() {
     public int getStackHeight() {
       return stackHeight;
     }
+
+    @Override
+    public boolean equals(final Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      ReturnStackItem that = (ReturnStackItem) o;
+      return codeSectionIndex == that.codeSectionIndex
+          && pc == that.pc
+          && stackHeight == that.stackHeight;
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(codeSectionIndex, pc, stackHeight);
+    }
+
+    @Override
+    public String toString() {
+      return "ReturnStackItem{"
+          + "codeSectionIndex="
+          + codeSectionIndex
+          + ", pc="
+          + pc
+          + ", stackHeight="
+          + stackHeight
+          + '}';
+    }
   }
 
   /**

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCallOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCallOperation.java
@@ -185,34 +185,38 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
             ? CodeV0.EMPTY_CODE
             : evm.getCode(contract.getCodeHash(), contract.getCode());
 
-    final MessageFrame childFrame =
-        MessageFrame.builder()
-            .type(MessageFrame.Type.MESSAGE_CALL)
-            .messageFrameStack(frame.getMessageFrameStack())
-            .worldUpdater(frame.getWorldUpdater().updater())
-            .initialGas(gasAvailableForChildCall(frame))
-            .address(address(frame))
-            .originator(frame.getOriginatorAddress())
-            .contract(to)
-            .gasPrice(frame.getGasPrice())
-            .inputData(inputData)
-            .sender(sender(frame))
-            .value(value(frame))
-            .apparentValue(apparentValue(frame))
-            .code(code)
-            .blockValues(frame.getBlockValues())
-            .depth(frame.getMessageStackDepth() + 1)
-            .isStatic(isStatic(frame))
-            .completer(child -> complete(frame, child))
-            .miningBeneficiary(frame.getMiningBeneficiary())
-            .blockHashLookup(frame.getBlockHashLookup())
-            .maxStackSize(frame.getMaxStackSize())
-            .build();
-    frame.incrementRemainingGas(cost);
-
-    frame.getMessageFrameStack().addFirst(childFrame);
-    frame.setState(MessageFrame.State.CODE_SUSPENDED);
-    return new OperationResult(cost, null, 0);
+    if (code.isValid()) {
+      final MessageFrame childFrame =
+          MessageFrame.builder()
+              .type(MessageFrame.Type.MESSAGE_CALL)
+              .messageFrameStack(frame.getMessageFrameStack())
+              .worldUpdater(frame.getWorldUpdater().updater())
+              .initialGas(gasAvailableForChildCall(frame))
+              .address(address(frame))
+              .originator(frame.getOriginatorAddress())
+              .contract(to)
+              .gasPrice(frame.getGasPrice())
+              .inputData(inputData)
+              .sender(sender(frame))
+              .value(value(frame))
+              .apparentValue(apparentValue(frame))
+              .code(code)
+              .blockValues(frame.getBlockValues())
+              .depth(frame.getMessageStackDepth() + 1)
+              .isStatic(isStatic(frame))
+              .completer(child -> complete(frame, child))
+              .miningBeneficiary(frame.getMiningBeneficiary())
+              .blockHashLookup(frame.getBlockHashLookup())
+              .maxStackSize(frame.getMaxStackSize())
+              .build();
+      frame.incrementRemainingGas(cost);
+
+      frame.getMessageFrameStack().addFirst(childFrame);
+      frame.setState(MessageFrame.State.CODE_SUSPENDED);
+      return new OperationResult(cost, null, 0);
+    } else {
+      return new OperationResult(cost, ExceptionalHaltReason.INVALID_CODE, 0);
+    }
   }
 
   protected abstract long cost(final MessageFrame frame);

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/RelativeJumpVectorOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/RelativeJumpVectorOperation.java
@@ -43,7 +43,7 @@ protected OperationResult executeFixedCostOperation(final MessageFrame frame, fi
             + 2 * vectorSize
             + ((offsetCase >= vectorSize)
                 ? 0
-                : readBigEndianI16(frame.getPC() + 1 + offsetCase * 2, code.toArrayUnsafe()))
+                : readBigEndianI16(frame.getPC() + 2 + offsetCase * 2, code.toArrayUnsafe()))
             + 1);
   }
 

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/RetFOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/RetFOperation.java
@@ -29,7 +29,11 @@ public RetFOperation(final GasCalculator gasCalculator) {
 
   @Override
   public OperationResult execute(final MessageFrame frame, final EVM evm) {
-    frame.returnFunction();
-    return retfSuccess;
+    var exception = frame.returnFunction();
+    if (exception == null) {
+      return retfSuccess;
+    } else {
+      return new OperationResult(retfSuccess.gasCost, exception);
+    }
   }
 }