Revoting multiple (DA0-DA0#433)

* allow revoting on prop-multiple

* proposal revoting tests

* tests; nullifying existing vote in case of a revote

* simplifying tests; formatting

* cargo schema; types

* checked arithmetics when adding & removing votes; improving majority revote rejection prop test

* mapping errors when doing checked arithmetic on multiple choice votes

* handling old vote removal within ballots update closure

* updating tests

Author: bekauz

contracts/cw-proposal-multiple/schema/config_response.json

@@ -4,13 +4,18 @@
   "description": "The governance module's configuration.",
   "type": "object",
   "required": [
+    "allow_revoting",
     "close_proposal_on_execution_failure",
     "dao",
     "max_voting_period",
     "only_members_execute",
     "voting_strategy"
   ],
   "properties": {
+    "allow_revoting": {
+      "description": "Allows changing votes before the proposal expires. If this is enabled proposals will not be able to complete early as final vote information is not known until the time of proposal expiration.",
+      "type": "boolean"
+    },
     "close_proposal_on_execution_failure": {
       "description": "If set to true proposals will be closed if their execution fails. Otherwise, proposals will remain open after execution failure. For example, with this enabled a proposal to send 5 tokens out of a DAO's treasury with 4 tokens would be closed when it is executed. With this disabled, that same proposal would remain open until the DAO's treasury was large enough for it to be executed.",
       "type": "boolean"

contracts/cw-proposal-multiple/schema/execute_msg.json

@@ -129,13 +129,18 @@
         "update_config": {
           "type": "object",
           "required": [
+            "allow_revoting",
             "close_proposal_on_execution_failure",
             "dao",
             "max_voting_period",
             "only_members_execute",
             "voting_strategy"
           ],
           "properties": {
+            "allow_revoting": {
+              "description": "Allows changing votes before the proposal expires. If this is enabled proposals will not be able to complete early as final vote information is not known until the time of proposal expiration.",
+              "type": "boolean"
+            },
             "close_proposal_on_execution_failure": {
               "description": "If set to true proposals will be closed if their execution fails. Otherwise, proposals will remain open after execution failure. For example, with this enabled a proposal to send 5 tokens out of a DAO's treasury with 4 tokens would be closed when it is executed. With this disabled, that same proposal would remain open until the DAO's treasury was large enough for it to be executed.",
               "type": "boolean"

contracts/cw-proposal-multiple/schema/instantiate_msg.json

@@ -3,12 +3,17 @@
   "title": "InstantiateMsg",
   "type": "object",
   "required": [
+    "allow_revoting",
     "close_proposal_on_execution_failure",
     "max_voting_period",
     "only_members_execute",
     "voting_strategy"
   ],
   "properties": {
+    "allow_revoting": {
+      "description": "Allows changing votes before the proposal expires. If this is enabled proposals will not be able to complete early as final vote information is not known until the time of proposal expiration.",
+      "type": "boolean"
+    },
     "close_proposal_on_execution_failure": {
       "description": "If set to true proposals will be closed if their execution fails. Otherwise, proposals will remain open after execution failure. For example, with this enabled a proposal to send 5 tokens out of a DAO's treasury with 4 tokens would be closed when it is executed. With this disabled, that same proposal would remain open until the DAO's treasury was large enough for it to be executed.",
       "type": "boolean"

contracts/cw-proposal-multiple/schema/list_proposals_response.json

@@ -569,6 +569,7 @@
     "MultipleChoiceProposal": {
       "type": "object",
       "required": [
+        "allow_revoting",
         "choices",
         "description",
         "expiration",
@@ -581,6 +582,9 @@
         "voting_strategy"
       ],
       "properties": {
+        "allow_revoting": {
+          "type": "boolean"
+        },
         "choices": {
           "type": "array",
           "items": {

contracts/cw-proposal-multiple/schema/proposal_response.json

@@ -573,6 +573,7 @@
     "MultipleChoiceProposal": {
       "type": "object",
       "required": [
+        "allow_revoting",
         "choices",
         "description",
         "expiration",
@@ -585,6 +586,9 @@
         "voting_strategy"
       ],
       "properties": {
+        "allow_revoting": {
+          "type": "boolean"
+        },
         "choices": {
           "type": "array",
           "items": {

contracts/cw-proposal-multiple/schema/reverse_proposals_response.json

@@ -569,6 +569,7 @@
     "MultipleChoiceProposal": {
       "type": "object",
       "required": [
+        "allow_revoting",
         "choices",
         "description",
         "expiration",
@@ -581,6 +582,9 @@
         "voting_strategy"
       ],
       "properties": {
+        "allow_revoting": {
+          "type": "boolean"
+        },
         "choices": {
           "type": "array",
           "items": {

contracts/cw-proposal-multiple/src/contract.rs

@@ -61,6 +61,7 @@ pub fn instantiate(
         min_voting_period,
         max_voting_period,
         only_members_execute: msg.only_members_execute,
+        allow_revoting: msg.allow_revoting,
         dao,
         deposit_info,
         close_proposal_on_execution_failure: msg.close_proposal_on_execution_failure,
@@ -96,6 +97,7 @@ pub fn execute(
             min_voting_period,
             max_voting_period,
             only_members_execute,
+            allow_revoting,
             dao,
             deposit_info,
             close_proposal_on_execution_failure,
@@ -106,6 +108,7 @@ pub fn execute(
             min_voting_period,
             max_voting_period,
             only_members_execute,
+            allow_revoting,
             dao,
             deposit_info,
             close_proposal_on_execution_failure,
@@ -176,6 +179,7 @@ pub fn execute_propose(
             total_power,
             status: Status::Open,
             votes: MultipleChoiceVotes::zero(checked_multiple_choice_options.len()),
+            allow_revoting: config.allow_revoting,
             deposit_info: config.deposit_info.clone(),
             choices: checked_multiple_choice_options,
         };
@@ -256,7 +260,26 @@ pub fn execute_vote(
         deps.storage,
         (proposal_id, info.sender.clone()),
         |bal| match bal {
-            Some(_) => Err(ContractError::AlreadyVoted {}),
+            Some(current_ballot) => {
+                if prop.allow_revoting {
+                    if current_ballot.vote == vote {
+                        // Don't allow casting the same vote more than
+                        // once. This seems liable to be confusing
+                        // behavior.
+                        Err(ContractError::AlreadyCast {})
+                    } else {
+                        // Remove the old vote if this is a re-vote.
+                        prop.votes
+                            .remove_vote(current_ballot.vote, current_ballot.power)?;
+                        Ok(Ballot {
+                            power: vote_power,
+                            vote,
+                        })
+                    }
+                } else {
+                    Err(ContractError::AlreadyVoted {})
+                }
+            }
             None => Ok(Ballot {
                 vote,
                 power: vote_power,
@@ -265,7 +288,8 @@ pub fn execute_vote(
     )?;
 
     let old_status = prop.status;
-    prop.votes.add_vote(vote, vote_power);
+
+    prop.votes.add_vote(vote, vote_power)?;
     prop.update_status(&env.block)?;
     PROPOSALS.save(deps.storage, proposal_id, &prop)?;
     let new_status = prop.status;
@@ -440,6 +464,7 @@ pub fn execute_update_config(
     min_voting_period: Option<Duration>,
     max_voting_period: Duration,
     only_members_execute: bool,
+    allow_revoting: bool,
     dao: String,
     deposit_info: Option<DepositInfo>,
     close_proposal_on_execution_failure: bool,
@@ -468,6 +493,7 @@ pub fn execute_update_config(
             min_voting_period,
             max_voting_period,
             only_members_execute,
+            allow_revoting,
             dao,
             deposit_info,
             close_proposal_on_execution_failure,

contracts/cw-proposal-multiple/src/error.rs

@@ -40,9 +40,12 @@ pub enum ContractError {
     #[error("Not registered to vote (no voting power) at time of proposal creation.")]
     NotRegistered {},
 
-    #[error("Already voted")]
+    #[error("Already voted. This proposal does not support revoting.")]
     AlreadyVoted {},
 
+    #[error("Already cast a vote with that option. Change your vote to revote.")]
+    AlreadyCast {},
+
     #[error("Proposal must be in 'passed' state to be executed.")]
     NotPassed {},
 

contracts/cw-proposal-multiple/src/msg.rs

@@ -22,6 +22,11 @@ pub struct InstantiateMsg {
     /// proposals. Otherwise, any address may execute a passed
     /// proposal.
     pub only_members_execute: bool,
+    /// Allows changing votes before the proposal expires. If this is
+    /// enabled proposals will not be able to complete early as final
+    /// vote information is not known until the time of proposal
+    /// expiration.
+    pub allow_revoting: bool,
     /// Information about the deposit required to create a
     /// proposal. None if there is no deposit requirement, Some
     /// otherwise.
@@ -88,6 +93,11 @@ pub enum ExecuteMsg {
         /// proposals. Otherwise, any address may execute a passed
         /// proposal. Applies to all outstanding and future proposals.
         only_members_execute: bool,
+        /// Allows changing votes before the proposal expires. If this is
+        /// enabled proposals will not be able to complete early as final
+        /// vote information is not known until the time of proposal
+        /// expiration.
+        allow_revoting: bool,
         /// The address if tge DAO that this governance module is
         /// associated with.
         dao: String,