Skip to content

Commit

Permalink
Removes useless VoteType enum. Misc refactors
Browse files Browse the repository at this point in the history
  • Loading branch information
grarco committed Jan 2, 2024
1 parent 909c948 commit 691fc8b
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 149 deletions.
6 changes: 2 additions & 4 deletions apps/src/lib/node/ledger/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::StorageProposalVote;
use namada::core::ledger::replay_protection;
use namada::core::types::storage::KeySeg;
use namada::eth_bridge::storage::bridge_pool::{
Expand Down Expand Up @@ -1644,7 +1642,7 @@ mod test_finalize_block {
};

// Add a proposal to be accepted and one to be rejected.
add_proposal(0, StorageProposalVote::Yay(VoteType::Default));
add_proposal(0, StorageProposalVote::Yay);
add_proposal(1, StorageProposalVote::Nay);

// Commit the genesis state
Expand Down
6 changes: 2 additions & 4 deletions benches/native_vps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::StorageProposalVote;
use namada::core::ledger::ibc::{IbcActions, TransferModule};
use namada::core::ledger::pgf::storage::steward::StewardDetail;
use namada::core::ledger::storage_api::{StorageRead, StorageWrite};
Expand Down Expand Up @@ -87,7 +85,7 @@ fn governance(c: &mut Criterion) {
TX_VOTE_PROPOSAL_WASM,
VoteProposalData {
id: 0,
vote: StorageProposalVote::Yay(VoteType::Default),
vote: StorageProposalVote::Yay,
voter: defaults::albert_address(),
delegations: vec![defaults::validator_address()],
},
Expand Down
6 changes: 2 additions & 4 deletions benches/txs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::StorageProposalVote;
use namada::core::ledger::pgf::storage::steward::StewardDetail;
use namada::core::types::key::{
common, SecretKey as SecretKeyInterface, SigScheme,
Expand Down Expand Up @@ -550,7 +548,7 @@ fn vote_proposal(c: &mut Criterion) {
TX_VOTE_PROPOSAL_WASM,
VoteProposalData {
id: 0,
vote: StorageProposalVote::Yay(VoteType::Default),
vote: StorageProposalVote::Yay,
voter: defaults::albert_address(),
delegations: vec![defaults::validator_address()],
},
Expand Down
10 changes: 4 additions & 6 deletions benches/vps.rs
Original file line number Diff line number Diff line change
@@ -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::StorageProposalVote;
use namada::core::types::address::{self, Address};
use namada::core::types::key::{
common, SecretKey as SecretKeyInterface, SigScheme,
Expand Down Expand Up @@ -99,7 +97,7 @@ fn vp_user(c: &mut Criterion) {
TX_VOTE_PROPOSAL_WASM,
VoteProposalData {
id: 0,
vote: StorageProposalVote::Yay(VoteType::Default),
vote: StorageProposalVote::Yay,
voter: defaults::albert_address(),
delegations: vec![defaults::validator_address()],
},
Expand Down Expand Up @@ -239,7 +237,7 @@ fn vp_implicit(c: &mut Criterion) {
TX_VOTE_PROPOSAL_WASM,
VoteProposalData {
id: 0,
vote: StorageProposalVote::Yay(VoteType::Default),
vote: StorageProposalVote::Yay,
voter: Address::from(&implicit_account.to_public()),
delegations: vec![], /* NOTE: no need to bond tokens because the
* implicit vp doesn't check that */
Expand Down Expand Up @@ -396,7 +394,7 @@ fn vp_validator(c: &mut Criterion) {
TX_VOTE_PROPOSAL_WASM,
VoteProposalData {
id: 0,
vote: StorageProposalVote::Yay(VoteType::Default),
vote: StorageProposalVote::Yay,
voter: defaults::validator_address(),
delegations: vec![],
},
Expand Down
5 changes: 0 additions & 5 deletions core/src/ledger/governance/cli/offline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 0 additions & 5 deletions core/src/ledger/governance/cli/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,4 @@ impl ProposalVote {
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)
}
}
103 changes: 10 additions & 93 deletions core/src/ledger/governance/storage/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,6 @@ use borsh::{BorshDeserialize, BorshSerialize};
use serde::{Deserialize, Serialize};

use super::super::cli::onchain::ProposalVote;
use super::proposal::ProposalType;

/// The type of a governance vote
#[derive(
Debug,
Clone,
PartialEq,
BorshSerialize,
BorshDeserialize,
Eq,
Serialize,
Deserialize,
)]
pub enum VoteType {
/// A default vote
Default,
/// A vote for the PGF stewards
PGFSteward,
/// A vote for a PGF payment proposal
PGFPayment,
}

#[derive(
Debug,
Expand All @@ -39,7 +18,7 @@ pub enum VoteType {
/// The vote for a proposal
pub enum StorageProposalVote {
/// Yes
Yay(VoteType),
Yay,
/// No
Nay,
/// Abstain
Expand All @@ -49,7 +28,7 @@ pub enum StorageProposalVote {
impl StorageProposalVote {
/// Check if a vote is yay
pub fn is_yay(&self) -> bool {
matches!(self, StorageProposalVote::Yay(_))
matches!(self, StorageProposalVote::Yay)
}

/// Check if a vote is nay
Expand All @@ -61,90 +40,28 @@ impl StorageProposalVote {
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<Self> {
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,
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 StorageProposalVote {
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::Yay => write!(f, "yay"),
StorageProposalVote::Nay => write!(f, "nay"),
StorageProposalVote::Abstain => write!(f, "abstain"),
}
}
}

impl PartialEq<VoteType> 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)
}
}
}
}

#[cfg(any(test, feature = "testing"))]
/// Testing helpers and strategies for governance votes
pub mod testing {
Expand Down
4 changes: 2 additions & 2 deletions core/src/ledger/governance/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,10 @@ impl TallyVote {
) -> Result<bool, &'static str> {
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"),
}
Expand Down
11 changes: 2 additions & 9 deletions sdk/src/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ 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,
StorageProposalVote,
};
use crate::core::types::eth_bridge_pool::PendingTransfer;
use crate::error::{EncodingError, Error, TxError};
Expand Down Expand Up @@ -900,14 +900,7 @@ struct LedgerProposalVote<'a>(&'a StorageProposalVote);
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::Yay => write!(f, "yay"),
StorageProposalVote::Nay => write!(f, "nay"),
StorageProposalVote::Abstain => write!(f, "abstain"),
}
Expand Down
8 changes: 1 addition & 7 deletions sdk/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1843,13 +1843,7 @@ 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 storage_vote = StorageProposalVote::from(&proposal_vote);

let is_validator = rpc::is_validator(context.client(), voter).await?;

Expand Down
10 changes: 0 additions & 10 deletions shared/src/ledger/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()?;

Expand All @@ -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)) => {
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 691fc8b

Please sign in to comment.