diff --git a/.changelog/unreleased/SDK/2330-governance-tests.md b/.changelog/unreleased/SDK/2330-governance-tests.md new file mode 100644 index 0000000000..f81043b7a2 --- /dev/null +++ b/.changelog/unreleased/SDK/2330-governance-tests.md @@ -0,0 +1,4 @@ +- Added `QueryProposalVotes` struct. Removes `VoteType`from + the `Display` implementation of `LedgerProposalVote`. Updates + `build_vote_proposal` to return an error if voter has no delegations. + ([\#2330](https://github.com/anoma/namada/pull/2330)) \ No newline at end of file diff --git a/.changelog/unreleased/improvements/2330-governance-tests.md b/.changelog/unreleased/improvements/2330-governance-tests.md new file mode 100644 index 0000000000..a94ea129df --- /dev/null +++ b/.changelog/unreleased/improvements/2330-governance-tests.md @@ -0,0 +1,3 @@ +- Adds a new `query_proposal_votes` query, improves the formatting of + `ProposalResult` and returns early in client if governance voter has no stake. + Misc refactors. ([\#2330](https://github.com/anoma/namada/pull/2330)) \ No newline at end of file diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 48f99422ae..21a415a8d4 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -257,6 +257,7 @@ pub mod cmds { .subcommand(QueryResult::def().display_order(5)) .subcommand(QueryRawBytes::def().display_order(5)) .subcommand(QueryProposal::def().display_order(5)) + .subcommand(QueryProposalVotes::def().display_order(5)) .subcommand(QueryProposalResult::def().display_order(5)) .subcommand(QueryProtocolParameters::def().display_order(5)) .subcommand(QueryPgf::def().display_order(5)) @@ -330,6 +331,8 @@ pub mod cmds { let query_result = Self::parse_with_ctx(matches, QueryResult); let query_raw_bytes = Self::parse_with_ctx(matches, QueryRawBytes); let query_proposal = Self::parse_with_ctx(matches, QueryProposal); + let query_proposal_votes = + Self::parse_with_ctx(matches, QueryProposalVotes); let query_proposal_result = Self::parse_with_ctx(matches, QueryProposalResult); let query_protocol_parameters = @@ -385,6 +388,7 @@ pub mod cmds { .or(query_result) .or(query_raw_bytes) .or(query_proposal) + .or(query_proposal_votes) .or(query_proposal_result) .or(query_protocol_parameters) .or(query_pgf) @@ -472,6 +476,7 @@ pub mod cmds { QueryFindValidator(QueryFindValidator), QueryRawBytes(QueryRawBytes), QueryProposal(QueryProposal), + QueryProposalVotes(QueryProposalVotes), QueryProposalResult(QueryProposalResult), QueryProtocolParameters(QueryProtocolParameters), QueryPgf(QueryPgf), @@ -1011,6 +1016,28 @@ pub mod cmds { } } + #[derive(Debug, Clone)] + pub struct QueryProposalVotes(pub args::QueryProposalVotes); + + impl SubCmd for QueryProposalVotes { + const CMD: &'static str = "query-proposal-votes"; + + fn parse(matches: &ArgMatches) -> Option + where + Self: Sized, + { + matches.subcommand_matches(Self::CMD).map(|matches| { + QueryProposalVotes(args::QueryProposalVotes::parse(matches)) + }) + } + + fn def() -> App { + App::new(Self::CMD) + .about("Query votes for the proposal.") + .add_args::>() + } + } + #[derive(Clone, Debug)] pub struct QueryProposal(pub args::QueryProposal); @@ -3063,6 +3090,7 @@ pub mod args { pub const VALIDATOR_ETH_HOT_KEY: ArgOpt = arg_opt("eth-hot-key"); pub const VALUE: Arg = arg("value"); + pub const VOTER_OPT: ArgOpt = arg_opt("voter"); pub const VIEWING_KEY: Arg = arg("key"); pub const VP: ArgOpt = arg_opt("vp"); pub const WALLET_ALIAS_FORCE: ArgFlag = flag("wallet-alias-force"); @@ -4801,6 +4829,36 @@ pub mod args { } } + impl CliToSdk> for QueryProposalVotes { + fn to_sdk(self, ctx: &mut Context) -> QueryProposalVotes { + QueryProposalVotes:: { + query: self.query.to_sdk(ctx), + proposal_id: self.proposal_id, + voter: self.voter.map(|x| ctx.borrow_chain_or_exit().get(&x)), + } + } + } + + impl Args for QueryProposalVotes { + fn parse(matches: &ArgMatches) -> Self { + let query = Query::parse(matches); + let proposal_id = PROPOSAL_ID.parse(matches); + let voter = VOTER_OPT.parse(matches); + + Self { + query, + proposal_id, + voter, + } + } + + fn def(app: App) -> App { + app.add_args::>() + .arg(PROPOSAL_ID_OPT.def().help("The proposal identifier.")) + .arg(VOTER_OPT.def().help("The address of the proposal voter.")) + } + } + #[derive(Clone, Debug)] pub struct QueryProposalResult { /// Common query args diff --git a/apps/src/lib/cli/client.rs b/apps/src/lib/cli/client.rs index 601988a5f8..2ae441d2d2 100644 --- a/apps/src/lib/cli/client.rs +++ b/apps/src/lib/cli/client.rs @@ -580,6 +580,17 @@ impl CliApi { let namada = ctx.to_sdk(client, io); rpc::query_proposal_result(&namada, args).await; } + Sub::QueryProposalVotes(QueryProposalVotes(mut args)) => { + let client = client.unwrap_or_else(|| { + C::from_tendermint_address( + &mut args.query.ledger_address, + ) + }); + client.wait_until_node_is_synced(&io).await?; + let args = args.to_sdk(&mut ctx); + let namada = ctx.to_sdk(client, io); + rpc::query_proposal_votes(&namada, args).await; + } Sub::QueryProtocolParameters(QueryProtocolParameters( mut args, )) => { diff --git a/apps/src/lib/client/rpc.rs b/apps/src/lib/client/rpc.rs index 489db7cc32..f0d6c34d91 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -733,6 +733,43 @@ async fn get_ibc_denom_alias( .unwrap_or(ibc_denom.as_ref().to_string()) } +/// Query votes for the given proposal +pub async fn query_proposal_votes( + context: &impl Namada, + args: args::QueryProposalVotes, +) { + let result = namada_sdk::rpc::query_proposal_votes( + context.client(), + args.proposal_id, + ) + .await + .unwrap(); + + match args.voter { + Some(voter) => { + match result.into_iter().find(|vote| vote.delegator == voter) { + Some(vote) => display_line!(context.io(), "{}", vote,), + None => display_line!( + context.io(), + "The address {} has not voted on proposal {}", + voter, + args.proposal_id + ), + } + } + None => { + display_line!( + context.io(), + "Votes for proposal id {}\n", + args.proposal_id + ); + for vote in result { + display_line!(context.io(), "{}\n", vote); + } + } + } +} + /// Query Proposals pub async fn query_proposal(context: &impl Namada, args: args::QueryProposal) { let current_epoch = query_and_print_epoch(context).await; diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index d76b12a7b5..c867affe75 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -11,8 +11,9 @@ use namada::core::ledger::governance::cli::offline::{ OfflineProposal, OfflineSignedProposal, OfflineVote, }; use namada::core::ledger::governance::cli::onchain::{ - DefaultProposal, PgfFundingProposal, PgfStewardProposal, ProposalVote, + DefaultProposal, PgfFundingProposal, PgfStewardProposal, }; +use namada::core::ledger::governance::storage::vote::ProposalVote; use namada::core::ledger::storage::EPOCH_SWITCH_BLOCKS_DELAY; use namada::ibc::apps::transfer::types::Memo; use namada::proto::{CompressedSignature, Section, Signer, Tx}; diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index 9d4ce98987..a69043b13a 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -817,9 +817,7 @@ mod test_finalize_block { use namada::core::ledger::eth_bridge::storage::wrapped_erc20s; use namada::core::ledger::governance::storage::keys::get_proposal_execution_key; use namada::core::ledger::governance::storage::proposal::ProposalType; - use namada::core::ledger::governance::storage::vote::{ - StorageProposalVote, VoteType, - }; + use namada::core::ledger::governance::storage::vote::ProposalVote; use namada::core::ledger::replay_protection; use namada::core::types::storage::KeySeg; use namada::eth_bridge::storage::bridge_pool::{ @@ -1644,8 +1642,8 @@ mod test_finalize_block { }; // Add a proposal to be accepted and one to be rejected. - add_proposal(0, StorageProposalVote::Yay(VoteType::Default)); - add_proposal(1, StorageProposalVote::Nay); + add_proposal(0, ProposalVote::Yay); + add_proposal(1, ProposalVote::Nay); // Commit the genesis state shell.wl_storage.commit_block().unwrap(); diff --git a/benches/native_vps.rs b/benches/native_vps.rs index ed6da9c22e..6dafdaa665 100644 --- a/benches/native_vps.rs +++ b/benches/native_vps.rs @@ -6,9 +6,7 @@ use std::str::FromStr; use criterion::{criterion_group, criterion_main, Criterion}; use masp_primitives::sapling::Node; use namada::core::ledger::governance::storage::proposal::ProposalType; -use namada::core::ledger::governance::storage::vote::{ - StorageProposalVote, VoteType, -}; +use namada::core::ledger::governance::storage::vote::ProposalVote; use namada::core::ledger::ibc::{IbcActions, TransferModule}; use namada::core::ledger::pgf::storage::steward::StewardDetail; use namada::core::ledger::storage_api::{StorageRead, StorageWrite}; @@ -83,7 +81,7 @@ fn governance(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay(VoteType::Default), + vote: ProposalVote::Yay, voter: defaults::albert_address(), delegations: vec![defaults::validator_address()], }, @@ -99,7 +97,7 @@ fn governance(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Nay, + vote: ProposalVote::Nay, voter: defaults::validator_address(), delegations: vec![], }, diff --git a/benches/txs.rs b/benches/txs.rs index e6493d6bbf..d3c3e3cda9 100644 --- a/benches/txs.rs +++ b/benches/txs.rs @@ -3,9 +3,7 @@ use std::str::FromStr; use criterion::{criterion_group, criterion_main, Criterion}; use namada::core::ledger::governance::storage::proposal::ProposalType; -use namada::core::ledger::governance::storage::vote::{ - StorageProposalVote, VoteType, -}; +use namada::core::ledger::governance::storage::vote::ProposalVote; use namada::core::ledger::pgf::storage::steward::StewardDetail; use namada::core::types::key::{ common, SecretKey as SecretKeyInterface, SigScheme, @@ -550,7 +548,7 @@ fn vote_proposal(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay(VoteType::Default), + vote: ProposalVote::Yay, voter: defaults::albert_address(), delegations: vec![defaults::validator_address()], }, @@ -563,7 +561,7 @@ fn vote_proposal(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Nay, + vote: ProposalVote::Nay, voter: defaults::validator_address(), delegations: vec![], }, diff --git a/benches/vps.rs b/benches/vps.rs index 620d14c43f..2a0036aadf 100644 --- a/benches/vps.rs +++ b/benches/vps.rs @@ -1,9 +1,7 @@ use std::collections::BTreeSet; use criterion::{criterion_group, criterion_main, Criterion}; -use namada::core::ledger::governance::storage::vote::{ - StorageProposalVote, VoteType, -}; +use namada::core::ledger::governance::storage::vote::ProposalVote; use namada::core::types::address::{self, Address}; use namada::core::types::key::{ common, SecretKey as SecretKeyInterface, SigScheme, @@ -99,7 +97,7 @@ fn vp_user(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay(VoteType::Default), + vote: ProposalVote::Yay, voter: defaults::albert_address(), delegations: vec![defaults::validator_address()], }, @@ -239,7 +237,7 @@ fn vp_implicit(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay(VoteType::Default), + vote: ProposalVote::Yay, voter: Address::from(&implicit_account.to_public()), delegations: vec![], /* NOTE: no need to bond tokens because the * implicit vp doesn't check that */ @@ -396,7 +394,7 @@ fn vp_validator(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay(VoteType::Default), + vote: ProposalVote::Yay, voter: defaults::validator_address(), delegations: vec![], }, diff --git a/core/src/ledger/governance/cli/offline.rs b/core/src/ledger/governance/cli/offline.rs index ccf06a0b98..cdd800a600 100644 --- a/core/src/ledger/governance/cli/offline.rs +++ b/core/src/ledger/governance/cli/offline.rs @@ -6,8 +6,8 @@ use borsh::{BorshDeserialize, BorshSerialize}; use borsh_ext::BorshSerializeExt; use serde::{Deserialize, Serialize}; -use super::onchain::ProposalVote; use super::validation::{is_valid_tally_epoch, ProposalValidation}; +use crate::ledger::governance::storage::vote::ProposalVote; use crate::proto::SignatureIndex; use crate::types::account::AccountPublicKeysMap; use crate::types::address::Address; @@ -251,11 +251,6 @@ impl OfflineVote { self.vote.is_abstain() } - /// Check if two votes are equal - pub fn is_same_side(&self, other: &Self) -> bool { - self.vote.is_same_side(&other.vote) - } - /// compute the hash of a proposal pub fn compute_hash(&self) -> Hash { let proposal_hash_data = self.proposal_hash.serialize_to_vec(); diff --git a/core/src/ledger/governance/cli/onchain.rs b/core/src/ledger/governance/cli/onchain.rs index bbfd25ca70..4cf6e7456e 100644 --- a/core/src/ledger/governance/cli/onchain.rs +++ b/core/src/ledger/governance/cli/onchain.rs @@ -302,57 +302,3 @@ pub struct PgfRetro { /// Pgf retro target pub target: PGFTarget, } - -/// Represent an proposal vote -#[derive( - Debug, - Clone, - BorshSerialize, - BorshDeserialize, - Serialize, - Deserialize, - PartialEq, -)] -pub enum ProposalVote { - /// Represent an yay proposal vote - Yay, - /// Represent an nay proposal vote - Nay, - /// Represent an abstain proposal vote - Abstain, -} - -impl TryFrom for ProposalVote { - type Error = String; - - fn try_from(value: String) -> Result { - match value.trim().to_lowercase().as_str() { - "yay" => Ok(ProposalVote::Yay), - "nay" => Ok(ProposalVote::Nay), - "abstain" => Ok(ProposalVote::Abstain), - _ => Err("invalid vote".to_string()), - } - } -} - -impl ProposalVote { - /// Check if the vote type is yay - pub fn is_yay(&self) -> bool { - matches!(self, ProposalVote::Yay) - } - - /// Check if the vote type is nay - pub fn is_nay(&self) -> bool { - matches!(self, ProposalVote::Nay) - } - - /// Check if the vote type is abstain - pub fn is_abstain(&self) -> bool { - matches!(self, ProposalVote::Abstain) - } - - /// Check if two votes are equal - pub fn is_same_side(&self, other: &Self) -> bool { - std::mem::discriminant(self) == std::mem::discriminant(other) - } -} diff --git a/core/src/ledger/governance/storage/vote.rs b/core/src/ledger/governance/storage/vote.rs index 9be49fa9b7..2016383664 100644 --- a/core/src/ledger/governance/storage/vote.rs +++ b/core/src/ledger/governance/storage/vote.rs @@ -3,29 +3,6 @@ use std::fmt::Display; use borsh::{BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; -use super::super::cli::onchain::ProposalVote; -use super::proposal::ProposalType; - -/// The type of a governance vote with the optional associated Memo -#[derive( - Debug, - Clone, - PartialEq, - BorshSerialize, - BorshDeserialize, - Eq, - Serialize, - Deserialize, -)] -pub enum VoteType { - /// A default vote without Memo - Default, - /// A vote for the PGF stewards - PGFSteward, - /// A vote for a PGF payment proposal - PGFPayment, -} - #[derive( Debug, Clone, @@ -37,110 +14,51 @@ pub enum VoteType { Deserialize, )] /// The vote for a proposal -pub enum StorageProposalVote { +pub enum ProposalVote { /// Yes - Yay(VoteType), + Yay, /// No Nay, /// Abstain Abstain, } -impl StorageProposalVote { +impl ProposalVote { /// Check if a vote is yay pub fn is_yay(&self) -> bool { - matches!(self, StorageProposalVote::Yay(_)) + matches!(self, ProposalVote::Yay) } /// Check if a vote is nay pub fn is_nay(&self) -> bool { - matches!(self, StorageProposalVote::Nay) + matches!(self, ProposalVote::Nay) } /// Check if a vote is abstain pub fn is_abstain(&self) -> bool { - matches!(self, StorageProposalVote::Abstain) - } - - /// Check if two votes are equal - pub fn is_same_side(&self, other: &Self) -> bool { - std::mem::discriminant(self) == std::mem::discriminant(other) - } - - /// Check if vote is of type default - pub fn is_default_vote(&self) -> bool { - matches!( - self, - StorageProposalVote::Yay(VoteType::Default) - | StorageProposalVote::Nay - ) - } - - /// Check if a vote is compatible with a proposal - pub fn is_compatible(&self, proposal_type: &ProposalType) -> bool { - match self { - StorageProposalVote::Yay(vote_type) => proposal_type.eq(vote_type), - StorageProposalVote::Nay => true, - StorageProposalVote::Abstain => true, - } - } - - /// Create a new vote - pub fn build( - proposal_vote: &ProposalVote, - proposal_type: &ProposalType, - ) -> Option { - match (proposal_vote, proposal_type) { - (ProposalVote::Yay, ProposalType::Default(_)) => { - Some(StorageProposalVote::Yay(VoteType::Default)) - } - (ProposalVote::Yay, ProposalType::PGFSteward(_)) => { - Some(StorageProposalVote::Yay(VoteType::PGFSteward)) - } - (ProposalVote::Yay, ProposalType::PGFPayment(_)) => { - Some(StorageProposalVote::Yay(VoteType::PGFPayment)) - } - (ProposalVote::Nay, ProposalType::Default(_)) => { - Some(StorageProposalVote::Nay) - } - (ProposalVote::Nay, ProposalType::PGFSteward(_)) => { - Some(StorageProposalVote::Nay) - } - (ProposalVote::Nay, ProposalType::PGFPayment(_)) => { - Some(StorageProposalVote::Nay) - } - _ => None, - } + matches!(self, ProposalVote::Abstain) } } -impl Display for StorageProposalVote { +impl Display for ProposalVote { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - StorageProposalVote::Yay(vote_type) => match vote_type { - VoteType::Default - | VoteType::PGFSteward - | VoteType::PGFPayment => write!(f, "yay"), - }, - - StorageProposalVote::Nay => write!(f, "nay"), - StorageProposalVote::Abstain => write!(f, "abstain"), + ProposalVote::Yay => write!(f, "yay"), + ProposalVote::Nay => write!(f, "nay"), + ProposalVote::Abstain => write!(f, "abstain"), } } } -impl PartialEq for ProposalType { - fn eq(&self, other: &VoteType) -> bool { - match self { - Self::Default(_) => { - matches!(other, VoteType::Default) - } - Self::PGFSteward(_) => { - matches!(other, VoteType::PGFSteward) - } - Self::PGFPayment(_) => { - matches!(other, VoteType::PGFPayment) - } +impl TryFrom for ProposalVote { + type Error = String; + + fn try_from(value: String) -> Result { + match value.trim().to_lowercase().as_str() { + "yay" => Ok(ProposalVote::Yay), + "nay" => Ok(ProposalVote::Nay), + "abstain" => Ok(ProposalVote::Abstain), + _ => Err("invalid vote".to_string()), } } } @@ -149,28 +67,14 @@ impl PartialEq for ProposalType { /// Testing helpers and strategies for governance votes pub mod testing { use proptest::prelude::{Just, Strategy}; - use proptest::prop_compose; use super::*; - prop_compose! { - /// Geerate an arbitrary vote type - pub fn arb_vote_type()(discriminant in 0..3) -> VoteType { - match discriminant { - 0 => VoteType::Default, - 1 => VoteType::PGFSteward, - 2 => VoteType::PGFPayment, - _ => unreachable!(), - } - } - } - /// Generate an arbitrary proposal vote - pub fn arb_proposal_vote() -> impl Strategy { - arb_vote_type() - .prop_map(StorageProposalVote::Yay) + pub fn arb_proposal_vote() -> impl Strategy { + Just(ProposalVote::Yay) .boxed() - .prop_union(Just(StorageProposalVote::Nay).boxed()) - .or(Just(StorageProposalVote::Abstain).boxed()) + .prop_union(Just(ProposalVote::Nay).boxed()) + .or(Just(ProposalVote::Abstain).boxed()) } } diff --git a/core/src/ledger/governance/utils.rs b/core/src/ledger/governance/utils.rs index 6b3129b588..fc23a27afe 100644 --- a/core/src/ledger/governance/utils.rs +++ b/core/src/ledger/governance/utils.rs @@ -5,7 +5,7 @@ use borsh::{BorshDeserialize, BorshSerialize}; use super::cli::offline::OfflineVote; use super::storage::proposal::ProposalType; -use super::storage::vote::StorageProposalVote; +use super::storage::vote::ProposalVote; use crate::types::address::Address; use crate::types::storage::Epoch; use crate::types::token; @@ -41,7 +41,14 @@ pub struct Vote { /// Field holding the address of the delegator pub delegator: Address, /// Field holding vote data - pub data: StorageProposalVote, + pub data: ProposalVote, +} + +impl Display for Vote { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + writeln!(f, "Voter: {}", self.delegator)?; + write!(f, "Vote: {}", self.data) + } } impl Vote { @@ -52,6 +59,7 @@ impl Vote { } /// Represent a tally type +#[derive(Copy, Clone, BorshSerialize, BorshDeserialize)] pub enum TallyType { /// Represent a tally type for proposal requiring 2/3 of the total voting /// power to be yay @@ -81,7 +89,7 @@ impl TallyType { } /// The result of a proposal -#[derive(Copy, Clone, BorshSerialize, BorshDeserialize)] +#[derive(Copy, Clone, Debug, BorshSerialize, BorshDeserialize)] pub enum TallyResult { /// Proposal was accepted with the associated value Passed, @@ -142,6 +150,8 @@ impl TallyResult { pub struct ProposalResult { /// The result of a proposal pub result: TallyResult, + /// The type of tally required for this proposal + pub tally_type: TallyType, /// The total voting power during the proposal tally pub total_voting_power: VotePower, /// The total voting power from yay votes @@ -152,27 +162,6 @@ pub struct ProposalResult { pub total_abstain_power: VotePower, } -impl Display for ProposalResult { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let percentage = self - .total_yay_power - .checked_div(self.total_voting_power) - .unwrap_or_default(); - - write!( - f, - "{} with {} yay votes and {} nay votes ({:.2}%)", - self.result, - self.total_yay_power.to_string_native(), - self.total_nay_power.to_string_native(), - percentage - .checked_mul(token::Amount::from_u64(100)) - .unwrap_or_default() - .to_string_native() - ) - } -} - impl ProposalResult { /// Return true if at least 1/3 of the total voting power voted and at least /// two third of the non-abstained voting power voted nay @@ -189,16 +178,41 @@ impl ProposalResult { } } +impl Display for ProposalResult { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let threshold = match self.tally_type { + TallyType::TwoThirds => self.total_voting_power / 3 * 2, + _ => { + let threshold_one_third = self.total_voting_power / 3; + threshold_one_third / 2 + } + }; + + write!( + f, + "{} with {} yay votes, {} nay votes and {} abstain votes, total \ + voting power: {} threshold was: {}", + self.result, + self.total_yay_power.to_string_native(), + self.total_nay_power.to_string_native(), + self.total_abstain_power.to_string_native(), + self.total_voting_power.to_string_native(), + threshold.to_string_native() + ) + } +} + /// /// General rappresentation of a vote +#[derive(Debug)] pub enum TallyVote { /// Rappresent a vote for a proposal onchain - OnChain(StorageProposalVote), + OnChain(ProposalVote), /// Rappresent a vote for a proposal offline Offline(OfflineVote), } -impl From for TallyVote { - fn from(vote: StorageProposalVote) -> Self { +impl From for TallyVote { + fn from(vote: ProposalVote) -> Self { Self::OnChain(vote) } } @@ -242,10 +256,10 @@ impl TallyVote { ) -> Result { match (self, other) { (TallyVote::OnChain(vote), TallyVote::OnChain(other_vote)) => { - Ok(vote.is_same_side(other_vote)) + Ok(vote == other_vote) } (TallyVote::Offline(vote), TallyVote::Offline(other_vote)) => { - Ok(vote.is_same_side(other_vote)) + Ok(vote.vote == other_vote.vote) } _ => Err("Cannot compare different variants of governance votes"), } @@ -254,6 +268,7 @@ impl TallyVote { /// Proposal structure holding votes information necessary to compute the /// outcome +#[derive(Default, Debug)] pub struct ProposalVotes { /// Map from validator address to vote pub validators_vote: HashMap, @@ -269,7 +284,7 @@ pub struct ProposalVotes { pub fn compute_proposal_result( votes: ProposalVotes, total_voting_power: VotePower, - tally_at: TallyType, + tally_type: TallyType, ) -> ProposalResult { let mut yay_voting_power = VotePower::default(); let mut nay_voting_power = VotePower::default(); @@ -310,6 +325,7 @@ pub fn compute_proposal_result( // Force failure of the proposal return ProposalResult { result: TallyResult::Rejected, + tally_type, total_voting_power: VotePower::default(), total_yay_power: VotePower::default(), total_nay_power: VotePower::default(), @@ -352,7 +368,7 @@ pub fn compute_proposal_result( } let tally_result = TallyResult::new( - &tally_at, + &tally_type, yay_voting_power, nay_voting_power, abstain_voting_power, @@ -361,6 +377,7 @@ pub fn compute_proposal_result( ProposalResult { result: tally_result, + tally_type, total_voting_power, total_yay_power: yay_voting_power, total_nay_power: nay_voting_power, diff --git a/core/src/ledger/storage_api/governance.rs b/core/src/ledger/storage_api/governance.rs index 3f3dfb2af6..4daf731fb4 100644 --- a/core/src/ledger/storage_api/governance.rs +++ b/core/src/ledger/storage_api/governance.rs @@ -10,7 +10,7 @@ use crate::ledger::governance::storage::keys as governance_keys; use crate::ledger::governance::storage::proposal::{ ProposalType, StorageProposal, }; -use crate::ledger::governance::storage::vote::StorageProposalVote; +use crate::ledger::governance::storage::vote::ProposalVote; use crate::ledger::governance::utils::Vote; use crate::ledger::governance::ADDRESS as governance_address; use crate::ledger::storage_api::{self, StorageRead, StorageWrite}; @@ -168,10 +168,8 @@ where { let vote_prefix_key = governance_keys::get_proposal_vote_prefix_key(proposal_id); - let vote_iter = storage_api::iter_prefix::( - storage, - &vote_prefix_key, - )?; + let vote_iter = + storage_api::iter_prefix::(storage, &vote_prefix_key)?; let votes = vote_iter .filter_map(|vote_result| { diff --git a/core/src/types/transaction/governance.rs b/core/src/types/transaction/governance.rs index 5876d8de60..fb16a6592f 100644 --- a/core/src/types/transaction/governance.rs +++ b/core/src/types/transaction/governance.rs @@ -10,7 +10,7 @@ use crate::ledger::governance::cli::onchain::{ use crate::ledger::governance::storage::proposal::{ AddRemove, PGFAction, ProposalType, }; -use crate::ledger::governance::storage::vote::StorageProposalVote; +use crate::ledger::governance::storage::vote::ProposalVote; use crate::types::address::Address; use crate::types::hash::Hash; use crate::types::storage::Epoch; @@ -73,7 +73,7 @@ pub struct VoteProposalData { /// The proposal id pub id: u64, /// The proposal vote - pub vote: StorageProposalVote, + pub vote: ProposalVote, /// The proposal author address pub voter: Address, /// Delegator addresses diff --git a/light_sdk/src/transaction/governance.rs b/light_sdk/src/transaction/governance.rs index ca6183569e..c09f0fc8c1 100644 --- a/light_sdk/src/transaction/governance.rs +++ b/light_sdk/src/transaction/governance.rs @@ -1,5 +1,5 @@ use namada_core::ledger::governance::storage::proposal::ProposalType; -use namada_core::ledger::governance::storage::vote::StorageProposalVote; +use namada_core::ledger::governance::storage::vote::ProposalVote; use namada_core::proto::Tx; use namada_core::types::address::Address; use namada_core::types::hash::Hash; @@ -75,7 +75,7 @@ impl VoteProposal { /// Build a raw VoteProposal transaction from the given parameters pub fn new( id: u64, - vote: StorageProposalVote, + vote: ProposalVote, voter: Address, delegations: Vec
, args: GlobalArgs, diff --git a/sdk/src/args.rs b/sdk/src/args.rs index 40d25bd2f0..b73907f4d9 100644 --- a/sdk/src/args.rs +++ b/sdk/src/args.rs @@ -1134,6 +1134,17 @@ impl RevealPk { } } +/// Query proposal votes +#[derive(Clone, Debug)] +pub struct QueryProposalVotes { + /// Common query args + pub query: Query, + /// Proposal id + pub proposal_id: u64, + /// Voter address + pub voter: Option, +} + /// Query proposal #[derive(Clone, Debug)] pub struct QueryProposal { diff --git a/sdk/src/queries/vp/governance.rs b/sdk/src/queries/vp/governance.rs index 1e3a5a8ece..ce3633f396 100644 --- a/sdk/src/queries/vp/governance.rs +++ b/sdk/src/queries/vp/governance.rs @@ -15,7 +15,7 @@ router! {GOV, ( "parameters" ) -> GovernanceParameters = parameters, } -/// Find if the given address belongs to a validator account. +/// Query the provided proposal id fn proposal_id( ctx: RequestCtx<'_, D, H, V, T>, id: u64, @@ -27,7 +27,7 @@ where storage_api::governance::get_proposal_by_id(ctx.wl_storage, id) } -/// Find if the given address belongs to a validator account. +/// Query all the votes for the given proposal id fn proposal_id_votes( ctx: RequestCtx<'_, D, H, V, T>, id: u64, @@ -39,7 +39,7 @@ where storage_api::governance::get_proposal_votes(ctx.wl_storage, id) } -/// Get the governane parameters +/// Get the governance parameters fn parameters( ctx: RequestCtx<'_, D, H, V, T>, ) -> storage_api::Result diff --git a/sdk/src/signing.rs b/sdk/src/signing.rs index 38aec72d4b..e52eb668b4 100644 --- a/sdk/src/signing.rs +++ b/sdk/src/signing.rs @@ -39,9 +39,7 @@ use tokio::sync::RwLock; use super::masp::{ShieldedContext, ShieldedTransfer}; use crate::args::SdkTypes; use crate::core::ledger::governance::storage::proposal::ProposalType; -use crate::core::ledger::governance::storage::vote::{ - StorageProposalVote, VoteType, -}; +use crate::core::ledger::governance::storage::vote::ProposalVote; use crate::core::types::eth_bridge_pool::PendingTransfer; use crate::error::{EncodingError, Error, TxError}; use crate::ibc::apps::transfer::types::msgs::transfer::MsgTransfer; @@ -895,21 +893,14 @@ fn to_ledger_decimal(amount: &str) -> String { /// A ProposalVote wrapper that prints the spending cap with Ledger decimal /// formatting. -struct LedgerProposalVote<'a>(&'a StorageProposalVote); +struct LedgerProposalVote<'a>(&'a ProposalVote); impl<'a> Display for LedgerProposalVote<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match &self.0 { - StorageProposalVote::Yay(vote_type) => match vote_type { - VoteType::Default => write!(f, "yay"), - VoteType::PGFSteward => write!(f, "yay for PGF steward"), - VoteType::PGFPayment => { - write!(f, "yay for PGF payment proposal") - } - }, - - StorageProposalVote::Nay => write!(f, "nay"), - StorageProposalVote::Abstain => write!(f, "abstain"), + ProposalVote::Yay => write!(f, "yay"), + ProposalVote::Nay => write!(f, "nay"), + ProposalVote::Abstain => write!(f, "abstain"), } } } diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index 4a42a41b1e..beb7dd448f 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -26,10 +26,9 @@ use namada_core::ibc::core::host::types::identifiers::{ChannelId, PortId}; use namada_core::ibc::primitives::{Msg, Timestamp as IbcTimestamp}; use namada_core::ledger::governance::cli::onchain::{ DefaultProposal, OnChainProposal, PgfFundingProposal, PgfStewardProposal, - ProposalVote, }; use namada_core::ledger::governance::storage::proposal::ProposalType; -use namada_core::ledger::governance::storage::vote::StorageProposalVote; +use namada_core::ledger::governance::storage::vote::ProposalVote; use namada_core::ledger::ibc::storage::channel_key; use namada_core::ledger::pgf::cli::steward::Commission; use namada_core::types::address::{Address, InternalAddress, MASP}; @@ -1845,14 +1844,6 @@ pub async fn build_vote_proposal( return Err(Error::from(TxError::ProposalDoesNotExist(proposal_id))); }; - let storage_vote = - StorageProposalVote::build(&proposal_vote, &proposal.r#type) - .ok_or_else(|| { - Error::from(TxError::Other( - "Should be able to build the proposal vote".to_string(), - )) - })?; - let is_validator = rpc::is_validator(context.client(), voter).await?; if !proposal.can_be_voted(epoch, is_validator) { @@ -1875,9 +1866,15 @@ pub async fn build_vote_proposal( .cloned() .collect::>(); + if delegations.is_empty() { + return Err(Error::Other( + "Voter address must have delegations".to_string(), + )); + } + let data = VoteProposalData { id: proposal_id, - vote: storage_vote, + vote: proposal_vote, voter: voter.clone(), delegations, }; diff --git a/shared/src/ledger/governance/mod.rs b/shared/src/ledger/governance/mod.rs index 610aaa831f..edcf77ff2d 100644 --- a/shared/src/ledger/governance/mod.rs +++ b/shared/src/ledger/governance/mod.rs @@ -9,7 +9,6 @@ use namada_core::ledger::governance::storage::keys as gov_storage; use namada_core::ledger::governance::storage::proposal::{ AddRemove, ProposalType, }; -use namada_core::ledger::governance::storage::vote::StorageProposalVote; use namada_core::ledger::governance::utils::is_valid_validator_voting_period; use namada_core::ledger::storage; use namada_core::ledger::storage_api::account; @@ -188,7 +187,6 @@ where gov_storage::get_voting_start_epoch_key(proposal_id); let voting_end_epoch_key = gov_storage::get_voting_end_epoch_key(proposal_id); - let proposal_type_key = gov_storage::get_proposal_type_key(proposal_id); let current_epoch = self.ctx.get_block_epoch()?; @@ -197,14 +195,10 @@ where self.force_read(&voting_start_epoch_key, ReadType::Pre)?; let pre_voting_end_epoch: Epoch = self.force_read(&voting_end_epoch_key, ReadType::Pre)?; - let proposal_type: ProposalType = - self.force_read(&proposal_type_key, ReadType::Pre)?; let voter = gov_storage::get_voter_address(key); let delegation_address = gov_storage::get_vote_delegation_address(key); - let vote: StorageProposalVote = self.force_read(key, ReadType::Post)?; - let (voter_address, delegation_address) = match (voter, delegation_address) { (Some(voter_address), Some(delegator_address)) => { @@ -238,10 +232,6 @@ where return Ok(false); } - if !vote.is_compatible(&proposal_type) { - return Err(Error::InvalidVoteType); - } - // first check if validator, then check if delegator let is_validator = self .is_validator( diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs index 299de13e96..4451be295a 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -1995,8 +1995,8 @@ fn proposal_submission() -> Result<()> { // this is valid because the client filter ALBERT delegation and there are // none let mut client = run!(test, Bin::Client, submit_proposal_vote, Some(15))?; - client.exp_string(TX_APPLIED_SUCCESS)?; - client.assert_success(); + client.exp_string("Voter address must have delegations")?; + client.assert_failure(); // 11. Query the proposal and check the result let mut epoch = get_epoch(&test, &validator_one_rpc).unwrap(); @@ -2016,7 +2016,9 @@ fn proposal_submission() -> Result<()> { let mut client = run!(test, Bin::Client, query_proposal, Some(15))?; client.exp_string("Proposal Id: 0")?; client.exp_string( - "passed with 100000.000000 yay votes and 900.000000 nay votes (0.%)", + "passed with 100000.000000 yay votes, 900.000000 nay votes and \ + 0.000000 abstain votes, total voting power: 100900.000000 threshold \ + was: 67266.666666", )?; client.assert_success();