diff --git a/.changelog/unreleased/improvements/2365-required-proposal-id.md b/.changelog/unreleased/improvements/2365-required-proposal-id.md new file mode 100644 index 0000000000..4debed4491 --- /dev/null +++ b/.changelog/unreleased/improvements/2365-required-proposal-id.md @@ -0,0 +1,2 @@ +- When constructing a governance proposal the id is now a required field. + ([\#2365](https://github.com/anoma/namada/pull/2365)) \ No newline at end of file diff --git a/apps/src/lib/bench_utils.rs b/apps/src/lib/bench_utils.rs index ddfae1a4cc..da55dfc8a8 100644 --- a/apps/src/lib/bench_utils.rs +++ b/apps/src/lib/bench_utils.rs @@ -244,7 +244,7 @@ impl Default for BenchShell { let signed_tx = bench_shell.generate_tx( TX_INIT_PROPOSAL_WASM, InitProposalData { - id: None, + id: 0, content: content_section.get_hash(), author: defaults::albert_address(), r#type: ProposalType::Default(None), diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index fca330ec48..3807f16b6d 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); @@ -3058,6 +3085,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"); @@ -4788,6 +4816,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 bc57f58f14..7997db2706 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -726,6 +726,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 f57cddd4eb..faf5236d2b 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 0e46577a0d..7ec6e3dd42 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::{ @@ -1614,7 +1612,7 @@ mod test_finalize_block { shell.proposal_data.insert(proposal_id); let proposal = InitProposalData { - id: Some(proposal_id), + id: proposal_id, content: Hash::default(), author: validator.clone(), voting_start_epoch: Epoch::default(), @@ -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 a1cf7e42f1..6959dfafe4 100644 --- a/benches/native_vps.rs +++ b/benches/native_vps.rs @@ -7,9 +7,7 @@ use criterion::{criterion_group, criterion_main, Criterion}; use masp_primitives::bls12_381; 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}; @@ -87,7 +85,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()], }, @@ -103,7 +101,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![], }, @@ -128,7 +126,7 @@ fn governance(c: &mut Criterion) { shell.generate_tx( TX_INIT_PROPOSAL_WASM, InitProposalData { - id: None, + id: 0, content: content_section.get_hash(), author: defaults::albert_address(), r#type: ProposalType::Default(None), @@ -180,7 +178,7 @@ fn governance(c: &mut Criterion) { shell.generate_tx( TX_INIT_PROPOSAL_WASM, InitProposalData { - id: Some(1), + id: 1, content: content_section.get_hash(), author: defaults::albert_address(), r#type: ProposalType::Default(Some( @@ -252,7 +250,7 @@ fn governance(c: &mut Criterion) { // let governance_proposal = shell.generate_tx( // TX_INIT_PROPOSAL_WASM, // InitProposalData { -// id: None, +// id: 0, // content: content_section.get_hash(), // author: defaults::albert_address(), // r#type: ProposalType::Default(None), diff --git a/benches/txs.rs b/benches/txs.rs index 523cd48489..2e32fed2ef 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, @@ -464,7 +462,7 @@ fn init_proposal(c: &mut Criterion) { shell.generate_tx( TX_INIT_PROPOSAL_WASM, InitProposalData { - id: None, + id: 0, content: content_section.get_hash(), author: defaults::albert_address(), r#type: ProposalType::Default(None), @@ -514,7 +512,7 @@ fn init_proposal(c: &mut Criterion) { shell.generate_tx( TX_INIT_PROPOSAL_WASM, InitProposalData { - id: Some(1), + id: 1, content: content_section.get_hash(), author: defaults::albert_address(), r#type: ProposalType::Default(Some( @@ -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 60e70ddf7d..14c93a068d 100644 --- a/core/src/ledger/governance/cli/onchain.rs +++ b/core/src/ledger/governance/cli/onchain.rs @@ -20,7 +20,7 @@ use crate::types::storage::Epoch; /// The proposal structure pub struct OnChainProposal { /// The proposal id - pub id: Option, + pub id: u64, /// The proposal content pub content: BTreeMap, /// The proposal author address @@ -312,57 +312,3 @@ pub struct PgfFundingTarget { /// Target address pub address: Address, } - -/// 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..592734eb89 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}; @@ -31,11 +31,10 @@ where S: StorageRead + StorageWrite, { let counter_key = governance_keys::get_counter_key(); - let proposal_id = if let Some(id) = data.id { - id - } else { - storage.read(&counter_key)?.unwrap() - }; + let proposal_id = storage.read(&counter_key)?.expect( + "Storage should have been initialized with an initial governance \ + proposal id", + ); let content_key = governance_keys::get_content_key(proposal_id); storage.write_bytes(&content_key, content)?; @@ -168,10 +167,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/dec.rs b/core/src/types/dec.rs index 4ec4e223ef..878ac57914 100644 --- a/core/src/types/dec.rs +++ b/core/src/types/dec.rs @@ -67,9 +67,9 @@ impl Dec { /// the division is impossible (e.g., division by zero or overflow), `None` /// is returned. /// - /// Example: - /// ``` - /// + /// For instance: + /// + /// ```ignore /// let x = Dec::new(3, 1).unwrap(); // Represents 0.3 /// let y = Dec::new(2, 1).unwrap(); // Represents 0.2 /// let result = x.trunc_div(&y).unwrap(); @@ -77,9 +77,11 @@ impl Dec { /// ``` /// /// # Arguments + /// /// * `rhs`: The right-hand side `Dec` value for the division. /// /// # Returns + /// /// An `Option` which is `Some` with the result if the division is /// successful, or `None` if the division cannot be performed. pub fn trunc_div(&self, rhs: &Self) -> Option { diff --git a/core/src/types/transaction/governance.rs b/core/src/types/transaction/governance.rs index 8e43b488e3..ce41734192 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, PGFTarget, 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; @@ -34,7 +34,7 @@ pub enum ProposalError { )] pub struct InitProposalData { /// The proposal id - pub id: Option, + pub id: u64, /// The proposal content pub content: Hash, /// The proposal author address @@ -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 @@ -180,7 +180,7 @@ pub mod tests { prop_compose! { /// Generate a proposal initialization pub fn arb_init_proposal()( - id: Option, + id: u64, content in arb_hash(), author in arb_non_internal_address(), r#type in arb_proposal_type(), diff --git a/light_sdk/src/transaction/governance.rs b/light_sdk/src/transaction/governance.rs index ca6183569e..a3a37a6fd2 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; @@ -19,7 +19,7 @@ impl InitProposal { /// Build a raw InitProposal transaction from the given parameters #[allow(clippy::too_many_arguments)] pub fn new( - id: Option, + id: u64, content: Hash, author: Address, r#type: ProposalType, @@ -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 c12ecdf537..9b465671e7 100644 --- a/sdk/src/args.rs +++ b/sdk/src/args.rs @@ -1129,6 +1129,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..87488cf4a2 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"), } } } @@ -1095,9 +1086,7 @@ pub async fn to_ledger_vector( .hash(); tv.output.push("Type : Init proposal".to_string()); - if let Some(id) = init_proposal_data.id.as_ref() { - tv.output.push(format!("ID : {}", id)); - } + tv.output.push(format!("ID : {}", init_proposal_data.id)); tv.output.extend(vec![ format!( "Proposal type : {}", @@ -1116,9 +1105,8 @@ pub async fn to_ledger_vector( format!("Content : {}", HEXLOWER.encode(&extra.0)), ]); - if let Some(id) = init_proposal_data.id.as_ref() { - tv.output_expert.push(format!("ID : {}", id)); - } + tv.output_expert + .push(format!("ID : {}", init_proposal_data.id)); tv.output_expert.extend(vec![ format!( "Proposal type : {}", diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index de0474615b..e86bfc0c1c 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}; @@ -1843,14 +1842,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) { @@ -1873,9 +1864,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 ca123601a1..4184c6de7e 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -1811,6 +1811,7 @@ fn proposal_submission() -> Result<()> { let albert = find_address(&test, ALBERT)?; let valid_proposal_json_path = prepare_proposal_data( &test, + 0, albert, TestWasms::TxProposalCode.read_bytes(), 12, @@ -1883,6 +1884,7 @@ fn proposal_submission() -> Result<()> { let albert = find_address(&test, ALBERT)?; let invalid_proposal_json = prepare_proposal_data( &test, + 1, albert, TestWasms::TxProposalCode.read_bytes(), 1, @@ -1992,8 +1994,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(); @@ -2013,7 +2015,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(); @@ -2139,7 +2143,7 @@ fn pgf_governance_proposal() -> Result<()> { }; let valid_proposal_json_path = - prepare_proposal_data(&test, albert, pgf_stewards, 12); + prepare_proposal_data(&test, 0, albert, pgf_stewards, 12); let validator_one_rpc = get_actor_rpc(&test, Who::Validator(0)); let submit_proposal_args = vec![ @@ -2328,7 +2332,7 @@ fn pgf_governance_proposal() -> Result<()> { }; let valid_proposal_json_path = - prepare_proposal_data(&test, albert, pgf_funding, 36); + prepare_proposal_data(&test, 1, albert, pgf_funding, 36); let validator_one_rpc = get_actor_rpc(&test, Who::Validator(0)); let submit_proposal_args = vec![ @@ -2924,6 +2928,7 @@ fn implicit_account_reveal_pk() -> Result<()> { let author = find_address(&test, source).unwrap(); let valid_proposal_json_path = prepare_proposal_data( &test, + 0, author, TestWasms::TxProposalCode.read_bytes(), 12, @@ -3040,12 +3045,14 @@ fn test_epoch_sleep() -> Result<()> { /// This can be submitted with "init-proposal" command. fn prepare_proposal_data( test: &setup::Test, + id: u64, source: Address, data: impl serde::Serialize, start_epoch: u64, ) -> PathBuf { let valid_proposal_json = json!({ "proposal": { + "id": id, "content": { "title": "TheTitle", "authors": "test@test.com",