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

Changed account id structure #58

Merged
merged 22 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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