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

Fix abstract-interface, abstract-client, abstract-testing #456

Merged
Merged
2 changes: 2 additions & 0 deletions framework/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions framework/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ version-control = { package = "abstract-version-control", path = "contracts/nati
ibc-client = { package = "abstract-ibc-client", path = "contracts/native/ibc-client" }
ibc-host = { package = "abstract-ibc-host", path = "contracts/native/ibc-host" }

proxy = { package = "abstract-proxy", path = "contracts/account/proxy", default-features = false }
manager = { package = "abstract-manager", path = "contracts/account/manager", default-features = false }
abstract-account = { path = "contracts/account", default-features = false }

abstract-ica = { path = "packages/abstract-ica" }
abstract-sdk = { version = "0.23.0", path = "packages/abstract-sdk" }
Expand Down
56 changes: 27 additions & 29 deletions framework/contracts/account/src/actions.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,37 @@
use abstract_sdk::std::{
account::state::{ADMIN, WHITELISTED_MODULES},
ibc_client::ExecuteMsg as IbcClientMsg,
IBC_CLIENT,
account::state::WHITELISTED_MODULES, ibc_client::ExecuteMsg as IbcClientMsg, IBC_CLIENT,
};
use abstract_std::ICA_CLIENT;
use abstract_std::{account::state::ACCOUNT_MODULES, objects::ownership, ICA_CLIENT};
use cosmwasm_std::{
wasm_execute, Binary, CosmosMsg, DepsMut, Empty, MessageInfo, StdError, SubMsg, WasmQuery,
wasm_execute, Addr, Binary, CosmosMsg, Deps, DepsMut, Empty, MessageInfo, StdError, SubMsg,
WasmQuery,
};

use crate::{
contract::{AccountResponse, AccountResult, RESPONSE_REPLY_ID},
error::AccountError,
};

/// 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)?;
if whitelisted_modules.0.contains(sender)
|| ownership::is_owner(deps.storage, &deps.querier, sender)?
{
Ok(())
} else {
Err(AccountError::SenderNotWhitelisted {})
}
}

/// Executes `Vec<CosmosMsg>` on the proxy.
/// Permission: Module
pub fn execute_module_action(
deps: DepsMut,
msg_info: MessageInfo,
msgs: Vec<CosmosMsg<Empty>>,
) -> AccountResult {
let whitelisted_modules = WHITELISTED_MODULES.load(deps.storage)?;
if !whitelisted_modules.0.contains(&msg_info.sender) {
return Err(AccountError::SenderNotWhitelisted {});
}
assert_whitelisted(deps.as_ref(), &msg_info.sender)?;

Ok(AccountResponse::action("execute_module_action").add_messages(msgs))
}
Expand All @@ -35,10 +43,7 @@ pub fn execute_module_action_response(
msg_info: MessageInfo,
msg: CosmosMsg<Empty>,
) -> AccountResult {
let whitelisted_modules = WHITELISTED_MODULES.load(deps.storage)?;
if !whitelisted_modules.0.contains(&msg_info.sender) {
return Err(AccountError::SenderNotWhitelisted {});
}
assert_whitelisted(deps.as_ref(), &msg_info.sender)?;

let submsg = SubMsg::reply_on_success(msg, RESPONSE_REPLY_ID);

Expand All @@ -52,16 +57,13 @@ pub fn execute_ibc_action(
msg_info: MessageInfo,
msg: IbcClientMsg,
) -> AccountResult {
let whitelisted_modules = WHITELISTED_MODULES.load(deps.storage)?;
if !whitelisted_modules.0.contains(&msg_info.sender) {
return Err(AccountError::SenderNotWhitelisted {});
}
let manager_address = ADMIN.get(deps.as_ref())?.unwrap();
let ibc_client_address = abstract_sdk::std::account::state::ACCOUNT_MODULES
.query(&deps.querier, manager_address, IBC_CLIENT)?
assert_whitelisted(deps.as_ref(), &msg_info.sender)?;

let ibc_client_address = ACCOUNT_MODULES
.may_load(deps.storage, IBC_CLIENT)?
.ok_or_else(|| {
StdError::generic_err(format!(
"ibc_client not found on manager. Add it under the {IBC_CLIENT} name."
"ibc_client not found on account. Add it under the {IBC_CLIENT} name."
))
})?;

Expand All @@ -83,17 +85,13 @@ pub fn execute_ibc_action(
///
/// The resulting `Vec<CosmosMsg>` are then executed on the proxy contract.
pub fn ica_action(deps: DepsMut, msg_info: MessageInfo, action_query: Binary) -> AccountResult {
let whitelisted_modules = WHITELISTED_MODULES.load(deps.storage)?;
if !whitelisted_modules.0.contains(&msg_info.sender) {
return Err(AccountError::SenderNotWhitelisted {});
}
assert_whitelisted(deps.as_ref(), &msg_info.sender)?;

let manager_address = ADMIN.get(deps.as_ref())?.unwrap();
let ica_client_address = abstract_sdk::std::account::state::ACCOUNT_MODULES
.query(&deps.querier, manager_address, ICA_CLIENT)?
let ica_client_address = ACCOUNT_MODULES
.may_load(deps.storage, ICA_CLIENT)?
.ok_or_else(|| {
StdError::generic_err(format!(
"ica_client not found on manager. Add it under the {ICA_CLIENT} name."
"ica_client not found on account. Add it under the {ICA_CLIENT} name."
))
})?;

Expand Down
45 changes: 23 additions & 22 deletions framework/contracts/account/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub const REGISTER_MODULES_DEPENDENCIES_REPLY_ID: u64 = 2;
#[cfg_attr(feature = "export", cosmwasm_std::entry_point)]
pub fn instantiate(
mut deps: DepsMut,
env: Env,
_env: Env,
info: MessageInfo,
InstantiateMsg {
account_id,
Expand Down Expand Up @@ -161,6 +161,26 @@ pub fn instantiate(
],
);

response = response.add_message(wasm_execute(
version_control_address,
&abstract_std::version_control::ExecuteMsg::AddAccount {
namespace,
creator: info.sender.to_string(),
},
vec![],
)?);

// Register on account if it's sub-account
if let GovernanceDetails::SubAccount { account } = cw_gov_owner.owner {
response = response.add_message(wasm_execute(
account,
&ExecuteMsg::UpdateSubAccount(UpdateSubAccountAction::RegisterSubAccount {
id: ACCOUNT_ID.load(deps.storage)?.seq(),
}),
vec![],
)?);
}

if !install_modules.is_empty() {
// Install modules
let (install_msgs, install_attribute) = _install_modules(
Expand All @@ -175,27 +195,6 @@ pub fn instantiate(
.add_attribute(install_attribute.key, install_attribute.value);
}

// Register on manager if it's sub-account
// TODO: Update sub-account creation logic
// if let GovernanceDetails::SubAccount { account } = cw_gov_owner.owner {
// response = response.add_message(wasm_execute(
// account,
// &ExecuteMsg::UpdateSubAccount(UpdateSubAccountAction::RegisterSubAccount {
// id: ACCOUNT_ID.load(deps.storage)?.seq(),
// }),
// vec![],
// )?);
// }

let response = response.add_message(wasm_execute(
version_control_address,
&abstract_std::version_control::ExecuteMsg::AddAccount {
namespace,
creator: info.sender.to_string(),
},
vec![],
)?);

Ok(response)
}

Expand Down Expand Up @@ -232,6 +231,7 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
link,
namespace,
install_modules,
account_id,
} => create_sub_account(
deps,
info,
Expand All @@ -241,6 +241,7 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
link,
namespace,
install_modules,
account_id,
)
.map_err(AccountError::from),
ExecuteMsg::Upgrade { modules } => {
Expand Down
13 changes: 8 additions & 5 deletions framework/contracts/account/src/sub_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use abstract_std::{
objects::{
gov_type::GovernanceDetails,
ownership::{self, GovOwnershipError},
AccountId,
salt, AccountId,
},
};
use cosmwasm_std::{
Expand All @@ -29,14 +29,17 @@ pub fn create_sub_account(
link: Option<String>,
namespace: Option<String>,
install_modules: Vec<ModuleInstallConfig>,
account_id: Option<u32>,
) -> AccountResult {
// only owner can create a subaccount
ownership::assert_nested_owner(deps.storage, &deps.querier, &info.sender)?;
let config = CONFIG.load(deps.storage)?;
let seq = abstract_std::version_control::state::LOCAL_ACCOUNT_SEQUENCE
.query(&deps.querier, config.version_control_address.clone())?;
let seq = account_id.unwrap_or(
abstract_std::version_control::state::LOCAL_ACCOUNT_SEQUENCE
.query(&deps.querier, config.version_control_address.clone())?,
);
let account_id = AccountId::local(seq);
let salt = cosmwasm_std::to_json_binary(&seq)?;
let salt = salt::generate_instantiate_salt(&account_id);

let self_code_id = deps
.querier
Expand All @@ -46,7 +49,7 @@ pub fn create_sub_account(
let self_canon_addr = deps.api.addr_canonicalize(env.contract.address.as_str())?;

let create_account_msg = abstract_std::account::InstantiateMsg {
account_id: None,
account_id: Some(account_id.clone()),
owner: GovernanceDetails::SubAccount {
account: env.contract.address.into_string(),
},
Expand Down
10 changes: 5 additions & 5 deletions framework/contracts/account/tests/subaccount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn updating_on_subaccount_should_succeed() -> AResult {
)?;

// Subaccount should have id 2 in this test, we try to update the config of this module
let account_contracts = get_account_contracts(&deployment.version_control, AccountId::local(2));
let account_contracts = get_account_contract(&deployment.version_control, AccountId::local(2));
let new_desc = "new desc";
account_contracts
.0
Expand Down Expand Up @@ -88,7 +88,7 @@ fn proxy_updating_on_subaccount_should_succeed() -> AResult {
)?;

// Subaccount should have id 2 in this test, we try to update the config of this module
let (sub_manager, _) = get_account_contracts(&deployment.version_control, AccountId::local(2));
let (sub_manager, _) = get_account_contract(&deployment.version_control, AccountId::local(2));
let new_desc = "new desc";

// We call as the proxy, it should also be possible
Expand Down Expand Up @@ -122,7 +122,7 @@ fn recursive_updating_on_subaccount_should_succeed() -> AResult {
)?;

// Subaccount should have id 2 in this test, we try to update the config of this module
let account_contracts = get_account_contracts(&deployment.version_control, AccountId::local(2));
let account_contracts = get_account_contract(&deployment.version_control, AccountId::local(2));

// We call as the manager, it should also be possible
account_contracts.0.create_sub_account(
Expand All @@ -134,7 +134,7 @@ fn recursive_updating_on_subaccount_should_succeed() -> AResult {
None,
&[],
)?;
let account_contracts = get_account_contracts(&deployment.version_control, AccountId::local(3));
let account_contracts = get_account_contract(&deployment.version_control, AccountId::local(3));
let new_desc = "new desc";

account_contracts
Expand Down Expand Up @@ -175,7 +175,7 @@ fn installed_app_updating_on_subaccount_should_succeed() -> AResult {
.add_modules(vec![mock_app.to_string()])?;

let (sub_manager, _sub_proxy) =
get_account_contracts(&deployment.version_control, AccountId::local(2));
get_account_contract(&deployment.version_control, AccountId::local(2));
let new_desc = "new desc";

// recover address on first proxy
Expand Down
2 changes: 1 addition & 1 deletion framework/contracts/native/version-control/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub fn add_account(
"add_account",
vec![
("account_id", account_id.to_string().as_str()),
("account", account.addr().as_str()),
("account_address", account.addr().as_str()),
("namespace", &format!("{namespace:?}")),
],
);
Expand Down
2 changes: 2 additions & 0 deletions framework/contracts/native/version-control/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,12 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> VCResult<Binary> {
}
QueryMsg::Config {} => {
let config = CONFIG.load(deps.storage)?;
let local_account_sequence = LOCAL_ACCOUNT_SEQUENCE.load(deps.storage)?;
to_json_binary(&ConfigResponse {
account_factory_address: config.account_factory_address,
security_disabled: config.security_disabled,
namespace_registration_fee: config.namespace_registration_fee,
local_account_sequence,
})
}
QueryMsg::ModuleList {
Expand Down
3 changes: 0 additions & 3 deletions framework/contracts/native/version-control/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ pub enum VCError {
#[error("Initialization funds can only be specified for apps and standalone modules")]
RedundantInitFunds {},

#[error("Predictable local account id sequence can't be lower than 2147483648")]
PredictableAccountIdFailed {},

#[error("Sender {0} is not the IBC host {1}")]
SenderNotIbcHost(String, String),

Expand Down
2 changes: 2 additions & 0 deletions framework/packages/abstract-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ workspace-hack = { version = "0.1", path = "../../workspace-hack" }
[dev-dependencies]
abstract-testing.workspace = true
abstract-client = { path = ".", features = ["test-utils", "interchain"] }
abstract-account = { workspace = true }
version-control = { workspace = true }
polytone = { workspace = true }
cw-asset.workspace = true
cw-controllers.workspace = true
Expand Down
21 changes: 8 additions & 13 deletions framework/packages/abstract-client/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
use std::fmt::{Debug, Display};

use abstract_interface::{
Abstract, AbstractInterfaceError, AccountDetails, AccountI, DependencyCreation, IbcClient,
InstallConfig, MFactoryQueryFns, ManagerExecFns, ManagerQueryFns, RegisteredModule, VCQueryFns,
Abstract, AbstractInterfaceError, AccountDetails, AccountExecFns, AccountI, AccountQueryFns,
DependencyCreation, IbcClient, InstallConfig, MFactoryQueryFns, RegisteredModule, VCQueryFns,
};
use abstract_std::{
account,
Expand All @@ -40,7 +40,7 @@ use abstract_std::{
AccountId,
},
version_control::{self, NamespaceResponse},
ACCOUNT, IBC_CLIENT,
IBC_CLIENT,
};
use cosmwasm_std::{to_json_binary, Attribute, Coins, CosmosMsg, Uint128};
use cw_orch::{
Expand Down Expand Up @@ -341,12 +341,10 @@ impl<'a, Chain: CwEnv> AccountBuilder<'a, Chain> {
link: self.link.clone(),
namespace: self.namespace.as_ref().map(ToString::to_string),
install_modules,
account_id: self.expected_local_account_id,
};
let abstract_account = match self.owner_account {
None => {
// https://github.com/AbstractSDK/abstract/pull/446#discussion_r1756768435
todo!()
}
None => AccountI::create(self.abstr, account_details, ownership, &funds)?,
Some(owner_account) => owner_account
.abstr_account
.create_sub_account_helper(account_details, &funds)?,
Expand Down Expand Up @@ -633,11 +631,7 @@ impl<Chain: CwEnv> Account<Chain> {
) -> AbstractClientResult<Chain::Response> {
let msgs = execute_msgs.into_iter().map(Into::into).collect();
self.configure(
&account::ExecuteMsg::ExecOnModule {
module_id: ACCOUNT.to_owned(),
exec_msg: to_json_binary(&abstract_std::account::ExecuteMsg::ModuleAction { msgs })
.map_err(AbstractInterfaceError::from)?,
},
&abstract_std::account::ExecuteMsg::ModuleAction { msgs },
funds,
)
}
Expand Down Expand Up @@ -838,6 +832,7 @@ impl<Chain: CwEnv> Account<Chain> {
None,
None,
None,
None,
funds,
)?;

Expand Down Expand Up @@ -996,7 +991,7 @@ pub mod test {
let abstr = AbstractClient::builder(mock.clone()).build()?;

let my_namespace = "my-namespace";
let new_account = abstr_builder().build()?;
let new_account = abstr.account_builder().build()?;
new_account.claim_namespace(my_namespace)?;

// Verify the namespace exists
Expand Down
Loading
Loading