diff --git a/.changelog/unreleased/bug-fixes/1083-fix-tm-voting-power-check.md b/.changelog/unreleased/bug-fixes/1083-fix-tm-voting-power-check.md new file mode 100644 index 0000000000..5265eb2163 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1083-fix-tm-voting-power-check.md @@ -0,0 +1,3 @@ +- Fixed Tendermint validator set update check to + respect the PoS tm_votes_per_token parameter. + ([#1083](https://github.com/anoma/namada/pull/1083)) \ No newline at end of file diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index f7fb2f5a39..8455522303 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -414,33 +414,37 @@ where let pos_params = self.storage.read_pos_params(); // TODO ABCI validator updates on block H affects the validator set // on block H+2, do we need to update a block earlier? - self.storage.validator_set_update(current_epoch, |update| { - let (consensus_key, power) = match update { - ValidatorSetUpdate::Active(ActiveValidator { - consensus_key, - bonded_stake, - }) => { - let power: i64 = into_tm_voting_power( - pos_params.tm_votes_per_token, + self.storage.validator_set_update( + current_epoch, + &pos_params, + |update| { + let (consensus_key, power) = match update { + ValidatorSetUpdate::Active(ActiveValidator { + consensus_key, bonded_stake, - ); - (consensus_key, power) - } - ValidatorSetUpdate::Deactivated(consensus_key) => { - // Any validators that have become inactive must - // have voting power set to 0 to remove them from - // the active set - let power = 0_i64; - (consensus_key, power) - } - }; - let pub_key = TendermintPublicKey { - sum: Some(key_to_tendermint(&consensus_key).unwrap()), - }; - let pub_key = Some(pub_key); - let update = ValidatorUpdate { pub_key, power }; - response.validator_updates.push(update); - }); + }) => { + let power: i64 = into_tm_voting_power( + pos_params.tm_votes_per_token, + bonded_stake, + ); + (consensus_key, power) + } + ValidatorSetUpdate::Deactivated(consensus_key) => { + // Any validators that have become inactive must + // have voting power set to 0 to remove them from + // the active set + let power = 0_i64; + (consensus_key, power) + } + }; + let pub_key = TendermintPublicKey { + sum: Some(key_to_tendermint(&consensus_key).unwrap()), + }; + let pub_key = Some(pub_key); + let update = ValidatorUpdate { pub_key, power }; + response.validator_updates.push(update); + }, + ); } } diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 661533cb16..4d91b58ece 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -34,6 +34,7 @@ use namada_core::types::storage::Epoch; use namada_core::types::token; pub use parameters::PosParams; use rust_decimal::Decimal; +use storage::into_tm_voting_power; use thiserror::Error; use types::{ ActiveValidator, Bonds, CommissionRates, GenesisValidator, Slash, @@ -770,6 +771,7 @@ pub trait PosBase { fn validator_set_update( &self, current_epoch: Epoch, + params: &PosParams, f: impl FnMut(ValidatorSetUpdate), ) { let current_epoch: Epoch = current_epoch; @@ -805,14 +807,26 @@ pub trait PosBase { if let (Some(prev_epoch), Some(prev_validators)) = (previous_epoch, prev_validators) { - if prev_validators.active.contains(validator) { + let prev_validator_stake = + self.validator_stake(&validator.address, prev_epoch); + let tm_vp = into_tm_voting_power( + params.tm_votes_per_token, + validator.bonded_stake, + ); + if prev_validators.active.contains(validator) + || tm_vp + == into_tm_voting_power( + params.tm_votes_per_token, + prev_validator_stake, + ) + { println!( "skipping validator update, still the same {}", validator.address ); return None; } - if validator.bonded_stake == 0 { + if tm_vp == 0 { // If the validator was `Pending` in the previous epoch, // it means that it just was just added to validator // set. We have to skip it, because it's 0. @@ -851,10 +865,22 @@ pub trait PosBase { if let (Some(prev_epoch), Some(prev_validators)) = (previous_epoch, prev_validators) { - if prev_validators.inactive.contains(validator) { + let prev_validator_stake = + self.validator_stake(&validator.address, prev_epoch); + let tm_vp = into_tm_voting_power( + params.tm_votes_per_token, + validator.bonded_stake, + ); + if prev_validators.inactive.contains(validator) + || tm_vp + == into_tm_voting_power( + params.tm_votes_per_token, + prev_validator_stake, + ) + { return None; } - if validator.bonded_stake == 0 { + if tm_vp == 0 { // If the validator was `Pending` in the previous epoch, // it means that it just was just added to validator // set. We have to skip it, because it's 0. @@ -937,6 +963,25 @@ pub trait PosBase { ); Ok(()) } + + /// Get the total stake of a validator at the given epoch or current when + /// `None`. The total stake is a sum of validator's self-bonds and + /// delegations to their address. + fn validator_stake( + &self, + validator: &Address, + epoch: Epoch, + ) -> token::Amount { + let deltas = self.read_validator_deltas(validator); + let total_stake = deltas.and_then(|deltas| deltas.get(epoch)).and_then( + |total_stake| { + let sum: i128 = total_stake; + let sum: u64 = sum.try_into().ok()?; + Some(sum.into()) + }, + ); + total_stake.unwrap_or_default() + } } #[allow(missing_docs)] diff --git a/wasm/checksums.json b/wasm/checksums.json index 0e29ab48e3..61c31b2a26 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,13 +1,13 @@ { - "tx_bond.wasm": "tx_bond.50cc7754f07b0553d22d2f259b7be0d0dfa83cac21f349b9a5ef0c7f62a4c8e3.wasm", - "tx_change_validator_commission.wasm": "tx_change_validator_commission.29944a5b1a31077c0d5e486c30338f9a46ff71daa91fc3619b8425c0a0592203.wasm", + "tx_bond.wasm": "tx_bond.986d39ef1b4f3a544fe21ca022487fa2d32acc21a711a36ce43a2983b12289d1.wasm", + "tx_change_validator_commission.wasm": "tx_change_validator_commission.da32ad4e242a2ca246e941f80280e60b6fa6f216b5f6eba55f418713b4f99938.wasm", "tx_ibc.wasm": "tx_ibc.4d8d041cea7a9fd165ad082ba24dcb1afc8591a0fafdf36b6729a2cf8389d001.wasm", "tx_init_account.wasm": "tx_init_account.604050807200b80fb6cccaf49e62cd43ab48a6fcc03fdec5c8148e8d01067117.wasm", "tx_init_proposal.wasm": "tx_init_proposal.f84c91b5306664c054f2924869f407d42cc39e7a268e9d418a631e89dd7acbf4.wasm", "tx_init_validator.wasm": "tx_init_validator.c03ffe483d285866f596fc38a6fc5df59dc1ce0a45d3b6fde9fd597552284a0b.wasm", "tx_reveal_pk.wasm": "tx_reveal_pk.4e0f779ed1600e7208620233cb64b93fc56afb81122dd3f9bd5cf76dc3beff94.wasm", "tx_transfer.wasm": "tx_transfer.8c4fccac307878e20df2b4e18f418eea2bd7b0c0cb70d4e97fdc861d3deb2b76.wasm", - "tx_unbond.wasm": "tx_unbond.879f36b2fe0f56f5f8599a02a4716ee296037abfe2c384bee55809976918feca.wasm", + "tx_unbond.wasm": "tx_unbond.1278c4e775e06194d87cde63f945b81683c53d258f3d32281aef07d2f7546668.wasm", "tx_update_vp.wasm": "tx_update_vp.db4d8c11658c5f8e99fc39f0112be1ee480ab1f0045010d323ee3081a7afe802.wasm", "tx_vote_proposal.wasm": "tx_vote_proposal.16fb6a28f46a2d3d6013f4b7b20d8fe556cdd11b38f9bf064b085b266e0b54cc.wasm", "tx_withdraw.wasm": "tx_withdraw.90cb4a92c2ffaf93dbb6825667aec8a4cb571f0f2c9dd4e0592590a289416011.wasm",