Skip to content

Commit

Permalink
fixed individual vote count revote bug
Browse files Browse the repository at this point in the history
  • Loading branch information
NoahSaso committed Feb 17, 2025
1 parent 60e1b71 commit 5cbd541
Show file tree
Hide file tree
Showing 11 changed files with 355 additions and 41 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions contracts/delegation/dao-vote-delegation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,4 @@ dao-voting-cw721-staked = { workspace = true, features = ["library"] }
dao-testing = { workspace = true }
dao-proposal-sudo = { workspace = true }
dao-proposal-single = { workspace = true }
dao-proposal-multiple = { workspace = true }
182 changes: 182 additions & 0 deletions contracts/delegation/dao-vote-delegation/src/testing/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use cosmwasm_std::{
to_json_binary, Addr, Decimal, Empty, Uint128,
};
use cw_multi_test::{Contract, ContractWrapper};
use cw_utils::Duration;
use dao_interface::helpers::OptionalUpdate;
use dao_testing::{ADDR0, ADDR1, ADDR2, ADDR3, ADDR4};

Expand Down Expand Up @@ -1499,3 +1500,184 @@ fn test_gas_limits() {
);
}
}

#[test]
fn test_revote() {
let mut suite = Cw4DaoVoteDelegationTestingSuite::new().build();
let dao = suite.dao.clone();

// Enable revoting on both proposal modules.
suite
.execute_smart(
&dao.core_addr,
&dao.proposal_modules[0].1,
&dao_proposal_single::msg::ExecuteMsg::UpdateConfig {
threshold: dao_voting::threshold::Threshold::AbsolutePercentage {
percentage: dao_voting::threshold::PercentageThreshold::Majority {},
},
max_voting_period: Duration::Height(10),
min_voting_period: None,
only_members_execute: true,
allow_revoting: true,
dao: dao.core_addr.to_string(),
close_proposal_on_execution_failure: true,
veto: None,
},
&[],
)
.unwrap();
suite
.execute_smart(
&dao.core_addr,
&dao.proposal_modules[1].1,
&dao_proposal_multiple::msg::ExecuteMsg::UpdateConfig {
voting_strategy: dao_voting::multiple_choice::VotingStrategy::SingleChoice {
quorum: dao_voting::threshold::PercentageThreshold::Majority {},
},
max_voting_period: Duration::Height(10),
min_voting_period: None,
only_members_execute: true,
allow_revoting: true,
dao: dao.core_addr.to_string(),
close_proposal_on_execution_failure: true,
veto: None,
},
&[],
)
.unwrap();

// register member as delegate
suite.register(ADDR0);

// delegate 100% of voting power to ADDR0
suite.delegate(ADDR1, ADDR0, Decimal::percent(100));

// delegations take effect on the next block
suite.advance_block();

// SINGLE CHOICE PROPOSAL

// propose a single choice proposal
let (proposal_module, proposal_id, _) =
suite.propose_single_choice(&dao, ADDR0, "test proposal", vec![]);

// delegate votes on the proposal
suite.vote_single_choice(&dao, ADDR0, proposal_id, dao_voting::voting::Vote::Yes);

// assert vote count on single choice proposal
suite.assert_single_choice_votes_count(
&proposal_module,
proposal_id,
dao_voting::voting::Vote::Yes,
// both members' weight is used by delegate
suite.members[0].weight + suite.members[1].weight,
);
// assert individual vote count on single choice proposal
suite.assert_single_choice_individual_votes_count(
&proposal_module,
proposal_id,
dao_voting::voting::Vote::Yes,
// only delegate weight is counted in individual votes
suite.members[0].weight,
);

// revote on the proposal
suite.vote_single_choice(&dao, ADDR0, proposal_id, dao_voting::voting::Vote::No);

// assert vote count on single choice proposal
suite.assert_single_choice_votes_count(
&proposal_module,
proposal_id,
dao_voting::voting::Vote::Yes,
0u128,
);
suite.assert_single_choice_votes_count(
&proposal_module,
proposal_id,
dao_voting::voting::Vote::No,
// both members' weight is used by delegate
suite.members[0].weight + suite.members[1].weight,
);
// assert individual vote count on single choice proposal
suite.assert_single_choice_individual_votes_count(
&proposal_module,
proposal_id,
dao_voting::voting::Vote::Yes,
0u128,
);
suite.assert_single_choice_individual_votes_count(
&proposal_module,
proposal_id,
dao_voting::voting::Vote::No,
// only delegate weight is counted in individual votes
suite.members[0].weight,
);

// MULTIPLE CHOICE PROPOSAL

// propose a multiple choice proposal
let (proposal_module, proposal_id, _) = suite.propose_multiple_choice(
&dao,
ADDR0,
"test proposal",
vec![
dao_voting::multiple_choice::MultipleChoiceOption {
title: "Option 1".to_string(),
description: "Option 1 description".to_string(),
msgs: vec![],
},
dao_voting::multiple_choice::MultipleChoiceOption {
title: "Option 2".to_string(),
description: "Option 2 description".to_string(),
msgs: vec![],
},
dao_voting::multiple_choice::MultipleChoiceOption {
title: "Option 3".to_string(),
description: "Option 3 description".to_string(),
msgs: vec![],
},
],
);

// delegate votes on the proposal
suite.vote_multiple_choice(&dao, ADDR0, proposal_id, 1);

// assert vote count on multiple choice proposal
suite.assert_multiple_choice_votes_count(
&proposal_module,
proposal_id,
1,
// both members' weight is used by delegate
suite.members[0].weight + suite.members[1].weight,
);
// assert individual vote count on multiple choice proposal
suite.assert_multiple_choice_individual_votes_count(
&proposal_module,
proposal_id,
1,
// only delegate weight is counted in individual votes
suite.members[0].weight,
);

// revote on the proposal
suite.vote_multiple_choice(&dao, ADDR0, proposal_id, 2);

// assert vote count on multiple choice proposal
suite.assert_multiple_choice_votes_count(&proposal_module, proposal_id, 1, 0u128);
suite.assert_multiple_choice_votes_count(
&proposal_module,
proposal_id,
2,
// both members' weight is used by delegate
suite.members[0].weight + suite.members[1].weight,
);
// assert individual vote count on multiple choice proposal
suite.assert_multiple_choice_individual_votes_count(&proposal_module, proposal_id, 1, 0u128);
suite.assert_multiple_choice_individual_votes_count(
&proposal_module,
proposal_id,
2,
// only delegate weight is counted in individual votes
suite.members[0].weight,
);
}
4 changes: 3 additions & 1 deletion contracts/proposal/dao-proposal-multiple/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,9 +435,10 @@ pub fn execute_vote(
prop.votes
.remove_vote(current_ballot.vote, current_ballot.power)?;
prop.individual_votes
.remove_vote(current_ballot.vote, current_ballot.power)?;
.remove_vote(current_ballot.vote, current_ballot.individual_power)?;
Ok(Ballot {
power: vote_power.total,
individual_power: vote_power.individual,
vote,
rationale: rationale.clone(),
})
Expand All @@ -449,6 +450,7 @@ pub fn execute_vote(
None => Ok(Ballot {
vote,
power: vote_power.total,
individual_power: vote_power.individual,
rationale: rationale.clone(),
}),
})?;
Expand Down
6 changes: 3 additions & 3 deletions contracts/proposal/dao-proposal-multiple/src/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ impl MultipleChoiceProposal {
votes_to_consider: &MultipleChoiceVotes,
winning_choice: &CheckedMultipleChoiceOption,
) -> StdResult<bool> {
let winning_choice_power = votes_to_consider.vote_weights[winning_choice.index as usize];
let winning_choice_power = votes_to_consider.get_id(winning_choice.index);
if let Some(second_choice_power) = votes_to_consider
.vote_weights
.iter()
Expand Down Expand Up @@ -841,7 +841,7 @@ mod tests {
);
// Everyone voted and proposal is in a tie...
assert_eq!(prop.total_power, prop.votes.total());
assert_eq!(prop.votes.vote_weights[0], prop.votes.vote_weights[1]);
assert_eq!(prop.votes.get_id(0), prop.votes.get_id(1));
// ... but proposal is still active => no rejection
assert!(!prop.is_rejected(&env.block).unwrap());

Expand All @@ -854,7 +854,7 @@ mod tests {
true,
);
// Proposal has expired and ended in a tie => rejection
assert_eq!(prop.votes.vote_weights[0], prop.votes.vote_weights[1]);
assert_eq!(prop.votes.get_id(0), prop.votes.get_id(1));
assert!(prop.is_rejected(&env.block).unwrap());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,8 @@ pub fn test_allow_voting_after_proposal_execution_pre_expiration_cw20() {
// assert proposal is passed with expected votes
let prop = query_proposal(&app, &proposal_module, proposal_id);
assert_eq!(prop.proposal.status, Status::Passed);
assert_eq!(prop.proposal.votes.vote_weights[0], Uint128::new(100000000));
assert_eq!(prop.proposal.votes.vote_weights[1], Uint128::new(0));
assert_eq!(prop.proposal.votes.get_id(0), Uint128::new(100000000));
assert_eq!(prop.proposal.votes.get_id(1), Uint128::new(0));

// someone wakes up and casts their vote to express their
// opinion (not affecting the result of proposal)
Expand All @@ -384,8 +384,8 @@ pub fn test_allow_voting_after_proposal_execution_pre_expiration_cw20() {
// assert proposal is passed with expected votes
let prop = query_proposal(&app, &proposal_module, proposal_id);
assert_eq!(prop.proposal.status, Status::Passed);
assert_eq!(prop.proposal.votes.vote_weights[0], Uint128::new(100000000));
assert_eq!(prop.proposal.votes.vote_weights[1], Uint128::new(50000000));
assert_eq!(prop.proposal.votes.get_id(0), Uint128::new(100000000));
assert_eq!(prop.proposal.votes.get_id(1), Uint128::new(50000000));

// execute the proposal expecting
app.execute_contract(
Expand Down
39 changes: 9 additions & 30 deletions contracts/proposal/dao-proposal-multiple/src/testing/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3533,14 +3533,8 @@ fn test_revoting() {
// Assert that both vote options have equal vote weights at some block
let proposal: ProposalResponse = query_proposal(&app, &govmod, 1);
assert_eq!(proposal.proposal.status, Status::Open);
assert_eq!(
proposal.proposal.votes.vote_weights[0],
Uint128::new(100_000_000),
);
assert_eq!(
proposal.proposal.votes.vote_weights[1],
Uint128::new(100_000_000),
);
assert_eq!(proposal.proposal.votes.get_id(0), Uint128::new(100_000_000),);
assert_eq!(proposal.proposal.votes.get_id(1), Uint128::new(100_000_000),);

// More time passes..
app.update_block(|b| b.height += 3);
Expand All @@ -3564,11 +3558,8 @@ fn test_revoting() {
// Assert that revote succeeded
let proposal: ProposalResponse = query_proposal(&app, &govmod, 1);
assert_eq!(proposal.proposal.status, Status::Passed);
assert_eq!(
proposal.proposal.votes.vote_weights[0],
Uint128::new(200_000_000),
);
assert_eq!(proposal.proposal.votes.vote_weights[1], Uint128::new(0),);
assert_eq!(proposal.proposal.votes.get_id(0), Uint128::new(200_000_000),);
assert_eq!(proposal.proposal.votes.get_id(1), Uint128::new(0),);
}

/// Tests that revoting is stored at a per-proposal level.
Expand Down Expand Up @@ -3920,14 +3911,8 @@ fn test_invalid_revote_does_not_invalidate_initial_vote() {
// Assert that both vote options have equal vote weights at some block
let proposal: ProposalResponse = query_proposal(&app, &proposal_module, 1);
assert_eq!(proposal.proposal.status, Status::Open);
assert_eq!(
proposal.proposal.votes.vote_weights[0],
Uint128::new(100_000_000),
);
assert_eq!(
proposal.proposal.votes.vote_weights[1],
Uint128::new(100_000_000),
);
assert_eq!(proposal.proposal.votes.get_id(0), Uint128::new(100_000_000),);
assert_eq!(proposal.proposal.votes.get_id(1), Uint128::new(100_000_000),);

// Time passes..
app.update_block(|b| b.height += 3);
Expand All @@ -3949,14 +3934,8 @@ fn test_invalid_revote_does_not_invalidate_initial_vote() {
.downcast()
.unwrap();
// Assert that prior votes remained the same
assert_eq!(
proposal.proposal.votes.vote_weights[0],
Uint128::new(100_000_000),
);
assert_eq!(
proposal.proposal.votes.vote_weights[1],
Uint128::new(100_000_000),
);
assert_eq!(proposal.proposal.votes.get_id(0), Uint128::new(100_000_000),);
assert_eq!(proposal.proposal.votes.get_id(1), Uint128::new(100_000_000),);
assert!(matches!(err, ContractError::InvalidVote {}));
}

Expand Down Expand Up @@ -4550,7 +4529,7 @@ pub fn test_not_allow_voting_on_expired_proposal() {
// assert the vote got rejected and did not count towards the votes
let proposal = query_proposal(&app, &proposal_module, 1);
assert_eq!(proposal.proposal.status, Status::Rejected);
assert_eq!(proposal.proposal.votes.vote_weights[0], Uint128::zero());
assert_eq!(proposal.proposal.votes.get_id(0), Uint128::zero());
assert!(matches!(err, ContractError::Expired { id: _proposal_id }));
}

Expand Down
4 changes: 3 additions & 1 deletion contracts/proposal/dao-proposal-single/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,10 @@ pub fn execute_vote(
prop.votes
.remove_vote(current_ballot.vote, current_ballot.power);
prop.individual_votes
.remove_vote(current_ballot.vote, current_ballot.power);
.remove_vote(current_ballot.vote, current_ballot.individual_power);
Ok(Ballot {
power: vote_power.total,
individual_power: vote_power.individual,
vote,
// Roll over the previous rationale. If
// you're changing your vote, you've also
Expand All @@ -555,6 +556,7 @@ pub fn execute_vote(
}
None => Ok(Ballot {
power: vote_power.total,
individual_power: vote_power.individual,
vote,
rationale: rationale.clone(),
}),
Expand Down
Loading

0 comments on commit 5cbd541

Please sign in to comment.