Merge branch 'fraccaman+grarco/governance-tests' (#2330)

* fraccaman+grarco/governance-tests:
  Changelog #2330
  Fixes broken governance test
  Removes duplicated proposal vote enum
  Removes useless `VoteType` enum. Misc refactors
  Returns early if proposal voter doesn't have a stake
  Improves governance messages
  Adds `query_proposal_votes`
  Improves formatting of `ProposalResult`

.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))

.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))

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<args::CliTypes>);
+
+    impl SubCmd for QueryProposalVotes {
+        const CMD: &'static str = "query-proposal-votes";
+
+        fn parse(matches: &ArgMatches) -> Option<Self>
+        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::<args::QueryProposalVotes<args::CliTypes>>()
+        }
+    }
+
     #[derive(Clone, Debug)]
     pub struct QueryProposal(pub args::QueryProposal<args::CliTypes>);
 
@@ -3063,6 +3090,7 @@ pub mod args {
     pub const VALIDATOR_ETH_HOT_KEY: ArgOpt<WalletPublicKey> =
         arg_opt("eth-hot-key");
     pub const VALUE: Arg<String> = arg("value");
+    pub const VOTER_OPT: ArgOpt<WalletAddress> = arg_opt("voter");
     pub const VIEWING_KEY: Arg<WalletViewingKey> = arg("key");
     pub const VP: ArgOpt<String> = arg_opt("vp");
     pub const WALLET_ALIAS_FORCE: ArgFlag = flag("wallet-alias-force");
@@ -4801,6 +4829,36 @@ pub mod args {
         }
     }
 
+    impl CliToSdk<QueryProposalVotes<SdkTypes>> for QueryProposalVotes<CliTypes> {
+        fn to_sdk(self, ctx: &mut Context) -> QueryProposalVotes<SdkTypes> {
+            QueryProposalVotes::<SdkTypes> {
+                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<CliTypes> {
+        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::<Query<CliTypes>>()
+                .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<C: NamadaTypes = SdkTypes> {
         /// Common query args

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,
                     )) => {

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;

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};

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();

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![],
                     },

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![],
         },

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![],
         },

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();

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<String> for ProposalVote {
-    type Error = String;
-
-    fn try_from(value: String) -> Result<Self, Self::Error> {
-        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)
-    }
-}