diff --git a/Cargo.lock b/Cargo.lock index 58d4b79d2..6565c11db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1851,6 +1851,7 @@ dependencies = [ name = "dao-proposal-multiple" version = "2.3.0" dependencies = [ + "anyhow", "cosmwasm-schema", "cosmwasm-std", "cosmwasm-storage", @@ -1888,6 +1889,7 @@ dependencies = [ name = "dao-proposal-single" version = "2.3.0" dependencies = [ + "anyhow", "cosmwasm-schema", "cosmwasm-std", "cosmwasm-storage", diff --git a/contracts/proposal/dao-proposal-multiple/Cargo.toml b/contracts/proposal/dao-proposal-multiple/Cargo.toml index a7016b1ee..92cc9004a 100644 --- a/contracts/proposal/dao-proposal-multiple/Cargo.toml +++ b/contracts/proposal/dao-proposal-multiple/Cargo.toml @@ -43,6 +43,7 @@ dao-pre-propose-multiple = { workspace = true } voting-v1 = { workspace = true } [dev-dependencies] +anyhow = { workspace = true } cw-multi-test = { workspace = true } dao-voting-cw4 = { workspace = true } dao-voting-cw20-balance = { workspace = true } diff --git a/contracts/proposal/dao-proposal-multiple/src/contract.rs b/contracts/proposal/dao-proposal-multiple/src/contract.rs index 83053b95e..632060f53 100644 --- a/contracts/proposal/dao-proposal-multiple/src/contract.rs +++ b/contracts/proposal/dao-proposal-multiple/src/contract.rs @@ -64,7 +64,7 @@ pub fn instantiate( // if veto is configured, validate its fields if let Some(veto_config) = &msg.veto { - veto_config.validate(&deps.as_ref())?; + veto_config.validate(&deps.as_ref(), &max_voting_period)?; }; let config = Config { @@ -631,7 +631,7 @@ pub fn execute_update_config( // if veto is configured, validate its fields if let Some(veto_config) = &veto { - veto_config.validate(&deps.as_ref())?; + veto_config.validate(&deps.as_ref(), &max_voting_period)?; }; CONFIG.save( diff --git a/contracts/proposal/dao-proposal-multiple/src/proposal.rs b/contracts/proposal/dao-proposal-multiple/src/proposal.rs index b73e5e835..454a60762 100644 --- a/contracts/proposal/dao-proposal-multiple/src/proposal.rs +++ b/contracts/proposal/dao-proposal-multiple/src/proposal.rs @@ -93,6 +93,14 @@ impl MultipleChoiceProposal { Status::Open if self.expiration.is_expired(block) || self.is_rejected(block)? => { Ok(Status::Rejected) } + Status::VetoTimelock { expiration } => { + // if prop timelock expired, proposal is now passed. + if expiration.is_expired(block) { + Ok(Status::Passed) + } else { + Ok(self.status) + } + } _ => Ok(self.status), } } diff --git a/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs b/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs index 2b20a9439..8064733d1 100644 --- a/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs +++ b/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs @@ -20,6 +20,7 @@ use dao_voting::{ status::Status, threshold::{ActiveThreshold, PercentageThreshold, Threshold}, }; +use std::ops::Add; use std::panic; use crate::{ @@ -4637,6 +4638,9 @@ fn test_open_proposal_passes_with_zero_timelock_veto_duration() { ) .unwrap(); + // pass enough time to expire the proposal voting + app.update_block(|b| b.height += 7); + let proposal: ProposalResponse = query_proposal(&app, &govmod, 1); assert_eq!(proposal.proposal.status, Status::Passed {},); @@ -4900,11 +4904,11 @@ fn test_veto_open_prop_with_veto_before_passed_disabled() { } #[test] -fn test_veto_when_veto_timelock_expired() { +fn test_veto_when_veto_timelock_expired() -> anyhow::Result<()> { let mut app = App::default(); - let timelock_duration = 3; + let timelock_duration = Duration::Height(3); let veto_config = VetoConfig { - timelock_duration: Duration::Height(timelock_duration), + timelock_duration, vetoer: "vetoer".to_string(), early_execute: false, veto_before_passed: false, @@ -4990,7 +4994,7 @@ fn test_veto_when_veto_timelock_expired() { assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: cw_utils::Expiration::AtHeight(app.block_info().height + timelock_duration), + expiration: proposal.proposal.expiration.add(timelock_duration)?, }, ); @@ -5009,14 +5013,16 @@ fn test_veto_when_veto_timelock_expired() { .unwrap(); assert_eq!(err, ContractError::VetoError(VetoError::TimelockExpired {}),); + + Ok(()) } #[test] -fn test_veto_sets_prop_status_to_vetoed() { +fn test_veto_sets_prop_status_to_vetoed() -> anyhow::Result<()> { let mut app = App::default(); - let timelock_duration = 3; + let timelock_duration = Duration::Height(3); let veto_config = VetoConfig { - timelock_duration: Duration::Height(timelock_duration), + timelock_duration, vetoer: "vetoer".to_string(), early_execute: false, veto_before_passed: false, @@ -5102,7 +5108,7 @@ fn test_veto_sets_prop_status_to_vetoed() { assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: cw_utils::Expiration::AtHeight(app.block_info().height + timelock_duration), + expiration: proposal.proposal.expiration.add(timelock_duration)?, }, ); @@ -5117,6 +5123,8 @@ fn test_veto_sets_prop_status_to_vetoed() { let proposal: ProposalResponse = query_proposal(&app, &govmod, 1); assert_eq!(proposal.proposal.status, Status::Vetoed {},); + + Ok(()) } #[test] @@ -5240,11 +5248,11 @@ fn test_veto_from_catchall_state() { } #[test] -fn test_veto_timelock_early_execute_happy() { +fn test_veto_timelock_early_execute_happy() -> anyhow::Result<()> { let mut app = App::default(); - let timelock_duration = 3; + let timelock_duration = Duration::Height(3); let veto_config = VetoConfig { - timelock_duration: Duration::Height(timelock_duration), + timelock_duration, vetoer: "vetoer".to_string(), early_execute: true, veto_before_passed: false, @@ -5330,7 +5338,7 @@ fn test_veto_timelock_early_execute_happy() { assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: cw_utils::Expiration::AtHeight(app.block_info().height + timelock_duration), + expiration: proposal.proposal.expiration.add(timelock_duration)?, }, ); @@ -5358,14 +5366,16 @@ fn test_veto_timelock_early_execute_happy() { let proposal: ProposalResponse = query_proposal(&app, &govmod, 1); assert_eq!(proposal.proposal.status, Status::Executed {},); + + Ok(()) } #[test] -fn test_veto_timelock_expires_happy() { +fn test_veto_timelock_expires_happy() -> anyhow::Result<()> { let mut app = App::default(); - let timelock_duration = 3; + let timelock_duration = Duration::Height(3); let veto_config = VetoConfig { - timelock_duration: Duration::Height(timelock_duration), + timelock_duration, vetoer: "vetoer".to_string(), early_execute: false, veto_before_passed: false, @@ -5451,7 +5461,7 @@ fn test_veto_timelock_expires_happy() { assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: cw_utils::Expiration::AtHeight(app.block_info().height + timelock_duration), + expiration: proposal.proposal.expiration.add(timelock_duration)?, }, ); @@ -5468,4 +5478,6 @@ fn test_veto_timelock_expires_happy() { let proposal: ProposalResponse = query_proposal(&app, &govmod, 1); assert_eq!(proposal.proposal.status, Status::Executed {},); + + Ok(()) } diff --git a/contracts/proposal/dao-proposal-single/Cargo.toml b/contracts/proposal/dao-proposal-single/Cargo.toml index eb23de7f3..af3d872ce 100644 --- a/contracts/proposal/dao-proposal-single/Cargo.toml +++ b/contracts/proposal/dao-proposal-single/Cargo.toml @@ -39,6 +39,7 @@ voting-v1 = { workspace = true } cw-proposal-single-v1 = { workspace = true, features = ["library"] } [dev-dependencies] +anyhow = { workspace = true } cosmwasm-schema = { workspace = true } cw-multi-test = { workspace = true } dao-dao-core = { workspace = true } diff --git a/contracts/proposal/dao-proposal-single/src/contract.rs b/contracts/proposal/dao-proposal-single/src/contract.rs index 88425438c..d9ac7c07f 100644 --- a/contracts/proposal/dao-proposal-single/src/contract.rs +++ b/contracts/proposal/dao-proposal-single/src/contract.rs @@ -65,7 +65,7 @@ pub fn instantiate( // if veto is configured, validate its fields if let Some(veto_config) = &msg.veto { - veto_config.validate(&deps.as_ref())?; + veto_config.validate(&deps.as_ref(), &max_voting_period)?; }; let config = Config { @@ -650,7 +650,7 @@ pub fn execute_update_config( // if veto is configured, validate its fields if let Some(veto_config) = &veto { - veto_config.validate(&deps.as_ref())?; + veto_config.validate(&deps.as_ref(), &max_voting_period)?; }; CONFIG.save( @@ -959,19 +959,21 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result { Ok(Status::Rejected) } + Status::VetoTimelock { expiration } => { + // if prop timelock expired, proposal is now passed. + if expiration.is_expired(block) { + Ok(Status::Passed) + } else { + Ok(self.status) + } + } _ => Ok(self.status), } } diff --git a/contracts/proposal/dao-proposal-single/src/testing/migration_tests.rs b/contracts/proposal/dao-proposal-single/src/testing/migration_tests.rs index 6b189f8cc..3dc5c33e8 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/migration_tests.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/migration_tests.rs @@ -1,6 +1,5 @@ use cosmwasm_std::{to_json_binary, Addr, Uint128, WasmMsg}; use cw20::Cw20Coin; -use cw_multi_test::error::AnyError; use cw_multi_test::{next_block, App, Executor}; use cw_utils::Duration; use dao_interface::query::{GetItemResponse, ProposalModuleCountResponse}; @@ -311,30 +310,6 @@ fn test_v1_v2_full_migration() { false, ); - // first we try to migrate with an invalid veto config and assert the err - let err: AnyError = app - .migrate_contract( - core.clone(), - proposal.clone(), - &crate::msg::MigrateMsg::FromV1 { - close_proposal_on_execution_failure: true, - pre_propose_info: pre_propose_info.clone(), - veto: Some(VetoConfig { - timelock_duration: Duration::Time(0), - vetoer: sender.to_string(), - early_execute: true, - veto_before_passed: false, - }), - }, - v2_proposal_code, - ) - .unwrap_err(); - - assert_eq!( - "Zero timelock duration is only permitted with veto_before_passed", - err.root_cause().to_string(), - ); - // now migrate with valid config app.execute_contract( sender.clone(), @@ -360,7 +335,7 @@ fn test_v1_v2_full_migration() { close_proposal_on_execution_failure: true, pre_propose_info, veto: Some(VetoConfig { - timelock_duration: Duration::Time(1000), + timelock_duration: Duration::Height(10), vetoer: sender.to_string(), early_execute: true, veto_before_passed: false, @@ -476,7 +451,7 @@ fn test_v1_v2_full_migration() { assert_eq!( new_prop.proposal.veto, Some(VetoConfig { - timelock_duration: Duration::Time(1000), + timelock_duration: Duration::Height(10), vetoer: sender.to_string(), early_execute: true, veto_before_passed: false, diff --git a/contracts/proposal/dao-proposal-single/src/testing/tests.rs b/contracts/proposal/dao-proposal-single/src/testing/tests.rs index c1994703a..63ddd5b9f 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/tests.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/tests.rs @@ -1,3 +1,5 @@ +use std::ops::Add; + use cosmwasm_std::{ coins, testing::{mock_dependencies, mock_env}, @@ -380,7 +382,7 @@ fn test_proposal_message_execution() { } #[test] -fn test_proposal_message_timelock_execution() { +fn test_proposal_message_timelock_execution() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); let veto_config = VetoConfig { @@ -445,11 +447,14 @@ fn test_proposal_message_timelock_execution() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); @@ -485,13 +490,15 @@ fn test_proposal_message_timelock_execution() { // Time passes app.update_block(|block| { - block.time = block.time.plus_seconds(604800); + block.time = block.time.plus_seconds(604800 + 200); }); // Proposal executes successfully execute_proposal(&mut app, &proposal_module, CREATOR_ADDR, proposal_id); let proposal = query_proposal(&app, &proposal_module, proposal_id); assert_eq!(proposal.proposal.status, Status::Executed); + + Ok(()) } // only the authorized vetoer can veto an open proposal @@ -817,7 +824,7 @@ fn test_open_proposal_veto_early() { // only the vetoer can veto during timelock period #[test] -fn test_timelocked_proposal_veto_unauthorized() { +fn test_timelocked_proposal_veto_unauthorized() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -878,11 +885,14 @@ fn test_timelocked_proposal_veto_unauthorized() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); @@ -902,14 +912,19 @@ fn test_timelocked_proposal_veto_unauthorized() { assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); + + Ok(()) } // vetoer can only veto the proposal before the timelock expires #[test] -fn test_timelocked_proposal_veto_expired_timelock() { +fn test_timelocked_proposal_veto_expired_timelock() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -970,14 +985,17 @@ fn test_timelocked_proposal_veto_expired_timelock() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); - app.update_block(|b| b.time = b.time.plus_seconds(200)); + app.update_block(|b| b.time = b.time.plus_seconds(604800 + 200)); let err: ContractError = app .execute_contract( @@ -991,11 +1009,13 @@ fn test_timelocked_proposal_veto_expired_timelock() { .unwrap(); assert_eq!(err, ContractError::VetoError(VetoError::TimelockExpired {}),); + + Ok(()) } // vetoer can only exec timelocked prop if the early exec flag is enabled #[test] -fn test_timelocked_proposal_execute_no_early_exec() { +fn test_timelocked_proposal_execute_no_early_exec() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -1050,11 +1070,14 @@ fn test_timelocked_proposal_execute_no_early_exec() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); @@ -1070,10 +1093,12 @@ fn test_timelocked_proposal_execute_no_early_exec() { .unwrap(); assert_eq!(err, ContractError::VetoError(VetoError::NoEarlyExecute {}),); + + Ok(()) } #[test] -fn test_timelocked_proposal_execute_early() { +fn test_timelocked_proposal_execute_early() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -1128,11 +1153,14 @@ fn test_timelocked_proposal_execute_early() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); @@ -1153,11 +1181,13 @@ fn test_timelocked_proposal_execute_early() { let proposal = query_proposal(&app, &proposal_module, proposal_id); assert_eq!(proposal.proposal.status, Status::Executed {}); + + Ok(()) } // only vetoer can exec timelocked prop early #[test] -fn test_timelocked_proposal_execute_active_timelock_unauthorized() { +fn test_timelocked_proposal_execute_active_timelock_unauthorized() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -1212,11 +1242,14 @@ fn test_timelocked_proposal_execute_active_timelock_unauthorized() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); @@ -1238,11 +1271,13 @@ fn test_timelocked_proposal_execute_active_timelock_unauthorized() { .unwrap(); assert_eq!(err, ContractError::VetoError(VetoError::Timelocked {}),); + + Ok(()) } // anyone can exec the prop after the timelock expires #[test] -fn test_timelocked_proposal_execute_expired_timelock_not_vetoer() { +fn test_timelocked_proposal_execute_expired_timelock_not_vetoer() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -1297,14 +1332,17 @@ fn test_timelocked_proposal_execute_expired_timelock_not_vetoer() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay - let expiration = veto_config.timelock_duration.after(&app.block_info()); + // Proposal is timelocked to the moment of prop expiring + timelock delay + let expiration = proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?; assert_eq!( proposal.proposal.status, Status::VetoTimelock { expiration } ); - app.update_block(|b| b.time = b.time.plus_seconds(201)); + app.update_block(|b| b.time = b.time.plus_seconds(604800 + 201)); // assert timelock is expired assert!(expiration.is_expired(&app.block_info())); mint_natives(&mut app, core_addr.as_str(), coins(10, "ujuno")); @@ -1319,10 +1357,12 @@ fn test_timelocked_proposal_execute_expired_timelock_not_vetoer() { let proposal = query_proposal(&app, &proposal_module, proposal_id); assert_eq!(proposal.proposal.status, Status::Executed {},); + + Ok(()) } #[test] -fn test_proposal_message_timelock_veto() { +fn test_proposal_message_timelock_veto() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -1398,11 +1438,14 @@ fn test_proposal_message_timelock_veto() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); @@ -1432,10 +1475,12 @@ fn test_proposal_message_timelock_veto() { let proposal = query_proposal(&app, &proposal_module, proposal_id); assert_eq!(proposal.proposal.status, Status::Vetoed); + + Ok(()) } #[test] -fn test_proposal_message_timelock_early_execution() { +fn test_proposal_message_timelock_early_execution() -> anyhow::Result<()> { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; @@ -1500,11 +1545,14 @@ fn test_proposal_message_timelock_early_execution() { ); let proposal = query_proposal(&app, &proposal_module, proposal_id); - // Proposal is timelocked to the moment of prop passing + timelock delay + // Proposal is timelocked to the moment of prop expiring + timelock delay assert_eq!( proposal.proposal.status, Status::VetoTimelock { - expiration: veto_config.timelock_duration.after(&app.block_info()), + expiration: proposal + .proposal + .expiration + .add(veto_config.timelock_duration)?, } ); @@ -1514,6 +1562,8 @@ fn test_proposal_message_timelock_early_execution() { execute_proposal(&mut app, &proposal_module, "oversight", proposal_id); let proposal = query_proposal(&app, &proposal_module, proposal_id); assert_eq!(proposal.proposal.status, Status::Executed); + + Ok(()) } #[test] @@ -1794,7 +1844,7 @@ fn test_update_config() { contract_addr: proposal_module.to_string(), msg: to_json_binary(&ExecuteMsg::UpdateConfig { veto: Some(VetoConfig { - timelock_duration: Duration::Time(100), + timelock_duration: Duration::Height(2), vetoer: CREATOR_ADDR.to_string(), early_execute: false, veto_before_passed: false, @@ -1828,7 +1878,7 @@ fn test_update_config() { config, Config { veto: Some(VetoConfig { - timelock_duration: Duration::Time(100), + timelock_duration: Duration::Height(2), vetoer: CREATOR_ADDR.to_string(), early_execute: false, veto_before_passed: false, @@ -1849,7 +1899,7 @@ fn test_update_config() { let err: ContractError = app .execute_contract( Addr::unchecked(CREATOR_ADDR), - proposal_module, + proposal_module.clone(), &&ExecuteMsg::UpdateConfig { veto: None, threshold: Threshold::AbsoluteCount { @@ -1867,7 +1917,39 @@ fn test_update_config() { .unwrap_err() .downcast() .unwrap(); - assert!(matches!(err, ContractError::Unauthorized {})) + assert!(matches!(err, ContractError::Unauthorized {})); + + // Check that veto config is validated (mismatching duration units). + let err: ContractError = app + .execute_contract( + Addr::unchecked(core_addr.clone()), + proposal_module, + &&ExecuteMsg::UpdateConfig { + veto: Some(VetoConfig { + timelock_duration: Duration::Time(100), + vetoer: CREATOR_ADDR.to_string(), + early_execute: false, + veto_before_passed: false, + }), + threshold: Threshold::AbsoluteCount { + threshold: Uint128::new(10_000), + }, + max_voting_period: Duration::Height(6), + min_voting_period: None, + only_members_execute: true, + allow_revoting: false, + dao: core_addr.to_string(), + close_proposal_on_execution_failure: false, + }, + &[], + ) + .unwrap_err() + .downcast() + .unwrap(); + assert!(matches!( + err, + ContractError::VetoError(VetoError::TimelockDurationUnitMismatch {}) + )) } #[test] diff --git a/packages/dao-voting/src/veto.rs b/packages/dao-voting/src/veto.rs index ad22d4525..fe5b77946 100644 --- a/packages/dao-voting/src/veto.rs +++ b/packages/dao-voting/src/veto.rs @@ -1,5 +1,5 @@ use cosmwasm_schema::cw_serde; -use cosmwasm_std::{Deps, MessageInfo, StdError, StdResult}; +use cosmwasm_std::{Deps, MessageInfo, StdError}; use cw_utils::Duration; use thiserror::Error; @@ -26,6 +26,9 @@ pub enum VetoError { #[error("The veto timelock duration has expired.")] TimelockExpired {}, + #[error("The veto timelock duration must have the same units as the max_voting_period of the proposal (height or time).")] + TimelockDurationUnitMismatch {}, + #[error("Only vetoer can veto a proposal.")] Unauthorized {}, } @@ -45,10 +48,17 @@ pub struct VetoConfig { } impl VetoConfig { - pub fn validate(&self, deps: &Deps) -> StdResult<()> { + pub fn validate(&self, deps: &Deps, max_voting_period: &Duration) -> Result<(), VetoError> { // Validate vetoer address. deps.api.addr_validate(&self.vetoer)?; + // Validate duration units match voting period. + match (self.timelock_duration, max_voting_period) { + (Duration::Time(_), Duration::Time(_)) => (), + (Duration::Height(_), Duration::Height(_)) => (), + _ => return Err(VetoError::TimelockDurationUnitMismatch {}), + }; + Ok(()) }