Merge branch 'brent/upgrade-pos-rewards' (#2217)

* origin/brent/upgrade-pos-rewards:
  changelog: add #2217
  improve RPC error logging
  query for available rewards from a bond
  fix slashing in `bond_amounts_for_rewards`

.changelog/unreleased/improvements/2217-upgrade-pos-rewards.md

@@ -0,0 +1,3 @@
+- Implement a CLI query for available rewards from a bond,
+  and improve the bond amount for rewards computation
+  ([\#2217](https://github.com/anoma/namada/pull/2217))

apps/src/lib/cli.rs

@@ -261,6 +261,7 @@ pub mod cmds {
                 .subcommand(QueryPgf::def().display_order(5))
                 .subcommand(QueryValidatorState::def().display_order(5))
                 .subcommand(QueryCommissionRate::def().display_order(5))
+                .subcommand(QueryRewards::def().display_order(5))
                 .subcommand(QueryMetaData::def().display_order(5))
                 // Actions
                 .subcommand(SignTx::def().display_order(6))
@@ -318,6 +319,7 @@ pub mod cmds {
             let query_bonded_stake =
                 Self::parse_with_ctx(matches, QueryBondedStake);
             let query_slashes = Self::parse_with_ctx(matches, QuerySlashes);
+            let query_rewards = Self::parse_with_ctx(matches, QueryRewards);
             let query_delegations =
                 Self::parse_with_ctx(matches, QueryDelegations);
             let query_find_validator =
@@ -373,6 +375,7 @@ pub mod cmds {
                 .or(query_bonds)
                 .or(query_bonded_stake)
                 .or(query_slashes)
+                .or(query_rewards)
                 .or(query_delegations)
                 .or(query_find_validator)
                 .or(query_result)
@@ -468,6 +471,7 @@ pub mod cmds {
         QueryProtocolParameters(QueryProtocolParameters),
         QueryPgf(QueryPgf),
         QueryValidatorState(QueryValidatorState),
+        QueryRewards(QueryRewards),
         SignTx(SignTx),
         GenIbcShieldedTransafer(GenIbcShieldedTransafer),
     }
@@ -1868,6 +1872,28 @@ pub mod cmds {
         }
     }
 
+    #[derive(Clone, Debug)]
+    pub struct QueryRewards(pub args::QueryRewards<args::CliTypes>);
+
+    impl SubCmd for QueryRewards {
+        const CMD: &'static str = "rewards";
+
+        fn parse(matches: &ArgMatches) -> Option<Self> {
+            matches
+                .subcommand_matches(Self::CMD)
+                .map(|matches| QueryRewards(args::QueryRewards::parse(matches)))
+        }
+
+        fn def() -> App {
+            App::new(Self::CMD)
+                .about(
+                    "Query the latest rewards available to claim for a given \
+                     delegation (or self-bond).",
+                )
+                .add_args::<args::QueryRewards<args::CliTypes>>()
+        }
+    }
+
     #[derive(Clone, Debug)]
     pub struct QueryDelegations(pub args::QueryDelegations<args::CliTypes>);
 
@@ -4964,8 +4990,8 @@ pub mod args {
                 .arg(VALIDATOR.def().help("Validator address."))
                 .arg(SOURCE_OPT.def().help(
                     "Source address for withdrawing from delegations. For \
-                     withdrawing from self-bonds, the validator is also the \
-                     source.",
+                     withdrawing from self-bonds, this arg does not need to \
+                     be supplied.",
                 ))
         }
     }
@@ -5690,6 +5716,42 @@ pub mod args {
         }
     }
 
+    impl CliToSdk<QueryRewards<SdkTypes>> for QueryRewards<CliTypes> {
+        fn to_sdk(self, ctx: &mut Context) -> QueryRewards<SdkTypes> {
+            QueryRewards::<SdkTypes> {
+                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)),
+            }
+        }
+    }
+
+    impl Args for QueryRewards<CliTypes> {
+        fn parse(matches: &ArgMatches) -> Self {
+            let query = Query::parse(matches);
+            let source = SOURCE_OPT.parse(matches);
+            let validator = VALIDATOR.parse(matches);
+            Self {
+                query,
+                source,
+                validator,
+            }
+        }
+
+        fn def(app: App) -> App {
+            app.add_args::<Query<CliTypes>>()
+                .arg(SOURCE_OPT.def().help(
+                    "Source address for the rewards query. For self-bonds, \
+                     this arg does not need to be supplied.",
+                ))
+                .arg(
+                    VALIDATOR
+                        .def()
+                        .help("Validator address for the rewards query."),
+                )
+        }
+    }
+
     impl Args for QueryDelegations<CliTypes> {
         fn parse(matches: &ArgMatches) -> Self {
             let query = Query::parse(matches);

apps/src/lib/cli/client.rs

@@ -493,6 +493,17 @@ impl CliApi {
                         let namada = ctx.to_sdk(client, io);
                         rpc::query_slashes(&namada, args).await;
                     }
+                    Sub::QueryRewards(QueryRewards(mut args)) => {
+                        let client = client.unwrap_or_else(|| {
+                            C::from_tendermint_address(
+                                &mut args.query.ledger_address,
+                            )
+                        });
+                        client.wait_until_node_is_synced(io).await?;
+                        let args = args.to_sdk(&mut ctx);
+                        let namada = ctx.to_sdk(&client, io);
+                        rpc::query_and_print_rewards(&namada, args).await;
+                    }
                     Sub::QueryDelegations(QueryDelegations(mut args)) => {
                         let client = client.unwrap_or_else(|| {
                             C::from_tendermint_address(

apps/src/lib/client/rpc.rs

@@ -1886,6 +1886,17 @@ pub async fn query_validator_state<
     )
 }
 
+/// Query and return the available reward tokens corresponding to the bond
+pub async fn query_rewards<C: namada::ledger::queries::Client + Sync>(
+    client: &C,
+    source: &Option<Address>,
+    validator: &Address,
+) -> token::Amount {
+    unwrap_client_response::<C, token::Amount>(
+        RPC.vp().pos().rewards(client, validator, source).await,
+    )
+}
+
 /// Query a validator's state information
 pub async fn query_and_print_validator_state(
     context: &impl Namada,
@@ -2194,6 +2205,20 @@ pub async fn query_slashes<N: Namada>(context: &N, args: args::QuerySlashes) {
     }
 }
 
+pub async fn query_and_print_rewards<N: Namada>(
+    context: &N,
+    args: args::QueryRewards,
+) {
+    let (source, validator) = (args.source, args.validator);
+
+    let rewards = query_rewards(context.client(), &source, &validator).await;
+    display_line!(
+        context.io(),
+        "Current rewards available for claim: {} NAM",
+        rewards.to_string_native()
+    );
+}
+
 pub async fn query_delegations<N: Namada>(
     context: &N,
     args: args::QueryDelegations,
@@ -2621,8 +2646,8 @@ pub async fn query_governance_parameters<
 fn unwrap_client_response<C: namada::ledger::queries::Client, T>(
     response: Result<T, C::Error>,
 ) -> T {
-    response.unwrap_or_else(|_err| {
-        eprintln!("Error in the query");
+    response.unwrap_or_else(|err| {
+        eprintln!("Error in the query: {:?}", err);
         cli::safe_exit(1)
     })
 }

apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -2175,7 +2175,7 @@ mod test_finalize_block {
         assert!(rp3 > rp4);
     }
 
-    /// A unit test for PoS inflationary rewards claiming
+    /// A unit test for PoS inflationary rewards claiming and querying
     #[test]
     fn test_claim_rewards() {
         let (mut shell, _recv, _, _) = setup_with_cfg(SetupCfg {
@@ -2251,6 +2251,15 @@ mod test_finalize_block {
             advance_epoch(&mut shell, &pkh1, &votes, None);
         total_rewards += inflation;
 
+        // Query the available rewards
+        let query_rewards = namada_proof_of_stake::query_reward_tokens(
+            &shell.wl_storage,
+            None,
+            &validator.address,
+            current_epoch,
+        )
+        .unwrap();
+
         // Claim the rewards from the initial epoch
         let reward_1 = namada_proof_of_stake::claim_reward_tokens(
             &mut shell.wl_storage,
@@ -2260,8 +2269,20 @@ mod test_finalize_block {
         )
         .unwrap();
         total_claimed += reward_1;
+        assert_eq!(reward_1, query_rewards);
         assert!(is_reward_equal_enough(total_rewards, total_claimed, 1));
 
+        // Query the available rewards again and check that it is 0 now after
+        // the claim
+        let query_rewards = namada_proof_of_stake::query_reward_tokens(
+            &shell.wl_storage,
+            None,
+            &validator.address,
+            current_epoch,
+        )
+        .unwrap();
+        assert_eq!(query_rewards, token::Amount::zero());
+
         // Try a claim the next block and ensure we get 0 tokens back
         next_block_for_inflation(
             &mut shell,
@@ -2296,6 +2317,15 @@ mod test_finalize_block {
         .unwrap();
         assert_eq!(unbond_res.sum, unbond_amount);
 
+        // Query the available rewards
+        let query_rewards = namada_proof_of_stake::query_reward_tokens(
+            &shell.wl_storage,
+            None,
+            &validator.address,
+            current_epoch,
+        )
+        .unwrap();
+
         let rew = namada_proof_of_stake::claim_reward_tokens(
             &mut shell.wl_storage,
             None,
@@ -2305,6 +2335,7 @@ mod test_finalize_block {
         .unwrap();
         total_claimed += rew;
         assert!(is_reward_equal_enough(total_rewards, total_claimed, 3));
+        assert_eq!(query_rewards, rew);
 
         // Check the bond amounts for rewards up thru the withdrawable epoch
         let withdraw_epoch = current_epoch + params.withdrawable_epoch_offset();
@@ -2364,6 +2395,15 @@ mod test_finalize_block {
         .unwrap();
         assert_eq!(withdraw_amount, unbond_amount);
 
+        // Query the available rewards
+        let query_rewards = namada_proof_of_stake::query_reward_tokens(
+            &shell.wl_storage,
+            None,
+            &validator.address,
+            current_epoch,
+        )
+        .unwrap();
+
         // Claim tokens
         let reward_2 = namada_proof_of_stake::claim_reward_tokens(
             &mut shell.wl_storage,
@@ -2373,6 +2413,7 @@ mod test_finalize_block {
         )
         .unwrap();
         total_claimed += reward_2;
+        assert_eq!(query_rewards, reward_2);
 
         // The total rewards claimed should be approximately equal to the total
         // minted inflation, minus (unbond_amount / initial_stake) * rewards
@@ -2383,6 +2424,16 @@ mod test_finalize_block {
         let token_uncertainty = uncertainty * lost_rewards;
         let token_diff = total_claimed + lost_rewards - total_rewards;
         assert!(token_diff < token_uncertainty);
+
+        // Query the available rewards to check that they are 0
+        let query_rewards = namada_proof_of_stake::query_reward_tokens(
+            &shell.wl_storage,
+            None,
+            &validator.address,
+            current_epoch,
+        )
+        .unwrap();
+        assert_eq!(query_rewards, token::Amount::zero());
     }
 
     /// A unit test for PoS inflationary rewards claiming