From ff09c19acc775a89931a0b364f7ee0bafdb9b0e7 Mon Sep 17 00:00:00 2001 From: pr0n00gler Date: Fri, 27 Sep 2024 12:48:08 +0300 Subject: [PATCH] review fixes --- Makefile | 2 +- src/contract.rs | 19 +------ src/execute.rs | 21 +++----- src/lib.rs | 6 +-- src/message_queue.rs | 10 +++- src/msg.rs | 1 - src/state/storage.rs | 3 -- src/{ => tests}/contract_tests.rs | 76 ++-------------------------- src/{ => tests}/helpers.rs | 0 src/{ => tests}/integration_tests.rs | 7 ++- src/tests/mod.rs | 3 ++ 11 files changed, 29 insertions(+), 119 deletions(-) rename src/{ => tests}/contract_tests.rs (85%) rename src/{ => tests}/helpers.rs (100%) rename src/{ => tests}/integration_tests.rs (99%) create mode 100644 src/tests/mod.rs diff --git a/Makefile b/Makefile index be3d3c7..a294e46 100644 --- a/Makefile +++ b/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 \ diff --git a/src/contract.rs b/src/contract.rs index d046d11..c2cef39 100644 --- a/src/contract.rs +++ b/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 { 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 { pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { 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)?; diff --git a/src/execute.rs b/src/execute.rs index 08fd696..63b5fe0 100644 --- a/src/execute.rs +++ b/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(); diff --git a/src/lib.rs b/src/lib.rs index e37c925..78896ec 100644 --- a/src/lib.rs +++ b/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; diff --git a/src/message_queue.rs b/src/message_queue.rs index 8ceb7d6..35f0817 100644 --- a/src/message_queue.rs +++ b/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 { 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 { diff --git a/src/msg.rs b/src/msg.rs index 2737691..91bad1b 100644 --- a/src/msg.rs +++ b/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, } diff --git a/src/state/storage.rs b/src/state/storage.rs index e8cb0a7..d37bf02 100644 --- a/src/state/storage.rs +++ b/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 = 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 = 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. diff --git a/src/contract_tests.rs b/src/tests/contract_tests.rs similarity index 85% rename from src/contract_tests.rs rename to src/tests/contract_tests.rs index c706b4d..e6c2fa4 100644 --- a/src/contract_tests.rs +++ b/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)); - } -} diff --git a/src/helpers.rs b/src/tests/helpers.rs similarity index 100% rename from src/helpers.rs rename to src/tests/helpers.rs diff --git a/src/integration_tests.rs b/src/tests/integration_tests.rs similarity index 99% rename from src/integration_tests.rs rename to src/tests/integration_tests.rs index 91002b6..5d30ff0 100644 --- a/src/integration_tests.rs +++ b/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> { } 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) -> (App, RateLimitingContract) { let msg = InstantiateMsg { gov_module: MockApi::default().addr_make(GOV_ADDR), - ibc_module: Addr::unchecked(IBC_ADDR), paths, }; diff --git a/src/tests/mod.rs b/src/tests/mod.rs new file mode 100644 index 0000000..474652d --- /dev/null +++ b/src/tests/mod.rs @@ -0,0 +1,3 @@ +pub mod contract_tests; +pub mod helpers; +pub mod integration_tests;