Fix sub account admin assertion

framework/CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 - Updated fetch_data arguments of CwStakingCommand
+- Owner of the sub-accounts now Proxy, allowing modules to interact with sub-accounts
 
 ### Fixed
 - Partially fixed cw-staking for Osmosis

framework/contracts/account/manager/src/commands.rs

@@ -1,19 +1,20 @@
 use crate::{contract::ManagerResult, error::ManagerError, queries::query_module_cw2};
 use crate::{validation, versioning};
-use abstract_core::manager::state::ACCOUNT_FACTORY;
+use abstract_core::manager::state::{ACCOUNT_FACTORY, OWNER};
 
 use abstract_core::adapter::{
     AuthorizedAddressesResponse, BaseExecuteMsg, BaseQueryMsg, ExecuteMsg as AdapterExecMsg,
     QueryMsg as AdapterQuery,
 };
-use abstract_core::manager::{InternalConfigAction, QueryMsg};
+use abstract_core::manager::InternalConfigAction;
 use abstract_core::objects::gov_type::GovernanceDetails;
 use abstract_core::objects::AssetEntry;
 
 use abstract_core::version_control::ModuleResponse;
 use abstract_macros::abstract_response;
 use abstract_sdk::cw_helpers::AbstractAttributes;
 
+use abstract_sdk::namespaces::ADMIN_NAMESPACE;
 use abstract_sdk::{
     core::{
         manager::state::DEPENDENTS,
@@ -199,7 +200,6 @@ pub fn exec_on_module(
 /// Creates a sub-account for this account,
 pub fn create_subaccount(
     deps: DepsMut,
-    env: Env,
     msg_info: MessageInfo,
     name: String,
     description: Option<String>,
@@ -211,9 +211,9 @@ pub fn create_subaccount(
     assert_admin_right(deps.as_ref(), &msg_info.sender)?;
 
     let create_account_msg = &abstract_core::account_factory::ExecuteMsg::CreateAccount {
-        /// this contract (the manager) will be the account owner
+        /// proxy of this manager will be the account owner
         governance: GovernanceDetails::SubAccount {
-            manager: env.contract.address.to_string(),
+            proxy: ACCOUNT_MODULES.load(deps.storage, PROXY)?.into_string(),
         },
         name,
         description,
@@ -788,13 +788,20 @@ pub fn update_internal_config(deps: DepsMut, info: MessageInfo, config: Binary)
     }
 }
 
-fn query_ownership(deps: Deps, maybe_manager: Addr) -> ManagerResult<String> {
-    let Ownership::<String> { owner, .. } = deps
+fn query_ownership(deps: Deps, maybe_proxy: Addr) -> ManagerResult<Addr> {
+    // query admin of the proxy (it's manager)
+    let manager_bytes = deps
         .querier
-        .query_wasm_smart(maybe_manager.clone(), &QueryMsg::Ownership {})
-        .map_err(|_| ManagerError::NoContractOwner(maybe_manager.to_string()))?;
-
-    owner.ok_or(ManagerError::NoContractOwner(maybe_manager.to_string()))
+        .query_wasm_raw(maybe_proxy, ADMIN_NAMESPACE.as_bytes())?
+        .ok_or(ManagerError::SubAccountAdminVerification)?;
+    let manager = from_binary::<Option<Addr>>(&Binary(manager_bytes))?
+        .ok_or(ManagerError::SubAccountAdminVerification)?;
+
+    // get owner of the manager
+    let Ownership { owner, .. } = OWNER
+        .query(&deps.querier, manager.clone())
+        .map_err(|_| ManagerError::NoContractOwner(manager.to_string()))?;
+    owner.ok_or(ManagerError::NoContractOwner(manager.into_string()))
 }
 
 fn assert_admin_right(deps: Deps, sender: &Addr) -> ManagerResult<()> {
@@ -807,19 +814,21 @@ fn assert_admin_right(deps: Deps, sender: &Addr) -> ManagerResult<()> {
     let account_info = INFO.load(deps.storage)?;
     match account_info.governance_details {
         // If the account has SubAccount governance, the owner of the manager also has admin rights over this account
-        GovernanceDetails::SubAccount { manager } => {
+        GovernanceDetails::SubAccount { proxy } => {
             // We try to query the ownership of the manager monarch account if the first query failed
-            let mut current = manager;
+            let mut current = proxy;
             let mut i = 0;
             while i < MAX_ADMIN_RECURSION {
-                let owner = query_ownership(deps, current)
+                // admin of a proxy is a manager
+                let owner = query_ownership(deps, current.clone())
                     .map_err(|_| ManagerError::SubAccountAdminVerification)?;
+
                 if *sender == owner {
                     // If the owner of the current contract is the sender, the admin test is passed
                     return Ok(());
                 } else {
                     // If not, we try again with the queried owner
-                    current = deps.api.addr_validate(&owner)?
+                    current = owner;
                 }
                 i += 1;
             }

framework/contracts/account/manager/src/contract.rs

@@ -123,16 +123,7 @@ pub fn execute(deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg) -> M
                     link,
                     base_asset,
                     namespace,
-                } => create_subaccount(
-                    deps,
-                    env,
-                    info,
-                    name,
-                    description,
-                    link,
-                    base_asset,
-                    namespace,
-                ),
+                } => create_subaccount(deps, info, name, description, link, base_asset, namespace),
                 ExecuteMsg::Upgrade { modules } => upgrade_modules(deps, env, info, modules),
                 ExecuteMsg::UpdateInfo {
                     name,

framework/contracts/account/manager/tests/subaccount.rs

@@ -2,7 +2,7 @@ mod common;
 
 use abstract_interface::*;
 use common::*;
-use cosmwasm_std::Addr;
+use cosmwasm_std::{wasm_execute, Addr};
 use cw_orch::deploy::Deploy;
 use cw_orch::prelude::*;
 // use cw_multi_test::StakingInfo;
@@ -45,30 +45,28 @@ fn updating_on_subaccount_should_succeed() -> AResult {
 }
 
 #[test]
-fn manager_updating_on_subaccount_should_succeed() -> AResult {
+fn proxy_updating_on_subaccount_should_succeed() -> AResult {
     let sender = Addr::unchecked(common::OWNER);
     let chain = Mock::new(&sender);
     let deployment = Abstract::deploy_on(chain, sender.to_string())?;
     let account = create_default_account(&deployment.account_factory)?;
-    let manager_address = account.manager.address()?;
+    let proxy_address = account.proxy.address()?;
     account
         .manager
         .create_sub_account("My subaccount".to_string(), None, None, None, None)?;
 
     // 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 (sub_manager, _sub_proxy) = get_account_contracts(&deployment.version_control, Some(2));
     let new_desc = "new desc";
 
-    // We call as the manager, it should also be possible
-    account_contracts.0.call_as(&manager_address).update_info(
-        Some(new_desc.to_string()),
-        None,
-        None,
-    )?;
+    // We call as the proxy, it should also be possible
+    sub_manager
+        .call_as(&proxy_address)
+        .update_info(Some(new_desc.to_owned()), None, None)?;
 
     assert_eq!(
         Some(new_desc.to_string()),
-        account_contracts.0.info()?.info.description
+        sub_manager.info()?.info.description
     );
 
     Ok(())
@@ -109,3 +107,50 @@ fn recursive_updating_on_subaccount_should_succeed() -> AResult {
 
     Ok(())
 }
+
+#[test]
+fn installed_app_updating_on_subaccount_should_succeed() -> AResult {
+    let sender = Addr::unchecked(common::OWNER);
+    let chain = Mock::new(&sender);
+    let deployment = Abstract::deploy_on(chain, sender.to_string())?;
+    let account = create_default_account(&deployment.account_factory)?;
+    account
+        .manager
+        .create_sub_account("My subaccount".to_string(), None, None, None, None)?;
+    let first_proxy_addr = account.proxy.address()?;
+
+    let mock_app = Addr::unchecked("mock_app");
+    account
+        .proxy
+        .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 new_desc = "new desc";
+
+    // recover address on first proxy
+    account.proxy.set_address(&first_proxy_addr);
+    // adding mock_app to whitelist on proxy
+
+    // We call as installed app of the owner-proxy, it should also be possible
+    account
+        .proxy
+        .call_as(&mock_app)
+        .module_action(vec![wasm_execute(
+            sub_manager.addr_str()?,
+            &abstract_core::manager::ExecuteMsg::UpdateInfo {
+                name: None,
+                description: Some(new_desc.to_owned()),
+                link: None,
+            },
+            vec![],
+        )?
+        .into()])?;
+
+    assert_eq!(
+        Some(new_desc.to_string()),
+        sub_manager.info()?.info.description
+    );
+
+    Ok(())
+}

framework/packages/abstract-core/src/objects/gov_type.rs

@@ -21,8 +21,8 @@ pub enum GovernanceDetails<T: AddressLike> {
     },
     /// Used when the account is a sub-account of another account.
     SubAccount {
-        /// The manager of the account of which this account is the sub-account.
-        manager: T,
+        /// The proxy of the account of which this account is the sub-account.
+        proxy: T,
     },
     /// An external governance source
     External {
@@ -41,9 +41,9 @@ impl GovernanceDetails<String> {
                 let addr = api.addr_validate(&monarch)?;
                 Ok(GovernanceDetails::Monarchy { monarch: addr })
             }
-            GovernanceDetails::SubAccount { manager } => {
-                let addr = api.addr_validate(&manager)?;
-                Ok(GovernanceDetails::SubAccount { manager: addr })
+            GovernanceDetails::SubAccount { proxy } => {
+                let addr = api.addr_validate(&proxy)?;
+                Ok(GovernanceDetails::SubAccount { proxy: addr })
             }
             GovernanceDetails::External {
                 governance_address,
@@ -95,7 +95,7 @@ impl GovernanceDetails<Addr> {
     pub fn owner_address(&self) -> Addr {
         match self {
             GovernanceDetails::Monarchy { monarch } => monarch.clone(),
-            GovernanceDetails::SubAccount { manager } => manager.clone(),
+            GovernanceDetails::SubAccount { proxy } => proxy.clone(),
             GovernanceDetails::External {
                 governance_address, ..
             } => governance_address.clone(),
@@ -109,8 +109,8 @@ impl From<GovernanceDetails<Addr>> for GovernanceDetails<String> {
             GovernanceDetails::Monarchy { monarch } => GovernanceDetails::Monarchy {
                 monarch: monarch.to_string(),
             },
-            GovernanceDetails::SubAccount { manager } => GovernanceDetails::SubAccount {
-                manager: manager.to_string(),
+            GovernanceDetails::SubAccount { proxy } => GovernanceDetails::SubAccount {
+                proxy: proxy.to_string(),
             },
             GovernanceDetails::External {
                 governance_address,