From d6040cdec0bbd7df95cdc6a0c7498e3337c66983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 28 Jun 2024 20:51:05 +0100 Subject: [PATCH 1/4] shielded_token: replace parameters dep with DI --- Cargo.lock | 1 + crates/node/src/bench_utils.rs | 7 ++++--- crates/node/src/shell/finalize_block.rs | 22 ++++++++++++++++++++-- crates/node/src/storage/mod.rs | 2 +- crates/parameters/src/lib.rs | 16 ++++++++++++++++ crates/shielded_token/Cargo.toml | 1 - crates/shielded_token/src/conversion.rs | 20 +++++++++++--------- crates/systems/src/parameters.rs | 10 ++++++---- crates/token/Cargo.toml | 1 + crates/token/src/lib.rs | 6 ++++-- wasm/Cargo.lock | 2 +- wasm_for_tests/Cargo.lock | 2 +- 12 files changed, 66 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 598fd20d09..0c41b13d14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5231,6 +5231,7 @@ dependencies = [ "namada_macros", "namada_shielded_token", "namada_storage", + "namada_systems", "namada_trans_token", "proptest", "serde", diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index d58839de4f..e63d57c64a 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -443,9 +443,10 @@ impl BenchShell { .is_masp_new_epoch(true, masp_epoch_multiplier) .unwrap() { - namada_sdk::token::conversion::update_allowed_conversions( - &mut self.state, - ) + namada_sdk::token::conversion::update_allowed_conversions::< + _, + parameters::Store<_>, + >(&mut self.state) .unwrap(); } } diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index 11e8593fb4..1e4c05146a 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -17,7 +17,9 @@ use namada_sdk::proof_of_stake::storage::{ find_validator_by_raw_hash, write_last_block_proposer_address, }; use namada_sdk::state::write_log::StorageModification; -use namada_sdk::state::{ResultExt, StorageWrite, EPOCH_SWITCH_BLOCKS_DELAY}; +use namada_sdk::state::{ + ResultExt, StorageResult, StorageWrite, EPOCH_SWITCH_BLOCKS_DELAY, +}; use namada_sdk::storage::{BlockResults, Epoch, Header}; use namada_sdk::tx::data::protocol::ProtocolTxType; use namada_sdk::tx::data::VpStatusFlags; @@ -102,7 +104,7 @@ where new_epoch, )?; // - Token - token::finalize_block(&mut self.state, emit_events, is_masp_new_epoch)?; + token_finalize_block(&mut self.state, emit_events, is_masp_new_epoch)?; // - PoS // - Must be applied after governance in case it changes PoS params proof_of_stake::finalize_block( @@ -1131,6 +1133,22 @@ fn pos_votes_from_abci( .collect() } +/// Dependency-injection indirection for token system +fn token_finalize_block( + storage: &mut S, + events: &mut Vec, + is_new_masp_epoch: bool, +) -> StorageResult<()> +where + S: StorageWrite + StorageRead + token::WithConversionState, +{ + token::finalize_block::>( + storage, + events, + is_new_masp_epoch, + ) +} + /// We test the failure cases of [`finalize_block`]. The happy flows /// are covered by the e2e tests. #[allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] diff --git a/crates/node/src/storage/mod.rs b/crates/node/src/storage/mod.rs index 4d6e5ea552..acee12eba0 100644 --- a/crates/node/src/storage/mod.rs +++ b/crates/node/src/storage/mod.rs @@ -171,7 +171,7 @@ mod tests { .new_epoch(BlockHeight(100)); // update conversion for a new epoch - update_allowed_conversions(&mut state) + update_allowed_conversions::<_, parameters::Store<_>>(&mut state) .expect("update conversions failed"); state.commit_block().expect("commit failed"); diff --git a/crates/parameters/src/lib.rs b/crates/parameters/src/lib.rs index 588f58af4d..abdd73b808 100644 --- a/crates/parameters/src/lib.rs +++ b/crates/parameters/src/lib.rs @@ -64,6 +64,10 @@ where fn is_native_token_transferable(storage: &S) -> Result { storage::is_native_token_transferable(storage) } + + fn epochs_per_year(storage: &S) -> Result { + read_epochs_per_year(storage) + } } impl Write for Store @@ -326,6 +330,18 @@ where Ok(gas_cost_table.get(token).map(|amount| amount.to_owned())) } +/// Read the number of epochs per year parameter +pub fn read_epochs_per_year(storage: &S) -> namada_storage::Result +where + S: StorageRead, +{ + let key = storage::get_epochs_per_year_key(); + let epochs_per_year = storage.read(&key)?; + epochs_per_year + .ok_or(ReadError::ParametersMissing) + .into_storage_result() +} + /// Read all the parameters from storage. Returns the parameters and gas /// cost. pub fn read(storage: &S) -> namada_storage::Result diff --git a/crates/shielded_token/Cargo.toml b/crates/shielded_token/Cargo.toml index 39f3d6aec7..fb27435b40 100644 --- a/crates/shielded_token/Cargo.toml +++ b/crates/shielded_token/Cargo.toml @@ -27,7 +27,6 @@ namada_account = { path = "../account" } namada_controller = { path = "../controller" } namada_core = { path = "../core" } namada_gas = { path = "../gas" } -namada_parameters = { path = "../parameters" } namada_state = { path = "../state" } namada_storage = { path = "../storage" } namada_systems = { path = "../systems" } diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index 6df567e27a..7535e3967b 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -10,6 +10,7 @@ use namada_core::dec::Dec; use namada_core::hash::Hash; use namada_core::uint::Uint; use namada_storage::{StorageRead, StorageWrite}; +use namada_systems::parameters; use namada_trans_token::{ get_effective_total_native_supply, read_balance, read_denom, Amount, DenominatedAmount, Denomination, @@ -223,22 +224,24 @@ where // This is only enabled when "wasm-runtime" is on, because we're using rayon #[cfg(not(any(feature = "multicore", test)))] /// Update the MASP's allowed conversions -pub fn update_allowed_conversions( +pub fn update_allowed_conversions( _storage: &mut S, ) -> namada_storage::Result<()> where S: StorageWrite + StorageRead + WithConversionState, + Params: parameters::Read, { Ok(()) } #[cfg(any(feature = "multicore", test))] /// Update the MASP's allowed conversions -pub fn update_allowed_conversions( +pub fn update_allowed_conversions( storage: &mut S, ) -> namada_storage::Result<()> where S: StorageWrite + StorageRead + WithConversionState, + Params: parameters::Read, { use std::cmp::Ordering; use std::collections::BTreeMap; @@ -251,7 +254,6 @@ where use masp_primitives::transaction::components::I128Sum as MaspAmount; use namada_core::arith::CheckedAdd; use namada_core::masp::{encode_asset_type, MaspEpoch}; - use namada_parameters as parameters; use namada_storage::conversion_state::ConversionLeaf; use namada_storage::{Error, OptionExt, ResultExt}; use namada_trans_token::{MaspDigitPos, NATIVE_MAX_DECIMAL_PLACES}; @@ -329,8 +331,7 @@ where calculate_masp_rewards_precision(storage, &native_token)?.0; // Reward all tokens according to above reward rates - let masp_epoch_multiplier = - parameters::read_masp_epoch_multiplier_parameter(storage)?; + let masp_epoch_multiplier = Params::masp_epoch_multiplier(storage)?; let masp_epoch = MaspEpoch::try_from_epoch( storage.get_block_epoch()?, masp_epoch_multiplier, @@ -340,9 +341,7 @@ where Some(epoch) => epoch, None => return Ok(()), }; - let epochs_per_year = storage - .read::(¶meters::storage::get_epochs_per_year_key())? - .expect("epochs per year should properly decode"); + let epochs_per_year = Params::epochs_per_year(storage)?; let masp_epochs_per_year = checked!(epochs_per_year / masp_epoch_multiplier)?; for token in &masp_reward_keys { @@ -718,7 +717,10 @@ mod tests { for i in 0..ROUNDS { println!("Round {i}"); - update_allowed_conversions(&mut s).unwrap(); + update_allowed_conversions::<_, namada_parameters::Store<_>>( + &mut s, + ) + .unwrap(); println!(); println!(); } diff --git a/crates/systems/src/parameters.rs b/crates/systems/src/parameters.rs index 049df39948..9a968c2b10 100644 --- a/crates/systems/src/parameters.rs +++ b/crates/systems/src/parameters.rs @@ -15,15 +15,17 @@ pub trait Read { /// Read all parameters fn read(storage: &S) -> Result; - /// Read MASP epoch multiplier + /// Read MASP epoch multiplier parameter fn masp_epoch_multiplier(storage: &S) -> Result; - /// Read the the epoch duration parameter from store + /// Read the the epoch duration parameter fn epoch_duration_parameter(storage: &S) -> Result; - /// Helper function to retrieve the `is_native_token_transferable` protocol - /// parameter from storage + /// Read the `is_native_token_transferable` parameter fn is_native_token_transferable(storage: &S) -> Result; + + /// Read the number of epochs per year parameter + fn epochs_per_year(storage: &S) -> Result; } /// Abstract parameters storage write interface diff --git a/crates/token/Cargo.toml b/crates/token/Cargo.toml index 97ac5f7103..10cb33eb77 100644 --- a/crates/token/Cargo.toml +++ b/crates/token/Cargo.toml @@ -24,6 +24,7 @@ namada_events = { path = "../events", default-features = false } namada_macros = { path = "../macros" } namada_shielded_token = { path = "../shielded_token" } namada_storage = { path = "../storage" } +namada_systems = { path = "../systems" } namada_trans_token = { path = "../trans_token" } borsh.workspace = true diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index f521450fd2..4102d29491 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -21,6 +21,7 @@ use namada_core::borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use namada_macros::BorshDeserializer; pub use namada_shielded_token::*; +use namada_systems::parameters; pub use namada_trans_token::*; /// Validity predicates @@ -65,16 +66,17 @@ where } /// Apply token logic for finalizing block (i.e. shielded token rewards) -pub fn finalize_block( +pub fn finalize_block( storage: &mut S, _events: &mut impl EmitEvents, is_new_masp_epoch: bool, ) -> Result<()> where S: StorageWrite + StorageRead + WithConversionState, + Params: parameters::Read, { if is_new_masp_epoch { - conversion::update_allowed_conversions(storage)?; + conversion::update_allowed_conversions::(storage)?; } Ok(()) } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 4610620c1e..9ca408edfd 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3826,7 +3826,6 @@ dependencies = [ "namada_controller", "namada_core", "namada_gas", - "namada_parameters", "namada_state", "namada_storage", "namada_systems", @@ -3940,6 +3939,7 @@ dependencies = [ "namada_macros", "namada_shielded_token", "namada_storage", + "namada_systems", "namada_trans_token", "proptest", "serde", diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index 633d4f6634..5fceabeee0 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -3826,7 +3826,6 @@ dependencies = [ "namada_controller", "namada_core", "namada_gas", - "namada_parameters", "namada_state", "namada_storage", "namada_systems", @@ -3940,6 +3939,7 @@ dependencies = [ "namada_macros", "namada_shielded_token", "namada_storage", + "namada_systems", "namada_trans_token", "proptest", "serde", From 0db2cd82ab033b0ce9e8fcd269c0f52cb406d2f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 3 Jul 2024 15:21:21 +0100 Subject: [PATCH 2/4] shielded_token: replace trans_token dep with DI --- crates/node/src/bench_utils.rs | 5 +- crates/node/src/storage/mod.rs | 8 ++- crates/sdk/src/validation.rs | 6 +- crates/shielded_token/Cargo.toml | 2 +- crates/shielded_token/src/conversion.rs | 90 +++++++++++++++--------- crates/shielded_token/src/storage.rs | 33 ++++++--- crates/shielded_token/src/storage_key.rs | 40 +++++++---- crates/shielded_token/src/vp.rs | 21 +++--- crates/systems/src/trans_token.rs | 32 ++++++++- crates/token/src/lib.rs | 49 ++++++++++++- crates/trans_token/src/lib.rs | 52 +++++++++++--- crates/trans_token/src/storage.rs | 1 + crates/trans_token/src/storage_key.rs | 2 +- wasm/Cargo.lock | 1 - wasm_for_tests/Cargo.lock | 1 - 15 files changed, 252 insertions(+), 91 deletions(-) diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index e63d57c64a..d03547d0c8 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -81,7 +81,7 @@ use namada_sdk::state::StorageRead; use namada_sdk::storage::testing::get_dummy_header; use namada_sdk::storage::{BlockHeight, Epoch, Key, KeySeg, TxIndex}; use namada_sdk::time::DateTimeUtc; -use namada_sdk::token::{Amount, DenominatedAmount, Transfer}; +use namada_sdk::token::{self, Amount, DenominatedAmount, Transfer}; use namada_sdk::tx::data::pos::Bond; use namada_sdk::tx::data::{BatchedTxResult, Fee, TxResult, VpsResult}; use namada_sdk::tx::event::{new_tx_event, Batch}; @@ -443,9 +443,10 @@ impl BenchShell { .is_masp_new_epoch(true, masp_epoch_multiplier) .unwrap() { - namada_sdk::token::conversion::update_allowed_conversions::< + token::conversion::update_allowed_conversions::< _, parameters::Store<_>, + token::Store<_>, >(&mut self.state) .unwrap(); } diff --git a/crates/node/src/storage/mod.rs b/crates/node/src/storage/mod.rs index acee12eba0..628cb47eb7 100644 --- a/crates/node/src/storage/mod.rs +++ b/crates/node/src/storage/mod.rs @@ -72,7 +72,7 @@ mod tests { use namada_sdk::storage::{BlockHeight, Key, KeySeg}; use namada_sdk::token::conversion::update_allowed_conversions; use namada_sdk::{ - address, decode, encode, parameters, storage, validation, + address, decode, encode, parameters, storage, token, validation, }; use proptest::collection::vec; use proptest::prelude::*; @@ -171,8 +171,10 @@ mod tests { .new_epoch(BlockHeight(100)); // update conversion for a new epoch - update_allowed_conversions::<_, parameters::Store<_>>(&mut state) - .expect("update conversions failed"); + update_allowed_conversions::<_, parameters::Store<_>, token::Store<_>>( + &mut state, + ) + .expect("update conversions failed"); state.commit_block().expect("commit failed"); // save the last state and the storage diff --git a/crates/sdk/src/validation.rs b/crates/sdk/src/validation.rs index e5de512c5c..b5e2643cc6 100644 --- a/crates/sdk/src/validation.rs +++ b/crates/sdk/src/validation.rs @@ -89,7 +89,7 @@ pub type MaspVp<'a, S, CA> = token::vp::MaspVp< ParamsPreStore<'a, S, CA>, GovPreStore<'a, S, CA>, IbcPostStore<'a, S, CA>, - TokenKeys, + TokenPreStore<'a, S, CA>, >; /// Native ETH bridge VP @@ -117,6 +117,10 @@ pub type PosPreStore<'a, S, CA> = proof_of_stake::Store< CtxPreStorageRead<'a, 'a, S, VpCache, Eval>, >; +/// Token store implementation over the native prior context +pub type TokenPreStore<'a, S, CA> = + token::Store, Eval>>; + /// Ibc store implementation over the native posterior context pub type IbcPostStore<'a, S, CA> = ibc::Store, Eval>>; diff --git a/crates/shielded_token/Cargo.toml b/crates/shielded_token/Cargo.toml index fb27435b40..e3677a3a20 100644 --- a/crates/shielded_token/Cargo.toml +++ b/crates/shielded_token/Cargo.toml @@ -30,7 +30,6 @@ namada_gas = { path = "../gas" } namada_state = { path = "../state" } namada_storage = { path = "../storage" } namada_systems = { path = "../systems" } -namada_trans_token = { path = "../trans_token" } namada_tx = { path = "../tx" } namada_vp = { path = "../vp" } @@ -52,6 +51,7 @@ namada_core = { path = "../core", features = ["testing"] } namada_gas = { path = "../gas" } namada_parameters = { path = "../parameters", features = ["testing"] } namada_storage = { path = "../storage", features = ["testing"] } +namada_trans_token = { path = "../trans_token" } lazy_static.workspace = true masp_proofs = { workspace = true, features = ["download-params"] } diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index 7535e3967b..292e3f6503 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -8,13 +8,10 @@ use namada_core::borsh::BorshSerializeExt; use namada_core::dec::Dec; #[cfg(any(feature = "multicore", test))] use namada_core::hash::Hash; +use namada_core::token::{Amount, DenominatedAmount, Denomination}; use namada_core::uint::Uint; use namada_storage::{StorageRead, StorageWrite}; -use namada_systems::parameters; -use namada_trans_token::{ - get_effective_total_native_supply, read_balance, read_denom, Amount, - DenominatedAmount, Denomination, -}; +use namada_systems::{parameters, trans_token}; #[cfg(any(feature = "multicore", test))] use crate::storage_key::{masp_assets_hash_key, masp_token_map_key}; @@ -66,15 +63,16 @@ pub fn compute_inflation( /// Compute the precision of MASP rewards for the given token. This function /// must be a non-zero constant for a given token. -pub fn calculate_masp_rewards_precision( +pub fn calculate_masp_rewards_precision( storage: &mut S, addr: &Address, ) -> namada_storage::Result<(u128, Denomination)> where S: StorageWrite + StorageRead, + TransToken: trans_token::Read, { - let denomination = - read_denom(storage, addr)?.expect("failed to read token denomination"); + let denomination = TransToken::read_denom(storage, addr)? + .expect("failed to read token denomination"); // Inflation is implicitly denominated by this value. The lower this // figure, the less precise inflation computations are. This is especially // problematic when inflation is coming from a token with much higher @@ -90,51 +88,54 @@ where /// Compute the MASP rewards by applying the PD-controller to the genesis /// parameters and the last inflation and last locked rewards ratio values. -pub fn calculate_masp_rewards( +pub fn calculate_masp_rewards( storage: &mut S, token: &Address, masp_epochs_per_year: u64, ) -> namada_storage::Result<((u128, u128), Denomination)> where S: StorageWrite + StorageRead, + TransToken: trans_token::Keys + trans_token::Read, { let (precision, denomination) = - calculate_masp_rewards_precision(storage, token)?; + calculate_masp_rewards_precision::(storage, token)?; let masp_addr = MASP; // Query the storage for information ------------------------- //// information about the amount of native tokens on the chain - let total_native_tokens = get_effective_total_native_supply(storage)?; + let total_native_tokens = + TransToken::get_effective_total_native_supply(storage)?; // total locked amount in the Shielded pool - let total_tokens_in_masp = read_balance(storage, token, &masp_addr)?; + let total_tokens_in_masp = + TransToken::read_balance(storage, token, &masp_addr)?; //// Values from the last epoch let last_inflation: Amount = storage - .read(&masp_last_inflation_key(token))? + .read(&masp_last_inflation_key::(token))? .expect("failure to read last inflation"); let last_locked_amount: Amount = storage - .read(&masp_last_locked_amount_key(token))? + .read(&masp_last_locked_amount_key::(token))? .expect("failure to read last inflation"); //// Parameters for each token let max_reward_rate: Dec = storage - .read(&masp_max_reward_rate_key(token))? + .read(&masp_max_reward_rate_key::(token))? .expect("max reward should properly decode"); let kp_gain_nom: Dec = storage - .read(&masp_kp_gain_key(token))? + .read(&masp_kp_gain_key::(token))? .expect("kp_gain_nom reward should properly decode"); let kd_gain_nom: Dec = storage - .read(&masp_kd_gain_key(token))? + .read(&masp_kd_gain_key::(token))? .expect("kd_gain_nom reward should properly decode"); let target_locked_amount: Amount = storage - .read(&masp_locked_amount_target_key(token))? + .read(&masp_locked_amount_target_key::(token))? .expect("locked ratio target should properly decode"); let target_locked_dec = Dec::try_from(target_locked_amount.raw_amount()) @@ -214,9 +215,15 @@ where // but we should make sure the return value's ratio matches // this new inflation rate in 'update_allowed_conversions', // otherwise we will have an inaccurate view of inflation - storage.write(&masp_last_inflation_key(token), inflation_amount)?; + storage.write( + &masp_last_inflation_key::(token), + inflation_amount, + )?; - storage.write(&masp_last_locked_amount_key(token), total_tokens_in_masp)?; + storage.write( + &masp_last_locked_amount_key::(token), + total_tokens_in_masp, + )?; Ok(((noterized_inflation, precision), denomination)) } @@ -224,24 +231,27 @@ where // This is only enabled when "wasm-runtime" is on, because we're using rayon #[cfg(not(any(feature = "multicore", test)))] /// Update the MASP's allowed conversions -pub fn update_allowed_conversions( +pub fn update_allowed_conversions( _storage: &mut S, ) -> namada_storage::Result<()> where S: StorageWrite + StorageRead + WithConversionState, Params: parameters::Read, + TransToken: trans_token::Keys, { Ok(()) } #[cfg(any(feature = "multicore", test))] /// Update the MASP's allowed conversions -pub fn update_allowed_conversions( +pub fn update_allowed_conversions( storage: &mut S, ) -> namada_storage::Result<()> where S: StorageWrite + StorageRead + WithConversionState, Params: parameters::Read, + TransToken: + trans_token::Keys + trans_token::Read + trans_token::Write, { use std::cmp::Ordering; use std::collections::BTreeMap; @@ -254,9 +264,9 @@ where use masp_primitives::transaction::components::I128Sum as MaspAmount; use namada_core::arith::CheckedAdd; use namada_core::masp::{encode_asset_type, MaspEpoch}; + use namada_core::token::{MaspDigitPos, NATIVE_MAX_DECIMAL_PLACES}; use namada_storage::conversion_state::ConversionLeaf; use namada_storage::{Error, OptionExt, ResultExt}; - use namada_trans_token::{MaspDigitPos, NATIVE_MAX_DECIMAL_PLACES}; use rayon::iter::{ IndexedParallelIterator, IntoParallelIterator, ParallelIterator, }; @@ -327,8 +337,11 @@ where AllowedConversion, >::new(); // Native token inflation values are always with respect to this - let ref_inflation = - calculate_masp_rewards_precision(storage, &native_token)?.0; + let ref_inflation = calculate_masp_rewards_precision::( + storage, + &native_token, + )? + .0; // Reward all tokens according to above reward rates let masp_epoch_multiplier = Params::masp_epoch_multiplier(storage)?; @@ -346,10 +359,14 @@ where checked!(epochs_per_year / masp_epoch_multiplier)?; for token in &masp_reward_keys { let ((reward, precision), denom) = - calculate_masp_rewards(storage, token, masp_epochs_per_year)?; + calculate_masp_rewards::( + storage, + token, + masp_epochs_per_year, + )?; masp_reward_denoms.insert(token.clone(), denom); // Dispense a transparent reward in parallel to the shielded rewards - let addr_bal = read_balance(storage, token, &masp_addr)?; + let addr_bal = TransToken::read_balance(storage, token, &masp_addr)?; // Get the last rewarded amount of the native token let normed_inflation = *storage @@ -566,7 +583,7 @@ where // Update the MASP's transparent reward token balance to ensure that it // is sufficiently backed to redeem rewards - mint_rewards(storage, total_reward)?; + mint_rewards::(storage, total_reward)?; // Try to distribute Merkle tree construction as evenly as possible // across multiple cores @@ -689,8 +706,13 @@ mod tests { for (token_addr, (alias, denom)) in tokens() { namada_trans_token::write_params(&mut s, &token_addr).unwrap(); - crate::write_params(&token_params, &mut s, &token_addr, &denom) - .unwrap(); + crate::write_params::<_, namada_trans_token::Store<()>>( + &token_params, + &mut s, + &token_addr, + &denom, + ) + .unwrap(); write_denom(&mut s, &token_addr, denom).unwrap(); @@ -717,9 +739,11 @@ mod tests { for i in 0..ROUNDS { println!("Round {i}"); - update_allowed_conversions::<_, namada_parameters::Store<_>>( - &mut s, - ) + update_allowed_conversions::< + _, + namada_parameters::Store<_>, + namada_trans_token::Store<_>, + >(&mut s) .unwrap(); println!(); println!(); diff --git a/crates/shielded_token/src/storage.rs b/crates/shielded_token/src/storage.rs index bddfcb7731..0dcc4cd051 100644 --- a/crates/shielded_token/src/storage.rs +++ b/crates/shielded_token/src/storage.rs @@ -5,21 +5,22 @@ use namada_core::token::Amount; use namada_core::uint::Uint; use namada_storage as storage; use namada_storage::{StorageRead, StorageWrite}; -use namada_trans_token::credit_tokens; +use namada_systems::trans_token; use storage::ResultExt; use crate::storage_key::*; use crate::ShieldedParams; /// Initialize parameters for the token in storage during the genesis block. -pub fn write_params( +pub fn write_params( params: &ShieldedParams, storage: &mut S, - address: &Address, + token: &Address, denom: &token::Denomination, ) -> storage::Result<()> where S: StorageRead + StorageWrite, + TransToken: trans_token::Keys, { let ShieldedParams { max_reward_rate: max_rate, @@ -27,31 +28,41 @@ where kp_gain_nom, locked_amount_target, } = params; - storage.write(&masp_last_inflation_key(address), Amount::zero())?; - storage.write(&masp_last_locked_amount_key(address), Amount::zero())?; - storage.write(&masp_max_reward_rate_key(address), max_rate)?; - storage.write(&masp_kp_gain_key(address), kp_gain_nom)?; - storage.write(&masp_kd_gain_key(address), kd_gain_nom)?; + storage.write( + &masp_last_inflation_key::(token), + Amount::zero(), + )?; + storage.write( + &masp_last_locked_amount_key::(token), + Amount::zero(), + )?; + storage.write(&masp_max_reward_rate_key::(token), max_rate)?; + storage.write(&masp_kp_gain_key::(token), kp_gain_nom)?; + storage.write(&masp_kd_gain_key::(token), kd_gain_nom)?; let locked_amount_target = Uint::from(*locked_amount_target); let raw_target = checked!( locked_amount_target * (Uint::from(10) ^ Uint::from(denom.0)) )?; let raw_target = Amount::from_uint(raw_target, 0).into_storage_result()?; - storage.write(&masp_locked_amount_target_key(address), raw_target)?; + storage.write( + &masp_locked_amount_target_key::(token), + raw_target, + )?; Ok(()) } /// Mint MASP rewards tokens and increment the stored total rewards. -pub fn mint_rewards( +pub fn mint_rewards( storage: &mut S, amount: token::Amount, ) -> storage::Result<()> where S: StorageRead + StorageWrite, + TransToken: trans_token::Write, { let native_token = storage.get_native_token()?; - credit_tokens(storage, &native_token, &address::MASP, amount)?; + TransToken::credit_tokens(storage, &native_token, &address::MASP, amount)?; let total_rewards_key = masp_total_rewards(); let mut total_rewards = read_total_rewards(storage)?; diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index df558b2e9e..e7a76a71e1 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -5,7 +5,7 @@ use masp_primitives::sapling::Nullifier; use namada_core::address::{self, Address}; use namada_core::hash::Hash; use namada_core::storage::{self, DbKeySeg, KeySeg}; -use namada_trans_token::storage_key::parameter_prefix; +use namada_systems::trans_token; /// Key segment prefix for the nullifiers pub const MASP_NULLIFIERS_KEY: &str = "nullifiers"; @@ -36,36 +36,50 @@ pub const MASP_MAX_REWARD_RATE_KEY: &str = "max_reward_rate"; pub const MASP_TOTAL_REWARDS: &str = "max_total_rewards"; /// Obtain the nominal proportional key for the given token -pub fn masp_kp_gain_key(token_addr: &Address) -> storage::Key { - parameter_prefix(token_addr).with_segment(MASP_KP_GAIN_KEY.to_owned()) +pub fn masp_kp_gain_key( + token_addr: &Address, +) -> storage::Key { + TransToken::parameter_prefix(token_addr) + .with_segment(MASP_KP_GAIN_KEY.to_owned()) } /// Obtain the nominal derivative key for the given token -pub fn masp_kd_gain_key(token_addr: &Address) -> storage::Key { - parameter_prefix(token_addr).with_segment(MASP_KD_GAIN_KEY.to_owned()) +pub fn masp_kd_gain_key( + token_addr: &Address, +) -> storage::Key { + TransToken::parameter_prefix(token_addr) + .with_segment(MASP_KD_GAIN_KEY.to_owned()) } /// The max reward rate key for the given token -pub fn masp_max_reward_rate_key(token_addr: &Address) -> storage::Key { - parameter_prefix(token_addr) +pub fn masp_max_reward_rate_key( + token_addr: &Address, +) -> storage::Key { + TransToken::parameter_prefix(token_addr) .with_segment(MASP_MAX_REWARD_RATE_KEY.to_owned()) } /// Obtain the locked target amount key for the given token -pub fn masp_locked_amount_target_key(token_addr: &Address) -> storage::Key { - parameter_prefix(token_addr) +pub fn masp_locked_amount_target_key( + token_addr: &Address, +) -> storage::Key { + TransToken::parameter_prefix(token_addr) .with_segment(MASP_LOCKED_AMOUNT_TARGET_KEY.to_owned()) } /// Obtain the storage key for the last locked amount of a token -pub fn masp_last_locked_amount_key(token_address: &Address) -> storage::Key { - parameter_prefix(token_address) +pub fn masp_last_locked_amount_key( + token_address: &Address, +) -> storage::Key { + TransToken::parameter_prefix(token_address) .with_segment(MASP_LAST_LOCKED_AMOUNT_KEY.to_owned()) } /// Obtain the storage key for the last inflation of a token -pub fn masp_last_inflation_key(token_address: &Address) -> storage::Key { - parameter_prefix(token_address) +pub fn masp_last_inflation_key( + token_address: &Address, +) -> storage::Key { + TransToken::parameter_prefix(token_address) .with_segment(MASP_LAST_INFLATION_KEY.to_owned()) } diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 2d73650b02..ed47f1a97f 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -18,11 +18,11 @@ use namada_core::booleans::BoolResultUnitExt; use namada_core::collections::HashSet; use namada_core::masp::{addr_taddr, encode_asset_type, MaspEpoch, TAddrData}; use namada_core::storage::Key; +use namada_core::token; +use namada_core::token::{Amount, MaspDigitPos}; use namada_core::uint::I320; use namada_state::{ConversionState, OptionExt, ResultExt, StateRead}; -use namada_systems::trans_token::{Amount, MaspDigitPos}; -use namada_systems::{governance, ibc, parameters, trans_token as token}; -use namada_trans_token::read_denom; +use namada_systems::{governance, ibc, parameters, trans_token}; use namada_tx::action::Read; use namada_tx::BatchedTxRef; use namada_vp::native_vp::{ @@ -82,7 +82,8 @@ where Params: parameters::Read>, Gov: governance::Read>, Ibc: ibc::Read>, - TransToken: token::Keys, + TransToken: trans_token::Keys + + trans_token::Read>, { /// Instantiate MASP VP pub fn new(ctx: Ctx<'ctx, S, CA, EVAL>) -> Self { @@ -291,13 +292,14 @@ where // Apply the balance change to the changed balances structure fn apply_balance_change( - &self, + &'view self, mut result: ChangedBalances, [token, counterpart]: [&Address; 2], ) -> Result { - let denom = read_denom(&self.ctx.pre(), token)?.ok_or_err_msg( - "No denomination found in storage for the given token", - )?; + let denom = TransToken::read_denom(&self.ctx.pre(), token)? + .ok_or_err_msg( + "No denomination found in storage for the given token", + )?; // Record the token without an epoch to facilitate later decoding unepoched_tokens(token, denom, &mut result.tokens)?; let counterpart_balance_key = @@ -922,7 +924,8 @@ where Params: parameters::Read>, Gov: governance::Read>, Ibc: ibc::Read>, - TransToken: token::Keys, + TransToken: trans_token::Keys + + trans_token::Read>, { type Error = Error; diff --git a/crates/systems/src/trans_token.rs b/crates/systems/src/trans_token.rs index 3399809a19..9496c0e310 100644 --- a/crates/systems/src/trans_token.rs +++ b/crates/systems/src/trans_token.rs @@ -1,8 +1,8 @@ //! Transparent token abstract interfaces use namada_core::address::Address; -use namada_core::storage; pub use namada_core::token::*; +use namada_core::{storage, token}; pub use namada_storage::Result; /// Abstract token keys interface @@ -23,10 +23,38 @@ pub trait Keys { /// Obtain a storage key for the multitoken minter. fn minter_key(token_addr: &Address) -> storage::Key; + + /// Obtain a storage key prefix for token parameters. + fn parameter_prefix(token_addr: &Address) -> storage::Key; + + /// Obtain a storage key for the minted multitoken balance. + fn minted_balance_key(token_addr: &Address) -> storage::Key; + + /// Check if the given storage key is for total supply of a unspecified + /// token. If it is, returns the token. + fn is_any_minted_balance_key(key: &storage::Key) -> Option<&Address>; } /// Abstract token storage read interface -pub trait Read {} +pub trait Read { + /// Read the denomination of a given token, if any. Note that native + /// transparent tokens do not have this set and instead use the constant + /// [`token::NATIVE_MAX_DECIMAL_PLACES`]. + fn read_denom( + storage: &S, + token: &Address, + ) -> Result>; + + /// Get the effective circulating total supply of native tokens. + fn get_effective_total_native_supply(storage: &S) -> Result; + + /// Read the balance of a given token and owner. + fn read_balance( + storage: &S, + token: &Address, + owner: &Address, + ) -> Result; +} /// Abstract token storage write interface pub trait Write: Read { diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index 4102d29491..bf408aac1f 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -37,8 +37,49 @@ use serde::{Deserialize, Serialize}; /// Token storage keys pub mod storage_key { - pub use namada_shielded_token::storage_key::*; + use namada_core::address::Address; + use namada_core::storage; + use namada_shielded_token::storage_key as shielded; + pub use namada_shielded_token::storage_key::{ + is_masp_commitment_anchor_key, is_masp_key, is_masp_nullifier_key, + is_masp_token_map_key, is_masp_transfer_key, masp_assets_hash_key, + masp_commitment_anchor_key, masp_commitment_tree_key, + masp_convert_anchor_key, masp_nullifier_key, masp_token_map_key, + masp_total_rewards, + }; pub use namada_trans_token::storage_key::*; + + type TransToken = namada_trans_token::Store<()>; + + /// Obtain the nominal proportional key for the given token + pub fn masp_kp_gain_key(token_addr: &Address) -> storage::Key { + shielded::masp_kp_gain_key::(token_addr) + } + + /// Obtain the nominal derivative key for the given token + pub fn masp_kd_gain_key(token_addr: &Address) -> storage::Key { + shielded::masp_kd_gain_key::(token_addr) + } + + /// The max reward rate key for the given token + pub fn masp_max_reward_rate_key(token_addr: &Address) -> storage::Key { + shielded::masp_max_reward_rate_key::(token_addr) + } + + /// Obtain the locked target amount key for the given token + pub fn masp_locked_amount_target_key(token_addr: &Address) -> storage::Key { + shielded::masp_locked_amount_target_key::(token_addr) + } + + /// Obtain the storage key for the last locked amount of a token + pub fn masp_last_locked_amount_key(token_addr: &Address) -> storage::Key { + shielded::masp_last_locked_amount_key::(token_addr) + } + + /// Obtain the storage key for the last inflation of a token + pub fn masp_last_inflation_key(token_addr: &Address) -> storage::Key { + shielded::masp_last_inflation_key::(token_addr) + } } use std::collections::BTreeMap; @@ -60,7 +101,9 @@ where { namada_trans_token::write_params(storage, address)?; if let Some(params) = params { - namada_shielded_token::write_params(params, storage, address, denom)?; + namada_shielded_token::write_params::>( + params, storage, address, denom, + )?; } Ok(()) } @@ -76,7 +119,7 @@ where Params: parameters::Read, { if is_new_masp_epoch { - conversion::update_allowed_conversions::(storage)?; + conversion::update_allowed_conversions::>(storage)?; } Ok(()) } diff --git a/crates/trans_token/src/lib.rs b/crates/trans_token/src/lib.rs index 50e37b1a55..b9cc0bdea7 100644 --- a/crates/trans_token/src/lib.rs +++ b/crates/trans_token/src/lib.rs @@ -25,6 +25,7 @@ pub mod vp; use std::marker::PhantomData; use namada_core::address::Address; +use namada_core::token; use namada_storage::{StorageRead, StorageWrite}; pub use namada_systems::trans_token::*; pub use storage::*; @@ -34,32 +35,63 @@ pub use storage::*; pub struct Store(PhantomData); impl Keys for Store { - fn balance_key( - token: &Address, - owner: &Address, - ) -> namada_core::storage::Key { + fn balance_key(token: &Address, owner: &Address) -> storage::Key { storage_key::balance_key(token, owner) } fn is_balance_key<'a>( token_addr: &Address, - key: &'a namada_core::storage::Key, + key: &'a storage::Key, ) -> Option<&'a Address> { storage_key::is_balance_key(token_addr, key) } - fn is_any_token_balance_key( - key: &namada_core::storage::Key, - ) -> Option<[&Address; 2]> { + fn is_any_token_balance_key(key: &storage::Key) -> Option<[&Address; 2]> { storage_key::is_any_token_balance_key(key) } - fn minter_key(token_addr: &Address) -> namada_core::storage::Key { + fn minter_key(token_addr: &Address) -> storage::Key { storage_key::minter_key(token_addr) } + + fn parameter_prefix(token_addr: &Address) -> storage::Key { + storage_key::parameter_prefix(token_addr) + } + + fn minted_balance_key(token_addr: &Address) -> namada_core::storage::Key { + storage_key::minted_balance_key(token_addr) + } + + fn is_any_minted_balance_key( + key: &namada_core::storage::Key, + ) -> Option<&Address> { + storage_key::is_any_minted_balance_key(key) + } } -impl Read for Store where S: StorageRead {} +impl Read for Store +where + S: StorageRead, +{ + fn read_denom( + storage: &S, + token: &Address, + ) -> Result> { + storage::read_denom(storage, token) + } + + fn get_effective_total_native_supply(storage: &S) -> Result { + storage::get_effective_total_native_supply(storage) + } + + fn read_balance( + storage: &S, + token: &Address, + owner: &Address, + ) -> Result { + storage::read_balance(storage, token, owner) + } +} impl Write for Store where diff --git a/crates/trans_token/src/storage.rs b/crates/trans_token/src/storage.rs index 4a21b2d222..da74b83595 100644 --- a/crates/trans_token/src/storage.rs +++ b/crates/trans_token/src/storage.rs @@ -3,6 +3,7 @@ use std::collections::{BTreeMap, BTreeSet}; use namada_core::address::{Address, InternalAddress}; use namada_core::collections::HashSet; use namada_core::hints; +pub use namada_core::storage::Key; use namada_core::token::{self, Amount, AmountError, DenominatedAmount}; use namada_storage as storage; use namada_storage::{StorageRead, StorageWrite}; diff --git a/crates/trans_token/src/storage_key.rs b/crates/trans_token/src/storage_key.rs index 4b5b132aef..4366f4d8fc 100644 --- a/crates/trans_token/src/storage_key.rs +++ b/crates/trans_token/src/storage_key.rs @@ -44,7 +44,7 @@ pub fn balance_prefix(token_addr: &Address) -> storage::Key { .expect("Cannot obtain a storage key") } -/// Obtain a storage key prefix for all users' balances. +/// Obtain a storage key prefix for token parameters. pub fn parameter_prefix(token_addr: &Address) -> storage::Key { storage::Key::from( Address::Internal(InternalAddress::Multitoken).to_db_key(), diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 9ca408edfd..a04c089eb0 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3829,7 +3829,6 @@ dependencies = [ "namada_state", "namada_storage", "namada_systems", - "namada_trans_token", "namada_tx", "namada_vp", "rand_core 0.6.4", diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index 5fceabeee0..0f2d4f5e36 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -3829,7 +3829,6 @@ dependencies = [ "namada_state", "namada_storage", "namada_systems", - "namada_trans_token", "namada_tx", "namada_vp", "rand_core 0.6.4", From 7b1041205900cde272aa28c7ec2eeb717bc34482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 28 Jun 2024 21:52:37 +0100 Subject: [PATCH 3/4] system/test: add a test for no cross-system deps --- Cargo.lock | 2 ++ crates/systems/Cargo.toml | 4 +++ crates/systems/src/lib.rs | 3 ++ crates/systems/src/test.rs | 56 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+) create mode 100644 crates/systems/src/test.rs diff --git a/Cargo.lock b/Cargo.lock index 0c41b13d14..a26f9aec36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5157,6 +5157,8 @@ dependencies = [ name = "namada_systems" version = "0.41.0" dependencies = [ + "cargo_metadata", + "lazy_static", "namada_core", "namada_storage", ] diff --git a/crates/systems/Cargo.toml b/crates/systems/Cargo.toml index de36aa7d86..8bfa748544 100644 --- a/crates/systems/Cargo.toml +++ b/crates/systems/Cargo.toml @@ -15,3 +15,7 @@ version.workspace = true [dependencies] namada_core = { path = "../core" } namada_storage = { path = "../storage" } + +[dev-dependencies] +cargo_metadata = "0.18.1" +lazy_static.workspace = true diff --git a/crates/systems/src/lib.rs b/crates/systems/src/lib.rs index 7874403d6a..3187f2b588 100644 --- a/crates/systems/src/lib.rs +++ b/crates/systems/src/lib.rs @@ -25,3 +25,6 @@ pub mod pgf; pub mod proof_of_stake; pub mod shielded_token; pub mod trans_token; + +#[cfg(test)] +pub mod test; diff --git a/crates/systems/src/test.rs b/crates/systems/src/test.rs new file mode 100644 index 0000000000..026850c0ac --- /dev/null +++ b/crates/systems/src/test.rs @@ -0,0 +1,56 @@ +use cargo_metadata::{DependencyKind, MetadataCommand}; +use lazy_static::lazy_static; +use namada_core::collections::HashSet; + +lazy_static! { + /// Namada system crate names. None of these crates are allowed to have + /// cross-dependencies (with an exception of dev-deps). + static ref SYSTEMS: HashSet<&'static str> = + HashSet::from_iter([ + "namada_shielded_token", + "namada_parameters", + "namada_trans_token", + "namada_token", + ]); +} + +/// Assert that none of the `SYSTEMS` have cross dependencies. +#[test] +fn test_no_system_cross_deps() { + let metadata = MetadataCommand::new() + .no_deps() + .other_options(vec!["--locked".to_string(), "--offline".to_string()]) + .exec() + .unwrap(); + + for package in metadata.packages { + for system in SYSTEMS.iter() { + if &package.name == system { + for dep in package + .dependencies + .iter() + .filter(|d| matches!(d.kind, DependencyKind::Normal)) + { + for other_system in SYSTEMS.iter() { + // Exception for the "token" crate which puts together + // "trans_token" and "shielded_token". + if &package.name == "namada_token" + && (&dep.name == "namada_trans_token" + || &dep.name == "namada_shielded_token") + { + continue; + } + + if &dep.name == other_system { + panic!( + "Forbidden cross-system dependency: {} \ + depends on {}", + system, other_system + ); + } + } + } + } + } + } +} From 0fc23dd8ea3926a1a684d0ae29f412332e8d469c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Tue, 9 Jul 2024 20:28:35 +0100 Subject: [PATCH 4/4] changelog: add #3466 --- .changelog/unreleased/improvements/3466-di-shielded-token.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/3466-di-shielded-token.md diff --git a/.changelog/unreleased/improvements/3466-di-shielded-token.md b/.changelog/unreleased/improvements/3466-di-shielded-token.md new file mode 100644 index 0000000000..6fc4f1e89c --- /dev/null +++ b/.changelog/unreleased/improvements/3466-di-shielded-token.md @@ -0,0 +1,2 @@ +- Replaced cross-system dependencies in namada_shielded_token crate with + dependency-injection. ([\#3466](https://github.com/anoma/namada/pull/3466)) \ No newline at end of file