diff --git a/.changelog/unreleased/improvements/1973-refine-commission-tx.md b/.changelog/unreleased/improvements/1973-refine-commission-tx.md new file mode 100644 index 0000000000..04a00bac66 --- /dev/null +++ b/.changelog/unreleased/improvements/1973-refine-commission-tx.md @@ -0,0 +1,2 @@ +- Add missing checks for the commission rate change tx and code clean-up + ([\#1973](https://github.com/anoma/namada/pull/1973)) \ No newline at end of file diff --git a/proof_of_stake/src/error.rs b/proof_of_stake/src/error.rs index 96123d6feb..d3eeecb3c8 100644 --- a/proof_of_stake/src/error.rs +++ b/proof_of_stake/src/error.rs @@ -97,6 +97,10 @@ pub enum SlashError { pub enum CommissionRateChangeError { #[error("Unexpected negative commission rate {0} for validator {1}")] NegativeRate(Dec, Address), + #[error( + "Unexpected commission rate {0} larger than 1.0 for validator {1}" + )] + LargerThanOne(Dec, Address), #[error("Rate change of {0} is too large for validator {1}")] RateChangeTooLarge(Dec, Address), #[error( diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 6ff7bbe16a..0aa24142fc 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -2996,13 +2996,21 @@ pub fn change_validator_commission_rate( where S: StorageRead + StorageWrite, { - // if new_rate < Uint::zero() { - // return Err(CommissionRateChangeError::NegativeRate( - // new_rate, - // validator.clone(), - // ) - // .into()); - // } + if new_rate.is_negative() { + return Err(CommissionRateChangeError::NegativeRate( + new_rate, + validator.clone(), + ) + .into()); + } + + if new_rate > Dec::one() { + return Err(CommissionRateChangeError::LargerThanOne( + new_rate, + validator.clone(), + ) + .into()); + } let max_change = read_validator_max_commission_rate_change(storage, validator)?; @@ -3027,14 +3035,7 @@ where .get(storage, pipeline_epoch.prev(), ¶ms)? .expect("Could not find a rate in given epoch"); - // TODO: change this back if we use `Dec` type with a signed integer - // let change_from_prev = new_rate - rate_before_pipeline; - // if change_from_prev.abs() > max_change.unwrap() { - let change_from_prev = if new_rate > rate_before_pipeline { - new_rate - rate_before_pipeline - } else { - rate_before_pipeline - new_rate - }; + let change_from_prev = new_rate.abs_diff(&rate_before_pipeline); if change_from_prev > max_change.unwrap() { return Err(CommissionRateChangeError::RateChangeTooLarge( change_from_prev, diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index b23a8f98d3..5a3264b9fa 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -544,6 +544,18 @@ pub async fn build_validator_commission_change<'a>( commission_rate, max_commission_change_per_epoch, }) => { + if rate.is_negative() || *rate > Dec::one() { + edisplay_line!( + context.io(), + "New rate is outside of the allowed range of values \ + between 0.0 and 1.0." + ); + if !tx_args.force { + return Err(Error::from( + TxError::InvalidCommissionRate(*rate), + )); + } + } if rate.abs_diff(&commission_rate) > max_commission_change_per_epoch { diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 8e5a004cc5..dfdf6f000b 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -2634,7 +2634,7 @@ dependencies = [ [[package]] name = "ibc" version = "0.41.0" -source = "git+https://github.com/heliaxdev/cosmos-ibc-rs.git?rev=38a827d3901e590b2935ee5b6b81b4d67c399560#38a827d3901e590b2935ee5b6b81b4d67c399560" +source = "git+https://github.com/heliaxdev/cosmos-ibc-rs.git?rev=206cb5fa74a7ca38038b937d202ae39fbbd63c19#206cb5fa74a7ca38038b937d202ae39fbbd63c19" dependencies = [ "bytes", "cfg-if 1.0.0",