From 4f3392dd359433f8a6c97845a31f4155f81d4e3d Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 11:01:21 +0200 Subject: [PATCH 01/17] Adds masp custom epoch --- crates/node/src/shell/finalize_block.rs | 13 +++++++------ crates/state/src/wl_state.rs | 22 +++++++++++++++++++--- crates/token/src/lib.rs | 4 ++-- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index 0d268545446..45bfff2a9b1 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -66,7 +66,8 @@ where let mut response = shim::response::FinalizeBlock::default(); // Begin the new block and check if a new epoch has begun - let (height, new_epoch) = self.update_state(req.header); + let (height, (new_epoch, masp_new_epoch)) = + self.update_state(req.header); let (current_epoch, _gas) = self.state.in_mem().get_current_epoch(); let update_for_tendermint = matches!( @@ -76,7 +77,7 @@ where tracing::info!( "Block height: {height}, epoch: {current_epoch}, is new epoch: \ - {new_epoch}." + {new_epoch}, is masp new epoxh: {masp_new_epoch}." ); if update_for_tendermint { tracing::info!( @@ -111,7 +112,7 @@ where new_epoch, )?; // - Token - token::finalize_block(&mut self.state, emit_events, new_epoch)?; + token::finalize_block(&mut self.state, emit_events, masp_new_epoch)?; // - PoS // - Must be applied after governance in case it changes PoS params proof_of_stake::finalize_block( @@ -219,9 +220,9 @@ where /// Sets the metadata necessary for a new block, including the height, /// validator changes, and evidence of byzantine behavior. Applies slashes - /// if necessary. Returns a bool indicating if a new epoch began and the - /// height of the new block. - fn update_state(&mut self, header: Header) -> (BlockHeight, bool) { + /// if necessary. Returns two booleans indicating if a new epoch and a new + /// masp epoch began and the height of the new block. + fn update_state(&mut self, header: Header) -> (BlockHeight, (bool, bool)) { let height = self.state.in_mem().get_last_block_height().next_height(); self.state diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index edb1bedb636..96180d9f98e 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -131,12 +131,14 @@ where } /// Initialize a new epoch when the current epoch is finished. Returns - /// `true` on a new epoch. + /// `true` on a new epoch. Also returns a second boolean stating if a new + /// masp epoch has begun. pub fn update_epoch( &mut self, height: BlockHeight, time: DateTimeUtc, - ) -> StorageResult { + // FIXME: return a struct? + ) -> StorageResult<(bool, bool)> { let parameters = namada_parameters::read(self) .expect("Couldn't read protocol parameters"); @@ -162,7 +164,9 @@ where }; let new_epoch = matches!(self.in_mem.update_epoch_blocks_delay, Some(0)); + let mut masp_new_epoch = false; + // FIXME: need testing if new_epoch { // Reset the delay tracker self.in_mem.update_epoch_blocks_delay = None; @@ -184,8 +188,20 @@ where self.in_mem.block.pred_epochs.new_epoch(height); tracing::info!("Began a new epoch {}", self.in_mem.block.epoch); + + // FIXME: need a protocol param + // FIXME: checked operations or use Epoch operations? + masp_new_epoch = (u64::from(self.in_mem.block.epoch) % 4) == 0; + if masp_new_epoch { + tracing::info!( + "Began a new masp epoch {}", + // FIXME: checked operation + u64::from(self.in_mem.block.epoch) / 4 + ); + } } - Ok(new_epoch) + + Ok((new_epoch, masp_new_epoch)) } /// Commit the current block's write log to the storage and commit the block diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index 2ce15898a56..3e158ac9eea 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -54,12 +54,12 @@ where pub fn finalize_block( storage: &mut S, _events: &mut impl EmitEvents, - is_new_epoch: bool, + is_new_masp_epoch: bool, ) -> Result<()> where S: StorageWrite + StorageRead + WithConversionState, { - if is_new_epoch { + if is_new_masp_epoch { conversion::update_allowed_conversions(storage)?; } Ok(()) From 893427a8f29c22fb9c003def7c76f81229f3d219 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 11:59:14 +0200 Subject: [PATCH 02/17] Refactors masp epoch --- crates/node/src/shell/finalize_block.rs | 14 +++++----- crates/state/src/wl_state.rs | 37 +++++++++++++------------ 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index 45bfff2a9b1..ba6ad34fa79 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -66,8 +66,8 @@ where let mut response = shim::response::FinalizeBlock::default(); // Begin the new block and check if a new epoch has begun - let (height, (new_epoch, masp_new_epoch)) = - self.update_state(req.header); + let (height, new_epoch) = self.update_state(req.header); + let is_masp_new_epoch = self.state.is_masp_new_epoch(new_epoch); let (current_epoch, _gas) = self.state.in_mem().get_current_epoch(); let update_for_tendermint = matches!( @@ -77,7 +77,7 @@ where tracing::info!( "Block height: {height}, epoch: {current_epoch}, is new epoch: \ - {new_epoch}, is masp new epoxh: {masp_new_epoch}." + {new_epoch}, is masp new epoch: {is_masp_new_epoch}." ); if update_for_tendermint { tracing::info!( @@ -112,7 +112,7 @@ where new_epoch, )?; // - Token - token::finalize_block(&mut self.state, emit_events, 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( @@ -220,9 +220,9 @@ where /// Sets the metadata necessary for a new block, including the height, /// validator changes, and evidence of byzantine behavior. Applies slashes - /// if necessary. Returns two booleans indicating if a new epoch and a new - /// masp epoch began and the height of the new block. - fn update_state(&mut self, header: Header) -> (BlockHeight, (bool, bool)) { + /// if necessary. Returns a boolean indicating if a new epoch and the height + /// of the new block. + fn update_state(&mut self, header: Header) -> (BlockHeight, bool) { let height = self.state.in_mem().get_last_block_height().next_height(); self.state diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index 96180d9f98e..951107cb6e9 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -131,14 +131,12 @@ where } /// Initialize a new epoch when the current epoch is finished. Returns - /// `true` on a new epoch. Also returns a second boolean stating if a new - /// masp epoch has begun. + /// `true` on a new epoch. pub fn update_epoch( &mut self, height: BlockHeight, time: DateTimeUtc, - // FIXME: return a struct? - ) -> StorageResult<(bool, bool)> { + ) -> StorageResult { let parameters = namada_parameters::read(self) .expect("Couldn't read protocol parameters"); @@ -164,9 +162,7 @@ where }; let new_epoch = matches!(self.in_mem.update_epoch_blocks_delay, Some(0)); - let mut masp_new_epoch = false; - // FIXME: need testing if new_epoch { // Reset the delay tracker self.in_mem.update_epoch_blocks_delay = None; @@ -188,20 +184,27 @@ where self.in_mem.block.pred_epochs.new_epoch(height); tracing::info!("Began a new epoch {}", self.in_mem.block.epoch); + } - // FIXME: need a protocol param - // FIXME: checked operations or use Epoch operations? - masp_new_epoch = (u64::from(self.in_mem.block.epoch) % 4) == 0; - if masp_new_epoch { - tracing::info!( - "Began a new masp epoch {}", - // FIXME: checked operation - u64::from(self.in_mem.block.epoch) / 4 - ); - } + Ok(new_epoch) + } + + /// Returns `true` if a new masp epoch has begun + pub fn is_masp_new_epoch(&self, is_new_epoch: bool) -> bool { + // FIXME: need a protocol param + // FIXME: checked operations or use Epoch operations? + let masp_new_epoch = + is_new_epoch && (u64::from(self.in_mem.block.epoch) % 4) == 0; + + if masp_new_epoch { + tracing::info!( + "Began a new masp epoch {}", + // FIXME: checked operation + u64::from(self.in_mem.block.epoch) / 4 + ); } - Ok((new_epoch, masp_new_epoch)) + masp_new_epoch } /// Commit the current block's write log to the storage and commit the block From 81cf5046b08cef7ababae58c7b1869714283b491 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 13:39:27 +0200 Subject: [PATCH 03/17] Changes epochs to masp epochs where needed. Extends some traits to return this value --- crates/namada/src/ledger/native_vp/masp.rs | 6 +++--- crates/shielded_token/src/conversion.rs | 14 +++++++++----- crates/state/src/wl_state.rs | 14 ++++++++------ crates/storage/src/lib.rs | 14 ++++++++++++++ crates/vp_env/src/lib.rs | 12 ++++++++++++ 5 files changed, 46 insertions(+), 14 deletions(-) diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index a3446299830..7510d965cc0 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -348,7 +348,7 @@ where tx_data: &BatchedTxRef<'_>, keys_changed: &BTreeSet, ) -> Result<()> { - let epoch = self.ctx.get_block_epoch()?; + let masp_epoch = self.ctx.get_block_masp_epoch()?; let conversion_state = self.ctx.state.in_mem().get_conversion_state(); // Get the Transaction object from the actions @@ -402,7 +402,7 @@ where .get(&masp_address_hash) .unwrap_or(&ValueSum::zero()), &shielded_tx.sapling_value_balance(), - epoch, + masp_epoch, &changed_balances.tokens, conversion_state, )?; @@ -428,7 +428,7 @@ where &shielded_tx, &mut changed_balances, &mut transparent_tx_pool, - epoch, + masp_epoch, conversion_state, &mut signers, )?; diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index e109b93dc2f..ea8b5f6cd56 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -110,9 +110,13 @@ where // total locked amount in the Shielded pool let total_tokens_in_masp = read_balance(storage, token, &masp_addr)?; - let epochs_per_year: u64 = storage - .read(¶meters::storage::get_epochs_per_year_key())? + // FIXME: read from param + let masp_epoch_multiplier = 4; + let epochs_per_year = storage + .read::(¶meters::storage::get_epochs_per_year_key())? .expect("epochs per year should properly decode"); + let masp_epochs_per_year = + checked!(epochs_per_year / masp_epoch_multiplier)?; //// Values from the last epoch let last_inflation: Amount = storage @@ -153,7 +157,7 @@ where last_inflation.raw_amount(), kp_gain_nom, kd_gain_nom, - epochs_per_year, + masp_epochs_per_year, target_locked_dec, last_locked_dec, ); @@ -205,7 +209,7 @@ where last_inflation, kp_gain_nom, kd_gain_nom, - epochs_per_year, + masp_epochs_per_year, ); tracing::debug!("Token address: {:?}", token); tracing::debug!("inflation from the pd controller {:?}", inflation); @@ -332,7 +336,7 @@ where calculate_masp_rewards_precision(storage, &native_token)?.0; // Reward all tokens according to above reward rates - let epoch = storage.get_block_epoch()?; + let epoch = storage.get_block_masp_epoch()?; let prev_epoch = match epoch.prev() { Some(epoch) => epoch, None => return Ok(()), diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index 951107cb6e9..7ddeaac5feb 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -191,16 +191,18 @@ where /// Returns `true` if a new masp epoch has begun pub fn is_masp_new_epoch(&self, is_new_epoch: bool) -> bool { - // FIXME: need a protocol param - // FIXME: checked operations or use Epoch operations? - let masp_new_epoch = - is_new_epoch && (u64::from(self.in_mem.block.epoch) % 4) == 0; + // FIXME: read from protocol param + // FIXME: need a function to convert from epoch to masp epoch? + let masp_new_epoch = is_new_epoch + && checked!(u64::from(self.in_mem.block.epoch) % 4) + .expect("Masp epoch multiplier cannot be 0") + == 0; if masp_new_epoch { tracing::info!( "Began a new masp epoch {}", - // FIXME: checked operation - u64::from(self.in_mem.block.epoch) / 4 + checked!(u64::from(self.in_mem.block.epoch) / 4) + .expect("Masp epoch multiplier cannot be 0") ); } diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index c13a60faa91..da67d13b1aa 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -102,6 +102,20 @@ pub trait StorageRead { /// current transaction is being applied. fn get_block_epoch(&self) -> Result; + /// Getting the block masp epoch. The epoch is that of the block to which + /// the current transaction is being applied. + // FIXME: different type for masp epochs? + fn get_block_masp_epoch(&self) -> Result { + // FIXME: read param from storage + let masp_epoch_multiplier = 4; + + // The masp epoch is the lower integer part of the current epoch divided + // by the masp epoch multiplier parameter + self.get_block_epoch()? + .checked_div(masp_epoch_multiplier) + .ok_or_else(|| Error::new_const("Found a 0 masp epoch multiplier")) + } + /// Given the information about predecessor block epochs fn get_pred_epochs(&self) -> Result; diff --git a/crates/vp_env/src/lib.rs b/crates/vp_env/src/lib.rs index 77516322997..850a33237b6 100644 --- a/crates/vp_env/src/lib.rs +++ b/crates/vp_env/src/lib.rs @@ -82,6 +82,18 @@ where /// current transaction is being applied. fn get_block_epoch(&self) -> Result; + /// Getting the block masp epoch. The epoch is that of the block to which + /// the current transaction is being applied. + // FIXME: better to have a sperate type for masp epochs? + fn get_block_masp_epoch(&self) -> Result { + let epoch = self.get_block_epoch()?; + + // FIXME: read param from storage + epoch.checked_div(4).ok_or_else(|| { + namada_storage::Error::new_const("Found 0 masp epoch multiplier") + }) + } + /// Get the shielded transaction index. fn get_tx_index(&self) -> Result; From eeed978a6cb9e72c0d3e4f3b01ba35b348b28789 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 15:43:33 +0200 Subject: [PATCH 04/17] Introduces `MaspEpoch` for type safety and a covnersion method from a normal `Epoch` --- crates/apps_lib/src/cli.rs | 12 ++-- crates/apps_lib/src/client/rpc.rs | 16 +++-- crates/apps_lib/src/client/tx.rs | 4 +- crates/core/src/masp.rs | 71 +++++++++++++++++++++- crates/namada/src/ledger/native_vp/masp.rs | 27 +++++--- crates/parameters/src/storage.rs | 6 ++ crates/sdk/src/args.rs | 8 +-- crates/sdk/src/masp.rs | 35 ++++++----- crates/sdk/src/queries/shell.rs | 30 ++++++++- crates/sdk/src/rpc.rs | 12 +++- crates/sdk/src/tx.rs | 6 +- crates/shielded_token/src/conversion.rs | 45 ++++++++------ crates/storage/src/conversion_state.rs | 6 +- crates/storage/src/lib.rs | 14 ----- crates/vp_env/src/lib.rs | 12 ---- genesis/localnet/parameters.toml | 2 + genesis/starter/parameters.toml | 2 + 17 files changed, 214 insertions(+), 94 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 62ebb3afa62..750a73c379f 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -2989,6 +2989,7 @@ pub mod args { use namada::core::token::NATIVE_MAX_DECIMAL_PLACES; use namada::hash::Hash; use namada::ibc::core::host::types::identifiers::{ChannelId, PortId}; + use namada::masp::MaspEpoch; use namada::tx::data::GasLimit; pub use namada_sdk::args::*; pub use namada_sdk::tx::{ @@ -3165,6 +3166,7 @@ pub mod args { pub const LIST_FIND_ADDRESSES_ONLY: ArgFlag = flag("addr"); pub const LIST_FIND_KEYS_ONLY: ArgFlag = flag("keys"); pub const LOCALHOST: ArgFlag = flag("localhost"); + pub const MASP_EPOCH: ArgOpt = arg_opt("masp-epoch"); pub const MAX_COMMISSION_RATE_CHANGE: Arg = arg("max-commission-rate-change"); pub const MAX_ETH_GAS: ArgOpt = arg_opt("max_eth-gas"); @@ -5504,7 +5506,7 @@ pub mod args { fn parse(matches: &ArgMatches) -> Self { let query = Query::parse(matches); let token = TOKEN_OPT.parse(matches); - let epoch = EPOCH.parse(matches); + let epoch = MASP_EPOCH.parse(matches); Self { query, epoch, @@ -5514,11 +5516,9 @@ pub mod args { fn def(app: App) -> App { app.add_args::>() - .arg( - EPOCH.def().help(wrap!( - "The epoch for which to query conversions." - )), - ) + .arg(MASP_EPOCH.def().help(wrap!( + "The masp epoch for which to query conversions." + ))) .arg(TOKEN_OPT.def().help(wrap!( "The token address for which to query conversions." ))) diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index ac8f98d3b28..941fa834444 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -32,6 +32,7 @@ use namada::ledger::parameters::{storage as param_storage, EpochDuration}; use namada::ledger::pos::types::{CommissionPair, Slash}; use namada::ledger::pos::PosParams; use namada::ledger::queries::RPC; +use namada::masp::MaspEpoch; use namada::proof_of_stake::types::{ ValidatorState, ValidatorStateInfo, WeightedValidator, }; @@ -72,6 +73,13 @@ pub async fn query_and_print_epoch(context: &impl Namada) -> Epoch { epoch } +/// Query and print the masp epoch of the last committed block +pub async fn query_and_print_masp_epoch(context: &impl Namada) -> MaspEpoch { + let epoch = rpc::query_masp_epoch(context.client()).await.unwrap(); + display_line!(context.io(), "Last committed masp epoch: {}", epoch); + epoch +} + /// Query and print some information to help discern when the next epoch will /// begin. pub async fn query_and_print_next_epoch_info(context: &impl Namada) { @@ -372,7 +380,7 @@ async fn query_shielded_balance( } // The epoch is required to identify timestamped tokens - let epoch = query_and_print_epoch(context).await; + let masp_epoch = query_and_print_masp_epoch(context).await; // Query the token alias in the wallet for pretty printing token balances let token_alias = lookup_token_alias(context, &token, &MASP).await; @@ -400,7 +408,7 @@ async fn query_shielded_balance( context.client(), context.io(), &viewing_key, - epoch, + masp_epoch, ) .await .unwrap() @@ -412,7 +420,7 @@ async fn query_shielded_balance( }; let total_balance = shielded - .decode_combine_sum_to_epoch(context.client(), balance, epoch) + .decode_combine_sum_to_epoch(context.client(), balance, masp_epoch) .await .0 .get(&token); @@ -1765,7 +1773,7 @@ pub async fn query_conversion( Address, token::Denomination, MaspDigitPos, - Epoch, + MaspEpoch, I128Sum, MerklePath, )> { diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index f7a2a182c68..c912be5b390 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -770,9 +770,9 @@ pub async fn submit_transfer( // And it is rejected by a VP matches!(resp.batch_result().get(&cmt_hash), Some(InnerTxResult::VpsRejected(_))) => { - let submission_epoch = rpc::query_and_print_epoch(namada).await; + let submission_masp_epoch = rpc::query_and_print_masp_epoch(namada).await; // And its submission epoch doesn't match construction epoch - if tx_epoch.unwrap() != submission_epoch { + if tx_epoch.unwrap() != submission_masp_epoch { // Then we probably straddled an epoch boundary. Let's retry... edisplay_line!(namada.io(), "MASP transaction rejected and this may be due to the \ diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index 58278eced66..7b3eb8b866c 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use std::fmt::Display; +use std::num::ParseIntError; use std::str::FromStr; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; @@ -23,6 +24,70 @@ use crate::string_encoding::{ }; use crate::token::{Denomination, MaspDigitPos}; +/// Wrapper type around `Epoch` for type safe operations involving the masp +/// epoch +#[derive( + BorshSerialize, + BorshDeserialize, + BorshDeserializer, + BorshSchema, + Clone, + Copy, + Debug, + PartialOrd, + Ord, + PartialEq, + Eq, + Hash, + Serialize, + Deserialize, +)] +pub struct MaspEpoch(Epoch); + +impl Display for MaspEpoch { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl FromStr for MaspEpoch { + type Err = ParseIntError; + + fn from_str(s: &str) -> std::result::Result { + let raw: u64 = u64::from_str(s)?; + Ok(Self(Epoch(raw))) + } +} + +impl MaspEpoch { + /// Converts and `Epoch` into a `MaspEpoch` based on the provided conversion + /// rate + pub fn from_epoch(epoch: Epoch, masp_epoch_multiplier: u64) -> Self { + Self( + epoch + .checked_div(masp_epoch_multiplier) + // FIXME: error here + .expect("Masp epoch multiplier should not be 0"), + ) + } + + /// Returns a 0 masp epoch + pub fn zero() -> Self { + Self(Epoch(0)) + } + + /// Change to the previous masp epoch. + pub fn prev(&self) -> Option { + Some(Self(self.0.checked_sub(1)?)) + } + + /// Initialize a new masp epoch from the provided one + #[cfg(any(test, feature = "testing"))] + pub fn new(epoch: u64) -> Self { + Self(Epoch(epoch)) + } +} + /// The plain representation of a MASP aaset #[derive( BorshSerialize, @@ -47,7 +112,7 @@ pub struct AssetData { /// The digit position covered by this asset type pub position: MaspDigitPos, /// The epoch of the asset type, if any - pub epoch: Option, + pub epoch: Option, } impl AssetData { @@ -66,7 +131,7 @@ impl AssetData { /// Give this pre-asset type the given epoch if already has an epoch. Return /// the replaced value. - pub fn redate(&mut self, to: Epoch) -> Option { + pub fn redate(&mut self, to: MaspEpoch) -> Option { if self.epoch.is_some() { self.epoch.replace(to) } else { @@ -85,7 +150,7 @@ pub fn encode_asset_type( token: Address, denom: Denomination, position: MaspDigitPos, - epoch: Option, + epoch: Option, ) -> Result { AssetData { token, diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 7510d965cc0..5fc31219903 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -17,11 +17,10 @@ use namada_core::address::InternalAddress::Masp; use namada_core::arith::{checked, CheckedAdd, CheckedSub}; use namada_core::booleans::BoolResultUnitExt; use namada_core::collections::HashSet; -use namada_core::masp::encode_asset_type; +use namada_core::masp::{encode_asset_type, MaspEpoch}; use namada_core::storage::Key; use namada_gas::GasMetering; use namada_governance::storage::is_proposal_accepted; -use namada_proof_of_stake::Epoch; use namada_sdk::masp::verify_shielded_tx; use namada_state::{ConversionState, OptionExt, ResultExt, StateRead}; use namada_token::read_denom; @@ -348,7 +347,21 @@ where tx_data: &BatchedTxRef<'_>, keys_changed: &BTreeSet, ) -> Result<()> { - let masp_epoch = self.ctx.get_block_masp_epoch()?; + let masp_epoch_multiplier = self + .ctx + .state + .read::( + &namada_parameters::storage::get_masp_epoch_multiplier_key(), + )? + .ok_or_else(|| { + Error::NativeVpError(native_vp::Error::SimpleMessage( + "Could not deserialize masp epoch multiplier", + )) + })?; + let masp_epoch = MaspEpoch::from_epoch( + self.ctx.get_block_epoch()?, + masp_epoch_multiplier, + ); let conversion_state = self.ctx.state.in_mem().get_conversion_state(); // Get the Transaction object from the actions @@ -567,7 +580,7 @@ fn validate_transparent_input( vin: &TxIn, changed_balances: &mut ChangedBalances, transparent_tx_pool: &mut I128Sum, - epoch: Epoch, + epoch: MaspEpoch, conversion_state: &ConversionState, signers: &mut BTreeSet, ) -> Result<()> { @@ -654,7 +667,7 @@ fn validate_transparent_output( out: &TxOut, changed_balances: &mut ChangedBalances, transparent_tx_pool: &mut I128Sum, - epoch: Epoch, + epoch: MaspEpoch, conversion_state: &ConversionState, ) -> Result<()> { // Non-masp destinations subtract from transparent tx pool @@ -720,7 +733,7 @@ fn validate_transparent_bundle( shielded_tx: &Transaction, changed_balances: &mut ChangedBalances, transparent_tx_pool: &mut I128Sum, - epoch: Epoch, + epoch: MaspEpoch, conversion_state: &ConversionState, signers: &mut BTreeSet, ) -> Result<()> { @@ -779,7 +792,7 @@ fn verify_sapling_balancing_value( pre: &ValueSum, post: &ValueSum, sapling_value_balance: &I128Sum, - target_epoch: Epoch, + target_epoch: MaspEpoch, tokens: &BTreeMap, conversion_state: &ConversionState, ) -> Result<()> { diff --git a/crates/parameters/src/storage.rs b/crates/parameters/src/storage.rs index ce649b07c1a..27204f95680 100644 --- a/crates/parameters/src/storage.rs +++ b/crates/parameters/src/storage.rs @@ -29,6 +29,7 @@ struct Keys { // ======================================== epoch_duration: &'static str, epochs_per_year: &'static str, + masp_epoch_multiplier: &'static str, implicit_vp: &'static str, max_expected_time_per_block: &'static str, tx_allowlist: &'static str, @@ -135,6 +136,11 @@ pub fn get_epochs_per_year_key() -> Key { get_epochs_per_year_key_at_addr(ADDRESS) } +/// Storage key used for masp_epoch_multiplier parameter. +pub fn get_masp_epoch_multiplier_key() -> Key { + get_masp_epoch_multiplier_key_at_addr(ADDRESS) +} + /// Storage key used for the max proposal bytes. pub fn get_max_proposal_bytes_key() -> Key { get_max_proposal_bytes_key_at_addr(ADDRESS) diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index b8cbe7ad068..715e9bcf9f7 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -10,7 +10,7 @@ use namada_core::dec::Dec; use namada_core::ethereum_events::EthAddress; use namada_core::keccak::KeccakHash; use namada_core::key::{common, SchemeType}; -use namada_core::masp::PaymentAddress; +use namada_core::masp::{MaspEpoch, PaymentAddress}; use namada_core::storage::{BlockHeight, Epoch}; use namada_core::time::DateTimeUtc; use namada_core::{storage, token}; @@ -290,7 +290,7 @@ impl TxTransfer { pub async fn build( &mut self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, Option)> + ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, Option)> { tx::build_transfer(context, self).await } @@ -414,7 +414,7 @@ impl TxIbcTransfer { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, Option)> + ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, Option)> { tx::build_ibc_transfer(context, self).await } @@ -1312,7 +1312,7 @@ pub struct QueryConversions { /// Address of a token pub token: Option, /// Epoch of the asset - pub epoch: Option, + pub epoch: Option, } /// Query token balance(s) diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index f37a8287851..68acbeef435 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -52,12 +52,13 @@ use masp_proofs::sapling::BatchValidator; use namada_core::address::Address; use namada_core::collections::{HashMap, HashSet}; use namada_core::dec::Dec; +use namada_core::masp::MaspEpoch; use namada_core::masp::MaspTxRefs; pub use namada_core::masp::{ encode_asset_type, AssetData, BalanceOwner, ExtendedViewingKey, PaymentAddress, TransferSource, TransferTarget, }; -use namada_core::storage::{BlockHeight, Epoch, TxIndex}; +use namada_core::storage::{BlockHeight, TxIndex}; use namada_core::time::{DateTimeUtc, DurationSecs}; use namada_core::uint::Uint; use namada_events::extend::{ @@ -118,7 +119,7 @@ pub struct ShieldedTransfer { /// Metadata pub metadata: SaplingMetadata, /// Epoch in which the transaction was created - pub epoch: Epoch, + pub epoch: MaspEpoch, } /// Shielded pool data for a token @@ -496,7 +497,7 @@ pub struct MaspChange { } /// a masp amount -pub type MaspAmount = ValueSum<(Option, Address), token::Change>; +pub type MaspAmount = ValueSum<(Option, Address), token::Change>; /// An extension of Option's cloned method for pair types fn cloned_pair((a, b): (&T, &U)) -> (T, U) { @@ -1123,7 +1124,7 @@ impl ShieldedContext { client: &(impl Client + Sync), io: &impl Io, vk: &ViewingKey, - target_epoch: Epoch, + target_epoch: MaspEpoch, ) -> Result, Error> { // First get the unexchanged balance if let Some(balance) = self.compute_shielded_balance(vk).await? { @@ -1204,7 +1205,7 @@ impl ShieldedContext { client: &(impl Client + Sync), io: &impl Io, mut input: I128Sum, - target_epoch: Epoch, + target_epoch: MaspEpoch, mut conversions: Conversions, ) -> Result<(I128Sum, I128Sum, Conversions), Error> { // Where we will store our exchanged value @@ -1330,7 +1331,7 @@ impl ShieldedContext { context: &impl Namada, vk: &ViewingKey, target: I128Sum, - target_epoch: Epoch, + target_epoch: MaspEpoch, ) -> Result< ( I128Sum, @@ -1424,7 +1425,7 @@ impl ShieldedContext { &mut self, client: &C, amt: I128Sum, - target_epoch: Epoch, + target_epoch: MaspEpoch, ) -> (ValueSum, I128Sum) { let mut res = ValueSum::zero(); let mut undecoded = ValueSum::zero(); @@ -1540,7 +1541,7 @@ impl ShieldedContext { let _ = shielded.load().await; } // Determine epoch in which to submit potential shielded transaction - let epoch = rpc::query_epoch(context.client()).await?; + let epoch = rpc::query_masp_epoch(context.client()).await?; // Context required for storing which notes are in the source's // possession let memo = MemoBytes::empty(); @@ -1950,7 +1951,7 @@ impl ShieldedContext { async fn convert_amount( &mut self, client: &C, - epoch: Epoch, + epoch: MaspEpoch, token: &Address, denom: Denomination, val: token::Amount, @@ -2166,7 +2167,6 @@ pub mod testing { use crate::masp_primitives::sapling::keys::OutgoingViewingKey; use crate::masp_primitives::sapling::redjubjub::PrivateKey; use crate::masp_primitives::transaction::components::transparent::testing::arb_transparent_address; - use crate::storage::testing::arb_epoch; use crate::token::testing::arb_denomination; /// A context object for verifying the Sapling components of MASP @@ -2732,13 +2732,20 @@ pub mod testing { } } + prop_compose! { + /// Generate an arbitrary masp epoch + pub fn arb_masp_epoch()(epoch: u64) -> MaspEpoch{ + MaspEpoch::new(epoch) + } + } + prop_compose! { /// Generate an arbitrary pre-asset type pub fn arb_pre_asset_type()( token in arb_address(), denom in arb_denomination(), position in arb_masp_digit_pos(), - epoch in option::of(arb_epoch()), + epoch in option::of(arb_masp_epoch()), ) -> AssetData { AssetData { token, @@ -2848,7 +2855,7 @@ pub mod testing { asset_range: impl Into, )(asset_range in Just(asset_range.into()))( (builder, asset_types) in arb_shielded_builder(asset_range), - epoch in arb_epoch(), + epoch in arb_masp_epoch(), prover_rng in arb_rng().prop_map(TestCsprng), mut rng in arb_rng().prop_map(TestCsprng), bparams_rng in arb_rng().prop_map(TestCsprng), @@ -2879,7 +2886,7 @@ pub mod testing { source, asset_range, ), - epoch in arb_epoch(), + epoch in arb_masp_epoch(), prover_rng in arb_rng().prop_map(TestCsprng), mut rng in arb_rng().prop_map(TestCsprng), bparams_rng in arb_rng().prop_map(TestCsprng), @@ -2910,7 +2917,7 @@ pub mod testing { target, asset_range, ), - epoch in arb_epoch(), + epoch in arb_masp_epoch(), prover_rng in arb_rng().prop_map(TestCsprng), mut rng in arb_rng().prop_map(TestCsprng), bparams_rng in arb_rng().prop_map(TestCsprng), diff --git a/crates/sdk/src/queries/shell.rs b/crates/sdk/src/queries/shell.rs index caaff8f7fbd..99c4944602b 100644 --- a/crates/sdk/src/queries/shell.rs +++ b/crates/sdk/src/queries/shell.rs @@ -13,7 +13,7 @@ use namada_core::arith::checked; use namada_core::dec::Dec; use namada_core::hash::Hash; use namada_core::hints; -use namada_core::masp::TokenMap; +use namada_core::masp::{MaspEpoch, TokenMap}; use namada_core::storage::{ self, BlockHeight, BlockResults, Epoch, KeySeg, PrefixValue, }; @@ -41,7 +41,7 @@ type ConversionWithoutPath = ( Address, Denomination, MaspDigitPos, - Epoch, + MaspEpoch, masp_primitives::transaction::components::I128Sum, ); @@ -49,7 +49,7 @@ type Conversion = ( Address, Denomination, MaspDigitPos, - Epoch, + MaspEpoch, masp_primitives::transaction::components::I128Sum, MerklePath, ); @@ -63,6 +63,9 @@ router! {SHELL, // Epoch of the last committed block ( "epoch" ) -> Epoch = epoch, + // Masp epoch of the last committed block + ( "masp_epoch" ) -> MaspEpoch = masp_epoch, + // The address of the native token ( "native_token" ) -> Address = native_token, @@ -321,6 +324,27 @@ where Ok(data) } +fn masp_epoch( + ctx: RequestCtx<'_, D, H, V, T>, +) -> namada_storage::Result +where + D: 'static + DB + for<'iter> DBIter<'iter> + Sync, + H: 'static + StorageHasher + Sync, +{ + let epoch = ctx.state.in_mem().last_epoch; + let masp_epoch_multiplier = ctx + .state + .read::( + &namada_parameters::storage::get_masp_epoch_multiplier_key(), + )? + .ok_or_else(|| { + namada_storage::Error::new_const( + "Could not deserialize masp epoch multiplier", + ) + })?; + Ok(MaspEpoch::from_epoch(epoch, masp_epoch_multiplier)) +} + fn native_token( ctx: RequestCtx<'_, D, H, V, T>, ) -> namada_storage::Result
diff --git a/crates/sdk/src/rpc.rs b/crates/sdk/src/rpc.rs index a03ec02ba9d..52f079c5add 100644 --- a/crates/sdk/src/rpc.rs +++ b/crates/sdk/src/rpc.rs @@ -15,6 +15,7 @@ use namada_core::collections::{HashMap, HashSet}; use namada_core::hash::Hash; use namada_core::ibc::IbcTokenHash; use namada_core::key::common; +use namada_core::masp::MaspEpoch; use namada_core::storage::{ BlockHeight, BlockResults, Epoch, Key, PrefixValue, }; @@ -136,6 +137,13 @@ pub async fn query_epoch( convert_response::(RPC.shell().epoch(client).await) } +/// Query the masp epoch of the last committed block +pub async fn query_masp_epoch( + client: &C, +) -> Result { + convert_response::(RPC.shell().masp_epoch(client).await) +} + /// Query the address of the native token pub async fn query_native_token( client: &C, @@ -311,7 +319,7 @@ pub async fn query_conversion( Address, Denomination, MaspDigitPos, - Epoch, + MaspEpoch, masp_primitives::transaction::components::I128Sum, MerklePath, )> { @@ -330,7 +338,7 @@ pub async fn query_conversions( Address, Denomination, MaspDigitPos, - Epoch, + MaspEpoch, masp_primitives::transaction::components::I128Sum, ), >, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 5dc90cec700..a1e443efcaf 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -38,7 +38,7 @@ use namada_core::ibc::primitives::Timestamp as IbcTimestamp; use namada_core::ibc::{is_nft_trace, MsgNftTransfer, MsgTransfer}; use namada_core::key::{self, *}; use namada_core::masp::{ - AssetData, PaymentAddress, TransferSource, TransferTarget, + AssetData, MaspEpoch, PaymentAddress, TransferSource, TransferTarget, }; use namada_core::storage::Epoch; use namada_core::time::DateTimeUtc; @@ -2438,7 +2438,7 @@ pub async fn build_pgf_stewards_proposal( pub async fn build_ibc_transfer( context: &impl Namada, args: &args::TxIbcTransfer, -) -> Result<(Tx, SigningTxData, Option)> { +) -> Result<(Tx, SigningTxData, Option)> { let refund_target = get_refund_target(context, &args.source, &args.refund_target).await?; @@ -2824,7 +2824,7 @@ pub fn build_batch( pub async fn build_transfer( context: &N, args: &mut args::TxTransfer, -) -> Result<(Tx, SigningTxData, Option)> { +) -> Result<(Tx, SigningTxData, Option)> { let default_signer = Some(args.source.effective_address()); let signing_data = signing::aux_signing_data( context, diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index ea8b5f6cd56..6f178dd9e1e 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -110,11 +110,13 @@ where // total locked amount in the Shielded pool let total_tokens_in_masp = read_balance(storage, token, &masp_addr)?; - // FIXME: read from param - let masp_epoch_multiplier = 4; let epochs_per_year = storage .read::(¶meters::storage::get_epochs_per_year_key())? .expect("epochs per year should properly decode"); + // FIXME: can avoid reading this twice? + let masp_epoch_multiplier = storage + .read::(¶meters::storage::get_masp_epoch_multiplier_key())? + .expect("masp epoch multiplier should properly decode"); let masp_epochs_per_year = checked!(epochs_per_year / masp_epoch_multiplier)?; @@ -258,8 +260,7 @@ where use masp_primitives::merkle_tree::FrozenCommitmentTree; use masp_primitives::sapling::Node; use masp_primitives::transaction::components::I128Sum as MaspAmount; - use namada_core::masp::encode_asset_type; - use namada_core::storage::Epoch; + use namada_core::masp::{encode_asset_type, MaspEpoch}; use namada_storage::conversion_state::ConversionLeaf; use namada_storage::{Error, ResultExt}; use namada_trans_token::storage_key::balance_key; @@ -301,28 +302,28 @@ where native_token.clone(), NATIVE_MAX_DECIMAL_PLACES.into(), MaspDigitPos::Zero, - Some(Epoch(0)), + Some(MaspEpoch::zero()), ) .into_storage_result()?, encode_asset_type( native_token.clone(), NATIVE_MAX_DECIMAL_PLACES.into(), MaspDigitPos::One, - Some(Epoch(0)), + Some(MaspEpoch::zero()), ) .into_storage_result()?, encode_asset_type( native_token.clone(), NATIVE_MAX_DECIMAL_PLACES.into(), MaspDigitPos::Two, - Some(Epoch(0)), + Some(MaspEpoch::zero()), ) .into_storage_result()?, encode_asset_type( native_token.clone(), NATIVE_MAX_DECIMAL_PLACES.into(), MaspDigitPos::Three, - Some(Epoch(0)), + Some(MaspEpoch::zero()), ) .into_storage_result()?, ]; @@ -336,8 +337,14 @@ where calculate_masp_rewards_precision(storage, &native_token)?.0; // Reward all tokens according to above reward rates - let epoch = storage.get_block_masp_epoch()?; - let prev_epoch = match epoch.prev() { + let masp_epoch_multiplier = storage + .read::(¶meters::storage::get_masp_epoch_multiplier_key())? + .expect("masp epoch multiplier should properly decode"); + let masp_epoch = MaspEpoch::from_epoch( + storage.get_block_epoch()?, + masp_epoch_multiplier, + ); + let prev_masp_epoch = match masp_epoch.prev() { Some(epoch) => epoch, None => return Ok(()), }; @@ -361,12 +368,16 @@ where token.clone(), denom, digit, - Some(prev_epoch), + Some(prev_masp_epoch), + ) + .into_storage_result()?; + let new_asset = encode_asset_type( + token.clone(), + denom, + digit, + Some(masp_epoch), ) .into_storage_result()?; - let new_asset = - encode_asset_type(token.clone(), denom, digit, Some(epoch)) - .into_storage_result()?; if *token == native_token { // The amount that will be given of the new native token for // every amount of the native token given in the @@ -496,7 +507,7 @@ where token: token.clone(), denom, digit_pos: digit, - epoch: prev_epoch, + epoch: prev_masp_epoch, conversion: MaspAmount::zero().into(), leaf_pos: 0, }, @@ -585,7 +596,7 @@ where // Add the decoding entry for the new asset type. An uncommitted // node position is used since this is not a conversion. let new_asset = - encode_asset_type(addr.clone(), denom, digit, Some(epoch)) + encode_asset_type(addr.clone(), denom, digit, Some(masp_epoch)) .into_storage_result()?; let tree_size = storage.conversion_state().tree.size(); storage.conversion_state_mut().assets.insert( @@ -594,7 +605,7 @@ where token: addr.clone(), denom, digit_pos: digit, - epoch, + epoch: masp_epoch, conversion: MaspAmount::zero().into(), leaf_pos: tree_size, }, diff --git a/crates/storage/src/conversion_state.rs b/crates/storage/src/conversion_state.rs index 43ad69037d2..3031783c724 100644 --- a/crates/storage/src/conversion_state.rs +++ b/crates/storage/src/conversion_state.rs @@ -4,11 +4,11 @@ use std::collections::BTreeMap; use namada_core::address::Address; use namada_core::borsh::{BorshDeserialize, BorshSerialize}; +use namada_core::masp::MaspEpoch; use namada_core::masp_primitives::asset_type::AssetType; use namada_core::masp_primitives::convert::AllowedConversion; use namada_core::masp_primitives::merkle_tree::FrozenCommitmentTree; use namada_core::masp_primitives::sapling; -use namada_core::storage::Epoch; use namada_core::token::{Denomination, MaspDigitPos}; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] @@ -23,8 +23,8 @@ pub struct ConversionLeaf { pub denom: Denomination, /// The digit position covered by this asset type pub digit_pos: MaspDigitPos, - /// The epoch of the asset type - pub epoch: Epoch, + /// The masp epoch of the asset type + pub epoch: MaspEpoch, /// The actual conversion and generator pub conversion: AllowedConversion, /// The position of this leaf in the conversion tree diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index da67d13b1aa..c13a60faa91 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -102,20 +102,6 @@ pub trait StorageRead { /// current transaction is being applied. fn get_block_epoch(&self) -> Result; - /// Getting the block masp epoch. The epoch is that of the block to which - /// the current transaction is being applied. - // FIXME: different type for masp epochs? - fn get_block_masp_epoch(&self) -> Result { - // FIXME: read param from storage - let masp_epoch_multiplier = 4; - - // The masp epoch is the lower integer part of the current epoch divided - // by the masp epoch multiplier parameter - self.get_block_epoch()? - .checked_div(masp_epoch_multiplier) - .ok_or_else(|| Error::new_const("Found a 0 masp epoch multiplier")) - } - /// Given the information about predecessor block epochs fn get_pred_epochs(&self) -> Result; diff --git a/crates/vp_env/src/lib.rs b/crates/vp_env/src/lib.rs index 850a33237b6..77516322997 100644 --- a/crates/vp_env/src/lib.rs +++ b/crates/vp_env/src/lib.rs @@ -82,18 +82,6 @@ where /// current transaction is being applied. fn get_block_epoch(&self) -> Result; - /// Getting the block masp epoch. The epoch is that of the block to which - /// the current transaction is being applied. - // FIXME: better to have a sperate type for masp epochs? - fn get_block_masp_epoch(&self) -> Result { - let epoch = self.get_block_epoch()?; - - // FIXME: read param from storage - epoch.checked_div(4).ok_or_else(|| { - namada_storage::Error::new_const("Found 0 masp epoch multiplier") - }) - } - /// Get the shielded transaction index. fn get_tx_index(&self) -> Result; diff --git a/genesis/localnet/parameters.toml b/genesis/localnet/parameters.toml index e9d6d83de16..d3e3cb5973f 100644 --- a/genesis/localnet/parameters.toml +++ b/genesis/localnet/parameters.toml @@ -18,6 +18,8 @@ tx_allowlist = [] implicit_vp = "vp_implicit" # Expected number of epochs per year (also sets the min duration of an epoch in seconds) epochs_per_year = 31_536_000 +# The multiplier for masp epochs +masp_epoch_multiplier = 1 # Maximum number of signature per transaction max_signatures_per_transaction = 15 # Max gas for block diff --git a/genesis/starter/parameters.toml b/genesis/starter/parameters.toml index 200c1c8fbe6..79fe3cd8886 100644 --- a/genesis/starter/parameters.toml +++ b/genesis/starter/parameters.toml @@ -18,6 +18,8 @@ tx_allowlist = [] implicit_vp = "vp_implicit" # Expected number of epochs per year (also sets the min duration of an epoch in seconds) epochs_per_year = 31_536_000 +# The multiplier for masp epochs +masp_epoch_multiplier = 1 # Maximum number of signature per transaction max_signatures_per_transaction = 15 # Max gas for block From 0829cadb55af8c377524eefdfccf374030c3a056 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 15:50:09 +0200 Subject: [PATCH 05/17] Refactors `calculate_masp_rewards` --- crates/shielded_token/src/conversion.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index 6f178dd9e1e..1d2740624a4 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -9,7 +9,6 @@ use namada_core::dec::Dec; #[cfg(any(feature = "multicore", test))] use namada_core::hash::Hash; use namada_core::uint::Uint; -use namada_parameters as parameters; use namada_storage::{StorageRead, StorageWrite}; use namada_trans_token::{ get_effective_total_native_supply, read_balance, read_denom, Amount, @@ -93,6 +92,7 @@ where 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, @@ -110,16 +110,6 @@ where // total locked amount in the Shielded pool let total_tokens_in_masp = read_balance(storage, token, &masp_addr)?; - let epochs_per_year = storage - .read::(¶meters::storage::get_epochs_per_year_key())? - .expect("epochs per year should properly decode"); - // FIXME: can avoid reading this twice? - let masp_epoch_multiplier = storage - .read::(¶meters::storage::get_masp_epoch_multiplier_key())? - .expect("masp epoch multiplier should properly decode"); - let masp_epochs_per_year = - checked!(epochs_per_year / masp_epoch_multiplier)?; - //// Values from the last epoch let last_inflation: Amount = storage .read(&masp_last_inflation_key(token))? @@ -261,6 +251,7 @@ where use masp_primitives::sapling::Node; use masp_primitives::transaction::components::I128Sum as MaspAmount; use namada_core::masp::{encode_asset_type, MaspEpoch}; + use namada_parameters as parameters; use namada_storage::conversion_state::ConversionLeaf; use namada_storage::{Error, ResultExt}; use namada_trans_token::storage_key::balance_key; @@ -348,8 +339,14 @@ 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 masp_epochs_per_year = + checked!(epochs_per_year / masp_epoch_multiplier)?; for token in &masp_reward_keys { - let (reward, denom) = calculate_masp_rewards(storage, token)?; + let (reward, denom) = + 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)?; From b11d3c7b82098b317d753b1cd145f5b5e625fe4d Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 16:23:37 +0200 Subject: [PATCH 06/17] Refactors `is_masp_new_epoch` --- crates/core/src/masp.rs | 11 ++++++++ crates/namada/src/ledger/native_vp/masp.rs | 2 +- crates/node/src/shell/finalize_block.rs | 2 +- crates/sdk/src/queries/shell.rs | 2 +- crates/state/src/wl_state.rs | 31 ++++++++++++++-------- 5 files changed, 34 insertions(+), 14 deletions(-) diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index 7b3eb8b866c..c4ed207ceba 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -81,6 +81,17 @@ impl MaspEpoch { Some(Self(self.0.checked_sub(1)?)) } + /// Check if the given epoch is also a new masp epoch based on the + /// multiplier provided + pub fn is_masp_new_epoch(epoch: Epoch, masp_epoch_multiplier: u64) -> bool { + matches!( + Self::from_epoch(epoch, masp_epoch_multiplier) + .0 + .checked_rem(masp_epoch_multiplier), + Some(Epoch(0)) + ) + } + /// Initialize a new masp epoch from the provided one #[cfg(any(test, feature = "testing"))] pub fn new(epoch: u64) -> Self { diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 5fc31219903..9be6ca3b0c3 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -355,7 +355,7 @@ where )? .ok_or_else(|| { Error::NativeVpError(native_vp::Error::SimpleMessage( - "Could not deserialize masp epoch multiplier", + "Missing expected masp epoch multiplier parameter", )) })?; let masp_epoch = MaspEpoch::from_epoch( diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index ba6ad34fa79..7d179f50612 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -67,7 +67,7 @@ where // Begin the new block and check if a new epoch has begun let (height, new_epoch) = self.update_state(req.header); - let is_masp_new_epoch = self.state.is_masp_new_epoch(new_epoch); + let is_masp_new_epoch = self.state.is_masp_new_epoch(new_epoch)?; let (current_epoch, _gas) = self.state.in_mem().get_current_epoch(); let update_for_tendermint = matches!( diff --git a/crates/sdk/src/queries/shell.rs b/crates/sdk/src/queries/shell.rs index 99c4944602b..6ce3ec9be6f 100644 --- a/crates/sdk/src/queries/shell.rs +++ b/crates/sdk/src/queries/shell.rs @@ -339,7 +339,7 @@ where )? .ok_or_else(|| { namada_storage::Error::new_const( - "Could not deserialize masp epoch multiplier", + "Missing expected masp epoch multiplier parameter", ) })?; Ok(MaspEpoch::from_epoch(epoch, masp_epoch_multiplier)) diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index 7ddeaac5feb..eafe5457bfe 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -5,6 +5,7 @@ use namada_core::address::Address; use namada_core::arith::checked; use namada_core::borsh::BorshSerializeExt; use namada_core::chain::ChainId; +use namada_core::masp::MaspEpoch; use namada_core::storage; use namada_core::time::DateTimeUtc; use namada_events::{EmitEvents, EventToEmit}; @@ -190,23 +191,31 @@ where } /// Returns `true` if a new masp epoch has begun - pub fn is_masp_new_epoch(&self, is_new_epoch: bool) -> bool { - // FIXME: read from protocol param - // FIXME: need a function to convert from epoch to masp epoch? + pub fn is_masp_new_epoch(&self, is_new_epoch: bool) -> StorageResult { + let masp_epoch_multiplier = self + .read::( + &namada_parameters::storage::get_masp_epoch_multiplier_key(), + )? + .ok_or_else(|| { + namada_storage::Error::new_const( + "Missing expected masp epoch multiplier parameter", + ) + })?; let masp_new_epoch = is_new_epoch - && checked!(u64::from(self.in_mem.block.epoch) % 4) - .expect("Masp epoch multiplier cannot be 0") - == 0; + && MaspEpoch::is_masp_new_epoch( + self.in_mem.block.epoch, + masp_epoch_multiplier, + ); if masp_new_epoch { - tracing::info!( - "Began a new masp epoch {}", - checked!(u64::from(self.in_mem.block.epoch) / 4) - .expect("Masp epoch multiplier cannot be 0") + let masp_epoch = MaspEpoch::from_epoch( + self.in_mem.block.epoch, + masp_epoch_multiplier, ); + tracing::info!("Began a new masp epoch {masp_epoch}"); } - masp_new_epoch + Ok(masp_new_epoch) } /// Commit the current block's write log to the storage and commit the block From 860aea672d6c306c1798ef6cd48ef5482ca630cb Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 16:51:57 +0200 Subject: [PATCH 07/17] Fallible masp epoch conversion --- crates/core/src/masp.rs | 23 +++++++++++++--------- crates/namada/src/ledger/native_vp/masp.rs | 7 +++++-- crates/sdk/src/queries/shell.rs | 3 ++- crates/shielded_token/src/conversion.rs | 5 +++-- crates/state/src/wl_state.rs | 8 +++++--- 5 files changed, 29 insertions(+), 17 deletions(-) diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index c4ed207ceba..df32e3b32cb 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -62,13 +62,15 @@ impl FromStr for MaspEpoch { impl MaspEpoch { /// Converts and `Epoch` into a `MaspEpoch` based on the provided conversion /// rate - pub fn from_epoch(epoch: Epoch, masp_epoch_multiplier: u64) -> Self { - Self( + pub fn try_from_epoch( + epoch: Epoch, + masp_epoch_multiplier: u64, + ) -> Result { + Ok(Self( epoch .checked_div(masp_epoch_multiplier) - // FIXME: error here - .expect("Masp epoch multiplier should not be 0"), - ) + .ok_or("Masp epoch multiplier cannot be 0")?, + )) } /// Returns a 0 masp epoch @@ -83,13 +85,16 @@ impl MaspEpoch { /// Check if the given epoch is also a new masp epoch based on the /// multiplier provided - pub fn is_masp_new_epoch(epoch: Epoch, masp_epoch_multiplier: u64) -> bool { - matches!( - Self::from_epoch(epoch, masp_epoch_multiplier) + pub fn is_masp_new_epoch( + epoch: Epoch, + masp_epoch_multiplier: u64, + ) -> Result { + Ok(matches!( + Self::try_from_epoch(epoch, masp_epoch_multiplier)? .0 .checked_rem(masp_epoch_multiplier), Some(Epoch(0)) - ) + )) } /// Initialize a new masp epoch from the provided one diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 9be6ca3b0c3..0f3c6ef420f 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -358,10 +358,13 @@ where "Missing expected masp epoch multiplier parameter", )) })?; - let masp_epoch = MaspEpoch::from_epoch( + let masp_epoch = MaspEpoch::try_from_epoch( self.ctx.get_block_epoch()?, masp_epoch_multiplier, - ); + ) + .map_err(|msg| { + Error::NativeVpError(native_vp::Error::new_const(msg)) + })?; let conversion_state = self.ctx.state.in_mem().get_conversion_state(); // Get the Transaction object from the actions diff --git a/crates/sdk/src/queries/shell.rs b/crates/sdk/src/queries/shell.rs index 6ce3ec9be6f..3bf8754f472 100644 --- a/crates/sdk/src/queries/shell.rs +++ b/crates/sdk/src/queries/shell.rs @@ -342,7 +342,8 @@ where "Missing expected masp epoch multiplier parameter", ) })?; - Ok(MaspEpoch::from_epoch(epoch, masp_epoch_multiplier)) + MaspEpoch::try_from_epoch(epoch, masp_epoch_multiplier) + .map_err(namada_storage::Error::new_const) } fn native_token( diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index 1d2740624a4..aaffd56a05c 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -331,10 +331,11 @@ where let masp_epoch_multiplier = storage .read::(¶meters::storage::get_masp_epoch_multiplier_key())? .expect("masp epoch multiplier should properly decode"); - let masp_epoch = MaspEpoch::from_epoch( + let masp_epoch = MaspEpoch::try_from_epoch( storage.get_block_epoch()?, masp_epoch_multiplier, - ); + ) + .map_err(namada_storage::Error::new_const)?; let prev_masp_epoch = match masp_epoch.prev() { Some(epoch) => epoch, None => return Ok(()), diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index eafe5457bfe..6c4a54fcd25 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -205,13 +205,15 @@ where && MaspEpoch::is_masp_new_epoch( self.in_mem.block.epoch, masp_epoch_multiplier, - ); + ) + .map_err(namada_storage::Error::new_const)?; if masp_new_epoch { - let masp_epoch = MaspEpoch::from_epoch( + let masp_epoch = MaspEpoch::try_from_epoch( self.in_mem.block.epoch, masp_epoch_multiplier, - ); + ) + .map_err(namada_storage::Error::new_const)?; tracing::info!("Began a new masp epoch {masp_epoch}"); } From 847b53aa17f2fa355afdbe8fd0d481a4475b52ba Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 17:49:03 +0200 Subject: [PATCH 08/17] Tracks `masp_epoch_multiplier` parameter --- crates/apps_lib/src/config/genesis.rs | 2 ++ crates/apps_lib/src/config/genesis/chain.rs | 2 ++ crates/apps_lib/src/config/genesis/templates.rs | 4 ++++ crates/core/src/parameters.rs | 3 +++ crates/node/src/storage/mod.rs | 1 + crates/parameters/src/lib.rs | 13 +++++++++++++ crates/proof_of_stake/src/lib.rs | 1 + crates/state/src/lib.rs | 1 + genesis/localnet/parameters.toml | 2 +- genesis/starter/parameters.toml | 2 +- 10 files changed, 29 insertions(+), 2 deletions(-) diff --git a/crates/apps_lib/src/config/genesis.rs b/crates/apps_lib/src/config/genesis.rs index ae439e784ad..d6e5d970468 100644 --- a/crates/apps_lib/src/config/genesis.rs +++ b/crates/apps_lib/src/config/genesis.rs @@ -309,6 +309,8 @@ pub struct Parameters { pub implicit_vp_sha256: [u8; 32], /// Expected number of epochs per year (read only) pub epochs_per_year: u64, + /// How many epochs it takes to transition to the next masp epoch + pub masp_epoch_multiplier: u64, /// Maximum amount of signatures per transaction pub max_signatures_per_transaction: u8, /// Fee unshielding gas limit diff --git a/crates/apps_lib/src/config/genesis/chain.rs b/crates/apps_lib/src/config/genesis/chain.rs index a47c37d9de8..690d67909f0 100644 --- a/crates/apps_lib/src/config/genesis/chain.rs +++ b/crates/apps_lib/src/config/genesis/chain.rs @@ -302,6 +302,7 @@ impl Finalized { tx_allowlist, implicit_vp, epochs_per_year, + masp_epoch_multiplier, max_signatures_per_transaction, fee_unshielding_gas_limit, max_block_gas, @@ -346,6 +347,7 @@ impl Finalized { tx_allowlist, implicit_vp_code_hash, epochs_per_year, + masp_epoch_multiplier, max_proposal_bytes, max_signatures_per_transaction, fee_unshielding_gas_limit, diff --git a/crates/apps_lib/src/config/genesis/templates.rs b/crates/apps_lib/src/config/genesis/templates.rs index 7a3ae4764c1..db5ac13772e 100644 --- a/crates/apps_lib/src/config/genesis/templates.rs +++ b/crates/apps_lib/src/config/genesis/templates.rs @@ -293,6 +293,8 @@ pub struct ChainParams { pub implicit_vp: String, /// Expected number of epochs per year pub epochs_per_year: u64, + /// How many epochs it takes to transition to the next masp epoch + pub masp_epoch_multiplier: u64, /// Maximum number of signature per transaction pub max_signatures_per_transaction: u8, /// Max gas for block @@ -319,6 +321,7 @@ impl ChainParams { tx_allowlist, implicit_vp, epochs_per_year, + masp_epoch_multiplier, max_signatures_per_transaction, max_block_gas, fee_unshielding_gas_limit, @@ -364,6 +367,7 @@ impl ChainParams { tx_allowlist, implicit_vp, epochs_per_year, + masp_epoch_multiplier, max_signatures_per_transaction, max_block_gas, fee_unshielding_gas_limit, diff --git a/crates/core/src/parameters.rs b/crates/core/src/parameters.rs index 2f047ce6706..aaf2c294f81 100644 --- a/crates/core/src/parameters.rs +++ b/crates/core/src/parameters.rs @@ -46,6 +46,9 @@ pub struct Parameters { pub implicit_vp_code_hash: Option, /// Expected number of epochs per year (read only) pub epochs_per_year: u64, + /// The multiplier for masp epochs (it requires this amount of epochs to + /// transition to the next masp epoch) + pub masp_epoch_multiplier: u64, /// Maximum number of signature per transaction pub max_signatures_per_transaction: u8, /// Fee unshielding gas limit diff --git a/crates/node/src/storage/mod.rs b/crates/node/src/storage/mod.rs index 5e3c030cf86..49d37c81a82 100644 --- a/crates/node/src/storage/mod.rs +++ b/crates/node/src/storage/mod.rs @@ -173,6 +173,7 @@ mod tests { tx_allowlist: vec![], implicit_vp_code_hash: Default::default(), epochs_per_year: 365, + masp_epoch_multiplier: 2, max_signatures_per_transaction: 10, fee_unshielding_gas_limit: 0, minimum_gas_price: Default::default(), diff --git a/crates/parameters/src/lib.rs b/crates/parameters/src/lib.rs index edd9dcb5055..16b6772508b 100644 --- a/crates/parameters/src/lib.rs +++ b/crates/parameters/src/lib.rs @@ -74,6 +74,7 @@ where tx_allowlist, implicit_vp_code_hash, epochs_per_year, + masp_epoch_multiplier, max_signatures_per_transaction, minimum_gas_price, fee_unshielding_gas_limit, @@ -135,6 +136,9 @@ where let epochs_per_year_key = storage::get_epochs_per_year_key(); storage.write(&epochs_per_year_key, epochs_per_year)?; + let masp_epoch_multiplier_key = storage::get_masp_epoch_multiplier_key(); + storage.write(&masp_epoch_multiplier_key, masp_epoch_multiplier)?; + let max_signatures_per_transaction_key = storage::get_max_signatures_per_transaction_key(); storage.write( @@ -367,6 +371,13 @@ where .ok_or(ReadError::ParametersMissing) .into_storage_result()?; + // read masp epoch multiplier + let masp_epoch_multiplier_key = storage::get_masp_epoch_multiplier_key(); + let value = storage.read(&masp_epoch_multiplier_key)?; + let masp_epoch_multiplier: u64 = value + .ok_or(ReadError::ParametersMissing) + .into_storage_result()?; + // read the maximum signatures per transaction let max_signatures_per_transaction_key = storage::get_max_signatures_per_transaction_key(); @@ -407,6 +418,7 @@ where tx_allowlist, implicit_vp_code_hash: Some(implicit_vp_code_hash), epochs_per_year, + masp_epoch_multiplier, max_signatures_per_transaction, minimum_gas_price, fee_unshielding_gas_limit, @@ -452,6 +464,7 @@ where tx_allowlist: vec![], implicit_vp_code_hash: Default::default(), epochs_per_year: 365, + masp_epoch_multiplier: 2, max_signatures_per_transaction: 10, fee_unshielding_gas_limit: 0, minimum_gas_price: Default::default(), diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 749c1121d41..19b003541f4 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -2724,6 +2724,7 @@ pub mod test_utils { tx_allowlist: vec![], implicit_vp_code_hash: Some(Hash::default()), epochs_per_year: 10000000, + masp_epoch_multiplier: 2, max_signatures_per_transaction: 15, fee_unshielding_gas_limit: 10000, minimum_gas_price: BTreeMap::new(), diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs index b7ff50f8370..f62ab1c862f 100644 --- a/crates/state/src/lib.rs +++ b/crates/state/src/lib.rs @@ -758,6 +758,7 @@ mod tests { tx_allowlist: vec![], implicit_vp_code_hash: Some(Hash::zero()), epochs_per_year: 100, + masp_epoch_multiplier: 2, max_signatures_per_transaction: 15, fee_unshielding_gas_limit: 20_000, minimum_gas_price: BTreeMap::default(), diff --git a/genesis/localnet/parameters.toml b/genesis/localnet/parameters.toml index d3e3cb5973f..bef2c1ae2d9 100644 --- a/genesis/localnet/parameters.toml +++ b/genesis/localnet/parameters.toml @@ -19,7 +19,7 @@ implicit_vp = "vp_implicit" # Expected number of epochs per year (also sets the min duration of an epoch in seconds) epochs_per_year = 31_536_000 # The multiplier for masp epochs -masp_epoch_multiplier = 1 +masp_epoch_multiplier = 2 # Maximum number of signature per transaction max_signatures_per_transaction = 15 # Max gas for block diff --git a/genesis/starter/parameters.toml b/genesis/starter/parameters.toml index 79fe3cd8886..dfb01522d6d 100644 --- a/genesis/starter/parameters.toml +++ b/genesis/starter/parameters.toml @@ -19,7 +19,7 @@ implicit_vp = "vp_implicit" # Expected number of epochs per year (also sets the min duration of an epoch in seconds) epochs_per_year = 31_536_000 # The multiplier for masp epochs -masp_epoch_multiplier = 1 +masp_epoch_multiplier = 2 # Maximum number of signature per transaction max_signatures_per_transaction = 15 # Max gas for block From f774690fec497ac2ad4cb11c86f555312c68240e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 19:14:43 +0200 Subject: [PATCH 09/17] Helper function for masp epoch parameter. Fixes bug in masp epoch detection --- crates/core/src/masp.rs | 14 -------------- crates/namada/src/ledger/native_vp/masp.rs | 15 ++++----------- crates/parameters/src/lib.rs | 21 ++++++++++++++++----- crates/sdk/src/queries/shell.rs | 12 ++---------- crates/shielded_token/src/conversion.rs | 5 ++--- crates/state/src/wl_state.rs | 20 ++++++-------------- 6 files changed, 30 insertions(+), 57 deletions(-) diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index df32e3b32cb..cac7dba6cf3 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -83,20 +83,6 @@ impl MaspEpoch { Some(Self(self.0.checked_sub(1)?)) } - /// Check if the given epoch is also a new masp epoch based on the - /// multiplier provided - pub fn is_masp_new_epoch( - epoch: Epoch, - masp_epoch_multiplier: u64, - ) -> Result { - Ok(matches!( - Self::try_from_epoch(epoch, masp_epoch_multiplier)? - .0 - .checked_rem(masp_epoch_multiplier), - Some(Epoch(0)) - )) - } - /// Initialize a new masp epoch from the provided one #[cfg(any(test, feature = "testing"))] pub fn new(epoch: u64) -> Self { diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 0f3c6ef420f..6d53bbc094d 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -347,17 +347,10 @@ where tx_data: &BatchedTxRef<'_>, keys_changed: &BTreeSet, ) -> Result<()> { - let masp_epoch_multiplier = self - .ctx - .state - .read::( - &namada_parameters::storage::get_masp_epoch_multiplier_key(), - )? - .ok_or_else(|| { - Error::NativeVpError(native_vp::Error::SimpleMessage( - "Missing expected masp epoch multiplier parameter", - )) - })?; + let masp_epoch_multiplier = + namada_parameters::read_masp_epoch_multiplier_parameter( + self.ctx.state, + )?; let masp_epoch = MaspEpoch::try_from_epoch( self.ctx.get_block_epoch()?, masp_epoch_multiplier, diff --git a/crates/parameters/src/lib.rs b/crates/parameters/src/lib.rs index 16b6772508b..b8dbe12a883 100644 --- a/crates/parameters/src/lib.rs +++ b/crates/parameters/src/lib.rs @@ -286,6 +286,21 @@ where .into_storage_result() } +/// Read the the masp epoch multiplier parameter from store +pub fn read_masp_epoch_multiplier_parameter( + storage: &S, +) -> namada_storage::Result +where + S: StorageRead, +{ + // read multiplier + let masp_epoch_multiplier_key = storage::get_masp_epoch_multiplier_key(); + let epoch_multiplier = storage.read(&masp_epoch_multiplier_key)?; + epoch_multiplier + .ok_or(ReadError::ParametersMissing) + .into_storage_result() +} + /// Read the cost per unit of gas for the provided token pub fn read_gas_cost( storage: &S, @@ -372,11 +387,7 @@ where .into_storage_result()?; // read masp epoch multiplier - let masp_epoch_multiplier_key = storage::get_masp_epoch_multiplier_key(); - let value = storage.read(&masp_epoch_multiplier_key)?; - let masp_epoch_multiplier: u64 = value - .ok_or(ReadError::ParametersMissing) - .into_storage_result()?; + let masp_epoch_multiplier = read_masp_epoch_multiplier_parameter(storage)?; // read the maximum signatures per transaction let max_signatures_per_transaction_key = diff --git a/crates/sdk/src/queries/shell.rs b/crates/sdk/src/queries/shell.rs index 3bf8754f472..071939bdd2f 100644 --- a/crates/sdk/src/queries/shell.rs +++ b/crates/sdk/src/queries/shell.rs @@ -332,16 +332,8 @@ where H: 'static + StorageHasher + Sync, { let epoch = ctx.state.in_mem().last_epoch; - let masp_epoch_multiplier = ctx - .state - .read::( - &namada_parameters::storage::get_masp_epoch_multiplier_key(), - )? - .ok_or_else(|| { - namada_storage::Error::new_const( - "Missing expected masp epoch multiplier parameter", - ) - })?; + let masp_epoch_multiplier = + namada_parameters::read_masp_epoch_multiplier_parameter(ctx.state)?; MaspEpoch::try_from_epoch(epoch, masp_epoch_multiplier) .map_err(namada_storage::Error::new_const) } diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index aaffd56a05c..e41ba0db689 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -328,9 +328,8 @@ where calculate_masp_rewards_precision(storage, &native_token)?.0; // Reward all tokens according to above reward rates - let masp_epoch_multiplier = storage - .read::(¶meters::storage::get_masp_epoch_multiplier_key())? - .expect("masp epoch multiplier should properly decode"); + let masp_epoch_multiplier = + parameters::read_masp_epoch_multiplier_parameter(storage)?; let masp_epoch = MaspEpoch::try_from_epoch( storage.get_block_epoch()?, masp_epoch_multiplier, diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index 6c4a54fcd25..b5103ce723d 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -192,21 +192,13 @@ where /// Returns `true` if a new masp epoch has begun pub fn is_masp_new_epoch(&self, is_new_epoch: bool) -> StorageResult { - let masp_epoch_multiplier = self - .read::( - &namada_parameters::storage::get_masp_epoch_multiplier_key(), - )? - .ok_or_else(|| { - namada_storage::Error::new_const( - "Missing expected masp epoch multiplier parameter", - ) - })?; + let masp_epoch_multiplier = + namada_parameters::read_masp_epoch_multiplier_parameter(self)?; let masp_new_epoch = is_new_epoch - && MaspEpoch::is_masp_new_epoch( - self.in_mem.block.epoch, - masp_epoch_multiplier, - ) - .map_err(namada_storage::Error::new_const)?; + && matches!( + self.in_mem.block.epoch.checked_rem(masp_epoch_multiplier), + Some(Epoch(0)) + ); if masp_new_epoch { let masp_epoch = MaspEpoch::try_from_epoch( From 166d4925dcf5eb3bbe7d758c6115247ecce62e59 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 19:15:06 +0200 Subject: [PATCH 10/17] Adds test for masp epoch progression --- crates/node/src/shell/finalize_block.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index 7d179f50612..7adbcd17ba5 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -1711,6 +1711,27 @@ mod test_finalize_block { }); } + /// Test the correct transition to a new masp epoch + #[test] + fn test_masp_epoch_progression() { + let (mut shell, _broadcaster, _, _eth_control) = setup(); + + let masp_epoch_multiplier = + namada::ledger::parameters::read_masp_epoch_multiplier_parameter( + &shell.state, + ) + .unwrap(); + + assert_eq!(shell.state.get_block_epoch().unwrap(), Epoch::default()); + + for _ in 1..masp_epoch_multiplier { + shell.start_new_epoch(None); + assert!(!shell.state.is_masp_new_epoch(true).unwrap()); + } + shell.start_new_epoch(None); + assert!(shell.state.is_masp_new_epoch(true).unwrap()); + } + /// Test that the finalize block handler never commits changes directly to /// the DB. #[test] From 9f161ed69264d6b250e06fdd14581425183801f6 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 28 May 2024 12:45:40 +0200 Subject: [PATCH 11/17] Fixes integration tests --- crates/node/src/shell/testing/node.rs | 15 +++++ crates/tests/src/integration/masp.rs | 80 +++++++++++++-------------- 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/crates/node/src/shell/testing/node.rs b/crates/node/src/shell/testing/node.rs index 9e74e6689c1..d01a750edb6 100644 --- a/crates/node/src/shell/testing/node.rs +++ b/crates/node/src/shell/testing/node.rs @@ -380,6 +380,21 @@ impl MockNode { .0 } + pub fn next_masp_epoch(&mut self) -> Epoch { + let masp_epoch_multiplier = + namada::parameters::read_masp_epoch_multiplier_parameter( + &self.shell.lock().unwrap().state, + ) + .unwrap(); + let mut epoch = Epoch::default(); + + for _ in 0..masp_epoch_multiplier { + epoch = self.next_epoch(); + } + + epoch + } + pub fn native_token(&self) -> Address { let locked = self.shell.lock().unwrap(); locked.state.get_native_token().unwrap() diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 4912e3624a7..f9a47c6f6ba 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -37,7 +37,7 @@ fn masp_incentives() -> Result<()> { // not invalidated. let (mut node, _services) = setup::setup()?; // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // Send 1 BTC from Albert to PA run( &node, @@ -112,7 +112,7 @@ fn masp_incentives() -> Result<()> { assert!(captured.contains("nam: 0")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( @@ -159,7 +159,7 @@ fn masp_incentives() -> Result<()> { }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.031")); + assert!(captured.contains("nam: 0.063")); // Assert NAM balance at MASP pool is exclusively the // rewards from the shielded BTC @@ -179,10 +179,10 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.031")); + assert!(captured.contains("nam: 0.063")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( @@ -229,7 +229,7 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.09292")); + assert!(captured.contains("nam: 0.18887")); // Assert NAM balance at MASP pool is exclusively the // rewards from the shielded BTC @@ -249,10 +249,10 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.09331")); + assert!(captured.contains("nam: 0.18963")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // Send 0.001 ETH from Albert to PA(B) run( @@ -321,7 +321,7 @@ fn masp_incentives() -> Result<()> { assert!(captured.contains("nam: 0")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( @@ -368,7 +368,7 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.362747")); + assert!(captured.contains("nam: 0.725514")); // Assert NAM balance at MASP pool is an accumulation of // rewards from both the shielded BTC and shielded ETH @@ -388,10 +388,10 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.674354")); + assert!(captured.contains("nam: 1.358764")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // Send 0.001 ETH from SK(B) to Christel run( &node, @@ -441,7 +441,7 @@ fn masp_incentives() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("eth: 0")); - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -468,9 +468,9 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.725855")); + assert!(captured.contains("nam: 1.451732")); - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -497,10 +497,10 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1.595775")); + assert!(captured.contains("nam: 3.219616")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // Send 1 BTC from SK(A) to Christel run( @@ -568,7 +568,7 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1.113911")); + assert!(captured.contains("nam: 2.268662")); // Assert NAM balance at MASP pool is // the accumulation of rewards from the shielded assets (BTC and ETH) @@ -588,10 +588,10 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1.843775")); + assert!(captured.contains("nam: 3.723616")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -619,7 +619,7 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1.113911")); + assert!(captured.contains("nam: 2.268662")); // Assert NAM balance at VK(B) is the rewards dispensed earlier // (since VK(A) has no shielded assets, no further rewards should @@ -640,7 +640,7 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.725855")); + assert!(captured.contains("nam: 1.451732")); // Assert NAM balance at MASP pool is // the accumulation of rewards from the shielded assets (BTC and ETH) @@ -660,11 +660,11 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1.843775")); + assert!(captured.contains("nam: 3.723616")); // Wait till epoch boundary to prevent conversion expiry during transaction // construction - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -685,7 +685,7 @@ fn masp_incentives() -> Result<()> { "--token", NAM, "--amount", - "0.725855", + "1.451732", "--signing-keys", BERTHA_KEY, "--node", @@ -695,7 +695,7 @@ fn masp_incentives() -> Result<()> { node.assert_success(); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -716,7 +716,7 @@ fn masp_incentives() -> Result<()> { "--token", NAM, "--amount", - "1.113911", + "2.268662", "--signing-keys", ALBERT_KEY, "--node", @@ -795,7 +795,7 @@ fn masp_incentives() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.004009")); + assert!(captured.contains("nam: 0.003222")); Ok(()) } @@ -1683,7 +1683,7 @@ fn dynamic_assets() -> Result<()> { )?; node.assert_success(); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // Send 1 BTC from Albert to PA run( &node, @@ -1770,7 +1770,7 @@ fn dynamic_assets() -> Result<()> { } // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -1884,7 +1884,7 @@ fn dynamic_assets() -> Result<()> { assert!(captured.contains("nam: 0")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -1910,7 +1910,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.0303")); + assert!(captured.contains("nam: 0.06262")); // Assert BTC balance at VK(A) is still 2 let captured = CapturedOutput::of(|| { @@ -1945,7 +1945,7 @@ fn dynamic_assets() -> Result<()> { } // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -1990,7 +1990,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.07575")); + assert!(captured.contains("nam: 0.15655")); { // Stop decoding and distributing shielded rewards for BTC in next epoch @@ -2012,7 +2012,7 @@ fn dynamic_assets() -> Result<()> { } // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -2057,10 +2057,10 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.07575")); + assert!(captured.contains("nam: 0.15655")); // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -2104,7 +2104,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.07575")); + assert!(captured.contains("nam: 0.15655")); { // Start distributing shielded rewards for NAM in next epoch @@ -2120,7 +2120,7 @@ fn dynamic_assets() -> Result<()> { } // Wait till epoch boundary - node.next_epoch(); + node.next_masp_epoch(); // sync the shielded context run( &node, @@ -2164,7 +2164,7 @@ fn dynamic_assets() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 0.075825")); + assert!(captured.contains("nam: 0.156705")); Ok(()) } From 981b2bb2f8a789ec4703b5eda8a55c6b20b6f95f Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 28 May 2024 13:41:38 +0200 Subject: [PATCH 12/17] Fixes ibc e2e test --- crates/tests/src/e2e/ibc_tests.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 02236e6f0bc..707f266f66d 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -579,10 +579,13 @@ fn pgf_over_ibc_with_hermes() -> Result<()> { #[test] fn proposal_ibc_token_inflation() -> Result<()> { + const MASP_EPOCH_MULTIPLIER: u64 = 1; let update_genesis = |mut genesis: templates::All, base_dir: &_| { genesis.parameters.parameters.epochs_per_year = epochs_per_year_from_min_duration(60); + genesis.parameters.parameters.masp_epoch_multiplier = + MASP_EPOCH_MULTIPLIER; genesis.parameters.gov_params.min_proposal_grace_epochs = 3; genesis.parameters.ibc_params.default_mint_limit = Amount::max_signed(); @@ -647,8 +650,8 @@ fn proposal_ibc_token_inflation() -> Result<()> { )?; wait_for_packet_relay(&port_id_a, &channel_id_a, &test_a)?; - // wait the next epoch to dispense the rewrad - wait_epochs(&test_b, 1)?; + // wait the next masp epoch to dispense the reward + wait_epochs(&test_b, MASP_EPOCH_MULTIPLIER)?; // Check balances check_inflated_balance(&test_b)?; From d0211f4e043d3f2475a8b74ab88d9db6fd7e7285 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 28 May 2024 15:19:59 +0200 Subject: [PATCH 13/17] Fixes benchmarks --- crates/node/src/bench_utils.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index e49c1d184bc..92a4f1f650b 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -441,8 +441,12 @@ impl BenchShell { ) .unwrap(); - namada::token::conversion::update_allowed_conversions(&mut self.state) + if self.state.is_masp_new_epoch(true).unwrap() { + namada::token::conversion::update_allowed_conversions( + &mut self.state, + ) .unwrap(); + } } pub fn init_ibc_client_state(&mut self, addr_key: Key) -> ClientId { From e74b13345781635b0ceec5ddb010d612f8fc9fad Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 30 May 2024 13:52:44 +0200 Subject: [PATCH 14/17] Changes masp epoch multiplier in ibc proposal test --- crates/tests/src/e2e/ibc_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 707f266f66d..d3eb2e28ffc 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -579,7 +579,7 @@ fn pgf_over_ibc_with_hermes() -> Result<()> { #[test] fn proposal_ibc_token_inflation() -> Result<()> { - const MASP_EPOCH_MULTIPLIER: u64 = 1; + const MASP_EPOCH_MULTIPLIER: u64 = 2; let update_genesis = |mut genesis: templates::All, base_dir: &_| { genesis.parameters.parameters.epochs_per_year = From 58f5590943619a8996e6a2a368ed082c92d4bf58 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 27 May 2024 19:17:26 +0200 Subject: [PATCH 15/17] Changelog #3318 --- .changelog/unreleased/improvements/3318-masp-custom-epoch.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/3318-masp-custom-epoch.md diff --git a/.changelog/unreleased/improvements/3318-masp-custom-epoch.md b/.changelog/unreleased/improvements/3318-masp-custom-epoch.md new file mode 100644 index 00000000000..7b1d0ea050c --- /dev/null +++ b/.changelog/unreleased/improvements/3318-masp-custom-epoch.md @@ -0,0 +1,2 @@ +- Added a separate epoch tracker for masp to decouple its logic from the rest of + the protocol. ([\#3318](https://github.com/anoma/namada/pull/3318)) \ No newline at end of file From 96365b2953cc2a962f15dd69756ca500de3a707e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 31 May 2024 13:07:20 +0200 Subject: [PATCH 16/17] Fmt --- crates/sdk/src/masp.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index 68acbeef435..516a7ad6115 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -52,12 +52,11 @@ use masp_proofs::sapling::BatchValidator; use namada_core::address::Address; use namada_core::collections::{HashMap, HashSet}; use namada_core::dec::Dec; -use namada_core::masp::MaspEpoch; -use namada_core::masp::MaspTxRefs; pub use namada_core::masp::{ encode_asset_type, AssetData, BalanceOwner, ExtendedViewingKey, PaymentAddress, TransferSource, TransferTarget, }; +use namada_core::masp::{MaspEpoch, MaspTxRefs}; use namada_core::storage::{BlockHeight, TxIndex}; use namada_core::time::{DateTimeUtc, DurationSecs}; use namada_core::uint::Uint; From 786dce45463489cabaf3e46b748c57058be52710 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 1 Jun 2024 18:50:20 +0200 Subject: [PATCH 17/17] Const masp epoch functions --- crates/core/src/masp.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index cac7dba6cf3..c31734a803e 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -74,7 +74,7 @@ impl MaspEpoch { } /// Returns a 0 masp epoch - pub fn zero() -> Self { + pub const fn zero() -> Self { Self(Epoch(0)) } @@ -85,7 +85,7 @@ impl MaspEpoch { /// Initialize a new masp epoch from the provided one #[cfg(any(test, feature = "testing"))] - pub fn new(epoch: u64) -> Self { + pub const fn new(epoch: u64) -> Self { Self(Epoch(epoch)) } }