Merge branch 'brent/fix-validator-commission-change-test' (#965/#984)…

… into main * brent/fix-validator-commission-change-test: wasm: update checksums.json changelog: add #965 remove `ChangeIsZero` error remove dbg statements fix arb rate: prevent same rate and ensure proper bounds
anoma · Jan 10, 2023 · 79928fc · 79928fc
2 parents b4269d6 + 01eb29e
commit 79928fc
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 16 deletions.
diff --git a/.changelog/unreleased/bug-fixes/965-fix-validator-commission-change-test.md b/.changelog/unreleased/bug-fixes/965-fix-validator-commission-change-test.md
@@ -0,0 +1,3 @@
+- Fix the commission rate change wasm test, which failed because an arbitrary
+  value for a new rate was allowed that could be equal to the previous rate.
+  ([#965](https://github.com/anoma/namada/pull/965))
diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs
@@ -563,15 +563,14 @@ pub trait PosActions: PosReadOnly {
                 }
             };
         let params = self.read_pos_params()?;
-
         let rate_at_pipeline = *commission_rates
             .get_at_offset(current_epoch, DynEpochOffset::PipelineLen, &params)
             .expect("Could not find a rate in given epoch");
+
+        // Return early with no further changes if there is no rate change
+        // instead of returning an error
         if new_rate == rate_at_pipeline {
-            return Err(CommissionRateChangeError::ChangeIsZero(
-                validator.clone(),
-            )
-            .into());
+            return Ok(());
         }
 
         let rate_before_pipeline = *commission_rates
@@ -1018,8 +1017,6 @@ pub enum CommissionRateChangeError {
     NegativeRate(Decimal, Address),
     #[error("Rate change of {0} is too large for validator {1}")]
     RateChangeTooLarge(Decimal, Address),
-    #[error("The rate change is 0 for validator {0}")]
-    ChangeIsZero(Address),
     #[error(
         "There is no maximum rate change written in storage for validator {0}"
     )]

diff --git a/wasm/checksums.json b/wasm/checksums.json
@@ -1,13 +1,13 @@
 {
-    "tx_bond.wasm": "tx_bond.00d0259ffcc79284aa3915cb84de855d05f9aa88553424f75566b7eeedc086a7.wasm",
-    "tx_change_validator_commission.wasm": "tx_change_validator_commission.7d09576a72068734b8e259df35ce98fabe98db990c337588793a616aa095d24f.wasm",
+    "tx_bond.wasm": "tx_bond.614cf9907c586300a777307455123def8d496e61c7cabca5b65bf24f51fb6e18.wasm",
+    "tx_change_validator_commission.wasm": "tx_change_validator_commission.541f44f500255891db7e4ae79710159213d30599cf40062d59db2faea6122560.wasm",
     "tx_ibc.wasm": "tx_ibc.88741a92e50564f791a0e33d05eb78bf9e530634ff8376b6182b6dbe68181ba3.wasm",
     "tx_init_account.wasm": "tx_init_account.a0a334950daa21e64fec0a98609268137eb2e35e5161cfa1da534ad71843c239.wasm",
     "tx_init_proposal.wasm": "tx_init_proposal.fc1a3347c4e978d579b074db77bea63db8f9ba450215bf556378fc6601c63f79.wasm",
     "tx_init_validator.wasm": "tx_init_validator.63a13e7aef346e1e48c8e848cb4c5d59ab8ce5a88bba2ef67b00837faa2350cb.wasm",
     "tx_reveal_pk.wasm": "tx_reveal_pk.05dfde69e33f0833fc25ad653066da73ef04a2b2f7894ba9fa509d2dd759ae8b.wasm",
     "tx_transfer.wasm": "tx_transfer.f10da787df300b60e1a9ff8e4bd51fc5246646e40c4c82f2245fc6c132b7f8f5.wasm",
-    "tx_unbond.wasm": "tx_unbond.59758396baa9c88432610a3912fa98060f07430fc182736eba7bfb1d53f4b71d.wasm",
+    "tx_unbond.wasm": "tx_unbond.7f1ed33691afd7dea8a2f611e325df31e66e5862e3de640ab8b0fa67c6421476.wasm",
     "tx_update_vp.wasm": "tx_update_vp.ee2e9b882c4accadf4626e87d801c9ac8ea8c61ccea677e0532fc6c1ee7db6a2.wasm",
     "tx_vote_proposal.wasm": "tx_vote_proposal.7764e840c0208e33c1d36248d90bbac1b46b3895da856f3b43023e4514452f7f.wasm",
     "tx_withdraw.wasm": "tx_withdraw.24cbb8d0e0f9a3d1358b67b5ed1c15cf1bb6be7a44f9cc0d49dfbe05f5945644.wasm",

diff --git a/wasm/wasm_source/src/tx_change_validator_commission.rs b/wasm/wasm_source/src/tx_change_validator_commission.rs
@@ -18,6 +18,8 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Vec<u8>) -> TxResult {
 
 #[cfg(test)]
 mod tests {
+    use std::cmp;
+
     use namada::ledger::pos::{PosParams, PosVP};
     use namada::proto::Tx;
     use namada::types::storage::Epoch;
@@ -102,9 +104,6 @@ mod tests {
             .read_validator_commission_rate(&commission_change.validator)?
             .unwrap();
 
-        dbg!(&commission_rates_pre);
-        dbg!(&commission_rates_post);
-
         // Before pipeline, the commission rates should not change
         for epoch in 0..pos_params.pipeline_len {
             assert_eq!(
@@ -160,13 +159,24 @@ mod tests {
             .prop_map(|num| Decimal::from(num) / Decimal::from(100_000_u64))
     }
 
+    fn arb_new_rate(
+        min: Decimal,
+        max: Decimal,
+        rate_pre: Decimal,
+    ) -> impl Strategy<Value = Decimal> {
+        arb_rate(min, max).prop_filter(
+            "New rate must not be equal to the previous epoch's rate",
+            move |v| v != &rate_pre,
+        )
+    }
+
     fn arb_commission_change(
         rate_pre: Decimal,
         max_change: Decimal,
     ) -> impl Strategy<Value = transaction::pos::CommissionChange> {
-        let min = rate_pre - max_change;
-        let max = rate_pre + max_change;
-        (arb_established_address(), arb_rate(min, max)).prop_map(
+        let min = cmp::max(rate_pre - max_change, Decimal::ZERO);
+        let max = cmp::min(rate_pre + max_change, Decimal::ONE);
+        (arb_established_address(), arb_new_rate(min, max, rate_pre)).prop_map(
             |(validator, new_rate)| transaction::pos::CommissionChange {
                 validator: Address::Established(validator),
                 new_rate,