diff --git a/.github/workflows/framework-test.yml b/.github/workflows/framework-test.yml index 10dcf57c0e..39770f6a32 100644 --- a/.github/workflows/framework-test.yml +++ b/.github/workflows/framework-test.yml @@ -40,6 +40,17 @@ jobs: if: hashFiles('Cargo.lock') == '' working-directory: ./framework run: cargo generate-lockfile + - name: Free up disk space on runner + run: | + df -h + rm -rf /usr/share/dotnet/ + sudo apt-get remove -y 'php.*' + sudo apt-get remove -y '^dotnet-.*' + sudo apt-get remove -y azure-cli firefox powershell mono-devel + sudo apt-get autoremove -y + sudo apt-get clean + df -h + shell: bash - uses: Swatinem/rust-cache@v2 with: workspaces: "framework -> target" diff --git a/framework/Cargo.lock b/framework/Cargo.lock index 61c7d75d05..c2fae45780 100644 --- a/framework/Cargo.lock +++ b/framework/Cargo.lock @@ -158,7 +158,6 @@ dependencies = [ "cw2 2.0.0", "cw20", "cw20-base", - "cw721 0.18.0", "rand", "semver", "serde", diff --git a/framework/contracts/account/src/absacc/auth/mod.rs b/framework/contracts/account/src/absacc/auth/mod.rs index 127558320d..2ebd8ae75d 100644 --- a/framework/contracts/account/src/absacc/auth/mod.rs +++ b/framework/contracts/account/src/absacc/auth/mod.rs @@ -390,11 +390,11 @@ mod test { for id in 0..0b10000000 { let (unmasked_id, admin) = AuthId::new(id, true).unwrap().cred_id(); assert_eq!(id, unmasked_id); - assert_eq!(admin, true); + assert!(admin); let (unmasked_id, admin) = AuthId::new(id, false).unwrap().cred_id(); assert_eq!(id, unmasked_id); - assert_eq!(admin, false); + assert!(!admin); } } } diff --git a/framework/contracts/account/src/absacc/auth/secp256r1.rs b/framework/contracts/account/src/absacc/auth/secp256r1.rs index b4c453335e..a34252ba2b 100644 --- a/framework/contracts/account/src/absacc/auth/secp256r1.rs +++ b/framework/contracts/account/src/absacc/auth/secp256r1.rs @@ -40,23 +40,16 @@ mod tests { let verifying_key_binary = Binary::from(verifying_key_bytes.to_vec()); println!("verifying key: {}", hex::encode(verifying_key_bytes)); - assert_eq!( - true, - verify( - &test_value.to_vec(), - signature_bytes.as_slice(), - &verifying_key_binary, - ) - .unwrap() - ); + assert!(verify( + test_value, + signature_bytes.as_slice(), + &verifying_key_binary, + ) + .unwrap()); // test with invalid msg let bad_value = "invalid starting msg".as_bytes(); - let result = verify( - &bad_value.to_vec(), - signature_bytes.as_slice(), - &verifying_key_binary, - ); + let result = verify(bad_value, signature_bytes.as_slice(), &verifying_key_binary); assert!(result.is_err()) } } diff --git a/framework/contracts/account/src/contract.rs b/framework/contracts/account/src/contract.rs index f78fa3355d..9b3baf980a 100644 --- a/framework/contracts/account/src/contract.rs +++ b/framework/contracts/account/src/contract.rs @@ -23,7 +23,7 @@ use abstract_std::{ }; use cosmwasm_std::{ ensure_eq, wasm_execute, Addr, Binary, Coins, Deps, DepsMut, Env, MessageInfo, Reply, Response, - StdResult, SubMsgResult, + StdResult, }; pub use crate::migrate::migrate; @@ -31,11 +31,11 @@ use crate::{ config::{update_account_status, update_info, update_internal_config}, error::AccountError, execution::{ - add_auth_method, execute_ibc_action, execute_module_action, execute_module_action_response, - ica_action, remove_auth_method, + add_auth_method, admin_execute, admin_execute_on_module, execute_ibc_action, execute_msgs, + execute_msgs_with_data, execute_on_module, ica_action, remove_auth_method, }, modules::{ - _install_modules, exec_on_module, install_modules, + _install_modules, install_modules, migration::{handle_callback, upgrade_modules}, uninstall_module, MIGRATE_CONTEXT, }, @@ -45,7 +45,7 @@ use crate::{ handle_module_info_query, handle_module_versions_query, handle_sub_accounts_query, handle_top_level_owner_query, }, - reply::{forward_response_data, register_dependencies}, + reply::{admin_action_reply, forward_response_reply, register_dependencies}, sub_account::{ create_sub_account, handle_sub_account_action, maybe_update_sub_account_governance, remove_account_from_contracts, @@ -59,8 +59,9 @@ pub type AccountResult = Result; pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); -pub const RESPONSE_REPLY_ID: u64 = 1; -pub const REGISTER_MODULES_DEPENDENCIES_REPLY_ID: u64 = 2; +pub const FORWARD_RESPONSE_REPLY_ID: u64 = 1; +pub const ADMIN_ACTION_REPLY_ID: u64 = 2; +pub const REGISTER_MODULES_DEPENDENCIES_REPLY_ID: u64 = 3; #[cfg_attr(feature = "export", cosmwasm_std::entry_point)] pub fn instantiate( @@ -258,13 +259,39 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg) is_suspended: suspension_status, } => update_account_status(deps, info, suspension_status).map_err(AccountError::from), msg => { - // Block actions if user is not subscribed + // Block actions if account is suspended let is_suspended = SUSPENSION_STATUS.load(deps.storage)?; if is_suspended { return Err(AccountError::AccountSuspended {}); } match msg { + // ## Execution ## + ExecuteMsg::Execute { msgs } => { + execute_msgs(deps, &info.sender, msgs).map_err(AccountError::from) + } + ExecuteMsg::AdminExecute { addr, msg } => { + let addr = deps.api.addr_validate(&addr)?; + admin_execute(deps, info, addr, msg) + } + ExecuteMsg::ExecuteWithData { msg } => { + execute_msgs_with_data(deps, &info.sender, msg).map_err(AccountError::from) + } + ExecuteMsg::ExecuteOnModule { + module_id, + exec_msg, + } => execute_on_module(deps, info, module_id, exec_msg).map_err(AccountError::from), + ExecuteMsg::AdminExecuteOnModule { module_id, msg } => { + admin_execute_on_module(deps, info, module_id, msg) + } + ExecuteMsg::IbcAction { msg } => { + execute_ibc_action(deps, info, msg).map_err(AccountError::from) + } + ExecuteMsg::IcaAction { action_query_msg } => { + ica_action(deps, info, action_query_msg).map_err(AccountError::from) + } + + // ## Configuration ## ExecuteMsg::UpdateInternalConfig(config) => { update_internal_config(deps, info, config).map_err(AccountError::from) } @@ -274,29 +301,6 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg) ExecuteMsg::UninstallModule { module_id } => { uninstall_module(deps, info, module_id).map_err(AccountError::from) } - ExecuteMsg::ExecOnModule { - module_id, - exec_msg, - } => exec_on_module(deps, info, module_id, exec_msg).map_err(AccountError::from), - ExecuteMsg::CreateSubAccount { - name, - description, - link, - namespace, - install_modules, - account_id, - } => create_sub_account( - deps, - info, - env, - name, - description, - link, - namespace, - install_modules, - account_id, - ) - .map_err(AccountError::from), ExecuteMsg::Upgrade { modules } => { upgrade_modules(deps, env, info, modules).map_err(AccountError::from) } @@ -305,12 +309,6 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg) description, link, } => update_info(deps, info, name, description, link).map_err(AccountError::from), - ExecuteMsg::UpdateSubAccount(action) => { - handle_sub_account_action(deps, info, action).map_err(AccountError::from) - } - // TODO: Update module migrate logic to not use callback! - // ExecuteMsg::Callback(CallbackMsg {}) => handle_callback(deps, env, info), - // Used to claim or renounce an ownership change. ExecuteMsg::UpdateOwnership(action) => { // If sub-account related it may require some messages to be constructed beforehand let msgs = match &action { @@ -337,18 +335,33 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg) .add_messages(msgs), ) } - ExecuteMsg::ModuleAction { msgs } => { - execute_module_action(deps, info, msgs).map_err(AccountError::from) - } - ExecuteMsg::ModuleActionWithData { msg } => { - execute_module_action_response(deps, info, msg).map_err(AccountError::from) - } - ExecuteMsg::IbcAction { msg } => { - execute_ibc_action(deps, info, msg).map_err(AccountError::from) - } - ExecuteMsg::IcaAction { action_query_msg } => { - ica_action(deps, info, action_query_msg).map_err(AccountError::from) + + // ## Sub-Accounts ## + ExecuteMsg::CreateSubAccount { + name, + description, + link, + namespace, + install_modules, + account_id, + } => create_sub_account( + deps, + info, + env, + name, + description, + link, + namespace, + install_modules, + account_id, + ) + .map_err(AccountError::from), + ExecuteMsg::UpdateSubAccount(action) => { + handle_sub_account_action(deps, info, action).map_err(AccountError::from) } + + // ## Other ## + // TODO: Update module migrate logic to not use callback! ExecuteMsg::UpdateStatus { is_suspended: _ } => { unreachable!("Update status case is reached above") } @@ -365,17 +378,11 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg) #[cfg_attr(feature = "export", cosmwasm_std::entry_point)] pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> AccountResult { - match msg { - Reply { - id: RESPONSE_REPLY_ID, - result: SubMsgResult::Ok(_), - .. - } => forward_response_data(msg), - Reply { - id: REGISTER_MODULES_DEPENDENCIES_REPLY_ID, - result: SubMsgResult::Ok(_), - .. - } => register_dependencies(deps), + match msg.id { + FORWARD_RESPONSE_REPLY_ID => forward_response_reply(msg), + ADMIN_ACTION_REPLY_ID => admin_action_reply(deps), + REGISTER_MODULES_DEPENDENCIES_REPLY_ID => register_dependencies(deps), + _ => Err(AccountError::UnexpectedReply {}), } } diff --git a/framework/contracts/account/src/execution.rs b/framework/contracts/account/src/execution.rs index 03a7f6329e..3a34a43a51 100644 --- a/framework/contracts/account/src/execution.rs +++ b/framework/contracts/account/src/execution.rs @@ -1,15 +1,20 @@ use abstract_sdk::std::{ account::state::WHITELISTED_MODULES, ibc_client::ExecuteMsg as IbcClientMsg, IBC_CLIENT, }; -use abstract_std::{account::state::ACCOUNT_MODULES, objects::ownership, ICA_CLIENT}; +use abstract_std::{ + account::state::{ACCOUNT_MODULES, CALLING_TO_AS_ADMIN}, + objects::ownership, + ICA_CLIENT, +}; use cosmwasm_std::{ wasm_execute, Addr, Binary, CosmosMsg, DepsMut, Empty, Env, MessageInfo, StdError, SubMsg, - WasmQuery, + WasmMsg, WasmQuery, }; use crate::{ - contract::{AccountResponse, AccountResult, RESPONSE_REPLY_ID}, + contract::{AccountResponse, AccountResult, ADMIN_ACTION_REPLY_ID, FORWARD_RESPONSE_REPLY_ID}, error::AccountError, + modules::load_module_addr, }; /// Check that sender either whitelisted or governance @@ -36,30 +41,82 @@ pub(crate) fn assert_whitelisted_or_owner(deps: &mut DepsMut, sender: &Addr) -> /// Executes `Vec` on the proxy. /// Permission: Module -pub fn execute_module_action( +pub fn execute_msgs( mut deps: DepsMut, - msg_info: MessageInfo, + msg_sender: &Addr, msgs: Vec>, ) -> AccountResult { - assert_whitelisted_or_owner(&mut deps, &msg_info.sender)?; + assert_whitelisted_or_owner(&mut deps, msg_sender)?; Ok(AccountResponse::action("execute_module_action").add_messages(msgs)) } /// Executes `CosmosMsg` on the proxy and forwards its response. /// Permission: Module -pub fn execute_module_action_response( +pub fn execute_msgs_with_data( mut deps: DepsMut, - msg_info: MessageInfo, + msg_sender: &Addr, msg: CosmosMsg, ) -> AccountResult { - assert_whitelisted_or_owner(&mut deps, &msg_info.sender)?; + assert_whitelisted_or_owner(&mut deps, msg_sender)?; - let submsg = SubMsg::reply_on_success(msg, RESPONSE_REPLY_ID); + let submsg = SubMsg::reply_on_success(msg, FORWARD_RESPONSE_REPLY_ID); Ok(AccountResponse::action("execute_module_action_response").add_submessage(submsg)) } +/// Execute the [`exec_msg`] on the provided [`module_id`], +/// This is a simple wrapper around [`ExecuteMsg::Execute`](abstract_std::account::ExecuteMsg::Execute). +pub fn execute_on_module( + deps: DepsMut, + info: MessageInfo, + module_id: String, + exec_msg: Binary, +) -> AccountResult { + let module_addr = load_module_addr(deps.storage, &module_id)?; + execute_msgs( + deps, + &info.sender, + vec![CosmosMsg::Wasm(WasmMsg::Execute { + contract_addr: module_addr.into(), + msg: exec_msg, + funds: info.funds, + })], + ) +} + +pub fn admin_execute( + deps: DepsMut, + info: MessageInfo, + addr: Addr, + exec_msg: Binary, +) -> AccountResult { + ownership::assert_nested_owner(deps.storage, &deps.querier, &info.sender)?; + + CALLING_TO_AS_ADMIN.save(deps.storage, &addr)?; + + let msg = SubMsg::reply_on_success( + WasmMsg::Execute { + contract_addr: addr.to_string(), + msg: exec_msg, + funds: info.funds, + }, + ADMIN_ACTION_REPLY_ID, + ); + + Ok(AccountResponse::action("admin_execute").add_submessage(msg)) +} + +pub fn admin_execute_on_module( + deps: DepsMut, + info: MessageInfo, + module_id: String, + exec_msg: Binary, +) -> AccountResult { + let module_addr = load_module_addr(deps.storage, &module_id)?; + admin_execute(deps, info, module_addr, exec_msg) +} + /// Executes IBC actions on the IBC client. /// Permission: Module pub fn execute_ibc_action( @@ -165,7 +222,7 @@ mod test { let mut deps = mock_dependencies(); mock_init(&mut deps)?; - let msg = ExecuteMsg::ModuleAction { msgs: vec![] }; + let msg = ExecuteMsg::Execute { msgs: vec![] }; let info = message_info(&deps.api.addr_make("not_whitelisted"), &[]); @@ -198,7 +255,7 @@ mod test { )? .into(); - let msg = ExecuteMsg::ModuleAction { + let msg = ExecuteMsg::Execute { msgs: vec![action.clone()], }; diff --git a/framework/contracts/account/src/modules.rs b/framework/contracts/account/src/modules.rs index 5b7199fc7f..c0b21ae079 100644 --- a/framework/contracts/account/src/modules.rs +++ b/framework/contracts/account/src/modules.rs @@ -18,7 +18,7 @@ use abstract_std::{ }; use cosmwasm_std::{ ensure, wasm_execute, Addr, Attribute, Binary, CanonicalAddr, Coin, CosmosMsg, Deps, DepsMut, - MessageInfo, StdError, StdResult, Storage, SubMsg, WasmMsg, + MessageInfo, StdError, StdResult, Storage, SubMsg, }; use cw2::ContractVersion; use cw_storage_plus::Item; @@ -223,29 +223,6 @@ pub fn uninstall_module(mut deps: DepsMut, info: MessageInfo, module_id: String) Ok(response) } -/// Execute the [`exec_msg`] on the provided [`module_id`], -pub fn exec_on_module( - deps: DepsMut, - info: MessageInfo, - module_id: String, - exec_msg: Binary, -) -> AccountResult { - // only owner can forward messages to modules - ownership::assert_nested_owner(deps.storage, &deps.querier, &info.sender)?; - - let module_addr = load_module_addr(deps.storage, &module_id)?; - - let response = AccountResponse::new("exec_on_module", vec![("module", module_id)]).add_message( - CosmosMsg::Wasm(WasmMsg::Execute { - contract_addr: module_addr.into(), - msg: exec_msg, - funds: info.funds, - }), - ); - - Ok(response) -} - /// Checked load of a module address pub fn load_module_addr(storage: &dyn Storage, module_id: &String) -> AccountResult { ACCOUNT_MODULES @@ -585,12 +562,26 @@ mod tests { #[test] fn only_owner() -> anyhow::Result<()> { - let msg = ExecuteMsg::ExecOnModule { - module_id: "test:module".to_string(), + let msg = ExecuteMsg::ExecuteOnModule { + module_id: TEST_MODULE_ID.to_string(), exec_msg: to_json_binary(&"some msg")?, }; - test_only_owner(msg) + let mut deps = mock_dependencies(); + let not_owner = deps.api.addr_make("not_owner"); + mock_init(&mut deps)?; + + ACCOUNT_MODULES.save( + deps.as_mut().storage, + TEST_MODULE_ID, + &Addr::unchecked("not-important"), + )?; + + let res = execute_as(deps.as_mut(), ¬_owner, msg); + assert_that!(&res) + .is_err() + .is_equal_to(AccountError::SenderNotWhitelistedOrOwner {}); + Ok(()) } #[test] @@ -602,7 +593,7 @@ mod tests { mock_init(&mut deps)?; let missing_module = "test:module".to_string(); - let msg = ExecuteMsg::ExecOnModule { + let msg = ExecuteMsg::ExecuteOnModule { module_id: missing_module.clone(), exec_msg: to_json_binary(&"some msg")?, }; @@ -631,7 +622,7 @@ mod tests { let exec_msg = "some msg"; - let msg = ExecuteMsg::ExecOnModule { + let msg = ExecuteMsg::ExecuteOnModule { module_id: "test_mod".to_string(), exec_msg: to_json_binary(&exec_msg)?, }; diff --git a/framework/contracts/account/src/modules/migration.rs b/framework/contracts/account/src/modules/migration.rs index d0d1f4396e..8146261376 100644 --- a/framework/contracts/account/src/modules/migration.rs +++ b/framework/contracts/account/src/modules/migration.rs @@ -1,5 +1,8 @@ use abstract_std::{ - account::{CallbackMsg, ExecuteMsg}, + account::{ + state::{CALLING_TO_AS_ADMIN, CALLING_TO_AS_ADMIN_WILD_CARD}, + CallbackMsg, ExecuteMsg, + }, adapter::{ AdapterBaseMsg, AuthorizedAddressesResponse, BaseQueryMsg, QueryMsg as AdapterQuery, }, @@ -52,6 +55,11 @@ pub fn upgrade_modules( let mut upgraded_module_ids = Vec::new(); + CALLING_TO_AS_ADMIN.save( + deps.storage, + &Addr::unchecked(CALLING_TO_AS_ADMIN_WILD_CARD), + )?; + // Set the migrate messages for each module that's not the manager and update the dependency store for (module_info, migrate_msg) in modules { let module_id = module_info.id(); @@ -317,6 +325,7 @@ pub fn handle_callback(mut deps: DepsMut, env: Env, info: MessageInfo) -> Accoun )?; } + CALLING_TO_AS_ADMIN.remove(deps.storage); MIGRATE_CONTEXT.save(deps.storage, &vec![])?; Ok(Response::new()) } diff --git a/framework/contracts/account/src/reply.rs b/framework/contracts/account/src/reply.rs index 1fe927cf57..89af7cfd39 100644 --- a/framework/contracts/account/src/reply.rs +++ b/framework/contracts/account/src/reply.rs @@ -2,18 +2,19 @@ use crate::{ contract::{AccountResponse, AccountResult}, modules::INSTALL_MODULES_CONTEXT, }; -use abstract_std::objects::{ - module::{assert_module_data_validity, Module}, - module_reference::ModuleReference, +use abstract_std::{ + account::state::CALLING_TO_AS_ADMIN, + objects::{ + module::{assert_module_data_validity, Module}, + module_reference::ModuleReference, + }, }; use cosmwasm_std::{DepsMut, Reply, Response, StdError}; /// Add the message's data to the response -pub fn forward_response_data(result: Reply) -> AccountResult { - // get the result from the reply +pub(crate) fn forward_response_reply(result: Reply) -> AccountResult { let res = result.result.into_result().map_err(StdError::generic_err)?; - // log and add data if needed #[allow(deprecated)] let resp = if let Some(data) = res.data { AccountResponse::new( @@ -27,10 +28,16 @@ pub fn forward_response_data(result: Reply) -> AccountResult { vec![("response_data", "false")], ) }; - Ok(resp) } +/// Remove the storage for an admin call after execution +pub(crate) fn admin_action_reply(deps: DepsMut) -> AccountResult { + CALLING_TO_AS_ADMIN.remove(deps.storage); + + Ok(Response::new()) +} + /// Adds the modules dependencies pub(crate) fn register_dependencies(deps: DepsMut) -> AccountResult { let modules = INSTALL_MODULES_CONTEXT.load(deps.storage)?; diff --git a/framework/contracts/account/tests/adapters.rs b/framework/contracts/account/tests/adapters.rs index 8b089dfbfc..5f2b4fee62 100644 --- a/framework/contracts/account/tests/adapters.rs +++ b/framework/contracts/account/tests/adapters.rs @@ -466,17 +466,33 @@ fn account_adapter_ownership() -> AResult { }), &[], )?; - adapter.call_as(&account.address()?).execute( - &mock::ExecuteMsg::Base(BaseExecuteMsg { + + account.call_as(sender).admin_execute( + adapter.address()?, + to_json_binary(&mock::ExecuteMsg::Base(BaseExecuteMsg { account_address: Some(proxy_addr.to_string()), msg: AdapterBaseMsg::UpdateAuthorizedAddresses { to_add: vec![chain.addr_make("234").to_string()], to_remove: vec![], }, - }), - &[], + }))?, )?; + // Raw account without the calling_to_as_admin variable set, should err. + adapter + .call_as(&account.address()?) + .execute( + &mock::ExecuteMsg::Base(BaseExecuteMsg { + account_address: Some(proxy_addr.to_string()), + msg: AdapterBaseMsg::UpdateAuthorizedAddresses { + to_add: vec![chain.addr_make("456").to_string()], + to_remove: vec![], + }, + }), + &[], + ) + .unwrap_err(); + // Not admin or manager let err: MockError = adapter .call_as(&who) @@ -550,14 +566,29 @@ fn subaccount_adapter_ownership() -> AResult { }), &[], )?; - adapter.call_as(&sub_account.address()?).execute( - &mock::ExecuteMsg::Module(AdapterRequestMsg { + sub_account.call_as(&sender).admin_execute( + adapter.address()?, + to_json_binary(&mock::ExecuteMsg::Module(AdapterRequestMsg { account_address: Some(proxy_addr.to_string()), request: MockExecMsg {}, - }), - &[], + }))?, )?; + // Raw account without the calling_to_as_admin variable set, should err + adapter + .call_as(&account.address()?) + .execute( + &mock::ExecuteMsg::Base(BaseExecuteMsg { + account_address: Some(proxy_addr.to_string()), + msg: AdapterBaseMsg::UpdateAuthorizedAddresses { + to_add: vec![chain.addr_make("456").to_string()], + to_remove: vec![], + }, + }), + &[], + ) + .unwrap_err(); + // Not admin or manager let who = chain.addr_make("who"); let err: MockError = adapter @@ -593,17 +624,32 @@ fn subaccount_adapter_ownership() -> AResult { }), &[], )?; - adapter.call_as(&sub_account.address()?).execute( - &mock::ExecuteMsg::Base(BaseExecuteMsg { + sub_account.call_as(&sender).admin_execute( + adapter.address()?, + to_json_binary(&mock::ExecuteMsg::Base(BaseExecuteMsg { account_address: Some(proxy_addr.to_string()), msg: AdapterBaseMsg::UpdateAuthorizedAddresses { to_add: vec![chain.addr_make("234").to_string()], to_remove: vec![], }, - }), - &[], + }))?, )?; + // Raw account without the calling_to_as_admin variable set, should err + adapter + .call_as(&sub_account.address()?) + .execute( + &mock::ExecuteMsg::Base(BaseExecuteMsg { + account_address: Some(proxy_addr.to_string()), + msg: AdapterBaseMsg::UpdateAuthorizedAddresses { + to_add: vec![chain.addr_make("345").to_string()], + to_remove: vec![], + }, + }), + &[], + ) + .unwrap_err(); + // Not admin or manager let err: MockError = adapter .call_as(&who) diff --git a/framework/contracts/account/tests/apps.rs b/framework/contracts/account/tests/apps.rs index c1d5009be2..4140f315f6 100644 --- a/framework/contracts/account/tests/apps.rs +++ b/framework/contracts/account/tests/apps.rs @@ -47,7 +47,7 @@ fn execute_on_account() -> AResult { let forwarded_coin: Coin = coin(100, "other_coin"); account.execute( - &abstract_std::account::ExecuteMsg::ModuleAction { + &abstract_std::account::ExecuteMsg::Execute { msgs: vec![CosmosMsg::Bank(cosmwasm_std::BankMsg::Burn { amount: burn_amount, })], @@ -103,11 +103,19 @@ fn account_app_ownership() -> AResult { &mock::ExecuteMsg::Module(MockExecMsg::DoSomethingAdmin {}), &[], )?; - app.call_as(&account.address()?).execute( - &mock::ExecuteMsg::Module(MockExecMsg::DoSomethingAdmin {}), - &[], + account.call_as(&sender).admin_execute( + app.address()?, + to_json_binary(&mock::ExecuteMsg::Module(MockExecMsg::DoSomethingAdmin {}))?, )?; + // Account cannot call by itself without the CALLING_TO variable set + app.call_as(&account.address()?) + .execute( + &mock::ExecuteMsg::Module(MockExecMsg::DoSomethingAdmin {}), + &[], + ) + .unwrap_err(); + // Not admin or manager let err: MockError = app .call_as(&Addr::unchecked("who")) diff --git a/framework/contracts/account/tests/proxy.rs b/framework/contracts/account/tests/proxy.rs index cf4d111a74..7cf15afa7a 100644 --- a/framework/contracts/account/tests/proxy.rs +++ b/framework/contracts/account/tests/proxy.rs @@ -114,9 +114,12 @@ fn exec_on_account() -> AResult { let burn_amount = vec![Coin::new(10_000u128, TTOKEN)]; // Burn coins from proxy - account.module_action(vec![CosmosMsg::Bank(cosmwasm_std::BankMsg::Burn { - amount: burn_amount, - })])?; + account.execute_msgs( + vec![CosmosMsg::Bank(cosmwasm_std::BankMsg::Burn { + amount: burn_amount, + })], + &[], + )?; // Assert balance has decreased let proxy_balance = chain.bank_querier().balance(&account.address()?, None)?; @@ -472,7 +475,7 @@ fn renounce_cleans_namespace() -> AResult { // chain.set_balance(&account.address()?, start_balance.clone())?; // // test sending msg as nft account by burning tokens from proxy -// let burn_msg = abstract_std::account::ExecuteMsg::ModuleAction { +// let burn_msg = abstract_std::account::ExecuteMsg::Execute { // msgs: vec![CosmosMsg::Bank(cosmwasm_std::BankMsg::Burn { // amount: burn_amount, // })], diff --git a/framework/contracts/account/tests/subaccount.rs b/framework/contracts/account/tests/subaccount.rs index e1ef1000f6..a5e57a2197 100644 --- a/framework/contracts/account/tests/subaccount.rs +++ b/framework/contracts/account/tests/subaccount.rs @@ -153,16 +153,19 @@ fn installed_app_updating_on_subaccount_should_succeed() -> AResult { // adding mock_app to whitelist on proxy // We call as installed app of the owner-proxy, it should also be possible - account.call_as(&mock_app).module_action(vec![wasm_execute( - sub_account.addr_str()?, - &abstract_account::msg::ExecuteMsg::UpdateInfo { - name: None, - description: Some(new_desc.to_owned()), - link: None, - }, - vec![], - )? - .into()])?; + account.call_as(&mock_app).execute_msgs( + vec![wasm_execute( + sub_account.addr_str()?, + &abstract_std::account::ExecuteMsg::::UpdateInfo { + name: None, + description: Some(new_desc.to_owned()), + link: None, + }, + vec![], + )? + .into()], + &[], + )?; assert_eq!( Some(new_desc.to_string()), @@ -296,12 +299,17 @@ fn sub_account_move_ownership_to_sub_account() -> AResult { .update_whitelist(vec![mock_module.to_string()], Vec::default()) .expect_err("ownership not accepted yet."); - sub_account.module_action(vec![wasm_execute( - new_account_sub_account_addr, - &abstract_account::msg::ExecuteMsg::UpdateOwnership(ownership::GovAction::AcceptOwnership), - vec![], - )? - .into()])?; + sub_account.execute_msgs( + vec![wasm_execute( + new_account_sub_account_addr, + &abstract_std::account::ExecuteMsg::::UpdateOwnership( + ownership::GovAction::AcceptOwnership, + ), + vec![], + )? + .into()], + &[], + )?; // sub-accounts state updated let sub_ids = sub_account.sub_account_ids(None, None)?; @@ -344,14 +352,13 @@ fn account_updated_to_subaccount() -> AResult { })?; // account1 accepting account2 as a sub-account - let accept_msg = - abstract_account::msg::ExecuteMsg::UpdateOwnership(ownership::GovAction::AcceptOwnership); - account_1.module_action(vec![wasm_execute( - account_2.addr_str()?, - &accept_msg, - vec![], - )? - .into()])?; + let accept_msg = abstract_std::account::ExecuteMsg::::UpdateOwnership( + ownership::GovAction::AcceptOwnership, + ); + account_1.execute_msgs( + vec![wasm_execute(account_2.addr_str()?, &accept_msg, vec![])?.into()], + &[], + )?; // Check account_1 knows about his new sub-account let ids = account_1.sub_account_ids(None, None)?; @@ -483,14 +490,19 @@ fn account_updated_to_subaccount_without_recursion() -> AResult { })?; // accepting ownership by sender instead of the manager - account_1.module_action(vec![WasmMsg::Execute { - contract_addr: account_2.addr_str()?, - msg: to_json_binary(&abstract_account::msg::ExecuteMsg::UpdateOwnership( - GovAction::AcceptOwnership, - ))?, - funds: Vec::default(), - } - .into()])?; + account_1.execute_msgs( + vec![WasmMsg::Execute { + contract_addr: account_2.addr_str()?, + msg: to_json_binary( + &abstract_std::account::ExecuteMsg::::UpdateOwnership( + GovAction::AcceptOwnership, + ), + )?, + funds: Vec::default(), + } + .into()], + &[], + )?; // Check manager knows about his new sub-account let ids = account_1.sub_account_ids(None, None)?; @@ -514,19 +526,22 @@ fn sub_account_to_regular_account_without_recursion() -> AResult { &[], )?; - account.module_action(vec![WasmMsg::Execute { - contract_addr: sub_account.addr_str()?, - msg: to_json_binary(&abstract_account::msg::ExecuteMsg::UpdateOwnership( - GovAction::TransferOwnership { - new_owner: GovernanceDetails::Monarchy { - monarch: chain.sender_addr().to_string(), + account.execute_msgs( + vec![WasmMsg::Execute { + contract_addr: sub_account.addr_str()?, + msg: to_json_binary(&abstract_account::msg::ExecuteMsg::UpdateOwnership( + GovAction::TransferOwnership { + new_owner: GovernanceDetails::Monarchy { + monarch: chain.sender_addr().to_string(), + }, + expiry: None, }, - expiry: None, - }, - ))?, - funds: vec![], - } - .into()])?; + ))?, + funds: vec![], + } + .into()], + &[], + )?; sub_account.update_ownership(GovAction::AcceptOwnership)?; let ownership = sub_account.ownership()?; diff --git a/framework/contracts/native/ibc-host/src/account_commands.rs b/framework/contracts/native/ibc-host/src/account_commands.rs index 92a1cf1489..15e19a8f4c 100644 --- a/framework/contracts/native/ibc-host/src/account_commands.rs +++ b/framework/contracts/native/ibc-host/src/account_commands.rs @@ -10,8 +10,8 @@ use abstract_std::{ ACCOUNT, }; use cosmwasm_std::{ - instantiate2_address, to_json_binary, wasm_execute, CosmosMsg, Deps, DepsMut, Env, IbcMsg, - Response, SubMsg, WasmMsg, + instantiate2_address, to_json_binary, wasm_execute, CosmosMsg, Deps, DepsMut, Empty, Env, + IbcMsg, Response, SubMsg, WasmMsg, }; use crate::{ @@ -167,7 +167,7 @@ pub fn send_all_back( // call the message to send everything back through the account let account_msg = wasm_execute( account.into_addr(), - &account::ExecuteMsg::ModuleAction:: { msgs }, + &account::ExecuteMsg::::Execute { msgs }, vec![], )?; Ok(account_msg.into()) diff --git a/framework/packages/abstract-adapter/src/endpoints/execute.rs b/framework/packages/abstract-adapter/src/endpoints/execute.rs index a4a27d75e0..78a2b3ee13 100644 --- a/framework/packages/abstract-adapter/src/endpoints/execute.rs +++ b/framework/packages/abstract-adapter/src/endpoints/execute.rs @@ -59,7 +59,7 @@ impl AdapterResult { @@ -68,35 +68,34 @@ impl { - let account_address = deps.api.addr_validate(&requested_account)?; - let requested_core = account_registry.assert_account(&account_address)?; - if requested_core.addr() == info.sender - || is_top_level_owner( - &deps.querier, - requested_core.addr().clone(), - &info.sender, - ) - .unwrap_or(false) - { - requested_core - } else { - return Err(AdapterError::UnauthorizedAdapterRequest { - adapter: self.module_id().to_string(), - sender: info.sender.to_string(), - }); - } - } - // If not provided the sender must be the direct owner - None => account_registry.assert_account(&info.sender).map_err(|_| { - AdapterError::UnauthorizedAdapterRequest { - adapter: self.module_id().to_string(), - sender: info.sender.to_string(), + let account = account_registry + .assert_is_account_admin(&env, &info.sender) + .map_err(|_| AdapterError::UnauthorizedAdapterRequest { + adapter: self.module_id().to_string(), + sender: info.sender.to_string(), + }) + .or_else(|e| { + // If the sender is not an account or doesn't have the admin functionality enabled, the sender must be a top-level account owner + match account_address { + Some(requested_account) => { + let account_address = deps.api.addr_validate(&requested_account)?; + let account = account_registry.assert_is_account(&account_address)?; + if is_top_level_owner(&deps.querier, account.addr().clone(), &info.sender) + .unwrap_or(false) + { + Ok(account) + } else { + Err(AdapterError::UnauthorizedAdapterRequest { + adapter: self.module_id().to_string(), + sender: info.sender.to_string(), + }) + } + } + // If not provided the sender must be the direct owner AND have admin execution rights + None => Err(e), } - })?, - }; + })?; + self.target_account = Some(account); match msg { AdapterBaseMsg::UpdateAuthorizedAddresses { to_add, to_remove } => { @@ -128,7 +127,7 @@ impl { let account_address = deps.api.addr_validate(&requested_account)?; - let requested_core = account_registry.assert_account(&account_address)?; + let requested_core = account_registry.assert_is_account(&account_address)?; if requested_core.addr() == sender { // If the caller is the account of the indicated proxy_address, it's authorized to do the operation @@ -154,7 +153,7 @@ impl account_registry - .assert_account(sender) + .assert_is_account(sender) .map_err(|_| unauthorized_sender())?, }; self.target_account = Some(account); @@ -290,6 +289,7 @@ mod tests { let account = test_account(deps.api); deps.querier = abstract_mock_querier_builder(deps.api) .account(&account, TEST_ACCOUNT_ID) + .set_account_admin_call_to(&account) .build(); mock_init(&mut deps)?; @@ -323,6 +323,7 @@ mod tests { let account = test_account(deps.api); deps.querier = abstract_mock_querier_builder(deps.api) .account(&account, TEST_ACCOUNT_ID) + .set_account_admin_call_to(&account) .build(); mock_init(&mut deps)?; @@ -363,6 +364,7 @@ mod tests { let account = test_account(deps.api); deps.querier = abstract_mock_querier_builder(deps.api) .account(&account, TEST_ACCOUNT_ID) + .set_account_admin_call_to(&account) .build(); mock_init(&mut deps)?; @@ -405,6 +407,7 @@ mod tests { let account = test_account(deps.api); deps.querier = abstract_mock_querier_builder(deps.api) .account(&account, TEST_ACCOUNT_ID) + .set_account_admin_call_to(&account) .build(); let abstr = AbstractMockAddrs::new(deps.api); @@ -438,6 +441,7 @@ mod tests { let account = test_account(deps.api); deps.querier = abstract_mock_querier_builder(deps.api) .account(&account, TEST_ACCOUNT_ID) + .set_account_admin_call_to(&account) .build(); mock_init(&mut deps)?; @@ -509,6 +513,7 @@ mod tests { let mut deps = mock_dependencies(); deps.querier = MockQuerierBuilder::new(deps.api) .account(&test_account(deps.api), TEST_ACCOUNT_ID) + .set_account_admin_call_to(&test_account(deps.api)) .build(); setup_with_authorized_addresses(&mut deps, vec![]); @@ -542,6 +547,7 @@ mod tests { let account = test_account(deps.api); deps.querier = MockQuerierBuilder::new(deps.api) .account(&account, TEST_ACCOUNT_ID) + .set_account_admin_call_to(&account) .build(); setup_with_authorized_addresses(&mut deps, vec![]); @@ -561,6 +567,7 @@ mod tests { let mut deps = mock_dependencies(); deps.querier = MockQuerierBuilder::new(deps.api) .account(&test_account(deps.api), TEST_ACCOUNT_ID) + .set_account_admin_call_to(&test_account(deps.api)) .build(); setup_with_authorized_addresses(&mut deps, vec![TEST_AUTHORIZED_ADDR]); @@ -582,6 +589,7 @@ mod tests { let account = test_account(deps.api); deps.querier = MockQuerierBuilder::new(deps.api) .account(&account, TEST_ACCOUNT_ID) + .set_account_admin_call_to(&account) .build(); setup_with_authorized_addresses(&mut deps, vec![TEST_AUTHORIZED_ADDR]); @@ -608,6 +616,7 @@ mod tests { &another_account, AccountId::new(69420u32, AccountTrace::Local).unwrap(), ) + .set_account_admin_call_to(&account) .build(); setup_with_authorized_addresses(&mut deps, vec![TEST_AUTHORIZED_ADDR]); diff --git a/framework/packages/abstract-app/examples/counter.rs b/framework/packages/abstract-app/examples/counter.rs index 4ea8c2e00c..8a512a163f 100644 --- a/framework/packages/abstract-app/examples/counter.rs +++ b/framework/packages/abstract-app/examples/counter.rs @@ -107,20 +107,27 @@ mod handlers { // ANCHOR: execute pub fn execute( deps: DepsMut, - _env: Env, + env: Env, info: MessageInfo, module: CounterApp, // <-- Notice how the `CounterApp` is available here msg: CounterExecMsg, ) -> CounterResult { match msg { - CounterExecMsg::UpdateConfig {} => update_config(deps, info, module), + CounterExecMsg::UpdateConfig {} => update_config(deps, env, info, module), } } /// Update the configuration of the app - fn update_config(deps: DepsMut, msg_info: MessageInfo, module: CounterApp) -> CounterResult { + fn update_config( + deps: DepsMut, + env: Env, + msg_info: MessageInfo, + module: CounterApp, + ) -> CounterResult { // Only the admin should be able to call this - module.admin.assert_admin(deps.as_ref(), &msg_info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &msg_info.sender)?; Ok(module .response("update_config") diff --git a/framework/packages/abstract-app/src/endpoints/execute.rs b/framework/packages/abstract-app/src/endpoints/execute.rs index 09a9963d4c..7bd4af52a5 100644 --- a/framework/packages/abstract-app/src/endpoints/execute.rs +++ b/framework/packages/abstract-app/src/endpoints/execute.rs @@ -55,7 +55,7 @@ impl< fn base_execute( &self, deps: DepsMut, - _env: Env, + env: Env, info: MessageInfo, message: BaseExecuteMsg, ) -> AppResult { @@ -63,20 +63,21 @@ impl< BaseExecuteMsg::UpdateConfig { ans_host_address, version_control_address, - } => self.update_config(deps, info, ans_host_address, version_control_address), + } => self.update_config(deps, env, info, ans_host_address, version_control_address), } } fn update_config( &self, deps: DepsMut, + env: Env, info: MessageInfo, ans_host_address: Option, version_control_address: Option, ) -> AppResult { // self._update_config(deps, info, ans_host_address)?; // Only the admin should be able to call this - self.admin.assert_admin(deps.as_ref(), &info.sender)?; + self.admin.assert_admin(deps.as_ref(), &env, &info.sender)?; let mut state = self.base_state.load(deps.storage)?; @@ -140,6 +141,9 @@ mod test { fn update_config_should_update_config() -> AppTestResult { let mut deps = mock_init(); let account = test_account(deps.api); + deps.querier = abstract_mock_querier_builder(deps.api) + .set_account_admin_call_to(&account) + .build(); let new_ans_host = deps.api.addr_make("new_ans_host"); let new_version_control = deps.api.addr_make("new_version_control"); @@ -165,6 +169,9 @@ mod test { let mut deps = mock_init(); let abstr = AbstractMockAddrs::new(deps.api); let account = test_account(deps.api); + deps.querier = abstract_mock_querier_builder(deps.api) + .set_account_admin_call_to(&account) + .build(); let update_ans = AppExecuteMsg::Base(BaseExecuteMsg::UpdateConfig { ans_host_address: None, diff --git a/framework/packages/abstract-app/src/lib.rs b/framework/packages/abstract-app/src/lib.rs index 888e46bb83..8f64cfd0b1 100644 --- a/framework/packages/abstract-app/src/lib.rs +++ b/framework/packages/abstract-app/src/lib.rs @@ -279,10 +279,10 @@ pub mod mock { type Migrate = app::MigrateMsg; const MOCK_APP_WITH_DEP: ::abstract_app::mock::MockAppContract = ::abstract_app::mock::MockAppContract::new($id, $version, None) .with_dependencies($deps) - .with_execute(|deps, _env, info, module, msg| { + .with_execute(|deps, env, info, module, msg| { match msg { MockExecMsg::DoSomethingAdmin{} => { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module.admin.assert_admin(deps.as_ref(), &env, &info.sender)?; }, _ => {}, } diff --git a/framework/packages/abstract-client/Cargo.toml b/framework/packages/abstract-client/Cargo.toml index 516bc82027..4b3565469b 100644 --- a/framework/packages/abstract-client/Cargo.toml +++ b/framework/packages/abstract-client/Cargo.toml @@ -40,7 +40,6 @@ cw-asset = { workspace = true, optional = true } cw-plus-interface = { package = "cw-plus-orch", version = "0.25.0", optional = true } cw20 = { version = "2.0.0", optional = true } cw20-base = { version = "2.0.0", optional = true } -cw721 = "0.18" # For random account seq rand = { version = "0.8.5" } diff --git a/framework/packages/abstract-client/src/account.rs b/framework/packages/abstract-client/src/account.rs index 0b09b1e8fb..02440d2b01 100644 --- a/framework/packages/abstract-client/src/account.rs +++ b/framework/packages/abstract-client/src/account.rs @@ -631,10 +631,7 @@ impl Account { funds: &[Coin], ) -> AbstractClientResult { let msgs = execute_msgs.into_iter().map(Into::into).collect(); - self.configure( - &abstract_std::account::ExecuteMsg::ModuleAction { msgs }, - funds, - ) + self.configure(&account::ExecuteMsg::Execute { msgs }, funds) } /// Executes a [`account::ExecuteMsg`] on the account. diff --git a/framework/packages/abstract-client/src/interchain/remote_account.rs b/framework/packages/abstract-client/src/interchain/remote_account.rs index a49c430308..5fced36d5b 100644 --- a/framework/packages/abstract-client/src/interchain/remote_account.rs +++ b/framework/packages/abstract-client/src/interchain/remote_account.rs @@ -383,9 +383,7 @@ impl> RemoteAccount>, ) -> AbstractClientResult> { let msgs = execute_msgs.into_iter().map(Into::into).collect(); - self.execute_on_account(vec![abstract_std::account::ExecuteMsg::ModuleAction { - msgs, - }]) + self.execute_on_account(vec![abstract_std::account::ExecuteMsg::Execute { msgs }]) } /// Executes a list of [manager::ExecuteMsg] on the manager of the account. diff --git a/framework/packages/abstract-client/src/interchain/remote_application.rs b/framework/packages/abstract-client/src/interchain/remote_application.rs index 43259f39c9..b1aeed828e 100644 --- a/framework/packages/abstract-client/src/interchain/remote_application.rs +++ b/framework/packages/abstract-client/src/interchain/remote_application.rs @@ -49,7 +49,7 @@ impl< .ibc_client_execute(ibc_client::ExecuteMsg::RemoteAction { host_chain: self.remote_account.host_chain_id(), action: ibc_host::HostAction::Dispatch { - account_msgs: vec![account::ExecuteMsg::ExecOnModule { + account_msgs: vec![account::ExecuteMsg::ExecuteOnModule { module_id: M::module_id().to_owned(), exec_msg: to_json_binary(execute).map_err(AbstractInterfaceError::from)?, }], @@ -84,7 +84,7 @@ impl, M: ContractInstance AbstractClientResult<()> { let mut account_msgs = vec![]; for module_id in adapter_ids { - account_msgs.push(account::ExecuteMsg::ExecOnModule { + account_msgs.push(account::ExecuteMsg::ExecuteOnModule { module_id: module_id.to_string(), exec_msg: to_json_binary(&adapter::ExecuteMsg::::Base( adapter::BaseExecuteMsg { diff --git a/framework/packages/abstract-integration-tests/src/account.rs b/framework/packages/abstract-integration-tests/src/account.rs index b84049e42d..41d78834b2 100644 --- a/framework/packages/abstract-integration-tests/src/account.rs +++ b/framework/packages/abstract-integration-tests/src/account.rs @@ -421,9 +421,9 @@ pub fn with_response_data>(mut chain: T) -> AResult { install_adapter(&account, TEST_MODULE_ID)?; - let account_address = account.address()?; - staking_adapter.call_as(&account_address).execute( - &abstract_std::adapter::ExecuteMsg::::Base( + account.admin_execute( + staking_adapter.address()?, + to_json_binary(&abstract_std::adapter::ExecuteMsg::::Base( abstract_std::adapter::BaseExecuteMsg { account_address: None, msg: AdapterBaseMsg::UpdateAuthorizedAddresses { @@ -431,8 +431,7 @@ pub fn with_response_data>(mut chain: T) -> AResult { to_remove: vec![], }, }, - ), - &[], + ))?, )?; chain @@ -443,7 +442,7 @@ pub fn with_response_data>(mut chain: T) -> AResult { .module_info(TEST_MODULE_ID)? .expect("test module installed"); // proxy should be final executor because of the reply - let resp = account.module_action_with_data( + let resp = account.execute_with_data( wasm_execute( adapter_addr.address, &abstract_std::adapter::ExecuteMsg::::Module(AdapterRequestMsg { @@ -453,6 +452,7 @@ pub fn with_response_data>(mut chain: T) -> AResult { vec![], )? .into(), + &[], )?; let response_data_attr_present = resp.event_attr_value("wasm-abstract", "response_data")?; @@ -485,14 +485,17 @@ pub fn account_move_ownership_to_sub_account>(chain: T) let new_account_account = new_account.address()?; let new_account_id = new_account.id()?; - sub_account.module_action(vec![wasm_execute( - new_account_account, - &::UpdateOwnership( - ownership::GovAction::AcceptOwnership, - ), - vec![], - )? - .into()])?; + sub_account.execute_msgs( + vec![wasm_execute( + new_account_account, + &abstract_std::account::ExecuteMsg::::UpdateOwnership( + ownership::GovAction::AcceptOwnership, + ), + vec![], + )? + .into()], + &[], + )?; // sub-accounts state updated let sub_ids = sub_account.sub_account_ids(None, None)?; diff --git a/framework/packages/abstract-interface/src/account.rs b/framework/packages/abstract-interface/src/account.rs index 9efbab3433..168f781b5d 100644 --- a/framework/packages/abstract-interface/src/account.rs +++ b/framework/packages/abstract-interface/src/account.rs @@ -308,8 +308,13 @@ impl AccountI { msg: impl Serialize, ) -> Result<::Response, crate::AbstractInterfaceError> { - self.exec_on_module(to_json_binary(&msg).unwrap(), module, &[]) - .map_err(Into::into) + as AccountExecFns>::execute_on_module( + self, + to_json_binary(&msg).unwrap(), + module, + &[], + ) + .map_err(Into::into) } pub fn update_adapter_authorized_addresses( @@ -318,12 +323,14 @@ impl AccountI { to_add: Vec, to_remove: Vec, ) -> Result<(), crate::AbstractInterfaceError> { - self.execute_on_module( + self.admin_execute_on_module( module_id, - adapter::ExecuteMsg::::Base(adapter::BaseExecuteMsg { - msg: AdapterBaseMsg::UpdateAuthorizedAddresses { to_add, to_remove }, - account_address: None, - }), + to_json_binary(&adapter::ExecuteMsg::::Base( + adapter::BaseExecuteMsg { + msg: AdapterBaseMsg::UpdateAuthorizedAddresses { to_add, to_remove }, + account_address: None, + }, + ))?, )?; Ok(()) @@ -448,7 +455,7 @@ impl AccountI { msg: abstract_std::ibc_client::ExecuteMsg::RemoteAction { host_chain, action: HostAction::Dispatch { - account_msgs: vec![ExecuteMsg::ExecOnModule { + account_msgs: vec![ExecuteMsg::ExecuteOnModule { module_id: module_id.to_string(), exec_msg: msg, }], diff --git a/framework/packages/abstract-sdk/src/apis/bank.rs b/framework/packages/abstract-sdk/src/apis/bank.rs index af9c8eb34c..0d471f6edb 100644 --- a/framework/packages/abstract-sdk/src/apis/bank.rs +++ b/framework/packages/abstract-sdk/src/apis/bank.rs @@ -307,7 +307,7 @@ mod test { assert_that!(response.messages[0].msg).is_equal_to( &wasm_execute( account.addr(), - &ExecuteMsg::ModuleAction:: { + &ExecuteMsg::::Execute { msgs: vec![expected_msg], }, vec![], diff --git a/framework/packages/abstract-sdk/src/apis/execution.rs b/framework/packages/abstract-sdk/src/apis/execution.rs index c29098acd6..20c8b0d7b2 100644 --- a/framework/packages/abstract-sdk/src/apis/execution.rs +++ b/framework/packages/abstract-sdk/src/apis/execution.rs @@ -85,7 +85,7 @@ impl<'a, T: Execution> Executor<'a, T> { fn execute_with_data(&self, msg: CosmosMsg) -> AbstractSdkResult { let msg = self.base.execute_on_account( self.deps, - &ExecuteMsg::ModuleActionWithData { msg }, + &ExecuteMsg::ExecuteWithData { msg }, vec![], )?; Ok(ExecutorMsg(msg)) @@ -112,9 +112,9 @@ impl<'a, T: Execution> Executor<'a, T> { .into_iter() .flat_map(|a| a.into().messages()) .collect(); - let msg = - self.base - .execute_on_account(self.deps, &ExecuteMsg::ModuleAction { msgs }, funds)?; + let msg = self + .base + .execute_on_account(self.deps, &ExecuteMsg::Execute { msgs }, funds)?; Ok(ExecutorMsg(msg)) } @@ -222,7 +222,7 @@ mod test { let expected = ExecutorMsg(CosmosMsg::Wasm(WasmMsg::Execute { contract_addr: account.addr().to_string(), - msg: to_json_binary(&ExecuteMsg::ModuleAction:: { + msg: to_json_binary(&ExecuteMsg::::Execute { msgs: flatten_actions(messages), }) .unwrap(), @@ -244,7 +244,7 @@ mod test { let expected = ExecutorMsg(CosmosMsg::Wasm(WasmMsg::Execute { contract_addr: account.addr().to_string(), - msg: to_json_binary(&ExecuteMsg::ModuleAction:: { + msg: to_json_binary(&ExecuteMsg::::Execute { msgs: flatten_actions(messages), }) .unwrap(), @@ -280,7 +280,7 @@ mod test { id: expected_reply_id, msg: CosmosMsg::Wasm(WasmMsg::Execute { contract_addr: account.addr().to_string(), - msg: to_json_binary(&ExecuteMsg::ModuleAction:: { + msg: to_json_binary(&ExecuteMsg::::Execute { msgs: flatten_actions(empty_actions), }) .unwrap(), @@ -315,7 +315,7 @@ mod test { id: expected_reply_id, msg: CosmosMsg::Wasm(WasmMsg::Execute { contract_addr: account.addr().to_string(), - msg: to_json_binary(&ExecuteMsg::ModuleAction:: { + msg: to_json_binary(&ExecuteMsg::::Execute { msgs: flatten_actions(action), }) .unwrap(), @@ -346,7 +346,7 @@ mod test { let expected_msg = CosmosMsg::Wasm(WasmMsg::Execute { contract_addr: account.addr().to_string(), - msg: to_json_binary(&ExecuteMsg::ModuleAction:: { + msg: to_json_binary(&ExecuteMsg::::Execute { msgs: flatten_actions(empty_actions), }) .unwrap(), @@ -378,7 +378,7 @@ mod test { let expected_msg = CosmosMsg::Wasm(WasmMsg::Execute { contract_addr: account.addr().to_string(), - msg: to_json_binary(&ExecuteMsg::ModuleAction:: { + msg: to_json_binary(&ExecuteMsg::::Execute { msgs: flatten_actions(action), }) .unwrap(), diff --git a/framework/packages/abstract-sdk/src/apis/ibc.rs b/framework/packages/abstract-sdk/src/apis/ibc.rs index 321c1ea0cf..551f0745dd 100644 --- a/framework/packages/abstract-sdk/src/apis/ibc.rs +++ b/framework/packages/abstract-sdk/src/apis/ibc.rs @@ -316,7 +316,7 @@ impl<'a, T: IbcInterface + AccountExecutor> IbcClient<'a, T> { self.host_action( host_chain, HostAction::Dispatch { - account_msgs: vec![abstract_std::account::ExecuteMsg::ExecOnModule { + account_msgs: vec![abstract_std::account::ExecuteMsg::ExecuteOnModule { module_id, exec_msg: to_json_binary(exec_msg)?, }], diff --git a/framework/packages/abstract-sdk/src/apis/verify.rs b/framework/packages/abstract-sdk/src/apis/verify.rs index 5dc568bd3f..2fd1098948 100644 --- a/framework/packages/abstract-sdk/src/apis/verify.rs +++ b/framework/packages/abstract-sdk/src/apis/verify.rs @@ -1,16 +1,19 @@ //! # Verification //! The `Verify` struct provides helper functions that enable the contract to verify if the sender is an Abstract Account, Account admin, etc. use abstract_std::{ - objects::{version_control::VersionControlContract, AccountId}, + objects::{ + ownership::nested_admin::assert_account_calling_to_as_admin_is_self, + version_control::VersionControlContract, AccountId, + }, version_control::Account, }; -use cosmwasm_std::{Addr, Deps}; +use cosmwasm_std::{Addr, Deps, Env}; use super::{AbstractApi, ApiIdentification}; use crate::{ cw_helpers::ApiQuery, features::{AbstractRegistryAccess, ModuleIdentification}, - AbstractSdkResult, + AbstractSdkError, AbstractSdkResult, }; /// Verify if an addresses is associated with an Abstract Account. @@ -83,7 +86,7 @@ pub struct AccountRegistry<'a, T: AccountVerification> { impl<'a, T: AccountVerification> AccountRegistry<'a, T> { /// Verify if the provided address is indeed an Abstract Account. - pub fn assert_account(&self, maybe_account: &Addr) -> AbstractSdkResult { + pub fn assert_is_account(&self, maybe_account: &Addr) -> AbstractSdkResult { self.vc .assert_account(maybe_account, &self.deps.querier) .map_err(|error| self.wrap_query_error(error)) @@ -109,6 +112,20 @@ impl<'a, T: AccountVerification> AccountRegistry<'a, T> { .namespace_registration_fee(&self.deps.querier) .map_err(|error| self.wrap_query_error(error)) } + + /// Verify if the provided address is indeed an Abstract Account AND if the current execution has admin rights. + pub fn assert_is_account_admin( + &self, + env: &Env, + maybe_account: &Addr, + ) -> AbstractSdkResult { + let account = self.assert_is_account(maybe_account)?; + + if !assert_account_calling_to_as_admin_is_self(&self.deps.querier, env, maybe_account) { + return Err(AbstractSdkError::OnlyAdmin {}); + } + Ok(account) + } } #[cfg(test)] @@ -165,7 +182,7 @@ mod test { let res = binding .account_registry(deps.as_ref()) .unwrap() - .assert_account(not_account.addr()); + .assert_is_account(not_account.addr()); let expected_err = AbstractSdkError::ApiQuery { api: AccountRegistry::::api_id(), @@ -196,7 +213,7 @@ mod test { let res = binding .account_registry(deps.as_ref()) .unwrap() - .assert_account(abstr.account.addr()); + .assert_is_account(abstr.account.addr()); assert_that!(res) .is_err() @@ -232,7 +249,7 @@ mod test { let res = binding .account_registry(deps.as_ref()) .unwrap() - .assert_account(account.addr()); + .assert_is_account(account.addr()); assert_that!(res).is_ok().is_equal_to(account); } diff --git a/framework/packages/abstract-sdk/src/error.rs b/framework/packages/abstract-sdk/src/error.rs index 3f27e1e11f..e67f3b5980 100644 --- a/framework/packages/abstract-sdk/src/error.rs +++ b/framework/packages/abstract-sdk/src/error.rs @@ -91,6 +91,12 @@ pub enum AbstractSdkError { module: String, err: String, }, + + // This call needs to be an admin call + #[error( + "Only the admin can execute this action. An admin is either the owner of an account of an account called by its owner" + )] + OnlyAdmin {}, } impl AbstractSdkError { diff --git a/framework/packages/abstract-std/src/account.rs b/framework/packages/abstract-std/src/account.rs index bba2e357a8..64230c95d7 100644 --- a/framework/packages/abstract-std/src/account.rs +++ b/framework/packages/abstract-std/src/account.rs @@ -50,6 +50,9 @@ pub mod state { pub link: Option, } + #[cosmwasm_schema::cw_serde] + pub struct WhitelistedModules(pub Vec); + pub const WHITELISTED_MODULES: Item = Item::new(storage_namespaces::account::WHITELISTED_MODULES); @@ -71,10 +74,12 @@ pub mod state { Map::new(storage_namespaces::account::SUB_ACCOUNTS); /// Account Id storage key pub const ACCOUNT_ID: Item = Item::new(storage_namespaces::account::ACCOUNT_ID); - // Additional states, not listed here: cw_gov_ownable::GovOwnership, authenticators, if chain supports it + /// Temporary state variable that allows for checking access control on admin operation + pub const CALLING_TO_AS_ADMIN: Item = + Item::new(storage_namespaces::account::CALLING_TO_AS_ADMIN); + pub const CALLING_TO_AS_ADMIN_WILD_CARD: &str = "calling-to-wild-card"; - #[cosmwasm_schema::cw_serde] - pub struct WhitelistedModules(pub Vec); + // Additional states, not listed here: cw_gov_ownable::GovOwnership, authenticators, if chain supports it } #[cosmwasm_schema::cw_serde] @@ -104,13 +109,32 @@ pub struct CallbackMsg {} #[derive(cw_orch::ExecuteFns)] pub enum ExecuteMsg { /// Executes the provided messages if sender is whitelisted - ModuleAction { + #[cw_orch(fn_name("execute_msgs"), payable)] + Execute { msgs: Vec>, }, /// Execute a message and forward the Response data - ModuleActionWithData { + #[cw_orch(payable)] + ExecuteWithData { msg: CosmosMsg, }, + /// Forward execution message to module + #[cw_orch(payable)] + ExecuteOnModule { + module_id: String, + exec_msg: Binary, + }, + /// Execute a Wasm Message with Account Admin privileges + AdminExecute { + addr: String, + msg: Binary, + }, + /// Forward execution message to module with Account Admin privileges + AdminExecuteOnModule { + module_id: String, + msg: Binary, + }, + /// Execute IBC action on Client IbcAction { msg: crate::ibc_client::ExecuteMsg, @@ -121,12 +145,6 @@ pub enum ExecuteMsg { /// Query of type `abstract-ica-client::msg::QueryMsg` action_query_msg: Binary, }, - /// Forward execution message to module - #[cw_orch(payable)] - ExecOnModule { - module_id: String, - exec_msg: Binary, - }, /// Update Abstract-specific configuration of the module. /// Only callable by the account factory or owner. UpdateInternalConfig(InternalConfigAction), diff --git a/framework/packages/abstract-std/src/objects/ownership/README.md b/framework/packages/abstract-std/src/objects/ownership/README.md index 4968ecc67e..f0f1ff1d84 100644 --- a/framework/packages/abstract-std/src/objects/ownership/README.md +++ b/framework/packages/abstract-std/src/objects/ownership/README.md @@ -1,8 +1,12 @@ -# CW Ownable +# Abstract Ownership + +Abstract uses multiple ownership capabilities for different use cases. + +## CW Ownable Utility for controlling ownership of [CosmWasm](https://github.com/CosmWasm/cosmwasm) smart contracts. -## How to use +### How to use Initialize the owner during instantiation using the `initialize_owner` method provided by this crate: @@ -92,12 +96,85 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult { } ``` -## Edge cases +### Edge cases + +## NFT governance type + +For NFT-owned accounts the account's ownership is determined by **who owns the related NFT**. I.e. when transferring the ownership of the NFT all the accounts related to that NFT also change ownership. +In the case where the NFT contract does not return a valid `owner_of`, the account's ownership will be treated as if it was renounced and the account becomes unavailable. This would happen when an NFT is burned or something happens with NFT contract. + +## Abstract Account Controlled Module + +For modules and contracts controlled by Abstract Accounts, we present a mechanism that allows those contracts to make sure that an in-coming message from the Account was originally called by an admin and not another module. This prevents modules from calling admin functions on other modules and thus makes the module system more resistent to malicious modules. + +### Mechanism + +Modules and Account Owners can execute actions through the Account using the `account::ExecuteMsg::Execute` message variant. In order to execute an admin call, owners need to call `account::ExecuteMsg::AdminExecute`. The admin function will then: + +- Set the `CALLING_TO_AS_ADMIN` storage item to the target address of the admin call. +- Call the specified function on the target module or contract. +- Remove the `CALLING_TO_AS_ADMIN` storage item. + +In order to check that the call is an admin call, the target module or contract needs to check that the `CALLING_TO_AS_ADMIN` storage item is present on the account contract and that it contains `env.contract.address`. If it's not set or a different address, it should error, as the call is not an authorized admin call. + +### Usage inside a module -### NFT governance type +To use this functionality, Abstract provides helpers in form of the `NestedAdmin` structure. This structure should be used to handle `Abstract Accounts` as admin of a contract. -In case NFT contract does not return owner of `owner_of`, ownership will act as renounced. For example NFT got burned or something happened with NFT contract. +The `NestedAdmin::assert_admin` function will only return an `Result::Ok` if any of those conditions is true: -## License +- The caller is the saved Account AND the `CALLING_TO_AS_ADMIN` variable is set on the account to either: + - The contract account address + - The `CALLING_TO_AS_ADMIN_WILD_CARD`, that is used for contract migrations to avoid re-setting the flag during migration events. +- The caller is the top-level owner of the saved Account -Contents of this crate at or prior to version `0.5.0` are published under [GNU Affero General Public License v3](https://github.com/steak-enjoyers/cw-plus-plus/blob/9c8fcf1c95b74dd415caf5602068c558e9d16ecc/LICENSE) or later; contents after the said version are published under [Apache-2.0](../../LICENSE) license. +So inside `Abstract Apps` for instance, one should write the following lines to shield admin actions: + +```rust +app.admin.assert_admin(deps.as_ref(), &env, info.sender)?; +``` + +### Graphical sequences + +#### Successful admin call + +```mermaid +sequenceDiagram +User ->> Account: ExecuteMsg::ConfigureModule
{ module_id: Module, msg: ...} +Account ->> Account: Store Module address as
`CALLING_TO_AS_ADMIN` +Account ->> Module: ExecuteMsg +alt query +Module ->> Account: Query `CALLING_TO_AS_ADMIN` +Account ->> Module: +end +Module ->> Module: Make sure `CALLING_TO_AS_ADMIN` == Module +Module ->> Module: Execute Admin Message +Account ->> Account: Remove `CALLING_TO_AS_ADMIN` +``` + +#### Error, not admin call + +```mermaid +sequenceDiagram +Bad Module ->> Account: ExecuteMsg::ExecuteOnModule
{ module_id: Module, msg: ...} +Account ->> Module: ExecuteMsg +alt query +Module -x Account: Query `CALLING_TO_AS_ADMIN` +Account -x Module: Not set +end +``` + +#### Malicious Module can’t execute Admin function of other Module + +```mermaid +sequenceDiagram +User ->> Account: ExecuteMsg::ConfigureModule
{ module_id: Module, msg: ...} +Account ->> Account: Store Bad Module address as
`CALLING_TO_AS_ADMIN` +Account ->> Bad Module: ExecuteMsg +Bad Module ->> Module: ChangeConfig +alt query +Module ->> Account: Query `CALLING_TO_AS_ADMIN` +Account ->> Module: +end +Module ->> Module: `CALLING_TO_AS_ADMIN` != Module --> Error +``` diff --git a/framework/packages/abstract-std/src/objects/ownership/nested_admin.rs b/framework/packages/abstract-std/src/objects/ownership/nested_admin.rs index 35380b0812..5357eca2b2 100644 --- a/framework/packages/abstract-std/src/objects/ownership/nested_admin.rs +++ b/framework/packages/abstract-std/src/objects/ownership/nested_admin.rs @@ -1,7 +1,10 @@ -use crate::objects::{gov_type::GovernanceDetails, ownership::Ownership}; +use crate::{ + account::state::{CALLING_TO_AS_ADMIN, CALLING_TO_AS_ADMIN_WILD_CARD}, + objects::{gov_type::GovernanceDetails, ownership::Ownership}, +}; use cosmwasm_std::{ - attr, Addr, CustomQuery, Deps, DepsMut, MessageInfo, QuerierWrapper, Response, StdError, + attr, Addr, CustomQuery, Deps, DepsMut, Env, MessageInfo, QuerierWrapper, Response, StdError, StdResult, }; use cw_controllers::{Admin, AdminError, AdminResponse}; @@ -12,10 +15,14 @@ use super::query_ownership; /// Max account admin recursion pub const MAX_ADMIN_RECURSION: usize = 2; -/// Abstract Admin object -/// This object has same api to the [cw_controllers::Admin] -/// With added query_account_owner method (will get top-level owner in case of sub-accounts) -/// but allows top-level abstract account owner to have admin privileges on the module +/// # Abstract Admin Object +/// This object has a similar api to the [cw_controllers::Admin] object but incorporates nested ownership and abstract-specific Admin checks. +/// +/// The ownership of a contract can be nested, meaning that the owner of the contract can be owned by another contract (or address) and so on. +/// +/// By using this structure we allow both the direct owner as well as the top-level owner to have permissions to perform actions that are gated by this object. +/// +/// See [NestedAdmin::assert_admin] for more details on how the admin rights are checked. pub struct NestedAdmin(Admin); impl NestedAdmin { @@ -31,22 +38,31 @@ impl NestedAdmin { self.0.get(deps) } - pub fn is_admin(&self, deps: Deps, caller: &Addr) -> StdResult { + /// See [NestedAdmin::assert_admin] for more details. + pub fn is_admin( + &self, + deps: Deps, + env: &Env, + caller: &Addr, + ) -> StdResult { match self.0.get(deps)? { - Some(admin) => Self::is_admin_custom(&deps.querier, caller, admin), + Some(admin) => Self::is_admin_custom(&deps.querier, env, caller, admin), None => Ok(false), } } - /// Compares the provided admin to the caller. - /// Can be used when other ownership structure than `cw-controller::Admin` is used. + /// See [NestedAdmin::assert_admin] for more details. pub fn is_admin_custom( querier: &QuerierWrapper, + env: &Env, caller: &Addr, admin: Addr, ) -> StdResult { // Initial check if directly called by the admin - if caller == admin { + if caller == admin && assert_account_calling_to_as_admin_is_self(querier, env, caller) { + // If the caller is the admin, we still need to check that + // if it's an account it's authorized to act as an admin + Ok(true) } else { // Check if top level owner address is equal to the caller @@ -56,27 +72,38 @@ impl NestedAdmin { } } - /// Assert the caller is the admin of this nested ownership structures + /// Assert that the caller is allowed to perform admin actions. + /// + /// This method will pass in two specific scenarios: + /// + /// - If the caller is the direct admin of the contract. I.e. the admin stored in this contract. AND the state `CALLING_TO_AS_ADMIN` is set to the contract address or a wildcard. + /// - If the caller is the **top-level** admin of the chain of ownership, starting from this contract. pub fn assert_admin( &self, deps: Deps, + env: &Env, caller: &Addr, ) -> Result<(), AdminError> { - if !self.is_admin(deps, caller)? { + if !self.is_admin(deps, env, caller)? { Err(AdminError::NotAdmin {}) } else { Ok(()) } } - /// Assert the caller is the admin of this nested ownership structures - /// Either directly or indirectly + /// Assert that the caller is allowed to perform admin actions. + /// + /// This method will pass in two specific scenarios: + /// + /// - If the caller is the direct admin of the contract. I.e. the admin stored in this contract. AND the state `CALLING_TO_AS_ADMIN` is set to the contract address or a wildcard. + /// - If the caller is the **top-level** admin of the chain of ownership, starting from this contract. pub fn assert_admin_custom( querier: &QuerierWrapper, + env: &Env, caller: &Addr, admin: Addr, ) -> Result<(), AdminError> { - if !Self::is_admin_custom(querier, caller, admin)? { + if !Self::is_admin_custom(querier, env, caller, admin)? { Err(AdminError::NotAdmin {}) } else { Ok(()) @@ -86,13 +113,14 @@ impl NestedAdmin { pub fn execute_update_admin( &self, deps: DepsMut, + env: &Env, info: MessageInfo, new_admin: Option, ) -> Result, AdminError> where C: Clone + core::fmt::Debug + PartialEq + JsonSchema, { - self.assert_admin(deps.as_ref(), &info.sender)?; + self.assert_admin(deps.as_ref(), env, &info.sender)?; let admin_str = match new_admin.as_ref() { Some(admin) => admin.to_string(), @@ -164,3 +192,18 @@ pub fn query_top_level_owner( current } + +/// Assert that the account has a valid calling to the contract as an admin. +pub fn assert_account_calling_to_as_admin_is_self( + querier: &QuerierWrapper, + env: &Env, + maybe_account: &Addr, +) -> bool { + CALLING_TO_AS_ADMIN + .query(querier, maybe_account.clone()) + .map(|admin_call_to| { + admin_call_to == env.contract.address + || admin_call_to.as_str() == CALLING_TO_AS_ADMIN_WILD_CARD + }) + .unwrap_or(false) +} diff --git a/framework/packages/abstract-std/src/objects/storage_namespaces.rs b/framework/packages/abstract-std/src/objects/storage_namespaces.rs index 358a5b7016..0cb91ac568 100644 --- a/framework/packages/abstract-std/src/objects/storage_namespaces.rs +++ b/framework/packages/abstract-std/src/objects/storage_namespaces.rs @@ -19,6 +19,7 @@ pub mod account { pub const ACCOUNT_ID: &str = "ag"; pub const INSTALL_MODULES_CONTEXT: &str = "ah"; pub const MIGRATE_CONTEXT: &str = "ai"; + pub const CALLING_TO_AS_ADMIN: &str = "aj"; // XION authentificators, could be there could be not pub const AUTH_ADMIN: &str = "ax"; diff --git a/framework/packages/abstract-testing/src/abstract_mock_querier.rs b/framework/packages/abstract-testing/src/abstract_mock_querier.rs index b6faeccd98..6850ecb878 100644 --- a/framework/packages/abstract-testing/src/abstract_mock_querier.rs +++ b/framework/packages/abstract-testing/src/abstract_mock_querier.rs @@ -1,5 +1,5 @@ use abstract_std::{ - account::state::ACCOUNT_ID, + account::state::{ACCOUNT_ID, CALLING_TO_AS_ADMIN}, ans_host::state::{ASSET_ADDRESSES, CHANNELS, CONTRACT_ADDRESSES}, objects::{ gov_type::GovernanceDetails, ownership::Ownership, @@ -8,7 +8,7 @@ use abstract_std::{ }, version_control::{state::ACCOUNT_ADDRESSES, Account}, }; -use cosmwasm_std::Addr; +use cosmwasm_std::{testing::mock_env, Addr}; use cw_asset::AssetInfo; use cw_storage_plus::Item; @@ -21,6 +21,8 @@ pub trait AbstractMockQuerier { /// Add mock assets into ANS fn assets(self, assets: Vec<(&AssetEntry, AssetInfo)>) -> Self; + fn set_account_admin_call_to(self, account: &Account) -> Self; + fn contracts(self, contracts: Vec<(&ContractEntry, Addr)>) -> Self; fn channels(self, channels: Vec<(&ChannelEntry, String)>) -> Self; @@ -77,4 +79,12 @@ impl AbstractMockQuerier for MockQuerierBuilder { fn addrs(&self) -> AbstractMockAddrs { AbstractMockAddrs::new(self.api) } + + fn set_account_admin_call_to(self, account: &Account) -> Self { + self.with_contract_item( + account.addr(), + CALLING_TO_AS_ADMIN, + &mock_env().contract.address, + ) + } } diff --git a/interchain/interchain-tests/src/interchain_accounts.rs b/interchain/interchain-tests/src/interchain_accounts.rs index 959768c2ea..7b6c6fdf85 100644 --- a/interchain/interchain-tests/src/interchain_accounts.rs +++ b/interchain/interchain-tests/src/interchain_accounts.rs @@ -191,8 +191,8 @@ mod test { )?; // The user on origin chain wants to change the account description - let ibc_transfer_result = - origin_account.module_action(vec![cosmwasm_std::CosmosMsg::Ibc( + let ibc_transfer_result = origin_account.execute_msgs( + vec![cosmwasm_std::CosmosMsg::Ibc( cosmwasm_std::IbcMsg::Transfer { channel_id: interchain_channel .interchain_channel @@ -213,7 +213,9 @@ mod test { ), memo: None, }, - )])?; + )], + &[], + )?; mock_interchain.await_and_check_packets(JUNO, ibc_transfer_result)?; @@ -404,7 +406,7 @@ mod test { }); let create_account_remote_tx = origin_account.execute_on_remote( TruncatedChainId::from_chain_id(STARGAZE), - abstract_std::account::ExecuteMsg::ModuleAction { + abstract_std::account::ExecuteMsg::Execute { msgs: vec![create_account_instantiate_msg], }, )?; diff --git a/interchain/modules-clone-testing/tests/dex/astrovault.rs b/interchain/modules-clone-testing/tests/dex/astrovault.rs index 1a25dfc32e..2d62dc2fb7 100644 --- a/interchain/modules-clone-testing/tests/dex/astrovault.rs +++ b/interchain/modules-clone-testing/tests/dex/astrovault.rs @@ -680,7 +680,7 @@ mod ratio_pool_tests { fn test_provide_liquidity() -> anyhow::Result<()> { let dex_tester = setup_ratio_pool()?; - let deposit_amount = 10_000_000; + let deposit_amount = 1_000_000; // Pool is normalized right now so should be fine to provide "non-equal" amounts @@ -694,7 +694,7 @@ mod ratio_pool_tests { fn test_withdraw_liquidity() -> anyhow::Result<()> { let dex_tester = setup_ratio_pool()?; - let deposit_amount = 10_000_000; + let deposit_amount = 1_000_000; dex_tester.test_withdraw_liquidity( Some(deposit_amount), diff --git a/modules/contracts/adapters/cw-staking/src/handlers/execute.rs b/modules/contracts/adapters/cw-staking/src/handlers/execute.rs index 59c6691bfc..fa071a8200 100644 --- a/modules/contracts/adapters/cw-staking/src/handlers/execute.rs +++ b/modules/contracts/adapters/cw-staking/src/handlers/execute.rs @@ -78,16 +78,18 @@ fn handle_ibc_request( // construct the action to be called on the host // construct the action to be called on the host let host_action = abstract_adapter::std::ibc_host::HostAction::Dispatch { - account_msgs: vec![abstract_adapter::std::account::ExecuteMsg::ExecOnModule { - module_id: CW_STAKING_ADAPTER_ID.to_string(), - exec_msg: to_json_binary::( - &StakingExecuteMsg { - provider: provider_name.clone(), - action: action.clone(), - } - .into(), - )?, - }], + account_msgs: vec![ + abstract_adapter::std::account::ExecuteMsg::ExecuteOnModule { + module_id: CW_STAKING_ADAPTER_ID.to_string(), + exec_msg: to_json_binary::( + &StakingExecuteMsg { + provider: provider_name.clone(), + action: action.clone(), + } + .into(), + )?, + }, + ], }; // If the calling entity is a contract, we provide a callback on successful cross-chain-staking diff --git a/modules/contracts/adapters/dex/Cargo.toml b/modules/contracts/adapters/dex/Cargo.toml index 2487713743..fc0fdc335b 100644 --- a/modules/contracts/adapters/dex/Cargo.toml +++ b/modules/contracts/adapters/dex/Cargo.toml @@ -121,7 +121,7 @@ clap = { workspace = true } dex = { path = ".", features = [ # "wynd", # "osmosis", - # "testing", + "testing", ], package = "abstract-dex-adapter" } # abstract-wyndex-adapter = { workspace = true, features = ["local"] } diff --git a/modules/contracts/adapters/dex/src/handlers/execute.rs b/modules/contracts/adapters/dex/src/handlers/execute.rs index d158bd5f7b..9d584a1e7c 100644 --- a/modules/contracts/adapters/dex/src/handlers/execute.rs +++ b/modules/contracts/adapters/dex/src/handlers/execute.rs @@ -125,16 +125,18 @@ fn handle_ibc_request( let ics20_transfer_msg = ibc_client.ics20_transfer(host_chain.clone(), coins, None)?; // construct the action to be called on the host let host_action = abstract_adapter::std::ibc_host::HostAction::Dispatch { - account_msgs: vec![abstract_adapter::std::account::ExecuteMsg::ExecOnModule { - module_id: DEX_ADAPTER_ID.to_string(), - exec_msg: to_json_binary::( - &DexExecuteMsg::Action { - dex: dex_name.clone(), - action: action.clone(), - } - .into(), - )?, - }], + account_msgs: vec![ + abstract_adapter::std::account::ExecuteMsg::ExecuteOnModule { + module_id: DEX_ADAPTER_ID.to_string(), + exec_msg: to_json_binary::( + &DexExecuteMsg::Action { + dex: dex_name.clone(), + action: action.clone(), + } + .into(), + )?, + }, + ], }; // If the calling entity is a contract, we provide a callback on successful swap diff --git a/modules/contracts/apps/calendar/src/handlers/execute.rs b/modules/contracts/apps/calendar/src/handlers/execute.rs index 38a65e5733..bd57735455 100644 --- a/modules/contracts/apps/calendar/src/handlers/execute.rs +++ b/modules/contracts/apps/calendar/src/handlers/execute.rs @@ -75,7 +75,7 @@ pub fn execute_handler( CalendarExecuteMsg::UpdateConfig { price_per_minute, denom, - } => update_config(deps, info, module, price_per_minute, denom), + } => update_config(deps, env, info, module, price_per_minute, denom), } } @@ -165,7 +165,9 @@ fn handle_stake( meeting_index: u32, stake_action: StakeAction, ) -> CalendarAppResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let config = CONFIG.load(deps.storage)?; @@ -234,12 +236,15 @@ fn handle_stake( fn update_config( deps: DepsMut, + env: Env, info: MessageInfo, module: CalendarApp, price_per_minute: Option, denom: Option, ) -> CalendarAppResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let mut config = CONFIG.load(deps.storage)?; let mut attrs = vec![]; if let Some(price_per_minute) = price_per_minute { diff --git a/modules/contracts/apps/challenge/src/handlers/execute.rs b/modules/contracts/apps/challenge/src/handlers/execute.rs index c7ae52e74c..3da1f78ea9 100644 --- a/modules/contracts/apps/challenge/src/handlers/execute.rs +++ b/modules/contracts/apps/challenge/src/handlers/execute.rs @@ -71,7 +71,9 @@ fn create_challenge( challenge_req: ChallengeRequest, ) -> AppResult { // Only the admin should be able to create a challenge. - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; ensure!( challenge_req.init_friends.len() < MAX_AMOUNT_OF_FRIENDS as usize, AppError::TooManyFriends {} @@ -111,13 +113,15 @@ fn create_challenge( fn update_challenge( deps: DepsMut, - _env: Env, + env: Env, info: MessageInfo, module: ChallengeApp, challenge_id: u64, new_challenge: ChallengeEntryUpdate, ) -> AppResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; // will return an error if the challenge doesn't exist let mut loaded_challenge: ChallengeEntry = CHALLENGES @@ -148,7 +152,9 @@ fn cancel_challenge( module: &ChallengeApp, challenge_id: u64, ) -> AppResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let mut challenge = CHALLENGES.load(deps.storage, challenge_id)?; // Check if this challenge still active if env.block.time >= challenge.end_timestamp { @@ -179,7 +185,9 @@ fn update_friends_for_challenge( friends: Vec>, op_kind: UpdateFriendsOpKind, ) -> AppResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; // Validate friend addr and account ids let friends_validated: Vec<(Addr, Friend)> = friends .iter() @@ -289,7 +297,7 @@ fn cast_vote( let voter = match module .account_registry(deps.as_ref())? - .assert_account(&info.sender) + .assert_is_account(&info.sender) { Ok(base) => base.into_addr(), Err(_) => info.sender, @@ -336,7 +344,9 @@ fn veto( let proposal_id = last_proposal(challenge_id, deps.as_ref())?.ok_or(AppError::ExpectedProposal {})?; - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let proposal_info = SIMPLE_VOTING.veto_proposal(deps.storage, &env.block, proposal_id)?; Ok(module diff --git a/modules/contracts/apps/challenge/tests/integrations.rs b/modules/contracts/apps/challenge/tests/integrations.rs index 5668182137..75b8dcbe29 100644 --- a/modules/contracts/apps/challenge/tests/integrations.rs +++ b/modules/contracts/apps/challenge/tests/integrations.rs @@ -159,7 +159,6 @@ fn setup() -> anyhow::Result<( &[], )?; - challenge_app.set_sender(&account.address()?); mock.set_balance( &account.address()?, vec![coin(50_000_000, DENOM), coin(10_000, "eur")], diff --git a/modules/contracts/apps/croncat/src/handlers/execute.rs b/modules/contracts/apps/croncat/src/handlers/execute.rs index 33e40287da..db85e3804b 100644 --- a/modules/contracts/apps/croncat/src/handlers/execute.rs +++ b/modules/contracts/apps/croncat/src/handlers/execute.rs @@ -37,7 +37,7 @@ pub fn execute_handler( msg: AppExecuteMsg, ) -> CroncatResult { match msg { - AppExecuteMsg::UpdateConfig {} => update_config(deps, info, module), + AppExecuteMsg::UpdateConfig {} => update_config(deps, env, info, module), AppExecuteMsg::CreateTask { task, task_tag, @@ -52,9 +52,16 @@ pub fn execute_handler( } /// Update the configuration of the app -fn update_config(deps: DepsMut, msg_info: MessageInfo, module: CroncatApp) -> CroncatResult { +fn update_config( + deps: DepsMut, + env: Env, + msg_info: MessageInfo, + module: CroncatApp, +) -> CroncatResult { // Only the admin should be able to call this - module.admin.assert_admin(deps.as_ref(), &msg_info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &msg_info.sender)?; CONFIG.save(deps.storage, &Config {})?; Ok(module.response("update_config")) @@ -63,7 +70,7 @@ fn update_config(deps: DepsMut, msg_info: MessageInfo, module: CroncatApp) -> Cr /// Create a task fn create_task( deps: DepsMut, - _env: Env, + env: Env, msg_info: MessageInfo, module: CroncatApp, task_request: Box, @@ -72,7 +79,7 @@ fn create_task( ) -> CroncatResult { if module .admin - .assert_admin(deps.as_ref(), &msg_info.sender) + .assert_admin(deps.as_ref(), &env, &msg_info.sender) .is_err() { assert_module_installed(deps.as_ref(), &msg_info.sender, &module)?; @@ -135,14 +142,14 @@ fn create_task( /// Remove a task fn remove_task( deps: DepsMut, - _env: Env, + env: Env, msg_info: MessageInfo, module: CroncatApp, task_tag: String, ) -> CroncatResult { if module .admin - .assert_admin(deps.as_ref(), &msg_info.sender) + .assert_admin(deps.as_ref(), &env, &msg_info.sender) .is_err() { assert_module_installed(deps.as_ref(), &msg_info.sender, &module)?; @@ -216,13 +223,17 @@ fn remove_task( /// Refill a task fn refill_task( deps: Deps, - _env: Env, + env: Env, msg_info: MessageInfo, module: CroncatApp, task_tag: String, assets: AssetListUnchecked, ) -> CroncatResult { - if module.admin.assert_admin(deps, &msg_info.sender).is_err() { + if module + .admin + .assert_admin(deps, &env, &msg_info.sender) + .is_err() + { assert_module_installed(deps, &msg_info.sender, &module)?; } @@ -276,7 +287,7 @@ fn refill_task( fn purge( deps: DepsMut, - _env: Env, + env: Env, msg_info: MessageInfo, module: CroncatApp, task_tags: Vec, @@ -284,7 +295,7 @@ fn purge( // In case module got unregistered or admin got changed they have no reason to purge now if module .admin - .assert_admin(deps.as_ref(), &msg_info.sender) + .assert_admin(deps.as_ref(), &env, &msg_info.sender) .is_err() { assert_module_installed(deps.as_ref(), &msg_info.sender, &module)?; diff --git a/modules/contracts/apps/dca/src/handlers/execute.rs b/modules/contracts/apps/dca/src/handlers/execute.rs index f5d17303bf..805aed69dc 100644 --- a/modules/contracts/apps/dca/src/handlers/execute.rs +++ b/modules/contracts/apps/dca/src/handlers/execute.rs @@ -70,6 +70,7 @@ pub fn execute_handler( max_spread, } => update_config( deps, + env, info, module, native_asset, @@ -109,7 +110,7 @@ pub fn execute_handler( new_frequency, new_dex, ), - DCAExecuteMsg::CancelDCA { dca_id } => cancel_dca(deps, info, module, dca_id), + DCAExecuteMsg::CancelDCA { dca_id } => cancel_dca(deps, env, info, module, dca_id), DCAExecuteMsg::Convert { dca_id } => convert(deps, env, info, module, dca_id), } } @@ -117,6 +118,7 @@ pub fn execute_handler( /// Update the configuration of the app fn update_config( deps: DepsMut, + env: Env, msg_info: MessageInfo, module: DCAApp, new_native_asset: Option, @@ -125,7 +127,9 @@ fn update_config( new_max_spread: Option, ) -> AppResult { // Only the admin should be able to call this - module.admin.assert_admin(deps.as_ref(), &msg_info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &msg_info.sender)?; let old_config = CONFIG.load(deps.storage)?; let new_native_denom = new_native_asset .map(|asset| { @@ -163,7 +167,9 @@ fn create_dca( dex_name: DexName, ) -> AppResult { // Only the admin should be able to create dca - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let config = CONFIG.load(deps.storage)?; @@ -204,7 +210,9 @@ fn update_dca( new_frequency: Option, new_dex: Option, ) -> AppResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; // Only if frequency is changed we have to re-create a task let recreate_task = new_frequency.is_some(); @@ -238,8 +246,16 @@ fn update_dca( } /// Remove existing dca, remove task from cron_cat -fn cancel_dca(deps: DepsMut, info: MessageInfo, module: DCAApp, dca_id: DCAId) -> AppResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; +fn cancel_dca( + deps: DepsMut, + env: Env, + info: MessageInfo, + module: DCAApp, + dca_id: DCAId, +) -> AppResult { + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; DCA_LIST.remove(deps.storage, dca_id); diff --git a/modules/contracts/apps/payment/src/handlers/execute.rs b/modules/contracts/apps/payment/src/handlers/execute.rs index e7f54dc194..55268598ea 100644 --- a/modules/contracts/apps/payment/src/handlers/execute.rs +++ b/modules/contracts/apps/payment/src/handlers/execute.rs @@ -38,7 +38,15 @@ pub fn execute_handler( desired_asset, denom_asset, exchanges, - } => update_config(deps, info, module, desired_asset, denom_asset, exchanges), + } => update_config( + deps, + env, + info, + module, + desired_asset, + denom_asset, + exchanges, + ), AppExecuteMsg::Tip {} => tip(deps, env, info, module, None), } } @@ -176,6 +184,7 @@ fn update_tipper_history( /// Update the configuration of the app fn update_config( deps: DepsMut, + env: Env, msg_info: MessageInfo, module: PaymentApp, desired_asset: Option>, @@ -183,7 +192,9 @@ fn update_config( exchanges: Option>, ) -> AppResult { // Only the admin should be able to call this - module.admin.assert_admin(deps.as_ref(), &msg_info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &msg_info.sender)?; let name_service = module.name_service(deps.as_ref()); let mut config = CONFIG.load(deps.storage)?; diff --git a/modules/contracts/apps/subscription/src/handlers/execute.rs b/modules/contracts/apps/subscription/src/handlers/execute.rs index 96d9e4edd7..52faef3c26 100644 --- a/modules/contracts/apps/subscription/src/handlers/execute.rs +++ b/modules/contracts/apps/subscription/src/handlers/execute.rs @@ -293,7 +293,7 @@ pub fn claim_subscriber_emissions( #[allow(clippy::too_many_arguments)] pub fn update_subscription_config( deps: DepsMut, - _env: Env, + env: Env, info: MessageInfo, module: SubscriptionApp, payment_asset: Option, @@ -301,7 +301,9 @@ pub fn update_subscription_config( subscription_per_second_emissions: Option>, unsubscribe_hook_addr: Option>, ) -> SubscriptionResult { - module.admin.assert_admin(deps.as_ref(), &info.sender)?; + module + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let mut config: SubscriptionConfig = SUBSCRIPTION_CONFIG.load(deps.storage)?; diff --git a/modules/contracts/apps/subscription/tests/integration.rs b/modules/contracts/apps/subscription/tests/integration.rs index 99e7460775..5121d87393 100644 --- a/modules/contracts/apps/subscription/tests/integration.rs +++ b/modules/contracts/apps/subscription/tests/integration.rs @@ -352,7 +352,7 @@ fn claim_emissions_none() -> anyhow::Result<()> { } = setup_native(&mock, [(&subscriber1, sub_amount.as_slice())])?; subscription_app - .call_as(&subscription_app.account().address()?) + .call_as(&subscription_app.account().owner()?) .update_subscription_config(None, None, Some(EmissionType::None), None)?; // 1 user subscribe @@ -392,7 +392,7 @@ fn claim_emissions_week_per_user() -> anyhow::Result<()> { )?; subscription_app - .call_as(&subscription_app.account().address()?) + .call_as(&subscription_app.account().owner()?) .update_subscription_config( None, None, diff --git a/modules/contracts/standalones/ica-owner/src/contract.rs b/modules/contracts/standalones/ica-owner/src/contract.rs index b75792d0fb..69cf34f3b2 100644 --- a/modules/contracts/standalones/ica-owner/src/contract.rs +++ b/modules/contracts/standalones/ica-owner/src/contract.rs @@ -61,7 +61,7 @@ pub fn execute( ica_callback(deps, info, standalone, callback_msg) } MyStandaloneExecuteMsg::SendAction { ica_id, msg } => { - send_action(deps, info, standalone, ica_id, msg) + send_action(deps, env, info, standalone, ica_id, msg) } } } @@ -74,7 +74,9 @@ fn create_ica_contract( salt: Option, channel_open_init_options: ChannelOpenInitOptions, ) -> MyStandaloneResult { - standalone.admin.assert_admin(deps.as_ref(), &info.sender)?; + standalone + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let config = CONFIG.load(deps.storage)?; let ica_code = CwIcaControllerCode::new(config.ica_controller_code_id); @@ -148,12 +150,15 @@ pub fn ica_callback( /// Sends a predefined action to the ICA host. pub fn send_action( deps: DepsMut, + env: Env, info: MessageInfo, standalone: MyStandalone, ica_id: u64, msg: CosmosMsg, ) -> MyStandaloneResult { - standalone.admin.assert_admin(deps.as_ref(), &info.sender)?; + standalone + .admin + .assert_admin(deps.as_ref(), &env, &info.sender)?; let ica_state = ICA_STATES.load(deps.storage, ica_id)?;