Merge pull request #60 from confio/55-voting-contract-status-reading-…

…bugs

Voting Contract: vote and close uses current status

packages/voting-contract/src/lib.rs

@@ -116,12 +116,10 @@ where
 {
     // ensure proposal exists and can be voted on
     let mut prop = proposals().load(deps.storage, proposal_id)?;
-    if prop.status != Status::Open {
+
+    if prop.current_status(&env.block) != Status::Open {
         return Err(ContractError::NotOpen {});
     }
-    if prop.expires.is_expired(&env.block) {
-        return Err(ContractError::Expired {});
-    }
 
     // use a snapshot of "start of proposal"
     // Must be a member of voting group and have voting power >= 1
@@ -197,8 +195,15 @@ where
 {
     // anyone can trigger this if the vote passed
 
-    let mut prop = proposals::<P>().load(deps.storage, proposal_id)?;
-    if [Status::Executed, Status::Rejected, Status::Passed]
+    let mut prop = proposals().load(deps.storage, proposal_id)?;
+
+    if prop.status == Status::Rejected {
+        return Err(ContractError::NotOpen {});
+    }
+
+    prop.update_status(&env.block);
+
+    if [Status::Executed, Status::Passed]
         .iter()
         .any(|x| *x == prop.status)
     {
@@ -208,7 +213,6 @@ where
         return Err(ContractError::NotExpired {});
     }
 
-    // set it to failed
     prop.status = Status::Rejected;
     proposals::<P>().save(deps.storage, proposal_id, &prop)?;
 

packages/voting-contract/src/multitest/closing.rs

@@ -8,6 +8,7 @@ use crate::ContractError;
 fn expired_proposals_can_be_closed() {
     let rules = RulesBuilder::new()
         .with_threshold(Decimal::percent(51))
+        .with_quorum(Decimal::percent(35))
         .build();
 
     let mut suite = SuiteBuilder::new()
@@ -39,6 +40,7 @@ fn expired_proposals_can_be_closed() {
 fn active_proposals_cannot_be_closed() {
     let rules = RulesBuilder::new()
         .with_threshold(Decimal::percent(51))
+        .with_quorum(Decimal::percent(35))
         .build();
 
     let mut suite = SuiteBuilder::new()
@@ -81,6 +83,7 @@ fn passed_proposals_cannot_be_closed() {
 fn expired_proposals_cannot_be_closed_twice() {
     let rules = RulesBuilder::new()
         .with_threshold(Decimal::percent(51))
+        .with_quorum(Decimal::percent(60))
         .build();
 
     let mut suite = SuiteBuilder::new()
@@ -100,5 +103,5 @@ fn expired_proposals_cannot_be_closed_twice() {
     suite.close("anybody", proposal_id).unwrap();
     // ...but a closed one can't be closed again
     let err = suite.close("anybody", proposal_id).unwrap_err();
-    assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap());
+    assert_eq!(ContractError::NotOpen {}, err.downcast().unwrap());
 }

packages/voting-contract/src/multitest/voting.rs

@@ -349,6 +349,7 @@ fn passing_a_proposal_after_voting_period_works() {
 fn expired_proposals_cannot_be_voted_on() {
     let rules = RulesBuilder::new()
         .with_threshold(Decimal::percent(51))
+        .with_quorum(Decimal::percent(35))
         .build();
 
     let mut suite = SuiteBuilder::new()
@@ -366,5 +367,6 @@ fn expired_proposals_cannot_be_voted_on() {
 
     // Bob can't vote on the expired proposal
     let err = suite.vote("bob", proposal_id, Vote::Yes).unwrap_err();
-    assert_eq!(ContractError::Expired {}, err.downcast().unwrap());
+    // proposal that is open and expired is rejected
+    assert_eq!(ContractError::NotOpen {}, err.downcast().unwrap());
 }

packages/voting-contract/src/state.rs

@@ -47,6 +47,58 @@ impl<P> From<Proposal<P>> for ProposalInfo {
     }
 }
 
+impl<P> Proposal<P> {
+    /// current_status is non-mutable and returns what the status should be.
+    /// (designed for queries)
+    pub fn current_status(&self, block: &BlockInfo) -> Status {
+        let mut status = self.status;
+
+        // if open, check if voting is passed or timed out
+        if status == Status::Open && self.is_passed(block) {
+            status = Status::Passed;
+        }
+        if status == Status::Open && self.expires.is_expired(block) {
+            status = Status::Rejected;
+        }
+
+        status
+    }
+
+    /// update_status sets the status of the proposal to current_status.
+    /// (designed for handler logic)
+    pub fn update_status(&mut self, block: &BlockInfo) {
+        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 {
+        let VotingRules {
+            quorum,
+            threshold,
+            allow_end_early,
+            ..
+        } = self.rules;
+
+        // we always require the quorum
+        if self.votes.total() < votes_needed(self.total_weight, quorum) {
+            return false;
+        }
+        if self.expires.is_expired(block) {
+            // If expired, we compare Yes votes against the total number of votes (minus abstain).
+            let opinions = self.votes.total() - self.votes.abstain;
+            self.votes.yes >= votes_needed(opinions, threshold)
+        } else if allow_end_early {
+            // If not expired, we must assume all non-votes will be cast as No.
+            // We compare threshold against the total weight (minus abstain).
+            let possible_opinions = self.total_weight - self.votes.abstain;
+            self.votes.yes >= votes_needed(possible_opinions, threshold)
+        } else {
+            false
+        }
+    }
+}
+
 /// Note, if you are storing custom messages in the proposal,
 /// the querier needs to know what possible custom message types
 /// those are in order to parse the response
@@ -121,7 +173,7 @@ impl RulesBuilder {
     pub fn new() -> Self {
         Self {
             voting_period: 14,
-            quorum: Decimal::percent(1),
+            quorum: Decimal::percent(20),
             threshold: Decimal::percent(50),
             allow_end_early: true,
         }
@@ -193,58 +245,6 @@ impl Votes {
     }
 }
 
-impl<P> Proposal<P> {
-    /// current_status is non-mutable and returns what the status should be.
-    /// (designed for queries)
-    pub fn current_status(&self, block: &BlockInfo) -> Status {
-        let mut status = self.status;
-
-        // if open, check if voting is passed or timed out
-        if status == Status::Open && self.is_passed(block) {
-            status = Status::Passed;
-        }
-        if status == Status::Open && self.expires.is_expired(block) {
-            status = Status::Rejected;
-        }
-
-        status
-    }
-
-    /// update_status sets the status of the proposal to current_status.
-    /// (designed for handler logic)
-    pub fn update_status(&mut self, block: &BlockInfo) {
-        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 {
-        let VotingRules {
-            quorum,
-            threshold,
-            allow_end_early,
-            ..
-        } = self.rules;
-
-        // we always require the quorum
-        if self.votes.total() < votes_needed(self.total_weight, quorum) {
-            return false;
-        }
-        if self.expires.is_expired(block) {
-            // If expired, we compare Yes votes against the total number of votes (minus abstain).
-            let opinions = self.votes.total() - self.votes.abstain;
-            self.votes.yes >= votes_needed(opinions, threshold)
-        } else if allow_end_early {
-            // If not expired, we must assume all non-votes will be cast as No.
-            // We compare threshold against the total weight (minus abstain).
-            let possible_opinions = self.total_weight - self.votes.abstain;
-            self.votes.yes >= votes_needed(possible_opinions, threshold)
-        } else {
-            false
-        }
-    }
-}
-
 // this is a helper function so Decimal works with u64 rather than Uint128
 // also, we must *round up* here, as we need 8, not 7 votes to reach 50% of 15 total
 fn votes_needed(weight: u64, percentage: Decimal) -> u64 {