From c2fa5291b5f648adbdfade9b7eb0e7d882c35a4e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 27 Dec 2023 17:55:15 +0100 Subject: [PATCH] Removes duplicated proposal vote enum --- apps/src/lib/client/tx.rs | 3 +- .../lib/node/ledger/shell/finalize_block.rs | 6 +- benches/native_vps.rs | 6 +- benches/txs.rs | 6 +- benches/vps.rs | 8 +-- core/src/ledger/governance/cli/offline.rs | 2 +- core/src/ledger/governance/cli/onchain.rs | 49 --------------- core/src/ledger/governance/storage/vote.rs | 61 ++++++++----------- core/src/ledger/governance/utils.rs | 10 +-- core/src/ledger/storage_api/governance.rs | 8 +-- core/src/types/transaction/governance.rs | 4 +- light_sdk/src/transaction/governance.rs | 4 +- sdk/src/signing.rs | 12 ++-- sdk/src/tx.rs | 7 +-- 14 files changed, 59 insertions(+), 127 deletions(-) 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 af29326e9f..3620d2f3b0 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -817,7 +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; + 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::{ @@ -1642,8 +1642,8 @@ mod test_finalize_block { }; // Add a proposal to be accepted and one to be rejected. - add_proposal(0, StorageProposalVote::Yay); - 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 9989a0b7c2..a4829c4601 100644 --- a/benches/native_vps.rs +++ b/benches/native_vps.rs @@ -7,7 +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; +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}; @@ -85,7 +85,7 @@ fn governance(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay, + vote: ProposalVote::Yay, voter: defaults::albert_address(), delegations: vec![defaults::validator_address()], }, @@ -101,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![], }, diff --git a/benches/txs.rs b/benches/txs.rs index 03b78f2bd0..d7290852bb 100644 --- a/benches/txs.rs +++ b/benches/txs.rs @@ -3,7 +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; +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, @@ -548,7 +548,7 @@ fn vote_proposal(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay, + vote: ProposalVote::Yay, voter: defaults::albert_address(), delegations: vec![defaults::validator_address()], }, @@ -561,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 e3fb0d0c30..2a0036aadf 100644 --- a/benches/vps.rs +++ b/benches/vps.rs @@ -1,7 +1,7 @@ use std::collections::BTreeSet; use criterion::{criterion_group, criterion_main, Criterion}; -use namada::core::ledger::governance::storage::vote::StorageProposalVote; +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, @@ -97,7 +97,7 @@ fn vp_user(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay, + vote: ProposalVote::Yay, voter: defaults::albert_address(), delegations: vec![defaults::validator_address()], }, @@ -237,7 +237,7 @@ fn vp_implicit(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay, + 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 */ @@ -394,7 +394,7 @@ fn vp_validator(c: &mut Criterion) { TX_VOTE_PROPOSAL_WASM, VoteProposalData { id: 0, - vote: StorageProposalVote::Yay, + 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 3a6e5ebb99..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; diff --git a/core/src/ledger/governance/cli/onchain.rs b/core/src/ledger/governance/cli/onchain.rs index bc691d87c7..976557c913 100644 --- a/core/src/ledger/governance/cli/onchain.rs +++ b/core/src/ledger/governance/cli/onchain.rs @@ -312,52 +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) - } -} diff --git a/core/src/ledger/governance/storage/vote.rs b/core/src/ledger/governance/storage/vote.rs index acde07f141..2016383664 100644 --- a/core/src/ledger/governance/storage/vote.rs +++ b/core/src/ledger/governance/storage/vote.rs @@ -3,8 +3,6 @@ use std::fmt::Display; use borsh::{BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; -use super::super::cli::onchain::ProposalVote; - #[derive( Debug, Clone, @@ -16,7 +14,7 @@ use super::super::cli::onchain::ProposalVote; Deserialize, )] /// The vote for a proposal -pub enum StorageProposalVote { +pub enum ProposalVote { /// Yes Yay, /// No @@ -25,39 +23,42 @@ pub enum StorageProposalVote { 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) + matches!(self, ProposalVote::Abstain) } } -impl From<&ProposalVote> for StorageProposalVote { - fn from(value: &ProposalVote) -> Self { - match value { - ProposalVote::Yay => StorageProposalVote::Yay, - ProposalVote::Nay => StorageProposalVote::Nay, - ProposalVote::Abstain => StorageProposalVote::Abstain, +impl Display for ProposalVote { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ProposalVote::Yay => write!(f, "yay"), + ProposalVote::Nay => write!(f, "nay"), + ProposalVote::Abstain => write!(f, "abstain"), } } } -impl Display for StorageProposalVote { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - StorageProposalVote::Yay => write!(f, "yay"), - StorageProposalVote::Nay => write!(f, "nay"), - StorageProposalVote::Abstain => write!(f, "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()), } } } @@ -66,28 +67,14 @@ impl Display for StorageProposalVote { /// 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 24ee23c128..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,7 @@ 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 { @@ -206,13 +206,13 @@ impl Display for ProposalResult { #[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) } } 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 8e43b488e3..0e7f41179b 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; @@ -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/signing.rs b/sdk/src/signing.rs index 0eb76d620c..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, -}; +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,14 +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 => 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"), } } } diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index a0e762d8c4..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,8 +1842,6 @@ pub async fn build_vote_proposal( return Err(Error::from(TxError::ProposalDoesNotExist(proposal_id))); }; - let storage_vote = StorageProposalVote::from(&proposal_vote); - let is_validator = rpc::is_validator(context.client(), voter).await?; if !proposal.can_be_voted(epoch, is_validator) { @@ -1875,7 +1872,7 @@ pub async fn build_vote_proposal( let data = VoteProposalData { id: proposal_id, - vote: storage_vote, + vote: proposal_vote, voter: voter.clone(), delegations, };