Skip to content

Commit

Permalink
Changed account id structure (#58)
Browse files Browse the repository at this point in the history
* Changed account id structure

* Corrected tests

* formatting

* Cleanup and test

* Changelog

* Corrected imports

* Fixed tests

* improve ChainName construction strictness

* fix tests

* Cleanup and storage key shortening

* fix test

* clippy fix

* remove osmosis remenant

* bump cw-orch

* Hot fix for cw-orch 0.16.0

* Moved from_str to trait

* bump cw-orch

* fix tests

* formatting

* rm unused import

---------

Co-authored-by: cyberhoward <cyberhoward@protonmail.com>
Co-authored-by: CyberHoward <88450409+CyberHoward@users.noreply.github.com>
  • Loading branch information
3 people committed Sep 12, 2023
1 parent 5869bd8 commit c2ad664
Show file tree
Hide file tree
Showing 72 changed files with 1,228 additions and 356 deletions.
26 changes: 15 additions & 11 deletions framework/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,26 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased] - yyyy-mm-dd

### Added
- Install modules on account or Sub-account creation
- Manager stores his sub-accounts and sub-accounts can register or unregister in case of ownership change
- Query on module factory to see how much funds needs to be attached for installing modules
- Version control on instantiation to the Apps alongside with registry traits
- Instantiation funds added to module configuration, allowing modules to perform external setup calls

- Install modules on account or Sub-account creation.
- Manager stores his sub-accounts and sub-accounts can register or unregister in case of ownership change.
- Query on module factory to see how much funds needs to be attached for installing modules.
- Version control on instantiation to the Apps alongside with registry traits.
- Instantiation funds added to module configuration, allowing modules to perform external setup calls.

### Changed

- Updated fetch_data arguments of CwStakingCommand
- StakingInfoResponse now returns staking target(which is either contract address or pool id) instead of always staking contract address
- Owner of the sub-accounts now Proxy, allowing modules to interact with sub-accounts
- Install modules replaced install module method on module factory to reduce gas consumption for multi-install cases
- StakingInfoResponse now returns staking target(which is either contract address or pool id) instead of always staking contract address.
- Owner of the sub-accounts now Proxy, allowing modules to interact with sub-accounts.
- Install modules replaced install module method on module factory to reduce gas consumption for multi-install cases.
- Modified the account id structure. Each account is now identified with a unique ID and a trace. This is a requirement for Abstract IBC.

### Fixed
- Partially fixed cw-staking for Osmosis
- Manager governance now changes only after new "owner" claimed ownership
- Fixed and separated cw-staking and dex adapters for kujira

- Partially fixed cw-staking for Osmosis.
- Manager governance now changes only after new "owner" claimed ownership.
- Fixed and separated cw-staking and dex adapters for kujira.
- `ExecOnModule` calls now forward any provided funds to the module that is called.

## [0.17.2] - 2023-07-27
Expand Down
2 changes: 1 addition & 1 deletion framework/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ protobuf = { version = "2", features = ["with-bytes"] }
clap = { version = "4.0.32", features = ["derive"] }
semver = "1.0"
cw-semver = { version = "1.0" }
cw-orch = { version = "0.15.0" }
cw-orch = { version = "0.16.1" }
tokio = { version = "1.4", features = ["full"] }

## crates in order of publishing ## see docs/Publishing.md
Expand Down
18 changes: 11 additions & 7 deletions framework/contracts/account/manager/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use abstract_core::adapter::{
use abstract_core::manager::{InternalConfigAction, UpdateSubAccountAction};
use abstract_core::module_factory::ModuleInstallConfig;
use abstract_core::objects::gov_type::GovernanceDetails;
use abstract_core::objects::AssetEntry;
use abstract_core::objects::{AccountId, AssetEntry};

use abstract_core::objects::version_control::VersionControlContract;
use abstract_core::proxy::state::ACCOUNT_ID;
Expand Down Expand Up @@ -288,7 +288,7 @@ fn unregister_sub_account(deps: DepsMut, info: MessageInfo, id: u32) -> ManagerR
let account = abstract_core::version_control::state::ACCOUNT_ADDRESSES.query(
&deps.querier,
config.version_control_address,
id,
&AccountId::local(id),
)?;

if account.is_some_and(|a| a.manager == info.sender) {
Expand All @@ -310,7 +310,7 @@ fn register_sub_account(deps: DepsMut, info: MessageInfo, id: u32) -> ManagerRes
let account = abstract_core::version_control::state::ACCOUNT_ADDRESSES.query(
&deps.querier,
config.version_control_address,
id,
&AccountId::local(id),
)?;

if account.is_some_and(|a| a.manager == info.sender) {
Expand Down Expand Up @@ -424,10 +424,12 @@ pub(crate) fn update_governance(storage: &mut dyn Storage) -> ManagerResult<Vec<
if let GovernanceDetails::SubAccount { manager, .. } = acc_info.governance_details {
let id = ACCOUNT_ID.load(storage)?;
// For optimizing the gas we save it, in case new owner is sub-account as well
account_id = Some(id);
account_id = Some(id.clone());
let unregister_message = wasm_execute(
manager,
&ExecuteMsg::UpdateSubAccount(UpdateSubAccountAction::UnregisterSubAccount { id }),
&ExecuteMsg::UpdateSubAccount(UpdateSubAccountAction::UnregisterSubAccount {
id: id.clone().seq(),
}),
vec![],
)?;
msgs.push(unregister_message.into());
Expand All @@ -442,7 +444,9 @@ pub(crate) fn update_governance(storage: &mut dyn Storage) -> ManagerResult<Vec<
};
let register_message = wasm_execute(
manager,
&ExecuteMsg::UpdateSubAccount(UpdateSubAccountAction::RegisterSubAccount { id }),
&ExecuteMsg::UpdateSubAccount(UpdateSubAccountAction::RegisterSubAccount {
id: id.seq(),
}),
vec![],
)?;
msgs.push(register_message.into());
Expand Down Expand Up @@ -952,7 +956,7 @@ pub fn update_internal_config(
response = response.add_message(wasm_execute(
manager,
&ExecuteMsg::UpdateSubAccount(UpdateSubAccountAction::RegisterSubAccount {
id: ACCOUNT_ID.load(deps.storage)?,
id: ACCOUNT_ID.load(deps.storage)?.seq(),
}),
vec![],
)?);
Expand Down
7 changes: 5 additions & 2 deletions framework/contracts/account/manager/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ mod versioning;
#[cfg(test)]
mod test_common {
use crate::contract::ManagerResult;
use abstract_core::{manager, objects::gov_type::GovernanceDetails};
use abstract_core::{
manager,
objects::{account::AccountTrace, gov_type::GovernanceDetails, AccountId},
};
use abstract_testing::prelude::*;
use cosmwasm_std::{testing::*, DepsMut};

Expand All @@ -21,7 +24,7 @@ mod test_common {
mock_env(),
info,
manager::InstantiateMsg {
account_id: 1,
account_id: AccountId::new(1, AccountTrace::Local).unwrap(),
owner: GovernanceDetails::Monarchy {
monarch: TEST_OWNER.to_string(),
},
Expand Down
4 changes: 2 additions & 2 deletions framework/contracts/account/manager/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use abstract_sdk::core::manager::{
ModuleVersionsResponse,
};
use cosmwasm_std::{
to_binary, Addr, Binary, Deps, Env, Order, QueryRequest, StdError, StdResult, Uint64, WasmQuery,
to_binary, Addr, Binary, Deps, Env, Order, QueryRequest, StdError, StdResult, WasmQuery,
};
use cw2::{ContractVersion, CONTRACT};
use cw_storage_plus::Bound;
Expand All @@ -33,7 +33,7 @@ pub fn handle_account_info_query(deps: Deps) -> StdResult<Binary> {
}

pub fn handle_config_query(deps: Deps) -> StdResult<Binary> {
let account_id = Uint64::from(ACCOUNT_ID.load(deps.storage)?);
let account_id = ACCOUNT_ID.load(deps.storage)?;
let Config {
version_control_address,
module_factory_address,
Expand Down
2 changes: 1 addition & 1 deletion framework/contracts/account/manager/tests/adapters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ fn account_install_adapter() -> AResult {

deployment
.version_control
.claim_namespace(1, "tester".to_owned())?;
.claim_namespace(TEST_ACCOUNT_ID, "tester".to_owned())?;

let adapter = BootMockAdapter1V1::new_test(chain);
adapter.deploy(V1.parse().unwrap(), MockInitMsg)?;
Expand Down
3 changes: 2 additions & 1 deletion framework/contracts/account/manager/tests/apps.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod common;
use abstract_app::gen_app_mock;
use abstract_core::objects::account::TEST_ACCOUNT_ID;
use abstract_core::PROXY;
use abstract_interface::*;

Expand Down Expand Up @@ -69,7 +70,7 @@ fn account_install_app() -> AResult {

deployment
.version_control
.claim_namespace(1, "tester".to_owned())?;
.claim_namespace(TEST_ACCOUNT_ID, "tester".to_owned())?;

let app = MockApp::new_test(chain);
app.deploy(APP_VERSION.parse().unwrap())?;
Expand Down
9 changes: 5 additions & 4 deletions framework/contracts/account/manager/tests/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use abstract_core::objects::fee::FixedFee;
use abstract_core::objects::module::{ModuleInfo, ModuleVersion, Monetization};
use abstract_core::objects::module_reference::ModuleReference;
use abstract_core::objects::namespace::Namespace;
use abstract_core::objects::{AccountId, ABSTRACT_ACCOUNT_ID};
use abstract_core::version_control::UpdateModule;
use abstract_core::{manager::ManagerModuleInfo, PROXY};
use abstract_interface::*;
Expand Down Expand Up @@ -46,7 +47,7 @@ fn instantiate() -> AResult {
assert_that!(account.manager.config()?).is_equal_to(abstract_core::manager::ConfigResponse {
version_control_address: deployment.version_control.address()?,
module_factory_address: deployment.module_factory.address()?,
account_id: TEST_ACCOUNT_ID.into(),
account_id: TEST_ACCOUNT_ID,
is_suspended: false,
});
Ok(())
Expand Down Expand Up @@ -181,7 +182,7 @@ fn install_standalone_modules() -> AResult {
let sender = Addr::unchecked(common::OWNER);
let chain = Mock::new(&sender);
let deployment = Abstract::deploy_on(chain.clone(), sender.to_string())?;
let account = AbstractAccount::new(&deployment, Some(0));
let account = AbstractAccount::new(&deployment, Some(AccountId::local(0)));

let standalone1_contract = Box::new(ContractWrapper::new(
mock_modules::standalone_cw2::mock_execute,
Expand Down Expand Up @@ -236,7 +237,7 @@ fn install_standalone_versions_not_met() -> AResult {
let sender = Addr::unchecked(common::OWNER);
let chain = Mock::new(&sender);
let deployment = Abstract::deploy_on(chain.clone(), sender.to_string())?;
let account = AbstractAccount::new(&deployment, Some(0));
let account = AbstractAccount::new(&deployment, Some(AccountId::local(0)));

let standalone1_contract = Box::new(ContractWrapper::new(
mock_modules::standalone_cw2::mock_execute,
Expand Down Expand Up @@ -285,7 +286,7 @@ fn install_multiple_modules() -> AResult {
let chain = Mock::new(&sender);
chain.add_balance(&sender, vec![coin(86, "token1"), coin(500, "token2")])?;
let deployment = Abstract::deploy_on(chain.clone(), sender.to_string())?;
let account = AbstractAccount::new(&deployment, Some(0));
let account = AbstractAccount::new(&deployment, Some(ABSTRACT_ACCOUNT_ID));

let standalone1_contract = Box::new(ContractWrapper::new(
mock_modules::standalone_cw2::mock_execute,
Expand Down
40 changes: 23 additions & 17 deletions framework/contracts/account/manager/tests/subaccount.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
mod common;

use abstract_core::manager::SubAccountIdsResponse;
use abstract_core::objects::gov_type::GovernanceDetails;
use abstract_core::objects::{gov_type::GovernanceDetails, AccountId};

use abstract_interface::*;
use common::*;
use cosmwasm_std::{wasm_execute, Addr};
Expand Down Expand Up @@ -50,7 +51,8 @@ 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, Some(2));
let account_contracts =
get_account_contracts(&deployment.version_control, Some(AccountId::local(2)));
let new_desc = "new desc";
account_contracts
.0
Expand Down Expand Up @@ -81,7 +83,8 @@ 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, _sub_proxy) = get_account_contracts(&deployment.version_control, Some(2));
let (sub_manager, _) =
get_account_contracts(&deployment.version_control, Some(AccountId::local(2)));
let new_desc = "new desc";

// We call as the proxy, it should also be possible
Expand Down Expand Up @@ -113,7 +116,8 @@ 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, Some(2));
let account_contracts =
get_account_contracts(&deployment.version_control, Some(AccountId::local(2)));

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

account_contracts
Expand Down Expand Up @@ -162,7 +167,8 @@ fn installed_app_updating_on_subaccount_should_succeed() -> AResult {
.call_as(&account.manager.address()?)
.add_module(mock_app.to_string())?;

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

// recover address on first proxy
Expand Down Expand Up @@ -219,7 +225,7 @@ fn sub_account_move_ownership() -> AResult {
}
);

let sub_account = AbstractAccount::new(&deployment, Some(2));
let sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(2)));
sub_account.manager.set_owner(GovernanceDetails::Monarchy {
monarch: new_owner.to_string(),
})?;
Expand All @@ -244,7 +250,7 @@ fn sub_account_move_ownership() -> AResult {
&abstract_core::manager::ExecuteMsg::UpdateOwnership(cw_ownable::Action::AcceptOwnership),
None,
)?;
let account = AbstractAccount::new(&deployment, Some(1));
let account = AbstractAccount::new(&deployment, Some(AccountId::local(1)));

// After claim it's updated
let sub_accounts = account.manager.sub_account_ids(None, None)?;
Expand Down Expand Up @@ -273,7 +279,7 @@ fn account_move_ownership_to_sub_account() -> AResult {
None,
None,
)?;
let sub_account = AbstractAccount::new(&deployment, Some(2));
let sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(2)));
let sub_manager_addr = sub_account.manager.address()?;
let sub_proxy_addr = sub_account.proxy.address()?;

Expand All @@ -285,7 +291,7 @@ fn account_move_ownership_to_sub_account() -> AResult {
new_account.manager.set_owner(new_governance.clone())?;
let new_account_manager = new_account.manager.address()?;

let sub_account = AbstractAccount::new(&deployment, Some(2));
let sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(2)));
let mock_module = Addr::unchecked("mock_module");
sub_account
.proxy
Expand All @@ -308,7 +314,7 @@ fn account_move_ownership_to_sub_account() -> AResult {
assert_eq!(sub_ids.sub_accounts, vec![3]);

// owner of new_account updated
let new_account = AbstractAccount::new(&deployment, Some(3));
let new_account = AbstractAccount::new(&deployment, Some(AccountId::local(3)));
let info = new_account.manager.info()?.info;
assert_eq!(new_governance, info.governance_details.into());

Expand All @@ -330,7 +336,7 @@ fn sub_account_move_ownership_to_sub_account() -> AResult {
None,
None,
)?;
let sub_account = AbstractAccount::new(&deployment, Some(2));
let sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(2)));
let sub_manager_addr = sub_account.manager.address()?;
let sub_proxy_addr = sub_account.proxy.address()?;

Expand All @@ -348,7 +354,7 @@ fn sub_account_move_ownership_to_sub_account() -> AResult {
let sub_ids = new_account.manager.sub_account_ids(None, None)?;
assert_eq!(sub_ids.sub_accounts, vec![4]);

let new_account_sub_account = AbstractAccount::new(&deployment, Some(4));
let new_account_sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(4)));
let new_governance = GovernanceDetails::SubAccount {
manager: sub_manager_addr.to_string(),
proxy: sub_proxy_addr.to_string(),
Expand All @@ -358,7 +364,7 @@ fn sub_account_move_ownership_to_sub_account() -> AResult {
.set_owner(new_governance.clone())?;
let new_account_sub_account_manager = new_account_sub_account.manager.address()?;

let sub_account = AbstractAccount::new(&deployment, Some(2));
let sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(2)));
let mock_module = Addr::unchecked("mock_module");
sub_account
.proxy
Expand All @@ -379,12 +385,12 @@ fn sub_account_move_ownership_to_sub_account() -> AResult {
// sub-accounts state updated
let sub_ids = sub_account.manager.sub_account_ids(None, None)?;
assert_eq!(sub_ids.sub_accounts, vec![4]);
let new_account = AbstractAccount::new(&deployment, Some(3));
let new_account = AbstractAccount::new(&deployment, Some(AccountId::local(3)));
// removed from the previous owner as well
let sub_ids = new_account.manager.sub_account_ids(None, None)?;
assert_eq!(sub_ids.sub_accounts, Vec::<u32>::new());

let new_account_sub_account = AbstractAccount::new(&deployment, Some(4));
let new_account_sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(4)));
let info = new_account_sub_account.manager.info()?.info;
assert_eq!(new_governance, info.governance_details.into());
Ok(())
Expand All @@ -406,7 +412,7 @@ fn account_move_ownership_to_falsy_sub_account() -> AResult {
None,
None,
)?;
let sub_account = AbstractAccount::new(&deployment, Some(2));
let sub_account = AbstractAccount::new(&deployment, Some(AccountId::local(2)));
let sub_manager_addr = sub_account.manager.address()?;

let new_account = create_default_account(&deployment.account_factory)?;
Expand Down
3 changes: 2 additions & 1 deletion framework/contracts/account/manager/tests/upgrades.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use abstract_core::{
module::{ModuleInfo, ModuleVersion, Monetization},
module_reference::ModuleReference,
namespace::Namespace,
AccountId,
},
version_control::UpdateModule,
AbstractError,
Expand Down Expand Up @@ -632,7 +633,7 @@ fn create_sub_account_with_installed_module() -> AResult {
None,
)?;

let account = AbstractAccount::new(&deployment, Some(2));
let account = AbstractAccount::new(&deployment, Some(AccountId::local(2)));

// Make sure all installed
let account_module_versions = account.manager.module_versions(vec![
Expand Down
2 changes: 1 addition & 1 deletion framework/contracts/account/proxy/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use abstract_core::objects::oracle::Oracle;
use abstract_macros::abstract_response;
use abstract_sdk::{
core::{
objects::account_id::ACCOUNT_ID,
objects::account::ACCOUNT_ID,
proxy::{
state::{State, ADMIN, ANS_HOST, STATE},
AssetConfigResponse, ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg,
Expand Down
Loading

0 comments on commit c2ad664

Please sign in to comment.