Skip to content

Commit

Permalink
Merge branch 'brent/refactor-voting-power' (#707)
Browse files Browse the repository at this point in the history
* brent/refactor-voting-power:
  [ci] wasm checksums update
  changelog: add #707
  remove comments to self
  convert to tm voting power in `update_epoch`
  client: replace voting power with bonded stake in queries, etc
  fix pos state machine test
  clean up naming of "validator total deltas" -> "validator deltas"
  fix client voting power query
  fix `TendermintValidator::power`
  clippy: suppress unused validation vars (may need later)
  fmt + cleanup after cherrypicking commits from #388
  more voting_power removal and accurate variable renaming
  keep voting_power as a possible client query
  change `validator_total_deltas` -> `validator_deltas`
  Update wasm tx_(un)bond with VotingPower removal
  refactor VotingPower out of PoS VP
  continue refactoring away VotingPower
  refactor out VotingPower(Delta) in pos crate, distinguish total and validator deltas
  • Loading branch information
tzemanovic committed Nov 17, 2022
2 parents 9c81efb + b7a2b86 commit 6cc4825
Show file tree
Hide file tree
Showing 21 changed files with 627 additions and 1,571 deletions.
5 changes: 5 additions & 0 deletions .changelog/unreleased/features/707-refactor-voting-powers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- Optimize the PoS code to depend only on bonded stake, removing
the VotingPower(Delta) structs. This mitigates some previous
information loss in PoS calculations. Instead, the notion of
voting power is only relevant when communicating with Tendermint.
([#707](https://github.com/anoma/namada/pull/707))
4 changes: 2 additions & 2 deletions apps/src/bin/anoma-client/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ pub async fn main() -> Result<()> {
Sub::QueryBonds(QueryBonds(args)) => {
rpc::query_bonds(ctx, args).await;
}
Sub::QueryVotingPower(QueryVotingPower(args)) => {
rpc::query_voting_power(ctx, args).await;
Sub::QueryBondedStake(QueryBondedStake(args)) => {
rpc::query_bonded_stake(ctx, args).await;
}
Sub::QueryCommissionRate(QueryCommissionRate(args)) => {
rpc::query_commission_rate(ctx, args).await;
Expand Down
32 changes: 16 additions & 16 deletions apps/src/lib/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub mod cmds {
.subcommand(QueryBlock::def().display_order(3))
.subcommand(QueryBalance::def().display_order(3))
.subcommand(QueryBonds::def().display_order(3))
.subcommand(QueryVotingPower::def().display_order(3))
.subcommand(QueryBondedStake::def().display_order(3))
.subcommand(QuerySlashes::def().display_order(3))
.subcommand(QueryResult::def().display_order(3))
.subcommand(QueryRawBytes::def().display_order(3))
Expand Down Expand Up @@ -207,8 +207,8 @@ pub mod cmds {
let query_block = Self::parse_with_ctx(matches, QueryBlock);
let query_balance = Self::parse_with_ctx(matches, QueryBalance);
let query_bonds = Self::parse_with_ctx(matches, QueryBonds);
let query_voting_power =
Self::parse_with_ctx(matches, QueryVotingPower);
let query_bonded_stake =
Self::parse_with_ctx(matches, QueryBondedStake);
let query_slashes = Self::parse_with_ctx(matches, QuerySlashes);
let query_result = Self::parse_with_ctx(matches, QueryResult);
let query_raw_bytes = Self::parse_with_ctx(matches, QueryRawBytes);
Expand Down Expand Up @@ -236,7 +236,7 @@ pub mod cmds {
.or(query_block)
.or(query_balance)
.or(query_bonds)
.or(query_voting_power)
.or(query_bonded_stake)
.or(query_slashes)
.or(query_result)
.or(query_raw_bytes)
Expand Down Expand Up @@ -298,7 +298,7 @@ pub mod cmds {
QueryBlock(QueryBlock),
QueryBalance(QueryBalance),
QueryBonds(QueryBonds),
QueryVotingPower(QueryVotingPower),
QueryBondedStake(QueryBondedStake),
QueryCommissionRate(QueryCommissionRate),
QuerySlashes(QuerySlashes),
QueryRawBytes(QueryRawBytes),
Expand Down Expand Up @@ -1215,21 +1215,21 @@ pub mod cmds {
}

#[derive(Clone, Debug)]
pub struct QueryVotingPower(pub args::QueryVotingPower);
pub struct QueryBondedStake(pub args::QueryBondedStake);

impl SubCmd for QueryVotingPower {
const CMD: &'static str = "voting-power";
impl SubCmd for QueryBondedStake {
const CMD: &'static str = "bonded-stake";

fn parse(matches: &ArgMatches) -> Option<Self> {
matches.subcommand_matches(Self::CMD).map(|matches| {
QueryVotingPower(args::QueryVotingPower::parse(matches))
QueryBondedStake(args::QueryBondedStake::parse(matches))
})
}

fn def() -> App {
App::new(Self::CMD)
.about("Query PoS voting power.")
.add_args::<args::QueryVotingPower>()
.about("Query PoS bonded stake.")
.add_args::<args::QueryBondedStake>()
}
}

Expand Down Expand Up @@ -2549,18 +2549,18 @@ pub mod args {
}
}

/// Query PoS voting power
/// Query PoS bonded stake
#[derive(Clone, Debug)]
pub struct QueryVotingPower {
pub struct QueryBondedStake {
/// Common query args
pub query: Query,
/// Address of a validator
pub validator: Option<WalletAddress>,
/// Epoch in which to find voting power
/// Epoch in which to find bonded stake
pub epoch: Option<Epoch>,
}

impl Args for QueryVotingPower {
impl Args for QueryBondedStake {
fn parse(matches: &ArgMatches) -> Self {
let query = Query::parse(matches);
let validator = VALIDATOR_OPT.parse(matches);
Expand All @@ -2575,7 +2575,7 @@ pub mod args {
fn def(app: App) -> App {
app.add_args::<Query>()
.arg(VALIDATOR_OPT.def().about(
"The validator's address whose voting power to query.",
"The validator's address whose bonded stake to query.",
))
.arg(EPOCH.def().about(
"The epoch at which to query (last committed, if not \
Expand Down
68 changes: 35 additions & 33 deletions apps/src/lib/client/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use namada::ledger::governance::storage as gov_storage;
use namada::ledger::governance::utils::Votes;
use namada::ledger::parameters::{storage as param_storage, EpochDuration};
use namada::ledger::pos::types::{
decimal_mult_u64, Epoch as PosEpoch, VotingPower, WeightedValidator,
decimal_mult_u64, Epoch as PosEpoch, WeightedValidator,
};
use namada::ledger::pos::{
self, is_validator_slashes_key, BondId, Bonds, PosParams, Slash, Unbonds,
Expand Down Expand Up @@ -1707,8 +1707,8 @@ pub async fn query_bonds(ctx: Context, args: args::QueryBonds) {
}
}

/// Query PoS voting power
pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) {
/// Query PoS bonded stake
pub async fn query_bonded_stake(ctx: Context, args: args::QueryBondedStake) {
let epoch = match args.epoch {
Some(epoch) => epoch,
None => query_epoch(args.query.clone()).await,
Expand All @@ -1724,26 +1724,26 @@ pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) {
let validator_set = validator_sets
.get(epoch)
.expect("Validator set should be always set in the current epoch");

match args.validator {
Some(validator) => {
let validator = ctx.get(&validator);
// Find voting power for the given validator
let voting_power_key = pos::validator_voting_power_key(&validator);
let voting_powers =
query_storage_value::<pos::ValidatorVotingPowers>(
&client,
&voting_power_key,
)
.await;
match voting_powers.and_then(|data| data.get(epoch)) {
Some(voting_power_delta) => {
let voting_power: VotingPower =
voting_power_delta.try_into().expect(
"The sum voting power deltas shouldn't be negative",
);
// Find bonded stake for the given validator
let validator_deltas_key = pos::validator_deltas_key(&validator);
let validator_deltas = query_storage_value::<pos::ValidatorDeltas>(
&client,
&validator_deltas_key,
)
.await;
match validator_deltas.and_then(|data| data.get(epoch)) {
Some(val_stake) => {
let bonded_stake: u64 = val_stake.try_into().expect(
"The sum of the bonded stake deltas shouldn't be \
negative",
);
let weighted = WeightedValidator {
address: validator.clone(),
voting_power,
bonded_stake,
};
let is_active = validator_set.active.contains(&weighted);
if !is_active {
Expand All @@ -1752,14 +1752,14 @@ pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) {
);
}
println!(
"Validator {} is {}, voting power: {}",
"Validator {} is {}, bonded stake: {}",
validator.encode(),
if is_active { "active" } else { "inactive" },
voting_power
bonded_stake,
)
}
None => {
println!("No voting power found for {}", validator.encode())
println!("No bonded stake found for {}", validator.encode())
}
}
}
Expand All @@ -1774,7 +1774,7 @@ pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) {
w,
" {}: {}",
active.address.encode(),
active.voting_power
active.bonded_stake
)
.unwrap();
}
Expand All @@ -1785,24 +1785,26 @@ pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) {
w,
" {}: {}",
inactive.address.encode(),
inactive.voting_power
inactive.bonded_stake
)
.unwrap();
}
}
}
}
let total_voting_power_key = pos::total_voting_power_key();
let total_voting_powers = query_storage_value::<pos::TotalVotingPowers>(
&client,
&total_voting_power_key,
)
.await
.expect("Total voting power should always be set");
let total_voting_power = total_voting_powers
let total_deltas_key = pos::total_deltas_key();
let total_deltas =
query_storage_value::<pos::TotalDeltas>(&client, &total_deltas_key)
.await
.expect("Total bonded stake should always be set");
let total_bonded_stake = total_deltas
.get(epoch)
.expect("Total voting power should be always set in the current epoch");
println!("Total voting power: {}", total_voting_power);
.expect("Total bonded stake should be always set in the current epoch");
let total_bonded_stake: u64 = total_bonded_stake
.try_into()
.expect("total_bonded_stake should be a positive value");

println!("Total bonded stake: {}", total_bonded_stake);
}

/// Query PoS validator's commission rate
Expand Down
2 changes: 1 addition & 1 deletion apps/src/lib/config/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ pub struct Validator {
pub protocol_key: common::PublicKey,
/// The public DKG session key used during the DKG protocol
pub dkg_public_key: DkgPublicKey,
/// These tokens are no staked and hence do not contribute to the
/// These tokens are not staked and hence do not contribute to the
/// validator's voting power
pub non_staked_balance: token::Amount,
/// Validity predicate code WASM
Expand Down
12 changes: 7 additions & 5 deletions apps/src/lib/node/ledger/shell/finalize_block.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Implementation of the `FinalizeBlock` ABCI++ method for the Shell
use namada::ledger::pos::types::into_tm_voting_power;
use namada::ledger::protocol;
use namada::types::storage::{BlockHash, BlockResults, Header};

Expand Down Expand Up @@ -292,18 +293,19 @@ where
fn update_epoch(&self, response: &mut shim::response::FinalizeBlock) {
// Apply validator set update
let (current_epoch, _gas) = self.storage.get_current_epoch();
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,
voting_power,
bonded_stake,
}) => {
let power: u64 = voting_power.into();
let power: i64 = power
.try_into()
.expect("unexpected validator's voting power");
let power: i64 = into_tm_voting_power(
pos_params.tm_votes_per_token,
bonded_stake,
);
(consensus_key, power)
}
ValidatorSetUpdate::Deactivated(consensus_key) => {
Expand Down
10 changes: 5 additions & 5 deletions apps/src/lib/node/ledger/shell/init_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::collections::HashMap;
use std::hash::Hash;

use namada::ledger::parameters::Parameters;
use namada::ledger::pos::into_tm_voting_power;
use namada::types::key::*;
#[cfg(not(feature = "dev"))]
use sha2::{Digest, Sha256};
Expand Down Expand Up @@ -318,11 +319,10 @@ where
sum: Some(key_to_tendermint(&consensus_key).unwrap()),
};
abci_validator.pub_key = Some(pub_key);
let power: u64 =
validator.pos_data.voting_power(&genesis.pos_params).into();
abci_validator.power = power
.try_into()
.expect("unexpected validator's voting power");
abci_validator.power = into_tm_voting_power(
genesis.pos_params.tm_votes_per_token,
validator.pos_data.tokens,
);
response.validators.push(abci_validator);
}
Ok(response)
Expand Down
8 changes: 7 additions & 1 deletion apps/src/lib/node/ledger/shell/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use borsh::{BorshDeserialize, BorshSerialize};
use ferveo_common::TendermintValidator;
use namada::ledger::pos::into_tm_voting_power;
use namada::ledger::queries::{RequestCtx, ResponseQuery};
use namada::ledger::storage_api;
use namada::types::address::Address;
Expand Down Expand Up @@ -90,6 +91,8 @@ where
.expect("Serializing public key should not fail");
// get the current epoch
let (current_epoch, _) = self.storage.get_current_epoch();
// get the PoS params
let pos_params = self.storage.read_pos_params();
// get the active validator set
self.storage
.read_validator_set()
Expand Down Expand Up @@ -121,7 +124,10 @@ where
"DKG public key in storage should be deserializable",
);
TendermintValidator {
power: validator.voting_power.into(),
power: into_tm_voting_power(
pos_params.tm_votes_per_token,
validator.bonded_stake,
) as u64,
address: validator.address.to_string(),
public_key: dkg_publickey.into(),
}
Expand Down
Loading

0 comments on commit 6cc4825

Please sign in to comment.