Fix slashing in client queries for bond and unbond info

.changelog/unreleased/bug-fixes/1533-fix-slashing-client-query.md

@@ -0,0 +1,3 @@
+- The slashed token amounts contained inside the bond and unbond information
+  returned by the PoS library fn bonds_and_unbonds are fixed and properly
+  computed. ([#1533](https://github.com/anoma/namada/pull/1533))

proof_of_stake/src/lib.rs

@@ -53,7 +53,7 @@ use storage::{
     bonds_for_source_prefix, bonds_prefix, consensus_keys_key,
     decimal_mult_amount, get_validator_address_from_bond, into_tm_voting_power,
     is_bond_key, is_unbond_key, is_validator_slashes_key,
-    last_block_proposer_key, mult_change_to_amount, params_key, slashes_prefix,
+    last_block_proposer_key, params_key, slashes_prefix,
     unbonds_for_source_prefix, unbonds_prefix, validator_address_raw_hash_key,
     validator_last_slash_key, validator_max_commission_rate_change_key,
     BondDetails, BondsAndUnbondsDetail, BondsAndUnbondsDetails,
@@ -2599,6 +2599,7 @@ where
         let validator = bond_id.validator.clone();
         let (bonds, _unbonds) = bonds_and_unbonds.entry(bond_id).or_default();
         bonds.push(make_bond_details(
+            params,
             &validator,
             change,
             start,
@@ -2662,6 +2663,7 @@ where
         .filter(|(_start, change)| *change > token::Change::default())
         .map(|(start, change)| {
             make_bond_details(
+                params,
                 &validator,
                 change,
                 start,
@@ -2694,8 +2696,8 @@ where
     Ok(HashMap::from_iter([(bond_id, details)]))
 }
 
-// TODO: check carefully for validity
 fn make_bond_details(
+    params: &PosParams,
     validator: &Address,
     change: token::Change,
     start: Epoch,
@@ -2708,36 +2710,37 @@ fn make_bond_details(
         .cloned()
         .unwrap_or_default();
     let amount = token::Amount::from_change(change);
-    let slashed_amount =
-        slashes
-            .iter()
-            .fold(None, |acc: Option<token::Amount>, slash| {
-                if slash.epoch >= start {
-                    let validator_slashes =
-                        applied_slashes.entry(validator.clone()).or_default();
-                    if !prev_applied_slashes
-                        .iter()
-                        .any(|s| s.clone() == slash.clone())
-                    {
-                        validator_slashes.push(slash.clone());
-                    }
-                    return Some(
-                        acc.unwrap_or_default()
-                            + mult_change_to_amount(slash.rate, change),
-                    );
-                }
-                acc
-            });
-    let slashed_amount =
-        slashed_amount.map(|slashed| cmp::min(amount, slashed));
+    let mut slash_rates_by_epoch = BTreeMap::<Epoch, Decimal>::new();
+
+    let validator_slashes =
+        applied_slashes.entry(validator.clone()).or_default();
+    for slash in slashes {
+        if slash.epoch >= start {
+            let cur_rate = slash_rates_by_epoch.entry(slash.epoch).or_default();
+            *cur_rate = cmp::min(Decimal::ONE, *cur_rate + slash.rate);
+
+            if !prev_applied_slashes.iter().any(|s| s == slash) {
+                validator_slashes.push(slash.clone());
+            }
+        }
+    }
+
+    let slashed_amount = if slash_rates_by_epoch.is_empty() {
+        None
+    } else {
+        let amount_after_slashing = token::Amount::from_change(
+            get_slashed_amount(params, amount, &slash_rates_by_epoch).unwrap(),
+        );
+        Some(amount - amount_after_slashing)
+    };
+
     BondDetails {
         start,
         amount,
         slashed_amount,
     }
 }
 
-// TODO: check carefully for validity
 fn make_unbond_details(
     params: &PosParams,
     validator: &Address,
@@ -2751,37 +2754,38 @@ fn make_unbond_details(
         .get(validator)
         .cloned()
         .unwrap_or_default();
-    // TODO: checks bounds for considering valid unbond with slash!
-    let slashed_amount =
-        slashes
-            .iter()
-            .fold(None, |acc: Option<token::Amount>, slash| {
-                if slash.epoch >= start
-                    && slash.epoch
-                        < withdraw
-                            .checked_sub(Epoch(
-                                params.unbonding_len
-                                    + params.cubic_slashing_window_length,
-                            ))
-                            .unwrap_or_default()
-                {
-                    let validator_slashes =
-                        applied_slashes.entry(validator.clone()).or_default();
-                    if !prev_applied_slashes
-                        .iter()
-                        .any(|s| s.clone() == slash.clone())
-                    {
-                        validator_slashes.push(slash.clone());
-                    }
-                    return Some(
-                        acc.unwrap_or_default()
-                            + decimal_mult_amount(slash.rate, amount),
-                    );
-                }
-                acc
-            });
-    let slashed_amount =
-        slashed_amount.map(|slashed| cmp::min(amount, slashed));
+    let mut slash_rates_by_epoch = BTreeMap::<Epoch, Decimal>::new();
+
+    let validator_slashes =
+        applied_slashes.entry(validator.clone()).or_default();
+    for slash in slashes {
+        if slash.epoch >= start
+            && slash.epoch
+                < withdraw
+                    .checked_sub(Epoch(
+                        params.unbonding_len
+                            + params.cubic_slashing_window_length,
+                    ))
+                    .unwrap_or_default()
+        {
+            let cur_rate = slash_rates_by_epoch.entry(slash.epoch).or_default();
+            *cur_rate = cmp::min(Decimal::ONE, *cur_rate + slash.rate);
+
+            if !prev_applied_slashes.iter().any(|s| s == slash) {
+                validator_slashes.push(slash.clone());
+            }
+        }
+    }
+
+    let slashed_amount = if slash_rates_by_epoch.is_empty() {
+        None
+    } else {
+        let amount_after_slashing = token::Amount::from_change(
+            get_slashed_amount(params, amount, &slash_rates_by_epoch).unwrap(),
+        );
+        Some(amount - amount_after_slashing)
+    };
+
     UnbondDetails {
         start,
         withdraw,

proof_of_stake/src/tests.rs

@@ -988,10 +988,23 @@ fn test_slashes_with_unbonding_aux(
     let token = staking_token_address(&s);
     let val_balance_pre = read_balance(&s, &token, val_addr).unwrap();
 
+    let bond_id = BondId {
+        source: val_addr.clone(),
+        validator: val_addr.clone(),
+    };
+    let binding =
+        super::bonds_and_unbonds(&s, None, Some(val_addr.clone())).unwrap();
+    let details = binding.get(&bond_id).unwrap();
+    let exp_withdraw_from_details = details.unbonds[0].amount
+        - details.unbonds[0].slashed_amount.unwrap_or_default();
+
     withdraw_tokens(&mut s, None, val_addr, current_epoch).unwrap();
 
     let val_balance_post = read_balance(&s, &token, val_addr).unwrap();
     let withdrawn_tokens = val_balance_post - val_balance_pre;
+    println!("Withdrew {withdrawn_tokens} tokens");
+
+    assert_eq!(exp_withdraw_from_details, withdrawn_tokens);
 
     let slash_rate_0 = validator_slashes_handle(val_addr)
         .get(&s, 0)
@@ -1003,7 +1016,7 @@ fn test_slashes_with_unbonding_aux(
         .unwrap()
         .unwrap()
         .rate;
-    println!("Slash 0 rate {slash_rate_0}, slash 1 {slash_rate_1}");
+    println!("Slash 0 rate {slash_rate_0}, slash 1 rate {slash_rate_1}");
 
     let expected_withdrawn_amount = decimal_mult_amount(
         dec!(1) - slash_rate_1,

wasm/checksums.json

@@ -1,17 +1,17 @@
 {
-    "tx_bond.wasm": "tx_bond.eb80d8b2c2c71a0d76eafe0c2b88149fd2a30be897b42ba3a62021062bf7e00d.wasm",
-    "tx_change_validator_commission.wasm": "tx_change_validator_commission.b07a31550d600b0f2b930cc1d30e9daaee2ac6138deebe1f7e8d6296d6183c21.wasm",
-    "tx_ibc.wasm": "tx_ibc.012d3f1da7532f3ff92bb56499c349ea3e5b6abaf4bdb7b8faa0177bf7ea2b93.wasm",
-    "tx_init_account.wasm": "tx_init_account.6359caa0d6fda1d896b84a881351c6827aca66240988b438697c966dec231be2.wasm",
-    "tx_init_proposal.wasm": "tx_init_proposal.339f4454df0be8ae670c5df84645c7b156f478726dc1e964a93cea4f8b6a4651.wasm",
-    "tx_init_validator.wasm": "tx_init_validator.a45ef2fc87cf2760107cd682a34772207da3ee96de778e5e83f22256df99ff7d.wasm",
-    "tx_reveal_pk.wasm": "tx_reveal_pk.b7541013221fedb42d9bdd4be6e972deac147b8c930b03db282f0b7811198554.wasm",
-    "tx_transfer.wasm": "tx_transfer.8b6fb1f418fea7d8909ed45160b9614f6ff50421fd766ce7ac9c1767af848aa7.wasm",
-    "tx_unbond.wasm": "tx_unbond.ba1c1f38680e8841d3a504661dbf580bcc5decba462739b6f76cf4330e698e16.wasm",
-    "tx_unjail_validator.wasm": "tx_unjail_validator.f71fb4b68b262d7007f4a9b6548538ec017218af5aeb34b9a48bb93689df6e15.wasm",
+    "tx_bond.wasm": "tx_bond.901088ca80efe7ade43d315236eb9078b55af41e09bdef5b90e07ac4de163661.wasm",
+    "tx_change_validator_commission.wasm": "tx_change_validator_commission.bb99b2e37f015ae35b296502f63d744841dd65998ec8e4c313b01ae7538391d4.wasm",
+    "tx_ibc.wasm": "tx_ibc.7eb91755194fa6c1b69b7c67cfe52bb72f5a29ea2de628262ed1de5d3c9e04bf.wasm",
+    "tx_init_account.wasm": "tx_init_account.9f0bb8e90039f6921e0e8a9f1cdd32d804ad2f76ebe67fe8d75b4c5236a31cac.wasm",
+    "tx_init_proposal.wasm": "tx_init_proposal.71559e8bfe82797e5b35cbeac5afe93a0a40b2d1a22c8854d266fcc76e347643.wasm",
+    "tx_init_validator.wasm": "tx_init_validator.313a11a31044ada43a104dda8e5ceb91d2e8da0d4023f63d8dfc938b67b970bf.wasm",
+    "tx_reveal_pk.wasm": "tx_reveal_pk.a76890f99b6bc0265c966989d1497c26b9c70accf4ba82bf1f65b7df6ea4faeb.wasm",
+    "tx_transfer.wasm": "tx_transfer.c91ebd9c4cdc8c082cc1b83c0cded0af56463715185cf0558f16bf011f01de1c.wasm",
+    "tx_unbond.wasm": "tx_unbond.47efe65d150e9b01d8ff9029ec8031b79f800699c3a16cb4d957248cb61d0ca8.wasm",
+    "tx_unjail_validator.wasm": "tx_unjail_validator.2ea989df3144c32f0c0f4ea5f40c1cce44512db47ae89245ae4630e81d21c07f.wasm",
     "tx_update_vp.wasm": "tx_update_vp.e5d25854a23186925caa2169869b1505e29132b8a8b396e911379d53ce5cf418.wasm",
     "tx_vote_proposal.wasm": "tx_vote_proposal.22b3358af6611d1786f6d49ccbf0757e3065a43fb37e8df0055f2173c8e14653.wasm",
-    "tx_withdraw.wasm": "tx_withdraw.62f7bf562a99f87586fae2c0f75d2fb0d57e4452d024848dcad2d8d01f78149c.wasm",
+    "tx_withdraw.wasm": "tx_withdraw.e8c99a4081d7a5fedd92b4c49d1156b2138ba42c342d6ea5b81643769dc15ffa.wasm",
     "vp_implicit.wasm": "vp_implicit.1e8355b50da06a5dcf9d2a8ab8ddc440f486ce039538dee415ab8625b1b11bae.wasm",
     "vp_masp.wasm": "vp_masp.7487613c4391eef753360c55e63979fad3855259f2fdb0e945fe702342ef5979.wasm",
     "vp_testnet_faucet.wasm": "vp_testnet_faucet.d3cafc4d31f1081a8cedfbfdb45e674235231716668a6b1309beece16fe5ea5d.wasm",