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 all commits
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));
};
}
}
112 changes: 110 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>,
transient_data_lifespan: Option<TransientDataLifespan>,

/// 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, "transient_state not in blockstore")?,
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 @@ -385,6 +410,89 @@ impl<'r, RT: Runtime> System<'r, RT> {
Ok(())
}

/// Returns the current transient data lifespan based on the execution environment.
pub fn get_current_transient_data_lifespan(&self) -> Option<TransientDataLifespan> {
match self.rt.message().origin().id() {
Ok(origin_id) => Some(TransientDataLifespan {
origin: origin_id,
nonce: self.rt.message().nonce(),
}),
Err(_) => None, // Handle the error case by returning None
}
}

/// Get value of a transient storage key.
pub fn get_transient_storage(&mut self, key: U256) -> Result<U256, ActorError> {
if self.transient_data_lifespan == self.get_current_transient_data_lifespan() {
// Lifespan matches, retrieve value
Ok(self
.transient_slots
.get(&key)
.context_code(ExitCode::USR_ILLEGAL_STATE, "failed to access transient storage slot")?
.cloned()
.unwrap_or_default())
} else {
// Lifespan mismatch, return default value
Ok(U256::zero())
}
}


/// Set value of a transient storage key.
pub fn set_transient_storage(&mut self, key: U256, value: U256) -> Result<(), ActorError> {
let current_lifespan = self.get_current_transient_data_lifespan();

if self.transient_data_lifespan == current_lifespan {
// Lifespan matches, proceed to set value
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 transient storage slot")?
} else {
self.transient_slots
.set(key, value)
.map(|v| v != Some(value))
.context_code(ExitCode::USR_ILLEGAL_STATE, "failed to update transient storage slot")?
};

if changed {
self.saved_state_root = None; // Mark state as dirty
}
} else {
// Lifespan mismatch
if value.is_zero() {
// If setting to default value, skip storage reset
return Ok(());
}

// Reset transient storage and update lifespan
self.reset_transient_storage(current_lifespan)?;
self.transient_slots
.set(key, value)
.context_code(ExitCode::USR_ILLEGAL_STATE, "failed to set transient storage after reset")?;

}

Ok(())
}

/// Reset transient storage and update lifespan.
pub fn reset_transient_storage(&mut self, new_lifespan: Option<TransientDataLifespan>) -> Result<(), ActorError> {
// Update lifespan
self.transient_data_lifespan = new_lifespan;

// Reinitialize the transient_slots with a fresh KAMT
//let transient_store = self.rt.store().clone();
//self.transient_slots = StateKamt::new_with_config(transient_store, KAMT_CONFIG.clone());
// TODO XXX reinitialize does not currently work due to blockstore reference issues
Copy link
Member

Choose a reason for hiding this comment

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

I'd be interested to know what these errors are and how you end up diagnosing the problem, for my own educational purposes because your commented code looks like it should work.

Copy link
Member

Choose a reason for hiding this comment

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

It's because self.rt.store() doesn't implement clone, while &... does, so rust is helpfully creating a reference for you.

Note: the new function explicitly requires that the store be clonable.

Options are:

  1. Lift that requirement up to the impl level.
  2. Use self.transient_slots.into_store() to "take" the existing store.


// Mark state as dirty
self.saved_state_root = None;

Ok(())
}

/// Resolve the address to the ethereum equivalent, if possible.
///
/// - Eth f4 maps directly to an Eth address.
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