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 deleted file mode 100644 index 274b48c5d1..0000000000 --- a/.changelog/unreleased/bug-fixes/965-fix-validator-commission-change-test.md +++ /dev/null @@ -1,3 +0,0 @@ -- 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)) \ No newline at end of file diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 661533cb16..64c7ffdff8 100644 --- 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, ¶ms) .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 index 3d30f25908..f32d83f34e 100644 --- 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) -> 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 { - 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 { - 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,