Merge branch 'brent/change-shielded-rewards' (#2424)

* origin/brent/change-shielded-rewards:
  changelog: add #2424
  fixes from comments
  better errors for `masp_reward_tokens`
  rework masp shielded set parameters
  reformat e2e test, fix pgf inflation computation

.changelog/unreleased/bug-fixes/2424-change-shielded-rewards.md

@@ -0,0 +1,2 @@
+- Fix the MASP VP to enable changes to the shielded set max reward rate for a
+  token. ([\#2424](https://github.com/anoma/namada/pull/2424))

.github/workflows/scripts/e2e.json

@@ -15,6 +15,7 @@
     "e2e::ledger_tests::proposal_offline": 21,
     "e2e::ledger_tests::pgf_governance_proposal": 320,
     "e2e::ledger_tests::proposal_submission": 200,
+    "e2e::ledger_tests::proposal_change_shielded_reward": 200,
     "e2e::ledger_tests::run_ledger": 5,
     "e2e::ledger_tests::run_ledger_load_state_and_reset": 23,
     "e2e::ledger_tests::test_namada_shuts_down_if_tendermint_dies": 2,

Makefile

@@ -169,6 +169,7 @@ test-e2e:
 	-Z unstable-options \
 	-- \
 	--test-threads=1 \
+	--nocapture \
 	-Z unstable-options --report-time
 
 # Run integration tests with pre-built MASP proofs

crates/apps/src/lib/client/rpc.rs

@@ -52,7 +52,7 @@ use namada::{state as storage, token};
 use namada_sdk::error::{
     is_pinned_error, Error, PinnedBalanceError, QueryError,
 };
-use namada_sdk::masp::{Conversions, MaspChange};
+use namada_sdk::masp::{Conversions, MaspChange, MaspTokenRewardData};
 use namada_sdk::proof_of_stake::types::ValidatorMetaData;
 use namada_sdk::rpc::{
     self, enriched_bonds_and_unbonds, query_epoch, TxResponse,
@@ -2516,9 +2516,25 @@ pub async fn query_masp_reward_tokens(context: &impl Namada) {
     let tokens = namada_sdk::rpc::query_masp_reward_tokens(context.client())
         .await
         .expect("The tokens that may earn MASP rewards should be defined");
-    display_line!(context.io(), "The following tokens may ear MASP rewards:");
-    for (alias, address) in tokens {
-        display_line!(context.io(), "{}: {}", alias, address);
+    display_line!(context.io(), "The following tokens may earn MASP rewards:");
+    for MaspTokenRewardData {
+        name,
+        address,
+        max_reward_rate,
+        kp_gain,
+        kd_gain,
+        locked_ratio_target,
+    } in tokens
+    {
+        display_line!(context.io(), "{}: {}", name, address);
+        display_line!(context.io(), "  Max reward rate: {}", max_reward_rate);
+        display_line!(context.io(), "  Kp gain: {}", kp_gain);
+        display_line!(context.io(), "  Kd gain: {}", kd_gain);
+        display_line!(
+            context.io(),
+            "  Locked ratio target: {}",
+            locked_ratio_target
+        );
     }
 }
 

crates/core/src/types/dec.rs

@@ -420,7 +420,7 @@ impl Mul<Amount> for Dec {
         if !self.is_negative() {
             (rhs * self.0.abs()) / 10u64.pow(POS_DECIMAL_PRECISION as u32)
         } else {
-            panic!("aaa");
+            panic!("Dec is negative and cannot produce a valid Amount output");
         }
     }
 }

crates/governance/src/pgf/inflation.rs

@@ -1,7 +1,6 @@
 //! PGF lib code.
 
 use namada_core::types::address::Address;
-use namada_core::types::dec::Dec;
 use namada_core::types::token;
 use namada_parameters::storage as params_storage;
 use namada_state::{
@@ -34,14 +33,12 @@ where
     let epochs_per_year: u64 = storage
         .read(&params_storage::get_epochs_per_year_key())?
         .expect("Epochs per year should exist in storage");
-    let total_tokens: token::Amount = storage
+    let total_supply: token::Amount = storage
         .read(&minted_balance_key(&staking_token))?
-        .expect("Total NAM balance should exist in storage");
+        .expect("Total native token balance should exist in storage");
 
-    let pgf_pd_rate =
-        pgf_parameters.pgf_inflation_rate / Dec::from(epochs_per_year);
-    let pgf_inflation = Dec::from(total_tokens) * pgf_pd_rate;
-    let pgf_inflation_amount = token::Amount::from(pgf_inflation);
+    let pgf_inflation_amount =
+        (pgf_parameters.pgf_inflation_rate * total_supply) / epochs_per_year;
 
     credit_tokens(
         storage,
@@ -51,8 +48,10 @@ where
     )?;
 
     tracing::info!(
-        "Minting {} tokens for PGF rewards distribution into the PGF account.",
-        pgf_inflation_amount.to_string_native()
+        "Minting {} tokens for PGF rewards distribution into the PGF account \
+         (total supply {}).",
+        pgf_inflation_amount.to_string_native(),
+        total_supply.to_string_native()
     );
 
     let mut pgf_fundings = get_payments(storage)?;
@@ -95,30 +94,35 @@ where
 
     // Pgf steward inflation
     let stewards = get_stewards(storage)?;
-    let pgf_stewards_pd_rate =
-        pgf_parameters.stewards_inflation_rate / Dec::from(epochs_per_year);
-    let pgf_steward_inflation = Dec::from(total_tokens) * pgf_stewards_pd_rate;
+    let pgf_steward_inflation = (pgf_parameters.stewards_inflation_rate
+        * total_supply)
+        / epochs_per_year;
 
     for steward in stewards {
         for (address, percentage) in steward.reward_distribution {
-            let pgf_steward_reward = pgf_steward_inflation
-                .checked_mul(&percentage)
-                .unwrap_or_default();
-            let reward_amount = token::Amount::from(pgf_steward_reward);
+            let pgf_steward_reward = percentage * pgf_steward_inflation;
 
-            if credit_tokens(storage, &staking_token, &address, reward_amount)
-                .is_ok()
+            if credit_tokens(
+                storage,
+                &staking_token,
+                &address,
+                pgf_steward_reward,
+            )
+            .is_ok()
             {
                 tracing::info!(
-                    "Minting {} tokens for steward {}.",
-                    reward_amount.to_string_native(),
+                    "Minting {} tokens for steward {} (total supply {})..",
+                    pgf_steward_reward.to_string_native(),
                     address,
+                    total_supply.to_string_native()
                 );
             } else {
                 tracing::warn!(
-                    "Failed minting {} tokens for steward {}.",
-                    reward_amount.to_string_native(),
+                    "Failed minting {} tokens for steward {} (total supply \
+                     {})..",
+                    pgf_steward_reward.to_string_native(),
                     address,
+                    total_supply.to_string_native()
                 );
             }
         }

crates/namada/src/ledger/governance/mod.rs

@@ -591,7 +591,7 @@ where
         match tx.data() {
             Some(data) => is_proposal_accepted(&self.ctx.pre(), data.as_ref())
                 .map_err(Error::NativeVpError),
-            None => Ok(true),
+            None => Ok(false),
         }
     }
 

crates/namada/src/ledger/native_vp/multitoken.rs

@@ -2,6 +2,8 @@
 
 use std::collections::{BTreeSet, HashMap};
 
+use namada_governance::is_proposal_accepted;
+use namada_token::storage_key::is_any_token_parameter_key;
 use namada_tx::Tx;
 use namada_vp_env::VpEnv;
 use thiserror::Error;
@@ -47,7 +49,7 @@ where
 
     fn validate_tx(
         &self,
-        _tx: &Tx,
+        tx_data: &Tx,
         keys_changed: &BTreeSet<Key>,
         verifiers: &BTreeSet<Address>,
     ) -> Result<bool> {
@@ -79,6 +81,8 @@ where
                 if !self.is_valid_minter(token, verifiers)? {
                     return Ok(false);
                 }
+            } else if is_any_token_parameter_key(key).is_some() {
+                return self.is_valid_parameter(tx_data);
             } else if key.segments.get(0)
                 == Some(
                     &Address::Internal(InternalAddress::Multitoken).to_db_key(),
@@ -133,6 +137,15 @@ where
             }
         }
     }
+
+    /// Return if the parameter change was done via a governance proposal
+    pub fn is_valid_parameter(&self, tx: &Tx) -> Result<bool> {
+        match tx.data() {
+            Some(data) => is_proposal_accepted(&self.ctx.pre(), data.as_ref())
+                .map_err(Error::NativeVpError),
+            None => Ok(false),
+        }
+    }
 }
 
 #[cfg(test)]

crates/namada/src/ledger/pgf/mod.rs

@@ -117,7 +117,7 @@ where
         match tx.data() {
             Some(data) => is_proposal_accepted(&self.ctx.pre(), data.as_ref())
                 .map_err(Error::NativeVpError),
-            None => Ok(true),
+            None => Ok(false),
         }
     }
 }

crates/namada/src/vm/host_env.rs

@@ -17,6 +17,7 @@ use namada_gas::{
 };
 use namada_state::write_log::{self, WriteLog};
 use namada_state::{self, ResultExt, State, StorageHasher};
+use namada_token::storage_key::is_any_token_parameter_key;
 use namada_tx::data::TxSentinel;
 use namada_tx::Tx;
 use thiserror::Error;
@@ -906,6 +907,8 @@ where
     // Get the token if the key is a balance or minter key
     let token = if let Some([token, _]) = is_any_token_balance_key(key) {
         Some(token)
+    } else if let Some(token) = is_any_token_parameter_key(key) {
+        Some(token)
     } else {
         is_any_minted_balance_key(key).or_else(|| is_any_minter_key(key))
     };

crates/sdk/src/masp.rs

@@ -52,6 +52,7 @@ use masp_proofs::bls12_381::Bls12;
 use masp_proofs::prover::LocalTxProver;
 use masp_proofs::sapling::SaplingVerificationContext;
 use namada_core::types::address::{Address, MASP};
+use namada_core::types::dec::Dec;
 use namada_core::types::ibc::IbcShieldedTransfer;
 use namada_core::types::masp::{
     encode_asset_type, AssetData, BalanceOwner, ExtendedViewingKey,
@@ -122,6 +123,17 @@ pub struct ShieldedTransfer {
     pub epoch: Epoch,
 }
 
+/// Shielded pool data for a token
+#[derive(BorshSerialize, BorshDeserialize)]
+pub struct MaspTokenRewardData {
+    pub name: String,
+    pub address: Address,
+    pub max_reward_rate: Dec,
+    pub kp_gain: Dec,
+    pub kd_gain: Dec,
+    pub locked_ratio_target: Dec,
+}
+
 #[cfg(feature = "testing")]
 #[derive(Clone, Copy, Debug)]
 enum LoadOrSaveProofs {

crates/sdk/src/queries/shell.rs

@@ -10,6 +10,7 @@ use masp_primitives::sapling::Node;
 use namada_account::{Account, AccountPublicKeysMap};
 use namada_core::hints;
 use namada_core::types::address::Address;
+use namada_core::types::dec::Dec;
 use namada_core::types::hash::Hash;
 use namada_core::types::storage::{
     self, BlockHeight, BlockResults, Epoch, KeySeg, PrefixValue,
@@ -26,6 +27,7 @@ use crate::events::{Event, EventType};
 use crate::ibc::core::host::types::identifiers::{
     ChannelId, ClientId, PortId, Sequence,
 };
+use crate::masp::MaspTokenRewardData;
 use crate::queries::types::{RequestCtx, RequestQuery};
 use crate::queries::{require_latest_height, EncodedResponseQuery};
 use crate::tendermint::merkle::proof::ProofOps;
@@ -87,7 +89,7 @@ router! {SHELL,
     ( "conversions" ) -> BTreeMap<AssetType, ConversionWithoutPath> = read_conversions,
 
     // Conversion state access - read conversion
-    ( "masp_reward_tokens" ) -> BTreeMap<String, Address> = masp_reward_tokens,
+    ( "masp_reward_tokens" ) -> Vec<MaspTokenRewardData> = masp_reward_tokens,
 
     // Block results access - read bit-vec
     ( "results" ) -> Vec<BlockResults> = read_results,
@@ -221,12 +223,80 @@ where
 /// Query to read the tokens that earn masp rewards.
 fn masp_reward_tokens<D, H, V, T>(
     ctx: RequestCtx<'_, D, H, V, T>,
-) -> namada_storage::Result<BTreeMap<String, Address>>
+) -> namada_storage::Result<Vec<MaspTokenRewardData>>
 where
     D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
     H: 'static + StorageHasher + Sync,
 {
-    Ok(ctx.wl_storage.storage.conversion_state.tokens.clone())
+    let tokens = ctx.wl_storage.storage.conversion_state.tokens.clone();
+    let mut data = Vec::<MaspTokenRewardData>::new();
+    for (name, token) in tokens {
+        let max_reward_rate = ctx
+            .wl_storage
+            .read::<Dec>(&namada_token::storage_key::masp_max_reward_rate_key(
+                &token,
+            ))?
+            .ok_or_else(|| {
+                namada_storage::Error::new(std::io::Error::new(
+                    std::io::ErrorKind::NotFound,
+                    format!(
+                        "Did not find max reward rate set for token {} ({})",
+                        &name, &token
+                    ),
+                ))
+            })?;
+        let kd_gain = ctx
+            .wl_storage
+            .read::<Dec>(&namada_token::storage_key::masp_kd_gain_key(&token))?
+            .ok_or_else(|| {
+                namada_storage::Error::new(std::io::Error::new(
+                    std::io::ErrorKind::NotFound,
+                    format!(
+                        "Did not find kd gain set for token {} ({})",
+                        &name, &token
+                    ),
+                ))
+            })?;
+        let kp_gain = ctx
+            .wl_storage
+            .read::<Dec>(&namada_token::storage_key::masp_kp_gain_key(&token))?
+            .ok_or_else(|| {
+                namada_storage::Error::new(std::io::Error::new(
+                    std::io::ErrorKind::NotFound,
+                    format!(
+                        "Did not find kp gain set for token {} ({})",
+                        &name, &token
+                    ),
+                ))
+            })?;
+        let locked_ratio_target = ctx
+            .wl_storage
+            .read::<Dec>(
+                &namada_token::storage_key::masp_locked_ratio_target_key(
+                    &token,
+                ),
+            )?
+            .ok_or_else(|| {
+                namada_storage::Error::new(std::io::Error::new(
+                    std::io::ErrorKind::NotFound,
+                    format!(
+                        "Did not find target locked ratio set for token {} \
+                         ({})",
+                        &name, &token
+                    ),
+                ))
+            })?;
+
+        data.push(MaspTokenRewardData {
+            name,
+            address: token,
+            max_reward_rate,
+            kp_gain,
+            kd_gain,
+            locked_ratio_target,
+        });
+    }
+    Ok(data)
 }
 
 fn epoch<D, H, V, T>(

crates/sdk/src/rpc.rs

@@ -44,6 +44,7 @@ use crate::error::{EncodingError, Error, QueryError, TxSubmitError};
 use crate::events::Event;
 use crate::internal_macros::echo_error;
 use crate::io::Io;
+use crate::masp::MaspTokenRewardData;
 use crate::queries::vp::pos::EnrichedBondsAndUnbondsDetails;
 use crate::queries::{Client, RPC};
 use crate::tendermint::block::Height;
@@ -324,7 +325,7 @@ pub async fn query_conversions<C: crate::queries::Client + Sync>(
 /// Query to read the tokens that earn masp rewards.
 pub async fn query_masp_reward_tokens<C: crate::queries::Client + Sync>(
     client: &C,
-) -> Result<BTreeMap<String, Address>, Error> {
+) -> Result<Vec<MaspTokenRewardData>, Error> {
     convert_response::<C, _>(RPC.shell().masp_reward_tokens(client).await)
 }