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 10 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
1 change: 1 addition & 0 deletions framework/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- 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. This is a requirement for Abstract IBC

### Fixed
- Partially fixed cw-staking for Osmosis
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
3 changes: 2 additions & 1 deletion framework/contracts/account/proxy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub mod reply;
#[cfg(test)]
mod test_common {
use crate::contract;
use abstract_core::objects::account::TEST_ACCOUNT_ID;
use abstract_core::proxy::InstantiateMsg;
use abstract_testing::prelude::*;
use cosmwasm_std::testing::{mock_env, mock_info, MOCK_CONTRACT_ADDR};
Expand All @@ -15,7 +16,7 @@ mod test_common {
pub fn mock_init(deps: DepsMut) {
let info = mock_info(TEST_MANAGER, &[]);
let msg = InstantiateMsg {
account_id: 0,
account_id: TEST_ACCOUNT_ID,
ans_host_address: MOCK_CONTRACT_ADDR.to_string(),
};
let _res = contract::instantiate(deps, mock_env(), info, msg).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion framework/contracts/account/proxy/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ mod test {
fn mock_init(deps: DepsMut) {
let info = mock_info(TEST_CREATOR, &[]);
let msg = InstantiateMsg {
account_id: 0,
account_id: TEST_ACCOUNT_ID,
ans_host_address: TEST_ANS_HOST.to_string(),
};
let _res = instantiate(deps, mock_env(), info, msg).unwrap();
Expand Down
Loading
Loading