Added validation that timelock_duration is the same units as the max_…

…voting_period, and fixed tests. Also fixed missing timelock status handler.

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 }

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(

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),
         }
     }

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(())
 }

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 }

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<Response, Co
                 return Err(ContractError::AlreadyMigrated {});
             }
 
+            let current_config = v1::state::CONFIG.load(deps.storage)?;
+            let max_voting_period = v1_duration_to_v2(current_config.max_voting_period);
+
             // 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)?;
             };
 
             // Update the stored config to have the new
             // `close_proposal_on_execution_failure` field.
-            let current_config = v1::state::CONFIG.load(deps.storage)?;
             CONFIG.save(
                 deps.storage,
                 &Config {
                     threshold: v1_threshold_to_v2(current_config.threshold),
-                    max_voting_period: v1_duration_to_v2(current_config.max_voting_period),
+                    max_voting_period,
                     min_voting_period: current_config.min_voting_period.map(v1_duration_to_v2),
                     only_members_execute: current_config.only_members_execute,
                     allow_revoting: current_config.allow_revoting,

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

@@ -93,6 +93,14 @@ impl SingleChoiceProposal {
             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),
         }
     }

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,