Merge branch 'tomas/ibc-tx-simplify' (#2848)

* origin/tomas/ibc-tx-simplify:
  changelog: add #2848
  ibc/tx: rm compatibility wrapper

.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))

crates/ibc/src/actions.rs

@@ -14,11 +14,9 @@ use namada_core::tendermint::Time as TmTime;
 use namada_core::token::Amount;
 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<D, H> 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<Option<Vec<u8>>> {
-        self.0.read_bytes(key)
-    }
-
-    fn has_key(&self, key: &namada_storage::Key) -> StorageResult<bool> {
-        self.0.has_key(key)
-    }
-
-    fn iter_prefix<'iter>(
-        &'iter self,
-        prefix: &namada_storage::Key,
-    ) -> StorageResult<Self::PrefixIter<'iter>> {
-        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<Option<(String, Vec<u8>)>> {
-        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<String> {
-        self.0.get_chain_id()
-    }
-
-    fn get_block_height(&self) -> StorageResult<namada_storage::BlockHeight> {
-        self.0.get_block_height()
-    }
-
-    fn get_block_header(
-        &self,
-        height: namada_storage::BlockHeight,
-    ) -> StorageResult<Option<namada_storage::Header>> {
-        StorageRead::get_block_header(&self.0, height)
-    }
-
-    fn get_block_hash(&self) -> StorageResult<namada_storage::BlockHash> {
-        self.0.get_block_hash()
-    }
-
-    fn get_block_epoch(&self) -> StorageResult<namada_storage::Epoch> {
-        self.0.get_block_epoch()
-    }
-
-    fn get_pred_epochs(&self) -> StorageResult<Epochs> {
-        self.0.get_pred_epochs()
-    }
-
-    fn get_tx_index(&self) -> StorageResult<namada_storage::TxIndex> {
-        self.0.get_tx_index()
-    }
-
-    fn get_native_token(&self) -> StorageResult<Address> {
-        self.0.get_native_token()
-    }
-}
-
-impl<D, H> 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<D, H> 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::H> {
-        self.0.in_mem()
-    }
-
-    fn charge_gas(&self, gas: u64) -> namada_state::Result<()> {
-        self.0.charge_gas(gas)
-    }
-}
-
-impl<D, H> 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::H>, &Self::D) {
-        self.0.split_borrow()
-    }
-}
-
-impl<D, H> IbcStorageContext for CompatibleIbcTxHostEnvState<'_, D, H>
+impl<D, H> IbcStorageContext for TxHostEnvState<'_, D, H>
 where
     D: 'static + DB + for<'iter> DBIter<'iter>,
     H: 'static + StorageHasher,
@@ -359,7 +183,7 @@ where
     }
 }
 
-impl<D, H> IbcCommonContext for CompatibleIbcTxHostEnvState<'_, D, H>
+impl<D, H> IbcCommonContext for TxHostEnvState<'_, D, H>
 where
     D: 'static + DB + for<'iter> DBIter<'iter>,
     H: 'static + StorageHasher,

crates/ibc/src/lib.rs

@@ -10,7 +10,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;
 pub use context::nft_transfer::NftTransferContext;

crates/namada/src/vm/host_env.rs

@@ -2054,18 +2054,15 @@ where
 {
     use std::rc::Rc;
 
-    use namada_ibc::{
-        CompatibleIbcTxHostEnvState, IbcActions, NftTransferModule,
-        TransferModule,
-    };
+    use namada_ibc::{IbcActions, NftTransferModule, TransferModule};
 
     let tx = unsafe { env.ctx.tx.get() };
     let tx_data = tx.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.clone());
     actions.add_transfer_module(module);