From 56d007dbe44633594fdae9b9912d25bec924c83a Mon Sep 17 00:00:00 2001 From: theghostmac Date: Sat, 6 Apr 2024 21:47:51 +0100 Subject: [PATCH 1/6] Add active threshold functionality to CW4 voting module, closes #781 --- Cargo.lock | 1 + contracts/voting/dao-voting-cw4/Cargo.toml | 1 + .../voting/dao-voting-cw4/src/contract.rs | 139 ++++++++++++++++-- contracts/voting/dao-voting-cw4/src/error.rs | 4 + contracts/voting/dao-voting-cw4/src/msg.rs | 14 +- contracts/voting/dao-voting-cw4/src/state.rs | 14 ++ 6 files changed, 160 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6742e170e..539833257 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2072,6 +2072,7 @@ dependencies = [ "cw4-group 1.1.2", "dao-dao-macros", "dao-interface", + "dao-voting 2.4.2", "thiserror", ] diff --git a/contracts/voting/dao-voting-cw4/Cargo.toml b/contracts/voting/dao-voting-cw4/Cargo.toml index 55c671560..52d81213a 100644 --- a/contracts/voting/dao-voting-cw4/Cargo.toml +++ b/contracts/voting/dao-voting-cw4/Cargo.toml @@ -27,6 +27,7 @@ dao-dao-macros = { workspace = true } dao-interface = { workspace = true } cw4 = { workspace = true } cw4-group = { workspace = true } +dao-voting = { workspace = true } [dev-dependencies] cw-multi-test = { workspace = true } diff --git a/contracts/voting/dao-voting-cw4/src/contract.rs b/contracts/voting/dao-voting-cw4/src/contract.rs index 48307676f..28f716b10 100644 --- a/contracts/voting/dao-voting-cw4/src/contract.rs +++ b/contracts/voting/dao-voting-cw4/src/contract.rs @@ -1,16 +1,24 @@ -#[cfg(not(feature = "library"))] -use cosmwasm_std::entry_point; +use std::ops::{Div, Mul}; + use cosmwasm_std::{ - to_json_binary, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg, - Uint128, WasmMsg, + Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg, + to_json_binary, Uint128, Uint256, WasmMsg, }; -use cw2::{get_contract_version, set_contract_version, ContractVersion}; +#[cfg(not(feature = "library"))] +use cosmwasm_std::entry_point; +use cw2::{ContractVersion, get_contract_version, set_contract_version}; use cw4::{MemberListResponse, MemberResponse, TotalWeightResponse}; use cw_utils::parse_reply_instantiate_data; +use dao_interface::voting::IsActiveResponse; +use dao_voting::threshold::{ + ActiveThreshold, assert_valid_percentage_threshold + , +}; + use crate::error::ContractError; use crate::msg::{ExecuteMsg, GroupContract, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{DAO, GROUP_CONTRACT}; +use crate::state::{ACTIVE_THRESHOLD, Config, CONFIG, DAO, GROUP_CONTRACT}; pub(crate) const CONTRACT_NAME: &str = "crates.io:dao-voting-cw4"; pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -26,6 +34,20 @@ pub fn instantiate( ) -> Result { set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; + let config: Config = if let Some(active_threshold) = msg.active_threshold { + Config { + active_threshold_enabled: true, + active_threshold: Some(active_threshold), + } + } else { + Config { + active_threshold_enabled: false, + active_threshold: None, + } + }; + + CONFIG.save(deps.storage, &config)?; + DAO.save(deps.storage, &info.sender)?; match msg.group_contract { @@ -108,12 +130,70 @@ pub fn instantiate( #[cfg_attr(not(feature = "library"), entry_point)] pub fn execute( - _deps: DepsMut, + deps: DepsMut, + env: Env, + info: MessageInfo, + msg: ExecuteMsg, +) -> Result { + match msg { + ExecuteMsg::UpdateActiveThreshold { new_threshold } => { + execute_update_active_threshold(deps, env, info, new_threshold) + } + } +} + +pub fn execute_update_active_threshold( + deps: DepsMut, _env: Env, - _info: MessageInfo, - _msg: ExecuteMsg, + info: MessageInfo, + new_active_threshold: Option, ) -> Result { - Err(ContractError::NoExecute {}) + let dao = DAO.load(deps.storage)?; + if info.sender != dao { + return Err(ContractError::Unauthorized {}); + } + + if let Some(active_threshold) = &new_active_threshold { + // Pre-validation before state changes + match active_threshold { + ActiveThreshold::Percentage { percent } => { + assert_valid_percentage_threshold(*percent)?; + } + ActiveThreshold::AbsoluteCount { count } => { + if *count.is_zero() { + return Err(ContractError::InvalidThreshold {}); + } + } + } + } + + // Safe to modify state after validation + match new_active_threshold { + Some(threshold) => ACTIVE_THRESHOLD.save(deps.storage, &threshold)?, + None => ACTIVE_THRESHOLD.remove(deps.storage), + } + + // As opposed to doing it this way: + // if let Some(active_threshold) = new_active_threshold { + // // Pre-validation before state changes. + // match active_threshold { + // ActiveThreshold::Percentage { percent } => { + // assert_valid_percentage_threshold(percent)?; + // } + // ActiveThreshold::AbsoluteCount { count } => { + // if count.is_zero() { + // return Err(ContractError::InvalidThreshold {}); + // } + // } + // } + // ACTIVE_THRESHOLD.save(deps.storage, &active_threshold)?; + // } else { + // ACTIVE_THRESHOLD.remove(deps.storage); + // } + + Ok(Response::new() + .add_attribute("method", "update_active_threshold") + .add_attribute("status", "success")) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -126,6 +206,7 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult { QueryMsg::Info {} => query_info(deps), QueryMsg::GroupContract {} => to_json_binary(&GROUP_CONTRACT.load(deps.storage)?), QueryMsg::Dao {} => to_json_binary(&DAO.load(deps.storage)?), + QueryMsg::IsActive {} => query_is_active(deps), } } @@ -164,12 +245,48 @@ pub fn query_total_power_at_height(deps: Deps, env: Env, height: Option) -> } pub fn query_info(deps: Deps) -> StdResult { - let info = cw2::get_contract_version(deps.storage)?; + let info = get_contract_version(deps.storage)?; to_json_binary(&dao_interface::voting::InfoResponse { info }) } +pub fn query_is_active(deps: Deps) -> StdResult { + let active_threshold = ACTIVE_THRESHOLD.may_load(deps.storage)?; + + match active_threshold { + Some(ActiveThreshold::AbsoluteCount { count }) => { + let group_contract = GROUP_CONTRACT.load(deps.storage)?; + let total_weight: TotalWeightResponse = deps.querier.query_wasm_smart( + &group_contract, + &cw4_group::msg::QueryMsg::TotalWeight { at_height: None }, + )?; + to_json_binary(&IsActiveResponse { + active: total_weight.weight >= count.into(), + }) + } + Some(ActiveThreshold::Percentage { percent, .. }) => { + let group_contract = GROUP_CONTRACT.load(deps.storage)?; + let total_weight: TotalWeightResponse = deps.querier.query_wasm_smart( + &group_contract, + &cw4_group::msg::QueryMsg::TotalWeight { at_height: None }, + )?; + let required_weight: Uint256 = Uint256::from(total_weight.weight).mul(Uint256::from(percent)).div(Uint256::from(100)); + + to_json_binary(&IsActiveResponse { + active: total_weight.weight >= required_weight.into(), + }) + } + None => { + to_json_binary(&IsActiveResponse { active: true }) + } + } +} + #[cfg_attr(not(feature = "library"), entry_point)] pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { + let config: Config = CONFIG.load(deps.storage)?; + // Update config as necessary + CONFIG.save(deps.storage, &config)?; + let storage_version: ContractVersion = get_contract_version(deps.storage)?; // Only migrate if newer diff --git a/contracts/voting/dao-voting-cw4/src/error.rs b/contracts/voting/dao-voting-cw4/src/error.rs index a0ce03e9c..03951acca 100644 --- a/contracts/voting/dao-voting-cw4/src/error.rs +++ b/contracts/voting/dao-voting-cw4/src/error.rs @@ -29,4 +29,8 @@ pub enum ContractError { #[error("Total weight of the CW4 contract cannot be zero")] ZeroTotalWeight {}, + + #[error("The provided threshold is invalid: thresholds must be positive and within designated limits (e.g., percentages between 0 and 100)")] + InvalidThreshold {}, + } diff --git a/contracts/voting/dao-voting-cw4/src/msg.rs b/contracts/voting/dao-voting-cw4/src/msg.rs index 24bd0eebc..271f9eb03 100644 --- a/contracts/voting/dao-voting-cw4/src/msg.rs +++ b/contracts/voting/dao-voting-cw4/src/msg.rs @@ -1,5 +1,6 @@ use cosmwasm_schema::{cw_serde, QueryResponses}; -use dao_dao_macros::voting_module_query; +use dao_dao_macros::{active_query, voting_module_query}; +use dao_voting::threshold::ActiveThreshold; #[cw_serde] pub enum GroupContract { @@ -15,11 +16,20 @@ pub enum GroupContract { #[cw_serde] pub struct InstantiateMsg { pub group_contract: GroupContract, + pub active_threshold: Option, } #[cw_serde] -pub enum ExecuteMsg {} +pub enum ExecuteMsg { + /// Sets the active threshold to a new value. Only the + /// instantiator of this contract (a DAO most likely) may call this + /// method. + UpdateActiveThreshold { + new_threshold: Option, + }, +} +#[active_query] #[voting_module_query] #[cw_serde] #[derive(QueryResponses)] diff --git a/contracts/voting/dao-voting-cw4/src/state.rs b/contracts/voting/dao-voting-cw4/src/state.rs index bdc0d8004..59a61c8c6 100644 --- a/contracts/voting/dao-voting-cw4/src/state.rs +++ b/contracts/voting/dao-voting-cw4/src/state.rs @@ -1,5 +1,19 @@ +use cosmwasm_schema::cw_serde; use cosmwasm_std::Addr; use cw_storage_plus::Item; +use cw_utils::Duration; +use dao_voting::threshold::ActiveThreshold; pub const GROUP_CONTRACT: Item = Item::new("group_contract"); pub const DAO: Item = Item::new("dao_address"); + +/// The minimum amount of users for the DAO to be active +pub const ACTIVE_THRESHOLD: Item = Item::new("active_threshold"); + +#[cw_serde] +pub struct Config { + pub active_threshold_enabled: bool, + pub active_threshold: Option, +} + +pub const CONFIG: Item = Item::new("config"); \ No newline at end of file From 6eacf9bec9f7eddc6555100826454784517d63a6 Mon Sep 17 00:00:00 2001 From: theghostmac Date: Sun, 7 Apr 2024 17:23:32 +0100 Subject: [PATCH 2/6] Wrote 5 tests for active threshold functionality in the CW4 voting module, closes #781 --- .../src/tests.rs | 3 + .../dao-pre-propose-approver/src/tests.rs | 2 + .../dao-pre-propose-multiple/src/tests.rs | 3 + .../dao-pre-propose-single/src/tests.rs | 3 + .../src/testing/suite.rs | 5 +- .../src/testing/instantiate.rs | 2 + .../src/testing/adversarial_tests.rs | 4 +- .../src/testing/do_votes.rs | 9 +- .../src/testing/instantiate.rs | 5 + .../dao-proposal-single/src/testing/tests.rs | 65 ++- .../voting/dao-voting-cw4/src/contract.rs | 74 ++-- contracts/voting/dao-voting-cw4/src/error.rs | 1 - contracts/voting/dao-voting-cw4/src/state.rs | 4 +- contracts/voting/dao-voting-cw4/src/tests.rs | 370 +++++++++++++++++- .../src/testing/mod.rs | 2 +- packages/dao-testing/src/helpers.rs | 3 + 16 files changed, 477 insertions(+), 78 deletions(-) diff --git a/contracts/pre-propose/dao-pre-propose-approval-single/src/tests.rs b/contracts/pre-propose/dao-pre-propose-approval-single/src/tests.rs index 99b8c0369..92240f9a1 100644 --- a/contracts/pre-propose/dao-pre-propose-approval-single/src/tests.rs +++ b/contracts/pre-propose/dao-pre-propose-approval-single/src/tests.rs @@ -135,6 +135,7 @@ fn setup_default_test( amount: Uint128::new(8), }, ]), + None, ); let proposal_modules: Vec = app .wrap() @@ -1385,6 +1386,7 @@ fn test_instantiate_with_zero_native_deposit() { amount: Uint128::new(8), }, ]), + None, ); } @@ -1450,6 +1452,7 @@ fn test_instantiate_with_zero_cw20_deposit() { amount: Uint128::new(8), }, ]), + None, ); } diff --git a/contracts/pre-propose/dao-pre-propose-approver/src/tests.rs b/contracts/pre-propose/dao-pre-propose-approver/src/tests.rs index fbef73e87..c47e4d550 100644 --- a/contracts/pre-propose/dao-pre-propose-approver/src/tests.rs +++ b/contracts/pre-propose/dao-pre-propose-approver/src/tests.rs @@ -201,6 +201,7 @@ fn setup_default_test( amount: Uint128::new(8), }, ]), + None, ); let proposal_modules: Vec = app .wrap() @@ -255,6 +256,7 @@ fn setup_default_test( amount: Uint128::new(8), }, ]), + None, ); let proposal_modules: Vec = app .wrap() diff --git a/contracts/pre-propose/dao-pre-propose-multiple/src/tests.rs b/contracts/pre-propose/dao-pre-propose-multiple/src/tests.rs index 56f310ee2..08d845179 100644 --- a/contracts/pre-propose/dao-pre-propose-multiple/src/tests.rs +++ b/contracts/pre-propose/dao-pre-propose-multiple/src/tests.rs @@ -134,6 +134,7 @@ fn setup_default_test( amount: Uint128::new(8), }, ]), + None, ); let proposal_modules: Vec = app .wrap() @@ -1088,6 +1089,7 @@ fn test_instantiate_with_zero_native_deposit() { amount: Uint128::new(8), }, ]), + None, ); } @@ -1151,6 +1153,7 @@ fn test_instantiate_with_zero_cw20_deposit() { amount: Uint128::new(8), }, ]), + None, ); } diff --git a/contracts/pre-propose/dao-pre-propose-single/src/tests.rs b/contracts/pre-propose/dao-pre-propose-single/src/tests.rs index 0475d13b6..827caba22 100644 --- a/contracts/pre-propose/dao-pre-propose-single/src/tests.rs +++ b/contracts/pre-propose/dao-pre-propose-single/src/tests.rs @@ -132,6 +132,7 @@ fn setup_default_test( amount: Uint128::new(8), }, ]), + None, ); let proposal_modules: Vec = app .wrap() @@ -1024,6 +1025,7 @@ fn test_instantiate_with_zero_native_deposit() { amount: Uint128::new(8), }, ]), + None, ); } @@ -1087,6 +1089,7 @@ fn test_instantiate_with_zero_cw20_deposit() { amount: Uint128::new(8), }, ]), + None, ); } diff --git a/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs b/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs index 79d18154f..58d55833a 100644 --- a/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs +++ b/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs @@ -8,7 +8,7 @@ use dao_interface::{ use dao_testing::contracts::{ cw4_group_contract, dao_dao_contract, dao_voting_cw4_contract, proposal_condorcet_contract, }; -use dao_voting::threshold::PercentageThreshold; +use dao_voting::threshold::{ActiveThreshold, PercentageThreshold}; use dao_voting_cw4::msg::GroupContract; use crate::{ @@ -30,6 +30,7 @@ pub(crate) struct SuiteBuilder { pub instantiate: InstantiateMsg, with_proposal: Option, with_voters: Vec<(String, u64)>, + active_threshold: Option, } impl Default for SuiteBuilder { @@ -43,6 +44,7 @@ impl Default for SuiteBuilder { }, with_proposal: None, with_voters: vec![("sender".to_string(), 10)], + active_threshold: None, } } } @@ -93,6 +95,7 @@ impl SuiteBuilder { cw4_group_code_id: cw4_id, initial_members, }, + active_threshold: self.active_threshold.clone(), }) .unwrap(), admin: Some(Admin::CoreModule {}), diff --git a/contracts/proposal/dao-proposal-multiple/src/testing/instantiate.rs b/contracts/proposal/dao-proposal-multiple/src/testing/instantiate.rs index 8fd7e0c95..f9c877cef 100644 --- a/contracts/proposal/dao-proposal-multiple/src/testing/instantiate.rs +++ b/contracts/proposal/dao-proposal-multiple/src/testing/instantiate.rs @@ -766,6 +766,7 @@ pub fn _instantiate_with_cw4_groups_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_weights: Option>, + active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_multiple_contract()); let cw4_id = app.store_code(cw4_group_contract()); @@ -813,6 +814,7 @@ pub fn _instantiate_with_cw4_groups_governance( cw4_group_code_id: cw4_id, initial_members: initial_weights, }, + active_threshold, }) .unwrap(), admin: Some(Admin::CoreModule {}), 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 beed03604..f8c1477ad 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs @@ -33,7 +33,7 @@ struct CommonTest { fn setup_test(messages: Vec) -> CommonTest { let mut app = App::default(); let instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -199,6 +199,7 @@ pub fn test_executed_prop_state_remains_after_vote_swing() { amount: Uint128::new(30), }, ]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -298,6 +299,7 @@ pub fn test_passed_prop_state_remains_after_vote_swing() { amount: Uint128::new(30), }, ]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); diff --git a/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs b/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs index 3646a9ecf..8db738cd9 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs @@ -94,7 +94,12 @@ fn do_test_votes( setup_governance: F, ) -> (App, Addr) where - F: Fn(&mut App, InstantiateMsg, Option>) -> Addr, + F: Fn( + &mut App, + InstantiateMsg, + Option>, + Option, + ) -> Addr, { let mut app = App::default(); @@ -144,7 +149,7 @@ where pre_propose_info, }; - let core_addr = setup_governance(&mut app, instantiate, Some(initial_balances)); + let core_addr = setup_governance(&mut app, instantiate, Some(initial_balances), None); let governance_modules: Vec = app .wrap() diff --git a/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs b/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs index 020154700..75d795e25 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs @@ -96,6 +96,7 @@ pub(crate) fn instantiate_with_staked_cw721_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_balances: Option>, + active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); @@ -230,6 +231,7 @@ pub(crate) fn instantiate_with_native_staked_balances_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_balances: Option>, + active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); @@ -341,6 +343,7 @@ pub(crate) fn instantiate_with_staked_balances_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_balances: Option>, + active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); @@ -542,6 +545,7 @@ pub(crate) fn instantiate_with_cw4_groups_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_weights: Option>, + active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); let cw4_id = app.store_code(cw4_group_contract()); @@ -590,6 +594,7 @@ pub(crate) fn instantiate_with_cw4_groups_governance( cw4_group_code_id: cw4_id, initial_members: initial_weights, }, + active_threshold, }) .unwrap(), admin: Some(Admin::CoreModule {}), diff --git a/contracts/proposal/dao-proposal-single/src/testing/tests.rs b/contracts/proposal/dao-proposal-single/src/testing/tests.rs index 4246e5a09..b58a239dc 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/tests.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/tests.rs @@ -82,7 +82,7 @@ struct CommonTest { fn setup_test(messages: Vec) -> CommonTest { let mut app = App::default(); let instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -155,7 +155,7 @@ fn test_simple_propose_staked_balances() { fn test_simple_proposal_cw4_voting() { let mut app = App::default(); let instantiate = get_default_non_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let id = make_proposal(&mut app, &proposal_module, CREATOR_ADDR, vec![], None); @@ -198,7 +198,7 @@ fn test_simple_proposal_cw4_voting() { fn test_simple_proposal_auto_vote_yes() { let mut app = App::default(); let instantiate = get_default_non_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let id = make_proposal( &mut app, @@ -247,7 +247,7 @@ fn test_simple_proposal_auto_vote_yes() { fn test_simple_proposal_auto_vote_no() { let mut app = App::default(); let instantiate = get_default_non_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let id = make_proposal( &mut app, @@ -336,7 +336,7 @@ fn test_voting_module_token_instantiate() { fn test_deposit_token_voting_module_token_fails_if_no_voting_module_token() { let mut app = App::default(); let instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); - instantiate_with_cw4_groups_governance(&mut app, instantiate, None); + instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); } #[test] @@ -358,7 +358,7 @@ fn test_instantiate_with_non_voting_module_cw20_deposit() { false, ); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let proposal_id = make_proposal(&mut app, &proposal_module, CREATOR_ADDR, vec![], None); @@ -409,7 +409,7 @@ fn test_proposal_message_execution() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -505,6 +505,7 @@ fn test_proposal_message_timelock_execution() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -621,6 +622,7 @@ fn test_open_proposal_veto_unauthorized() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -684,6 +686,7 @@ fn test_open_proposal_veto_with_early_veto_flag_disabled() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -742,6 +745,7 @@ fn test_open_proposal_veto_with_no_timelock() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -808,6 +812,7 @@ fn test_vetoed_proposal_veto() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -886,6 +891,7 @@ fn test_open_proposal_veto_early() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -953,6 +959,7 @@ fn test_timelocked_proposal_veto_unauthorized() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1054,7 +1061,9 @@ fn test_timelocked_proposal_veto_expired_timelock() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1140,7 +1149,9 @@ fn test_timelocked_proposal_execute_no_early_exec() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1224,7 +1235,9 @@ fn test_timelocked_proposal_execute_early() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1314,7 +1327,9 @@ fn test_timelocked_proposal_execute_active_timelock_unauthorized() -> anyhow::Re address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1405,7 +1420,9 @@ fn test_timelocked_proposal_execute_expired_timelock_not_vetoer() -> anyhow::Res address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1491,7 +1508,9 @@ fn test_proposal_message_timelock_veto() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1616,7 +1635,9 @@ fn test_proposal_message_timelock_early_execution() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1703,7 +1724,9 @@ fn test_proposal_message_timelock_veto_before_passed() { amount: Uint128::new(85), }, ]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1776,7 +1799,9 @@ fn test_veto_only_members_execute_proposal() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1901,7 +1926,9 @@ fn test_proposal_cant_close_after_expiry_is_passed() { amount: Uint128::new(85), }, ]), + None, ); + let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -2173,7 +2200,7 @@ fn test_anyone_may_propose_and_proposal_listing() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.pre_propose_info = PreProposeInfo::AnyoneMayPropose {}; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); for addr in 'm'..'z' { @@ -2256,7 +2283,7 @@ fn test_propose_non_member_auto_vote_fails() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.pre_propose_info = PreProposeInfo::AnyoneMayPropose {}; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); // Should fail if non-member tries to vote on proposal creation. @@ -2568,7 +2595,7 @@ fn test_min_duration_unit_missmatch() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.min_voting_period = Some(Duration::Height(10)); - instantiate_with_staked_balances_governance(&mut app, instantiate, None); + instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); } #[test] @@ -2577,7 +2604,7 @@ fn test_min_duration_larger_than_proposal_duration() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.min_voting_period = Some(Duration::Time(604801)); - instantiate_with_staked_balances_governance(&mut app, instantiate, None); + instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); } #[test] @@ -2586,7 +2613,7 @@ fn test_min_voting_period_no_early_pass() { let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.min_voting_period = Some(Duration::Height(10)); instantiate.max_voting_period = Duration::Height(100); - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2628,7 +2655,9 @@ fn test_min_duration_same_as_proposal_duration() { amount: Uint128::new(90), }, ]), + None, ); + let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2650,7 +2679,7 @@ fn test_revoting_playthrough() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.allow_revoting = true; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2723,7 +2752,7 @@ fn test_allow_revoting_config_changes() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.allow_revoting = true; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2828,6 +2857,7 @@ fn test_three_of_five_multisig() { amount: Uint128::new(1), }, ]), + None, ); let core_state: dao_interface::query::DumpStateResponse = app @@ -2910,6 +2940,7 @@ fn test_three_of_five_multisig_revoting() { amount: Uint128::new(1), }, ]), + None, ); let core_state: dao_interface::query::DumpStateResponse = app @@ -3064,6 +3095,7 @@ fn test_proposal_count_initialized_to_zero() { amount: Uint128::new(90), }, ]), + None, ); let core_state: dao_interface::query::DumpStateResponse = app @@ -3554,7 +3586,7 @@ fn test_proposal_too_large() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.pre_propose_info = PreProposeInfo::AnyoneMayPropose {}; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let err = app @@ -3839,6 +3871,7 @@ fn test_query_list_votes() { amount: Uint128::new(1), }, ]), + None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let proposal_id = make_proposal(&mut app, &proposal_module, "one", vec![], None); @@ -4141,7 +4174,7 @@ fn test_rational_clobbered_on_revote() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.allow_revoting = true; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); diff --git a/contracts/voting/dao-voting-cw4/src/contract.rs b/contracts/voting/dao-voting-cw4/src/contract.rs index 28f716b10..0cb897ff3 100644 --- a/contracts/voting/dao-voting-cw4/src/contract.rs +++ b/contracts/voting/dao-voting-cw4/src/contract.rs @@ -1,24 +1,19 @@ -use std::ops::{Div, Mul}; - -use cosmwasm_std::{ - Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg, - to_json_binary, Uint128, Uint256, WasmMsg, -}; #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; -use cw2::{ContractVersion, get_contract_version, set_contract_version}; +use cosmwasm_std::{ + to_json_binary, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg, + Uint128, Uint256, WasmMsg, +}; +use cw2::{get_contract_version, set_contract_version, ContractVersion}; use cw4::{MemberListResponse, MemberResponse, TotalWeightResponse}; use cw_utils::parse_reply_instantiate_data; use dao_interface::voting::IsActiveResponse; -use dao_voting::threshold::{ - ActiveThreshold, assert_valid_percentage_threshold - , -}; +use dao_voting::threshold::ActiveThreshold; use crate::error::ContractError; use crate::msg::{ExecuteMsg, GroupContract, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{ACTIVE_THRESHOLD, Config, CONFIG, DAO, GROUP_CONTRACT}; +use crate::state::{Config, ACTIVE_THRESHOLD, CONFIG, DAO, GROUP_CONTRACT}; pub(crate) const CONTRACT_NAME: &str = "crates.io:dao-voting-cw4"; pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -36,12 +31,10 @@ pub fn instantiate( let config: Config = if let Some(active_threshold) = msg.active_threshold { Config { - active_threshold_enabled: true, active_threshold: Some(active_threshold), } } else { Config { - active_threshold_enabled: false, active_threshold: None, } }; @@ -153,44 +146,23 @@ pub fn execute_update_active_threshold( return Err(ContractError::Unauthorized {}); } - if let Some(active_threshold) = &new_active_threshold { - // Pre-validation before state changes + if let Some(active_threshold) = new_active_threshold { match active_threshold { - ActiveThreshold::Percentage { percent } => { - assert_valid_percentage_threshold(*percent)?; - } ActiveThreshold::AbsoluteCount { count } => { - if *count.is_zero() { + if count.is_zero() { return Err(ContractError::InvalidThreshold {}); } + ACTIVE_THRESHOLD.save(deps.storage, &active_threshold)?; + } + // Reject percentage-based thresholds + ActiveThreshold::Percentage { .. } => { + return Err(ContractError::InvalidThreshold {}); } } + } else { + ACTIVE_THRESHOLD.remove(deps.storage); } - // Safe to modify state after validation - match new_active_threshold { - Some(threshold) => ACTIVE_THRESHOLD.save(deps.storage, &threshold)?, - None => ACTIVE_THRESHOLD.remove(deps.storage), - } - - // As opposed to doing it this way: - // if let Some(active_threshold) = new_active_threshold { - // // Pre-validation before state changes. - // match active_threshold { - // ActiveThreshold::Percentage { percent } => { - // assert_valid_percentage_threshold(percent)?; - // } - // ActiveThreshold::AbsoluteCount { count } => { - // if count.is_zero() { - // return Err(ContractError::InvalidThreshold {}); - // } - // } - // } - // ACTIVE_THRESHOLD.save(deps.storage, &active_threshold)?; - // } else { - // ACTIVE_THRESHOLD.remove(deps.storage); - // } - Ok(Response::new() .add_attribute("method", "update_active_threshold") .add_attribute("status", "success")) @@ -260,24 +232,24 @@ pub fn query_is_active(deps: Deps) -> StdResult { &cw4_group::msg::QueryMsg::TotalWeight { at_height: None }, )?; to_json_binary(&IsActiveResponse { - active: total_weight.weight >= count.into(), + active: total_weight.weight >= count.u128() as u64, }) } - Some(ActiveThreshold::Percentage { percent, .. }) => { + Some(ActiveThreshold::Percentage { percent }) => { let group_contract = GROUP_CONTRACT.load(deps.storage)?; let total_weight: TotalWeightResponse = deps.querier.query_wasm_smart( &group_contract, &cw4_group::msg::QueryMsg::TotalWeight { at_height: None }, )?; - let required_weight: Uint256 = Uint256::from(total_weight.weight).mul(Uint256::from(percent)).div(Uint256::from(100)); + let percentage_base = Uint256::from(10u64).pow(percent.decimal_places()); // Ensure correct power base for scaling + let required_weight = Uint256::from(total_weight.weight) + .multiply_ratio(percent.atomics(), percentage_base); // Correct ratio calculation to_json_binary(&IsActiveResponse { - active: total_weight.weight >= required_weight.into(), + active: Uint256::from(total_weight.weight) >= required_weight, }) } - None => { - to_json_binary(&IsActiveResponse { active: true }) - } + None => to_json_binary(&IsActiveResponse { active: true }), } } diff --git a/contracts/voting/dao-voting-cw4/src/error.rs b/contracts/voting/dao-voting-cw4/src/error.rs index 03951acca..d7846fe98 100644 --- a/contracts/voting/dao-voting-cw4/src/error.rs +++ b/contracts/voting/dao-voting-cw4/src/error.rs @@ -32,5 +32,4 @@ pub enum ContractError { #[error("The provided threshold is invalid: thresholds must be positive and within designated limits (e.g., percentages between 0 and 100)")] InvalidThreshold {}, - } diff --git a/contracts/voting/dao-voting-cw4/src/state.rs b/contracts/voting/dao-voting-cw4/src/state.rs index 59a61c8c6..3427a4599 100644 --- a/contracts/voting/dao-voting-cw4/src/state.rs +++ b/contracts/voting/dao-voting-cw4/src/state.rs @@ -1,7 +1,6 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::Addr; use cw_storage_plus::Item; -use cw_utils::Duration; use dao_voting::threshold::ActiveThreshold; pub const GROUP_CONTRACT: Item = Item::new("group_contract"); @@ -12,8 +11,7 @@ pub const ACTIVE_THRESHOLD: Item = Item::new("active_threshold" #[cw_serde] pub struct Config { - pub active_threshold_enabled: bool, pub active_threshold: Option, } -pub const CONFIG: Item = Item::new("config"); \ No newline at end of file +pub const CONFIG: Item = Item::new("config"); diff --git a/contracts/voting/dao-voting-cw4/src/tests.rs b/contracts/voting/dao-voting-cw4/src/tests.rs index 769c53c4c..04a71745d 100644 --- a/contracts/voting/dao-voting-cw4/src/tests.rs +++ b/contracts/voting/dao-voting-cw4/src/tests.rs @@ -1,13 +1,15 @@ use cosmwasm_std::{ testing::{mock_dependencies, mock_env}, - to_json_binary, Addr, CosmosMsg, Empty, Uint128, WasmMsg, + to_json_binary, Addr, CosmosMsg, Decimal, Empty, Uint128, WasmMsg, }; use cw2::ContractVersion; use cw_multi_test::{next_block, App, Contract, ContractWrapper, Executor}; use dao_interface::voting::{ - InfoResponse, TotalPowerAtHeightResponse, VotingPowerAtHeightResponse, + InfoResponse, IsActiveResponse, TotalPowerAtHeightResponse, VotingPowerAtHeightResponse, }; +use dao_voting::threshold::ActiveThreshold; +use crate::msg::ExecuteMsg::UpdateActiveThreshold; use crate::{ contract::{migrate, CONTRACT_NAME, CONTRACT_VERSION}, msg::{GroupContract, InstantiateMsg, MigrateMsg, QueryMsg}, @@ -82,6 +84,7 @@ fn setup_test_case(app: &mut App) -> Addr { cw4_group_code_id: cw4_id, initial_members: members, }, + active_threshold: None, }, ) } @@ -100,6 +103,7 @@ fn test_instantiate() { cw4_group_code_id: cw4_id, initial_members: [].into(), }, + active_threshold: None, }; let _err = app .instantiate_contract( @@ -131,6 +135,7 @@ fn test_instantiate() { }, ], }, + active_threshold: None, }; let _err = app .instantiate_contract( @@ -174,6 +179,7 @@ pub fn test_instantiate_existing_contract() { group_contract: GroupContract::Existing { address: cw4_addr.to_string(), }, + active_threshold: None, }, &[], "voting module", @@ -206,6 +212,7 @@ pub fn test_instantiate_existing_contract() { group_contract: GroupContract::Existing { address: cw4_addr.to_string(), }, + active_threshold: None, }; let _err = app .instantiate_contract( @@ -580,6 +587,7 @@ fn test_migrate() { cw4_group_code_id: cw4_id, initial_members, }, + active_threshold: None, }; let voting_addr = app .instantiate_contract( @@ -657,6 +665,7 @@ fn test_duplicate_member() { }, ], }, + active_threshold: None, }; // Previous versions voting power was 100, due to no dedup. // Now we error @@ -741,3 +750,360 @@ pub fn test_migrate_update_version() { assert_eq!(version.version, CONTRACT_VERSION); assert_eq!(version.contract, CONTRACT_NAME); } + +/// First test checks if the contract correctly initializes with the specified +/// active threshold. +#[test] +fn test_initialization_with_active_threshold() { + let mut app = App::default(); + let voting_id = app.store_code(voting_contract()); + + // Define an active threshold for initialization. + let active_threshold = Some(dao_voting::threshold::ActiveThreshold::AbsoluteCount { + count: Uint128::new(5), + }); + + // Instantiate the contract with the defined active threshold. + let instantiate_msg = InstantiateMsg { + group_contract: GroupContract::New { + cw4_group_code_id: app.store_code(cw4_contract()), + initial_members: vec![ + cw4::Member { + addr: ADDR1.to_string(), + weight: 1, + }, + cw4::Member { + addr: ADDR2.to_string(), + weight: 2, + }, + cw4::Member { + addr: ADDR3.to_string(), + weight: 3, + }, + ], + }, + active_threshold: active_threshold.clone(), + }; + let voting_addr = app + .instantiate_contract( + voting_id, + Addr::unchecked(DAO_ADDR), + &instantiate_msg, + &[], + "voting", + None, + ) + .unwrap(); + + // Query the contract to see if the active threshold was set correctly. + let active_response: dao_interface::voting::IsActiveResponse = app + .wrap() + .query_wasm_smart(voting_addr, &QueryMsg::IsActive {}) + .unwrap(); + + // Making sure the response matches the expected active status based on the active threshold set. + let expected_active_status = active_response.active; + // Now assuming that the total weight is greater than or equal to the threshold count for the DAO to be active. + assert_eq!(expected_active_status, true); +} + +/// Second test checks if the contract rejects updates to the active +/// threshold from unauthorized accounts. +#[test] +fn test_reject_active_threshold_update_unauthorized() { + let mut app = App::default(); + let voting_id = app.store_code(voting_contract()); + let cw4_id = app.store_code(cw4_contract()); + + // Instantiate the contract. + let instantiate_msg = InstantiateMsg { + group_contract: GroupContract::New { + cw4_group_code_id: cw4_id, + initial_members: vec![ + cw4::Member { + addr: ADDR1.to_string(), + weight: 1, + }, + cw4::Member { + addr: ADDR2.to_string(), + weight: 1, + }, + ], + }, + active_threshold: None, + }; + let voting_addr = app + .instantiate_contract( + voting_id, + Addr::unchecked(DAO_ADDR), + &instantiate_msg, + &[], + "voting", + None, + ) + .unwrap(); + + // Attempt to update the active threshold from a non-DAO account. + let unauthorized_addr = "unauthorized"; + let update_msg = UpdateActiveThreshold { + new_threshold: Some(dao_voting::threshold::ActiveThreshold::AbsoluteCount { + count: Uint128::new(10), + }), + }; + let err = app + .execute_contract( + Addr::unchecked(unauthorized_addr), + voting_addr, + &update_msg, + &[], + ) + .unwrap_err(); + + // Check that the error returned matches the expected unauthorized error. + assert_eq!( + err.root_cause().to_string(), + ContractError::Unauthorized {}.to_string() + ); +} + +/// Third test verifies if the contract accepts updates to the active +/// threshold from the DAO account. This is important to ensure only +/// the authorized DAO address can update the active threshold. +#[test] +fn test_accept_active_threshold_update_authorized() { + let mut app = App::default(); + let voting_id = app.store_code(voting_contract()); + let cw4_id = app.store_code(cw4_contract()); + + // Instantiate the contract. + let instantiate_msg = InstantiateMsg { + group_contract: GroupContract::New { + cw4_group_code_id: cw4_id, + initial_members: vec![ + cw4::Member { + addr: ADDR1.to_string(), + weight: 1, + }, + cw4::Member { + addr: ADDR2.to_string(), + weight: 1, + }, + ], + }, + active_threshold: None, + }; + let voting_addr = app + .instantiate_contract( + voting_id, + Addr::unchecked(DAO_ADDR), + &instantiate_msg, + &[], + "voting", + None, + ) + .unwrap(); + + // Update the active threshold from the DAO account. + let update_msg = UpdateActiveThreshold { + new_threshold: Some(dao_voting::threshold::ActiveThreshold::AbsoluteCount { + count: Uint128::new(10), + }), + }; + let response = app + .execute_contract( + Addr::unchecked(DAO_ADDR), + voting_addr.clone(), + &update_msg, + &[], + ) + .unwrap(); + + // Check the response for a successful update using events. + assert!(response.events.iter().any(|event| event.ty == "wasm" + && event + .attributes + .iter() + .any(|attr| attr.key == "method" && attr.value == "update_active_threshold") + && event + .attributes + .iter() + .any(|attr| attr.key == "status" && attr.value == "success"))); + + // Query if IsActive is true when it should not be (since the count is set to 10, but we don't have that many members). + let is_active: IsActiveResponse = app + .wrap() + .query_wasm_smart(voting_addr, &QueryMsg::IsActive {}) + .unwrap(); + assert_eq!(is_active.active, false); +} + +/// Fourth test checks if the IsActive query responds correctly based on the +/// set active threshold. +#[test] +fn test_is_active_query_response() { + let mut app = App::default(); + let voting_id = app.store_code(voting_contract()); + let cw4_id = app.store_code(cw4_contract()); + + // Define members and set a specific active threshold. + let members = vec![ + cw4::Member { + addr: ADDR1.to_string(), + weight: 3, + }, + cw4::Member { + addr: ADDR2.to_string(), + weight: 2, + }, + cw4::Member { + addr: ADDR3.to_string(), + weight: 1, + }, + ]; + let active_threshold = dao_voting::threshold::ActiveThreshold::AbsoluteCount { + count: Uint128::new(5), // Threshold set such that it's just met by the sum of members' weights + }; + + // Instantiate the contract with these settings. + let instantiate_msg = InstantiateMsg { + group_contract: GroupContract::New { + cw4_group_code_id: cw4_id, + initial_members: members, + }, + active_threshold: Some(active_threshold), + }; + let voting_addr = app + .instantiate_contract( + voting_id, + Addr::unchecked(DAO_ADDR), + &instantiate_msg, + &[], + "voting", + None, + ) + .unwrap(); + + // Query the IsActive endpoint to check the active status. + let is_active_response: IsActiveResponse = app + .wrap() + .query_wasm_smart(voting_addr.clone(), &QueryMsg::IsActive {}) + .unwrap(); + + // Check if IsActive correctly reflects the status based on the active threshold. + // Since the total weight (6) meets the active threshold (5), the DAO should be active. + assert!( + is_active_response.active, + "DAO should be active as the threshold is met by the members' total weight." + ); + + // Change the threshold to a value higher than the total weight to test the negative case. + let update_threshold_msg = UpdateActiveThreshold { + new_threshold: Some(dao_voting::threshold::ActiveThreshold::AbsoluteCount { + count: Uint128::new(10), // This threshold is not met. + }), + }; + app.execute_contract( + Addr::unchecked(DAO_ADDR), + voting_addr.clone(), + &update_threshold_msg, + &[], + ) + .unwrap(); + + // Query again after updating the threshold. + let is_active_response_after_update: IsActiveResponse = app + .wrap() + .query_wasm_smart(voting_addr, &QueryMsg::IsActive {}) + .unwrap(); + + // Now, the DAO should not be active as the total weight does not meet the new threshold. + assert!( + !is_active_response_after_update.active, + "DAO should not be active as the new threshold is not met." + ); +} + +/// Fifth test checks if the contract rejects active threshold updates. +/// I included a trial to update the threshold to a percentage, which should fail +/// because the contract only supports absolute count. +#[test] +fn test_reject_invalid_active_threshold_updates() { + let mut app = App::default(); + let voting_id = app.store_code(voting_contract()); + let cw4_id = app.store_code(cw4_contract()); + + // Instantiate the contract with a valid initial threshold. + let members = vec![ + cw4::Member { + addr: ADDR1.to_string(), + weight: 1, + }, + cw4::Member { + addr: ADDR2.to_string(), + weight: 2, + }, + ]; + let instantiate_msg = InstantiateMsg { + group_contract: GroupContract::New { + cw4_group_code_id: cw4_id, + initial_members: members, + }, + active_threshold: Some(ActiveThreshold::AbsoluteCount { + count: Uint128::new(3), + }), + }; + let voting_addr = app + .instantiate_contract( + voting_id, + Addr::unchecked(DAO_ADDR), + &instantiate_msg, + &[], + "voting", + None, + ) + .unwrap(); + + // Attempt to set an invalid percentage threshold. + let update_msg_percentage = UpdateActiveThreshold { + new_threshold: Some(ActiveThreshold::Percentage { + percent: Decimal::percent(50), + }), + }; + let err_percentage = app + .execute_contract( + Addr::unchecked(DAO_ADDR), + voting_addr.clone(), + &update_msg_percentage, + &[], + ) + .unwrap_err(); + + // Expect the error to be about invalid threshold type. + assert_eq!( + err_percentage.root_cause().to_string(), + ContractError::InvalidThreshold {}.to_string(), + "Expected to fail with InvalidThreshold error for percentage update" + ); + + // Attempt to set an absolute count to zero. + let update_msg_zero = UpdateActiveThreshold { + new_threshold: Some(ActiveThreshold::AbsoluteCount { + count: Uint128::zero(), + }), + }; + let err_zero = app + .execute_contract( + Addr::unchecked(DAO_ADDR), + voting_addr, + &update_msg_zero, + &[], + ) + .unwrap_err(); + + // Expect the error to be about invalid zero threshold. + assert_eq!( + err_zero.root_cause().to_string(), + ContractError::InvalidThreshold {}.to_string(), + "Expected to fail with InvalidThreshold error for zero absolute count" + ); +} diff --git a/contracts/voting/dao-voting-cw721-staked/src/testing/mod.rs b/contracts/voting/dao-voting-cw721-staked/src/testing/mod.rs index de0824f52..1ffb20058 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/testing/mod.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/testing/mod.rs @@ -5,7 +5,7 @@ mod instantiate; mod queries; mod tests; -// Integrationg tests using an actual chain binary, requires +// Integrating tests using an actual chain binary, requires // the "test-tube" feature to be enabled // cargo test --features test-tube #[cfg(test)] diff --git a/packages/dao-testing/src/helpers.rs b/packages/dao-testing/src/helpers.rs index b629e7ba0..c61a143cb 100644 --- a/packages/dao-testing/src/helpers.rs +++ b/packages/dao-testing/src/helpers.rs @@ -99,6 +99,7 @@ pub fn instantiate_with_staked_balances_governance( governance_code_id: u64, governance_instantiate: Binary, initial_balances: Option>, + active_threshold: Option, ) -> Addr { let initial_balances = initial_balances.unwrap_or_else(|| { vec![Cw20Coin { @@ -305,6 +306,7 @@ pub fn instantiate_with_cw4_groups_governance( core_code_id: u64, proposal_module_instantiate: Binary, initial_weights: Option>, + active_threshold: Option, ) -> Addr { let cw4_id = app.store_code(cw4_group_contract()); let core_id = app.store_code(dao_dao_contract()); @@ -347,6 +349,7 @@ pub fn instantiate_with_cw4_groups_governance( cw4_group_code_id: cw4_id, initial_members: initial_weights, }, + active_threshold, }) .unwrap(), admin: Some(Admin::CoreModule {}), From 080e56e12fa954608896e1a0a3b0e1671a3fa9b7 Mon Sep 17 00:00:00 2001 From: theghostmac Date: Sun, 7 Apr 2024 17:24:44 +0100 Subject: [PATCH 3/6] Wrote 5 tests for active threshold functionality in the CW4 voting module, closes #781 --- packages/dao-testing/src/helpers.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/dao-testing/src/helpers.rs b/packages/dao-testing/src/helpers.rs index c61a143cb..c03eda2e3 100644 --- a/packages/dao-testing/src/helpers.rs +++ b/packages/dao-testing/src/helpers.rs @@ -99,7 +99,6 @@ pub fn instantiate_with_staked_balances_governance( governance_code_id: u64, governance_instantiate: Binary, initial_balances: Option>, - active_threshold: Option, ) -> Addr { let initial_balances = initial_balances.unwrap_or_else(|| { vec![Cw20Coin { From b519151da5d21e9b00fbe9cab31505f04dc97198 Mon Sep 17 00:00:00 2001 From: theghostmac Date: Sun, 7 Apr 2024 18:58:34 +0100 Subject: [PATCH 4/6] [fix] removed and blocked Percentage, removed Config, and saved ACTIVE_THRESHOLD in instantiate --- .../src/testing/adversarial_tests.rs | 4 +- .../src/testing/do_votes.rs | 3 +- .../src/testing/instantiate.rs | 6 +- .../dao-proposal-single/src/testing/tests.rs | 54 ++++------- .../voting/dao-voting-cw4/src/contract.rs | 93 ++++++++++--------- contracts/voting/dao-voting-cw4/src/state.rs | 8 -- contracts/voting/dao-voting-cw4/src/tests.rs | 6 +- 7 files changed, 71 insertions(+), 103 deletions(-) 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 f8c1477ad..beed03604 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/adversarial_tests.rs @@ -33,7 +33,7 @@ struct CommonTest { fn setup_test(messages: Vec) -> CommonTest { let mut app = App::default(); let instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -199,7 +199,6 @@ pub fn test_executed_prop_state_remains_after_vote_swing() { amount: Uint128::new(30), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -299,7 +298,6 @@ pub fn test_passed_prop_state_remains_after_vote_swing() { amount: Uint128::new(30), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); diff --git a/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs b/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs index 8db738cd9..d64bae788 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs @@ -98,7 +98,6 @@ where &mut App, InstantiateMsg, Option>, - Option, ) -> Addr, { let mut app = App::default(); @@ -149,7 +148,7 @@ where pre_propose_info, }; - let core_addr = setup_governance(&mut app, instantiate, Some(initial_balances), None); + let core_addr = setup_governance(&mut app, instantiate, Some(initial_balances)); let governance_modules: Vec = app .wrap() diff --git a/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs b/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs index 75d795e25..eb99cac8d 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/instantiate.rs @@ -96,7 +96,6 @@ pub(crate) fn instantiate_with_staked_cw721_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_balances: Option>, - active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); @@ -231,7 +230,6 @@ pub(crate) fn instantiate_with_native_staked_balances_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_balances: Option>, - active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); @@ -343,7 +341,6 @@ pub(crate) fn instantiate_with_staked_balances_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_balances: Option>, - active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); @@ -545,7 +542,6 @@ pub(crate) fn instantiate_with_cw4_groups_governance( app: &mut App, proposal_module_instantiate: InstantiateMsg, initial_weights: Option>, - active_threshold: Option, ) -> Addr { let proposal_module_code_id = app.store_code(proposal_single_contract()); let cw4_id = app.store_code(cw4_group_contract()); @@ -594,7 +590,7 @@ pub(crate) fn instantiate_with_cw4_groups_governance( cw4_group_code_id: cw4_id, initial_members: initial_weights, }, - active_threshold, + active_threshold: None, }) .unwrap(), admin: Some(Admin::CoreModule {}), diff --git a/contracts/proposal/dao-proposal-single/src/testing/tests.rs b/contracts/proposal/dao-proposal-single/src/testing/tests.rs index b58a239dc..20123565d 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/tests.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/tests.rs @@ -82,7 +82,7 @@ struct CommonTest { fn setup_test(messages: Vec) -> CommonTest { let mut app = App::default(); let instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -155,7 +155,7 @@ fn test_simple_propose_staked_balances() { fn test_simple_proposal_cw4_voting() { let mut app = App::default(); let instantiate = get_default_non_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let id = make_proposal(&mut app, &proposal_module, CREATOR_ADDR, vec![], None); @@ -198,7 +198,7 @@ fn test_simple_proposal_cw4_voting() { fn test_simple_proposal_auto_vote_yes() { let mut app = App::default(); let instantiate = get_default_non_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let id = make_proposal( &mut app, @@ -247,7 +247,7 @@ fn test_simple_proposal_auto_vote_yes() { fn test_simple_proposal_auto_vote_no() { let mut app = App::default(); let instantiate = get_default_non_token_dao_proposal_module_instantiate(&mut app); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let id = make_proposal( &mut app, @@ -336,7 +336,7 @@ fn test_voting_module_token_instantiate() { fn test_deposit_token_voting_module_token_fails_if_no_voting_module_token() { let mut app = App::default(); let instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); - instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); + instantiate_with_cw4_groups_governance(&mut app, instantiate, None); } #[test] @@ -358,7 +358,7 @@ fn test_instantiate_with_non_voting_module_cw20_deposit() { false, ); - let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_cw4_groups_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let proposal_id = make_proposal(&mut app, &proposal_module, CREATOR_ADDR, vec![], None); @@ -409,7 +409,7 @@ fn test_proposal_message_execution() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.close_proposal_on_execution_failure = false; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -505,7 +505,6 @@ fn test_proposal_message_timelock_execution() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -622,7 +621,6 @@ fn test_open_proposal_veto_unauthorized() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -686,7 +684,6 @@ fn test_open_proposal_veto_with_early_veto_flag_disabled() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -745,7 +742,6 @@ fn test_open_proposal_veto_with_no_timelock() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -812,7 +808,6 @@ fn test_vetoed_proposal_veto() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -891,7 +886,6 @@ fn test_open_proposal_veto_early() { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -959,7 +953,6 @@ fn test_timelocked_proposal_veto_unauthorized() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let gov_token = query_dao_token(&app, &core_addr); @@ -1061,7 +1054,6 @@ fn test_timelocked_proposal_veto_expired_timelock() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1149,7 +1141,6 @@ fn test_timelocked_proposal_execute_no_early_exec() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1235,7 +1226,6 @@ fn test_timelocked_proposal_execute_early() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1327,7 +1317,6 @@ fn test_timelocked_proposal_execute_active_timelock_unauthorized() -> anyhow::Re address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1420,7 +1409,6 @@ fn test_timelocked_proposal_execute_expired_timelock_not_vetoer() -> anyhow::Res address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1508,7 +1496,6 @@ fn test_proposal_message_timelock_veto() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1635,7 +1622,6 @@ fn test_proposal_message_timelock_early_execution() -> anyhow::Result<()> { amount: Uint128::new(85), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1724,7 +1710,6 @@ fn test_proposal_message_timelock_veto_before_passed() { amount: Uint128::new(85), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1799,7 +1784,6 @@ fn test_veto_only_members_execute_proposal() -> anyhow::Result<()> { address: CREATOR_ADDR.to_string(), amount: Uint128::new(85), }]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -1926,7 +1910,6 @@ fn test_proposal_cant_close_after_expiry_is_passed() { amount: Uint128::new(85), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2200,7 +2183,7 @@ fn test_anyone_may_propose_and_proposal_listing() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.pre_propose_info = PreProposeInfo::AnyoneMayPropose {}; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); for addr in 'm'..'z' { @@ -2283,7 +2266,7 @@ fn test_propose_non_member_auto_vote_fails() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.pre_propose_info = PreProposeInfo::AnyoneMayPropose {}; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); // Should fail if non-member tries to vote on proposal creation. @@ -2595,7 +2578,7 @@ fn test_min_duration_unit_missmatch() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.min_voting_period = Some(Duration::Height(10)); - instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + instantiate_with_staked_balances_governance(&mut app, instantiate, None); } #[test] @@ -2604,7 +2587,7 @@ fn test_min_duration_larger_than_proposal_duration() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.min_voting_period = Some(Duration::Time(604801)); - instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + instantiate_with_staked_balances_governance(&mut app, instantiate, None); } #[test] @@ -2613,7 +2596,7 @@ fn test_min_voting_period_no_early_pass() { let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.min_voting_period = Some(Duration::Height(10)); instantiate.max_voting_period = Duration::Height(100); - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2655,7 +2638,6 @@ fn test_min_duration_same_as_proposal_duration() { amount: Uint128::new(90), }, ]), - None, ); let gov_token = query_dao_token(&app, &core_addr); @@ -2679,7 +2661,7 @@ fn test_revoting_playthrough() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.allow_revoting = true; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2752,7 +2734,7 @@ fn test_allow_revoting_config_changes() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.allow_revoting = true; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); @@ -2857,7 +2839,6 @@ fn test_three_of_five_multisig() { amount: Uint128::new(1), }, ]), - None, ); let core_state: dao_interface::query::DumpStateResponse = app @@ -2940,7 +2921,6 @@ fn test_three_of_five_multisig_revoting() { amount: Uint128::new(1), }, ]), - None, ); let core_state: dao_interface::query::DumpStateResponse = app @@ -3095,7 +3075,6 @@ fn test_proposal_count_initialized_to_zero() { amount: Uint128::new(90), }, ]), - None, ); let core_state: dao_interface::query::DumpStateResponse = app @@ -3586,7 +3565,7 @@ fn test_proposal_too_large() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.pre_propose_info = PreProposeInfo::AnyoneMayPropose {}; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let proposal_module = query_single_proposal_module(&app, &core_addr); let err = app @@ -3871,7 +3850,6 @@ fn test_query_list_votes() { amount: Uint128::new(1), }, ]), - None, ); let proposal_module = query_single_proposal_module(&app, &core_addr); let proposal_id = make_proposal(&mut app, &proposal_module, "one", vec![], None); @@ -4174,7 +4152,7 @@ fn test_rational_clobbered_on_revote() { let mut app = App::default(); let mut instantiate = get_default_token_dao_proposal_module_instantiate(&mut app); instantiate.allow_revoting = true; - let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None, None); + let core_addr = instantiate_with_staked_balances_governance(&mut app, instantiate, None); let gov_token = query_dao_token(&app, &core_addr); let proposal_module = query_single_proposal_module(&app, &core_addr); diff --git a/contracts/voting/dao-voting-cw4/src/contract.rs b/contracts/voting/dao-voting-cw4/src/contract.rs index 0cb897ff3..e63691d1b 100644 --- a/contracts/voting/dao-voting-cw4/src/contract.rs +++ b/contracts/voting/dao-voting-cw4/src/contract.rs @@ -2,7 +2,7 @@ use cosmwasm_std::entry_point; use cosmwasm_std::{ to_json_binary, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg, - Uint128, Uint256, WasmMsg, + Uint128, WasmMsg, }; use cw2::{get_contract_version, set_contract_version, ContractVersion}; use cw4::{MemberListResponse, MemberResponse, TotalWeightResponse}; @@ -13,7 +13,7 @@ use dao_voting::threshold::ActiveThreshold; use crate::error::ContractError; use crate::msg::{ExecuteMsg, GroupContract, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{Config, ACTIVE_THRESHOLD, CONFIG, DAO, GROUP_CONTRACT}; +use crate::state::{ACTIVE_THRESHOLD, DAO, GROUP_CONTRACT}; pub(crate) const CONTRACT_NAME: &str = "crates.io:dao-voting-cw4"; pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -29,17 +29,18 @@ pub fn instantiate( ) -> Result { set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; - let config: Config = if let Some(active_threshold) = msg.active_threshold { - Config { - active_threshold: Some(active_threshold), - } - } else { - Config { - active_threshold: None, + // Validate and save the active threshold if provided + if let Some(threshold) = msg.active_threshold { + if let ActiveThreshold::AbsoluteCount { count } = threshold { + if count > Uint128::zero() { + ACTIVE_THRESHOLD.save(deps.storage, &threshold)?; + } else { + return Err(ContractError::InvalidThreshold {}); + } + } else { + return Err(ContractError::InvalidThreshold {}); } - }; - - CONFIG.save(deps.storage, &config)?; + } DAO.save(deps.storage, &info.sender)?; @@ -124,13 +125,33 @@ pub fn instantiate( #[cfg_attr(not(feature = "library"), entry_point)] pub fn execute( deps: DepsMut, - env: Env, + _env: Env, info: MessageInfo, msg: ExecuteMsg, ) -> Result { match msg { ExecuteMsg::UpdateActiveThreshold { new_threshold } => { - execute_update_active_threshold(deps, env, info, new_threshold) + let dao = DAO.load(deps.storage)?; + if info.sender != dao { + return Err(ContractError::Unauthorized {}); + } + if let Some(threshold) = new_threshold { + if let ActiveThreshold::AbsoluteCount { count } = threshold { + if count > Uint128::zero() { + ACTIVE_THRESHOLD.save(deps.storage, &threshold)?; + } else { + return Err(ContractError::InvalidThreshold {}); + } + } else { + return Err(ContractError::InvalidThreshold {}); + } + } else { + ACTIVE_THRESHOLD.remove(deps.storage); + } + + Ok(Response::new() + .add_attribute("method", "update_active_threshold") + .add_attribute("status", "success")) } } } @@ -222,43 +243,25 @@ pub fn query_info(deps: Deps) -> StdResult { } pub fn query_is_active(deps: Deps) -> StdResult { - let active_threshold = ACTIVE_THRESHOLD.may_load(deps.storage)?; - - match active_threshold { - Some(ActiveThreshold::AbsoluteCount { count }) => { - let group_contract = GROUP_CONTRACT.load(deps.storage)?; - let total_weight: TotalWeightResponse = deps.querier.query_wasm_smart( - &group_contract, - &cw4_group::msg::QueryMsg::TotalWeight { at_height: None }, - )?; - to_json_binary(&IsActiveResponse { - active: total_weight.weight >= count.u128() as u64, - }) - } - Some(ActiveThreshold::Percentage { percent }) => { - let group_contract = GROUP_CONTRACT.load(deps.storage)?; - let total_weight: TotalWeightResponse = deps.querier.query_wasm_smart( - &group_contract, - &cw4_group::msg::QueryMsg::TotalWeight { at_height: None }, - )?; - let percentage_base = Uint256::from(10u64).pow(percent.decimal_places()); // Ensure correct power base for scaling - let required_weight = Uint256::from(total_weight.weight) - .multiply_ratio(percent.atomics(), percentage_base); // Correct ratio calculation + let active_threshold = ACTIVE_THRESHOLD.load(deps.storage)?; + let group_contract = GROUP_CONTRACT.load(deps.storage)?; + let total_weight: TotalWeightResponse = deps.querier.query_wasm_smart( + group_contract, + &cw4_group::msg::QueryMsg::TotalWeight { at_height: None }, + )?; - to_json_binary(&IsActiveResponse { - active: Uint256::from(total_weight.weight) >= required_weight, - }) + let is_active = match active_threshold { + ActiveThreshold::AbsoluteCount { count } => { + total_weight.weight >= count.to_string().parse::().unwrap() } - None => to_json_binary(&IsActiveResponse { active: true }), - } + _ => false, // Should never happen as percentage is not supported + }; + + to_json_binary(&IsActiveResponse { active: is_active }) } #[cfg_attr(not(feature = "library"), entry_point)] pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { - let config: Config = CONFIG.load(deps.storage)?; - // Update config as necessary - CONFIG.save(deps.storage, &config)?; - let storage_version: ContractVersion = get_contract_version(deps.storage)?; // Only migrate if newer diff --git a/contracts/voting/dao-voting-cw4/src/state.rs b/contracts/voting/dao-voting-cw4/src/state.rs index 3427a4599..7e74fdb26 100644 --- a/contracts/voting/dao-voting-cw4/src/state.rs +++ b/contracts/voting/dao-voting-cw4/src/state.rs @@ -1,4 +1,3 @@ -use cosmwasm_schema::cw_serde; use cosmwasm_std::Addr; use cw_storage_plus::Item; use dao_voting::threshold::ActiveThreshold; @@ -8,10 +7,3 @@ pub const DAO: Item = Item::new("dao_address"); /// The minimum amount of users for the DAO to be active pub const ACTIVE_THRESHOLD: Item = Item::new("active_threshold"); - -#[cw_serde] -pub struct Config { - pub active_threshold: Option, -} - -pub const CONFIG: Item = Item::new("config"); diff --git a/contracts/voting/dao-voting-cw4/src/tests.rs b/contracts/voting/dao-voting-cw4/src/tests.rs index 04a71745d..f3311b5cd 100644 --- a/contracts/voting/dao-voting-cw4/src/tests.rs +++ b/contracts/voting/dao-voting-cw4/src/tests.rs @@ -804,7 +804,7 @@ fn test_initialization_with_active_threshold() { // Making sure the response matches the expected active status based on the active threshold set. let expected_active_status = active_response.active; // Now assuming that the total weight is greater than or equal to the threshold count for the DAO to be active. - assert_eq!(expected_active_status, true); + assert!(expected_active_status); } /// Second test checks if the contract rejects updates to the active @@ -934,7 +934,9 @@ fn test_accept_active_threshold_update_authorized() { .wrap() .query_wasm_smart(voting_addr, &QueryMsg::IsActive {}) .unwrap(); - assert_eq!(is_active.active, false); + + // The DAO should not be active as the total weight does not meet the new threshold. + assert!(!is_active.active, "DAO should be inactive as the threshold is not met by the members' total weight."); } /// Fourth test checks if the IsActive query responds correctly based on the From 0f563425ea9aa46d2e2fb4cd633940a6ab0c5467 Mon Sep 17 00:00:00 2001 From: theghostmac Date: Sun, 7 Apr 2024 19:01:21 +0100 Subject: [PATCH 5/6] [chore] ran cargo clippy and cargo fmt locally | pass --- .../proposal/dao-proposal-single/src/testing/do_votes.rs | 6 +----- contracts/voting/dao-voting-cw4/src/tests.rs | 5 ++++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs b/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs index d64bae788..3646a9ecf 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs @@ -94,11 +94,7 @@ fn do_test_votes( setup_governance: F, ) -> (App, Addr) where - F: Fn( - &mut App, - InstantiateMsg, - Option>, - ) -> Addr, + F: Fn(&mut App, InstantiateMsg, Option>) -> Addr, { let mut app = App::default(); diff --git a/contracts/voting/dao-voting-cw4/src/tests.rs b/contracts/voting/dao-voting-cw4/src/tests.rs index f3311b5cd..14c830b6f 100644 --- a/contracts/voting/dao-voting-cw4/src/tests.rs +++ b/contracts/voting/dao-voting-cw4/src/tests.rs @@ -936,7 +936,10 @@ fn test_accept_active_threshold_update_authorized() { .unwrap(); // The DAO should not be active as the total weight does not meet the new threshold. - assert!(!is_active.active, "DAO should be inactive as the threshold is not met by the members' total weight."); + assert!( + !is_active.active, + "DAO should be inactive as the threshold is not met by the members' total weight." + ); } /// Fourth test checks if the IsActive query responds correctly based on the From f0e69e3a453079fb7ea25092e510f63663cc8cf5 Mon Sep 17 00:00:00 2001 From: GhostMac <83423188+theghostmac@users.noreply.github.com> Date: Thu, 11 Jul 2024 07:14:11 +0100 Subject: [PATCH 6/6] Update contracts/voting/dao-voting-cw4/src/contract.rs Co-authored-by: Jake Hartnell --- contracts/voting/dao-voting-cw4/src/contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/voting/dao-voting-cw4/src/contract.rs b/contracts/voting/dao-voting-cw4/src/contract.rs index e63691d1b..a9e1da3d9 100644 --- a/contracts/voting/dao-voting-cw4/src/contract.rs +++ b/contracts/voting/dao-voting-cw4/src/contract.rs @@ -252,7 +252,7 @@ pub fn query_is_active(deps: Deps) -> StdResult { let is_active = match active_threshold { ActiveThreshold::AbsoluteCount { count } => { - total_weight.weight >= count.to_string().parse::().unwrap() + Uint128::new(total_weight.weight as u128) >= count } _ => false, // Should never happen as percentage is not supported };