Implemented reading conversion updates from memory.

Author: murisi

Cargo.lock

@@ -357,7 +357,7 @@ impl Epoch {
     pub fn iter_bounds_inclusive(
         start: Self,
         end: Self,
-    ) -> impl Iterator<Item = Epoch> + Clone {
+    ) -> impl DoubleEndedIterator<Item = Epoch> + Clone {
         let start_ix = start.0;
         let end_ix = end.0;
         (start_ix..=end_ix).map(Epoch::from)

crates/core/src/chain.rs

@@ -135,6 +135,14 @@ impl MaspEpoch {
         ))
     }
 
+    /// Iterate a range of epochs, inclusive of the start and end.
+    pub fn iter_bounds_inclusive(
+        start: Self,
+        end: Self,
+    ) -> impl DoubleEndedIterator<Item = Self> + Clone {
+        Epoch::iter_bounds_inclusive(start.0, end.0).map(Self)
+    }
+
     /// Returns a 0 masp epoch
     pub const fn zero() -> Self {
         Self(Epoch(0))

crates/core/src/masp.rs

@@ -402,7 +402,7 @@ pub fn is_key_diff_storable(key: &namada_sdk::storage::Key) -> bool {
         && *key != token::storage_key::masp_convert_anchor_key()
         && *key != token::storage_key::masp_token_map_key()
         && *key != token::storage_key::masp_assets_hash_key()
-        && !token::storage_key::is_masp_commitment_anchor_key(key)
+        && token::storage_key::is_masp_commitment_anchor_key(key).is_none()
         || ibc::storage::is_ibc_counter_key(key)
         || proof_of_stake::storage_key::is_delegation_targets_key(key))
 }

crates/node/src/shell/mod.rs

@@ -50,6 +50,7 @@ use crate::ethereum_oracle::{
     control, last_processed_block, try_process_eth_events,
 };
 use crate::shell::testing::utils::TestDir;
+use crate::shell::token::MaspEpoch;
 use crate::shell::{EthereumOracleChannels, Shell};
 use crate::shims::abcipp_shim_types::shim::request::{
     FinalizeBlock, ProcessedTx,
@@ -407,6 +408,16 @@ impl MockNode {
             .0
     }
 
+    pub fn current_masp_epoch(&mut self) -> MaspEpoch {
+        let masp_epoch_multiplier =
+            namada_sdk::parameters::read_masp_epoch_multiplier_parameter(
+                &self.shell.lock().unwrap().state,
+            )
+            .unwrap();
+        let current_epoch = self.current_epoch();
+        MaspEpoch::try_from_epoch(current_epoch, masp_epoch_multiplier).unwrap()
+    }
+
     pub fn next_masp_epoch(&mut self) -> Epoch {
         let masp_epoch_multiplier =
             namada_sdk::parameters::read_masp_epoch_multiplier_parameter(

crates/node/src/shell/testing/node.rs

@@ -27,16 +27,18 @@ use namada_core::masp::MaspEpoch;
 use namada_core::token::MaspDigitPos;
 use namada_core::token::{Amount, DenominatedAmount, Denomination};
 use namada_core::uint::Uint;
+use namada_state::iter_prefix_with_filter_map;
 use namada_systems::{parameters, trans_token};
 
-#[cfg(any(feature = "multicore", test))]
-use crate::storage_key::{masp_assets_hash_key, masp_token_map_key};
 use crate::storage_key::{
-    masp_kd_gain_key, masp_kp_gain_key, masp_last_inflation_key,
-    masp_last_locked_amount_key, masp_locked_amount_target_key,
-    masp_max_reward_rate_key, masp_reward_precision_key,
+    is_masp_conversion_key, masp_conversion_key_prefix, masp_kd_gain_key,
+    masp_kp_gain_key, masp_last_inflation_key, masp_last_locked_amount_key,
+    masp_locked_amount_target_key, masp_max_reward_rate_key,
+    masp_reward_precision_key,
 };
 #[cfg(any(feature = "multicore", test))]
+use crate::storage_key::{masp_assets_hash_key, masp_token_map_key};
+#[cfg(any(feature = "multicore", test))]
 use crate::{ConversionLeaf, Error, OptionExt, ResultExt};
 use crate::{Result, StorageRead, StorageWrite, WithConversionState};
 
@@ -516,6 +518,51 @@ where
     Ok(denom)
 }
 
+#[cfg(any(feature = "multicore", test))]
+/// Apply the conversion updates that are in storage to the in memory structure
+/// and delete them.
+fn apply_stored_conversion_updates<S>(storage: &mut S) -> Result<()>
+where
+    S: StorageWrite + StorageRead + WithConversionState,
+{
+    use masp_primitives::transaction::components::I128Sum;
+
+    let conversion_key_prefix = masp_conversion_key_prefix();
+    let mut conversion_updates = BTreeMap::new();
+    // Read conversion updates from storage and store them in a map
+    for conv_result in iter_prefix_with_filter_map(
+        storage,
+        &conversion_key_prefix,
+        is_masp_conversion_key,
+    )? {
+        match conv_result {
+            Ok((asset_type, conv)) => {
+                conversion_updates.insert(asset_type, conv);
+            }
+            Err(err) => {
+                tracing::warn!("Encountered malformed conversion: {}", err);
+                continue;
+            }
+        }
+    }
+    // Apply the conversion updates to the in memory structure
+    let assets = &mut storage.conversion_state_mut().assets;
+    for (asset_type, conv) in conversion_updates {
+        let Some(leaf) = assets.get_mut(&asset_type) else {
+            tracing::warn!(
+                "Encountered non-existent asset type: {}",
+                asset_type
+            );
+            continue;
+        };
+        // This operation will be expensive for large conversions
+        leaf.conversion = From::<I128Sum>::from(conv);
+    }
+    // Delete the updates now that they have been applied
+    storage.delete_prefix(&conversion_key_prefix)?;
+    Ok(())
+}
+
 #[cfg(any(feature = "multicore", test))]
 /// Update the MASP's allowed conversions
 pub fn update_allowed_conversions<S, Params, TransToken>(
@@ -542,6 +589,7 @@ where
 
     use crate::mint_rewards;
 
+    apply_stored_conversion_updates(storage)?;
     let token_map_key = masp_token_map_key();
     let token_map: namada_core::masp::TokenMap =
         storage.read(&token_map_key)?.unwrap_or_default();

crates/shielded_token/src/conversion.rs

@@ -2,8 +2,9 @@
 
 use std::str::FromStr;
 
+use masp_primitives::asset_type::AssetType;
 use masp_primitives::bls12_381::Scalar;
-use masp_primitives::sapling::Nullifier;
+use masp_primitives::sapling::{Node, Nullifier};
 use namada_core::address::{self, Address};
 use namada_core::hash::Hash;
 use namada_core::storage::{self, DbKeySeg, KeySeg};
@@ -15,6 +16,8 @@ const BALANCE_STORAGE_KEY: &str = "balance";
 pub const MASP_NULLIFIERS_KEY: &str = "nullifiers";
 /// The key for the masp reward balance
 pub const MASP_UNDATED_BALANCE_KEY: &str = "undated_balance";
+/// Key segment prefix for the conversions
+pub const MASP_CONVERSIONS_KEY: &str = "conversions";
 /// Key segment prefix for the note commitment merkle tree
 pub const MASP_NOTE_COMMITMENT_TREE_KEY: &str = "commitment_tree";
 /// Key segment prefix for the note commitment anchor
@@ -141,21 +144,26 @@ pub fn is_masp_key(key: &storage::Key) -> bool {
     }
 }
 
+/// Check if the given storage key is allowed to be touched by a governance
+/// proposal
+pub fn is_masp_governance_key(key: &storage::Key) -> bool {
+    is_masp_token_map_key(key) || is_masp_conversion_key(key).is_some()
+}
+
 /// Check if the given storage key is allowed to be touched by a masp transfer
 pub fn is_masp_transfer_key(key: &storage::Key) -> bool {
-    match &key.segments[..] {
+    is_masp_commitment_tree_key(key)
+        || is_masp_nullifier_key(key)
+        || is_masp_balance_key(key)
+        || is_masp_undated_balance_key(key).is_some()
+}
+
+/// Check if the given storage key is a masp commitment tree key
+pub fn is_masp_commitment_tree_key(key: &storage::Key) -> bool {
+    matches!(&key.segments[..],
         [DbKeySeg::AddressSeg(addr), DbKeySeg::StringSeg(key)]
             if *addr == address::MASP
-                && key == MASP_NOTE_COMMITMENT_TREE_KEY =>
-        {
-            true
-        }
-        _ => {
-            is_masp_nullifier_key(key)
-                || is_masp_balance_key(key)
-                || is_masp_undated_balance_key(key).is_some()
-        }
-    }
+                && key == MASP_NOTE_COMMITMENT_TREE_KEY)
 }
 
 /// Check if the given storage key is a masp nullifier key
@@ -168,12 +176,19 @@ pub fn is_masp_nullifier_key(key: &storage::Key) -> bool {
 }
 
 /// Check if the given key is a masp commitment anchor
-pub fn is_masp_commitment_anchor_key(key: &storage::Key) -> bool {
-    matches!(&key.segments[..],
-    [DbKeySeg::AddressSeg(addr),
-             DbKeySeg::StringSeg(prefix),
-            ..
-        ] if *addr == address::MASP && prefix == MASP_NOTE_COMMITMENT_ANCHOR_PREFIX)
+pub fn is_masp_commitment_anchor_key(key: &storage::Key) -> Option<Node> {
+    match &key.segments[..] {
+        [
+            DbKeySeg::AddressSeg(addr),
+            DbKeySeg::StringSeg(prefix),
+            DbKeySeg::StringSeg(anchor),
+        ] if *addr == address::MASP
+            && prefix == MASP_NOTE_COMMITMENT_ANCHOR_PREFIX =>
+        {
+            Hash::from_str(anchor).map(|x| Node::new(x.0)).ok()
+        }
+        _ => None,
+    }
 }
 
 /// Check if the given storage key is a masp token map key
@@ -184,6 +199,20 @@ pub fn is_masp_token_map_key(key: &storage::Key) -> bool {
         ] if *addr == address::MASP && prefix == MASP_TOKEN_MAP_KEY)
 }
 
+/// Check if the given storage key is a masp nullifier key
+pub fn is_masp_conversion_key(key: &storage::Key) -> Option<AssetType> {
+    match &key.segments[..] {
+        [
+            DbKeySeg::AddressSeg(address::MASP),
+            DbKeySeg::StringSeg(prefix),
+            DbKeySeg::StringSeg(asset_type),
+        ] if prefix == MASP_CONVERSIONS_KEY => {
+            AssetType::from_str(asset_type).ok()
+        }
+        _ => None,
+    }
+}
+
 /// Get a key for a masp nullifier
 pub fn masp_nullifier_key(nullifier: &Nullifier) -> storage::Key {
     storage::Key::from(address::MASP.to_db_key())
@@ -193,6 +222,22 @@ pub fn masp_nullifier_key(nullifier: &Nullifier) -> storage::Key {
         .expect("Cannot obtain a storage key")
 }
 
+/// Get a key for a masp conversion
+pub fn masp_conversion_key(asset_type: &AssetType) -> storage::Key {
+    storage::Key::from(address::MASP.to_db_key())
+        .push(&MASP_CONVERSIONS_KEY.to_owned())
+        .expect("Cannot obtain a storage key")
+        .push(&asset_type.to_string())
+        .expect("Cannot obtain a storage key")
+}
+
+/// Get the key prefix for masp conversions
+pub fn masp_conversion_key_prefix() -> storage::Key {
+    storage::Key::from(address::MASP.to_db_key())
+        .push(&MASP_CONVERSIONS_KEY.to_owned())
+        .expect("Cannot obtain a storage key")
+}
+
 /// Get the key for the masp commitment tree
 pub fn masp_commitment_tree_key() -> storage::Key {
     storage::Key::from(address::MASP.to_db_key())

crates/shielded_token/src/storage_key.rs

@@ -30,7 +30,7 @@ use namada_tx::BatchedTxRef;
 use namada_vp_env::{Error, Result, VpEnv};
 
 use crate::storage_key::{
-    is_masp_key, is_masp_nullifier_key, is_masp_token_map_key,
+    is_masp_governance_key, is_masp_key, is_masp_nullifier_key,
     is_masp_transfer_key, is_masp_undated_balance_key,
     masp_commitment_anchor_key, masp_commitment_tree_key,
     masp_convert_anchor_key, masp_nullifier_key, masp_undated_balance_key,
@@ -99,7 +99,7 @@ where
         let masp_keys_changed: Vec<&Key> =
             keys_changed.iter().filter(|key| is_masp_key(key)).collect();
         let non_allowed_changes = masp_keys_changed.iter().any(|key| {
-            !is_masp_transfer_key(key) && !is_masp_token_map_key(key)
+            !is_masp_transfer_key(key) && !is_masp_governance_key(key)
         });
 
         // Check that the transaction didn't write unallowed masp keys
@@ -108,20 +108,20 @@ where
                 "Found modifications to non-allowed masp keys",
             ));
         }
-        let masp_token_map_changed = masp_keys_changed
+        let masp_governance_changes = masp_keys_changed
             .iter()
-            .any(|key| is_masp_token_map_key(key));
+            .any(|key| is_masp_governance_key(key));
         let masp_transfer_changes = masp_keys_changed
             .iter()
             .any(|key| is_masp_transfer_key(key));
-        if masp_token_map_changed && masp_transfer_changes {
+        if masp_governance_changes && masp_transfer_changes {
             Err(Error::new_const(
                 "Cannot simultaneously do governance proposal and MASP \
                  transfer",
             ))
-        } else if masp_token_map_changed {
-            // The token map can only be changed by a successful governance
-            // proposal
+        } else if masp_governance_changes {
+            // The token map or allowed conversions can only be changed by a
+            // successful governance proposal
             Self::is_valid_parameter_change(ctx, tx_data)
         } else if masp_transfer_changes {
             // The MASP transfer keys can only be changed by a valid Transaction

crates/shielded_token/src/vp.rs

@@ -59,9 +59,9 @@ pub use namada_storage::conversion_state::{
 pub use namada_storage::types::{KVBytes, PatternIterator, PrefixIterator};
 pub use namada_storage::{
     collections, iter_prefix, iter_prefix_bytes, iter_prefix_with_filter,
-    mockdb, tx_queue, BlockStateRead, BlockStateWrite, DBIter, DBWriteBatch,
-    DbError, DbResult, Error, OptionExt, Result, ResultExt, StorageHasher,
-    StorageRead, StorageWrite, DB,
+    iter_prefix_with_filter_map, mockdb, tx_queue, BlockStateRead,
+    BlockStateWrite, DBIter, DBWriteBatch, DbError, DbResult, Error, OptionExt,
+    Result, ResultExt, StorageHasher, StorageRead, StorageWrite, DB,
 };
 use namada_systems::parameters;
 use thiserror::Error;