diff --git a/contracts/proposal/dao-proposal-single/src/contract.rs b/contracts/proposal/dao-proposal-single/src/contract.rs index df1373f07..31bbcec4b 100644 --- a/contracts/proposal/dao-proposal-single/src/contract.rs +++ b/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 { - 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>, + /// 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, } pub fn next_proposal_id(store: &dyn Storage) -> StdResult { @@ -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, }; diff --git a/contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs b/contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs index 66186f97f..7c285568a 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs +++ b/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)); diff --git a/contracts/proposal/dao-proposal-single/src/testing/tests.rs b/contracts/proposal/dao-proposal-single/src/testing/tests.rs index 4b291b513..6b9505206 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/tests.rs +++ b/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(), }, ) diff --git a/packages/dao-testing/src/tests.rs b/packages/dao-testing/src/tests.rs index 4d9b5c223..17a9b6023 100644 --- a/packages/dao-testing/src/tests.rs +++ b/packages/dao-testing/src/tests.rs @@ -1,4 +1,4 @@ -use cosmwasm_std::{Decimal, Timestamp, Uint128}; +use cosmwasm_std::{Decimal, Uint128}; use dao_voting::status::Status; use dao_voting::threshold::{PercentageThreshold, Threshold}; use dao_voting::voting::Vote; @@ -40,9 +40,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(100)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); @@ -75,9 +73,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(100)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); } @@ -96,9 +92,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(100)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); @@ -120,9 +114,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(99)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ) } @@ -216,9 +208,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(1)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, Some(Uint128::new(100)), ); @@ -232,9 +222,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(1)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, Some(Uint128::new(1000)), ); @@ -330,9 +318,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(50)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); @@ -354,9 +340,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(50)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); @@ -385,9 +369,7 @@ where Threshold::AbsolutePercentage { percentage: PercentageThreshold::Percent(Decimal::percent(50)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); } @@ -415,9 +397,7 @@ where threshold: PercentageThreshold::Percent(Decimal::percent(10)), quorum: PercentageThreshold::Majority {}, }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); @@ -447,9 +427,7 @@ where threshold: PercentageThreshold::Percent(Decimal::percent(10)), quorum: PercentageThreshold::Majority {}, }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); } @@ -477,9 +455,7 @@ where threshold: PercentageThreshold::Percent(Decimal::percent(50)), quorum: PercentageThreshold::Majority {}, }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, None, ); @@ -559,9 +535,7 @@ where threshold: PercentageThreshold::Majority {}, quorum: PercentageThreshold::Percent(Decimal::percent(60)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, Some(Uint128::new(100)), ); do_votes( @@ -590,9 +564,7 @@ where threshold: PercentageThreshold::Majority {}, quorum: PercentageThreshold::Percent(Decimal::percent(60)), }, - Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + Status::Passed {}, Some(Uint128::new(100)), ); do_votes( @@ -627,9 +599,7 @@ where std::cmp::Ordering::Less => Status::Rejected, // Depends on which reaches the threshold first. Ignore for now. std::cmp::Ordering::Equal => Status::Rejected, - std::cmp::Ordering::Greater => Status::Passed { - at_time: Timestamp::from_nanos(1572402219879305533), - }, + std::cmp::Ordering::Greater => Status::Passed {}, }; let yes = yes diff --git a/packages/dao-voting/src/status.rs b/packages/dao-voting/src/status.rs index 1295ff270..f75147a79 100644 --- a/packages/dao-voting/src/status.rs +++ b/packages/dao-voting/src/status.rs @@ -1,7 +1,6 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::Timestamp; -// TODO add more timestamps for consistency? #[cw_serde] #[derive(Copy)] pub enum Status { @@ -10,7 +9,7 @@ pub enum Status { /// The proposal has been rejected. Rejected, /// The proposal has been passed but has not been executed. - Passed { at_time: Timestamp }, + Passed, /// The proposal has been passed and executed. Executed, /// The proposal has failed or expired and has been closed. A @@ -18,6 +17,9 @@ pub enum Status { Closed, /// The proposal's execution failed. ExecutionFailed, + /// Proposal is timelocked and can not be until the timelock expires + /// During this time the proposal may be vetoed. + Timelocked { expires: Timestamp }, /// The proposal has been vetoed. Vetoed, } @@ -27,10 +29,11 @@ impl std::fmt::Display for Status { match self { Status::Open => write!(f, "open"), Status::Rejected => write!(f, "rejected"), - Status::Passed { at_time } => write!(f, "passed {:?}", at_time), + Status::Passed => write!(f, "passed"), Status::Executed => write!(f, "executed"), Status::Closed => write!(f, "closed"), Status::ExecutionFailed => write!(f, "execution_failed"), + Status::Timelocked { expires } => write!(f, "timelocked {:?}", expires), Status::Vetoed => write!(f, "vetoed"), } } diff --git a/packages/dao-voting/src/timelock.rs b/packages/dao-voting/src/timelock.rs index 30f8dbf4d..967807955 100644 --- a/packages/dao-voting/src/timelock.rs +++ b/packages/dao-voting/src/timelock.rs @@ -35,8 +35,13 @@ pub struct Timelock { } impl Timelock { + /// Calculate the expiration time for the timelock + pub fn calculate_timelock_expiration(&self, current_time: Timestamp) -> Timestamp { + Timestamp::from_seconds(current_time.seconds() + self.delay.seconds()) + } + /// Whether early execute is enabled - pub fn early_excute_enabled(&self) -> Result<(), TimelockError> { + pub fn check_early_excute_enabled(&self) -> Result<(), TimelockError> { if self.early_execute { Ok(()) } else { @@ -45,12 +50,12 @@ impl Timelock { } /// Takes two timestamps and returns true if the proposal is locked or not. - pub fn is_locked( + pub fn check_is_locked( &self, - proposal_passed: Timestamp, current_time: Timestamp, + expires: Timestamp, ) -> Result<(), TimelockError> { - if proposal_passed.seconds() + self.delay.seconds() < current_time.seconds() { + if current_time.seconds() > expires.seconds() { Ok(()) } else { Err(TimelockError::Timelocked {}) @@ -58,7 +63,7 @@ impl Timelock { } /// Checks whether the message sender is the vetoer. - pub fn is_vetoer(&self, info: &MessageInfo) -> Result<(), TimelockError> { + pub fn check_is_vetoer(&self, info: &MessageInfo) -> Result<(), TimelockError> { if self.vetoer == info.sender.to_string() { Ok(()) } else {