Skip to content

Commit

Permalink
Update documentiation and code-naming (#483)
Browse files Browse the repository at this point in the history
* update docs

* remove occurrences of account factory

* rename all references to version control to registry in docs

* rid documentation of "manager"

* get rid of references to "proxy"

* rename comments with "version control" in them

* rename mentions of VC to Registry

* update `manager` to account in code comments and variables

* formatting [skip ci]

* partially replace use of "proxy"

* formatting [skip ci]

* further fix use of "proxy" in codebase

* finish migration to "account" from "proxy"

* fix unused warnings for non-xion compilation

* formatting [skip ci]

* rename uses of `Proxy`

* rename uses of "Manager"

* Last renames and test fixes

* formatting [skip ci]
  • Loading branch information
CyberHoward authored Oct 7, 2024
1 parent 317a969 commit b4b9bc0
Show file tree
Hide file tree
Showing 138 changed files with 1,203 additions and 941 deletions.
6 changes: 3 additions & 3 deletions framework/contracts/account/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,14 @@ pub fn instantiate(
mut deps: DepsMut,
env: Env,
info: MessageInfo,
InstantiateMsg {
#[cfg_attr(not(feature = "xion"), allow(unused_variables))] InstantiateMsg {
account_id,
owner,
install_modules,
name,
description,
link,
namespace,

authenticator,
}: InstantiateMsg,
) -> AccountResult {
Expand Down Expand Up @@ -123,7 +122,7 @@ pub fn instantiate(
.clone()
.verify(deps.as_ref(), registry.address.clone())?;
match governance {
// Check if the caller is the manager the proposed owner account when creating a sub-account.
// Check if the caller is the proposed owner account when creating a sub-account.
// This prevents other users from creating sub-accounts for accounts they don't own.
GovernanceDetails::SubAccount { account } => {
ensure_eq!(
Expand Down Expand Up @@ -409,6 +408,7 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult<Binary> {
QueryMsg::Ownership {} => {
cosmwasm_std::to_json_binary(&ownership::get_ownership(deps.storage)?)
}
#[cfg_attr(not(feature = "xion"), allow(unused_variables))]
QueryMsg::AuthenticatorByID { id } => {
#[cfg(feature = "xion")]
{
Expand Down
26 changes: 13 additions & 13 deletions framework/contracts/account/src/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(crate) fn assert_whitelisted_or_owner(deps: &mut DepsMut, sender: &Addr) ->
}
}

/// Executes `Vec<CosmosMsg>` on the proxy.
/// Executes `Vec<CosmosMsg>` on the account.
/// Permission: Module
pub fn execute_msgs(
mut deps: DepsMut,
Expand All @@ -51,7 +51,7 @@ pub fn execute_msgs(
Ok(AccountResponse::action("execute_module_action").add_messages(msgs))
}

/// Executes `CosmosMsg` on the proxy and forwards its response.
/// Executes `CosmosMsg` on the account and forwards its response.
/// Permission: Module
pub fn execute_msgs_with_data(
mut deps: DepsMut,
Expand Down Expand Up @@ -173,10 +173,10 @@ pub fn remove_auth_method(_deps: DepsMut, _env: Env, _id: u8) -> AccountResult {
/// Execute an action on an ICA.
/// Permission: Module
///
/// This function queries the `abstract:ica-client` contract from the account's manager.
/// This function queries the `abstract:ica-client` contract from the account.
/// It then fires a smart-query on that address of type [`QueryMsg::IcaAction`](abstract_ica::msg::QueryMsg).
///
/// The resulting `Vec<CosmosMsg>` are then executed on the proxy contract.
/// The resulting `Vec<CosmosMsg>` are then executed on the account contract.
pub fn ica_action(mut deps: DepsMut, msg_info: MessageInfo, action_query: Binary) -> AccountResult {
assert_whitelisted_or_owner(&mut deps, &msg_info.sender)?;

Expand Down Expand Up @@ -309,12 +309,12 @@ mod test {
)
.unwrap_err();

let manager_info = message_info(abstr.account.addr(), &[]);
let account_info = message_info(abstr.account.addr(), &[]);
// ibc not enabled
execute(
deps.as_mut(),
env.clone(),
manager_info.clone(),
account_info.clone(),
msg.clone(),
)
.unwrap_err();
Expand All @@ -334,7 +334,7 @@ mod test {
vec![],
)?;

let res = execute(deps.as_mut(), env, manager_info, msg)?;
let res = execute(deps.as_mut(), env, account_info, msg)?;
assert_that(&res.messages).has_length(1);
assert_that!(res.messages[0]).is_equal_to(SubMsg::new(CosmosMsg::Wasm(
cosmwasm_std::WasmMsg::Execute {
Expand Down Expand Up @@ -380,12 +380,12 @@ mod test {
)
.unwrap_err();

let manager_info = message_info(abstr.account.addr(), &[]);
let account_info = message_info(abstr.account.addr(), &[]);
// ibc not enabled
execute(
deps.as_mut(),
env.clone(),
manager_info.clone(),
account_info.clone(),
msg.clone(),
)
.unwrap_err();
Expand All @@ -404,7 +404,7 @@ mod test {
vec![],
)?;

let res = execute(deps.as_mut(), env, manager_info, msg)?;
let res = execute(deps.as_mut(), env, account_info, msg)?;
assert_that(&res.messages).has_length(1);
assert_that!(res.messages[0]).is_equal_to(SubMsg::new(CosmosMsg::Wasm(
cosmwasm_std::WasmMsg::Execute {
Expand Down Expand Up @@ -457,12 +457,12 @@ mod test {
)
.unwrap_err();

let manager_info = message_info(abstr.account.addr(), &[]);
let account_info = message_info(abstr.account.addr(), &[]);
// ibc not enabled
execute(
deps.as_mut(),
env.clone(),
manager_info.clone(),
account_info.clone(),
msg.clone(),
)
.unwrap_err();
Expand All @@ -486,7 +486,7 @@ mod test {
})
.build();

let res = execute(deps.as_mut(), env, manager_info, msg)?;
let res = execute(deps.as_mut(), env, account_info, msg)?;
assert_that(&res.messages).has_length(1);
assert_that!(res.messages[0]).is_equal_to(SubMsg::new(CosmosMsg::Custom(Empty {})));
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion framework/contracts/account/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ mod test_common {

use crate::{contract::AccountResult, error::AccountError, msg::ExecuteMsg};

/// Initialize the manager with the test owner as the owner
/// Initialize the account with the test owner as the owner
pub(crate) fn mock_init(
deps: &mut OwnedDeps<MockStorage, MockApi, MockQuerier, Empty>,
) -> AccountResult {
Expand Down
24 changes: 12 additions & 12 deletions framework/contracts/account/src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ pub fn install_modules(
}

/// Generate message and attribute for installing module
/// Adds the modules to the internal store for reference and adds them to the proxy allowlist if applicable.
/// Adds the modules to the internal store for reference and adds them to the account allowlist if applicable.
pub fn _install_modules(
mut deps: DepsMut,
env: &Env,
modules: Vec<ModuleInstallConfig>,
funds: Vec<Coin>,
) -> AccountResult<(Vec<SubMsg>, Attribute)> {
let mut installed_modules = Vec::with_capacity(modules.len());
let mut manager_modules = Vec::with_capacity(modules.len());
let mut account_modules = Vec::with_capacity(modules.len());
let account_id = ACCOUNT_ID.load(deps.storage)?;

let registry = RegistryContract::new(deps.api, env)?;
Expand All @@ -86,7 +86,7 @@ pub fn _install_modules(

let mut install_context = Vec::with_capacity(modules.len());
let mut add_to_whitelist: Vec<Addr> = Vec::with_capacity(modules.len());
let mut add_to_manager: Vec<(String, Addr)> = Vec::with_capacity(modules.len());
let mut add_to_account: Vec<(String, Addr)> = Vec::with_capacity(modules.len());

let salt: Binary = generate_instantiate_salt(&account_id);
for (ModuleResponse { module, .. }, init_msg) in modules.into_iter().zip(init_msgs) {
Expand All @@ -106,7 +106,7 @@ pub fn _install_modules(
if module.should_be_whitelisted() {
add_to_whitelist.push(module_address.clone());
}
add_to_manager.push((module.info.id(), module_address.clone()));
add_to_account.push((module.info.id(), module_address.clone()));
install_context.push((module.clone(), None));
None
}
Expand All @@ -127,14 +127,14 @@ pub fn _install_modules(
if module.should_be_whitelisted() {
add_to_whitelist.push(module_address.clone());
}
add_to_manager.push((module.info.id(), module_address.clone()));
add_to_account.push((module.info.id(), module_address.clone()));
install_context.push((module.clone(), Some(module_address)));

Some(init_msg.unwrap())
}
_ => return Err(AccountError::ModuleNotInstallable(module.info.to_string())),
};
manager_modules.push(FactoryModuleInstallConfig::new(module.info, init_msg_salt));
account_modules.push(FactoryModuleInstallConfig::new(module.info, init_msg_salt));
}
_whitelist_modules(deps.branch(), add_to_whitelist)?;

Expand All @@ -143,14 +143,14 @@ pub fn _install_modules(
let mut messages = vec![];

// Update module addrs
update_module_addresses(deps.branch(), add_to_manager, vec![])?;
update_module_addresses(deps.branch(), add_to_account, vec![])?;

// Install modules message
messages.push(SubMsg::reply_on_success(
wasm_execute(
module_factory.address,
&ModuleFactoryMsg::InstallModules {
modules: manager_modules,
modules: account_modules,
salt,
},
funds,
Expand Down Expand Up @@ -213,7 +213,7 @@ pub fn uninstall_module(
let module_dependencies = module_data.dependencies;
crate::versioning::remove_as_dependent(deps.storage, &module_id, module_dependencies)?;

// Remove for proxy if needed
// Remove for account if needed
let vc = RegistryContract::new(deps.api, env)?;

let module = vc.query_module(
Expand Down Expand Up @@ -481,7 +481,7 @@ mod tests {
};
let msg = ExecuteMsg::UpdateInternalConfig(action_add);

// the version control can not call this
// the registry can not call this
let res = execute_as(&mut deps, &abstr.registry, msg.clone());
assert_that!(&res).is_err();

Expand Down Expand Up @@ -707,7 +707,7 @@ mod tests {
// modules: vec![test_module_addr.to_string()],
// };

// // -1 because manager counts as module as well
// // -1 because account counts as module as well
// for i in 0..LIST_SIZE_LIMIT - 1 {
// let test_module = format!("module_{i}");
// let test_module_addr = deps.api.addr_make(&test_module);
Expand Down Expand Up @@ -748,7 +748,7 @@ mod tests {
// }

// #[test]
// fn remove_module() -> ProxyTestResult {
// fn remove_module() -> AccountTestResult {
// let mut deps = mock_dependencies();
// mock_init(&mut deps);

Expand Down
16 changes: 8 additions & 8 deletions framework/contracts/account/src/modules/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub fn upgrade_modules(
&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
// Set the migrate messages for each module that's not the account and update the dependency store
for (module_info, migrate_msg) in modules {
let module_id = module_info.id();

Expand Down Expand Up @@ -127,7 +127,7 @@ pub fn set_migrate_msgs_and_context(
)?;

let migrate_msgs = match requested_module.module.reference {
// upgrading an adapter is done by moving the authorized addresses to the new contract address and updating the permissions on the proxy.
// upgrading an adapter is done by moving the authorized addresses to the new contract address and updating the permissions on the account.
ModuleReference::Adapter(new_adapter_addr) => handle_adapter_migration(
deps,
env,
Expand Down Expand Up @@ -254,7 +254,7 @@ pub fn replace_adapter(
} = deps.querier.query_wasm_smart(
old_adapter_addr.to_string(),
&<AdapterQuery<Empty>>::Base(BaseQueryMsg::AuthorizedAddresses {
proxy_address: env.contract.address.to_string(),
account_address: env.contract.address.to_string(),
}),
)?;
let authorized_to_migrate: Vec<String> = authorized_addresses
Expand All @@ -277,17 +277,17 @@ pub fn replace_adapter(
to_remove: vec![],
},
)?);
// Remove adapter permissions from proxy
// Remove adapter permissions from account
_remove_whitelist_modules(deps.branch(), vec![old_adapter_addr])?;
// Add new adapter to proxy
// Add new adapter to account
_whitelist_modules(deps.branch(), vec![new_adapter_addr])?;

Ok(msgs)
}

/// Generate message for upgrading account
///
/// Safety: Account cannot be upgraded to contract that is not confirmed by version control
/// Safety: Account cannot be upgraded to contract that is not confirmed by registry
pub(crate) fn self_upgrade_msg(
deps: DepsMut,
env: &Env,
Expand All @@ -296,10 +296,10 @@ pub(crate) fn self_upgrade_msg(
) -> AccountResult<CosmosMsg> {
let contract = get_contract_version(deps.storage)?;
let module = query_module(deps.as_ref(), env, module_info.clone(), Some(contract))?;
if let ModuleReference::Account(manager_code_id) = module.module.reference {
if let ModuleReference::Account(account_code_id) = module.module.reference {
let migration_msg: CosmosMsg<Empty> = CosmosMsg::Wasm(WasmMsg::Migrate {
contract_addr: env.contract.address.to_string(),
new_code_id: manager_code_id,
new_code_id: account_code_id,
msg: migrate_msg,
});
Ok(migration_msg)
Expand Down
4 changes: 2 additions & 2 deletions framework/contracts/account/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub fn query_module_versions(
Ok(module_versions)
}

/// RawQuery module addresses from manager
/// RawQuery module addresses from account
/// Errors if not present
pub fn query_module_addresses(
deps: Deps,
Expand All @@ -183,7 +183,7 @@ pub fn query_module_addresses(

// Query over
for module in module_names {
// Add to map if present, skip otherwise. Allows version control to check what modules are present.
// Add to map if present, skip otherwise. Allows registry to check what modules are present.
if let Some(address) = ACCOUNT_MODULES.may_load(deps.storage, &module)? {
modules.insert(module, address);
}
Expand Down
4 changes: 2 additions & 2 deletions framework/contracts/account/src/sub_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ pub fn maybe_update_sub_account_governance(deps: DepsMut) -> AccountResult<Vec<C
.pending_owner
.ok_or(GovOwnershipError::TransferNotFound)?;

// Clear state for previous manager if it was sub-account
// Clear state for previous account if it was sub-account
if let GovernanceDetails::SubAccount { account } = ownership.owner {
let id = ACCOUNT_ID.load(deps.storage)?;
let unregister_message = wasm_execute(
Expand All @@ -168,7 +168,7 @@ pub fn maybe_update_sub_account_governance(deps: DepsMut) -> AccountResult<Vec<C
msgs.push(unregister_message.into());
}

// Update state for new manager if owner will be the sub-account
// Update state for new account if owner will be the sub-account
if let GovernanceDetails::SubAccount { account } = &pending_governance {
let id = if let Some(id) = account_id {
id
Expand Down
Loading

0 comments on commit b4b9bc0

Please sign in to comment.