Remove owner from dao-voting-cw721-staked

The concept of ownership doesn't really make sense for voting contracts.
IMO the owner should always be the DAO.

ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs

@@ -1,7 +1,6 @@
 use cosm_orc::orchestrator::{ExecReq, SigningKey};
 use cosmwasm_std::{Binary, Empty, Uint128};
 use cw_utils::Duration;
-use dao_interface::state::Admin;
 use test_context::test_context;
 
 use dao_voting_cw721_staked as module;
@@ -38,7 +37,6 @@ pub fn instantiate_cw721_base(chain: &mut Chain, key: &SigningKey, minter: &str)
 
 fn setup_test(
     chain: &mut Chain,
-    owner: Option<Admin>,
     unstaking_duration: Option<Duration>,
     key: &SigningKey,
     minter: &str,
@@ -50,7 +48,6 @@ fn setup_test(
             CONTRACT_NAME,
             "instantiate_dao_voting_cw721_staked",
             &module::msg::InstantiateMsg {
-                owner,
                 nft_contract: module::msg::NftContract::Existing {
                     address: cw721.clone(),
                 },
@@ -169,7 +166,7 @@ fn cw721_stake_tokens(chain: &mut Chain) {
     let user_addr = chain.users["user1"].account.address.clone();
     let user_key = chain.users["user1"].key.clone();
 
-    let CommonTest { module, .. } = setup_test(chain, None, None, &user_key, &user_addr);
+    let CommonTest { module, .. } = setup_test(chain, None, &user_key, &user_addr);
 
     mint_and_stake_nft(chain, &user_key, &user_addr, &module, "a");
 
@@ -202,13 +199,8 @@ fn cw721_stake_max_claims_works(chain: &mut Chain) {
     let user_addr = chain.users["user1"].account.address.clone();
     let user_key = chain.users["user1"].key.clone();
 
-    let CommonTest { module, .. } = setup_test(
-        chain,
-        None,
-        Some(Duration::Height(1)),
-        &user_key,
-        &user_addr,
-    );
+    let CommonTest { module, .. } =
+        setup_test(chain, Some(Duration::Height(1)), &user_key, &user_addr);
 
     // Create `MAX_CLAIMS` claims.
 

contracts/external/cw721-roles/src/tests.rs

@@ -260,7 +260,6 @@ fn test_send_permissions() {
             dao_voting_cw721_staked_id,
             Addr::unchecked(DAO),
             &Cw721StakedInstantiateMsg {
-                owner: None,
                 nft_contract: NftContract::Existing {
                     address: cw721_addr.to_string(),
                 },

contracts/proposal/dao-proposal-multiple/src/testing/instantiate.rs

@@ -148,7 +148,6 @@ pub fn _instantiate_with_staked_cw721_governance(
         voting_module_instantiate_info: ModuleInstantiateInfo {
             code_id: cw721_stake_id,
             msg: to_binary(&dao_voting_cw721_staked::msg::InstantiateMsg {
-                owner: Some(Admin::CoreModule {}),
                 unstaking_duration: None,
                 nft_contract: dao_voting_cw721_staked::msg::NftContract::Existing {
                     address: nft_address.to_string(),

contracts/proposal/dao-proposal-single/src/testing/instantiate.rs

@@ -146,7 +146,6 @@ pub(crate) fn instantiate_with_staked_cw721_governance(
         voting_module_instantiate_info: ModuleInstantiateInfo {
             code_id: cw721_stake_id,
             msg: to_binary(&dao_voting_cw721_staked::msg::InstantiateMsg {
-                owner: Some(Admin::CoreModule {}),
                 unstaking_duration: None,
                 nft_contract: dao_voting_cw721_staked::msg::NftContract::Existing {
                     address: nft_address.to_string(),

contracts/voting/dao-voting-cw721-staked/schema/dao-voting-cw721-staked.json

@@ -29,17 +29,6 @@
           }
         ]
       },
-      "owner": {
-        "description": "TODO use cw_ownable May change unstaking duration and add hooks.",
-        "anyOf": [
-          {
-            "$ref": "#/definitions/Admin"
-          },
-          {
-            "type": "null"
-          }
-        ]
-      },
       "unstaking_duration": {
         "description": "Amount of time between unstaking and tokens being avaliable. To unstake with no delay, leave as `None`.",
         "anyOf": [
@@ -103,47 +92,6 @@
           }
         ]
       },
-      "Admin": {
-        "description": "Information about the CosmWasm level admin of a contract. Used in conjunction with `ModuleInstantiateInfo` to instantiate modules.",
-        "oneOf": [
-          {
-            "description": "Set the admin to a specified address.",
-            "type": "object",
-            "required": [
-              "address"
-            ],
-            "properties": {
-              "address": {
-                "type": "object",
-                "required": [
-                  "addr"
-                ],
-                "properties": {
-                  "addr": {
-                    "type": "string"
-                  }
-                },
-                "additionalProperties": false
-              }
-            },
-            "additionalProperties": false
-          },
-          {
-            "description": "Sets the admin as the core module address.",
-            "type": "object",
-            "required": [
-              "core_module"
-            ],
-            "properties": {
-              "core_module": {
-                "type": "object",
-                "additionalProperties": false
-              }
-            },
-            "additionalProperties": false
-          }
-        ]
-      },
       "Binary": {
         "description": "Binary is a wrapper around Vec<u8> to add base64 de/serialization with serde. It also adds some helper methods to help encode inline.\n\nThis is only needed as serde-json-{core,wasm} has a horrible encoding for Vec<u8>. See also <https://github.com/CosmWasm/cosmwasm/blob/main/docs/MESSAGE_TYPES.md>.",
         "type": "string"
@@ -332,12 +280,6 @@
                     "type": "null"
                   }
                 ]
-              },
-              "owner": {
-                "type": [
-                  "string",
-                  "null"
-                ]
               }
             },
             "additionalProperties": false
@@ -820,16 +762,6 @@
         "nft_address": {
           "$ref": "#/definitions/Addr"
         },
-        "owner": {
-          "anyOf": [
-            {
-              "$ref": "#/definitions/Addr"
-            },
-            {
-              "type": "null"
-            }
-          ]
-        },
         "unstaking_duration": {
           "anyOf": [
             {

contracts/voting/dao-voting-cw721-staked/src/contract.rs

@@ -10,7 +10,6 @@ use cw721::{Cw721QueryMsg, Cw721ReceiveMsg, NumTokensResponse};
 use cw_storage_plus::Bound;
 use cw_utils::{parse_reply_instantiate_data, Duration};
 use dao_hooks::nft_stake::{stake_nft_hook_msgs, unstake_nft_hook_msgs};
-use dao_interface::state::Admin;
 use dao_interface::voting::IsActiveResponse;
 use dao_voting::threshold::{ActiveThreshold, ActiveThresholdResponse};
 
@@ -87,15 +86,6 @@ pub fn instantiate(
 
     DAO.save(deps.storage, &info.sender)?;
 
-    let owner = msg
-        .owner
-        .as_ref()
-        .map(|owner| match owner {
-            Admin::Address { addr } => deps.api.addr_validate(addr),
-            Admin::CoreModule {} => Ok(info.sender.clone()),
-        })
-        .transpose()?;
-
     if let Some(active_threshold) = msg.active_threshold.as_ref() {
         match active_threshold {
             ActiveThreshold::Percentage { percent } => {
@@ -128,20 +118,13 @@ pub fn instantiate(
     match msg.nft_contract {
         NftContract::Existing { address } => {
             let config = Config {
-                owner: owner.clone(),
                 nft_address: deps.api.addr_validate(&address)?,
                 unstaking_duration: msg.unstaking_duration,
             };
             CONFIG.save(deps.storage, &config)?;
 
             Ok(Response::default()
                 .add_attribute("method", "instantiate")
-                .add_attribute(
-                    "owner",
-                    owner
-                        .map(|a| a.into_string())
-                        .unwrap_or_else(|| "None".to_string()),
-                )
                 .add_attribute("nft_contract", address))
         }
         NftContract::New {
@@ -169,7 +152,6 @@ pub fn instantiate(
 
             // Save config with empty nft_address
             let config = Config {
-                owner: owner.clone(),
                 nft_address: Addr::unchecked(""),
                 unstaking_duration: msg.unstaking_duration,
             };
@@ -192,12 +174,6 @@ pub fn instantiate(
 
             Ok(Response::default()
                 .add_attribute("method", "instantiate")
-                .add_attribute(
-                    "owner",
-                    owner
-                        .map(|a| a.into_string())
-                        .unwrap_or_else(|| "None".to_string()),
-                )
                 .add_submessage(instantiate_msg))
         }
     }
@@ -214,9 +190,7 @@ pub fn execute(
         ExecuteMsg::ReceiveNft(msg) => execute_stake(deps, env, info, msg),
         ExecuteMsg::Unstake { token_ids } => execute_unstake(deps, env, info, token_ids),
         ExecuteMsg::ClaimNfts {} => execute_claim_nfts(deps, env, info),
-        ExecuteMsg::UpdateConfig { owner, duration } => {
-            execute_update_config(info, deps, owner, duration)
-        }
+        ExecuteMsg::UpdateConfig { duration } => execute_update_config(info, deps, duration),
         ExecuteMsg::AddHook { addr } => execute_add_hook(deps, info, addr),
         ExecuteMsg::RemoveHook { addr } => execute_remove_hook(deps, info, addr),
         ExecuteMsg::UpdateActiveThreshold { new_threshold } => {
@@ -386,32 +360,21 @@ pub fn execute_claim_nfts(
 pub fn execute_update_config(
     info: MessageInfo,
     deps: DepsMut,
-    new_owner: Option<String>,
     duration: Option<Duration>,
 ) -> Result<Response, ContractError> {
     let mut config: Config = CONFIG.load(deps.storage)?;
+    let dao = DAO.load(deps.storage)?;
 
-    if config.owner.map_or(true, |owner| owner != info.sender) {
-        return Err(ContractError::NotOwner {});
+    // Only the DAO can update the config.
+    if info.sender != dao {
+        return Err(ContractError::Unauthorized {});
     }
 
-    let new_owner = new_owner
-        .map(|new_owner| deps.api.addr_validate(&new_owner))
-        .transpose()?;
-
-    config.owner = new_owner;
     config.unstaking_duration = duration;
     CONFIG.save(deps.storage, &config)?;
 
     Ok(Response::default()
         .add_attribute("action", "update_config")
-        .add_attribute(
-            "owner",
-            config
-                .owner
-                .map(|a| a.to_string())
-                .unwrap_or_else(|| "none".to_string()),
-        )
         .add_attribute(
             "unstaking_duration",
             config
@@ -426,9 +389,11 @@ pub fn execute_add_hook(
     info: MessageInfo,
     addr: String,
 ) -> Result<Response, ContractError> {
-    let config: Config = CONFIG.load(deps.storage)?;
-    if config.owner.map_or(true, |owner| owner != info.sender) {
-        return Err(ContractError::NotOwner {});
+    let dao = DAO.load(deps.storage)?;
+
+    // Only the DAO can add a hook
+    if info.sender != dao {
+        return Err(ContractError::Unauthorized {});
     }
 
     let hook = deps.api.addr_validate(&addr)?;
@@ -444,9 +409,11 @@ pub fn execute_remove_hook(
     info: MessageInfo,
     addr: String,
 ) -> Result<Response, ContractError> {
-    let config: Config = CONFIG.load(deps.storage)?;
-    if config.owner.map_or(true, |owner| owner != info.sender) {
-        return Err(ContractError::NotOwner {});
+    let dao = DAO.load(deps.storage)?;
+
+    // Only the DAO can remove a hook
+    if info.sender != dao {
+        return Err(ContractError::Unauthorized {});
     }
 
     let hook = deps.api.addr_validate(&addr)?;

contracts/voting/dao-voting-cw721-staked/src/msg.rs

@@ -3,7 +3,6 @@ use cosmwasm_std::Binary;
 use cw721::Cw721ReceiveMsg;
 use cw_utils::Duration;
 use dao_dao_macros::{active_query, voting_module_query};
-use dao_interface::state::Admin;
 use dao_voting::threshold::{ActiveThreshold, ActiveThresholdResponse};
 
 #[cw_serde]
@@ -28,9 +27,6 @@ pub enum NftContract {
 
 #[cw_serde]
 pub struct InstantiateMsg {
-    /// TODO use cw_ownable
-    /// May change unstaking duration and add hooks.
-    pub owner: Option<Admin>,
     /// Address of the cw721 NFT contract that may be staked.
     pub nft_contract: NftContract,
     /// Amount of time between unstaking and tokens being
@@ -55,7 +51,6 @@ pub enum ExecuteMsg {
     },
     ClaimNfts {},
     UpdateConfig {
-        owner: Option<String>,
         duration: Option<Duration>,
     },
     AddHook {

contracts/voting/dao-voting-cw721-staked/src/state.rs

@@ -10,7 +10,6 @@ use crate::ContractError;
 
 #[cw_serde]
 pub struct Config {
-    pub owner: Option<Addr>,
     pub nft_address: Addr,
     pub unstaking_duration: Option<Duration>,
 }

contracts/voting/dao-voting-cw721-staked/src/testing/adversarial.rs

@@ -27,7 +27,7 @@ fn test_circular_stake() -> anyhow::Result<()> {
         mut app,
         module,
         nft,
-    } = setup_test(None, None);
+    } = setup_test(None);
 
     mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, "1")?;
     mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, "2")?;
@@ -72,7 +72,7 @@ fn test_immediate_unstake() -> anyhow::Result<()> {
         mut app,
         module,
         nft,
-    } = setup_test(None, None);
+    } = setup_test(None);
 
     mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, "1")?;
     mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, "2")?;
@@ -94,7 +94,7 @@ fn test_immediate_unstake() -> anyhow::Result<()> {
 fn test_stake_wrong_nft() -> anyhow::Result<()> {
     let CommonTest {
         mut app, module, ..
-    } = setup_test(None, None);
+    } = setup_test(None);
     let other_nft = instantiate_cw721_base(&mut app, CREATOR_ADDR, CREATOR_ADDR);
 
     let res = mint_and_stake_nft(&mut app, &other_nft, &module, CREATOR_ADDR, "1");
@@ -115,7 +115,7 @@ fn test_query_the_future() -> anyhow::Result<()> {
         mut app,
         module,
         nft,
-    } = setup_test(None, None);
+    } = setup_test(None);
 
     mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, "1")?;
 
@@ -154,7 +154,7 @@ fn test_bypass_max_claims() -> anyhow::Result<()> {
         mut app,
         module,
         nft,
-    } = setup_test(None, Some(Duration::Height(1)));
+    } = setup_test(Some(Duration::Height(1)));
     let mut to_stake = vec![];
     for i in 1..(MAX_CLAIMS + 10) {
         let i_str = &i.to_string();