Skip to content

Commit

Permalink
Merge branch 'brent/fix-validator-commission-change-test' into tiago+…
Browse files Browse the repository at this point in the history
…tomas/main/storage-keys-procmacro

This branch surfaced a bug in a wasm test. Merge the fix to pass tests
on this branch.

* 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
  • Loading branch information
juped committed Jan 10, 2023
2 parents 704b175 + 01eb29e commit 5420d29
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -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))
11 changes: 4 additions & 7 deletions proof_of_stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}"
)]
Expand Down
6 changes: 3 additions & 3 deletions wasm/checksums.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"tx_bond.wasm": "tx_bond.59f751c77d75d96679ec6df7376d896e3cbf9598846ea09e2caef8114de97932.wasm",
"tx_change_validator_commission.wasm": "tx_change_validator_commission.cd861e0e82f4934be6d8382d6fff98286b4fadbc20ab826b9e817f6666021273.wasm",
"tx_bond.wasm": "tx_bond.1b720ddefc9b6e0f37145ab041363426cc0e0d82f2fa33e71063ca61efe2389e.wasm",
"tx_change_validator_commission.wasm": "tx_change_validator_commission.f6fd5102d74a171f11f404f00198d2e24b9596eca1b2e78b6dedd22cb81f73f7.wasm",
"tx_ibc.wasm": "tx_ibc.f261d127df2cb05489ad2655fcc6d0d8e7378789226ee27f0251ae90adfc1b0a.wasm",
"tx_init_account.wasm": "tx_init_account.e21cfd7e96802f8e841613fb89f1571451401d002a159c5e9586855ac1374df5.wasm",
"tx_init_proposal.wasm": "tx_init_proposal.211265350906ad7aef3aca30d6ef463d065b738707accd0acaa19992977bfabe.wasm",
"tx_init_validator.wasm": "tx_init_validator.eac9858c4f96bbefd120e3ebe0489e1700a83e8a22d778d5aa2b14ab0627f172.wasm",
"tx_reveal_pk.wasm": "tx_reveal_pk.47bc922a8be5571620a647ae442a1af7d03d05d29bef95f0b32cdfe00b11fee9.wasm",
"tx_transfer.wasm": "tx_transfer.5b95e6f1f6b2d4b0ec6f07ac1ec66089374ece2e4a6c5bdb32297985670a2ab0.wasm",
"tx_unbond.wasm": "tx_unbond.a33d113d04786c6638f5181b1bd65413b6b199e0c37985f444bea56430fc3f4c.wasm",
"tx_unbond.wasm": "tx_unbond.d943090852fee7e2f4f72ac3981e0181003bc5da0e46bec786a136298974e6fa.wasm",
"tx_update_vp.wasm": "tx_update_vp.ee2e9b882c4accadf4626e87d801c9ac8ea8c61ccea677e0532fc6c1ee7db6a2.wasm",
"tx_vote_proposal.wasm": "tx_vote_proposal.263fd9f4cb40f283756f394d86bdea3417e9ecd0568d6582c07a5b6bd14287d6.wasm",
"tx_withdraw.wasm": "tx_withdraw.b9b8623217202de2cb2c3941f740f6acd8558eaa05406efc60ee471c212aa513.wasm",
Expand Down
22 changes: 16 additions & 6 deletions wasm/wasm_source/src/tx_change_validator_commission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 5420d29

Please sign in to comment.