review fixes

Makefile

@@ -14,7 +14,7 @@ fmt:
 
 check_contracts:
 	@cargo install cosmwasm-check --version 2.0.4 --locked
-	@cosmwasm-check --available-capabilities iterator,staking,stargate,neutron artifacts/*.wasm
+	@cosmwasm-check --available-capabilities iterator,staking,stargate,neutron,cosmwasm_1_1,cosmwasm_1_2,cosmwasm_1_3,cosmwasm_1_4,cosmwasm_2_0 artifacts/*.wasm
 
 compile:
 	@docker run --rm -v "$(CURDIR)":/code \

src/contract.rs

@@ -9,10 +9,7 @@ use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg, SudoMsg};
 use crate::rbac::can_invoke_message;
 use crate::state::rbac::Roles;
 use crate::state::storage::RBAC_PERMISSIONS;
-use crate::state::{
-    flow::FlowType,
-    storage::{GOVMODULE, IBCMODULE},
-};
+use crate::state::{flow::FlowType, storage::GOVMODULE};
 use crate::{execute, message_queue, query, rbac, sudo};
 
 // version info for migration info
@@ -27,7 +24,6 @@ pub fn instantiate(
     msg: InstantiateMsg,
 ) -> Result<Response, ContractError> {
     set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
-    IBCMODULE.save(deps.storage, &msg.ibc_module)?;
     GOVMODULE.save(deps.storage, &msg.gov_module)?;
     // grant the gov address full permissions
     RBAC_PERMISSIONS.save(
@@ -40,7 +36,6 @@ pub fn instantiate(
 
     Ok(Response::new()
         .add_attribute("method", "instantiate")
-        .add_attribute("ibc_module", msg.ibc_module.to_string())
         .add_attribute("gov_module", msg.gov_module.to_string()))
 }
 
@@ -118,18 +113,6 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> {
 pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> {
     let contract_version = get_contract_version(deps.storage)?;
 
-    // check contract version, and apply version specific migrations
-    if contract_version.version.eq("0.1.0") {
-        let gov_module = GOVMODULE.load(deps.storage)?;
-
-        // grant the gov address full permissions
-        RBAC_PERMISSIONS.save(
-            deps.storage,
-            gov_module.to_string(),
-            &Roles::all_roles().into_iter().collect(),
-        )?;
-    }
-
     // update contract version
     set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
 

src/execute.rs

@@ -147,27 +147,20 @@ mod tests {
     use cosmwasm_std::{from_json, StdError};
 
     use crate::contract::{execute, query};
-    use crate::helpers::tests::verify_query_response;
     use crate::msg::{ExecuteMsg, QueryMsg, QuotaMsg};
     use crate::state::rbac::Roles;
     use crate::state::{
         rate_limit::RateLimit,
-        storage::{ACCEPTED_CHANNELS_FOR_RESTRICTED_DENOM, GOVMODULE, IBCMODULE},
+        storage::{ACCEPTED_CHANNELS_FOR_RESTRICTED_DENOM, GOVMODULE},
     };
+    use crate::tests::helpers::tests::verify_query_response;
     use crate::ContractError;
 
-    const IBC_ADDR: &str = "neutron1cdlz8scnf3mmxdnf4njmtp7vz4gps7fswphrqn";
     const GOV_ADDR: &str = "neutron1w02khza7ux68ccwmz2hln97mkjspjxes8y2k9v";
 
     #[test] // Tests AddPath and RemovePath messages
     fn management_add_and_remove_path() {
         let mut deps = mock_dependencies();
-        IBCMODULE
-            .save(
-                deps.as_mut().storage,
-                &MockApi::default().addr_make(IBC_ADDR),
-            )
-            .unwrap();
         GOVMODULE
             .save(
                 deps.as_mut().storage,
@@ -178,7 +171,7 @@ mod tests {
         // grant role to IBC_ADDR
         crate::rbac::grant_role(
             &mut deps.as_mut(),
-            MockApi::default().addr_make(IBC_ADDR).to_string(),
+            MockApi::default().addr_make(GOV_ADDR).to_string(),
             vec![Roles::AddRateLimit, Roles::RemoveRateLimit],
         )
         .unwrap();
@@ -192,7 +185,7 @@ mod tests {
                 send_recv: (3, 5),
             }],
         };
-        let info = message_info(&MockApi::default().addr_make(IBC_ADDR), &[]);
+        let info = message_info(&MockApi::default().addr_make(GOV_ADDR), &[]);
 
         let env = mock_env();
         let res = execute(deps.as_mut(), env.clone(), info, msg).unwrap();
@@ -228,7 +221,7 @@ mod tests {
                 send_recv: (3, 5),
             }],
         };
-        let info = message_info(&MockApi::default().addr_make(IBC_ADDR), &[]);
+        let info = message_info(&MockApi::default().addr_make(GOV_ADDR), &[]);
 
         let env = mock_env();
         execute(deps.as_mut(), env.clone(), info, msg).unwrap();
@@ -239,7 +232,7 @@ mod tests {
             denom: "denom".to_string(),
         };
 
-        let info = message_info(&MockApi::default().addr_make(IBC_ADDR), &[]);
+        let info = message_info(&MockApi::default().addr_make(GOV_ADDR), &[]);
         let env = mock_env();
         execute(deps.as_mut(), env.clone(), info, msg).unwrap();
 
@@ -275,7 +268,7 @@ mod tests {
                 send_recv: (50, 30),
             }],
         };
-        let info = message_info(&MockApi::default().addr_make(IBC_ADDR), &[]);
+        let info = message_info(&MockApi::default().addr_make(GOV_ADDR), &[]);
 
         let env = mock_env();
         execute(deps.as_mut(), env.clone(), info, msg).unwrap();

src/lib.rs

@@ -16,9 +16,7 @@ mod execute;
 mod query;
 mod sudo;
 
-// Tests
-mod contract_tests;
-mod helpers;
-mod integration_tests;
+#[cfg(test)]
+pub mod tests;
 
 pub use crate::error::ContractError;

src/message_queue.rs

@@ -1,4 +1,4 @@
-use cosmwasm_std::{DepsMut, Env, MessageInfo, Response, Storage};
+use cosmwasm_std::{DepsMut, Env, MessageInfo, Response, Storage, TransactionInfo};
 
 use crate::{
     error::ContractError,
@@ -72,7 +72,13 @@ pub fn queue_message(
     info: MessageInfo,
 ) -> Result<String, ContractError> {
     let timelock_delay = TIMELOCK_DELAY.load(deps.storage, info.sender.to_string())?;
-    let message_id = format!("{}_{}", env.block.height, env.transaction.unwrap().index);
+    let message_id = format!(
+        "{}_{}",
+        env.block.height,
+        env.transaction
+            .unwrap_or(TransactionInfo { index: 0 })
+            .index
+    );
     MESSAGE_QUEUE.push_back(
         deps.storage,
         &QueuedMessage {

src/msg.rs

@@ -53,7 +53,6 @@ impl QuotaMsg {
 #[cw_serde]
 pub struct InstantiateMsg {
     pub gov_module: Addr,
-    pub ibc_module: Addr,
     pub paths: Vec<PathMsg>,
 }
 

src/state/storage.rs

@@ -13,9 +13,6 @@ use super::{
 /// Only this address can manage the contract. This will likely be the
 /// governance module, but could be set to something else if needed
 pub const GOVMODULE: Item<Addr> = Item::new("gov_module");
-/// Only this address can execute transfers. This will likely be the
-/// IBC transfer module, but could be set to something else if needed
-pub const IBCMODULE: Item<Addr> = Item::new("ibc_module");
 
 /// RATE_LIMIT_TRACKERS is the main state for this contract. It maps a path (IBC
 /// Channel + denom) to a vector of `RateLimit`s.

src/contract_tests.rs → src/tests/contract_tests.rs

@@ -4,13 +4,13 @@ use crate::packet::Packet;
 use crate::state::rbac::Roles;
 use crate::{contract::*, test_msg_recv, test_msg_send, ContractError};
 use cosmwasm_std::testing::{message_info, mock_dependencies, mock_env, MockApi};
-use cosmwasm_std::{from_json, Attribute, MessageInfo, Uint256};
+use cosmwasm_std::{from_json, Attribute, Uint256};
 
-use crate::helpers::tests::verify_query_response;
-use crate::msg::{InstantiateMsg, MigrateMsg, PathMsg, QueryMsg, QuotaMsg, SudoMsg};
+use crate::msg::{InstantiateMsg, PathMsg, QueryMsg, QuotaMsg, SudoMsg};
 use crate::state::flow::tests::RESET_TIME_WEEKLY;
 use crate::state::rate_limit::RateLimit;
-use crate::state::storage::{GOVMODULE, IBCMODULE, RATE_LIMIT_TRACKERS, RBAC_PERMISSIONS};
+use crate::state::storage::{GOVMODULE, RATE_LIMIT_TRACKERS, RBAC_PERMISSIONS};
+use crate::tests::helpers::tests::verify_query_response;
 const IBC_ADDR: &str = "neutron1cdlz8scnf3mmxdnf4njmtp7vz4gps7fswphrqn";
 const GOV_ADDR: &str = "neutron1w02khza7ux68ccwmz2hln97mkjspjxes8y2k9v";
 
@@ -20,7 +20,6 @@ fn proper_instantiation() {
 
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: MockApi::default().addr_make(IBC_ADDR),
         paths: vec![],
     };
     let info = message_info(&MockApi::default().addr_make(IBC_ADDR), &[]);
@@ -30,10 +29,6 @@ fn proper_instantiation() {
     assert_eq!(0, res.messages.len());
 
     // The ibc and gov modules are properly stored
-    assert_eq!(
-        IBCMODULE.load(deps.as_ref().storage).unwrap().to_string(),
-        MockApi::default().addr_make(IBC_ADDR).to_string()
-    );
     assert_eq!(
         GOVMODULE.load(deps.as_ref().storage).unwrap().to_string(),
         MockApi::default().addr_make(GOV_ADDR).to_string()
@@ -57,7 +52,6 @@ fn consume_allowance() {
     let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10);
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: MockApi::default().addr_make(IBC_ADDR),
         paths: vec![PathMsg {
             channel_id: "any".to_string(),
             denom: "denom".to_string(),
@@ -96,7 +90,6 @@ fn symetric_flows_dont_consume_allowance() {
     let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10);
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: MockApi::default().addr_make(IBC_ADDR),
         paths: vec![PathMsg {
             channel_id: "any".to_string(),
             denom: "denom".to_string(),
@@ -158,7 +151,6 @@ fn asymetric_quotas() {
     let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 4, 1);
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: MockApi::default().addr_make(IBC_ADDR),
         paths: vec![PathMsg {
             channel_id: "any".to_string(),
             denom: "denom".to_string(),
@@ -241,7 +233,6 @@ fn query_state() {
     let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10);
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: MockApi::default().addr_make(IBC_ADDR),
         paths: vec![PathMsg {
             channel_id: "any".to_string(),
             denom: "denom".to_string(),
@@ -306,7 +297,6 @@ fn bad_quotas() {
 
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: MockApi::default().addr_make(IBC_ADDR),
         paths: vec![PathMsg {
             channel_id: "any".to_string(),
             denom: "denom".to_string(),
@@ -347,7 +337,6 @@ fn undo_send() {
     let quota = QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 10, 10);
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: MockApi::default().addr_make(IBC_ADDR),
         paths: vec![PathMsg {
             channel_id: "any".to_string(),
             denom: "denom".to_string(),
@@ -414,60 +403,3 @@ fn test_tokenfactory_message() {
     let _parsed: SudoMsg = serde_json_wasm::from_str(json).unwrap();
     //println!("{parsed:?}");
 }
-
-#[test] // Tests we ccan instantiate the contract and that the owners are set correctly
-fn proper_migrate() {
-    let mut deps = mock_dependencies();
-    let env = mock_env();
-
-    crate::contract::instantiate(
-        deps.as_mut(),
-        env,
-        MessageInfo {
-            sender: MockApi::default().addr_make("osmo16tumts0kckpfp9fk7e3rnx9ahzn70dyyqfypgh"),
-            funds: vec![],
-        },
-        InstantiateMsg {
-            gov_module: MockApi::default().addr_make(GOV_ADDR),
-            ibc_module: MockApi::default().addr_make(IBC_ADDR),
-            paths: vec![],
-        },
-    )
-    .unwrap();
-
-    // test that instantiate set the correct gov module address and RBAC permissions
-    let permissions = RBAC_PERMISSIONS
-        .load(
-            &deps.storage,
-            MockApi::default().addr_make(GOV_ADDR).to_string(),
-        )
-        .unwrap();
-    for permission in Roles::all_roles() {
-        assert!(permissions.contains(&permission));
-    }
-    assert_eq!(
-        GOVMODULE.load(deps.as_ref().storage).unwrap().to_string(),
-        MockApi::default().addr_make(GOV_ADDR).to_string()
-    );
-
-    // revoke all roles from the gov contract, instantiation should re-asssign
-    crate::rbac::revoke_role(
-        &mut deps.as_mut(),
-        MockApi::default().addr_make(GOV_ADDR).to_string(),
-        Roles::all_roles(),
-    )
-    .unwrap();
-
-    migrate(deps.as_mut(), mock_env(), MigrateMsg {}).unwrap();
-
-    // ensure migration assigned all the roles
-    let permissions = RBAC_PERMISSIONS
-        .load(
-            &deps.storage,
-            MockApi::default().addr_make(GOV_ADDR).to_string(),
-        )
-        .unwrap();
-    for permission in Roles::all_roles() {
-        assert!(permissions.contains(&permission));
-    }
-}

src/integration_tests.rs → src/tests/integration_tests.rs

@@ -1,10 +1,11 @@
 #![cfg(test)]
 
 use crate::{
-    helpers::RateLimitingContract,
     msg::{ExecuteMsg, QueryMsg},
     state::{rate_limit::RateLimit, rbac::Roles},
-    test_msg_send, ContractError,
+    test_msg_send,
+    tests::helpers::RateLimitingContract,
+    ContractError,
 };
 use cosmwasm_std::{testing::MockApi, Addr, Coin, Empty, Timestamp, Uint128, Uint256};
 use cw_multi_test::{App, AppBuilder, Contract, ContractWrapper, Executor};
@@ -25,7 +26,6 @@ pub fn contract_template() -> Box<dyn Contract<Empty>> {
 }
 
 const USER: &str = "USER";
-const IBC_ADDR: &str = "neutron1cdlz8scnf3mmxdnf4njmtp7vz4gps7fswphrqn";
 const GOV_ADDR: &str = "neutron1w02khza7ux68ccwmz2hln97mkjspjxes8y2k9v";
 const NATIVE_DENOM: &str = "nosmo";
 
@@ -52,7 +52,6 @@ fn proper_instantiate(paths: Vec<PathMsg>) -> (App, RateLimitingContract) {
 
     let msg = InstantiateMsg {
         gov_module: MockApi::default().addr_make(GOV_ADDR),
-        ibc_module: Addr::unchecked(IBC_ADDR),
         paths,
     };
 

src/tests/mod.rs

@@ -0,0 +1,3 @@
+pub mod contract_tests;
+pub mod helpers;
+pub mod integration_tests;