Merge branch 'brent/fix-bonding-query-logging' (#1239)

* brent/fix-bonding-query-logging:
  changelog: add #1239
  [ci] wasm checksums update
  client/rpc/query_bonds: refactor writelns
  test for the CLI output
  Fix CLI output of `submit_unbond`
  fix unbond query details when only one of source/validator is specified (and tests)
  submit_unbond: only print out the unbond tx amount and its withdrawable epoch
  fix `query_and_print_unbonds`
  query_bonds: don't show bond entries with 0 amount

.changelog/unreleased/bug-fixes/1239-fix-bonding-query-logging.md

@@ -0,0 +1,2 @@
+- Fixed various features of the CLI output for querying bonds and performing an
+  unbond action. ([#1239](https://github.com/anoma/namada/pull/1239))

apps/src/bin/namada-client/cli.rs

@@ -93,7 +93,7 @@ pub async fn main() -> Result<()> {
                 Sub::QueryBonds(QueryBonds(args)) => {
                     wait_until_node_is_synched(&args.query.ledger_address)
                         .await;
-                    rpc::query_bonds(ctx, args).await;
+                    rpc::query_bonds(ctx, args).await.unwrap();
                 }
                 Sub::QueryBondedStake(QueryBondedStake(args)) => {
                     wait_until_node_is_synched(&args.query.ledger_address)

apps/src/lib/client/rpc.rs

@@ -1430,20 +1430,25 @@ pub async fn query_and_print_unbonds(
 ) {
     let unbonds = query_unbond_with_slashing(client, source, validator).await;
     let current_epoch = query_epoch(client).await;
-    let (withdrawable, not_yet_withdrawable): (HashMap<_, _>, HashMap<_, _>) =
-        unbonds.into_iter().partition(|((_, withdraw_epoch), _)| {
-            withdraw_epoch <= &current_epoch
-        });
-    let total_withdrawable = withdrawable
-        .into_iter()
-        .fold(token::Amount::default(), |acc, (_, amount)| acc + amount);
+
+    let mut total_withdrawable = token::Amount::default();
+    let mut not_yet_withdrawable = HashMap::<Epoch, token::Amount>::new();
+    for ((_start_epoch, withdraw_epoch), amount) in unbonds.into_iter() {
+        if withdraw_epoch <= current_epoch {
+            total_withdrawable += amount;
+        } else {
+            let withdrawable_amount =
+                not_yet_withdrawable.entry(withdraw_epoch).or_default();
+            *withdrawable_amount += amount;
+        }
+    }
     if total_withdrawable != token::Amount::default() {
         println!("Total withdrawable now: {total_withdrawable}.");
     }
     if !not_yet_withdrawable.is_empty() {
         println!("Current epoch: {current_epoch}.")
     }
-    for ((_start_epoch, withdraw_epoch), amount) in not_yet_withdrawable {
+    for (withdraw_epoch, amount) in not_yet_withdrawable {
         println!(
             "Amount {amount} withdrawable starting from epoch \
              {withdraw_epoch}."
@@ -1466,7 +1471,10 @@ pub async fn query_withdrawable_tokens(
 }
 
 /// Query PoS bond(s) and unbond(s)
-pub async fn query_bonds(ctx: Context, args: args::QueryBonds) {
+pub async fn query_bonds(
+    ctx: Context,
+    args: args::QueryBonds,
+) -> std::io::Result<()> {
     let _epoch = query_and_print_epoch(args.query.clone()).await;
     let client = HttpClient::new(args.query.ledger_address).unwrap();
 
@@ -1499,14 +1507,13 @@ pub async fn query_bonds(ctx: Context, args: args::QueryBonds) {
                 bond_id.source, bond_id.validator
             )
         };
-        writeln!(w, "{}:", bond_type).unwrap();
+        writeln!(w, "{}:", bond_type)?;
         for bond in details.bonds {
             writeln!(
                 w,
                 "  Remaining active bond from epoch {}: Δ {}",
                 bond.start, bond.amount
-            )
-            .unwrap();
+            )?;
             total += bond.amount;
             total_slashed += bond.slashed_amount.unwrap_or_default();
         }
@@ -1515,10 +1522,10 @@ pub async fn query_bonds(ctx: Context, args: args::QueryBonds) {
                 w,
                 "Active (slashed) bonds total: {}",
                 total - total_slashed
-            )
-            .unwrap();
+            )?;
         }
-        writeln!(w, "Bonds total: {}", total).unwrap();
+        writeln!(w, "Bonds total: {}", total)?;
+        writeln!(w)?;
         bonds_total += total;
         bonds_total_slashed += total_slashed;
 
@@ -1531,43 +1538,45 @@ pub async fn query_bonds(ctx: Context, args: args::QueryBonds) {
             } else {
                 format!("Unbonded delegations from {}", bond_id.source)
             };
-            writeln!(w, "{}:", bond_type).unwrap();
+            writeln!(w, "{}:", bond_type)?;
             for unbond in details.unbonds {
                 total += unbond.amount;
                 total_slashed += unbond.slashed_amount.unwrap_or_default();
                 writeln!(
                     w,
                     "  Withdrawable from epoch {} (active from {}): Δ {}",
                     unbond.withdraw, unbond.start, unbond.amount
-                )
-                .unwrap();
+                )?;
             }
             withdrawable = total - total_slashed;
-            writeln!(w, "Unbonded total: {}", total).unwrap();
+            writeln!(w, "Unbonded total: {}", total)?;
 
             unbonds_total += total;
             unbonds_total_slashed += total_slashed;
             total_withdrawable += withdrawable;
         }
-        writeln!(w, "Withdrawable total: {}", withdrawable).unwrap();
-        println!();
+        writeln!(w, "Withdrawable total: {}", withdrawable)?;
+        writeln!(w)?;
     }
     if bonds_total != bonds_total_slashed {
-        println!(
+        writeln!(
+            w,
             "All bonds total active: {}",
             bonds_total - bonds_total_slashed
-        );
+        )?;
     }
-    println!("All bonds total: {}", bonds_total);
+    writeln!(w, "All bonds total: {}", bonds_total)?;
 
     if unbonds_total != unbonds_total_slashed {
-        println!(
+        writeln!(
+            w,
             "All unbonds total active: {}",
             unbonds_total - unbonds_total_slashed
-        );
+        )?;
     }
-    println!("All unbonds total: {}", unbonds_total);
-    println!("All unbonds total withdrawable: {}", total_withdrawable);
+    writeln!(w, "All unbonds total: {}", unbonds_total)?;
+    writeln!(w, "All unbonds total withdrawable: {}", total_withdrawable)?;
+    Ok(())
 }
 
 /// Query PoS bonded stake

apps/src/lib/client/tx.rs

@@ -2527,7 +2527,7 @@ pub async fn submit_unbond(ctx: Context, args: args::Unbond) {
     let bond_source = source.clone().unwrap_or_else(|| validator.clone());
     let bond_amount =
         rpc::query_bond(&client, &bond_source, &validator, None).await;
-    println!("BOND AMOUNT REMAINING IS {}", bond_amount);
+    println!("Bond amount available for unbonding: {} NAM", bond_amount);
 
     if args.amount > bond_amount {
         eprintln!(
@@ -2540,6 +2540,17 @@ pub async fn submit_unbond(ctx: Context, args: args::Unbond) {
         }
     }
 
+    // Query the unbonds before submitting the tx
+    let unbonds =
+        rpc::query_unbond_with_slashing(&client, &bond_source, &validator)
+            .await;
+    let mut withdrawable = BTreeMap::<Epoch, token::Amount>::new();
+    for ((_start_epoch, withdraw_epoch), amount) in unbonds.into_iter() {
+        let to_withdraw = withdrawable.entry(withdraw_epoch).or_default();
+        *to_withdraw += amount;
+    }
+    let latest_withdrawal_pre = withdrawable.into_iter().last();
+
     let data = pos::Unbond {
         validator: validator.clone(),
         amount: args.amount,
@@ -2565,7 +2576,50 @@ pub async fn submit_unbond(ctx: Context, args: args::Unbond) {
     )
     .await;
 
-    rpc::query_and_print_unbonds(&client, &bond_source, &validator).await;
+    // Query the unbonds post-tx
+    let unbonds =
+        rpc::query_unbond_with_slashing(&client, &bond_source, &validator)
+            .await;
+    let mut withdrawable = BTreeMap::<Epoch, token::Amount>::new();
+    for ((_start_epoch, withdraw_epoch), amount) in unbonds.into_iter() {
+        let to_withdraw = withdrawable.entry(withdraw_epoch).or_default();
+        *to_withdraw += amount;
+    }
+    let (latest_withdraw_epoch_post, latest_withdraw_amount_post) =
+        withdrawable.into_iter().last().unwrap();
+
+    if let Some((latest_withdraw_epoch_pre, latest_withdraw_amount_pre)) =
+        latest_withdrawal_pre
+    {
+        match latest_withdraw_epoch_post.cmp(&latest_withdraw_epoch_pre) {
+            std::cmp::Ordering::Less => {
+                eprintln!(
+                    "Unexpected behavior reading the unbonds data has occurred"
+                );
+                if !args.tx.force {
+                    safe_exit(1)
+                }
+            }
+            std::cmp::Ordering::Equal => {
+                println!(
+                    "Amount {} withdrawable starting from epoch {}",
+                    latest_withdraw_amount_post - latest_withdraw_amount_pre,
+                    latest_withdraw_epoch_post
+                );
+            }
+            std::cmp::Ordering::Greater => {
+                println!(
+                    "Amount {} withdrawable starting from epoch {}",
+                    latest_withdraw_amount_post, latest_withdraw_epoch_post
+                );
+            }
+        }
+    } else {
+        println!(
+            "Amount {} withdrawable starting from epoch {}",
+            latest_withdraw_amount_post, latest_withdraw_epoch_post
+        );
+    }
 }
 
 pub async fn submit_withdraw(ctx: Context, args: args::Withdraw) {

proof_of_stake/src/lib.rs

@@ -2321,6 +2321,9 @@ where
                     }
                     let change: token::Change =
                         BorshDeserialize::try_from_slice(&val_bytes).ok()?;
+                    if change == 0 {
+                        return None;
+                    }
                     return Some((bond_id, start, change));
                 }
             }
@@ -2334,24 +2337,33 @@ where
     let mut raw_unbonds = storage_api::iter_prefix_bytes(storage, &prefix)?
         .filter_map(|result| {
             if let Ok((key, val_bytes)) = result {
-                if let Some((_bond_id, _start, withdraw)) = is_unbond_key(&key)
-                {
-                    if let Some((bond_id, start)) = is_bond_key(&key) {
-                        if source.is_some()
-                            && source.as_ref().unwrap() != &bond_id.source
-                        {
-                            return None;
+                if let Some((bond_id, start, withdraw)) = is_unbond_key(&key) {
+                    if source.is_some()
+                        && source.as_ref().unwrap() != &bond_id.source
+                    {
+                        return None;
+                    }
+                    if validator.is_some()
+                        && validator.as_ref().unwrap() != &bond_id.validator
+                    {
+                        return None;
+                    }
+                    match (source.clone(), validator.clone()) {
+                        (None, Some(validator)) => {
+                            if bond_id.validator != validator {
+                                return None;
+                            }
                         }
-                        if validator.is_some()
-                            && validator.as_ref().unwrap() != &bond_id.validator
-                        {
-                            return None;
+                        (Some(owner), None) => {
+                            if owner != bond_id.source {
+                                return None;
+                            }
                         }
-                        let amount: token::Amount =
-                            BorshDeserialize::try_from_slice(&val_bytes)
-                                .ok()?;
-                        return Some((bond_id, start, withdraw, amount));
+                        _ => {}
                     }
+                    let amount: token::Amount =
+                        BorshDeserialize::try_from_slice(&val_bytes).ok()?;
+                    return Some((bond_id, start, withdraw, amount));
                 }
             }
             None
@@ -2434,6 +2446,7 @@ where
 
     let bonds = find_bonds(storage, &source, &validator)?
         .into_iter()
+        .filter(|(_start, change)| *change > token::Change::default())
         .map(|(start, change)| {
             make_bond_details(
                 storage,