Repository: DA0-DA0/dao-contracts
Pull request #181 (2022-02-17 19:11:51)

Reject proposals when they cannot pass
Addresses Issue #169 

Adds an is_rejected method to both Proposal types which does the same as is_passed however checks no votes for a majority.
Added unit tests  for the logic on the Proposal side.
Added integration tests for both a DAO and Multisig.

Feedback appreciated.


Patch:
diff --git a/contracts/cw3-dao/src/state.rs b/contracts/cw3-dao/src/state.rs
index 4df171f1..f01d2fde 100644
--- a/contracts/cw3-dao/src/state.rs
+++ b/contracts/cw3-dao/src/state.rs
@@ -88,7 +88,7 @@ impl Proposal {
         if status == Status::Open && self.is_passed(block) {
             status = Status::Passed;
         }
-        if status == Status::Open && self.expires.is_expired(block) {
+        if status == Status::Open && (self.expires.is_expired(block) || self.is_rejected(block)) {
             status = Status::Rejected;
         }
 
@@ -101,14 +101,21 @@ impl Proposal {
         self.status = self.current_status(block);
     }
 
-    // returns true iff this proposal is sure to pass (even before expiration if no future
-    // sequence of possible votes can cause it to fail)
-    pub fn is_passed(&self, block: &BlockInfo) -> bool {
+    /// Helper function to check if a certain vote count has reached threshold.
+    /// Only called from is_rejected and is_passed for no and yes votes
+    /// Handles the different threshold types accordingly.
+    /// This function returns true if and only if vote_count is greater than the threshold which
+    /// is calculated.
+    /// In the case where we use yes votes, this function will return true if and only if the
+    /// proposal will pass.
+    /// In the case where we use no votes, this function will return true if and only if the
+    /// proposal will be rejected regardless of other votes.
+    fn does_vote_count_reach_threshold(&self, vote_count: Uint128, block: &BlockInfo) -> bool {
         match self.threshold {
             Threshold::AbsolutePercentage {
                 percentage: percentage_needed,
             } => {
-                self.votes.yes
+                vote_count
                     >= votes_needed(self.total_weight - self.votes.abstain, percentage_needed)
             }
             Threshold::ThresholdQuorum { threshold, quorum } => {
@@ -117,18 +124,30 @@ impl Proposal {
                     return false;
                 }
                 if self.expires.is_expired(block) {
-                    // If expired, we compare Yes votes against the total number of votes (minus abstain).
+                    // If expired, we compare vote_count against the total number of votes (minus abstain).
                     let opinions = self.votes.total() - self.votes.abstain;
-                    self.votes.yes >= votes_needed(opinions, threshold)
+                    vote_count >= votes_needed(opinions, threshold)
                 } else {
-                    // If not expired, we must assume all non-votes will be cast as No.
-                    // We compare threshold against the total weight (minus abstain).
+                    // If not expired, we must assume all non-votes will be cast against
+                    // vote_count
                     let possible_opinions = self.total_weight - self.votes.abstain;
-                    self.votes.yes >= votes_needed(possible_opinions, threshold)
+                    vote_count >= votes_needed(possible_opinions, threshold)
                 }
             }
         }
     }
+
+    /// returns true iff this proposal is sure to pass (even before expiration if no future
+    /// sequence of possible votes can cause it to fail)
+    pub fn is_passed(&self, block: &BlockInfo) -> bool {
+        self.does_vote_count_reach_threshold(self.votes.yes, block)
+    }
+
+    /// As above for the rejected check, used to check if a proposal is
+    /// already rejected.
+    pub fn is_rejected(&self, block: &BlockInfo) -> bool {
+        self.does_vote_count_reach_threshold(self.votes.no, block)
+    }
 }
 
 // this is a helper function so Decimal works with u64 rather than Uint128
@@ -232,12 +251,12 @@ mod test {
         );
     }
 
-    fn check_is_passed(
+    fn setup_prop(
         threshold: Threshold,
         votes: Votes,
         total_weight: Uint128,
         is_expired: bool,
-    ) -> bool {
+    ) -> (Proposal, BlockInfo) {
         let block = mock_env().block;
         let expires = match is_expired {
             true => Expiration::AtHeight(block.height - 5),
@@ -256,9 +275,29 @@ mod test {
             votes,
             deposit: Uint128::zero(),
         };
+        (prop, block)
+    }
+
+    fn check_is_passed(
+        threshold: Threshold,
+        votes: Votes,
+        total_weight: Uint128,
+        is_expired: bool,
+    ) -> bool {
+        let (prop, block) = setup_prop(threshold, votes, total_weight, is_expired);
         prop.is_passed(&block)
     }
 
+    fn check_is_rejected(
+        threshold: Threshold,
+        votes: Votes,
+        total_weight: Uint128,
+        is_expired: bool,
+    ) -> bool {
+        let (prop, block) = setup_prop(threshold, votes, total_weight, is_expired);
+        prop.is_rejected(&block)
+    }
+
     #[test]
     fn proposal_passed_absolute_percentage() {
         let percent = Threshold::AbsolutePercentage {
@@ -299,6 +338,63 @@ mod test {
         assert!(check_is_passed(percent, votes, Uint128::new(14), true));
     }
 
+    #[test]
+    fn proposal_rejected_absolute_percentage() {
+        let percent = Threshold::AbsolutePercentage {
+            percentage: Decimal::percent(50),
+        };
+
+        // 4 YES, 7 NO, 2 ABSTAIN
+        let mut votes = Votes::new(Uint128::new(4));
+        votes.add_vote(Vote::No, Uint128::new(7));
+        votes.add_vote(Vote::Abstain, Uint128::new(2));
+
+        // 15 total voting power
+        // 7 / (15 - 2) > 50%
+        // Expiry does not matter
+        assert!(check_is_rejected(
+            percent.clone(),
+            votes.clone(),
+            Uint128::new(15),
+            false
+        ));
+        assert!(check_is_rejected(
+            percent.clone(),
+            votes.clone(),
+            Uint128::new(15),
+            true
+        ));
+
+        // 17 total voting power
+        // 7 / (17 - 2) < 50%
+        assert!(!check_is_rejected(
+            percent.clone(),
+            votes.clone(),
+            Uint128::new(17),
+            false
+        ));
+        assert!(!check_is_rejected(
+            percent.clone(),
+            votes.clone(),
+            Uint128::new(17),
+            true
+        ));
+
+        // Rejected if total was lower
+        assert!(check_is_rejected(
+            percent.clone(),
+            votes.clone(),
+            Uint128::new(14),
+            false
+        ));
+        assert!(check_is_rejected(
+            percent,
+            votes.clone(),
+            Uint128::new(14),
+            true
+        ));
+    }
+
     #[test]
     fn proposal_passed_quorum() {
         let quorum = Threshold::ThresholdQuorum {
@@ -391,6 +487,105 @@ mod test {
         assert!(check_is_passed(quorum, passing, Uint128::new(16), false));
     }
 
+    #[test]
+    fn proposal_rejected_quorum() {
+        let quorum = Threshold::ThresholdQuorum {
+            threshold: Decimal::percent(50),
+            quorum: Decimal::percent(40),
+        };
+        // all non-yes votes are counted for quorum
+        let rejecting = Votes {
+            yes: Uint128::new(3),
+            no: Uint128::new(7),
+            abstain: Uint128::new(2),
+            veto: Uint128::new(1),
+        };
+        // abstain votes are not counted for threshold => yes / (yes + no + veto)
+        let rejected_ignoring_abstain = Votes {
+            yes: Uint128::new(4),
+            no: Uint128::new(6),
+            abstain: Uint128::new(5),
+            veto: Uint128::new(2),
+        };
+        // fails any way you look at it
+        let failing = Votes {
+            yes: Uint128::new(5),
+            no: Uint128::new(6),
+            abstain: Uint128::new(2),
+            veto: Uint128::new(2),
+        };
+
+        // first, expired (voting period over)
+        // over quorum (40% of 30 = 12), over threshold (7/11 > 50%)
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            Uint128::new(30),
+            true
+        ));
+        // Under quorum means it cannot be rejected
+        assert!(!check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            Uint128::new(33),
+            true
+        ));
+
+        // over quorum, threshold passes if we ignore abstain
+        // 17 total votes w/ abstain => 40% quorum of 40 total
+        // 6 no / (6 no + 4 yes + 2 votes) => 50% threshold
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejected_ignoring_abstain.clone(),
+            Uint128::new(40),
+            true
+        ));
+
+        // over quorum, but under threshold fails also
+        assert!(!check_is_rejected(
+            quorum.clone(),
+            failing,
+            Uint128::new(20),
+            true
+        ));
+
+        // Voting is still open so assume rest of votes are yes
+        // threshold not reached
+        assert!(!check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            Uint128::new(30),
+            false
+        ));
+        assert!(!check_is_rejected(
+            quorum.clone(),
+            rejected_ignoring_abstain.clone(),
+            Uint128::new(40),
+            false
+        ));
+        // if we have threshold * total_weight as no votes this must reject
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            Uint128::new(14),
+            false
+        ));
+        // all votes have been cast, some abstain
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejected_ignoring_abstain,
+            Uint128::new(17),
+            false
+        ));
+        // 3 votes uncast, if they all vote yes, we have 7 no, 7 yes+veto, 2 abstain (out of 16)
+        assert!(check_is_rejected(
+            quorum,
+            rejecting,
+            Uint128::new(16),
+            false
+        ));
+    }
+
     #[test]
     fn quorum_edge_cases() {
         // when we pass absolute threshold (everyone else voting no, we pass), but still don't hit quorum
diff --git a/contracts/cw3-multisig/src/state.rs b/contracts/cw3-multisig/src/state.rs
index 2075cd67..cf0576b2 100644
--- a/contracts/cw3-multisig/src/state.rs
+++ b/contracts/cw3-multisig/src/state.rs
@@ -91,7 +91,7 @@ impl Proposal {
         if status == Status::Open && self.is_passed(block) {
             status = Status::Passed;
         }
-        if status == Status::Open && self.expires.is_expired(block) {
+        if status == Status::Open && (self.expires.is_expired(block) || self.is_rejected(block)) {
             status = Status::Rejected;
         }
 
@@ -104,17 +104,24 @@ impl Proposal {
         self.status = self.current_status(block);
     }
 
-    // returns true iff this proposal is sure to pass (even before expiration if no future
-    // sequence of possible votes can cause it to fail)
-    pub fn is_passed(&self, block: &BlockInfo) -> bool {
+    /// Helper function to check if a certain vote count has reached threshold.
+    /// Only called from is_rejected and is_passed for no and yes votes
+    /// Handles the different threshold types accordingly.
+    /// This function returns true if and only if vote_count is greater than the threshold which
+    /// is calculated.
+    /// In the case where we use yes votes, this function will return true if and only if the
+    /// proposal will pass.
+    /// In the case where we use no votes, this function will return true if and only if the
+    /// proposal will be rejected regardless of other votes.
+    fn does_vote_count_reach_threshold(&self, vote_count: u64, block: &BlockInfo) -> bool {
         match self.threshold {
             Threshold::AbsoluteCount {
                 weight: weight_needed,
-            } => self.votes.yes >= weight_needed,
+            } => vote_count >= weight_needed,
             Threshold::AbsolutePercentage {
                 percentage: percentage_needed,
             } => {
-                self.votes.yes
+                vote_count
                     >= votes_needed(self.total_weight - self.votes.abstain, percentage_needed)
             }
             Threshold::ThresholdQuorum { threshold, quorum } => {
@@ -123,19 +130,31 @@ impl Proposal {
                     return false;
                 }
                 if self.expires.is_expired(block) {
-                    // If expired, we compare Yes votes against the total number of votes (minus abstain).
+                    // If expired, we compare vote_count against the total number of votes (minus abstain).
                     let opinions = self.votes.total() - self.votes.abstain;
-                    self.votes.yes >= votes_needed(opinions, threshold)
+                    vote_count >= votes_needed(opinions, threshold)
                 } else {
-                    // If not expired, we must assume all non-votes will be cast as No.
-                    // We compare threshold against the total weight (minus abstain).
+                    // If not expired, we must assume all non-votes will be cast against
+                    // vote_count
                     let possible_opinions = self.total_weight - self.votes.abstain;
-                    self.votes.yes >= votes_needed(possible_opinions, threshold)
+                    vote_count >= votes_needed(possible_opinions, threshold)
                 }
             }
         }
     }
 
+    /// returns true iff this proposal is sure to pass (even before expiration if no future
+    /// sequence of possible votes can cause it to fail)
+    pub fn is_passed(&self, block: &BlockInfo) -> bool {
+        self.does_vote_count_reach_threshold(self.votes.yes, block)
+    }
+
+    /// As above for the rejected check, used to check if a proposal is
+    /// already rejected.
+    pub fn is_rejected(&self, block: &BlockInfo) -> bool {
+        self.does_vote_count_reach_threshold(self.votes.no, block)
+    }
+
     /// Validates that the thresholds proposed in `UpdateConfig`
     /// messages do not excede sum of weights of multisig members.
     pub fn validate_update_config_msgs(
@@ -248,12 +267,12 @@ mod test {
         assert_eq!(12, votes_needed(48, Decimal::percent(25)));
     }
 
-    fn check_is_passed(
+    fn setup_prop(
         threshold: Threshold,
         votes: Votes,
         total_weight: u64,
         is_expired: bool,
-    ) -> bool {
+    ) -> (Proposal, BlockInfo) {
         let block = mock_env().block;
         let expires = match is_expired {
             true => Expiration::AtHeight(block.height - 5),
@@ -271,9 +290,30 @@ mod test {
             total_weight,
             votes,
         };
+
+        (prop, block)
+    }
+
+    fn check_is_passed(
+        threshold: Threshold,
+        votes: Votes,
+        total_weight: u64,
+        is_expired: bool,
+    ) -> bool {
+        let (prop, block) = setup_prop(threshold, votes, total_weight, is_expired);
         prop.is_passed(&block)
     }
 
+    fn check_is_rejected(
+        threshold: Threshold,
+        votes: Votes,
+        total_weight: u64,
+        is_expired: bool,
+    ) -> bool {
+        let (prop, block) = setup_prop(threshold, votes, total_weight, is_expired);
+        prop.is_rejected(&block)
+    }
+
     #[test]
     fn proposal_passed_absolute_count() {
         let fixed = Threshold::AbsoluteCount { weight: 10 };
@@ -288,6 +328,21 @@ mod test {
         assert!(check_is_passed(fixed, votes, 30, true));
     }
 
+    #[test]
+    fn proposal_rejected_absolute_count() {
+        let fixed = Threshold::AbsoluteCount { weight: 10 };
+        let mut votes = Votes::yes(0);
+        votes.add_vote(Vote::Veto, 4);
+        votes.add_vote(Vote::No, 7);
+        // same expired or not, total_weight or whatever
+        assert!(!check_is_rejected(fixed.clone(), votes.clone(), 30, false));
+        assert!(!check_is_rejected(fixed.clone(), votes.clone(), 30, true));
+        // a few more no votes and we have rejected the prop
+        votes.add_vote(Vote::No, 3);
+        assert!(check_is_rejected(fixed.clone(), votes.clone(), 30, false));
+        assert!(check_is_rejected(fixed, votes, 30, true));
+    }
+
     #[test]
     fn proposal_passed_absolute_percentage() {
         let percent = Threshold::AbsolutePercentage {
@@ -308,6 +363,38 @@ mod test {
         assert!(check_is_passed(percent, votes, 14, true));
     }
 
+    #[test]
+    fn proposal_rejected_absolute_percentage() {
+        let percent = Threshold::AbsolutePercentage {
+            percentage: Decimal::percent(50),
+        };
+
+        // 4 YES, 7 NO, 2 ABSTAIN
+        let mut votes = Votes::yes(4);
+        votes.add_vote(Vote::No, 7);
+        votes.add_vote(Vote::Abstain, 2);
+
+        // 15 total voting power
+        // 7 / (15 - 2) > 50%
+        // Expiry does not matter
+        assert!(check_is_rejected(percent.clone(), votes.clone(), 15, false));
+        assert!(check_is_rejected(percent.clone(), votes.clone(), 15, true));
+
+        // 17 total voting power
+        // 7 / (17 - 2) < 50%
+        assert!(!check_is_rejected(
+            percent.clone(),
+            votes.clone(),
+            17,
+            false
+        ));
+        assert!(!check_is_rejected(percent.clone(), votes.clone(), 17, true));
+
+        // Rejected if total was lower
+        assert!(check_is_rejected(percent.clone(), votes.clone(), 14, false));
+        assert!(check_is_rejected(percent, votes.clone(), 14, true));
+    }
+
     #[test]
     fn proposal_passed_quorum() {
         let quorum = Threshold::ThresholdQuorum {
@@ -375,6 +462,95 @@ mod test {
         assert!(check_is_passed(quorum, passing, 16, false));
     }
 
+    #[test]
+    fn proposal_rejected_quorum() {
+        let quorum = Threshold::ThresholdQuorum {
+            threshold: Decimal::percent(50),
+            quorum: Decimal::percent(40),
+        };
+        // all non-yes votes are counted for quorum
+        let rejecting = Votes {
+            yes: 3,
+            no: 7,
+            abstain: 2,
+            veto: 1,
+        };
+        // abstain votes are not counted for threshold => yes / (yes + no + veto)
+        let rejected_ignoring_abstain = Votes {
+            yes: 4,
+            no: 6,
+            abstain: 5,
+            veto: 2,
+        };
+        // fails any way you look at it
+        let failing = Votes {
+            yes: 5,
+            no: 6,
+            abstain: 2,
+            veto: 2,
+        };
+
+        // first, expired (voting period over)
+        // over quorum (40% of 30 = 12), over threshold (7/11 > 50%)
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            30,
+            true
+        ));
+        // Under quorum means it cannot be rejected
+        assert!(!check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            33,
+            true
+        ));
+
+        // over quorum, threshold passes if we ignore abstain
+        // 17 total votes w/ abstain => 40% quorum of 40 total
+        // 6 no / (6 no + 4 yes + 2 votes) => 50% threshold
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejected_ignoring_abstain.clone(),
+            40,
+            true
+        ));
+
+        // over quorum, but under threshold fails also
+        assert!(!check_is_rejected(quorum.clone(), failing, 20, true));
+
+        // Voting is still open so assume rest of votes are yes
+        // threshold not reached
+        assert!(!check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            30,
+            false
+        ));
+        assert!(!check_is_rejected(
+            quorum.clone(),
+            rejected_ignoring_abstain.clone(),
+            40,
+            false
+        ));
+        // if we have threshold * total_weight as no votes this must reject
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejecting.clone(),
+            14,
+            false
+        ));
+        // all votes have been cast, some abstain
+        assert!(check_is_rejected(
+            quorum.clone(),
+            rejected_ignoring_abstain,
+            17,
+            false
+        ));
+        // 3 votes uncast, if they all vote yes, we have 7 no, 7 yes+veto, 2 abstain (out of 16)
+        assert!(check_is_rejected(quorum, rejecting, 16, false));
+    }
+
     #[test]
     fn quorum_edge_cases() {
         // when we pass absolute threshold (everyone else voting no, we pass), but still don't hit quorum


Test patch:
diff --git a/contracts/cw3-dao/src/tests.rs b/contracts/cw3-dao/src/tests.rs
index 5b43747f..81b30bcf 100644
--- a/contracts/cw3-dao/src/tests.rs
+++ b/contracts/cw3-dao/src/tests.rs
@@ -1108,6 +1108,57 @@ fn test_vote_works() {
         .query_wasm_smart(&dao_addr, &QueryMsg::Vote { proposal_id, voter })
         .unwrap();
     assert!(vote.vote.is_none());
+
+    // Testing for rejecting when threshold reached
+    // create proposal with 0 vote power
+    let proposal = pay_somebody_proposal();
+    let res = app
+        .execute_contract(Addr::unchecked(OWNER), dao_addr.clone(), &proposal, &[])
+        .unwrap();
+
+    // Get the proposal id from the logs
+    let proposal_id: u64 = res.custom_attrs(1)[2].value.parse().unwrap();
+
+    // Owner votes, yes has not reached threshold, still open
+    let yes_vote = ExecuteMsg::Vote(VoteMsg {
+        proposal_id,
+        vote: Vote::Yes,
+    });
+    let res = app.execute_contract(Addr::unchecked(OWNER), dao_addr.clone(), &yes_vote, &[]);
+    assert!(res.is_ok());
+
+    // Cast a No vote, no has not reached threshold, still open
+    let no_vote = ExecuteMsg::Vote(VoteMsg {
+        proposal_id,
+        vote: Vote::No,
+    });
+    let res = app
+        .execute_contract(Addr::unchecked(VOTER2), dao_addr.clone(), &no_vote, &[])
+        .unwrap();
+    assert_eq!(
+        res.custom_attrs(1),
+        [
+            ("action", "vote"),
+            ("sender", VOTER2),
+            ("proposal_id", proposal_id.to_string().as_str()),
+            ("status", "Open"),
+        ],
+    );
+
+    // Power voter votes no, no reaches threshold + quorum reached, proposal rejected
+    let res = app
+        .execute_contract(Addr::unchecked(POWER_VOTER), dao_addr, &no_vote, &[])
+        .unwrap();
+
+    assert_eq!(
+        res.custom_attrs(1),
+        [
+            ("action", "vote"),
+            ("sender", POWER_VOTER),
+            ("proposal_id", proposal_id.to_string().as_str()),
+            ("status", "Rejected"),
+        ],
+    );
 }
 
 #[test]
diff --git a/contracts/cw3-multisig/src/tests.rs b/contracts/cw3-multisig/src/tests.rs
index 1bfadf90..f76e9f8c 100644
--- a/contracts/cw3-multisig/src/tests.rs
+++ b/contracts/cw3-multisig/src/tests.rs
@@ -1130,6 +1130,66 @@ fn test_vote_works() {
         .query_wasm_smart(&multisig_addr, &QueryMsg::Vote { proposal_id, voter })
         .unwrap();
     assert!(vote.vote.is_none());
+
+    // member(OWNER, 0),
+    // member(VOTER1, 1),
+    // member(VOTER2, 2),
+    // member(VOTER3, 3),
+    // member(VOTER4, 12), // so that he alone can pass a 50 / 52% threshold proposal
+    // member(VOTER5, 5),
+
+    // Testing for rejecting when threshold reached
+    // create proposal with 0 vote power
+    let proposal = pay_somebody_proposal();
+    let res = app
+        .execute_contract(
+            Addr::unchecked(OWNER),
+            multisig_addr.clone(),
+            &proposal,
+            &[],
+        )
+        .unwrap();
+
+    // Get the proposal id from the logs
+    let proposal_id: u64 = res.custom_attrs(1)[2].value.parse().unwrap();
+
+    // Cast a No vote, no has not reached threshold, still open
+    let no_vote = ExecuteMsg::Vote {
+        proposal_id,
+        vote: Vote::No,
+    };
+    let res = app
+        .execute_contract(
+            Addr::unchecked(VOTER2),
+            multisig_addr.clone(),
+            &no_vote,
+            &[],
+        )
+        .unwrap();
+    assert_eq!(
+        res.custom_attrs(1),
+        [
+            ("action", "vote"),
+            ("sender", VOTER2),
+            ("proposal_id", proposal_id.to_string().as_str()),
+            ("status", "Open"),
+        ],
+    );
+
+    // Cast a No vote, as threshold is 51 we have reached threshold and it will now be rejected
+    let res = app
+        .execute_contract(Addr::unchecked(VOTER4), multisig_addr, &no_vote, &[])
+        .unwrap();
+
+    assert_eq!(
+        res.custom_attrs(1),
+        [
+            ("action", "vote"),
+            ("sender", VOTER4),
+            ("proposal_id", proposal_id.to_string().as_str()),
+            ("status", "Rejected"),
+        ],
+    );
 }
 
 #[test]