Revert "Merge pull request #965 from anoma/brent/fix-validator-commis…

…sion-change-test" This reverts commit bd4dff2, reversing changes made to 05f7fe5. This branch was merged in an incorrect state. A rerolled version will be merged later.
anoma · Jan 5, 2023 · d2f8dfc · d2f8dfc
1 parent bd4dff2
commit d2f8dfc
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 23 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
diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs
@@ -563,14 +563,15 @@ 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 Ok(());
+            return Err(CommissionRateChangeError::ChangeIsZero(
+                validator.clone(),
+            )
+            .into());
         }
 
         let rate_before_pipeline = *commission_rates
@@ -1017,6 +1018,8 @@ 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/wasm_source/src/tx_change_validator_commission.rs b/wasm/wasm_source/src/tx_change_validator_commission.rs
@@ -18,8 +18,6 @@ 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;
@@ -104,6 +102,9 @@ 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!(
@@ -159,24 +160,13 @@ 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 = 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(
+        let min = rate_pre - max_change;
+        let max = rate_pre + max_change;
+        (arb_established_address(), arb_rate(min, max)).prop_map(
             |(validator, new_rate)| transaction::pos::CommissionChange {
                 validator: Address::Established(validator),
                 new_rate,