Refactor timelock

Should break less of the existing tests, doesn't lead to weird migration
state (passed at time zero), and if timelock configuration is updated it
won't affect currently open proposals.

contracts/proposal/dao-proposal-single/src/contract.rs

@@ -218,6 +218,7 @@ pub fn execute_propose(
             status: Status::Open,
             votes: Votes::zero(),
             allow_revoting: config.allow_revoting,
+            timelock: config.timelock,
         };
         // Update the proposal's status. Addresses case where proposal
         // expires on the same block as it is created.
@@ -266,21 +267,19 @@ pub fn execute_veto(
     info: MessageInfo,
     proposal_id: u64,
 ) -> Result<Response, ContractError> {
-    let config = CONFIG.load(deps.storage)?;
-
     let mut prop = PROPOSALS
         .may_load(deps.storage, proposal_id)?
         .ok_or(ContractError::NoSuchProposal { id: proposal_id })?;
 
     match prop.status {
-        Status::Passed { at_time } => {
-            match config.timelock {
-                Some(timelock) => {
-                    // Check if the proposal is timelocked
-                    timelock.is_locked(at_time, env.block.time)?;
+        Status::Timelocked { expires } => {
+            match prop.timelock {
+                Some(ref timelock) => {
+                    // Check if the proposal is still timelocked
+                    timelock.check_is_locked(env.block.time, expires)?;
 
                     // Check sender is vetoer
-                    timelock.is_vetoer(&info)?;
+                    timelock.check_is_vetoer(&info)?;
 
                     let old_status = prop.status;
 
@@ -323,7 +322,7 @@ pub fn execute_veto(
                         .add_attribute("proposal_id", proposal_id.to_string())
                         .add_submessages(hooks))
                 }
-                // If timelock is not configured throw error.
+                // If timelock is not configured throw error. This should never happen.
                 None => Err(ContractError::TimelockError(TimelockError::NoTimelock {})),
             }
         }
@@ -358,17 +357,19 @@ pub fn execute_execute(
     // Check here that the proposal is passed. Allow it to be executed
     // even if it is expired so long as it passed during its voting
     // period.
-    let old_status = prop.status;
     prop.update_status(&env.block);
+    let old_status = prop.status;
     match prop.status {
-        Status::Passed { at_time } => {
-            if let Some(timelock) = config.timelock {
-                // Check proposal is not timelocked
-                timelock.is_locked(at_time, env.block.time)?;
-
-                // If the sender is the vetoer, check if they can execute early
-                if timelock.is_vetoer(&info).is_ok() {
-                    timelock.early_excute_enabled()?;
+        Status::Passed {} => (),
+        Status::Timelocked { expires } => {
+            if let Some(ref timelock) = prop.timelock {
+                // Check if the sender is the vetoer
+                if timelock.check_is_vetoer(&info).is_ok() {
+                    // check if they can execute early
+                    timelock.check_early_excute_enabled()?;
+                } else {
+                    // Check if proposal is timelocked
+                    timelock.check_is_locked(env.block.time, expires)?;
                 }
             }
         }
@@ -1011,6 +1012,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result<Response, Co
                         status: v2_status_to_v3(prop.status),
                         votes: v2_votes_to_v3(prop.votes),
                         allow_revoting: prop.allow_revoting,
+                        timelock: None,
                     };
 
                     PROPOSALS

contracts/proposal/dao-proposal-single/src/msg.rs

@@ -75,7 +75,7 @@ pub enum ExecuteMsg {
     },
     /// Callable only if timelock is configured
     Veto {
-        /// The ID of the proposal to execute.
+        /// The ID of the proposal to veto.
         proposal_id: u64,
     },
     /// Closes a proposal that has failed (either not passed or timed

contracts/proposal/dao-proposal-single/src/proposal.rs

@@ -5,11 +5,14 @@ use cosmwasm_std::{Addr, BlockInfo, CosmosMsg, Decimal, Empty, StdResult, Storag
 use cw_utils::Expiration;
 use dao_voting::status::Status;
 use dao_voting::threshold::{PercentageThreshold, Threshold};
+use dao_voting::timelock::Timelock;
 use dao_voting::voting::{does_vote_count_fail, does_vote_count_pass, Votes};
 
 #[cw_serde]
 pub struct SingleChoiceProposal {
+    /// The title of the proposal
     pub title: String,
+    /// The main body of the proposal text
     pub description: String,
     /// The address that created this proposal.
     pub proposer: Addr,
@@ -31,9 +34,15 @@ pub struct SingleChoiceProposal {
     pub total_power: Uint128,
     /// The messages that will be executed should this proposal pass.
     pub msgs: Vec<CosmosMsg<Empty>>,
+    /// The proposal status
     pub status: Status,
+    /// Votes on a particular proposal
     pub votes: Votes,
+    /// Whether or not revoting is enabled. If revoting is enabled, a proposal
+    /// cannot pass until the voting period has elapsed.
     pub allow_revoting: bool,
+    /// Timelock info, if configured
+    pub timelock: Option<Timelock>,
 }
 
 pub fn next_proposal_id(store: &dyn Storage) -> StdResult<u64> {
@@ -62,8 +71,16 @@ impl SingleChoiceProposal {
     /// Gets the current status of the proposal.
     pub fn current_status(&self, block: &BlockInfo) -> Status {
         if self.status == Status::Open && self.is_passed(block) {
-            Status::Passed {
-                at_time: block.time,
+            // If time lock is configured for the proposal, calculate lock
+            // expiration and set status to Timelocked.
+            //
+            // Otherwise the proposal is simply passed
+            if let Some(timelock) = &self.timelock {
+                Status::Timelocked {
+                    expires: timelock.calculate_timelock_expiration(block.time),
+                }
+            } else {
+                Status::Passed
             }
         } else if self.status == Status::Open
             && (self.expiration.is_expired(block) || self.is_rejected(block))
@@ -277,6 +294,7 @@ mod test {
             msgs: vec![],
             status: Status::Open,
             threshold,
+            timelock: None,
             total_power,
             votes,
         };

contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs

@@ -139,12 +139,7 @@ fn test_execute_proposal_more_than_once() {
 
     // assert proposal is passed, execute it
     let proposal = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {});
     execute_proposal(&mut app, &proposal_module, CREATOR_ADDR, proposal_id);
 
     app.update_block(next_block);
@@ -338,12 +333,7 @@ pub fn test_passed_prop_state_remains_after_vote_swing() {
 
     // assert proposal is passed with 20 votes in favor and none opposed
     let proposal = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {});
     assert_eq!(proposal.proposal.votes.yes, Uint128::new(20));
     assert_eq!(proposal.proposal.votes.no, Uint128::zero());
 
@@ -370,12 +360,7 @@ pub fn test_passed_prop_state_remains_after_vote_swing() {
     // assert that the late votes have been counted and proposal
     // is still in passed state before executing it
     let proposal = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {});
     assert_eq!(proposal.proposal.votes.yes, Uint128::new(20));
     assert_eq!(proposal.proposal.votes.no, Uint128::new(80));
 

contracts/proposal/dao-proposal-single/src/testing/tests.rs

@@ -131,6 +131,7 @@ fn test_simple_propose_staked_balances() {
         total_power: Uint128::new(100_000_000),
         msgs: vec![],
         status: Status::Open,
+        timelock: None,
         votes: Votes::zero(),
     };
 
@@ -180,6 +181,7 @@ fn test_simple_proposal_cw4_voting() {
         total_power: Uint128::new(1),
         msgs: vec![],
         status: Status::Open,
+        timelock: None,
         votes: Votes::zero(),
     };
 
@@ -285,6 +287,7 @@ fn test_instantiate_with_non_voting_module_cw20_deposit() {
         msgs: vec![],
         status: Status::Open,
         votes: Votes::zero(),
+        timelock: None,
     };
 
     assert_eq!(created.proposal, expected);
@@ -350,12 +353,7 @@ fn test_proposal_message_execution() {
         Vote::Yes,
     );
     let proposal = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {});
 
     // Can't use library function because we expect this to fail due
     // to insufficent balance in the bank module.
@@ -367,12 +365,7 @@ fn test_proposal_message_execution() {
     )
     .unwrap_err();
     let proposal = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {});
 
     mint_natives(&mut app, core_addr.as_str(), coins(10, "ujuno"));
     execute_proposal(&mut app, &proposal_module, CREATOR_ADDR, proposal_id);
@@ -457,12 +450,7 @@ fn test_proposal_cant_close_after_expiry_is_passed() {
     // Expire the proposal. This should pass it.
     app.update_block(|b| b.time = b.time.plus_seconds(604800));
     let proposal = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1572402219879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {},);
 
     // Make sure it can't be closed.
     let err = close_proposal_should_fail(&mut app, &proposal_module, CREATOR_ADDR, proposal_id);
@@ -746,6 +734,7 @@ fn test_anyone_may_propose_and_proposal_listing() {
                     no: Uint128::zero(),
                     abstain: Uint128::zero()
                 },
+                timelock: None
             }
         }
     )
@@ -1079,12 +1068,7 @@ fn test_min_voting_period_no_early_pass() {
 
     app.update_block(|block| block.height += 10);
     let proposal_response = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal_response.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal_response.proposal.status, Status::Passed {});
 }
 
 // Setting the min duration the same as the proposal duration just
@@ -1122,12 +1106,7 @@ fn test_min_duration_same_as_proposal_duration() {
 
     app.update_block(|b| b.height += 100);
     let proposal_response = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal_response.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal_response.proposal.status, Status::Passed {});
 }
 
 #[test]
@@ -1186,12 +1165,7 @@ fn test_revoting_playthrough() {
     // Expire the proposal allowing the votes to be tallied.
     app.update_block(|b| b.time = b.time.plus_seconds(604800));
     let proposal_response = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal_response.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1572402219879305533)
-        }
-    );
+    assert_eq!(proposal_response.proposal.status, Status::Passed {});
     execute_proposal(&mut app, &proposal_module, CREATOR_ADDR, proposal_id);
 
     // Can't vote once the proposal is passed.
@@ -1264,12 +1238,7 @@ fn test_allow_revoting_config_changes() {
 
     // Proposal without revoting should have passed.
     let proposal_resp = query_proposal(&app, &proposal_module, no_revoting_proposal);
-    assert_eq!(
-        proposal_resp.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal_resp.proposal.status, Status::Passed {});
 
     // Proposal with revoting should not have passed.
     let proposal_resp = query_proposal(&app, &proposal_module, revoting_proposal);
@@ -1347,12 +1316,7 @@ fn test_three_of_five_multisig() {
     vote_on_proposal(&mut app, &proposal_module, "three", proposal_id, Vote::Yes);
 
     let proposal: ProposalResponse = query_proposal(&app, &proposal_module, 1);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {});
 
     execute_proposal(&mut app, &proposal_module, "four", proposal_id);
 
@@ -1479,9 +1443,7 @@ fn test_absolute_count_threshold_non_multisig() {
         Threshold::AbsoluteCount {
             threshold: Uint128::new(11),
         },
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533),
-        },
+        Status::Passed {},
         None,
     );
 }
@@ -1985,12 +1947,7 @@ fn test_execution_failed() {
     // Even though this proposal was created before the config change
     // was made it still gets retroactively applied.
     let proposal = query_proposal(&app, &proposal_module, proposal_id);
-    assert_eq!(
-        proposal.proposal.status,
-        Status::Passed {
-            at_time: Timestamp::from_nanos(1571797419879305533)
-        }
-    );
+    assert_eq!(proposal.proposal.status, Status::Passed {});
 
     // This proposal's deposit should not have been returned. It will
     // not be returnable until this is executed, or close on execution
@@ -2026,6 +1983,7 @@ fn test_reply_proposal_mock() {
                 total_power: Uint128::new(100_000_000),
                 msgs: vec![],
                 status: Status::Open,
+                timelock: None,
                 votes: Votes::zero(),
             },
         )