From 31c5fcc153a11ac992df955c44a0d149ec8d243a Mon Sep 17 00:00:00 2001 From: ismellike Date: Fri, 6 Sep 2024 04:16:57 -0500 Subject: [PATCH] Add support for message forwarding from proposal executes --- contracts/dao-dao-core/src/contract.rs | 36 ++++++++---- .../external/dao-migrator/src/contract.rs | 4 +- .../dao-pre-propose-approver/src/contract.rs | 4 +- .../dao-proposal-condorcet/src/contract.rs | 29 ++++++---- .../dao-proposal-condorcet/src/proposal.rs | 6 +- .../dao-proposal-multiple/src/contract.rs | 56 ++++++++++--------- .../dao-proposal-single/src/contract.rs | 53 ++++++++++-------- .../dao-test-custom-factory/src/contract.rs | 12 ++-- .../dao-voting-cw721-staked/src/contract.rs | 6 +- .../dao-voting-token-staked/src/contract.rs | 6 +- .../src/tests/test_tube/integration_tests.rs | 2 +- packages/dao-interface/src/nft.rs | 4 +- packages/dao-interface/src/state.rs | 4 +- packages/dao-interface/src/token.rs | 4 +- packages/dao-voting/src/reply.rs | 12 ++-- 15 files changed, 136 insertions(+), 102 deletions(-) diff --git a/contracts/dao-dao-core/src/contract.rs b/contracts/dao-dao-core/src/contract.rs index 1ee52a165..6ba3de5c1 100644 --- a/contracts/dao-dao-core/src/contract.rs +++ b/contracts/dao-dao-core/src/contract.rs @@ -2,7 +2,7 @@ use cosmwasm_std::entry_point; use cosmwasm_std::{ from_json, to_json_binary, Addr, Binary, CosmosMsg, Deps, DepsMut, Empty, Env, MessageInfo, - Order, Reply, Response, StdError, StdResult, SubMsg, WasmMsg, + Order, Reply, Response, StdError, StdResult, SubMsg, SubMsgResult, WasmMsg, }; use cw2::{get_contract_version, set_contract_version, ContractVersion}; use cw_paginate_storage::{paginate_map, paginate_map_keys, paginate_map_values}; @@ -15,7 +15,7 @@ use dao_interface::{ GetItemResponse, PauseInfoResponse, ProposalModuleCountResponse, SubDao, }, state::{ - Admin, Config, ModuleInstantiateCallback, ModuleInstantiateInfo, ProposalModule, + Admin, CallbackMessages, Config, ModuleInstantiateInfo, ProposalModule, ProposalModuleStatus, }, voting, @@ -30,9 +30,10 @@ use crate::state::{ pub(crate) const CONTRACT_NAME: &str = "crates.io:dao-dao-core"; pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); -const PROPOSAL_MODULE_REPLY_ID: u64 = 0; +const PROPOSAL_MODULE_INSTANTIATE_REPLY_ID: u64 = 0; const VOTE_MODULE_INSTANTIATE_REPLY_ID: u64 = 1; const VOTE_MODULE_UPDATE_REPLY_ID: u64 = 2; +const PROPOSAL_MODULE_EXECUTE_REPLY_ID: u64 = 3; #[cfg_attr(not(feature = "library"), entry_point)] pub fn instantiate( @@ -71,7 +72,7 @@ pub fn instantiate( .proposal_modules_instantiate_info .into_iter() .map(|info| info.into_wasm_msg(env.contract.address.clone())) - .map(|wasm| SubMsg::reply_on_success(wasm, PROPOSAL_MODULE_REPLY_ID)) + .map(|wasm| SubMsg::reply_on_success(wasm, PROPOSAL_MODULE_INSTANTIATE_REPLY_ID)) .collect(); if proposal_module_msgs.is_empty() { return Err(ContractError::NoActiveProposalModules {}); @@ -228,7 +229,10 @@ pub fn execute_proposal_hook( Ok(Response::default() .add_attribute("action", "execute_proposal_hook") - .add_messages(msgs)) + .add_submessages( + msgs.into_iter() + .map(|msg| SubMsg::reply_on_success(msg, PROPOSAL_MODULE_EXECUTE_REPLY_ID)), + )) } pub fn execute_nominate_admin( @@ -392,7 +396,7 @@ pub fn execute_update_proposal_modules( let to_add: Vec> = to_add .into_iter() .map(|info| info.into_wasm_msg(env.contract.address.clone())) - .map(|wasm| SubMsg::reply_on_success(wasm, PROPOSAL_MODULE_REPLY_ID)) + .map(|wasm| SubMsg::reply_on_success(wasm, PROPOSAL_MODULE_INSTANTIATE_REPLY_ID)) .collect(); Ok(Response::default() @@ -952,7 +956,7 @@ pub fn migrate(deps: DepsMut, env: Env, msg: MigrateMsg) -> Result Result { match msg.id { - PROPOSAL_MODULE_REPLY_ID => { + PROPOSAL_MODULE_INSTANTIATE_REPLY_ID => { let res = parse_reply_instantiate_data(msg)?; let prop_module_addr = deps.api.addr_validate(&res.contract_address)?; let total_module_count = TOTAL_PROPOSAL_MODULE_COUNT.load(deps.storage)?; @@ -973,7 +977,7 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result from_json::(&data) + Some(data) => from_json::(&data) .map(|m| m.msgs) .unwrap_or_else(|_| vec![]), None => vec![], @@ -983,7 +987,6 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result { let res = parse_reply_instantiate_data(msg)?; let vote_module_addr = deps.api.addr_validate(&res.contract_address)?; @@ -999,7 +1002,7 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result from_json::(&data) + Some(data) => from_json::(&data) .map(|m| m.msgs) .unwrap_or_else(|_| vec![]), None => vec![], @@ -1017,6 +1020,19 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result match msg.result { + SubMsgResult::Ok(res) => { + let callback_msgs = match res.data { + Some(data) => from_json::(&data) + .map(|m| m.msgs) + .unwrap_or_else(|_| vec![]), + None => vec![], + }; + + Ok(Response::default().add_messages(callback_msgs)) + } + SubMsgResult::Err(_) => Ok(Response::default()), + }, _ => Err(ContractError::UnknownReplyID {}), } } diff --git a/contracts/external/dao-migrator/src/contract.rs b/contracts/external/dao-migrator/src/contract.rs index 625d0276b..c99988268 100644 --- a/contracts/external/dao-migrator/src/contract.rs +++ b/contracts/external/dao-migrator/src/contract.rs @@ -9,7 +9,7 @@ use cosmwasm_std::{ use cw2::set_contract_version; use dao_interface::{ query::SubDao, - state::{ModuleInstantiateCallback, ProposalModule}, + state::{CallbackMessages, ProposalModule}, }; use crate::{ @@ -42,7 +42,7 @@ pub fn instantiate( CORE_ADDR.save(deps.storage, &info.sender)?; Ok( - Response::default().set_data(to_json_binary(&ModuleInstantiateCallback { + Response::default().set_data(to_json_binary(&CallbackMessages { msgs: vec![WasmMsg::Execute { contract_addr: env.contract.address.to_string(), msg: to_json_binary(&MigrateV1ToV2 { diff --git a/contracts/pre-propose/dao-pre-propose-approver/src/contract.rs b/contracts/pre-propose/dao-pre-propose-approver/src/contract.rs index 268d3bae8..321098c53 100644 --- a/contracts/pre-propose/dao-pre-propose-approver/src/contract.rs +++ b/contracts/pre-propose/dao-pre-propose-approver/src/contract.rs @@ -6,7 +6,7 @@ use cosmwasm_std::{ }; use cw2::set_contract_version; -use dao_interface::state::ModuleInstantiateCallback; +use dao_interface::state::CallbackMessages; use dao_pre_propose_approval_single::msg::{ ApproverProposeMessage, ExecuteExt as ApprovalExt, ExecuteMsg as PreProposeApprovalExecuteMsg, }; @@ -59,7 +59,7 @@ pub fn instantiate( let addr = deps.api.addr_validate(&msg.pre_propose_approval_contract)?; PRE_PROPOSE_APPROVAL_CONTRACT.save(deps.storage, &addr)?; - Ok(resp.set_data(to_json_binary(&ModuleInstantiateCallback { + Ok(resp.set_data(to_json_binary(&CallbackMessages { msgs: vec![ CosmosMsg::Wasm(WasmMsg::Execute { contract_addr: addr.to_string(), diff --git a/contracts/proposal/dao-proposal-condorcet/src/contract.rs b/contracts/proposal/dao-proposal-condorcet/src/contract.rs index 3d3785002..cec1c673e 100644 --- a/contracts/proposal/dao-proposal-condorcet/src/contract.rs +++ b/contracts/proposal/dao-proposal-condorcet/src/contract.rs @@ -2,6 +2,7 @@ use cosmwasm_std::entry_point; use cosmwasm_std::{ to_json_binary, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, + SubMsgResult, }; use cw2::set_contract_version; @@ -267,18 +268,24 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult { pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result { let repl = TaggedReplyId::new(msg.id)?; match repl { - TaggedReplyId::FailedProposalExecution(proposal_id) => { - let mut proposal = PROPOSAL.load(deps.storage, proposal_id as u32)?; - proposal.set_execution_failed(); - PROPOSAL.save(deps.storage, proposal_id as u32, &proposal)?; + TaggedReplyId::ProposalExecution(proposal_id) => match msg.result { + SubMsgResult::Ok(res) => match res.data { + Some(data) => Ok(Response::new() + .add_attribute("proposal_execution_success", proposal_id.to_string()) + .set_data(data)), + None => Ok(Response::new() + .add_attribute("proposal_execution_success", proposal_id.to_string())), + }, + SubMsgResult::Err(error) => { + let mut proposal = PROPOSAL.load(deps.storage, proposal_id as u32)?; + proposal.set_execution_failed(); + PROPOSAL.save(deps.storage, proposal_id as u32, &proposal)?; - Ok(Response::default() - .add_attribute("proposal_execution_failed", proposal_id.to_string()) - .add_attribute( - "error", - msg.result.into_result().err().unwrap_or("None".to_string()), - )) - } + Ok(Response::new() + .add_attribute("proposal_execution_failed", proposal_id.to_string()) + .add_attribute("error", error)) + } + }, _ => unimplemented!("pre-propose and hooks not yet supported"), } } diff --git a/contracts/proposal/dao-proposal-condorcet/src/proposal.rs b/contracts/proposal/dao-proposal-condorcet/src/proposal.rs index bd2f82486..f0acea111 100644 --- a/contracts/proposal/dao-proposal-condorcet/src/proposal.rs +++ b/contracts/proposal/dao-proposal-condorcet/src/proposal.rs @@ -164,11 +164,11 @@ impl Proposal { msg: to_json_binary(&dao_interface::msg::ExecuteMsg::ExecuteProposalHook { msgs })?, funds: vec![], }; + let masked_id = mask_proposal_execution_proposal_id(self.id as u64); Ok(if self.close_on_execution_failure { - let masked_id = mask_proposal_execution_proposal_id(self.id as u64); - SubMsg::reply_on_error(core_exec, masked_id) + SubMsg::reply_always(core_exec, masked_id) } else { - SubMsg::new(core_exec) + SubMsg::reply_on_success(core_exec, masked_id) }) } diff --git a/contracts/proposal/dao-proposal-multiple/src/contract.rs b/contracts/proposal/dao-proposal-multiple/src/contract.rs index 6fc42f899..da318c3f1 100644 --- a/contracts/proposal/dao-proposal-multiple/src/contract.rs +++ b/contracts/proposal/dao-proposal-multiple/src/contract.rs @@ -2,7 +2,7 @@ use cosmwasm_std::entry_point; use cosmwasm_std::{ to_json_binary, Addr, Attribute, Binary, Deps, DepsMut, Empty, Env, MessageInfo, Order, Reply, - Response, StdResult, Storage, SubMsg, WasmMsg, + Response, StdResult, Storage, SubMsg, SubMsgResult, WasmMsg, }; use cw2::set_contract_version; @@ -545,15 +545,14 @@ pub fn execute_execute( })?, funds: vec![], }; + let masked_proposal_id = mask_proposal_execution_proposal_id(proposal_id); match config.close_proposal_on_execution_failure { - true => { - let masked_proposal_id = mask_proposal_execution_proposal_id(proposal_id); - Response::default().add_submessage(SubMsg::reply_on_error( - execute_message, - masked_proposal_id, - )) - } - false => Response::default().add_message(execute_message), + true => Response::default() + .add_submessage(SubMsg::reply_always(execute_message, masked_proposal_id)), + false => Response::default().add_submessage(SubMsg::reply_on_success( + execute_message, + masked_proposal_id, + )), } } else { Response::default() @@ -986,22 +985,29 @@ pub fn query_info(deps: Deps) -> StdResult { pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result { let repl = TaggedReplyId::new(msg.id)?; match repl { - TaggedReplyId::FailedProposalExecution(proposal_id) => { - PROPOSALS.update(deps.storage, proposal_id, |prop| match prop { - Some(mut prop) => { - prop.status = Status::ExecutionFailed; - Ok(prop) - } - None => Err(ContractError::NoSuchProposal { id: proposal_id }), - })?; - - Ok(Response::new() - .add_attribute("proposal execution failed", proposal_id.to_string()) - .add_attribute( - "error", - msg.result.into_result().err().unwrap_or("None".to_string()), - )) - } + TaggedReplyId::ProposalExecution(proposal_id) => match msg.result { + SubMsgResult::Ok(res) => match res.data { + Some(data) => Ok(Response::new() + .add_attribute("proposal_execution_success", proposal_id.to_string()) + .set_data(data)), + None => Ok(Response::new() + .add_attribute("proposal_execution_success", proposal_id.to_string())), + }, + SubMsgResult::Err(error) => { + PROPOSALS.update(deps.storage, proposal_id, |prop| match prop { + Some(mut prop) => { + prop.status = Status::ExecutionFailed; + + Ok(prop) + } + None => Err(ContractError::NoSuchProposal { id: proposal_id }), + })?; + + Ok(Response::new() + .add_attribute("proposal_execution_failed", proposal_id.to_string()) + .add_attribute("error", error)) + } + }, TaggedReplyId::FailedProposalHook(idx) => { let addr = PROPOSAL_HOOKS.remove_hook_by_index(deps.storage, idx)?; Ok(Response::new().add_attribute("removed_proposal_hook", format!("{addr}:{idx}"))) diff --git a/contracts/proposal/dao-proposal-single/src/contract.rs b/contracts/proposal/dao-proposal-single/src/contract.rs index 1943081d1..e5a1fa905 100644 --- a/contracts/proposal/dao-proposal-single/src/contract.rs +++ b/contracts/proposal/dao-proposal-single/src/contract.rs @@ -2,7 +2,7 @@ use cosmwasm_std::entry_point; use cosmwasm_std::{ to_json_binary, Addr, Attribute, Binary, Deps, DepsMut, Env, MessageInfo, Order, Reply, - Response, StdResult, Storage, SubMsg, WasmMsg, + Response, StdResult, Storage, SubMsg, SubMsgResult, WasmMsg, }; use cw2::{get_contract_version, set_contract_version, ContractVersion}; use cw_hooks::Hooks; @@ -433,13 +433,14 @@ pub fn execute_execute( })?, funds: vec![], }; + let masked_proposal_id = mask_proposal_execution_proposal_id(proposal_id); match config.close_proposal_on_execution_failure { - true => { - let masked_proposal_id = mask_proposal_execution_proposal_id(proposal_id); - Response::default() - .add_submessage(SubMsg::reply_on_error(execute_message, masked_proposal_id)) - } - false => Response::default().add_message(execute_message), + true => Response::default() + .add_submessage(SubMsg::reply_always(execute_message, masked_proposal_id)), + false => Response::default().add_submessage(SubMsg::reply_on_success( + execute_message, + masked_proposal_id, + )), } } else { Response::default() @@ -1069,23 +1070,29 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result Result { let repl = TaggedReplyId::new(msg.id)?; match repl { - TaggedReplyId::FailedProposalExecution(proposal_id) => { - PROPOSALS.update(deps.storage, proposal_id, |prop| match prop { - Some(mut prop) => { - prop.status = Status::ExecutionFailed; + TaggedReplyId::ProposalExecution(proposal_id) => match msg.result { + SubMsgResult::Ok(res) => match res.data { + Some(data) => Ok(Response::new() + .add_attribute("proposal_execution_success", proposal_id.to_string()) + .set_data(data)), + None => Ok(Response::new() + .add_attribute("proposal_execution_success", proposal_id.to_string())), + }, + SubMsgResult::Err(error) => { + PROPOSALS.update(deps.storage, proposal_id, |prop| match prop { + Some(mut prop) => { + prop.status = Status::ExecutionFailed; + + Ok(prop) + } + None => Err(ContractError::NoSuchProposal { id: proposal_id }), + })?; - Ok(prop) - } - None => Err(ContractError::NoSuchProposal { id: proposal_id }), - })?; - - Ok(Response::new() - .add_attribute("proposal_execution_failed", proposal_id.to_string()) - .add_attribute( - "error", - msg.result.into_result().err().unwrap_or("None".to_string()), - )) - } + Ok(Response::new() + .add_attribute("proposal_execution_failed", proposal_id.to_string()) + .add_attribute("error", error)) + } + }, TaggedReplyId::FailedProposalHook(idx) => { let addr = PROPOSAL_HOOKS.remove_hook_by_index(deps.storage, idx)?; Ok(Response::new().add_attribute("removed_proposal_hook", format!("{addr}:{idx}"))) diff --git a/contracts/test/dao-test-custom-factory/src/contract.rs b/contracts/test/dao-test-custom-factory/src/contract.rs index 67ad19686..95a727155 100644 --- a/contracts/test/dao-test-custom-factory/src/contract.rs +++ b/contracts/test/dao-test-custom-factory/src/contract.rs @@ -15,7 +15,7 @@ use cw_tokenfactory_issuer::msg::{ use cw_utils::{one_coin, parse_reply_instantiate_data}; use dao_interface::{ nft::NftFactoryCallback, - state::ModuleInstantiateCallback, + state::CallbackMessages, token::{InitialBalance, NewTokenInfo, TokenFactoryCallback}, voting::{ActiveThresholdQuery, Query as VotingModuleQueryMsg}, }; @@ -274,7 +274,7 @@ pub fn execute_token_factory_factory_wrong_callback( ) } -/// Example method called in the ModuleInstantiateCallback providing +/// Example method called in the CallbackMessages providing /// an example for checking the DAO has been setup correctly. pub fn execute_validate_nft_dao( deps: DepsMut, @@ -432,10 +432,10 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result Result = initial_nfts .iter() .flat_map(|nft| -> Result { @@ -494,7 +494,7 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result Result Result, + pub module_instantiate_callback: Option, } diff --git a/packages/dao-interface/src/state.rs b/packages/dao-interface/src/state.rs index 2022640eb..b19c274ea 100644 --- a/packages/dao-interface/src/state.rs +++ b/packages/dao-interface/src/state.rs @@ -81,9 +81,9 @@ impl ModuleInstantiateInfo { } } -/// Callbacks to be executed when a module is instantiated +/// Callbacks to be executed from response data #[cw_serde] -pub struct ModuleInstantiateCallback { +pub struct CallbackMessages { pub msgs: Vec, } diff --git a/packages/dao-interface/src/token.rs b/packages/dao-interface/src/token.rs index c735a15c4..79570ec93 100644 --- a/packages/dao-interface/src/token.rs +++ b/packages/dao-interface/src/token.rs @@ -5,7 +5,7 @@ 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; +use crate::state::CallbackMessages; #[cw_serde] pub struct InitialBalance { @@ -48,5 +48,5 @@ pub struct NewTokenInfo { pub struct TokenFactoryCallback { pub denom: String, pub token_contract: Option, - pub module_instantiate_callback: Option, + pub module_instantiate_callback: Option, } diff --git a/packages/dao-voting/src/reply.rs b/packages/dao-voting/src/reply.rs index dbf916e2b..8e8b3d0b1 100644 --- a/packages/dao-voting/src/reply.rs +++ b/packages/dao-voting/src/reply.rs @@ -1,6 +1,6 @@ use dao_dao_macros::limit_variant_count; -const FAILED_PROPOSAL_EXECUTION_MASK: u64 = 0b000; +const PROPOSAL_EXECUTION_MASK: u64 = 0b000; const FAILED_PROPOSAL_HOOK_MASK: u64 = 0b001; const FAILED_VOTE_HOOK_MASK: u64 = 0b010; @@ -23,7 +23,7 @@ const REPLY_TYPE_MASK: u64 = (1 << BITS_RESERVED_FOR_REPLY_TYPE) - 1; #[cfg_attr(not(target_arch = "wasm32"), derive(Debug, PartialEq, Eq))] pub enum TaggedReplyId { /// Fired when a proposal's execution fails. - FailedProposalExecution(u64), + ProposalExecution(u64), /// Fired when a proposal hook's execution fails. FailedProposalHook(u64), /// Fired when a vote hook's execution fails. @@ -43,9 +43,7 @@ impl TaggedReplyId { let reply_type = id & REPLY_TYPE_MASK; let id_after_shift = id >> BITS_RESERVED_FOR_REPLY_TYPE; match reply_type { - FAILED_PROPOSAL_EXECUTION_MASK => { - Ok(TaggedReplyId::FailedProposalExecution(id_after_shift)) - } + PROPOSAL_EXECUTION_MASK => Ok(TaggedReplyId::ProposalExecution(id_after_shift)), FAILED_PROPOSAL_HOOK_MASK => Ok(TaggedReplyId::FailedProposalHook(id_after_shift)), FAILED_VOTE_HOOK_MASK => Ok(TaggedReplyId::FailedVoteHook(id_after_shift)), PRE_PROPOSE_MODULE_INSTANTIATION_ID => Ok(TaggedReplyId::PreProposeModuleInstantiation), @@ -57,7 +55,7 @@ impl TaggedReplyId { /// This function can drop bits, if you have more than `u(64-[`BITS_RESERVED_FOR_REPLY_TYPE`])` proposals. pub const fn mask_proposal_execution_proposal_id(proposal_id: u64) -> u64 { - FAILED_PROPOSAL_EXECUTION_MASK | (proposal_id << BITS_RESERVED_FOR_REPLY_TYPE) + PROPOSAL_EXECUTION_MASK | (proposal_id << BITS_RESERVED_FOR_REPLY_TYPE) } pub const fn mask_proposal_hook_index(index: u64) -> u64 { @@ -103,7 +101,7 @@ mod test { assert_eq!( TaggedReplyId::new(m_proposal_id).unwrap(), - TaggedReplyId::FailedProposalExecution(proposal_id_max) + TaggedReplyId::ProposalExecution(proposal_id_max) ); assert_eq!( TaggedReplyId::new(m_proposal_hook_idx).unwrap(),