From 9e260651a432e6982755cabe7fd0f1737aa7f3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Thu, 7 Mar 2024 16:51:36 +0000 Subject: [PATCH 1/2] ibc/tx: rm compatibility wrapper --- crates/ibc/src/actions.rs | 184 +------------------------------ crates/ibc/src/lib.rs | 2 +- crates/namada/src/vm/host_env.rs | 4 +- 3 files changed, 7 insertions(+), 183 deletions(-) diff --git a/crates/ibc/src/actions.rs b/crates/ibc/src/actions.rs index 57f0ee5335..d72c7ae1a8 100644 --- a/crates/ibc/src/actions.rs +++ b/crates/ibc/src/actions.rs @@ -14,11 +14,9 @@ use namada_core::tendermint::Time as TmTime; use namada_core::token::DenominatedAmount; use namada_governance::storage::proposal::PGFIbcTarget; use namada_parameters::read_epoch_duration_parameter; -use namada_state::write_log::WriteLog; use namada_state::{ - DBIter, Epochs, InMemory, ResultExt, State, StateRead, StorageError, - StorageHasher, StorageRead, StorageResult, StorageWrite, TxHostEnvState, - WlState, DB, + DBIter, Epochs, ResultExt, State, StateRead, StorageError, StorageHasher, + StorageRead, StorageResult, StorageWrite, TxHostEnvState, WlState, DB, }; use namada_token as token; @@ -117,181 +115,7 @@ where } } -/// Temporary wrapper to have gas cost compatible with v0.31.6. -// TODO: Delete this wrapper and use `TxHostEnvState` directly in a breaking -// release. Differs in `iter_next`. -#[derive(Debug)] -pub struct CompatibleIbcTxHostEnvState<'a, D, H>(pub TxHostEnvState<'a, D, H>) -where - D: DB + for<'iter> DBIter<'iter>, - H: StorageHasher; - -impl StorageRead for CompatibleIbcTxHostEnvState<'_, D, H> -where - D: DB + for<'iter> DBIter<'iter> + 'static, - H: StorageHasher + 'static, -{ - type PrefixIter<'iter> = namada_state::PrefixIter<'iter, D> where Self: 'iter; - - fn read_bytes( - &self, - key: &namada_storage::Key, - ) -> StorageResult>> { - self.0.read_bytes(key) - } - - fn has_key(&self, key: &namada_storage::Key) -> StorageResult { - self.0.has_key(key) - } - - fn iter_prefix<'iter>( - &'iter self, - prefix: &namada_storage::Key, - ) -> StorageResult> { - let (iter, gas) = namada_state::iter_prefix_post( - self.0.write_log(), - self.0.db(), - prefix, - ); - self.0.charge_gas(gas).into_storage_result()?; - Ok(iter) - } - - fn iter_next<'iter>( - &'iter self, - iter: &mut Self::PrefixIter<'iter>, - ) -> StorageResult)>> { - use namada_state::write_log; - let write_log = self.0.write_log(); - for (key, val, iter_gas) in iter.by_ref() { - let (log_val, log_gas) = write_log.read( - &namada_storage::Key::parse(key.clone()) - .into_storage_result()?, - ); - self.0 - .charge_gas(iter_gas + log_gas) - .into_storage_result()?; - match log_val { - Some(write_log::StorageModification::Write { ref value }) => { - return Ok(Some((key, value.clone()))); - } - Some(&write_log::StorageModification::Delete) => { - // check the next because the key has already deleted - continue; - } - Some(&write_log::StorageModification::InitAccount { - .. - }) => { - // a VP of a new account doesn't need to be iterated - continue; - } - Some(write_log::StorageModification::Temp { ref value }) => { - return Ok(Some((key, value.clone()))); - } - None => { - return Ok(Some((key, val))); - } - } - } - Ok(None) - } - - fn get_chain_id(&self) -> StorageResult { - self.0.get_chain_id() - } - - fn get_block_height(&self) -> StorageResult { - self.0.get_block_height() - } - - fn get_block_header( - &self, - height: namada_storage::BlockHeight, - ) -> StorageResult> { - StorageRead::get_block_header(&self.0, height) - } - - fn get_block_hash(&self) -> StorageResult { - self.0.get_block_hash() - } - - fn get_block_epoch(&self) -> StorageResult { - self.0.get_block_epoch() - } - - fn get_pred_epochs(&self) -> StorageResult { - self.0.get_pred_epochs() - } - - fn get_tx_index(&self) -> StorageResult { - self.0.get_tx_index() - } - - fn get_native_token(&self) -> StorageResult
{ - self.0.get_native_token() - } -} - -impl StorageWrite for CompatibleIbcTxHostEnvState<'_, D, H> -where - D: DB + for<'iter> DBIter<'iter> + 'static, - H: StorageHasher + 'static, -{ - fn write_bytes( - &mut self, - key: &namada_storage::Key, - val: impl AsRef<[u8]>, - ) -> StorageResult<()> { - self.0.write_bytes(key, val) - } - - fn delete(&mut self, key: &namada_storage::Key) -> StorageResult<()> { - self.0.delete(key) - } -} - -impl StateRead for CompatibleIbcTxHostEnvState<'_, D, H> -where - D: DB + for<'iter> DBIter<'iter> + 'static, - H: StorageHasher + 'static, -{ - type D = D; - type H = H; - - fn write_log(&self) -> &WriteLog { - self.0.write_log - } - - fn db(&self) -> &D { - self.0.db() - } - - fn in_mem(&self) -> &InMemory { - self.0.in_mem() - } - - fn charge_gas(&self, gas: u64) -> namada_state::Result<()> { - self.0.charge_gas(gas) - } -} - -impl State for CompatibleIbcTxHostEnvState<'_, D, H> -where - D: 'static + DB + for<'iter> DBIter<'iter>, - H: 'static + StorageHasher, -{ - fn write_log_mut(&mut self) -> &mut WriteLog { - self.0.write_log_mut() - } - - fn split_borrow( - &mut self, - ) -> (&mut WriteLog, &InMemory, &Self::D) { - self.0.split_borrow() - } -} - -impl IbcStorageContext for CompatibleIbcTxHostEnvState<'_, D, H> +impl IbcStorageContext for TxHostEnvState<'_, D, H> where D: 'static + DB + for<'iter> DBIter<'iter>, H: 'static + StorageHasher, @@ -359,7 +183,7 @@ where } } -impl IbcCommonContext for CompatibleIbcTxHostEnvState<'_, D, H> +impl IbcCommonContext for TxHostEnvState<'_, D, H> where D: 'static + DB + for<'iter> DBIter<'iter>, H: 'static + StorageHasher, diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index 765c03d1ca..3f9a5fa09b 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -9,7 +9,7 @@ use std::fmt::Debug; use std::rc::Rc; use std::str::FromStr; -pub use actions::{transfer_over_ibc, CompatibleIbcTxHostEnvState}; +pub use actions::transfer_over_ibc; use borsh::BorshDeserialize; pub use context::common::IbcCommonContext; use context::router::IbcRouter; diff --git a/crates/namada/src/vm/host_env.rs b/crates/namada/src/vm/host_env.rs index 5139421d0a..f3a219e93e 100644 --- a/crates/namada/src/vm/host_env.rs +++ b/crates/namada/src/vm/host_env.rs @@ -2054,14 +2054,14 @@ where { use std::rc::Rc; - use namada_ibc::{CompatibleIbcTxHostEnvState, IbcActions, TransferModule}; + use namada_ibc::{IbcActions, TransferModule}; let tx_data = unsafe { env.ctx.tx.get().data() }.ok_or_else(|| { let sentinel = unsafe { env.ctx.sentinel.get() }; sentinel.borrow_mut().set_invalid_commitment(); TxRuntimeError::MissingTxData })?; - let state = Rc::new(RefCell::new(CompatibleIbcTxHostEnvState(env.state()))); + let state = Rc::new(RefCell::new(env.state())); let mut actions = IbcActions::new(state.clone()); let module = TransferModule::new(state); actions.add_transfer_module(module.module_id(), module); From d87aa92b882096718175cfee4a38a0e2c1731eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 8 Mar 2024 09:37:11 +0000 Subject: [PATCH 2/2] changelog: add #2848 --- .changelog/unreleased/bug-fixes/2848-ibc-tx-simplify.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/2848-ibc-tx-simplify.md diff --git a/.changelog/unreleased/bug-fixes/2848-ibc-tx-simplify.md b/.changelog/unreleased/bug-fixes/2848-ibc-tx-simplify.md new file mode 100644 index 0000000000..9190f91040 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/2848-ibc-tx-simplify.md @@ -0,0 +1,3 @@ +- Reduce the gas cost of prefix iterator in IBC transactions + to match the cost of prefix iterator elsewhere. + ([\#2848](https://github.com/anoma/namada/pull/2848)) \ No newline at end of file