Fix the stack_height_required range to accommodate DUPN/SWAPN

ethereum · Jan 10, 2024 · bcedb55 · bcedb55
1 parent 896a029
commit bcedb55
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 21 deletions.
diff --git a/lib/evmone/advanced_analysis.hpp b/lib/evmone/advanced_analysis.hpp
@@ -149,7 +149,7 @@ struct OpTableEntry
 {
     instruction_exec_fn fn;
     int16_t gas_cost;
-    int8_t stack_req;
+    uint8_t stack_req;
     int8_t stack_change;
 };
 

diff --git a/lib/evmone/eof.cpp b/lib/evmone/eof.cpp
@@ -364,7 +364,7 @@ std::variant<EOFValidationError, int32_t> validate_max_stack_height(
 
         const auto opcode = static_cast<Opcode>(code[i]);
 
-        auto stack_height_required = instr::traits[opcode].stack_height_required;
+        int stack_height_required = instr::traits[opcode].stack_height_required;
         auto stack_height_change = instr::traits[opcode].stack_height_change;
 
         auto stack_height = stack_heights[i];
@@ -374,7 +374,7 @@ std::variant<EOFValidationError, int32_t> validate_max_stack_height(
         {
             const auto fid = read_uint16_be(&code[i + 1]);
 
-            stack_height_required = static_cast<int8_t>(code_types[fid].inputs);
+            stack_height_required = code_types[fid].inputs;
 
             if (stack_height + code_types[fid].max_stack_height - stack_height_required >
                 STACK_SIZE_LIMIT)
@@ -395,34 +395,33 @@ std::variant<EOFValidationError, int32_t> validate_max_stack_height(
 
             if (code_types[fid].outputs == NON_RETURNING_FUNCITON)
             {
-                stack_height_required = static_cast<int8_t>(code_types[fid].inputs);
+                stack_height_required = code_types[fid].inputs;
             }
             else
             {
                 if (code_types[func_index].outputs < code_types[fid].outputs)
                     return EOFValidationError::jumpf_destination_incompatible_outputs;
 
-                stack_height_required =
-                    static_cast<int8_t>(code_types[func_index].outputs + code_types[fid].inputs -
-                                        code_types[fid].outputs);
+                stack_height_required = code_types[func_index].outputs + code_types[fid].inputs -
+                                        code_types[fid].outputs;
                 if (stack_heights[i] > stack_height_required)
                     return EOFValidationError::stack_higher_than_outputs_required;
             }
         }
         else if (opcode == OP_RETF)
         {
-            stack_height_required = static_cast<int8_t>(code_types[func_index].outputs);
+            stack_height_required = code_types[func_index].outputs;
             if (stack_height > code_types[func_index].outputs)
                 return EOFValidationError::stack_higher_than_outputs_required;
         }
         else if (opcode == OP_DUPN)
         {
-            const auto n = static_cast<int8_t>(code[i + 1]);
+            const auto n = static_cast<uint8_t>(code[i + 1]);
             stack_height_required = n + 1;
         }
         else if (opcode == OP_SWAPN)
         {
-            const auto n = static_cast<int8_t>(code[i + 1]);
+            const auto n = static_cast<uint8_t>(code[i + 1]);
             stack_height_required = n + 2;
         }
 

diff --git a/lib/evmone/instructions_traits.hpp b/lib/evmone/instructions_traits.hpp
@@ -204,7 +204,7 @@ struct Traits
     bool is_terminating = false;
 
     /// The number of stack items the instruction accesses during execution.
-    int8_t stack_height_required = 0;
+    uint8_t stack_height_required = 0;
 
     /// The stack height change caused by the instruction execution. Can be negative.
     int8_t stack_height_change = 0;

diff --git a/test/unittests/eof_validation_test.cpp b/test/unittests/eof_validation_test.cpp
@@ -962,20 +962,36 @@ TEST(eof_validation, retf_stack_validation)
 
 TEST(eof_validation, dupn_stack_validation)
 {
-    auto pushes = bytecode{};
-    for (uint64_t i = 1; i <= 20; ++i)
-        pushes += push(i);
-    auto code = eof_bytecode(pushes + OP_DUPN + "14" + OP_STOP, 21);
-    EXPECT_EQ(validate_eof(code), EOFValidationError::stack_underflow);
+    const auto pushes = 20 * push(1);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_DUPN + "00" + OP_STOP, 21)),
+        EOFValidationError::success);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_DUPN + "13" + OP_STOP, 21)),
+        EOFValidationError::success);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_DUPN + "14" + OP_STOP, 21)),
+        EOFValidationError::stack_underflow);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_DUPN + "d0" + OP_STOP, 21)),
+        EOFValidationError::stack_underflow);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_DUPN + "fe" + OP_STOP, 21)),
+        EOFValidationError::stack_underflow);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_DUPN + "ff" + OP_STOP, 21)),
+        EOFValidationError::stack_underflow);
 }
 
 TEST(eof_validation, swapn_stack_validation)
 {
-    auto pushes = bytecode{};
-    for (uint64_t i = 1; i <= 20; ++i)
-        pushes += push(i);
-    auto code = eof_bytecode(pushes + OP_SWAPN + "13" + OP_STOP, 20);
-    EXPECT_EQ(validate_eof(code), EOFValidationError::stack_underflow);
+    const auto pushes = 20 * push(1);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_SWAPN + "00" + OP_STOP, 20)),
+        EOFValidationError::success);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_SWAPN + "12" + OP_STOP, 20)),
+        EOFValidationError::success);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_SWAPN + "13" + OP_STOP, 20)),
+        EOFValidationError::stack_underflow);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_SWAPN + "d0" + OP_STOP, 20)),
+        EOFValidationError::stack_underflow);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_SWAPN + "fe" + OP_STOP, 20)),
+        EOFValidationError::stack_underflow);
+    EXPECT_EQ(validate_eof(eof_bytecode(pushes + OP_SWAPN + "ff" + OP_STOP, 20)),
+        EOFValidationError::stack_underflow);
 }
 
 TEST(eof_validation, non_returning_status)