diff --git a/Cargo.lock b/Cargo.lock index 284445c3d..ff8b2b2f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1977,6 +1977,7 @@ dependencies = [ "cw-tokenfactory-issuer", "cw-utils 1.0.1", "cw2 1.1.0", + "cw721 0.18.0", "cw721-base 0.18.0", "dao-dao-macros", "dao-interface", @@ -2915,6 +2916,7 @@ dependencies = [ "dao-interface", "dao-pre-propose-single", "dao-proposal-single", + "dao-test-custom-factory", "dao-voting 2.2.0", "dao-voting-cw20-staked", "dao-voting-cw721-staked", diff --git a/ci/integration-tests/Cargo.toml b/ci/integration-tests/Cargo.toml index 3d9e217b8..6291ec502 100644 --- a/ci/integration-tests/Cargo.toml +++ b/ci/integration-tests/Cargo.toml @@ -25,6 +25,7 @@ dao-dao-core = { workspace = true } dao-interface = { workspace = true } dao-pre-propose-single = { workspace = true } dao-proposal-single = { workspace = true } +dao-test-custom-factory = { workspace = true } dao-voting = { workspace = true } dao-voting-cw20-staked = { workspace = true } dao-voting-cw721-staked = { workspace = true } diff --git a/contracts/test/dao-test-custom-factory/Cargo.toml b/contracts/test/dao-test-custom-factory/Cargo.toml index 8c5dff781..47ffd045e 100644 --- a/contracts/test/dao-test-custom-factory/Cargo.toml +++ b/contracts/test/dao-test-custom-factory/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "dao-test-custom-factory" authors = ["Jake Hartnell"] -description = "A test contract for testing factory patterns in dao-voting-token-staked." +description = "A test contract for testing factory patterns in dao-voting-token-staked and dao-voting-cw721-staked." edition = { workspace = true } license = { workspace = true } repository = { workspace = true } @@ -21,6 +21,7 @@ cosmwasm-std = { workspace = true } cosmwasm-schema = { workspace = true } cosmwasm-storage = { workspace = true } cw2 = { workspace = true } +cw721 = { workspace = true } cw721-base = { workspace = true, features = ["library"] } cw-ownable = { workspace = true } cw-storage-plus = { workspace = true } diff --git a/contracts/test/dao-test-custom-factory/README b/contracts/test/dao-test-custom-factory/README index 9dc8b8b94..61c1265e5 100644 --- a/contracts/test/dao-test-custom-factory/README +++ b/contracts/test/dao-test-custom-factory/README @@ -1,2 +1,2 @@ # Test Custom Factory contract -Used for testing custom factories with `dao-voting-token-staked`. +Used for testing custom factories with `dao-voting-token-staked` and `dao-voting-cw721-staked`. This also serves an example for how to build custom factory contracts for token or NFT based DAOs. diff --git a/contracts/test/dao-test-custom-factory/src/contract.rs b/contracts/test/dao-test-custom-factory/src/contract.rs index 96127718f..7091c027a 100644 --- a/contracts/test/dao-test-custom-factory/src/contract.rs +++ b/contracts/test/dao-test-custom-factory/src/contract.rs @@ -1,11 +1,13 @@ #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - to_binary, Addr, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg, - Uint128, WasmMsg, + to_binary, Addr, Binary, CosmosMsg, Deps, DepsMut, Empty, Env, MessageInfo, Reply, Response, + StdResult, SubMsg, Uint128, WasmMsg, }; use cw2::set_contract_version; +use cw721::{Cw721QueryMsg, NumTokensResponse}; use cw721_base::InstantiateMsg as Cw721InstantiateMsg; +use cw_ownable::Ownership; use cw_storage_plus::Item; use cw_tokenfactory_issuer::msg::{ ExecuteMsg as IssuerExecuteMsg, InstantiateMsg as IssuerInstantiateMsg, @@ -13,6 +15,7 @@ use cw_tokenfactory_issuer::msg::{ use cw_utils::{one_coin, parse_reply_instantiate_data}; use dao_interface::{ nft::NftFactoryCallback, + state::ModuleInstantiateCallback, token::{InitialBalance, NewTokenInfo, TokenFactoryCallback}, voting::{ActiveThresholdQuery, Query as VotingModuleQueryMsg}, }; @@ -33,6 +36,8 @@ const INSTANTIATE_ISSUER_REPLY_ID: u64 = 1; const INSTANTIATE_NFT_REPLY_ID: u64 = 2; const DAO: Item = Item::new("dao"); +const INITIAL_NFTS: Item> = Item::new("initial_nfts"); +const NFT_CONTRACT: Item = Item::new("nft_contract"); const VOTING_MODULE: Item = Item::new("voting_module"); const TOKEN_INFO: Item = Item::new("token_info"); @@ -59,11 +64,27 @@ pub fn execute( ExecuteMsg::NftFactory { code_id, cw721_instantiate_msg, - } => execute_nft_factory(deps, env, info, cw721_instantiate_msg, code_id), + initial_nfts, + } => execute_nft_factory( + deps, + env, + info, + cw721_instantiate_msg, + code_id, + initial_nfts, + ), ExecuteMsg::NftFactoryWithFunds { code_id, cw721_instantiate_msg, - } => execute_nft_factory_with_funds(deps, env, info, cw721_instantiate_msg, code_id), + initial_nfts, + } => execute_nft_factory_with_funds( + deps, + env, + info, + cw721_instantiate_msg, + code_id, + initial_nfts, + ), ExecuteMsg::NftFactoryNoCallback {} => execute_nft_factory_no_callback(deps, env, info), ExecuteMsg::NftFactoryWrongCallback {} => { execute_nft_factory_wrong_callback(deps, env, info) @@ -80,6 +101,7 @@ pub fn execute( ExecuteMsg::TokenFactoryFactoryWrongCallback {} => { execute_token_factory_factory_wrong_callback(deps, env, info) } + ExecuteMsg::ValidateNftDao {} => execute_validate_nft_dao(deps, info), } } @@ -92,7 +114,11 @@ pub fn execute_nft_factory( info: MessageInfo, cw721_instantiate_msg: Cw721InstantiateMsg, code_id: u64, + initial_nfts: Vec, ) -> Result { + // Save voting module address + VOTING_MODULE.save(deps.storage, &info.sender)?; + // Query for DAO let dao: Addr = deps .querier @@ -101,13 +127,23 @@ pub fn execute_nft_factory( // Save DAO and TOKEN_INFO for use in replies DAO.save(deps.storage, &dao)?; + // Save initial NFTs for use in replies + INITIAL_NFTS.save(deps.storage, &initial_nfts)?; + + // Override minter to be the DAO address + let msg = to_binary(&Cw721InstantiateMsg { + name: cw721_instantiate_msg.name, + symbol: cw721_instantiate_msg.symbol, + minter: dao.to_string(), + })?; + // Instantiate new contract, further setup is handled in the // SubMsg reply. let msg = SubMsg::reply_on_success( WasmMsg::Instantiate { admin: Some(dao.to_string()), code_id, - msg: to_binary(&cw721_instantiate_msg)?, + msg, funds: vec![], label: "cw_tokenfactory_issuer".to_string(), }, @@ -124,11 +160,19 @@ pub fn execute_nft_factory_with_funds( info: MessageInfo, cw721_instantiate_msg: Cw721InstantiateMsg, code_id: u64, + initial_nfts: Vec, ) -> Result { // Validate one coin was sent one_coin(&info)?; - execute_nft_factory(deps, env, info, cw721_instantiate_msg, code_id) + execute_nft_factory( + deps, + env, + info, + cw721_instantiate_msg, + code_id, + initial_nfts, + ) } /// No callback for testing @@ -149,6 +193,7 @@ pub fn execute_nft_factory_wrong_callback( Ok(Response::new().set_data(to_binary(&TokenFactoryCallback { denom: "wrong".to_string(), token_contract: None, + module_instantiate_callback: None, })?)) } @@ -221,9 +266,59 @@ pub fn execute_token_factory_factory_wrong_callback( ) -> Result { Ok(Response::new().set_data(to_binary(&NftFactoryCallback { nft_contract: "nope".to_string(), + module_instantiate_callback: None, })?)) } +/// Example method called in the ModuleInstantiateCallback providing +/// an example for checking the DAO has been setup correctly. +pub fn execute_validate_nft_dao( + deps: DepsMut, + info: MessageInfo, +) -> Result { + // Load the collection and voting module address + let collection_addr = NFT_CONTRACT.load(deps.storage)?; + let voting_module = VOTING_MODULE.load(deps.storage)?; + + // Query the collection owner and check that it's the DAO. + let owner: Ownership = deps.querier.query_wasm_smart( + collection_addr.clone(), + &cw721_base::msg::QueryMsg::::Ownership {}, + )?; + match owner.owner { + Some(owner) => { + if owner != info.sender { + return Err(ContractError::Unauthorized {}); + } + } + None => return Err(ContractError::Unauthorized {}), + } + + // Query the total supply of the NFT contract + let nft_supply: NumTokensResponse = deps + .querier + .query_wasm_smart(collection_addr.clone(), &Cw721QueryMsg::NumTokens {})?; + + // Check greater than zero + if nft_supply.count == 0 { + return Err(ContractError::NoInitialNfts {}); + } + + // Query active threshold + let active_threshold: ActiveThresholdResponse = deps + .querier + .query_wasm_smart(voting_module, &ActiveThresholdQuery::ActiveThreshold {})?; + + // If Active Threshold absolute count is configured, + // check the count is not greater than supply. + // Percentage is validated in the voting module contract. + if let Some(ActiveThreshold::AbsoluteCount { count }) = active_threshold.active_threshold { + assert_valid_absolute_count_threshold(count, Uint128::new(nft_supply.count.into()))?; + } + + Ok(Response::new()) +} + #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { @@ -321,7 +416,6 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result Result { // Parse nft address from instantiate reply let nft_address = parse_reply_instantiate_data(msg)?.contract_address; + // Save NFT contract for use in validation reply + NFT_CONTRACT.save(deps.storage, &deps.api.addr_validate(&nft_address)?)?; + + let initial_nfts = INITIAL_NFTS.load(deps.storage)?; + + // Add mint messages that will be called by the DAO in the + // ModuleInstantiateCallback + let mut msgs: Vec = initial_nfts + .iter() + .flat_map(|nft| -> Result { + Ok(CosmosMsg::Wasm(WasmMsg::Execute { + contract_addr: nft_address.clone(), + funds: vec![], + msg: nft.clone(), + })) + }) + .collect::>(); + + // Clear space + INITIAL_NFTS.remove(deps.storage); + + // After DAO mints NFT, it calls back into the factory contract + // To validate the setup. NOTE: other patterns could be used for this + // but factory contracts SHOULD validate setups. + msgs.push(CosmosMsg::Wasm(WasmMsg::Execute { + contract_addr: env.contract.address.to_string(), + msg: to_binary(&ExecuteMsg::ValidateNftDao {})?, + funds: vec![], + })); + + // Responses for `dao-voting-cw721-staked` MUST include a + // NftFactoryCallback. Ok(Response::new().set_data(to_binary(&NftFactoryCallback { nft_contract: nft_address.to_string(), + module_instantiate_callback: Some(ModuleInstantiateCallback { msgs }), })?)) } _ => Err(ContractError::UnknownReplyId { id: msg.id }), diff --git a/contracts/test/dao-test-custom-factory/src/error.rs b/contracts/test/dao-test-custom-factory/src/error.rs index 4a29782c7..f7d753d54 100644 --- a/contracts/test/dao-test-custom-factory/src/error.rs +++ b/contracts/test/dao-test-custom-factory/src/error.rs @@ -17,6 +17,9 @@ pub enum ContractError { #[error(transparent)] PaymentError(#[from] PaymentError), + #[error("New NFT contract must be instantiated with at least one NFT")] + NoInitialNfts {}, + #[error("Unauthorized")] Unauthorized {}, diff --git a/contracts/test/dao-test-custom-factory/src/msg.rs b/contracts/test/dao-test-custom-factory/src/msg.rs index 5d9e9f8e0..42128cd2e 100644 --- a/contracts/test/dao-test-custom-factory/src/msg.rs +++ b/contracts/test/dao-test-custom-factory/src/msg.rs @@ -1,4 +1,5 @@ use cosmwasm_schema::{cw_serde, QueryResponses}; +use cosmwasm_std::Binary; use cw721_base::InstantiateMsg as Cw721InstantiateMsg; use dao_interface::token::NewTokenInfo; @@ -7,20 +8,32 @@ pub struct InstantiateMsg {} #[cw_serde] pub enum ExecuteMsg { - TokenFactoryFactory(NewTokenInfo), - TokenFactoryFactoryWithFunds(NewTokenInfo), - TokenFactoryFactoryNoCallback {}, - TokenFactoryFactoryWrongCallback {}, + /// Example NFT factory implementation NftFactory { code_id: u64, cw721_instantiate_msg: Cw721InstantiateMsg, + initial_nfts: Vec, }, + /// Example NFT factory implentation that execpts funds NftFactoryWithFunds { code_id: u64, cw721_instantiate_msg: Cw721InstantiateMsg, + initial_nfts: Vec, }, + /// Used for testing no callback NftFactoryNoCallback {}, + /// Used for testing wrong callback NftFactoryWrongCallback {}, + /// Example Factory Implementation + TokenFactoryFactory(NewTokenInfo), + /// Example Factory Implementation that accepts funds + TokenFactoryFactoryWithFunds(NewTokenInfo), + /// Used for testing no callback + TokenFactoryFactoryNoCallback {}, + /// Used for testing wrong callback + TokenFactoryFactoryWrongCallback {}, + /// Validate NFT DAO + ValidateNftDao {}, } #[cw_serde] diff --git a/contracts/voting/dao-voting-cw721-staked/README.md b/contracts/voting/dao-voting-cw721-staked/README.md index 8ea1d05e7..595faec5f 100644 --- a/contracts/voting/dao-voting-cw721-staked/README.md +++ b/contracts/voting/dao-voting-cw721-staked/README.md @@ -13,4 +13,6 @@ To support Stargaze NFTs and other custom NFT contracts or setups with minters ( **NOTE:** when using the factory pattern, it is important to only use a trusted factory contract, as all validation happens in the factory contract. +Those implementing custom factory contracts MUST handle any validation that is to happen, and the custom `WasmMsg::Execute` message MUST include `NftFactoryCallback` data respectively. + The [dao-test-custom-factory contract](../test/dao-test-custom-factory) provides an example of how this can be done and is used for tests. It is NOT production ready, but meant to serve as an example for building factory contracts. diff --git a/contracts/voting/dao-voting-cw721-staked/src/contract.rs b/contracts/voting/dao-voting-cw721-staked/src/contract.rs index 202340f52..982d40e78 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/contract.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/contract.rs @@ -175,6 +175,9 @@ pub fn instantiate( }; CONFIG.save(deps.storage, &config)?; + // Call factory contract. Use only a trusted factory contract, + // as this is a critical security component and valdiation of + // setup will happen in the factory. Ok(Response::new() .add_attribute("action", "intantiate") .add_submessage(SubMsg::reply_on_success( @@ -779,7 +782,16 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result Err(ContractError::NoFactoryCallback {}), } diff --git a/contracts/voting/dao-voting-cw721-staked/src/testing/tests.rs b/contracts/voting/dao-voting-cw721-staked/src/testing/tests.rs index 761619dda..2f2ada85d 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/testing/tests.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/testing/tests.rs @@ -1162,6 +1162,7 @@ fn test_factory() { symbol: "TEST".to_string(), minter: CREATOR_ADDR.to_string(), }, + initial_nfts: vec![], }) .unwrap(), funds: vec![], @@ -1225,6 +1226,13 @@ fn test_factory_with_funds_pass_through() { symbol: "TEST".to_string(), minter: CREATOR_ADDR.to_string(), }, + initial_nfts: vec![to_binary(&Cw721ExecuteMsg::::Mint { + owner: CREATOR_ADDR.to_string(), + token_uri: Some("https://example.com".to_string()), + token_id: "1".to_string(), + extension: Empty {}, + }) + .unwrap()], }, ) .unwrap(), @@ -1263,6 +1271,13 @@ fn test_factory_with_funds_pass_through() { symbol: "TEST".to_string(), minter: CREATOR_ADDR.to_string(), }, + initial_nfts: vec![to_binary(&Cw721ExecuteMsg::::Mint { + owner: CREATOR_ADDR.to_string(), + token_uri: Some("https://example.com".to_string()), + token_id: "1".to_string(), + extension: Empty {}, + }) + .unwrap()], }, ) .unwrap(), @@ -1304,6 +1319,7 @@ fn test_unsupported_factory_msg() { symbol: "TEST".to_string(), minter: CREATOR_ADDR.to_string(), }, + initial_nfts: vec![], }) .unwrap(), admin: None, diff --git a/contracts/voting/dao-voting-token-staked/README.md b/contracts/voting/dao-voting-token-staked/README.md index 2f1b9d693..288a6cda8 100644 --- a/contracts/voting/dao-voting-token-staked/README.md +++ b/contracts/voting/dao-voting-token-staked/README.md @@ -87,4 +87,6 @@ The `factory` pattern takes a single `WasmMsg::Execute` message that calls into **NOTE:** when using the factory pattern, it is important to only use a trusted factory contract, as all validation happens in the factory contract. +Those implementing custom factory contracts MUST handle any validation that is to happen, and the custom `WasmMsg::Execute` message MUST include `TokenFactoryCallback` data respectively. + The [dao-test-custom-factory contract](../test/dao-test-custom-factory) provides an example of how this can be done and is used for tests. It is NOT production ready, but meant to serve as an example for building factory contracts. diff --git a/contracts/voting/dao-voting-token-staked/src/contract.rs b/contracts/voting/dao-voting-token-staked/src/contract.rs index 1fb2ec411..93bb5c8e3 100644 --- a/contracts/voting/dao-voting-token-staked/src/contract.rs +++ b/contracts/voting/dao-voting-token-staked/src/contract.rs @@ -131,17 +131,22 @@ pub fn instantiate( msg, contract_addr, funds, - } => Ok(Response::new() - .add_attribute("action", "intantiate") - .add_attribute("token", "custom_factory") - .add_submessage(SubMsg::reply_on_success( - WasmMsg::Execute { - contract_addr, - msg, - funds, - }, - FACTORY_EXECUTE_REPLY_ID, - ))), + } => { + // Call factory contract. Use only a trusted factory contract, + // as this is a critical security component and valdiation of + // setup will happen in the factory. + Ok(Response::new() + .add_attribute("action", "intantiate") + .add_attribute("token", "custom_factory") + .add_submessage(SubMsg::reply_on_success( + WasmMsg::Execute { + contract_addr, + msg, + funds, + }, + FACTORY_EXECUTE_REPLY_ID, + ))) + } _ => Err(ContractError::UnsupportedFactoryMsg {}), }, } @@ -735,9 +740,18 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result Err(ContractError::NoFactoryCallback {}), } diff --git a/contracts/voting/dao-voting-token-staked/src/tests/test_tube/integration_tests.rs b/contracts/voting/dao-voting-token-staked/src/tests/test_tube/integration_tests.rs index ed1441e23..07ef96891 100644 --- a/contracts/voting/dao-voting-token-staked/src/tests/test_tube/integration_tests.rs +++ b/contracts/voting/dao-voting-token-staked/src/tests/test_tube/integration_tests.rs @@ -1,12 +1,13 @@ use cosmwasm_std::{to_binary, Addr, Coin, Decimal, Uint128, WasmMsg}; -use cw_tokenfactory_issuer::msg::DenomUnit; +use cw_ownable::Ownership; +use cw_tokenfactory_issuer::msg::{DenomUnit, QueryMsg as IssuerQueryMsg}; use cw_utils::Duration; use dao_interface::{ msg::QueryMsg as DaoQueryMsg, state::{Admin, ModuleInstantiateInfo}, token::{InitialBalance, NewDenomMetadata, NewTokenInfo}, }; -use dao_testing::test_tube::dao_dao_core::DaoCore; +use dao_testing::test_tube::{cw_tokenfactory_issuer::TokenfactoryIssuer, dao_dao_core::DaoCore}; use dao_voting::{ pre_propose::PreProposeInfo, threshold::{ActiveThreshold, ActiveThresholdError, PercentageThreshold, Threshold}, @@ -420,6 +421,15 @@ fn test_factory() { // Check the TF denom is as expected assert_eq!(denom, format!("factory/{}/{}", token_contract, DENOM)); + + // Check issuer ownership is the DAO and the ModuleInstantiateCallback + // has successfully accepted ownership. + let issuer = + TokenfactoryIssuer::new_with_values(&app, tf_issuer.code_id, token_contract.to_string()) + .unwrap(); + let ownership: Ownership = issuer.query(&IssuerQueryMsg::Ownership {}).unwrap(); + let owner = ownership.owner.unwrap(); + assert_eq!(owner, dao.contract_addr); } #[test] @@ -628,7 +638,7 @@ fn test_factory_no_callback() { initial_items: None, }; - // Instantiate DAO fails because no funds to create the token were sent + // Instantiate DAO fails because no callback let err = DaoCore::new(&app, &msg, &accounts[0], &vec![]).unwrap_err(); // Check error is no reply data @@ -709,14 +719,14 @@ fn test_factory_wrong_callback() { initial_items: None, }; - // Instantiate DAO fails because no funds to create the token were sent + // Instantiate DAO fails because of wrong callback let err = DaoCore::new(&app, &msg, &accounts[0], &vec![]).unwrap_err(); // Check error is wrong reply type assert_eq!( err, RunnerError::ExecuteError { - msg: "failed to execute message; message index: 0: dispatch: submessages: dispatch: submessages: reply: Error parsing into type dao_interface::token::TokenFactoryCallback: unknown field `nft_contract`, expected `denom` or `token_contract`: execute wasm contract failed".to_string(), + msg: "failed to execute message; message index: 0: dispatch: submessages: dispatch: submessages: reply: Error parsing into type dao_interface::token::TokenFactoryCallback: unknown field `nft_contract`, expected one of `denom`, `token_contract`, `module_instantiate_callback`: execute wasm contract failed".to_string(), } ); } diff --git a/packages/dao-interface/src/nft.rs b/packages/dao-interface/src/nft.rs index 4f7281734..82e7da52e 100644 --- a/packages/dao-interface/src/nft.rs +++ b/packages/dao-interface/src/nft.rs @@ -1,6 +1,9 @@ use cosmwasm_schema::cw_serde; +use crate::state::ModuleInstantiateCallback; + #[cw_serde] pub struct NftFactoryCallback { pub nft_contract: String, + pub module_instantiate_callback: Option, } diff --git a/packages/dao-interface/src/token.rs b/packages/dao-interface/src/token.rs index 3bd8e6875..c735a15c4 100644 --- a/packages/dao-interface/src/token.rs +++ b/packages/dao-interface/src/token.rs @@ -5,6 +5,8 @@ use cosmwasm_std::Uint128; // We re-export them here for convenience. pub use osmosis_std::types::cosmos::bank::v1beta1::{DenomUnit, Metadata}; +use crate::state::ModuleInstantiateCallback; + #[cw_serde] pub struct InitialBalance { pub amount: Uint128, @@ -46,4 +48,5 @@ pub struct NewTokenInfo { pub struct TokenFactoryCallback { pub denom: String, pub token_contract: Option, + pub module_instantiate_callback: Option, }