From 472aeee6d7a0644fa6516f2cfa71c1b02ae9598b Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Wed, 24 Sep 2025 10:02:01 +0200 Subject: [PATCH 1/7] bugfix revm set_storage gas cost --- substrate/frame/revive/src/vm/evm/instructions/host.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/substrate/frame/revive/src/vm/evm/instructions/host.rs b/substrate/frame/revive/src/vm/evm/instructions/host.rs index dbb65c89de64a..0b4407ff21c4e 100644 --- a/substrate/frame/revive/src/vm/evm/instructions/host.rs +++ b/substrate/frame/revive/src/vm/evm/instructions/host.rs @@ -169,9 +169,10 @@ fn store_helper<'ext, E: Ext>( /// /// Stores a word to storage. pub fn sstore<'ext, E: Ext>(context: Context<'_, 'ext, E>) { + let old_bytes = context.interpreter.extend.max_value_size(); store_helper( context, - RuntimeCosts::SetStorage { new_bytes: 32, old_bytes: 0 }, + RuntimeCosts::SetStorage { new_bytes: 32, old_bytes }, |ext, key, value, take_old| ext.set_storage(key, value, take_old), |new_bytes, old_bytes| RuntimeCosts::SetStorage { new_bytes, old_bytes }, ); @@ -180,9 +181,10 @@ pub fn sstore<'ext, E: Ext>(context: Context<'_, 'ext, E>) { /// EIP-1153: Transient storage opcodes /// Store value to transient storage pub fn tstore<'ext, E: Ext>(context: Context<'_, 'ext, E>) { + let old_bytes = context.interpreter.extend.max_value_size(); store_helper( context, - RuntimeCosts::SetTransientStorage { new_bytes: 32, old_bytes: 0 }, + RuntimeCosts::SetTransientStorage { new_bytes: 32, old_bytes }, |ext, key, value, take_old| ext.set_transient_storage(key, value, take_old), |new_bytes, old_bytes| RuntimeCosts::SetTransientStorage { new_bytes, old_bytes }, ); From 7476ae8ba8cc3277d961e4bcf1f802464d3f1d03 Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Wed, 24 Sep 2025 11:05:54 +0200 Subject: [PATCH 2/7] added sload_error_reading_non_32_byte_value in host.rs --- substrate/frame/revive/src/tests/sol/host.rs | 35 +++++++++++++++++++ .../revive/src/vm/evm/instructions/host.rs | 1 + 2 files changed, 36 insertions(+) diff --git a/substrate/frame/revive/src/tests/sol/host.rs b/substrate/frame/revive/src/tests/sol/host.rs index 7ee487dda90df..849ca89046ce6 100644 --- a/substrate/frame/revive/src/tests/sol/host.rs +++ b/substrate/frame/revive/src/tests/sol/host.rs @@ -364,6 +364,41 @@ fn sload_works() { } } +#[test] +fn sload_error_reading_non_32_byte_value() { + let (code, _) = compile_module_with_type("Host", FixtureType::Solc).unwrap(); + + let index = U256::from(13); + let expected_value = U256::from(17); + + ExtBuilder::default().build().execute_with(|| { + ::Currency::set_balance(&ALICE, 100_000_000_000); + + let Contract { addr, .. } = + builder::bare_instantiate(Code::Upload(code)).build_and_unwrap_contract(); + + { + let contract_info = test_utils::get_contract(&addr); + let key = Key::Fix(index.to_be_bytes()); + contract_info + .write(&key, Some(expected_value.to_be_bytes::<32>()[..31].to_vec()), None, false) + .unwrap(); + } + + { + let result = builder::bare_call(addr) + .data(Host::HostCalls::sloadOp(Host::sloadOpCall { slot: index }).abi_encode()) + .build(); + assert!(result.result.is_err(), "test should error"); + let err = result.result.unwrap_err(); + let sp_runtime::DispatchError::Module(module_err) = &err else { + panic!("expected Module error (ContractTrapped), got {:?}", err) + }; + assert_eq!(module_err.message, Some("ContractTrapped")); + } + }); +} + #[test] fn sstore_works() { for fixture_type in [FixtureType::Solc, FixtureType::Resolc] { diff --git a/substrate/frame/revive/src/vm/evm/instructions/host.rs b/substrate/frame/revive/src/vm/evm/instructions/host.rs index 0b4407ff21c4e..27e8df80ce938 100644 --- a/substrate/frame/revive/src/vm/evm/instructions/host.rs +++ b/substrate/frame/revive/src/vm/evm/instructions/host.rs @@ -117,6 +117,7 @@ pub fn sload<'ext, E: Ext>(context: Context<'_, 'ext, E>) { *index = if let Some(storage_value) = value { // sload always reads a word let Ok::<[u8; 32], _>(bytes) = storage_value.try_into() else { + log::error!(target: crate::LOG_TARGET, "sload read invalid storage value length. Expected 32."); context.interpreter.halt(InstructionResult::FatalExternalError); return }; From b2a1d81a2e13fa76aa6927e6467622bad2b1bf4e Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Wed, 24 Sep 2025 11:25:27 +0200 Subject: [PATCH 3/7] added assert for 33 bytes in sload_error_reading_non_32_byte_value --- substrate/frame/revive/src/tests/sol/host.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/substrate/frame/revive/src/tests/sol/host.rs b/substrate/frame/revive/src/tests/sol/host.rs index 849ca89046ce6..251d7f2c2aae2 100644 --- a/substrate/frame/revive/src/tests/sol/host.rs +++ b/substrate/frame/revive/src/tests/sol/host.rs @@ -383,9 +383,25 @@ fn sload_error_reading_non_32_byte_value() { contract_info .write(&key, Some(expected_value.to_be_bytes::<32>()[..31].to_vec()), None, false) .unwrap(); + + let result = builder::bare_call(addr) + .data(Host::HostCalls::sloadOp(Host::sloadOpCall { slot: index }).abi_encode()) + .build(); + assert!(result.result.is_err(), "test should error"); + let err = result.result.unwrap_err(); + let sp_runtime::DispatchError::Module(module_err) = &err else { + panic!("expected Module error (ContractTrapped), got {:?}", err) + }; + assert_eq!(module_err.message, Some("ContractTrapped")); } { + let contract_info = test_utils::get_contract(&addr); + let key = Key::Fix(index.to_be_bytes()); + let mut bytes = expected_value.to_be_bytes::<32>().to_vec(); + bytes.push(0u8); + contract_info.write(&key, Some(bytes), None, false).unwrap(); + let result = builder::bare_call(addr) .data(Host::HostCalls::sloadOp(Host::sloadOpCall { slot: index }).abi_encode()) .build(); From eebff13b7efb97b12c8e54831b7fb377a58a0cc6 Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Wed, 24 Sep 2025 11:26:39 +0200 Subject: [PATCH 4/7] comments --- substrate/frame/revive/src/tests/sol/host.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/substrate/frame/revive/src/tests/sol/host.rs b/substrate/frame/revive/src/tests/sol/host.rs index 251d7f2c2aae2..d18f1cb18f5e4 100644 --- a/substrate/frame/revive/src/tests/sol/host.rs +++ b/substrate/frame/revive/src/tests/sol/host.rs @@ -378,6 +378,7 @@ fn sload_error_reading_non_32_byte_value() { builder::bare_instantiate(Code::Upload(code)).build_and_unwrap_contract(); { + // Test that reading storage value of 31 bytes results in contract trapped let contract_info = test_utils::get_contract(&addr); let key = Key::Fix(index.to_be_bytes()); contract_info @@ -396,6 +397,7 @@ fn sload_error_reading_non_32_byte_value() { } { + // Test that reading storage value of 33 bytes results in contract trapped let contract_info = test_utils::get_contract(&addr); let key = Key::Fix(index.to_be_bytes()); let mut bytes = expected_value.to_be_bytes::<32>().to_vec(); From 2385c2dcee8cff96202ed674f0792b4dd14145b7 Mon Sep 17 00:00:00 2001 From: Robert van Eerdewijk Date: Wed, 24 Sep 2025 11:27:22 +0200 Subject: [PATCH 5/7] log in tload --- substrate/frame/revive/src/vm/evm/instructions/host.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/revive/src/vm/evm/instructions/host.rs b/substrate/frame/revive/src/vm/evm/instructions/host.rs index 27e8df80ce938..a6c0c2b35bb38 100644 --- a/substrate/frame/revive/src/vm/evm/instructions/host.rs +++ b/substrate/frame/revive/src/vm/evm/instructions/host.rs @@ -202,6 +202,7 @@ pub fn tload<'ext, E: Ext>(context: Context<'_, 'ext, E>) { *index = if let Some(storage_value) = bytes { if storage_value.len() != 32 { // tload always reads a word + log::error!(target: crate::LOG_TARGET, "tload read invalid storage value length. Expected 32."); context.interpreter.halt(InstructionResult::FatalExternalError); return; } From a1396fcea2949da11505458b83d782467b736df2 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 24 Sep 2025 13:33:55 +0000 Subject: [PATCH 6/7] Update from github-actions[bot] running command 'prdoc --audience runtime_dev --bump patch' --- prdoc/pr_9823.prdoc | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 prdoc/pr_9823.prdoc diff --git a/prdoc/pr_9823.prdoc b/prdoc/pr_9823.prdoc new file mode 100644 index 0000000000000..fe07007a015ee --- /dev/null +++ b/prdoc/pr_9823.prdoc @@ -0,0 +1,8 @@ +title: bugfix revm set_storage gas cost +doc: +- audience: Runtime Dev + description: "Fixes bug in revm gasmetering where the initial charge was less than\ + \ the adjusted charge.\r\n" +crates: +- name: pallet-revive + bump: patch From ac1f6652d1beb626fd60515bc60b0eb933342a09 Mon Sep 17 00:00:00 2001 From: 0xRVE Date: Thu, 25 Sep 2025 10:23:59 +0200 Subject: [PATCH 7/7] Update substrate/frame/revive/src/vm/evm/instructions/host.rs Co-authored-by: PG Herveou --- substrate/frame/revive/src/vm/evm/instructions/host.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/revive/src/vm/evm/instructions/host.rs b/substrate/frame/revive/src/vm/evm/instructions/host.rs index a6c0c2b35bb38..6e2a3abb8bd16 100644 --- a/substrate/frame/revive/src/vm/evm/instructions/host.rs +++ b/substrate/frame/revive/src/vm/evm/instructions/host.rs @@ -117,7 +117,7 @@ pub fn sload<'ext, E: Ext>(context: Context<'_, 'ext, E>) { *index = if let Some(storage_value) = value { // sload always reads a word let Ok::<[u8; 32], _>(bytes) = storage_value.try_into() else { - log::error!(target: crate::LOG_TARGET, "sload read invalid storage value length. Expected 32."); + log::debug!(target: crate::LOG_TARGET, "sload read invalid storage value length. Expected 32."); context.interpreter.halt(InstructionResult::FatalExternalError); return };