Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Adapter Access Control Logic #458

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
03c4a6b
Added local Action on account
Kayanski Sep 16, 2024
e1e3663
Make sure the state is erasedé
Kayanski Sep 16, 2024
92fc1f2
Change to actual impl
Kayanski Sep 16, 2024
0acd9ea
Merge
Kayanski Sep 16, 2024
348a2f4
formatting [skip ci]
Kayanski Sep 16, 2024
6433389
Renamed local to account action
Kayanski Sep 16, 2024
3bfc41f
Renamed
Kayanski Sep 16, 2024
27d86c1
Added sdk method for admin checks
Kayanski Sep 17, 2024
387334b
Merge
Kayanski Sep 17, 2024
3c98507
Rename
Kayanski Sep 17, 2024
2a2dad7
Nits
Kayanski Sep 17, 2024
ddc778a
is_admin now takes the call_to context into account
Kayanski Sep 18, 2024
e570ad9
admin_addr instead of env in nestedadmin
Kayanski Sep 23, 2024
e218ffd
Apply github suggestinos
Kayanski Sep 23, 2024
9cb93e6
Merge with develop/v2
Kayanski Sep 23, 2024
03576b5
re-order execute messages in account
CyberHoward Sep 23, 2024
95829f6
add doc-comments
CyberHoward Sep 23, 2024
c96e5a8
formatting [skip ci]
CyberHoward Sep 23, 2024
b7279b8
fix missing permission check and nits
CyberHoward Sep 23, 2024
2ba5280
formatting [skip ci]
CyberHoward Sep 23, 2024
24b6346
undo added authentication
CyberHoward Sep 23, 2024
68f3854
update state assertion fn name to `assert_account_calling_to_as_admin…
CyberHoward Sep 23, 2024
c8ddfed
formatting [skip ci]
CyberHoward Sep 23, 2024
9efd64a
nit renames
CyberHoward Sep 23, 2024
37baf29
formatting [skip ci]
CyberHoward Sep 23, 2024
93754b4
move abstract specific function to abstract trait
CyberHoward Sep 23, 2024
d7069b8
rename
CyberHoward Sep 23, 2024
4f08116
readme
Kayanski Sep 23, 2024
508042e
Moved to env instead of self_addr inside assert_admin
Kayanski Sep 24, 2024
314b22a
Doing so inside modules
Kayanski Sep 24, 2024
9ffb16c
Merge with base
Kayanski Oct 1, 2024
eff33bc
readme nit
CyberHoward Oct 1, 2024
3d659e8
fix manager usage
CyberHoward Oct 1, 2024
4528609
fix test compilation
CyberHoward Oct 1, 2024
6b5326d
fix cargo hack
CyberHoward Oct 1, 2024
9484a89
remove unused dep
CyberHoward Oct 1, 2024
e3a7334
rm ignored dep
CyberHoward Oct 1, 2024
4d0e906
fix module tests
CyberHoward Oct 2, 2024
bdcf977
free disk space on framework test
CyberHoward Oct 2, 2024
74064d3
lower deposit amount
Buckram123 Oct 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 67 additions & 10 deletions framework/contracts/account/src/actions.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,42 @@
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, ADMIN_CALL_TO_CONTEXT},
objects::ownership,
ICA_CLIENT,
};
use cosmwasm_std::{
wasm_execute, Addr, Binary, CosmosMsg, Deps, DepsMut, Empty, 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,
};

/// Execute the [`exec_msg`] on the provided [`module_id`],
/// This is a simple wrapper around `LocalAction`
pub fn exec_on_module(
deps: DepsMut,
info: MessageInfo,
module_id: String,
exec_msg: Binary,
) -> AccountResult {
let module_addr = load_module_addr(deps.storage, &module_id)?;

execute_account_action(
deps,
&info.sender,
vec![CosmosMsg::Wasm(WasmMsg::Execute {
contract_addr: module_addr.into(),
msg: exec_msg,
funds: info.funds,
})],
)
}
/// Check that sender either whitelisted or governance
pub(crate) fn assert_whitelisted(deps: Deps, sender: &Addr) -> AccountResult<()> {
let whitelisted_modules = WHITELISTED_MODULES.load(deps.storage)?;
Expand All @@ -26,30 +51,62 @@ pub(crate) fn assert_whitelisted(deps: Deps, sender: &Addr) -> AccountResult<()>

/// Executes `Vec<CosmosMsg>` on the proxy.
/// Permission: Module
pub fn execute_module_action(
pub fn execute_account_action(
deps: DepsMut,
msg_info: MessageInfo,
msg_sender: &Addr,
msgs: Vec<CosmosMsg<Empty>>,
) -> AccountResult {
assert_whitelisted(deps.as_ref(), &msg_info.sender)?;
assert_whitelisted(deps.as_ref(), 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_account_action_response(
deps: DepsMut,
msg_info: MessageInfo,
msg_sender: &Addr,
msg: CosmosMsg<Empty>,
) -> AccountResult {
assert_whitelisted(deps.as_ref(), &msg_info.sender)?;
assert_whitelisted(deps.as_ref(), 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))
}

pub fn admin_account_action(
deps: DepsMut,
info: MessageInfo,
addr: Addr,
exec_msg: Binary,
) -> AccountResult {
ownership::assert_nested_owner(deps.storage, &deps.querier, &info.sender)?;

ADMIN_CALL_TO_CONTEXT.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_account_action").add_submessage(msg))
}

pub fn exec_admin_on_module(
deps: DepsMut,
info: MessageInfo,
module_id: String,
exec_msg: Binary,
) -> AccountResult {
let module_addr = load_module_addr(deps.storage, &module_id)?;
admin_account_action(deps, info, module_addr, exec_msg)
}

/// Executes IBC actions on the IBC client.
/// Permission: Module
pub fn execute_ibc_action(
Expand Down
56 changes: 30 additions & 26 deletions framework/contracts/account/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,24 @@ use abstract_std::{
AccountId,
},
version_control::state::LOCAL_ACCOUNT_SEQUENCE,
version_control::Account,
};
use cosmwasm_std::{
ensure_eq, wasm_execute, Addr, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response,
StdError, StdResult, SubMsgResult,
StdResult,
};

pub use crate::migrate::migrate;
use crate::{
actions::{
execute_ibc_action, execute_module_action, execute_module_action_response, ica_action,
admin_account_action, exec_admin_on_module, exec_on_module, execute_account_action,
execute_account_action_response, execute_ibc_action, ica_action,
},
config::{
remove_account_from_contracts, update_account_status, update_info, update_internal_config,
},
error::AccountError,
modules::{
_install_modules, exec_on_module, install_modules,
_install_modules, install_modules,
migration::{handle_callback, upgrade_modules},
uninstall_module, MIGRATE_CONTEXT,
},
Expand All @@ -44,7 +44,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,
},
Expand All @@ -57,8 +57,9 @@ pub type AccountResult<R = Response> = Result<R, AccountError>;

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(
Expand Down Expand Up @@ -221,10 +222,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,
Expand Down Expand Up @@ -284,12 +281,18 @@ 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::AccountActions { msgs } => {
execute_account_action(deps, &info.sender, msgs).map_err(AccountError::from)
}
ExecuteMsg::ModuleActionWithData { msg } => {
execute_module_action_response(deps, info, msg).map_err(AccountError::from)
ExecuteMsg::AccountActionWithData { msg } => {
execute_account_action_response(deps, &info.sender, msg)
.map_err(AccountError::from)
}

ExecuteMsg::ExecOnModule {
module_id,
exec_msg,
} => exec_on_module(deps, info, module_id, exec_msg).map_err(AccountError::from),
Buckram123 marked this conversation as resolved.
Show resolved Hide resolved
ExecuteMsg::IbcAction { msg } => {
execute_ibc_action(deps, info, msg).map_err(AccountError::from)
}
Expand All @@ -300,24 +303,25 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
unreachable!("Update status case is reached above")
}
ExecuteMsg::Callback(_) => handle_callback(deps, env, info),
ExecuteMsg::AdminExecOnModule { module_id, msg } => {
exec_admin_on_module(deps, info, module_id, msg)
}
ExecuteMsg::AdminAccountAction { addr, msg } => {
let addr = deps.api.addr_validate(&addr)?;
admin_account_action(deps, info, addr, msg)
}
}
}
}
}

#[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 {}),
}
}
Expand Down
25 changes: 1 addition & 24 deletions framework/contracts/account/src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use abstract_std::{
};
use cosmwasm_std::{
ensure, wasm_execute, Addr, Attribute, Binary, Coin, CosmosMsg, Deps, DepsMut, MessageInfo,
StdResult, Storage, SubMsg, WasmMsg,
StdResult, Storage, SubMsg,
};
use cw2::ContractVersion;
use cw_storage_plus::Item;
Expand Down Expand Up @@ -234,29 +234,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(
CyberHoward marked this conversation as resolved.
Show resolved Hide resolved
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<Addr> {
ACCOUNT_MODULES
Expand Down
21 changes: 14 additions & 7 deletions framework/contracts/account/src/reply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::ADMIN_CALL_TO_CONTEXT,
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 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(
Expand All @@ -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 fn admin_action_reply(deps: DepsMut) -> AccountResult {
ADMIN_CALL_TO_CONTEXT.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)?;
Expand Down
5 changes: 1 addition & 4 deletions framework/contracts/native/ibc-host/src/account_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,7 @@ pub fn send_all_back(
// call the message to send everything back through the manager
let manager_msg = wasm_execute(
account.into_addr(),
&account::ExecuteMsg::ExecOnModule {
module_id: ACCOUNT.into(),
exec_msg: to_json_binary(&account::ExecuteMsg::ModuleAction { msgs })?,
},
&account::ExecuteMsg::AccountActions { msgs },
vec![],
)?;
Ok(manager_msg.into())
Expand Down
12 changes: 6 additions & 6 deletions framework/packages/abstract-adapter/src/endpoints/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<Error: ContractError, CustomInitMsg, CustomExecMsg, CustomQueryMsg, SudoMsg
fn base_execute(
&mut self,
deps: DepsMut,
_env: Env,
env: Env,
info: MessageInfo,
message: BaseExecuteMsg,
) -> AdapterResult {
Expand Down Expand Up @@ -89,13 +89,13 @@ impl<Error: ContractError, CustomInitMsg, CustomExecMsg, CustomQueryMsg, SudoMsg
});
}
}
// If not provided the sender must be the direct owner
None => account_registry.assert_account(&info.sender).map_err(|_| {
AdapterError::UnauthorizedAdapterRequest {
// If not provided the sender must be the direct owner AND have admin execution rights
None => account_registry
.assert_account_admin(&env, &info.sender)
.map_err(|_| AdapterError::UnauthorizedAdapterRequest {
adapter: self.module_id().to_string(),
sender: info.sender.to_string(),
}
})?,
})?,
};
self.target_account = Some(account_base);
match msg {
Expand Down
5 changes: 1 addition & 4 deletions framework/packages/abstract-client/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,10 +630,7 @@ impl<Chain: CwEnv> Account<Chain> {
funds: &[Coin],
) -> AbstractClientResult<Chain::Response> {
let msgs = execute_msgs.into_iter().map(Into::into).collect();
self.configure(
&abstract_std::account::ExecuteMsg::ModuleAction { msgs },
funds,
)
self.configure(&account::ExecuteMsg::AccountActions { msgs }, funds)
Kayanski marked this conversation as resolved.
Show resolved Hide resolved
}

/// Executes a [`account::ExecuteMsg`] on the account.
Expand Down
4 changes: 2 additions & 2 deletions framework/packages/abstract-sdk/src/apis/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl<'a, T: Execution> Executor<'a, T> {
fn execute_with_data(&self, msg: CosmosMsg) -> AbstractSdkResult<ExecutorMsg> {
let msg = self.base.execute_on_account(
self.deps,
&ExecuteMsg::ModuleActionWithData { msg },
&ExecuteMsg::AccountActionWithData { msg },
vec![],
)?;
Ok(ExecutorMsg(msg))
Expand Down Expand Up @@ -110,7 +110,7 @@ impl<'a, T: Execution> Executor<'a, T> {
.collect();
let msg =
self.base
.execute_on_account(self.deps, &ExecuteMsg::ModuleAction { msgs }, funds)?;
.execute_on_account(self.deps, &ExecuteMsg::AccountActions { msgs }, funds)?;
Ok(ExecutorMsg(msg))
}

Expand Down
Loading
Loading