From eff2403dad3a9a9649f1a468eec2f60bb7802d8b Mon Sep 17 00:00:00 2001 From: Gregory Edison Date: Thu, 18 Sep 2025 11:59:33 +0200 Subject: [PATCH] fix: gas used issue with instructions --- Cargo.lock | 123 ++++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 3 ++ src/instructions.rs | 73 +++++++++++++++++++++----- 3 files changed, 185 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e736f52..c3bc252 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -25,6 +25,15 @@ dependencies = [ "zerocopy 0.7.35", ] +[[package]] +name = "aho-corasick" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e60d3430d3a69478ad0993f19238d2df97c507009a52b3c10addcd7f6bcb916" +dependencies = [ + "memchr", +] + [[package]] name = "allocator-api2" version = "0.2.21" @@ -995,6 +1004,49 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" +[[package]] +name = "futures-core" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f29059c0c2090612e8d742178b0580d2dc940c837851ad723096f87af6663e" + +[[package]] +name = "futures-macro" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.100", +] + +[[package]] +name = "futures-task" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" + +[[package]] +name = "futures-timer" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f288b0a4f20f9a56b5d1da57e2227c661b7b16168e2f72365f57b63326e29b24" + +[[package]] +name = "futures-util" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" +dependencies = [ + "futures-core", + "futures-macro", + "futures-task", + "pin-project-lite", + "pin-utils", + "slab", +] + [[package]] name = "gcd" version = "2.3.0" @@ -1645,6 +1697,12 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "pkcs8" version = "0.10.2" @@ -1820,12 +1878,41 @@ dependencies = [ "rand_core 0.6.4", ] +[[package]] +name = "regex" +version = "1.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23d7fd106d8c02486a8d64e778353d1cffe08ce79ac2e82f540c86d0facf6912" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b9458fa0bfeeac22b5ca447c63aaf45f28439a709ccd244698632f9aa6394d6" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + [[package]] name = "regex-syntax" version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" +[[package]] +name = "relative-path" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" + [[package]] name = "revm" version = "29.0.0" @@ -2004,6 +2091,7 @@ dependencies = [ "revm", "revm-inspector", "revm-primitives", + "rstest", "serde", ] @@ -2047,6 +2135,35 @@ dependencies = [ "rustc-hex", ] +[[package]] +name = "rstest" +version = "0.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5a3193c063baaa2a95a33f03035c8a72b83d97a54916055ba22d35ed3839d49" +dependencies = [ + "futures-timer", + "futures-util", + "rstest_macros", +] + +[[package]] +name = "rstest_macros" +version = "0.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c845311f0ff7951c5506121a9ad75aec44d083c31583b2ea5a30bcb0b0abba0" +dependencies = [ + "cfg-if", + "glob", + "proc-macro-crate", + "proc-macro2", + "quote", + "regex", + "relative-path", + "rustc_version 0.4.1", + "syn 2.0.100", + "unicode-ident", +] + [[package]] name = "rug" version = "1.27.0" @@ -2319,6 +2436,12 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56199f7ddabf13fe5074ce809e7d3f42b42ae711800501b5b16ea82ad029c39d" +[[package]] +name = "slab" +version = "0.4.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a2ae44ef20feb57a68b23d846850f861394c2e02dc425a50098ae8c90267589" + [[package]] name = "sp1-lib" version = "5.0.5" diff --git a/Cargo.toml b/Cargo.toml index 9469c7c..f8e27a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,3 +31,6 @@ c-kzg = ["revm/c-kzg"] # `kzg-rs` is not audited but useful for `no_std` environment, use it with causing and default to `c-kzg` if possible. kzg-rs = ["revm/kzg-rs"] blst = ["revm/blst"] + +[dev-dependencies] +rstest = "0.26.1" diff --git a/src/instructions.rs b/src/instructions.rs index 4d596a9..fd2c606 100644 --- a/src/instructions.rs +++ b/src/instructions.rs @@ -93,11 +93,11 @@ pub fn make_scroll_instruction_table fn blockhash(context: InstructionContext<'_, H, WIRE>) { let host = context.host; let interpreter = context.interpreter; - gas!(interpreter, gas::BLOCKHASH); popn_top!([], number, interpreter); let requested_number = *number; @@ -144,6 +144,9 @@ fn blockhash(context: InstructionCon }; } +/// Implements the SELFDESTRUCT instruction. +/// +/// Halt execution and register account for later deletion. fn selfdestruct(context: InstructionContext<'_, H, WIRE>) { context.interpreter.halt(InstructionResult::NotActivated); } @@ -151,6 +154,8 @@ fn selfdestruct(context: InstructionContext<'_, // CURIE OPCODE IMPLEMENTATIONS // ================================================================================================ +/// EIP-3198: BASEFEE opcode +/// Gas is accounted in the interpreter fn basefee(context: InstructionContext<'_, H, WIRE>) { let host = context.host; let interpreter = context.interpreter; @@ -159,10 +164,16 @@ fn basefee(context: InstructionConte return; } - gas!(interpreter, gas::BASE); push!(interpreter, U256::from(host.basefee())); } +/// Store transient storage tied to the account. +/// +/// If values is different add entry to the journal +/// so that old state can be reverted if that action is needed. +/// +/// EIP-1153: Transient storage opcodes +/// Gas is accounted in the interpreter fn tstore(context: InstructionContext<'_, H, WIRE>) { let host = context.host; let interpreter = context.interpreter; @@ -172,13 +183,16 @@ fn tstore(context: InstructionContex } require_non_staticcall!(interpreter); - gas!(interpreter, gas::WARM_STORAGE_READ_COST); popn!([index, value], interpreter); host.tstore(interpreter.input.target_address(), index, value); } +/// Read transient storage tied to the account. +/// +/// EIP-1153: Transient storage opcodes +/// Gas is accounted in the interpreter fn tload(context: InstructionContext<'_, H, WIRE>) { let host = context.host; let interpreter = context.interpreter; @@ -187,13 +201,14 @@ fn tload(context: InstructionContext return; } - gas!(interpreter, gas::WARM_STORAGE_READ_COST); - popn_top!([], index, interpreter); *index = host.tload(interpreter.input.target_address(), *index); } +/// Implements the MCOPY instruction. +/// +/// EIP-5656: Memory copying instruction that copies memory from one location to another. fn mcopy(context: InstructionContext<'_, H, WIRE>) { let host = context.host; let interpreter = context.interpreter; @@ -223,10 +238,10 @@ fn mcopy(context: InstructionContext /// Implements the DIFFICULTY instruction. /// /// Pushes the block difficulty(default to 0) onto the stack. +/// Gas is accounted in the interpreter pub fn difficulty( context: InstructionContext<'_, H, WIRE>, ) { - gas!(context.interpreter, gas::BASE); push!(context.interpreter, DIFFICULTY); } @@ -245,6 +260,13 @@ fn compute_block_hash(chain_id: u64, block_number: u64) -> U256 { #[cfg(test)] mod tests { + use super::{compute_block_hash, make_scroll_instruction_table}; + use crate::{ + builder::{DefaultScrollContext, ScrollContext}, + instructions::HISTORY_STORAGE_ADDRESS, + ScrollSpecId::*, + }; + use revm::{ bytecode::{opcode::*, Bytecode}, database::{EmptyDB, InMemoryDB}, @@ -252,14 +274,7 @@ mod tests { primitives::{Bytes, U256}, DatabaseRef, }; - - use crate::{ - builder::{DefaultScrollContext, ScrollContext}, - instructions::HISTORY_STORAGE_ADDRESS, - ScrollSpecId::*, - }; - - use super::{compute_block_hash, make_scroll_instruction_table}; + use rstest::rstest; #[test] fn test_blockhash_before_feynman() { @@ -316,4 +331,34 @@ mod tests { let actual = interpreter.stack.pop().expect("stack is not empty"); assert_eq!(actual, expected); } + + #[rstest] + #[case(BLOCKHASH, 20)] + #[case(BASEFEE, 2)] + #[case(TSTORE, 100)] + #[case(TLOAD, 100)] + #[case(MCOPY, 9)] + #[case(SELFDESTRUCT, 0)] + #[case(DIFFICULTY, 2)] + fn test_gas_used(#[case] opcode: u8, #[case] expected_gas_used: u64) { + let (chain_id, current_block, spec) = (123, U256::from(1024), FEYNMAN); + + let db = EmptyDB::new(); + let mut context = ScrollContext::scroll().with_db(InMemoryDB::new(db)); + context.modify_block(|block| block.number = current_block); + context.modify_cfg(|cfg| cfg.chain_id = chain_id); + context.modify_cfg(|cfg| cfg.spec = spec); + + let instructions = make_scroll_instruction_table(); + + let bytecode = Bytecode::new_legacy(Bytes::from([opcode, STOP].to_vec())); + let mut interpreter = Interpreter::default().with_bytecode(bytecode); + let _ = interpreter.stack.push(U256::from(1)); + let _ = interpreter.stack.push(U256::from(0)); + let _ = interpreter.stack.push(U256::from(0)); + interpreter.run_plain(&instructions, &mut context); + + let actual_gas_used = interpreter.gas.used(); + assert_eq!(actual_gas_used, expected_gas_used); + } }