From cb2a22a5f6715291ff71345774c52a4f38dfef7b Mon Sep 17 00:00:00 2001
From: Robin Salen <30937548+Nashtare@users.noreply.github.com>
Date: Mon, 4 Dec 2023 10:32:52 -0500
Subject: [PATCH] Update stack op cost (#1402)

* Update stack op cost

* Update from review
---
 evm/src/cpu/kernel/cost_estimator.rs           |  2 +-
 evm/src/cpu/kernel/stack/stack_manipulation.rs | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/evm/src/cpu/kernel/cost_estimator.rs b/evm/src/cpu/kernel/cost_estimator.rs
index ae837647..bb2b0a40 100644
--- a/evm/src/cpu/kernel/cost_estimator.rs
+++ b/evm/src/cpu/kernel/cost_estimator.rs
@@ -1,3 +1,4 @@
+use super::opcodes::get_opcode;
 use crate::cpu::kernel::assembler::BYTES_PER_OFFSET;
 use crate::cpu::kernel::ast::Item;
 use crate::cpu::kernel::ast::Item::*;
@@ -32,6 +33,5 @@ fn cost_estimate_standard_op(_op: &str) -> u32 {
 }
 
 fn cost_estimate_push(num_bytes: usize) -> u32 {
-    // TODO: Once PUSH is actually implemented, check if this needs to be revised.
     num_bytes as u32
 }
diff --git a/evm/src/cpu/kernel/stack/stack_manipulation.rs b/evm/src/cpu/kernel/stack/stack_manipulation.rs
index 47b02cf6..5214351c 100644
--- a/evm/src/cpu/kernel/stack/stack_manipulation.rs
+++ b/evm/src/cpu/kernel/stack/stack_manipulation.rs
@@ -286,15 +286,15 @@ impl StackOp {
                         panic!("Target should have been expanded already: {target:?}")
                     }
                 };
-                // This is just a rough estimate; we can update it after implementing PUSH.
-                (bytes, bytes)
+                // A PUSH takes one cycle, and 1 memory read per byte.
+                (1, bytes + 1)
             }
-            // A POP takes one cycle, and doesn't involve memory, it just decrements a pointer.
-            Pop => (1, 0),
+            // A POP takes one cycle, and most of the time a read to update the top of the stack.
+            Pop => (1, 1),
             // A DUP takes one cycle, and a read and a write.
             StackOp::Dup(_) => (1, 2),
-            // A SWAP takes one cycle with four memory ops, to read both values then write to them.
-            StackOp::Swap(_) => (1, 4),
+            // A SWAP takes one cycle with three memory ops, to read both values then write to them.
+            StackOp::Swap(_) => (1, 3),
         };
 
         let cpu_cost = cpu_rows * NUM_CPU_COLUMNS as u32;