diff --git a/.changelog/unreleased/SDK/4196-rewards-at-past-epoch.md b/.changelog/unreleased/SDK/4196-rewards-at-past-epoch.md new file mode 100644 index 0000000000..d312fb757f --- /dev/null +++ b/.changelog/unreleased/SDK/4196-rewards-at-past-epoch.md @@ -0,0 +1,2 @@ +- Allow querying PoS rewards at a specified epoch + ([\#4196](https://github.com/anoma/namada/pull/4196)) \ No newline at end of file diff --git a/.vscode/launch.json b/.vscode/launch.json index 7c13fc3f35..f8984e9e0d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -229,6 +229,25 @@ "args": [], "cwd": "${workspaceFolder}" }, + { + "type": "lldb", + "request": "launch", + "name": "Debug unit tests in library 'namada_sdk'", + "cargo": { + "args": [ + "test", + "--no-run", + "--lib", + "--package=namada_sdk" + ], + "filter": { + "name": "namada_sdk", + "kind": "lib" + } + }, + "args": [], + "cwd": "${workspaceFolder}" + }, { "type": "lldb", "request": "launch", diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 90a1066b98..f7f4d3d3c5 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -2049,7 +2049,7 @@ pub mod cmds { fn def() -> App { App::new(Self::CMD) .about(wrap!( - "Query the latest rewards available to claim for a given \ + "Query the rewards available to claim for a given \ delegation (or self-bond)." )) .add_args::>() @@ -7309,6 +7309,7 @@ pub mod args { query: self.query.to_sdk(ctx)?, validator: ctx.borrow_chain_or_exit().get(&self.validator), source: self.source.map(|x| ctx.borrow_chain_or_exit().get(&x)), + epoch: self.epoch, }) } } @@ -7318,10 +7319,12 @@ pub mod args { let query = Query::parse(matches); let source = SOURCE_OPT.parse(matches); let validator = VALIDATOR.parse(matches); + let epoch = EPOCH.parse(matches); Self { query, source, validator, + epoch, } } @@ -7336,6 +7339,14 @@ pub mod args { "Validator address for the rewards query." )), ) + .arg(EPOCH.def().help(wrap!( + "The epoch at which to query (corresponding to the last \ + committed block, if not specified). \ + \ + Note: when querying by epoch, this returns the accumulated \ + rewards that were available to claim at the start of the \ + epoch." + ))) } } diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 22e1cafd62..5338376f44 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -1503,8 +1503,11 @@ pub async fn query_rewards( client: &C, source: &Option
, validator: &Address, + epoch: &Option, ) -> token::Amount { - unwrap_sdk_result(rpc::query_rewards(client, source, validator).await) + unwrap_sdk_result( + rpc::query_rewards(client, source, validator, epoch).await, + ) } /// Query token total supply. @@ -1920,12 +1923,18 @@ pub async fn query_and_print_rewards( context: &N, args: args::QueryRewards, ) { - let (source, validator) = (args.source, args.validator); + let (source, validator, epoch) = (args.source, args.validator, args.epoch); - let rewards = query_rewards(context.client(), &source, &validator).await; + let rewards = + query_rewards(context.client(), &source, &validator, &epoch).await; display_line!( context.io(), - "Current rewards available for claim: {} NAM", + "{}: {} NAM", + epoch + .map(|e| format!("Rewards at epoch {}", e)) + .unwrap_or_else( + || "Current rewards available for claim".to_string() + ), rewards.to_string_native() ); } diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 754633a36c..3777654e1d 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -2545,6 +2545,8 @@ pub struct QueryRewards { pub source: Option, /// Address of the validator pub validator: C::Address, + /// Epoch in which to find rewards + pub epoch: Option, } /// Query PoS delegations diff --git a/crates/sdk/src/queries/shell.rs b/crates/sdk/src/queries/shell.rs index f515296404..0d6abd13f7 100644 --- a/crates/sdk/src/queries/shell.rs +++ b/crates/sdk/src/queries/shell.rs @@ -429,7 +429,7 @@ where /// borsh-encoded types, it is safe to check `data.is_empty()` to see if the /// value was found, except for unit - see `fn query_storage_value` in /// `apps/src/lib/client/rpc.rs` for unit type handling via `storage_has_key`. -fn storage_value( +pub fn storage_value( ctx: RequestCtx<'_, D, H, V, T>, request: &RequestQuery, storage_key: storage::Key, diff --git a/crates/sdk/src/queries/vp/pos.rs b/crates/sdk/src/queries/vp/pos.rs index 5fcf4e06fd..5f3609bb84 100644 --- a/crates/sdk/src/queries/vp/pos.rs +++ b/crates/sdk/src/queries/vp/pos.rs @@ -13,6 +13,7 @@ use namada_proof_of_stake::parameters::PosParams; use namada_proof_of_stake::queries::{ find_delegation_validators, find_delegations, }; +use namada_proof_of_stake::rewards::read_rewards_counter; use namada_proof_of_stake::slashing::{ find_all_enqueued_slashes, find_all_slashes, }; @@ -34,12 +35,13 @@ use namada_proof_of_stake::types::{ WeightedValidator, }; use namada_proof_of_stake::{bond_amount, query_reward_tokens}; -use namada_state::{DBIter, KeySeg, StorageHasher, DB}; +use namada_state::{DBIter, KeySeg, StorageHasher, StorageRead, DB}; use namada_storage::collections::lazy_map; -use namada_storage::OptionExt; +use namada_storage::{OptionExt, ResultExt}; use crate::governance; use crate::queries::types::RequestCtx; +use crate::queries::{shell, RequestQuery}; // PoS validity predicate queries router! {POS, @@ -103,7 +105,7 @@ router! {POS, ( "bond" / [source: Address] / [validator: Address] / [epoch: opt Epoch] ) -> token::Amount = bond, - ( "rewards" / [validator: Address] / [source: opt Address] ) + ( "rewards" / [validator: Address] / [source: opt Address] / [epoch: opt Epoch] ) -> token::Amount = rewards, ( "bond_with_slashing" / [source: Address] / [validator: Address] / [epoch: opt Epoch] ) @@ -588,18 +590,86 @@ fn rewards( ctx: RequestCtx<'_, D, H, V, T>, validator: Address, source: Option
, + epoch: Option, ) -> namada_storage::Result where D: 'static + DB + for<'iter> DBIter<'iter> + Sync, H: 'static + StorageHasher + Sync, { - let current_epoch = ctx.state.in_mem().last_epoch; - query_reward_tokens::<_, governance::Store<_>>( + let reward_tokens = query_reward_tokens::<_, governance::Store<_>>( ctx.state, source.as_ref(), &validator, - current_epoch, - ) + epoch.unwrap_or(ctx.state.in_mem().last_epoch), + )?; + + match epoch { + None => Ok(reward_tokens), + Some(epoch) => { + // When querying by epoch, since query_reward_tokens includes + // rewards_counter not based on epoch, we need to + // subtract it and instead add the rewards_counter from + // the height of the epoch we are querying. + let source = source.unwrap_or_else(|| validator.clone()); + let rewards_counter_last_epoch = + read_rewards_counter(ctx.state, &source, &validator)?; + + let rewards_counter_at_epoch = + get_rewards_counter_at_epoch(ctx, &source, &validator, epoch)?; + + // Add before subtracting because Amounts are unsigned + checked!( + reward_tokens + rewards_counter_at_epoch + - rewards_counter_last_epoch + ) + .into_storage_result() + } + } +} + +fn get_rewards_counter_at_epoch( + ctx: RequestCtx<'_, D, H, V, T>, + source: &Address, + validator: &Address, + epoch: Epoch, +) -> namada_storage::Result +where + D: 'static + DB + for<'iter> DBIter<'iter> + Sync, + H: 'static + StorageHasher + Sync, +{ + // Do this first so that we return an error if an invalid epoch is requested + let queried_height = ctx + .state + .get_epoch_start_height(epoch)? + .ok_or(namada_storage::Error::new_const("Epoch not found"))?; + + let storage_key = namada_proof_of_stake::storage_key::rewards_counter_key( + source, validator, + ); + + // Shortcut: avoid costly lookup of non-existent storage key in history + // by first checking to see if it currently exists in memory before + // querying by height. + // TODO: this might not be valid if rewards have been claimed in the current + // epoch? (because the rewards_counter would be removed from + // storage until the next epoch) + if !ctx.state.has_key(&storage_key)? { + return Ok(token::Amount::zero()); + } + + let query = RequestQuery { + height: queried_height.try_into().into_storage_result()?, + prove: false, + data: Default::default(), + path: Default::default(), + }; + + let value = shell::storage_value(ctx, &query, storage_key)?; + if value.data.is_empty() { + Ok(token::Amount::zero()) + } else { + token::Amount::try_from_slice(&value.data).into_storage_result() + } } fn bonds_and_unbonds( @@ -834,7 +904,11 @@ fn enrich_bonds_and_unbonds( mod test { use super::*; use crate::queries::testing::TestClient; - use crate::queries::{RequestCtx, RequestQuery, Router}; + use crate::queries::{RequestCtx, RequestQuery, Router, RPC}; + use namada_core::chain::Epoch; + use namada_core::token; + use namada_core::{address, storage}; + use namada_state::StorageWrite; #[tokio::test] async fn test_validator_by_tm_addr_sanitized_input() { @@ -861,11 +935,307 @@ mod test { }; let result = POS.handle(ctx, &request); assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("Invalid Tendermint address") - ) + assert!(result + .unwrap_err() + .to_string() + .contains("Invalid Tendermint address")) + } + + // Helpers for test_rewards_query + mod helpers { + use super::*; + + pub fn init_validator( + client: &mut TestClient, + ) -> (Address, namada_proof_of_stake::PosParams) { + let genesis_validator = + namada_proof_of_stake::test_utils::get_dummy_genesis_validator( + ); + let validator_address = genesis_validator.address.clone(); + + let params = + namada_proof_of_stake::test_utils::test_init_genesis::< + _, + namada_parameters::Store<_>, + governance::Store<_>, + namada_token::Store<_>, + >( + &mut client.state, + namada_proof_of_stake::OwnedPosParams::default(), + std::iter::once(genesis_validator), + Epoch(0), + ) + .expect("Test initialization failed"); + + (validator_address, params) + } + + pub fn setup_delegator( + client: &mut TestClient, + validator_address: &Address, + bond_amount: token::Amount, + ) -> Address { + let delegator = address::testing::established_address_2(); + + // Credit tokens to delegator + let native_token = client.state.get_native_token().unwrap(); + StorageWrite::write( + &mut client.state, + &namada_token::storage_key::balance_key( + &native_token, + &delegator, + ), + bond_amount, + ) + .expect("Credit tokens failed"); + + // Bond tokens from delegator to validator + namada_proof_of_stake::bond_tokens::< + _, + governance::Store<_>, + namada_token::Store<_>, + >( + &mut client.state, + Some(&delegator), + validator_address, + bond_amount, + Epoch(1), + Some(0), + ) + .expect("Bonding tokens failed"); + + delegator + } + + pub fn init_state( + client: &mut TestClient, + bond_amount: token::Amount, + ) -> (Address, Address, token::Amount) { + let (validator, _params) = init_validator(client); + let delegator = setup_delegator(client, &validator, bond_amount); + + // Initialize the predecessor epochs + client + .state + .in_mem_mut() + .block + .pred_epochs + .new_epoch(0.into()); + + (validator, delegator, bond_amount) + } + + pub fn advance_epoch( + client: &mut TestClient, + validator_delegator: &(Address, Address), + reward: &(Option, Option), + ) -> Epoch { + let (validator, delegator) = validator_delegator; + let (validator_reward, delegator_reward) = reward; + let current_epoch = client.state.in_mem().last_epoch; + let next_epoch = current_epoch.next(); + let height = client.state.in_mem().block.height; + + // Advance block height and epoch + let next_height = height + 1; + client + .state + .in_mem_mut() + .begin_block(next_height) + .expect("Test failed"); + client.state.in_mem_mut().block.epoch = next_epoch; + client.state.in_mem_mut().block.height = next_height; + client + .state + .in_mem_mut() + .block + .pred_epochs + .new_epoch(next_height); + + // Add rewards + if let Some(rewards_amount) = delegator_reward { + namada_proof_of_stake::rewards::add_rewards_to_counter( + &mut client.state, + delegator, + validator, + *rewards_amount, + ) + .expect("Adding delegator rewards failed"); + } + + if let Some(rewards_amount) = validator_reward { + namada_proof_of_stake::rewards::add_rewards_to_counter( + &mut client.state, + validator, + validator, + *rewards_amount, + ) + .expect("Adding validator rewards failed"); + } + + client.state.commit_block().expect("Test failed"); + + next_epoch + } + } + + #[tokio::test] + async fn test_rewards_query() { + // Initialize test client + let mut client = TestClient::new(RPC); + + // We will be reusing this route frequently, so alias it here + let pos = RPC.vp().pos(); + + // Set up validator + let (validator, delegator, _params) = + helpers::init_state(&mut client, token::Amount::native_whole(100)); + + let bond = (validator.clone(), delegator.clone()); + let reward = (None, None); + + // Advance to next epoch without rewards + let epoch = helpers::advance_epoch(&mut client, &bond, &reward); + assert_eq!(epoch, Epoch(1)); + + // Test querying rewards (should be 0) + let result = pos + .rewards(&client, &validator, &Some(delegator.clone()), &None) + .await + .expect("Rewards query failed"); + assert_eq!(result, token::Amount::zero()); + + let result = pos + .rewards( + &client, + &validator, + &Some(delegator.clone()), + &Some(epoch), + ) + .await + .expect("Rewards query failed"); + assert_eq!(result, token::Amount::zero()); + + let result = pos + .rewards(&client, &validator, &None, &None) + .await + .expect("Rewards query failed"); + assert_eq!(result, token::Amount::zero()); + + // Advance to next epoch with some rewards + let val_reward_epoch_2 = token::Amount::native_whole(5); + let del_reward_epoch_2 = token::Amount::native_whole(7); + let reward_epoch_2 = + (Some(val_reward_epoch_2), Some(del_reward_epoch_2)); + let epoch = helpers::advance_epoch(&mut client, &bond, &reward_epoch_2); + assert_eq!(epoch, Epoch(2)); + + // Query latest rewards for delegator + let result = pos + .rewards(&client, &validator, &Some(delegator.clone()), &None) + .await + .expect("Rewards query failed"); + assert_eq!(result, del_reward_epoch_2); + + // Query latest rewards for validator + let result = pos + .rewards(&client, &validator, &None, &None) + .await + .expect("Rewards query failed"); + assert_eq!(result, val_reward_epoch_2); + + // Query delegator rewards at specific epoch + let result = pos + .rewards( + &client, + &validator, + &Some(delegator.clone()), + &Some(epoch), + ) + .await + .expect("Rewards query failed"); + assert_eq!(result, del_reward_epoch_2); + + // Query validator rewards at specific epoch + let result = pos + .rewards(&client, &validator, &None, &Some(epoch)) + .await + .expect("Rewards query failed"); + assert_eq!(result, val_reward_epoch_2); + + // Ensure no rewards at previous epoch + let result = pos + .rewards( + &client, + &validator, + &Some(delegator.clone()), + &epoch.prev(), + ) + .await + .expect("Rewards query failed"); + assert_eq!(result, token::Amount::zero()); + + let result = pos + .rewards(&client, &validator, &None, &epoch.prev()) + .await + .expect("Rewards query failed"); + assert_eq!(result, token::Amount::zero()); + + // Advance to another epoch with more rewards + let val_reward_epoch_3 = token::Amount::native_whole(9); + let del_reward_epoch_3 = token::Amount::native_whole(11); + let reward_epoch_3 = + (Some(val_reward_epoch_3), Some(del_reward_epoch_3)); + let epoch = helpers::advance_epoch(&mut client, &bond, &reward_epoch_3); + assert_eq!(epoch, Epoch(3)); + + // Query latest rewards + let result = pos + .rewards(&client, &validator, &Some(delegator.clone()), &None) + .await + .expect("Rewards query failed"); + assert_eq!(result, del_reward_epoch_3 + del_reward_epoch_2); + + let result = pos + .rewards(&client, &validator, &None, &None) + .await + .expect("Rewards query failed"); + assert_eq!(result, val_reward_epoch_3 + val_reward_epoch_2); + + // Query rewards at specific epoch + let result = pos + .rewards( + &client, + &validator, + &Some(delegator.clone()), + &Some(epoch), + ) + .await + .expect("Rewards query failed"); + assert_eq!(result, del_reward_epoch_3 + del_reward_epoch_2); + + let result = pos + .rewards(&client, &validator, &None, &Some(epoch)) + .await + .expect("Rewards query failed"); + assert_eq!(result, val_reward_epoch_3 + val_reward_epoch_2); + + // Query at previous epoch + let result = pos + .rewards( + &client, + &validator, + &Some(delegator.clone()), + &epoch.prev(), + ) + .await + .expect("Rewards query failed"); + assert_eq!(result, del_reward_epoch_2); + + let result = pos + .rewards(&client, &validator, &None, &epoch.prev()) + .await + .expect("Rewards query failed"); + assert_eq!(result, val_reward_epoch_2); } } diff --git a/crates/sdk/src/rpc.rs b/crates/sdk/src/rpc.rs index 362048a564..c7e0ef1466 100644 --- a/crates/sdk/src/rpc.rs +++ b/crates/sdk/src/rpc.rs @@ -872,9 +872,13 @@ pub async fn query_rewards( client: &C, source: &Option
, validator: &Address, + epoch: &Option, ) -> Result { convert_response::( - RPC.vp().pos().rewards(client, validator, source).await, + RPC.vp() + .pos() + .rewards(client, validator, source, epoch) + .await, ) } @@ -1682,16 +1686,15 @@ pub async fn osmosis_denom_from_namada_denom( if nam_denom.trace_path.is_empty() { // Namada native asset - let address = nam_denom - .base_denom - .as_str() - .parse::
() - .map_err(|err| { - Error::Encode(EncodingError::Decoding(format!( + let address = + nam_denom.base_denom.as_str().parse::
().map_err( + |err| { + Error::Encode(EncodingError::Decoding(format!( "Failed to parse base denom {} as Namada address: {err}", nam_denom.base_denom ))) - })?; + }, + )?; // validate that the base denom is not another ibc token if matches!(&address, Address::Internal(InternalAddress::IbcToken(_))) { diff --git a/crates/storage/src/mockdb.rs b/crates/storage/src/mockdb.rs index 272e177923..6b0161b4e8 100644 --- a/crates/storage/src/mockdb.rs +++ b/crates/storage/src/mockdb.rs @@ -381,14 +381,39 @@ impl DB for MockDB { fn read_subspace_val_with_height( &self, key: &Key, - _height: BlockHeight, - _last_height: BlockHeight, + height: BlockHeight, + last_height: BlockHeight, ) -> Result>> { - tracing::warn!( - "read_subspace_val_with_height is not implemented, will read \ - subspace value from latest height" - ); - self.read_subspace_val(key) + if height == last_height { + self.read_subspace_val(key) + } else { + // Quick-n-dirty implementation for reading subspace value at height: + // - See if there are any diffs between height+1..last_height. + // - If so, the first one will provide the value we want as its old value. + // - If not, we can just read the value at the latest height. + for h in (height.0 + 1)..=last_height.0 { + let old_diff = self.read_diffs_val(key, h.into(), true)?; + let new_diff = self.read_diffs_val(key, h.into(), false)?; + + match (old_diff, new_diff) { + (Some(old_diff), Some(_)) | (Some(old_diff), None) => { + // If there is an old diff, it contains the value at the requested height. + return Ok(Some(old_diff)); + } + (None, Some(_)) => { + // If there is a new diff but no old diff, there was + // no value at the requested height. + return Ok(None); + } + (None, None) => { + // If there are no diffs, keep looking. + continue; + } + } + } + + self.read_subspace_val(key) + } } fn write_subspace_val(