Skip to content

Commit

Permalink
state/write_log: refactor temp key-vals for better type-safety
Browse files Browse the repository at this point in the history
  • Loading branch information
tzemanovic committed Apr 24, 2024
1 parent 97ec5b4 commit b518472
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 167 deletions.
5 changes: 0 additions & 5 deletions crates/namada/src/ledger/native_vp/ibc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ where
)?;
Ok(None)
}
Some(StorageModification::Temp { .. }) => {
Err(StorageError::new_const(
"Temp shouldn't be inserted in an IBC transaction",
))
}
Some(StorageModification::InitAccount { .. }) => Err(
StorageError::new_const("InitAccount shouldn't be inserted"),
),
Expand Down
41 changes: 14 additions & 27 deletions crates/namada/src/ledger/vp_host_fns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,8 @@ where
// Read the VP of a new account
Ok(Some(vp_code_hash.to_vec()))
}
Some(&write_log::StorageModification::Temp { .. }) | None => {
// When not found in write log or only found a temporary value, try
// to read from the storage
None => {
// When not found in write log, try to read from the storage
let (value, gas) =
state.db_read(key).map_err(RuntimeError::StorageError)?;
add_gas(gas_meter, gas)?;
Expand All @@ -108,19 +107,17 @@ where
S: StateRead + Debug,
{
// Try to read from the write log first
let (log_val, gas) = state.write_log().read_persistent(key);
let (log_val, gas) = state.write_log().read(key);
add_gas(gas_meter, gas)?;
match log_val {
Some(write_log::PersistentStorageModification::Write { value }) => {
Some(write_log::StorageModification::Write { value }) => {
Ok(Some(value.clone()))
}
Some(write_log::PersistentStorageModification::Delete) => {
Some(write_log::StorageModification::Delete) => {
// Given key has been deleted
Ok(None)
}
Some(write_log::PersistentStorageModification::InitAccount {
vp_code_hash,
}) => {
Some(write_log::StorageModification::InitAccount { vp_code_hash }) => {
// Read the VP code hash of a new account
Ok(Some(vp_code_hash.to_vec()))
}
Expand All @@ -145,14 +142,9 @@ pub fn read_temp<S>(
where
S: StateRead + Debug,
{
let (log_val, gas) = state.write_log().read(key);
let (log_val, gas) = state.write_log().read_temp(key);
add_gas(gas_meter, gas)?;
match log_val {
Some(write_log::StorageModification::Temp { ref value }) => {
Ok(Some(value.clone()))
}
_ => Ok(None),
}
Ok(log_val.cloned())
}

/// Storage `has_key` in prior state (before tx execution). It will try to read
Expand All @@ -175,9 +167,8 @@ where
Ok(false)
}
Some(&write_log::StorageModification::InitAccount { .. }) => Ok(true),
Some(&write_log::StorageModification::Temp { .. }) | None => {
// When not found in write log or only found a temporary value, try
// to check the storage
None => {
// When not found in write log, try to check the storage
let (present, gas) =
state.db_has_key(key).map_err(RuntimeError::StorageError)?;
add_gas(gas_meter, gas)?;
Expand All @@ -197,19 +188,15 @@ where
S: StateRead + Debug,
{
// Try to read from the write log first
let (log_val, gas) = state.write_log().read_persistent(key);
let (log_val, gas) = state.write_log().read(key);
add_gas(gas_meter, gas)?;
match log_val {
Some(write_log::PersistentStorageModification::Write { .. }) => {
Ok(true)
}
Some(write_log::PersistentStorageModification::Delete) => {
Some(write_log::StorageModification::Write { .. }) => Ok(true),
Some(write_log::StorageModification::Delete) => {
// The given key has been deleted
Ok(false)
}
Some(write_log::PersistentStorageModification::InitAccount {
..
}) => Ok(true),
Some(write_log::StorageModification::InitAccount { .. }) => Ok(true),
None => {
// When not found in write log, try
// to check the storage
Expand Down
19 changes: 3 additions & 16 deletions crates/namada/src/vm/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ pub enum TxRuntimeError {
StorageError(#[from] StorageError),
#[error("Storage data error: {0}")]
StorageDataError(crate::storage::Error),
#[error("Trying to read a permanent value with read_temp")]
ReadPermanentValueError,
#[error("Encoding error: {0}")]
EncodingError(std::io::Error),
#[error("Address error: {0}")]
Expand Down Expand Up @@ -714,23 +712,16 @@ where
let key = Key::parse(key).map_err(TxRuntimeError::StorageDataError)?;

let write_log = unsafe { env.ctx.write_log.get() };
let (log_val, gas) = write_log.read(&key);
let (log_val, gas) = write_log.read_temp(&key);
tx_charge_gas::<MEM, D, H, CA>(env, gas)?;
let value = match log_val {
Some(write_log::StorageModification::Temp { ref value }) => {
Ok(Some(value.clone()))
}
None => Ok(None),
_ => Err(TxRuntimeError::ReadPermanentValueError),
}?;
match value {
match log_val {
Some(value) => {
let len: i64 = value
.len()
.try_into()
.map_err(TxRuntimeError::NumConversionError)?;
let result_buffer = unsafe { env.ctx.result_buffer.get() };
result_buffer.replace(value);
result_buffer.replace(value.clone());
Ok(len)
}
None => Ok(HostEnvResult::Fail.to_i64()),
Expand Down Expand Up @@ -850,10 +841,6 @@ where
// a VP of a new account doesn't need to be iterated
continue;
}
Some(write_log::StorageModification::Temp { .. }) => {
// temporary values are not returned by the iterator
continue;
}
None => {
let key_val = borsh::to_vec(&KeyVal { key, val })
.map_err(TxRuntimeError::EncodingError)?;
Expand Down
17 changes: 7 additions & 10 deletions crates/state/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,14 @@ macro_rules! impl_storage_read {
key: &storage::Key,
) -> namada_storage::Result<Option<Vec<u8>>> {
// try to read from the write log first
let (log_val, gas) = self.write_log().read_persistent(key);
let (log_val, gas) = self.write_log().read(key);
self.charge_gas(gas).into_storage_result()?;
match log_val {
Some(write_log::PersistentStorageModification::Write { value }) => {
Some(write_log::StorageModification::Write { value }) => {
Ok(Some(value.clone()))
}
Some(write_log::PersistentStorageModification::Delete) => Ok(None),
Some(write_log::PersistentStorageModification::InitAccount {
Some(write_log::StorageModification::Delete) => Ok(None),
Some(write_log::StorageModification::InitAccount {
ref vp_code_hash,
}) => Ok(Some(vp_code_hash.to_vec())),
None => {
Expand All @@ -224,8 +224,8 @@ macro_rules! impl_storage_read {
// the given key has been deleted
Ok(false)
}
Some(&write_log::StorageModification::Temp { .. }) | None => {
// when not found in write log or only found a temporary value, try to check the storage
None => {
// when not found in write log try to check the storage
let (present, gas) = self.db_has_key(key).into_storage_result()?;
self.charge_gas(gas).into_storage_result()?;
Ok(present)
Expand Down Expand Up @@ -407,8 +407,6 @@ pub fn merklize_all_keys(_key: &storage::Key) -> bool {
pub enum Error {
#[error("TEMPORARY error: {error}")]
Temporary { error: String },
#[error("Found an unknown key: {key}")]
UnknownKey { key: String },
#[error("Storage key error {0}")]
KeyError(namada_core::storage::Error),
#[error("Coding error: {0}")]
Expand Down Expand Up @@ -554,8 +552,7 @@ where
let gas = vp_code_hash.len() as u64;
return Some((key, vp_code_hash.to_vec(), gas));
}
write_log::StorageModification::Delete
| write_log::StorageModification::Temp { .. } => {
write_log::StorageModification::Delete => {
continue;
}
}
Expand Down
9 changes: 2 additions & 7 deletions crates/state/src/wl_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,6 @@ where
StorageModification::InitAccount { vp_code_hash } => {
self.batch_write_subspace_val(batch, &key, vp_code_hash)?;
}
// temporary value isn't persisted
StorageModification::Temp { .. } => {}
}
}
debug_assert!(self.0.write_log.block_write_log.is_empty());
Expand Down Expand Up @@ -1274,18 +1272,15 @@ where
&self,
key: &storage::Key,
) -> Result<Option<T>> {
let (log_val, _) = self.write_log().read(key);
let (log_val, _) = self.write_log().read_temp(key);
match log_val {
Some(crate::write_log::StorageModification::Temp { value }) => {
Some(value) => {
let value =
namada_core::borsh::BorshDeserialize::try_from_slice(value)
.map_err(Error::BorshCodingError)?;
Ok(Some(value))
}
None => Ok(None),
_ => Err(Error::UnknownKey {
key: key.to_string(),
}),
}
}
}
Expand Down
Loading

0 comments on commit b518472

Please sign in to comment.