From 80360063c93ed7dba943a0c6471957d82db49a36 Mon Sep 17 00:00:00 2001 From: Gabriel Lopez Date: Mon, 11 Sep 2023 13:38:29 -0500 Subject: [PATCH 1/4] Use indexed map for proposal modules Added an optional query parameter 'include_disabled' that is false by default Removed ActiveProposalModule query Added a migrate path FromV2 --- Cargo.lock | 209 ++++++++++++------ Cargo.toml | 4 + contracts/dao-dao-core/Cargo.toml | 2 + contracts/dao-dao-core/src/contract.rs | 168 ++++++++------ contracts/dao-dao-core/src/state.rs | 26 ++- contracts/dao-dao-core/src/tests.rs | 22 +- .../external/dao-migrator/src/contract.rs | 1 + packages/dao-interface/src/msg.rs | 9 +- packages/dao-interface/src/state.rs | 9 + .../dao-proposal-hook-counter/src/tests.rs | 1 + 10 files changed, 302 insertions(+), 149 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 29caf23d6..4ffc99b79 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -277,8 +277,8 @@ dependencies = [ "cw-utils 1.0.1", "cw20 1.1.0", "cw20-stake 2.2.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-single", "dao-proposal-single", "dao-voting 2.2.0", @@ -688,8 +688,8 @@ dependencies = [ "cw-utils 1.0.1", "cw2 1.1.0", "cw20-base 1.1.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "thiserror", ] @@ -829,8 +829,8 @@ dependencies = [ "cw20 1.1.0", "cw20-base 1.1.0", "cw20-stake 2.2.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-voting-cw20-staked", "thiserror", ] @@ -845,6 +845,17 @@ dependencies = [ "thiserror", ] +[[package]] +name = "cw-hooks" +version = "2.2.0" +source = "git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988#7f89ad1604e8022f202aef729853b0c8c7196988" +dependencies = [ + "cosmwasm-schema", + "cosmwasm-std", + "cw-storage-plus 1.1.0", + "thiserror", +] + [[package]] name = "cw-multi-test" version = "0.16.5" @@ -912,6 +923,17 @@ dependencies = [ "serde", ] +[[package]] +name = "cw-paginate-storage" +version = "2.2.0" +source = "git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988#7f89ad1604e8022f202aef729853b0c8c7196988" +dependencies = [ + "cosmwasm-std", + "cosmwasm-storage", + "cw-storage-plus 1.1.0", + "serde", +] + [[package]] name = "cw-payroll-factory" version = "2.2.0" @@ -1207,6 +1229,19 @@ dependencies = [ "serde", ] +[[package]] +name = "cw20" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a45a8794a5dd33b66af34caee52a7beceb690856adcc1682b6e3db88b2cdee62" +dependencies = [ + "cosmwasm-schema", + "cosmwasm-std", + "cw-utils 0.16.0", + "schemars", + "serde", +] + [[package]] name = "cw20" version = "1.1.0" @@ -1611,40 +1646,86 @@ dependencies = [ "cw20-base 1.1.0", "cw721 0.18.0", "cw721-base 0.18.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-core 2.2.0 (git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988)", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", + "dao-interface 2.2.0 (git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988)", "dao-proposal-sudo", "dao-voting-cw20-balance", "thiserror", ] +[[package]] +name = "dao-dao-core" +version = "2.2.0" +source = "git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988#7f89ad1604e8022f202aef729853b0c8c7196988" +dependencies = [ + "cosmwasm-schema", + "cosmwasm-std", + "cw-core", + "cw-paginate-storage 2.2.0 (git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988)", + "cw-storage-plus 1.1.0", + "cw-utils 0.16.0", + "cw2 0.16.0", + "cw20 0.16.0", + "cw721 0.16.0", + "dao-dao-macros 2.2.0 (git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988)", + "dao-interface 2.2.0 (git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988)", + "thiserror", +] + [[package]] name = "dao-dao-macros" version = "2.2.0" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-hooks", - "dao-interface", + "cw-hooks 2.2.0", + "dao-interface 2.2.0", "dao-voting 2.2.0", "proc-macro2", "quote", "syn 1.0.109", ] +[[package]] +name = "dao-dao-macros" +version = "2.2.0" +source = "git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988#7f89ad1604e8022f202aef729853b0c8c7196988" +dependencies = [ + "cosmwasm-schema", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "dao-interface" version = "2.2.0" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-hooks", + "cw-hooks 2.2.0", "cw-utils 1.0.1", "cw2 1.1.0", "cw20 1.1.0", "cw721 0.18.0", ] +[[package]] +name = "dao-interface" +version = "2.2.0" +source = "git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988#7f89ad1604e8022f202aef729853b0c8c7196988" +dependencies = [ + "cosmwasm-schema", + "cosmwasm-std", + "cw-hooks 2.2.0 (git+https://github.com/DA0-DA0/dao-contracts.git?rev=7f89ad1604e8022f202aef729853b0c8c7196988)", + "cw-utils 0.16.0", + "cw2 0.16.0", + "cw20 0.16.0", + "cw721 0.16.0", +] + [[package]] name = "dao-migrator" version = "2.2.0" @@ -1668,8 +1749,8 @@ dependencies = [ "cw20-staked-balance-voting", "cw4 0.13.4", "cw4-voting", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-proposal-single", "dao-testing", "dao-voting 0.1.0", @@ -1694,8 +1775,8 @@ dependencies = [ "cw20 1.1.0", "cw20-base 1.1.0", "cw4-group 1.1.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-base", "dao-proposal-hooks", "dao-proposal-single", @@ -1720,8 +1801,8 @@ dependencies = [ "cw20 1.1.0", "cw20-base 1.1.0", "cw4-group 1.1.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-approval-single", "dao-pre-propose-base", "dao-proposal-hooks", @@ -1739,12 +1820,12 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cw-denom", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-storage-plus 1.1.0", "cw-utils 1.0.1", "cw2 1.1.0", - "dao-interface", + "dao-interface 2.2.0", "dao-proposal-hooks", "dao-voting 2.2.0", "serde", @@ -1764,8 +1845,8 @@ dependencies = [ "cw20 1.1.0", "cw20-base 1.1.0", "cw4-group 1.1.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-base", "dao-proposal-hooks", "dao-proposal-multiple", @@ -1782,15 +1863,15 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cw-denom", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-utils 1.0.1", "cw2 1.1.0", "cw20 1.1.0", "cw20-base 1.1.0", "cw4-group 1.1.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-base", "dao-proposal-hooks", "dao-proposal-single", @@ -1813,9 +1894,9 @@ dependencies = [ "cw2 1.1.0", "cw4 1.1.0", "cw4-group 1.1.0", - "dao-dao-core", - "dao-dao-macros", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-testing", "dao-voting 2.2.0", "dao-voting-cw4", @@ -1828,15 +1909,15 @@ version = "2.2.0" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-storage-plus 1.1.0", "cw-utils 1.0.1", "cw2 1.1.0", "cw20 1.1.0", "cw20-base 1.1.0", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-proposal-hooks", "dao-proposal-single", "dao-vote-hooks", @@ -1851,7 +1932,7 @@ version = "2.2.0" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-hooks", + "cw-hooks 2.2.0", "dao-voting 2.2.0", ] @@ -1863,7 +1944,7 @@ dependencies = [ "cosmwasm-std", "cosmwasm-storage", "cw-denom", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-storage-plus 1.1.0", "cw-utils 1.0.1", @@ -1875,8 +1956,8 @@ dependencies = [ "cw4 1.1.0", "cw4-group 1.1.0", "cw721-base 0.18.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-base", "dao-pre-propose-multiple", "dao-proposal-hooks", @@ -1902,7 +1983,7 @@ dependencies = [ "cosmwasm-storage", "cw-core", "cw-denom", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-proposal-single", "cw-storage-plus 1.1.0", @@ -1916,9 +1997,9 @@ dependencies = [ "cw4 1.1.0", "cw4-group 1.1.0", "cw721-base 0.18.0", - "dao-dao-core", - "dao-dao-macros", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-base", "dao-pre-propose-single", "dao-proposal-hooks", @@ -1944,8 +2025,8 @@ dependencies = [ "cw-multi-test", "cw-storage-plus 1.1.0", "cw2 1.1.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "thiserror", ] @@ -1956,7 +2037,7 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cw-core", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-proposal-single", "cw-tokenfactory-issuer", @@ -1970,8 +2051,8 @@ dependencies = [ "cw4-group 1.1.0", "cw721-base 0.18.0", "cw721-roles", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-multiple", "dao-pre-propose-single", "dao-proposal-condorcet", @@ -2000,7 +2081,7 @@ version = "2.2.0" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cw-hooks", + "cw-hooks 2.2.0", "dao-voting 2.2.0", ] @@ -2026,8 +2107,8 @@ dependencies = [ "cw-storage-plus 1.1.0", "cw-utils 1.0.1", "cw20 1.1.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "thiserror", ] @@ -2043,8 +2124,8 @@ dependencies = [ "cw2 1.1.0", "cw20 1.1.0", "cw20-base 1.1.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "thiserror", ] @@ -2062,8 +2143,8 @@ dependencies = [ "cw20 1.1.0", "cw20-base 1.1.0", "cw20-stake 2.2.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-voting 2.2.0", "thiserror", ] @@ -2081,8 +2162,8 @@ dependencies = [ "cw2 1.1.0", "cw4 1.1.0", "cw4-group 1.1.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "thiserror", ] @@ -2105,8 +2186,8 @@ dependencies = [ "cw721-controllers", "cw721-roles", "dao-cw721-extensions", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-testing", "thiserror", ] @@ -2127,8 +2208,8 @@ dependencies = [ "cw721 0.18.0", "cw721-base 0.18.0", "cw721-controllers", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-testing", "dao-voting 2.2.0", "sg-multi-test", @@ -2147,14 +2228,14 @@ dependencies = [ "cosmwasm-std", "cosmwasm-storage", "cw-controllers 1.1.0", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-paginate-storage 2.2.0", "cw-storage-plus 1.1.0", "cw-utils 1.0.1", "cw2 1.1.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-voting 2.2.0", "thiserror", ] @@ -2168,15 +2249,15 @@ dependencies = [ "cosmwasm-std", "cosmwasm-storage", "cw-controllers 1.1.0", - "cw-hooks", + "cw-hooks 2.2.0", "cw-multi-test", "cw-paginate-storage 2.2.0", "cw-storage-plus 1.1.0", "cw-tokenfactory-issuer", "cw-utils 1.0.1", "cw2 1.1.0", - "dao-dao-macros", - "dao-interface", + "dao-dao-macros 2.2.0", + "dao-interface 2.2.0", "dao-testing", "dao-voting 2.2.0", "osmosis-std 0.17.0-rc0", @@ -2831,8 +2912,8 @@ dependencies = [ "cw721 0.18.0", "cw721-base 0.18.0", "cw721-roles", - "dao-dao-core", - "dao-interface", + "dao-dao-core 2.2.0", + "dao-interface 2.2.0", "dao-pre-propose-single", "dao-proposal-single", "dao-voting 2.2.0", diff --git a/Cargo.toml b/Cargo.toml index 2d5bff497..0d75400b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -129,6 +129,10 @@ cw4-voting-v1 = { package = "cw4-voting", version = "0.1.0" } voting-v1 = { package = "dao-voting", version = "0.1.0" } stake-cw20-v03 = { package = "stake-cw20", version = "0.2.6" } +# v2 dependencies. used for state migrations +cw-core-v2 = { git = "https://github.com/DA0-DA0/dao-contracts.git", rev = "7f89ad1604e8022f202aef729853b0c8c7196988", package = "dao-dao-core" } +dao-interface-v2 = { git = "https://github.com/DA0-DA0/dao-contracts.git", rev = "7f89ad1604e8022f202aef729853b0c8c7196988", package = "dao-interface" } + # TODO remove when upstream PR merged and new release tagged: https://github.com/CosmWasm/cw-multi-test/pull/51 [patch.crates-io] cw-multi-test = { git = "https://github.com/JakeHartnell/cw-multi-test.git", branch = "bank-supply-support" } diff --git a/contracts/dao-dao-core/Cargo.toml b/contracts/dao-dao-core/Cargo.toml index ac8e2060d..9428e2679 100644 --- a/contracts/dao-dao-core/Cargo.toml +++ b/contracts/dao-dao-core/Cargo.toml @@ -29,6 +29,8 @@ dao-interface = { workspace = true } dao-dao-macros = { workspace = true } cw-paginate-storage = { workspace = true } cw-core-v1 = { workspace = true, features = ["library"] } +cw-core-v2 = { workspace = true, features = ["library"] } +dao-interface-v2 = { workspace = true } [dev-dependencies] cw-multi-test = { workspace = true, features = ["stargate"] } diff --git a/contracts/dao-dao-core/src/contract.rs b/contracts/dao-dao-core/src/contract.rs index 97f3a7d3e..dba1b490f 100644 --- a/contracts/dao-dao-core/src/contract.rs +++ b/contracts/dao-dao-core/src/contract.rs @@ -5,9 +5,9 @@ use cosmwasm_std::{ Reply, Response, StdError, StdResult, SubMsg, WasmMsg, }; use cw2::{get_contract_version, set_contract_version, ContractVersion}; -use cw_paginate_storage::{paginate_map, paginate_map_keys, paginate_map_values}; -use cw_storage_plus::Map; -use cw_utils::{parse_reply_instantiate_data, Duration}; +use cw_paginate_storage::{paginate_map, paginate_map_keys}; +use cw_storage_plus::{Bound, Map}; +use cw_utils::{maybe_addr, parse_reply_instantiate_data, Duration}; use dao_interface::{ msg::{ExecuteMsg, InitialItem, InstantiateMsg, MigrateMsg, QueryMsg}, query::{ @@ -23,8 +23,8 @@ use dao_interface::{ use crate::error::ContractError; use crate::state::{ - ACTIVE_PROPOSAL_MODULE_COUNT, ADMIN, CONFIG, CW20_LIST, CW721_LIST, ITEMS, NOMINATED_ADMIN, - PAUSED, PROPOSAL_MODULES, SUBDAO_LIST, TOTAL_PROPOSAL_MODULE_COUNT, VOTING_MODULE, + proposal_modules, ACTIVE_PROPOSAL_MODULE_COUNT, ADMIN, CONFIG, CW20_LIST, CW721_LIST, ITEMS, + NOMINATED_ADMIN, PAUSED, SUBDAO_LIST, TOTAL_PROPOSAL_MODULE_COUNT, VOTING_MODULE, }; pub(crate) const CONTRACT_NAME: &str = "crates.io:dao-dao-core"; @@ -196,7 +196,7 @@ pub fn execute_proposal_hook( sender: Addr, msgs: Vec>, ) -> Result { - let module = PROPOSAL_MODULES + let module = proposal_modules() .may_load(deps.storage, sender.clone())? .ok_or(ContractError::Unauthorized {})?; @@ -342,7 +342,7 @@ pub fn execute_update_proposal_modules( let disable_count = to_disable.len() as u32; for addr in to_disable { let addr = deps.api.addr_validate(&addr)?; - let mut module = PROPOSAL_MODULES + let mut module = proposal_modules() .load(deps.storage, addr.clone()) .map_err(|_| ContractError::ProposalModuleDoesNotExist { address: addr.clone(), @@ -355,7 +355,7 @@ pub fn execute_update_proposal_modules( } module.status = ProposalModuleStatus::Disabled {}; - PROPOSAL_MODULES.save(deps.storage, addr, &module)?; + proposal_modules().save(deps.storage, addr, &module)?; } // If disabling this module will cause there to be no active modules, return error. @@ -558,18 +558,22 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult { QueryMsg::Info {} => query_info(deps), QueryMsg::ListItems { start_after, limit } => query_list_items(deps, start_after, limit), QueryMsg::PauseInfo {} => query_paused(deps, env), - QueryMsg::ProposalModules { start_after, limit } => { - query_proposal_modules(deps, start_after, limit) - } + QueryMsg::ProposalModules { + start_after, + limit, + include_disabled, + } => to_binary(&query_proposal_modules( + deps, + start_after, + limit, + include_disabled, + )?), QueryMsg::ProposalModuleCount {} => query_proposal_module_count(deps), QueryMsg::TotalPowerAtHeight { height } => query_total_power_at_height(deps, height), QueryMsg::VotingModule {} => query_voting_module(deps), QueryMsg::VotingPowerAtHeight { address, height } => { query_voting_power_at_height(deps, address, height) } - QueryMsg::ActiveProposalModules { start_after, limit } => { - query_active_proposal_modules(deps, start_after, limit) - } QueryMsg::ListSubDaos { start_after, limit } => { query_list_sub_daos(deps, start_after, limit) } @@ -601,57 +605,45 @@ pub fn query_proposal_modules( deps: Deps, start_after: Option, limit: Option, -) -> StdResult { - // This query is will run out of gas due to the size of the - // returned message before it runs out of compute so taking a - // limit here is still nice. As removes happen in constant time - // the contract is still recoverable if too many items end up in - // here. - // - // Further, as the `range` method on a map returns an iterator it - // is possible (though implementation dependent) that new keys are - // loaded on demand as the iterator is moved. Should this be the - // case we are only paying for what we need here. - // - // Even if this does lock up one can determine the existing - // proposal modules by looking at past transactions on chain. - to_binary(&paginate_map_values( - deps, - &PROPOSAL_MODULES, - start_after - .map(|s| deps.api.addr_validate(&s)) - .transpose()?, - limit, - cosmwasm_std::Order::Ascending, - )?) -} - -pub fn query_active_proposal_modules( - deps: Deps, - start_after: Option, - limit: Option, -) -> StdResult { - // Note: this is not gas efficient as we need to potentially visit all modules in order to - // filter out the modules with active status. - let values = paginate_map_values( - deps, - &PROPOSAL_MODULES, - start_after - .map(|s| deps.api.addr_validate(&s)) - .transpose()?, - None, - cosmwasm_std::Order::Ascending, - )?; + include_disabled: Option, +) -> StdResult> { + let start_after_bound = maybe_addr(deps.api, start_after)?.map(Bound::exclusive); + let include_disabled = include_disabled.unwrap_or(false); + + let items_iter: Box>> = if include_disabled { + Box::new( + proposal_modules() + .range( + deps.storage, + start_after_bound, + None, + cosmwasm_std::Order::Ascending, + ) + .map(|x| x.map(|y| y.1)), + ) + } else { + Box::new( + proposal_modules() + .idx + .status + .prefix(ProposalModuleStatus::Enabled.to_string()) + .range( + deps.storage, + start_after_bound, + None, + cosmwasm_std::Order::Ascending, + ) + .map(|x| x.map(|y| y.1)), + ) + }; - let limit = limit.unwrap_or(values.len() as u32); + let items: Vec<_> = if let Some(l) = limit { + items_iter.take(l as usize).collect::>>()? + } else { + items_iter.collect::>>()? + }; - to_binary::>( - &values - .into_iter() - .filter(|module: &ProposalModule| module.status == ProposalModuleStatus::Enabled) - .take(limit as usize) - .collect(), - ) + Ok(items) } fn get_pause_info(deps: Deps, env: Env) -> StdResult { @@ -675,10 +667,7 @@ pub fn query_dump_state(deps: Deps, env: Env) -> StdResult { let admin = ADMIN.load(deps.storage)?; let config = CONFIG.load(deps.storage)?; let voting_module = VOTING_MODULE.load(deps.storage)?; - let proposal_modules = PROPOSAL_MODULES - .range(deps.storage, None, None, cosmwasm_std::Order::Ascending) - .map(|kv| Ok(kv?.1)) - .collect::>>()?; + let proposal_modules = query_proposal_modules(deps, None, None, None)?; let pause_info = get_pause_info(deps, env)?; let version = get_contract_version(deps.storage)?; let active_proposal_module_count = ACTIVE_PROPOSAL_MODULE_COUNT.load(deps.storage)?; @@ -883,7 +872,7 @@ pub fn migrate(deps: DepsMut, env: Env, msg: MigrateMsg) -> Result Result { + // `CONTRACT_VERSION` here is from the data section of the + // blob we are migrating to. `version` is from storage. If + // the version in storage matches the version in the blob + // we are not upgrading. + if version == CONTRACT_VERSION { + return Err(ContractError::AlreadyMigrated {}); + } + + use cw_core_v2 as v2; + use dao_interface_v2 as v2_interface; + + let current_keys = v2::state::PROPOSAL_MODULES + .keys(deps.storage, None, None, Order::Ascending) + .collect::>>()?; + + // Update proposal modules to v3. + current_keys + .into_iter() + .enumerate() + .try_for_each::<_, StdResult<()>>(|(_idx, address)| { + let proposal_module = + v2::state::PROPOSAL_MODULES.load(deps.storage, address.clone())?; + let proposal_module = &ProposalModule { + address: address.clone(), + status: match proposal_module.status { + v2_interface::state::ProposalModuleStatus::Disabled => { + ProposalModuleStatus::Disabled + } + v2_interface::state::ProposalModuleStatus::Enabled => { + ProposalModuleStatus::Enabled + } + }, + prefix: proposal_module.prefix.clone(), + }; + proposal_modules().save(deps.storage, address, proposal_module)?; + Ok(()) + })?; + + Ok(Response::default()) + } MigrateMsg::FromCompatible {} => Ok(Response::default()), } } @@ -942,7 +972,7 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result = Item::new("paused"); /// The voting module associated with this contract. pub const VOTING_MODULE: Item = Item::new("voting_module"); +pub struct ProposalModulesIndexes<'a> { + pub status: MultiIndex<'a, String, ProposalModule, Addr>, +} + +impl<'a> IndexList for ProposalModulesIndexes<'a> { + fn get_indexes(&'_ self) -> Box> + '_> { + let v: Vec<&dyn Index> = vec![&self.status]; + Box::new(v.into_iter()) + } +} + /// The proposal modules associated with this contract. -/// When we change the data format of this map, we update the key (previously "proposal_modules") +/// When we change the data format of this map, we update the key (previously "proposal_modules" or "proposal_modules_v2") /// to create a new namespace for the changed state. -pub const PROPOSAL_MODULES: Map = Map::new("proposal_modules_v2"); +pub fn proposal_modules<'a>() -> IndexedMap<'a, Addr, ProposalModule, ProposalModulesIndexes<'a>> { + let indexes = ProposalModulesIndexes { + status: MultiIndex::new( + |_x, d: &ProposalModule| d.status.to_string(), + "proposal_modules_v3", + "proposal_modules_v3__status", + ), + }; + IndexedMap::new("proposal_modules_v3", indexes) +} /// The count of active proposal modules associated with this contract. pub const ACTIVE_PROPOSAL_MODULE_COUNT: Item = Item::new("active_proposal_module_count"); diff --git a/contracts/dao-dao-core/src/tests.rs b/contracts/dao-dao-core/src/tests.rs index dac148175..ec66bbdb8 100644 --- a/contracts/dao-dao-core/src/tests.rs +++ b/contracts/dao-dao-core/src/tests.rs @@ -20,7 +20,7 @@ use dao_interface::{ use crate::{ contract::{derive_proposal_module_prefix, migrate, CONTRACT_NAME, CONTRACT_VERSION}, - state::PROPOSAL_MODULES, + state::proposal_modules, ContractError, }; @@ -281,6 +281,7 @@ fn test_update_config() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -378,6 +379,7 @@ fn test_swap_governance(swaps: Vec<(u32, u32)>) { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -407,6 +409,7 @@ fn test_swap_governance(swaps: Vec<(u32, u32)>) { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -554,6 +557,7 @@ fn test_removed_modules_can_not_execute() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -634,9 +638,10 @@ fn test_removed_modules_can_not_execute() { .wrap() .query_wasm_smart( &gov_addr, - &QueryMsg::ActiveProposalModules { + &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: None, }, ) .unwrap(); @@ -711,6 +716,7 @@ fn test_module_already_disabled() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -815,6 +821,7 @@ fn test_swap_voting_module() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -1018,6 +1025,7 @@ fn test_admin_permissions() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -2285,6 +2293,7 @@ fn test_pause() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -2518,6 +2527,7 @@ fn test_dump_state_proposal_modules() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -2782,7 +2792,7 @@ fn test_migrate_mock() { // Migrate to v2 migrate(deps.as_mut(), env, msg).unwrap(); - let new_path = PROPOSAL_MODULES.key(proposal_modules_key); + let new_path = proposal_modules().key(proposal_modules_key); let prop_module_bytes = deps.storage.get(&new_path).unwrap(); let module: ProposalModule = from_slice(&prop_module_bytes).unwrap(); assert_eq!(module.address, Addr::unchecked("addr")); @@ -2815,6 +2825,7 @@ fn test_execute_stargate_msg() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -2902,6 +2913,7 @@ fn test_module_prefixes() { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -2932,14 +2944,12 @@ fn get_active_modules(app: &App, gov_addr: Addr) -> Vec { &QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: None, }, ) .unwrap(); modules - .into_iter() - .filter(|module: &ProposalModule| module.status == ProposalModuleStatus::Enabled) - .collect() } fn query_proposal_module_count(app: &App, core_addr: &Addr) -> ProposalModuleCountResponse { diff --git a/contracts/external/dao-migrator/src/contract.rs b/contracts/external/dao-migrator/src/contract.rs index ae91152d5..4347275e9 100644 --- a/contracts/external/dao-migrator/src/contract.rs +++ b/contracts/external/dao-migrator/src/contract.rs @@ -230,6 +230,7 @@ fn execute_migration_v1_v2( &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, )?; diff --git a/packages/dao-interface/src/msg.rs b/packages/dao-interface/src/msg.rs index 797865c2e..29a1ee205 100644 --- a/packages/dao-interface/src/msg.rs +++ b/packages/dao-interface/src/msg.rs @@ -190,13 +190,7 @@ pub enum QueryMsg { ProposalModules { start_after: Option, limit: Option, - }, - /// Gets the active proposal modules associated with the - /// contract. - #[returns(Vec)] - ActiveProposalModules { - start_after: Option, - limit: Option, + include_disabled: Option, }, /// Gets the number of active and total proposal modules /// registered with this module. @@ -236,5 +230,6 @@ pub enum MigrateMsg { dao_uri: Option, params: Option, }, + FromV2 {}, FromCompatible {}, } diff --git a/packages/dao-interface/src/state.rs b/packages/dao-interface/src/state.rs index 3a0841f9b..680083600 100644 --- a/packages/dao-interface/src/state.rs +++ b/packages/dao-interface/src/state.rs @@ -40,6 +40,15 @@ pub enum ProposalModuleStatus { Disabled, } +impl std::fmt::Display for ProposalModuleStatus { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match *self { + ProposalModuleStatus::Enabled => write!(f, "Enabled"), + ProposalModuleStatus::Disabled => write!(f, "Disabled"), + } + } +} + /// Information about the CosmWasm level admin of a contract. Used in /// conjunction with `ModuleInstantiateInfo` to instantiate modules. #[cw_serde] diff --git a/test-contracts/dao-proposal-hook-counter/src/tests.rs b/test-contracts/dao-proposal-hook-counter/src/tests.rs index cb3483a81..a376890f5 100644 --- a/test-contracts/dao-proposal-hook-counter/src/tests.rs +++ b/test-contracts/dao-proposal-hook-counter/src/tests.rs @@ -164,6 +164,7 @@ fn test_counters() { &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); From cfd02331133d87411893cdf7b34f1e9f800cd1b9 Mon Sep 17 00:00:00 2001 From: Gabriel Lopez Date: Mon, 11 Sep 2023 13:53:22 -0500 Subject: [PATCH 2/4] Update test_migration.rs Errors weren't visible, because test-tube doesn't run for the windows plebs --- contracts/external/dao-migrator/src/testing/test_migration.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/external/dao-migrator/src/testing/test_migration.rs b/contracts/external/dao-migrator/src/testing/test_migration.rs index cf936f7e4..edbdd8946 100644 --- a/contracts/external/dao-migrator/src/testing/test_migration.rs +++ b/contracts/external/dao-migrator/src/testing/test_migration.rs @@ -78,6 +78,7 @@ pub fn basic_test(voting_type: VotingType, from_core: bool) { &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -146,6 +147,7 @@ fn test_migrator_address_is_first() { &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); From ce6e5dcfc4f5bdc4afc66363f643bb26091f508e Mon Sep 17 00:00:00 2001 From: Gabriel Lopez Date: Mon, 11 Sep 2023 14:07:10 -0500 Subject: [PATCH 3/4] Fix proposal module msgs --- .../pre-propose/dao-pre-propose-approval-single/src/tests.rs | 1 + contracts/pre-propose/dao-pre-propose-approver/src/tests.rs | 2 ++ contracts/pre-propose/dao-pre-propose-multiple/src/tests.rs | 1 + contracts/pre-propose/dao-pre-propose-single/src/tests.rs | 1 + contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs | 1 + .../proposal/dao-proposal-multiple/src/testing/do_votes.rs | 1 + contracts/proposal/dao-proposal-multiple/src/testing/queries.rs | 1 + contracts/proposal/dao-proposal-multiple/src/testing/tests.rs | 1 + contracts/proposal/dao-proposal-single/src/testing/do_votes.rs | 1 + contracts/proposal/dao-proposal-single/src/testing/queries.rs | 1 + 10 files changed, 11 insertions(+) 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 2127cebf5..f96ae038b 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 @@ -140,6 +140,7 @@ fn setup_default_test( &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); 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 5aea7b2bc..b9b886c86 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( &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); @@ -255,6 +256,7 @@ fn setup_default_test( &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); 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 e256a1f47..613b5a062 100644 --- a/contracts/pre-propose/dao-pre-propose-multiple/src/tests.rs +++ b/contracts/pre-propose/dao-pre-propose-multiple/src/tests.rs @@ -140,6 +140,7 @@ fn setup_default_test( &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); 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 6a8b48e9f..595e2bc4d 100644 --- a/contracts/pre-propose/dao-pre-propose-single/src/tests.rs +++ b/contracts/pre-propose/dao-pre-propose-single/src/tests.rs @@ -138,6 +138,7 @@ fn setup_default_test( &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); diff --git a/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs b/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs index a9e7ac30a..da967a079 100644 --- a/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs +++ b/contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs @@ -124,6 +124,7 @@ impl SuiteBuilder { &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); diff --git a/contracts/proposal/dao-proposal-multiple/src/testing/do_votes.rs b/contracts/proposal/dao-proposal-multiple/src/testing/do_votes.rs index 32eb43168..7cc2eecd9 100644 --- a/contracts/proposal/dao-proposal-multiple/src/testing/do_votes.rs +++ b/contracts/proposal/dao-proposal-multiple/src/testing/do_votes.rs @@ -141,6 +141,7 @@ where &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); diff --git a/contracts/proposal/dao-proposal-multiple/src/testing/queries.rs b/contracts/proposal/dao-proposal-multiple/src/testing/queries.rs index da528383d..c93d78e8e 100644 --- a/contracts/proposal/dao-proposal-multiple/src/testing/queries.rs +++ b/contracts/proposal/dao-proposal-multiple/src/testing/queries.rs @@ -52,6 +52,7 @@ pub fn query_multiple_proposal_module(app: &App, core_addr: &Addr) -> Addr { &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); diff --git a/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs b/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs index eaf69fcc8..73375278d 100644 --- a/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs +++ b/contracts/proposal/dao-proposal-multiple/src/testing/tests.rs @@ -2372,6 +2372,7 @@ fn test_query_list_proposals() { &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); 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 ecd72e325..1c69c6fbe 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/do_votes.rs @@ -139,6 +139,7 @@ where &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); diff --git a/contracts/proposal/dao-proposal-single/src/testing/queries.rs b/contracts/proposal/dao-proposal-single/src/testing/queries.rs index a246f1c64..20270c06a 100644 --- a/contracts/proposal/dao-proposal-single/src/testing/queries.rs +++ b/contracts/proposal/dao-proposal-single/src/testing/queries.rs @@ -142,6 +142,7 @@ pub(crate) fn query_single_proposal_module(app: &App, core_addr: &Addr) -> Addr &dao_interface::msg::QueryMsg::ProposalModules { start_after: None, limit: None, + include_disabled: Some(true), }, ) .unwrap(); From e6fb683a4507ee97a85ef58782816cc953f73806 Mon Sep 17 00:00:00 2001 From: Gabriel Lopez Date: Mon, 11 Sep 2023 16:48:56 -0500 Subject: [PATCH 4/4] Slight simplification on query result --- contracts/dao-dao-core/src/contract.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/contracts/dao-dao-core/src/contract.rs b/contracts/dao-dao-core/src/contract.rs index dba1b490f..7089cd6e7 100644 --- a/contracts/dao-dao-core/src/contract.rs +++ b/contracts/dao-dao-core/src/contract.rs @@ -637,13 +637,11 @@ pub fn query_proposal_modules( ) }; - let items: Vec<_> = if let Some(l) = limit { - items_iter.take(l as usize).collect::>>()? + if let Some(l) = limit { + items_iter.take(l as usize).collect() } else { - items_iter.collect::>>()? - }; - - Ok(items) + items_iter.collect() + } } fn get_pause_info(deps: Deps, env: Env) -> StdResult {