feat: EIP-1153 Transient storage opcodes #546
Conversation
| pub fn tload<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &mut H) { | ||
| gas!(interpreter, gas::WARM_STORAGE_READ_COST); | ||
|
|
||
| pop!(interpreter, index); |
There was a problem hiding this comment.
you can use pop_top! here and you set tload value to it.
| use crate::primitives::{B160, U256}; | ||
| use std::collections::HashMap; | ||
|
|
||
| #[derive(Debug, Clone, Eq, PartialEq)] |
| } | ||
|
|
||
| pub fn set(&mut self, address: B160, key: U256, value: U256) { | ||
| match self.data.get_mut(&address) { |
There was a problem hiding this comment.
You can use simpler form with entry. something like self.data.entry(&dadress).or_default().insert(key,value)
| } | ||
|
|
||
| pub fn get(&self, address: B160, key: U256) -> U256 { | ||
| match self.data.get(&address) { |
There was a problem hiding this comment.
Better way i to use and_then. Something like self.data.get(&address).and_then(|s| s.get(&key)).cloned().unwrap_or_default()
crates/revm/src/evm.rs
Outdated
| @@ -196,7 +196,6 @@ pub fn evm_inner<'a, DB: Database, const INSPECT: bool>( | |||
| } | |||
| SpecId::MERGE => create_evm!(MergeSpec, db, env, insp), | |||
| SpecId::SHANGHAI => create_evm!(ShanghaiSpec, db, env, insp), | |||
| SpecId::CANCUN => create_evm!(LatestSpec, db, env, insp), | |||
| SpecId::LATEST => create_evm!(LatestSpec, db, env, insp), | |||
| SpecId::CANCUN | SpecId::LATEST => create_evm!(LatestSpec, db, env, insp), | |||
There was a problem hiding this comment.
LATEST should be LatestSpec, Cancun should have its own spec
crates/revm/src/journaled_state.rs
Outdated
| @@ -657,6 +690,23 @@ impl JournaledState { | |||
| Ok((slot.original_value, present, new, is_cold)) | |||
| } | |||
|
|
|||
| pub fn tstore(&mut self, address: B160, key: U256, new: U256) { | |||
| let present = self.tload(address, key); | |||
There was a problem hiding this comment.
We are not calling tstore anywhere
crates/revm/src/journaled_state.rs
Outdated
| key, | ||
| had_value, | ||
| } => { | ||
| transient_storage.set(address, key, had_value.unwrap()); |
There was a problem hiding this comment.
If had_value is None we should remove the entry from transient storage.
| @@ -2,6 +2,7 @@ pub mod analysis; | |||
| mod contract; | |||
| pub(crate) mod memory; | |||
| mod stack; | |||
| pub mod transient; | |||
There was a problem hiding this comment.
Would be good to have full name transient_storage
|
@tynes can you double check this, made some changes and imo is ready to be merged. |
|
Thank you for picking this up @rakita. For test coverage, do you think using the Ethereum state tests are sufficient? The implementation looks good to me, looks much more idiomatic than before |
using eth tests should be sufficient to cover edge cases |
Implement EIP-1153 for the Cancun hardfork
The changes to the specs in this PR I'd like some feedback on, happy to revert things that aren't directly related to the EIP implementation
Need to update the opcode numbers before this PR is ready, they are still on the older values
https://eips.ethereum.org/EIPS/eip-1153
Closes #154