Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Implementation of EIP-1153: Transient Storage using Disk Persistence and Lifecycle Management #1588

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions actors/evm/src/interpreter/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ pub mod opcodes {
0x59: MSIZE,
0x5a: GAS,
0x5b: JUMPDEST,
0x5c: TLOAD,
0x5d: TSTORE,
0x5e: MCOPY,
0x5F: PUSH0,
0x60: PUSH1,
Expand Down Expand Up @@ -400,6 +402,8 @@ mod tests {
MSTORE8,
SLOAD,
SSTORE,
TLOAD,
TSTORE,
LOG0,
LOG1,
LOG2,
Expand Down
2 changes: 2 additions & 0 deletions actors/evm/src/interpreter/instructions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,8 @@ def_stdproc! { MSTORE(a, b) => memory::mstore }
def_stdproc! { MSTORE8(a, b) => memory::mstore8 }
def_stdfun! { SLOAD(a) => storage::sload }
def_stdproc! { SSTORE(a, b) => storage::sstore }
def_stdfun! { TLOAD(a) => storage::tload }
def_stdproc! { TSTORE(a, b) => storage::tstore }
def_stdfun! { MSIZE() => memory::msize }
def_stdfun! { GAS() => context::gas }
def_stdlog! { LOG0(0, ()) }
Expand Down
73 changes: 73 additions & 0 deletions actors/evm/src/interpreter/instructions/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,30 @@ pub fn sstore(
system.set_storage(key, value)
}

#[inline]
pub fn tload(
_state: &mut ExecutionState,
system: &mut System<impl Runtime>,
location: U256,
) -> Result<U256, ActorError> {
// get from storage and place on stack
system.get_transient_storage(location)
}

#[inline]
pub fn tstore(
_state: &mut ExecutionState,
system: &mut System<impl Runtime>,
key: U256,
value: U256,
) -> Result<(), ActorError> {
if system.readonly {
return Err(ActorError::read_only("store called while read-only".into()));
}

system.set_transient_storage(key, value)
}

#[cfg(test)]
mod tests {
use fil_actors_evm_shared::uints::U256;
Expand Down Expand Up @@ -82,4 +106,53 @@ mod tests {
assert_eq!(m.system.get_storage(U256::from(0)).unwrap(), U256::from(0x42));
};
}

// TODO test transient storage lifecycle

#[test]
fn test_tload() {
// happy path
evm_unit_test! {
(m) {
TLOAD;
}
m.system.set_transient_storage(U256::from(0), U256::from(0x42)).unwrap();
m.state.stack.push(U256::from(0)).unwrap();
let result = m.step();
assert!(result.is_ok(), "execution step failed");
assert_eq!(m.state.stack.len(), 1);
assert_eq!(m.state.stack.pop().unwrap(), U256::from(0x42));
};
}

#[test]
fn test_tload_oob() {
// oob access -- it is a zero
evm_unit_test! {
(m) {
TLOAD;
}
m.state.stack.push(U256::from(1234)).unwrap();
let result = m.step();
assert!(result.is_ok(), "execution step failed");
assert_eq!(m.state.stack.len(), 1);
assert_eq!(m.state.stack.pop().unwrap(), U256::from(0));
};
}

#[test]
fn test_tstore() {
evm_unit_test! {
(m) {
TSTORE;
}

m.state.stack.push(U256::from(0x42)).unwrap();
m.state.stack.push(U256::from(0)).unwrap();
let result = m.step();
assert!(result.is_ok(), "execution step failed");
assert_eq!(m.state.stack.len(), 0);
assert_eq!(m.system.get_transient_storage(U256::from(0)).unwrap(), U256::from(0x42));
};
}
}
61 changes: 59 additions & 2 deletions actors/evm/src/interpreter/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use fvm_shared::error::{ErrorNumber, ExitCode};
use fvm_shared::sys::SendFlags;
use fvm_shared::{MethodNum, Response, IPLD_RAW, METHOD_SEND};

use crate::state::{State, Tombstone};
use crate::state::{State, Tombstone, TransientDataLifespan};
use crate::BytecodeHash;

use cid::Cid;
Expand Down Expand Up @@ -91,6 +91,11 @@ pub struct System<'r, RT: Runtime> {
bytecode: Option<EvmBytecode>,
/// The contract's EVM storage slots.
slots: StateKamt<RT::Blockstore>,

/// The contract's EVM transient storage slots.
transient_slots: StateKamt<RT::Blockstore>,
pub(crate) transient_data_lifespan: Option<TransientDataLifespan>,
snissn marked this conversation as resolved.
Show resolved Hide resolved

/// The contracts "nonce" (incremented when creating new actors).
pub(crate) nonce: u64,
/// The last saved state root. None if the current state hasn't been saved yet.
Expand All @@ -111,9 +116,12 @@ impl<'r, RT: Runtime> System<'r, RT> {
RT::Blockstore: Clone,
{
let store = rt.store().clone();
let transient_store = rt.store().clone();
Self {
rt,
slots: StateKamt::new_with_config(store, KAMT_CONFIG.clone()),
transient_slots: StateKamt::new_with_config(transient_store, KAMT_CONFIG.clone()),
transient_data_lifespan: None,
nonce: 1,
saved_state_root: None,
bytecode: None,
Expand Down Expand Up @@ -164,6 +172,7 @@ impl<'r, RT: Runtime> System<'r, RT> {
RT::Blockstore: Clone,
{
let store = rt.store().clone();
let transient_store = rt.store().clone();
let state_root = rt.get_state_root()?;
let state: State = store
.get_cbor(&state_root)
Expand All @@ -182,6 +191,13 @@ impl<'r, RT: Runtime> System<'r, RT> {
rt,
slots: StateKamt::load_with_config(&state.contract_state, store, KAMT_CONFIG.clone())
.context_code(ExitCode::USR_ILLEGAL_STATE, "state not in blockstore")?,
transient_slots: StateKamt::load_with_config(
&state.transient_state,
transient_store,
KAMT_CONFIG.clone(),
)
.context_code(ExitCode::USR_ILLEGAL_STATE, "state not in blockstore")?,
Stebalien marked this conversation as resolved.
Show resolved Hide resolved
transient_data_lifespan: state.transient_data_lifespan,
nonce: state.nonce,
saved_state_root: Some(state_root),
bytecode: Some(EvmBytecode::new(state.bytecode, state.bytecode_hash)),
Expand Down Expand Up @@ -255,7 +271,7 @@ impl<'r, RT: Runtime> System<'r, RT> {
Ok(result.map_err(|e| e.0))
}

/// Flush the actor state (bytecode, nonce, and slots).
/// Flush the actor state (bytecode, nonce, transient data and slots).
pub fn flush(&mut self) -> Result<(), ActorError> {
if self.saved_state_root.is_some() {
return Ok(());
Expand All @@ -281,6 +297,11 @@ impl<'r, RT: Runtime> System<'r, RT> {
ExitCode::USR_ILLEGAL_STATE,
"failed to flush contract state",
)?,
transient_state: self.transient_slots.flush().context_code(
ExitCode::USR_ILLEGAL_STATE,
"failed to flush contract state",
)?,
transient_data_lifespan: self.transient_data_lifespan,
nonce: self.nonce,
tombstone: self.tombstone,
},
Expand Down Expand Up @@ -314,6 +335,10 @@ impl<'r, RT: Runtime> System<'r, RT> {
self.slots
.set_root(&state.contract_state)
.context_code(ExitCode::USR_ILLEGAL_STATE, "state not in blockstore")?;
self.transient_slots
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could load this lazily on first-use, or even just if non-empty.

.set_root(&state.transient_state)
.context_code(ExitCode::USR_ILLEGAL_STATE, "transient_state not in blockstore")?;
self.transient_data_lifespan = state.transient_data_lifespan;
self.nonce = state.nonce;
self.saved_state_root = Some(root);
self.bytecode = Some(EvmBytecode::new(state.bytecode, state.bytecode_hash));
Expand Down Expand Up @@ -384,6 +409,38 @@ impl<'r, RT: Runtime> System<'r, RT> {
};
Ok(())
}
///
/// Get value of a storage key.
pub fn get_transient_storage(&mut self, key: U256) -> Result<U256, ActorError> {
//TODO check tombstone for liveliness of data
Ok(self
.transient_slots
.get(&key)
.context_code(ExitCode::USR_ILLEGAL_STATE, "failed to clear storage slot")?
.cloned()
.unwrap_or_default())
}

/// Set value of a storage key.
pub fn set_transient_storage(&mut self, key: U256, value: U256) -> Result<(), ActorError> {
//TODO check tombstone for liveliness of data
let changed = if value.is_zero() {
self.transient_slots
.delete(&key)
.map(|v| v.is_some())
.context_code(ExitCode::USR_ILLEGAL_STATE, "failed to clear storage slot")?
} else {
self.transient_slots
.set(key, value)
.map(|v| v != Some(value))
.context_code(ExitCode::USR_ILLEGAL_STATE, "failed to update storage slot")?
};

if changed {
self.saved_state_root = None; // dirty.
};
Ok(())
}

/// Resolve the address to the ethereum equivalent, if possible.
///
Expand Down
14 changes: 14 additions & 0 deletions actors/evm/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ pub struct Tombstone {
pub nonce: u64,
}

/// A structure representing the transient data lifespan.
#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize_tuple, Deserialize_tuple)]
pub struct TransientDataLifespan {
/// The origin actor ID associated with the transient data.
pub origin: ActorID,
/// A unique nonce identifying the transaction.
pub nonce: u64,
}

/// A Keccak256 digest of EVM bytecode.
#[derive(Deserialize, Serialize, Clone, Copy, Eq, PartialEq)]
#[serde(transparent)]
Expand Down Expand Up @@ -106,6 +115,11 @@ pub struct State {
/// KAMT<U256, U256>
pub contract_state: Cid,

/// The EVM contract state diciontary that represents the transient storage
pub transient_state: Cid,
/// The nonce and actor id that represents the lifespan of the transient storage data
pub transient_data_lifespan: Option<TransientDataLifespan>,
Comment on lines +119 to +121
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we combine these? That'll let us store a single "null" if there is no transient data.


/// The EVM nonce used to track how many times CREATE or CREATE2 have been called.
pub nonce: u64,

Expand Down
Loading