Audit Fixes and Improvements

Cargo.toml

@@ -54,8 +54,8 @@ cw721 = "0.18"
 cw721-base = "0.18"
 env_logger = "0.10"
 once_cell = "1.18"
-osmosis-std = { git = "https://github.com/osmosis-labs/osmosis-rust", branch = "autobuild-v17.0.0-rc0" }
-osmosis-test-tube = "17.0.0-rc0"
+osmosis-std = "0.19.0"
+osmosis-test-tube = "19.0.0"
 proc-macro2 = "1.0"
 prost = "0.11"
 quote = "1.0"
@@ -71,9 +71,7 @@ sg-multi-test = "3.1.0"
 syn = { version = "1.0", features = ["derive"] }
 test-context = "0.1"
 thiserror = { version = "1.0" }
-# TODO use upstream when PR merged and new release tagged: https://github.com/CosmWasm/cw-multi-test/pull/51
-token-bindings = { git = "https://github.com/CosmosContracts/token-bindings.git", rev = "0cd084b68172ffc9af29eb37fb915392ce351954" }
-token-bindings-test = { git = "https://github.com/CosmosContracts/token-bindings.git", rev = "0cd084b68172ffc9af29eb37fb915392ce351954" }
+token-bindings = "0.11.0"
 wynd-utils = "0.4"
 
 # One commit ahead of version 0.3.0. Allows initialization with an
@@ -94,28 +92,27 @@ cw721-controllers = { path = "./packages/cw721-controllers", version = "2.2.0" }
 cw721-roles = { path = "./contracts/external/cw721-roles", version = "*" }
 dao-cw721-extensions = { path = "./packages/dao-cw721-extensions", version = "*" }
 dao-dao-core = { path = "./contracts/dao-dao-core", version = "2.2.0" }
-dao-interface = { path = "./packages/dao-interface", version = "2.2.0" }
 dao-dao-macros = { path = "./packages/dao-dao-macros", version = "2.2.0" }
+dao-hooks = { path = "./packages/dao-hooks", version = "*" }
+dao-interface = { path = "./packages/dao-interface", version = "2.2.0" }
 dao-pre-propose-approval-single = { path = "./contracts/pre-propose/dao-pre-propose-approval-single", version = "2.2.0" }
 dao-pre-propose-approver = { path = "./contracts/pre-propose/dao-pre-propose-approver", version = "2.2.0" }
 dao-pre-propose-base = { path = "./packages/dao-pre-propose-base", version = "2.2.0" }
 dao-pre-propose-multiple = { path = "./contracts/pre-propose/dao-pre-propose-multiple", version = "2.2.0" }
 dao-pre-propose-single = { path = "./contracts/pre-propose/dao-pre-propose-single", version = "2.2.0" }
 dao-proposal-condorcet = { path = "./contracts/proposal/dao-proposal-condorcet", version = "2.2.0" }
-dao-proposal-hooks = { path = "./packages/dao-proposal-hooks", version = "2.2.0" }
 dao-proposal-multiple = { path = "./contracts/proposal/dao-proposal-multiple", version = "2.2.0" }
 dao-proposal-single = { path = "./contracts/proposal/dao-proposal-single", version = "2.2.0" }
 dao-proposal-sudo = { path = "./test-contracts/dao-proposal-sudo", version = "2.2.0" }
+dao-proposal-hook-counter = { path = "./test-contracts/dao-proposal-hook-counter", version = "2.2.0" }
 dao-testing = { path = "./packages/dao-testing", version = "2.2.0" }
-dao-vote-hooks = { path = "./packages/dao-vote-hooks", version = "2.2.0" }
 dao-voting = { path = "./packages/dao-voting", version = "2.2.0" }
 dao-voting-cw20-balance = { path = "./test-contracts/dao-voting-cw20-balance", version = "2.2.0" }
 dao-voting-cw20-staked = { path = "./contracts/voting/dao-voting-cw20-staked", version = "2.2.0" }
 dao-voting-cw4 = { path = "./contracts/voting/dao-voting-cw4", version = "2.2.0" }
 dao-voting-cw721-roles = { path = "./contracts/voting/dao-voting-cw721-roles", version = "*" }
 dao-voting-cw721-staked = { path = "./contracts/voting/dao-voting-cw721-staked", version = "2.2.0" }
-dao-voting-native-staked = { path = "./contracts/voting/dao-voting-native-staked", version = "2.2.0" }
-dao-voting-token-factory-staked = { path = "./contracts/voting/dao-voting-token-factory-staked", version = "2.2.0" }
+dao-voting-token-staked = { path = "./contracts/voting/dao-voting-token-staked", version = "2.2.0" }
 
 # v1 dependencies. used for state migrations.
 cw-core-v1 = { package = "cw-core", version = "0.1.0" }
@@ -129,6 +126,6 @@ cw4-voting-v1 = { package = "cw4-voting", version = "0.1.0" }
 voting-v1 = { package = "dao-voting", version = "0.1.0" }
 stake-cw20-v03 = { package = "stake-cw20", version = "0.2.6" }
 
-# TODO remove when upstream PR merged and new release tagged: https://github.com/CosmWasm/cw-multi-test/pull/51
+# TODO remove when new release is tagged upstream
 [patch.crates-io]
-cw-multi-test = { git = "https://github.com/JakeHartnell/cw-multi-test.git", branch = "bank-supply-support" }
+cw-multi-test = { git = "https://github.com/CosmWasm/cw-multi-test.git", rev = "d38db7752b9f054c395d6108453f8b321e4cab02" }

ci/integration-tests/src/helpers/helper.rs

@@ -118,15 +118,16 @@ pub fn create_dao(
         .unwrap();
 
     let ProposalCreationPolicy::Module { addr: pre_propose } = chain
-	.orc
-	.query(
-	    "dao_proposal_single",
-	    &dao_proposal_single::msg::QueryMsg::ProposalCreationPolicy {}
-	).unwrap()
-	.data()
-	.unwrap()
+        .orc
+        .query(
+            "dao_proposal_single",
+            &dao_proposal_single::msg::QueryMsg::ProposalCreationPolicy {},
+        )
+        .unwrap()
+        .data()
+        .unwrap()
     else {
-	panic!("expected pre-propose module")
+        panic!("expected pre-propose module")
     };
     chain
         .orc

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/dao-dao-core/schema/dao-dao-core.json

@@ -1029,7 +1029,7 @@
                     ]
                   },
                   "channel_id": {
-                    "description": "exisiting channel to send the tokens over",
+                    "description": "existing channel to send the tokens over",
                     "type": "string"
                   },
                   "timeout": {
@@ -1147,7 +1147,7 @@
             "minimum": 0.0
           },
           "revision": {
-            "description": "the version that the client is currently on (eg. after reseting the chain this could increment 1 as height drops to 0)",
+            "description": "the version that the client is currently on (e.g. after resetting the chain this could increment 1 as height drops to 0)",
             "type": "integer",
             "format": "uint64",
             "minimum": 0.0
@@ -1398,7 +1398,7 @@
                     }
                   },
                   "label": {
-                    "description": "A human-readbale label for the contract",
+                    "description": "A human-readable label for the contract.\n\nValid values should: - not be empty - not be bigger than 128 bytes (or some chain-specific limit) - not start / end with whitespace",
                     "type": "string"
                   },
                   "msg": {

contracts/external/cw-tokenfactory-issuer/Cargo.toml

@@ -37,8 +37,9 @@ test-tube = []
 cosmwasm-schema = { workspace = true }
 cosmwasm-std = { workspace = true }
 cosmwasm-storage = { workspace = true }
-cw-storage-plus = { workspace = true }
 cw2 = { workspace = true }
+cw-ownable = { workspace = true }
+cw-storage-plus = { workspace = true }
 osmosis-std = { workspace = true }
 prost = { workspace = true }
 schemars = { workspace = true }

contracts/external/cw-tokenfactory-issuer/README.md

@@ -6,27 +6,28 @@ This repo contains a set of contracts that when used in conjunction with the x/t
 - Creating a new Token Factory token or using an existing one
 - Granting and revoking allowances for the minting and burning of tokens
 - Updating token metadata
-- Freezing and unfreezing transfers, with a whitelist to allow some address to continue to transfer
-- Force transfering tokens via the contract owner
 - Updating the contract owner or Token Factory admin
+- And more! (see [Advanced Features](#advanced-features))
 
 It is intended to work on multiple chains supporting Token Factory, and has been tested on Juno Network and Osmosis.
 
-The contract has an owner (which can be removed or updated via `ExecuteMsg::ChangeContractOwner {}`), but it can delegate capabilities to other acccounts. For example, the owner of a contract can delegate minting allowance of 1000 tokens to a new address.
+The contract has an owner (which can be removed or updated via `ExecuteMsg::UpdateOwnership {}`), but it can delegate capabilities to other acccounts. For example, the owner of a contract can delegate minting allowance of 1000 tokens to a new address. 
 
-The contract is also the admin of the newly created Token Factory denom. For minting and burning, users then interact with the contract using its own ExecuteMsgs which trigger the contract's access control logic, and the contract then dispatches tokenfactory sdk.Msgs from its own contract account.
+Ownership functionality for this contract is implemented using the `cw-ownable` library.
 
-NOTE: this contract contains a `SudoMsg::BlockBeforeSend` hook that allows for the blacklisting of specific accounts as well as the freezing of all transfers if necessary. This feature is not enabled on every chain using Token Factory, and so blacklisting and freezing features are disabled if `MsgBeforeSendHook` is not supported. DAOs wishing to leverage these features on chains after support is added can call `ExecuteMsg::SetBeforeSendHook {}`.
+The `cw_tokenfactory_issuer` contract is also the admin of newly created Token Factory denoms. For minting and burning, users then interact with the contract using its own ExecuteMsgs which trigger the contract's access control logic, and the contract then dispatches tokenfactory sdk.Msgs from its own contract account.
 
 ## Instantiation
-When instantiating `cw-tokenfactory-issuer`, you can either create a `new_token` or an `existing_token`.
+
+When instantiating `cw-tokenfactory-issuer`, you can either create a `new` or an `existing`.
 
 ### Creating a new Token Factory token
+
 To create a new Token Factory token, simply instantiate the contract with a `subdenom`, this will create a new contract as well as a token with a denom formatted as `factory/{contract_address}/{subdenom}`.
 
 Example instantiate message:
 
-``` json
+```json
 {
   "new_token": {
     "subdenom": "test"
@@ -37,13 +38,67 @@ Example instantiate message:
 All other updates can be preformed afterwards via this contract's `ExecuteMsg` enum. See `src/msg.rs` for available methods.
 
 ### Using an Existing Token
+
 You can also instantiate this contract with an existing token, however most features will not be available until the previous Token Factory admin transfers admin rights to the instantiated contract and optionally calls `ExecuteMsg::SetBeforeSendHook {}` to enable dependent features.
 
 Example instantiate message:
-``` json
+
+```json
 {
   "existing_token": {
     "denom": "factory/{contract_address}/{subdenom}"
   }
 }
 ```
+
+## Renouncing Token Factory Admin
+Some DAOs or protocols after the initial setup phase may wish to render their tokens immutable, permanently disabling features of this contract.
+
+To do so, they must execute a `ExcuteMessage::UpdateTokenFactoryAdmin {}` method, setting the Admin to a null address or the bank module for your respective chain.
+
+For example, on Juno this could be:
+
+``` json
+{
+  "update_token_factory_admin": {
+    "new_admin": "juno1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+  }
+}
+```
+
+The Token Factory standard requires a Token Factory admin per token, by setting to a null address the Token is rendered immutable and the `cw-tokenfactory-issuer` will be unable to make future updates. This is secure as the cryptography that underlies the chain enforces that even with the largest super computers in the world it would take an astonomically large amount of time to compute the private key for this address.
+
+### Advanced Features
+
+This contract supports a number of advanced features which DAOs or token issuers may wist to leverage:
+- Freezing and unfreezing transfers, with an allowlist to allow specified addresses to allow transfer to or from
+- Denylist to prevent certain addresses from transferring
+- Force transfering tokens via the contract owner
+
+**By default, these features are disabled**, and must be explictly enabled by the contract owner (for example via a DAO governance prop).
+
+Moreover, for these features to work, your chain must support the `MsgBeforeSendHook` bank module hook. This is not yet available on every chain using Token Factory, and so denylisting and freezing features are not available if `MsgBeforeSendHook` is not supported.
+
+On chains where `MsgBeforeSendHook` is supported, DAOs or issuers wishing to leverage these features must set the before send hook with `ExecuteMsg::SetBeforeSendHook {}`.
+
+This method takes a `cosmwasm_address`, which is the address of a contract implement a `SudoMsg::BlockBeforeSend` entrypoint. Normally this will be the address of the `cw_tokenfactory_issuer` contract itself, but it is possible to specify a custom contract. This contract contains a `SudoMsg::BlockBeforeSend` hook that allows for the denylisting of specific accounts as well as the freezing of all transfers if necessary. 
+
+Example message to set before send hook:
+``` json
+{
+  "set_before_send_hook": {
+    "cosmwasm_address": "<address of your cw_tokenfactory_issuer contract>"
+  }
+}
+```
+
+DAOs or issuers wishing to leverage these features on chains without support can call `ExecuteMsg::SetBeforeSendHook {}` when support is added.
+
+If a DAO or issuer wishes to disable and removed before send hook related functionality, they simply need to call `ExecuteMsg::SetBeforeSendHook {}` with an empty string for the `cosmwasm_address` like so:
+``` json
+{
+  "set_before_send_hook": {
+    "cosmwasm_address": ""
+  }
+}
+```