From aed786033a7a6ae55ef5f59d044c53b33f018568 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 9 Jan 2024 14:04:52 -0500 Subject: [PATCH 01/17] Add pausable ism with tests --- contracts/isms/pausable/Cargo.toml | 43 +++++++ contracts/isms/pausable/src/lib.rs | 164 +++++++++++++++++++++++++ packages/interface/src/ism/mod.rs | 1 + packages/interface/src/ism/pausable.rs | 26 ++++ 4 files changed, 234 insertions(+) create mode 100644 contracts/isms/pausable/Cargo.toml create mode 100644 contracts/isms/pausable/src/lib.rs create mode 100644 packages/interface/src/ism/pausable.rs diff --git a/contracts/isms/pausable/Cargo.toml b/contracts/isms/pausable/Cargo.toml new file mode 100644 index 00000000..41cef4a1 --- /dev/null +++ b/contracts/isms/pausable/Cargo.toml @@ -0,0 +1,43 @@ +[package] +name = "hpl-ism-pausable" +version.workspace = true +authors.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +homepage.workspace = true +documentation.workspace = true +keywords.workspace = true + +[lib] +crate-type = ["cdylib", "rlib"] + +[features] +# for more explicit tests, cargo test --features=backtraces +backtraces = ["cosmwasm-std/backtraces"] +# use library feature to disable all instantiate/execute/query exports +library = [] + +[dependencies] +cosmwasm-std.workspace = true +cosmwasm-storage.workspace = true +cosmwasm-schema.workspace = true + +cw-storage-plus.workspace = true +cw2.workspace = true +cw-utils.workspace = true + +schemars.workspace = true +serde-json-wasm.workspace = true + +thiserror.workspace = true + +hpl-ownable.workspace = true +hpl-pausable.workspace = true +hpl-interface.workspace = true + +[dev-dependencies] +rstest.workspace = true +ibcx-test-utils.workspace = true + +anyhow.workspace = true diff --git a/contracts/isms/pausable/src/lib.rs b/contracts/isms/pausable/src/lib.rs new file mode 100644 index 00000000..51c44146 --- /dev/null +++ b/contracts/isms/pausable/src/lib.rs @@ -0,0 +1,164 @@ +#[cfg(not(feature = "library"))] +use cosmwasm_std::entry_point; +use cosmwasm_std::{ + ensure, to_json_binary, Deps, DepsMut, Env, Event, MessageInfo, QueryResponse, Response, + StdError, +}; +use hpl_interface::ism::{ + pausable::{ExecuteMsg, InstantiateMsg, QueryMsg}, + IsmQueryMsg, IsmType, ModuleTypeResponse, VerifyResponse, +}; + +#[derive(thiserror::Error, Debug, PartialEq)] +pub enum ContractError { + #[error("{0}")] + Std(#[from] StdError), + + #[error("{0}")] + PaymentError(#[from] cw_utils::PaymentError), + + #[error("unauthorized")] + Unauthorized {}, + + #[error("hook paused")] + Paused {}, +} + +// version info for migration info +pub const CONTRACT_NAME: &str = env!("CARGO_PKG_NAME"); +pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); + +fn new_event(name: &str) -> Event { + Event::new(format!("hpl_hook_pausable::{}", name)) +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn instantiate( + deps: DepsMut, + _env: Env, + info: MessageInfo, + msg: InstantiateMsg, +) -> Result { + cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; + + let owner = deps.api.addr_validate(&msg.owner)?; + + hpl_ownable::initialize(deps.storage, &owner)?; + hpl_pausable::initialize(deps.storage, &msg.paused)?; + + Ok(Response::new().add_event( + new_event("initialize") + .add_attribute("sender", info.sender) + .add_attribute("owner", owner), + )) +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn execute( + deps: DepsMut, + env: Env, + info: MessageInfo, + msg: ExecuteMsg, +) -> Result { + match msg { + ExecuteMsg::Ownable(msg) => Ok(hpl_ownable::handle(deps, env, info, msg)?), + ExecuteMsg::Pausable(msg) => Ok(hpl_pausable::handle(deps, env, info, msg)?), + } +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result { + use IsmQueryMsg::*; + + match msg { + QueryMsg::Pausable(msg) => Ok(hpl_pausable::handle_query(deps, env, msg)?), + QueryMsg::Ownable(msg) => Ok(hpl_ownable::handle_query(deps, env, msg)?), + QueryMsg::Ism(msg) => match msg { + ModuleType {} => Ok(to_json_binary(&ModuleTypeResponse { typ: IsmType::Null })?), + Verify { + metadata: _, + message: _, + } => { + ensure!( + !hpl_pausable::get_pause_info(deps.storage)?, + ContractError::Paused {} + ); + Ok(to_json_binary(&VerifyResponse { verified: true })?) + } + _ => unimplemented!(), + }, + } +} + +#[cfg(test)] +mod test { + use cosmwasm_schema::serde::{de::DeserializeOwned, Serialize}; + use cosmwasm_std::{ + from_json, + testing::{mock_dependencies, mock_env, mock_info, MockApi, MockQuerier, MockStorage}, + to_json_binary, Addr, OwnedDeps, + }; + use hpl_ownable::get_owner; + use hpl_pausable::get_pause_info; + use ibcx_test_utils::{addr, hex}; + use rstest::{fixture, rstest}; + + use super::*; + + type TestDeps = OwnedDeps; + + fn query(deps: Deps, msg: S) -> T { + let req: QueryMsg = from_json(to_json_binary(&msg).unwrap()).unwrap(); + let res = crate::query(deps, mock_env(), req).unwrap(); + from_json(res).unwrap() + } + + #[fixture] + fn deps( + #[default(addr("deployer"))] sender: Addr, + #[default(addr("owner"))] owner: Addr, + #[default(false)] paused: bool, + ) -> TestDeps { + let mut deps = mock_dependencies(); + + instantiate( + deps.as_mut(), + mock_env(), + mock_info(sender.as_str(), &[]), + InstantiateMsg { + owner: owner.to_string(), + paused, + }, + ) + .unwrap(); + + deps + } + + #[rstest] + fn test_init(deps: TestDeps) { + assert!(!get_pause_info(deps.as_ref().storage).unwrap()); + assert_eq!("owner", get_owner(deps.as_ref().storage).unwrap().as_str()); + } + + #[rstest] + #[case(false)] + #[should_panic(expected = "hook paused")] + #[case(true)] + fn test_query(mut deps: TestDeps, #[case] paused: bool) { + if paused { + hpl_pausable::pause(deps.as_mut().storage, &addr("owner")).unwrap(); + } + + let raw_message = hex("0000000000000068220000000000000000000000000d1255b09d94659bb0888e0aa9fca60245ce402a0000682155208cd518cffaac1b5d8df216a9bd050c9a03f0d4f3ba88e5268ac4cd12ee2d68656c6c6f"); + let raw_metadata = raw_message.clone(); + + query::<_, VerifyResponse>( + deps.as_ref(), + QueryMsg::Ism(IsmQueryMsg::Verify { + metadata: raw_metadata, + message: raw_message, + }), + ); + } +} diff --git a/packages/interface/src/ism/mod.rs b/packages/interface/src/ism/mod.rs index 13cccbe4..f3947053 100644 --- a/packages/interface/src/ism/mod.rs +++ b/packages/interface/src/ism/mod.rs @@ -1,6 +1,7 @@ pub mod aggregate; pub mod multisig; pub mod routing; +pub mod pausable; use cosmwasm_schema::{cw_serde, QueryResponses}; use cosmwasm_std::{Addr, CustomQuery, HexBinary, QuerierWrapper, StdResult}; diff --git a/packages/interface/src/ism/pausable.rs b/packages/interface/src/ism/pausable.rs new file mode 100644 index 00000000..1f48934f --- /dev/null +++ b/packages/interface/src/ism/pausable.rs @@ -0,0 +1,26 @@ +use cosmwasm_schema::{cw_serde, QueryResponses}; + +use crate::{ownable::{OwnableMsg, OwnableQueryMsg}, pausable::{PausableMsg, PausableQueryMsg}}; + +use super::IsmQueryMsg; + +#[cw_serde] +pub struct InstantiateMsg { + pub owner: String, + pub paused: bool +} + +#[cw_serde] +pub enum ExecuteMsg { + Ownable(OwnableMsg), + Pausable(PausableMsg) +} + +#[cw_serde] +#[derive(QueryResponses)] +#[query_responses(nested)] +pub enum QueryMsg { + Ownable(OwnableQueryMsg), + Ism(IsmQueryMsg), + Pausable(PausableQueryMsg) +} From ce5c82b6752c354b59ed4c115135cdac489cd8b1 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 9 Jan 2024 14:08:49 -0500 Subject: [PATCH 02/17] Fix paused query error case --- contracts/isms/pausable/src/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/contracts/isms/pausable/src/lib.rs b/contracts/isms/pausable/src/lib.rs index 51c44146..d89ebe22 100644 --- a/contracts/isms/pausable/src/lib.rs +++ b/contracts/isms/pausable/src/lib.rs @@ -107,10 +107,9 @@ mod test { type TestDeps = OwnedDeps; - fn query(deps: Deps, msg: S) -> T { + fn query(deps: Deps, msg: crate::QueryMsg) -> Result { let req: QueryMsg = from_json(to_json_binary(&msg).unwrap()).unwrap(); - let res = crate::query(deps, mock_env(), req).unwrap(); - from_json(res).unwrap() + crate::query(deps, mock_env(), req) } #[fixture] @@ -153,12 +152,14 @@ mod test { let raw_message = hex("0000000000000068220000000000000000000000000d1255b09d94659bb0888e0aa9fca60245ce402a0000682155208cd518cffaac1b5d8df216a9bd050c9a03f0d4f3ba88e5268ac4cd12ee2d68656c6c6f"); let raw_metadata = raw_message.clone(); - query::<_, VerifyResponse>( + query( deps.as_ref(), QueryMsg::Ism(IsmQueryMsg::Verify { metadata: raw_metadata, message: raw_message, }), - ); + ) + .map_err(|e| e.to_string()) + .unwrap(); } } From f623cbc5c65119e4e667b6697f280a6d953462ce Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 9 Jan 2024 14:19:55 -0500 Subject: [PATCH 03/17] Run CI against all PRs --- .github/workflows/test.yaml | 2 -- contracts/isms/pausable/src/lib.rs | 1 - 2 files changed, 3 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 7959e3cd..ae8bdc8f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -2,8 +2,6 @@ name: test on: pull_request: - branches: - - "main" push: branches: - "main" diff --git a/contracts/isms/pausable/src/lib.rs b/contracts/isms/pausable/src/lib.rs index d89ebe22..16b82b85 100644 --- a/contracts/isms/pausable/src/lib.rs +++ b/contracts/isms/pausable/src/lib.rs @@ -92,7 +92,6 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result Date: Tue, 9 Jan 2024 14:22:53 -0500 Subject: [PATCH 04/17] Add pausable ISM to README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 79814e61..fe00de58 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,8 @@ grcov . -s . --binary-path ./target/debug/ -t lcov --branch --ignore-not-existin - [aggregate ism](./contracts/isms/aggregate) + - [pausable](./contracts/isms/pausable) + - For testing: [mock ism](./contracts/mocks/mock-ism) 5. Set deployed hooks and isms to Mailbox From d9d89378b65bc4709cea9a4c64cbdff03bfe4f28 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 9 Jan 2024 16:46:04 -0500 Subject: [PATCH 05/17] Fix scripts --- scripts/action/ism.ts | 14 +++++++------- scripts/action/mailbox.ts | 18 +++++++++--------- scripts/src/contracts/hpl_ism_pausable.ts | 7 +++++++ scripts/src/contracts/index.ts | 5 +++-- scripts/src/migrations/InitializeStandalone.ts | 9 +++++---- scripts/src/types.ts | 10 +++++----- 6 files changed, 36 insertions(+), 27 deletions(-) create mode 100644 scripts/src/contracts/hpl_ism_pausable.ts diff --git a/scripts/action/ism.ts b/scripts/action/ism.ts index 897a8d3f..f842d776 100644 --- a/scripts/action/ism.ts +++ b/scripts/action/ism.ts @@ -1,17 +1,17 @@ -import { Command } from "commander"; import { ExecuteResult } from "@cosmjs/cosmwasm-stargate"; +import { Command } from "commander"; import { version } from "../package.json"; import { config, getSigningClient } from "../src/config"; -import { loadContext } from "../src/load_context"; -import { ContractFetcher } from "./fetch"; import { - HplMailbox, - HplIgp, - HplIgpGasOracle, HplHookMerkle, + HplIgp, + HplIgpOracle, HplIsmAggregate, + HplMailbox, } from "../src/contracts"; +import { loadContext } from "../src/load_context"; +import { ContractFetcher } from "./fetch"; const program = new Command(); @@ -51,7 +51,7 @@ function makeHandler( const fetcher = new ContractFetcher(ctx, client); const mailbox = fetcher.get(HplMailbox, "hpl_mailbox"); const igp = fetcher.get(HplIgp, "hpl_igp"); - const igp_oracle = fetcher.get(HplIgpGasOracle, "hpl_igp_oracle"); + const igp_oracle = fetcher.get(HplIgpOracle, "hpl_igp_oracle"); const hook_merkle = fetcher.get(HplHookMerkle, "hpl_hook_merkle"); const hook_aggregate = fetcher.get(HplIsmAggregate, "hpl_hook_aggregate"); diff --git a/scripts/action/mailbox.ts b/scripts/action/mailbox.ts index 2509708d..bd40ea1a 100644 --- a/scripts/action/mailbox.ts +++ b/scripts/action/mailbox.ts @@ -1,18 +1,18 @@ -import { Command } from "commander"; import { ExecuteResult } from "@cosmjs/cosmwasm-stargate"; +import { Command } from "commander"; import { version } from "../package.json"; import { config, getSigningClient } from "../src/config"; +import { + HplHookMerkle, + HplIgp, + HplIgpOracle, + HplIsmAggregate, + HplMailbox, +} from "../src/contracts"; import { addPad } from "../src/conv"; import { loadContext } from "../src/load_context"; import { ContractFetcher } from "./fetch"; -import { - HplMailbox, - HplIgp, - HplIgpGasOracle, - HplHookMerkle, - HplIsmAggregate, -} from "../src/contracts"; const program = new Command(); @@ -54,7 +54,7 @@ function makeHandler( const fetcher = new ContractFetcher(ctx, client); const mailbox = fetcher.get(HplMailbox, "hpl_mailbox"); const igp = fetcher.get(HplIgp, "hpl_igp"); - const igp_oracle = fetcher.get(HplIgpGasOracle, "hpl_igp_oracle"); + const igp_oracle = fetcher.get(HplIgpOracle, "hpl_igp_oracle"); const hook_merkle = fetcher.get(HplHookMerkle, "hpl_hook_merkle"); const hook_aggregate = fetcher.get(HplIsmAggregate, "hpl_hook_aggregate"); diff --git a/scripts/src/contracts/hpl_ism_pausable.ts b/scripts/src/contracts/hpl_ism_pausable.ts new file mode 100644 index 00000000..7fce739a --- /dev/null +++ b/scripts/src/contracts/hpl_ism_pausable.ts @@ -0,0 +1,7 @@ +import { injectable } from "inversify"; +import { BaseContract } from "../types"; + +@injectable() +export class HplIsmPausable extends BaseContract { + contractName: string = "hpl_ism_pausable"; +} diff --git a/scripts/src/contracts/index.ts b/scripts/src/contracts/index.ts index 8016e991..417f7be2 100644 --- a/scripts/src/contracts/index.ts +++ b/scripts/src/contracts/index.ts @@ -8,6 +8,7 @@ export * from "./hpl_igp"; export * from "./hpl_igp_oracle"; export * from "./hpl_ism_aggregate"; export * from "./hpl_ism_multisig"; +export * from "./hpl_ism_pausable"; export * from "./hpl_ism_routing"; export * from "./hpl_mailbox"; export * from "./hpl_test_mock_hook"; @@ -16,10 +17,10 @@ export * from "./hpl_validator_announce"; export * from "./hpl_warp_cw20"; export * from "./hpl_warp_native"; -import { readdirSync } from "fs"; -import { Context, Contract, ContractConstructor } from "../types"; import { SigningCosmWasmClient } from "@cosmjs/cosmwasm-stargate"; +import { readdirSync } from "fs"; import { Container } from "inversify"; +import { Context, Contract, ContractConstructor } from "../types"; const contractNames: string[] = readdirSync(__dirname) .filter((f) => f !== "index.ts") diff --git a/scripts/src/migrations/InitializeStandalone.ts b/scripts/src/migrations/InitializeStandalone.ts index 920cc5b4..1bede003 100644 --- a/scripts/src/migrations/InitializeStandalone.ts +++ b/scripts/src/migrations/InitializeStandalone.ts @@ -1,12 +1,13 @@ import { injectable } from "inversify"; -import { Context, Migration } from "../types"; import { - HplMailbox, HplHookMerkle, - HplIgpGasOracle, + HplIgp, + HplIgpOracle, HplIsmMultisig, + HplMailbox, HplTestMockHook, } from "../contracts"; +import { Context, Migration } from "../types"; @injectable() export default class InitializeStandalone implements Migration { @@ -18,7 +19,7 @@ export default class InitializeStandalone implements Migration { private mailbox: HplMailbox, private hook_merkle: HplHookMerkle, private igp: HplIgp, - private igp_oracle: HplIgpGasOracle, + private igp_oracle: HplIgpOracle, private ism_multisig: HplIsmMultisig, private test_mock_hook: HplTestMockHook ) {} diff --git a/scripts/src/types.ts b/scripts/src/types.ts index b28f9edc..95c63af6 100644 --- a/scripts/src/types.ts +++ b/scripts/src/types.ts @@ -1,10 +1,10 @@ import { - ExecuteResult, - SigningCosmWasmClient, + ExecuteResult, + SigningCosmWasmClient, } from "@cosmjs/cosmwasm-stargate"; -import { getWasmPath } from "./load_wasm"; -import fs from "fs"; import { fromBech32 } from "@cosmjs/encoding"; +import fs from "fs"; +import { getWasmPath } from "./load_wasm"; export interface ContractContext { codeId: number | undefined; @@ -165,7 +165,7 @@ export interface HplIgpCoreInstantiateMsg { beneficiary: string; } -export interface HplIgpGasOracleInstantiateMsg {} +export interface HplIgpOracleInstantiateMsg {} export interface HplIsmMultisigInstantiateMsg { owner: string; From 496e66b91d8e85754cdacabccd16fe20214dd899 Mon Sep 17 00:00:00 2001 From: nambrot Date: Tue, 9 Jan 2024 17:32:25 -0500 Subject: [PATCH 06/17] Allow threshold == set size and add tests --- contracts/isms/multisig/src/contract.rs | 76 +++++- contracts/isms/multisig/src/execute.rs | 319 ------------------------ 2 files changed, 75 insertions(+), 320 deletions(-) delete mode 100644 contracts/isms/multisig/src/execute.rs diff --git a/contracts/isms/multisig/src/contract.rs b/contracts/isms/multisig/src/contract.rs index ce4075a1..6741189d 100644 --- a/contracts/isms/multisig/src/contract.rs +++ b/contracts/isms/multisig/src/contract.rs @@ -64,7 +64,7 @@ pub fn execute( ContractError::invalid_addr("length should be 20") ); ensure!( - validators.len() > threshold as usize && threshold > 0, + validators.len() >= threshold as usize && threshold > 0, ContractError::invalid_args(&format!( "threshold not in range. 0 < <= {}", validators.len(), @@ -137,3 +137,77 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result Result { Ok(Response::new()) } + +#[cfg(test)] +mod test { + use cosmwasm_std::{ + testing::mock_dependencies, HexBinary, + }; + use hpl_interface::{ + build_test_executor, build_test_querier, + ism::multisig::ExecuteMsg, + }; + use ibcx_test_utils::{addr, hex}; + use rstest::rstest; + + use crate::state::VALIDATORS; + + build_test_executor!(crate::contract::execute); + build_test_querier!(crate::contract::query); + + #[rstest] + #[case("owner", vec![hex(&"deadbeef".repeat(5))])] + #[should_panic(expected = "unauthorized")] + #[case("someone", vec![hex(&"deadbeef".repeat(5))])] + fn test_enroll(#[case] sender: &str, #[case] validators: Vec) { + let mut deps = mock_dependencies(); + + hpl_ownable::initialize(deps.as_mut().storage, &addr("owner")).unwrap(); + + test_execute( + deps.as_mut(), + &addr(sender), + ExecuteMsg::SetValidators { + domain: 1, + threshold: 1, + validators: validators.clone(), + }, + vec![], + ); + + assert_eq!( + VALIDATORS.load(deps.as_ref().storage, 1).unwrap(), + validators + ); + } + + #[rstest] + #[case("owner")] + #[should_panic(expected = "unauthorized")] + #[case("someone")] + fn test_unenroll(#[case] sender: &str) { + let mut deps = mock_dependencies(); + + hpl_ownable::initialize(deps.as_mut().storage, &addr("owner")).unwrap(); + + test_execute( + deps.as_mut(), + &addr("owner"), + ExecuteMsg::SetValidators { + domain: 1, + threshold: 1, + validators: vec![hex(&"deadbeef".repeat(5))], + }, + vec![], + ); + + test_execute( + deps.as_mut(), + &addr(sender), + ExecuteMsg::UnsetDomain { domain: 1 } , + vec![], + ); + + assert!(!VALIDATORS.has(deps.as_ref().storage, 1)); + } +} diff --git a/contracts/isms/multisig/src/execute.rs b/contracts/isms/multisig/src/execute.rs deleted file mode 100644 index d4661fbb..00000000 --- a/contracts/isms/multisig/src/execute.rs +++ /dev/null @@ -1,319 +0,0 @@ -use cosmwasm_std::{ensure_eq, DepsMut, Event, HexBinary, MessageInfo, Response, StdResult}; -use hpl_interface::ism::multisig::{ThresholdSet, ValidatorSet as MsgValidatorSet}; -use hpl_ownable::get_owner; - -use crate::{ - event::{emit_enroll_validator, emit_set_threshold, emit_unenroll_validator}, - state::{THRESHOLD, VALIDATORS}, - ContractError, -}; - -pub fn set_threshold( - deps: DepsMut, - info: MessageInfo, - threshold: ThresholdSet, -) -> Result { - ensure_eq!( - get_owner(deps.storage)?, - info.sender, - ContractError::Unauthorized - ); - THRESHOLD.save(deps.storage, threshold.domain, &threshold.threshold)?; - - Ok(Response::new().add_event(emit_set_threshold(threshold.domain, threshold.threshold))) -} - -pub fn set_thresholds( - deps: DepsMut, - info: MessageInfo, - thresholds: Vec, -) -> Result { - ensure_eq!( - get_owner(deps.storage)?, - info.sender, - ContractError::Unauthorized - ); - - let events: Vec = thresholds - .into_iter() - .map(|v| { - THRESHOLD.save(deps.storage, v.domain, &v.threshold)?; - Ok(emit_set_threshold(v.domain, v.threshold)) - }) - .collect::>()?; - - Ok(Response::new().add_events(events)) -} - -pub fn enroll_validator( - deps: DepsMut, - info: MessageInfo, - msg: MsgValidatorSet, -) -> Result { - ensure_eq!( - info.sender, - get_owner(deps.storage)?, - ContractError::Unauthorized {} - ); - - ensure_eq!( - msg.validator.len(), - 20, - ContractError::invalid_addr("length should be 20") - ); - - let validator_state = VALIDATORS.may_load(deps.storage, msg.domain)?; - - if let Some(mut validators) = validator_state { - if validators.contains(&msg.validator) { - return Err(ContractError::ValidatorDuplicate {}); - } - - validators.push(msg.validator.clone()); - validators.sort(); - - VALIDATORS.save(deps.storage, msg.domain, &validators)?; - } else { - VALIDATORS.save(deps.storage, msg.domain, &vec![msg.validator.clone()])?; - } - - Ok(Response::new().add_event(emit_enroll_validator(msg.domain, msg.validator.to_hex()))) -} - -pub fn enroll_validators( - deps: DepsMut, - info: MessageInfo, - validators: Vec, -) -> Result { - ensure_eq!( - info.sender, - get_owner(deps.storage)?, - ContractError::Unauthorized {} - ); - - let mut events: Vec = Vec::new(); - - for msg in validators.into_iter() { - ensure_eq!( - msg.validator.len(), - 20, - ContractError::invalid_addr("length should be 20") - ); - - let validators_state = VALIDATORS.may_load(deps.storage, msg.domain)?; - - if let Some(mut validators) = validators_state { - if validators.contains(&msg.validator) { - return Err(ContractError::ValidatorDuplicate {}); - } - - validators.push(msg.validator.clone()); - validators.sort(); - - VALIDATORS.save(deps.storage, msg.domain, &validators)?; - events.push(emit_enroll_validator(msg.domain, msg.validator.to_hex())); - } else { - VALIDATORS.save(deps.storage, msg.domain, &vec![msg.validator.clone()])?; - events.push(emit_enroll_validator(msg.domain, msg.validator.to_hex())); - } - } - - Ok(Response::new().add_events(events)) -} - -pub fn unenroll_validator( - deps: DepsMut, - info: MessageInfo, - domain: u32, - validator: HexBinary, -) -> Result { - ensure_eq!( - info.sender, - get_owner(deps.storage)?, - ContractError::Unauthorized {} - ); - - let validators = VALIDATORS - .load(deps.storage, domain) - .map_err(|_| ContractError::ValidatorNotExist {})?; - - if !validators.contains(&validator) { - return Err(ContractError::ValidatorNotExist {}); - } - - let mut validator_list: Vec = - validators.into_iter().filter(|v| v != &validator).collect(); - - validator_list.sort(); - - VALIDATORS.save(deps.storage, domain, &validator_list)?; - - Ok(Response::new().add_event(emit_unenroll_validator(domain, validator.to_hex()))) -} - -#[cfg(test)] -mod test { - use cosmwasm_std::{ - testing::{mock_dependencies, mock_info}, - Addr, HexBinary, Storage, - }; - use hpl_interface::{ - build_test_executor, build_test_querier, - ism::multisig::{ExecuteMsg, ValidatorSet}, - }; - use ibcx_test_utils::{addr, hex}; - use rstest::rstest; - - use crate::state::VALIDATORS; - - build_test_executor!(crate::contract::execute); - build_test_querier!(crate::contract::query); - - use super::*; - const ADDR1_VAULE: &str = "addr1"; - const ADDR2_VAULE: &str = "addr2"; - - fn mock_owner(storage: &mut dyn Storage, owner: Addr) { - hpl_ownable::initialize(storage, &owner).unwrap(); - } - - #[test] - fn test_set_threshold() { - let mut deps = mock_dependencies(); - let owner = Addr::unchecked(ADDR1_VAULE); - mock_owner(deps.as_mut().storage, owner.clone()); - - let threshold = ThresholdSet { - domain: 1u32, - threshold: 8u8, - }; - - // set_threshold failure test - let info = mock_info(ADDR2_VAULE, &[]); - let fail_result = set_threshold(deps.as_mut(), info, threshold.clone()).unwrap_err(); - - assert!(matches!(fail_result, ContractError::Unauthorized {})); - - // set_threshold success test - let info = mock_info(owner.as_str(), &[]); - let result = set_threshold(deps.as_mut(), info, threshold.clone()).unwrap(); - - assert_eq!( - result.events, - vec![emit_set_threshold(threshold.domain, threshold.threshold)] - ); - - // check it actually saved - let saved_threshold = THRESHOLD.load(&deps.storage, threshold.domain).unwrap(); - assert_eq!(saved_threshold, threshold.threshold); - } - - #[test] - fn test_set_thresholds() { - let mut deps = mock_dependencies(); - let owner = Addr::unchecked(ADDR1_VAULE); - mock_owner(deps.as_mut().storage, owner.clone()); - - let thresholds: Vec = vec![ - ThresholdSet { - domain: 1u32, - threshold: 8u8, - }, - ThresholdSet { - domain: 2u32, - threshold: 7u8, - }, - ThresholdSet { - domain: 3u32, - threshold: 6u8, - }, - ]; - - // set_threshold failure test - let info = mock_info(ADDR2_VAULE, &[]); - let fail_result = set_thresholds(deps.as_mut(), info, thresholds.clone()).unwrap_err(); - - assert!(matches!(fail_result, ContractError::Unauthorized {})); - - // set_threshold success test - let info = mock_info(owner.as_str(), &[]); - let result = set_thresholds(deps.as_mut(), info, thresholds.clone()).unwrap(); - - assert_eq!( - result.events, - vec![ - emit_set_threshold(1u32, 8u8), - emit_set_threshold(2u32, 7u8), - emit_set_threshold(3u32, 6u8), - ] - ); - - // check it actually saved - for threshold in thresholds { - let saved_threshold = THRESHOLD.load(&deps.storage, threshold.domain).unwrap(); - assert_eq!(saved_threshold, threshold.threshold); - } - } - - #[rstest] - #[case("owner", vec![hex(&"deadbeef".repeat(5))])] - #[should_panic(expected = "unauthorized")] - #[case("someone", vec![hex(&"deadbeef".repeat(5))])] - #[should_panic(expected = "duplicate validator")] - #[case("owner", vec![hex(&"deadbeef".repeat(5)),hex(&"deadbeef".repeat(5))])] - fn test_enroll(#[case] sender: &str, #[case] validators: Vec) { - let mut deps = mock_dependencies(); - - hpl_ownable::initialize(deps.as_mut().storage, &addr("owner")).unwrap(); - - for validator in validators.clone() { - test_execute( - deps.as_mut(), - &addr(sender), - ExecuteMsg::EnrollValidator { - set: ValidatorSet { - domain: 1, - validator, - }, - }, - vec![], - ); - } - - assert_eq!( - VALIDATORS.load(deps.as_ref().storage, 1).unwrap(), - validators - ); - } - - #[rstest] - #[case("owner", hex("deadbeef"))] - #[should_panic(expected = "unauthorized")] - #[case("someone", hex("deadbeef"))] - #[should_panic(expected = "validator not exist")] - #[case("owner", hex("debeefed"))] - fn test_unenroll(#[case] sender: &str, #[case] target: HexBinary) { - let mut deps = mock_dependencies(); - - hpl_ownable::initialize(deps.as_mut().storage, &addr("owner")).unwrap(); - - VALIDATORS - .save(deps.as_mut().storage, 1, &vec![hex("deadbeef")]) - .unwrap(); - - test_execute( - deps.as_mut(), - &addr(sender), - ExecuteMsg::UnenrollValidator { - domain: 1, - validator: target, - }, - vec![], - ); - - assert!(VALIDATORS - .load(deps.as_ref().storage, 1) - .unwrap() - .is_empty()); - } -} From 1eb2687c3e1c8ea43a843e63883e47f583caae26 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Fri, 12 Jan 2024 14:46:19 -0500 Subject: [PATCH 07/17] Refactor mailbox hook fee logic --- contracts/core/mailbox/src/execute.rs | 74 +++++++++------------------ 1 file changed, 24 insertions(+), 50 deletions(-) diff --git a/contracts/core/mailbox/src/execute.rs b/contracts/core/mailbox/src/execute.rs index 3e43a62f..a1edef71 100644 --- a/contracts/core/mailbox/src/execute.rs +++ b/contracts/core/mailbox/src/execute.rs @@ -1,13 +1,14 @@ use cosmwasm_std::{ - ensure, ensure_eq, to_json_binary, wasm_execute, BankMsg, Coins, DepsMut, Env, HexBinary, - MessageInfo, Response, StdResult, + ensure, ensure_eq, has_coins, to_json_binary, wasm_execute, BankMsg, Coin, Coins, DepsMut, Env, + HexBinary, MessageInfo, Response, StdResult, }; +use cw_utils::PaymentError::MissingDenom; use hpl_interface::{ core::{ mailbox::{DispatchMsg, DispatchResponse}, HandleMsg, }, - hook::{self, post_dispatch}, + hook::{post_dispatch, quote_dispatch}, ism, types::Message, }; @@ -107,74 +108,47 @@ pub fn dispatch( } ); - // calculate gas - let default_hook = config.get_default_hook(); - let required_hook = config.get_required_hook(); - + // build hyperlane message let msg = dispatch_msg .clone() .to_msg(MAILBOX_VERSION, nonce, config.local_domain, &info.sender)?; - let msg_id = msg.id(); + let metadata = dispatch_msg.clone().metadata.unwrap_or_default(); + let hook = dispatch_msg.get_hook_addr(deps.api, config.get_default_hook())?; - let base_fee = hook::quote_dispatch( - &deps.querier, - dispatch_msg.get_hook_addr(deps.api, default_hook)?, - dispatch_msg.metadata.clone().unwrap_or_default(), - msg.clone(), - )? - .fees; - - let required_fee = hook::quote_dispatch( - &deps.querier, - &required_hook, - dispatch_msg.metadata.clone().unwrap_or_default(), - msg.clone(), - )? - .fees; - - // assert gas received is satisfies required gas - let mut total_fee = required_fee.clone().into_iter().try_fold( - Coins::try_from(base_fee.clone())?, - |mut acc, fee| { - acc.add(fee)?; - StdResult::Ok(acc) - }, - )?; - for fund in info.funds { - total_fee.sub(fund)?; - } + // assert gas received satisfies required gas + let required_hook = config.get_required_hook(); + let required_hook_fees: Vec = + quote_dispatch(&deps.querier, &required_hook, metadata.clone(), msg.clone())?.fees; - // interaction - let hook = dispatch_msg.get_hook_addr(deps.api, config.get_default_hook())?; - let hook_metadata = dispatch_msg.metadata.unwrap_or_default(); + let mut funds = Coins::try_from(info.funds)?; + for coin in required_hook_fees.iter() { + if let Err(_) = funds.sub(coin.clone()) { + return Err(ContractError::Payment(MissingDenom(coin.denom.clone()))); + } + } - // effects + // commit to message + let msg_id = msg.id(); NONCE.save(deps.storage, &(nonce + 1))?; LATEST_DISPATCHED_ID.save(deps.storage, &msg_id.to_vec())?; - // make message + // build post dispatch calls let post_dispatch_msgs = vec![ post_dispatch( required_hook, - hook_metadata.clone(), + metadata.clone(), msg.clone(), - Some(required_fee), + Some(required_hook_fees), )?, - post_dispatch(hook, hook_metadata, msg.clone(), Some(base_fee))?, + post_dispatch(hook, metadata, msg.clone(), Some(funds.to_vec()))?, ]; - let refund_msg = BankMsg::Send { - to_address: info.sender.to_string(), - amount: total_fee.to_vec(), - }; - Ok(Response::new() .add_event(emit_dispatch_id(msg_id.clone())) .add_event(emit_dispatch(msg)) .set_data(to_json_binary(&DispatchResponse { message_id: msg_id })?) - .add_messages(post_dispatch_msgs) - .add_message(refund_msg)) + .add_messages(post_dispatch_msgs)) } pub fn process( From bf30fe317c01957e4cc8457e940c5da3570c7ca0 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Fri, 12 Jan 2024 14:55:32 -0500 Subject: [PATCH 08/17] Remove unused imports --- contracts/core/mailbox/src/execute.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/core/mailbox/src/execute.rs b/contracts/core/mailbox/src/execute.rs index a1edef71..33792d39 100644 --- a/contracts/core/mailbox/src/execute.rs +++ b/contracts/core/mailbox/src/execute.rs @@ -1,6 +1,6 @@ use cosmwasm_std::{ - ensure, ensure_eq, has_coins, to_json_binary, wasm_execute, BankMsg, Coin, Coins, DepsMut, Env, - HexBinary, MessageInfo, Response, StdResult, + ensure, ensure_eq, to_json_binary, wasm_execute, Coin, Coins, DepsMut, Env, + HexBinary, MessageInfo, Response, }; use cw_utils::PaymentError::MissingDenom; use hpl_interface::{ From 25f1e46fa08d2aec588ca1ed5ca27a1b85086cd8 Mon Sep 17 00:00:00 2001 From: ByeongSu Hong Date: Tue, 16 Jan 2024 00:20:11 +0900 Subject: [PATCH 09/17] fix: coverage (#87) remove flag --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index ae8bdc8f..780d7c8e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -52,7 +52,7 @@ jobs: uses: taiki-e/install-action@cargo-llvm-cov - name: Generate code coverage - run: cargo llvm-cov --all-features --workspace --exclude hpl-tests --codecov --output-path codecov.json + run: cargo llvm-cov --workspace --exclude hpl-tests --codecov --output-path codecov.json - name: Upload to codecov.io uses: codecov/codecov-action@v3 From 282430c2d5d4ec6905ff958f6641e7847272592f Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 16 Jan 2024 13:00:45 -0500 Subject: [PATCH 10/17] Implement simple fee hook_ --- contracts/hooks/fees/Cargo.toml | 42 ++++++ contracts/hooks/fees/src/lib.rs | 235 +++++++++++++++++++++++++++++ packages/interface/src/hook/fee.rs | 83 ++++++++++ packages/interface/src/hook/mod.rs | 1 + 4 files changed, 361 insertions(+) create mode 100644 contracts/hooks/fees/Cargo.toml create mode 100644 contracts/hooks/fees/src/lib.rs create mode 100644 packages/interface/src/hook/fee.rs diff --git a/contracts/hooks/fees/Cargo.toml b/contracts/hooks/fees/Cargo.toml new file mode 100644 index 00000000..d5f6e792 --- /dev/null +++ b/contracts/hooks/fees/Cargo.toml @@ -0,0 +1,42 @@ +[package] +name = "hpl-hook-fee" +version.workspace = true +authors.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +homepage.workspace = true +documentation.workspace = true +keywords.workspace = true + +[lib] +crate-type = ["cdylib", "rlib"] + +[features] +# for more explicit tests, cargo test --features=backtraces +backtraces = ["cosmwasm-std/backtraces"] +# use library feature to disable all instantiate/execute/query exports +library = [] + +[dependencies] +cosmwasm-std.workspace = true +cosmwasm-storage.workspace = true +cosmwasm-schema.workspace = true + +cw-storage-plus.workspace = true +cw2.workspace = true +cw-utils.workspace = true + +schemars.workspace = true +serde-json-wasm.workspace = true + +thiserror.workspace = true + +hpl-ownable.workspace = true +hpl-interface.workspace = true + +[dev-dependencies] +rstest.workspace = true +ibcx-test-utils.workspace = true + +anyhow.workspace = true diff --git a/contracts/hooks/fees/src/lib.rs b/contracts/hooks/fees/src/lib.rs new file mode 100644 index 00000000..adc42c16 --- /dev/null +++ b/contracts/hooks/fees/src/lib.rs @@ -0,0 +1,235 @@ +#[cfg(not(feature = "library"))] +use cosmwasm_std::entry_point; +use cosmwasm_std::{ + ensure, ensure_eq, BankMsg, Coin, CosmosMsg, Deps, DepsMut, Env, Event, + MessageInfo, QueryResponse, Response, StdError, +}; +use cw_storage_plus::Item; +use hpl_interface::{ + hook::{ + fee::{ExecuteMsg, FeeHookMsg, FeeHookQueryMsg, FeeResponse, InstantiateMsg, QueryMsg}, + HookQueryMsg, MailboxResponse, QuoteDispatchResponse, + }, + to_binary, +}; + +#[derive(thiserror::Error, Debug, PartialEq)] +pub enum ContractError { + #[error("{0}")] + Std(#[from] StdError), + + #[error("{0}")] + PaymentError(#[from] cw_utils::PaymentError), + + #[error("unauthorized")] + Unauthorized {}, + + #[error("hook paused")] + Paused {}, +} + +// version info for migration info +pub const CONTRACT_NAME: &str = env!("CARGO_PKG_NAME"); +pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); + +pub const COIN_FEE_KEY: &str = "coin_fee"; +pub const COIN_FEE: Item = Item::new(COIN_FEE_KEY); + +fn new_event(name: &str) -> Event { + Event::new(format!("hpl_hook_fee::{}", name)) +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn instantiate( + deps: DepsMut, + _env: Env, + info: MessageInfo, + msg: InstantiateMsg, +) -> Result { + cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; + + let owner = deps.api.addr_validate(&msg.owner)?; + + hpl_ownable::initialize(deps.storage, &owner)?; + COIN_FEE.save(deps.storage, &msg.fee)?; + + Ok(Response::new().add_event( + new_event("initialize") + .add_attribute("sender", info.sender) + .add_attribute("owner", owner) + .add_attribute("fee_denom", msg.fee.denom) + .add_attribute("fee_amount", msg.fee.amount), + )) +} + +fn get_fee(deps: Deps) -> Result { + let fee = COIN_FEE.load(deps.storage)?; + + Ok(FeeResponse { fee }) +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn execute( + deps: DepsMut, + env: Env, + info: MessageInfo, + msg: ExecuteMsg, +) -> Result { + match msg { + ExecuteMsg::Ownable(msg) => Ok(hpl_ownable::handle(deps, env, info, msg)?), + ExecuteMsg::FeeHook(msg) => match msg { + FeeHookMsg::SetFee { fee } => { + COIN_FEE.save(deps.storage, &fee)?; + + Ok(Response::new().add_event( + new_event("set_fee") + .add_attribute("fee_denom", fee.denom) + .add_attribute("fee_amount", fee.amount), + )) + } + FeeHookMsg::Claim { recipient } => { + let owner = hpl_ownable::get_owner(deps.storage)?; + ensure_eq!(owner, info.sender, StdError::generic_err("unauthorized")); + + let recipient = recipient.unwrap_or(owner); + let balances = deps.querier.query_all_balances(&env.contract.address)?; + + let claim_msg: CosmosMsg = BankMsg::Send { + to_address: recipient.into_string(), + amount: balances, + } + .into(); + + Ok(Response::new() + .add_message(claim_msg) + .add_event(new_event("claim"))) + } + }, + ExecuteMsg::PostDispatch(_) => { + let fee = COIN_FEE.load(deps.storage)?; + let supplied = cw_utils::must_pay(&info, &fee.denom)?; + + ensure!( + supplied.u128() >= fee.amount.u128(), + StdError::generic_err("insufficient fee") + ); + + Ok(Response::new().add_event( + new_event("post_dispatch") + .add_attribute("paid_denom", fee.denom) + .add_attribute("paid_amount", supplied), + )) + } + } +} + +#[cfg_attr(not(feature = "library"), entry_point)] +pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result { + match msg { + QueryMsg::Ownable(msg) => Ok(hpl_ownable::handle_query(deps, env, msg)?), + QueryMsg::Hook(msg) => match msg { + HookQueryMsg::Mailbox {} => to_binary(get_mailbox(deps)), + HookQueryMsg::QuoteDispatch(_) => to_binary(quote_dispatch()), + }, + QueryMsg::FeeHook(FeeHookQueryMsg::Fee {}) => to_binary(get_fee(deps)), + } +} + +fn get_mailbox(_deps: Deps) -> Result { + Ok(MailboxResponse { + mailbox: "unrestricted".to_string(), + }) +} + +fn quote_dispatch() -> Result { + Ok(QuoteDispatchResponse { fees: vec![] }) +} + +#[cfg(test)] +mod test { + use cosmwasm_schema::serde::{de::DeserializeOwned, Serialize}; + use cosmwasm_std::{ + from_json, + testing::{mock_dependencies, mock_env, mock_info, MockApi, MockQuerier, MockStorage}, + to_json_binary, Addr, HexBinary, OwnedDeps, Uint128, + }; + use hpl_interface::hook::{PostDispatchMsg, QuoteDispatchMsg}; + use hpl_ownable::get_owner; + use ibcx_test_utils::{addr, gen_bz}; + use rstest::{fixture, rstest}; + + use super::*; + + type TestDeps = OwnedDeps; + + fn query(deps: Deps, msg: S) -> T { + let req: QueryMsg = from_json(to_json_binary(&msg).unwrap()).unwrap(); + let res = crate::query(deps, mock_env(), req).unwrap(); + from_json(res).unwrap() + } + + #[fixture] + fn deps( + #[default(addr("deployer"))] sender: Addr, + #[default(addr("owner"))] owner: Addr, + #[default(false)] paused: bool, + ) -> TestDeps { + let mut deps = mock_dependencies(); + + instantiate( + deps.as_mut(), + mock_env(), + mock_info(sender.as_str(), &[]), + InstantiateMsg { + owner: owner.to_string(), + fee: Coin { + denom: "uusd".to_string(), + amount: Uint128::new(100), + }, + }, + ) + .unwrap(); + + deps + } + + #[rstest] + fn test_init(deps: TestDeps) { + assert_eq!( + "uusd", + query::<_, MailboxResponse>(deps.as_ref(), QueryMsg::Hook(HookQueryMsg::Mailbox {})) + .mailbox + ); + assert_eq!("owner", get_owner(deps.as_ref().storage).unwrap().as_str()); + } + + #[rstest] + #[case("owner")] + #[should_panic(expected = "hook paused")] + #[case("owner")] + fn test_post_dispatch(mut deps: TestDeps, #[case] sender: &str) { + execute( + deps.as_mut(), + mock_env(), + mock_info(sender, &[]), + ExecuteMsg::PostDispatch(PostDispatchMsg { + metadata: HexBinary::default(), + message: gen_bz(100), + }), + ) + .map_err(|e| e.to_string()) + .unwrap(); + } + + #[rstest] + fn test_query(deps: TestDeps) { + let res: MailboxResponse = query(deps.as_ref(), QueryMsg::Hook(HookQueryMsg::Mailbox {})); + assert_eq!("unrestricted", res.mailbox.as_str()); + + let res: QuoteDispatchResponse = query( + deps.as_ref(), + QueryMsg::Hook(HookQueryMsg::QuoteDispatch(QuoteDispatchMsg::default())), + ); + assert_eq!(res.fees, vec![]); + } +} diff --git a/packages/interface/src/hook/fee.rs b/packages/interface/src/hook/fee.rs new file mode 100644 index 00000000..b814b642 --- /dev/null +++ b/packages/interface/src/hook/fee.rs @@ -0,0 +1,83 @@ +use cosmwasm_schema::{cw_serde, QueryResponses}; +use cosmwasm_std::{Addr, Coin}; + +use crate::ownable::{OwnableMsg, OwnableQueryMsg}; + +use super::{HookQueryMsg, PostDispatchMsg}; + +pub const TREE_DEPTH: usize = 32; + +#[cw_serde] +pub struct InstantiateMsg { + pub owner: String, + pub fee: Coin, +} + +#[cw_serde] +pub enum ExecuteMsg { + Ownable(OwnableMsg), + PostDispatch(PostDispatchMsg), + FeeHook(FeeHookMsg), +} + +#[cw_serde] +pub enum FeeHookMsg { + SetFee { + fee: Coin, + }, + Claim { + recipient: Option + } +} + +#[cw_serde] +#[derive(QueryResponses)] +#[query_responses(nested)] +pub enum QueryMsg { + Ownable(OwnableQueryMsg), + Hook(HookQueryMsg), + FeeHook(FeeHookQueryMsg), +} + +#[cw_serde] +#[derive(QueryResponses)] +pub enum FeeHookQueryMsg { + #[returns(FeeResponse)] + Fee {} +} + +#[cw_serde] +pub struct FeeResponse { + pub fee: Coin, +} + +#[cfg(test)] +mod test { + use cosmwasm_std::HexBinary; + + use super::*; + use crate::{ + hook::{ExpectedHookQueryMsg, PostDispatchMsg, QuoteDispatchMsg}, + msg_checker, + }; + + #[test] + fn test_hook_interface() { + let _checked: ExecuteMsg = msg_checker( + PostDispatchMsg { + metadata: HexBinary::default(), + message: HexBinary::default(), + } + .wrap(), + ); + + let _checked: QueryMsg = msg_checker(ExpectedHookQueryMsg::Hook(HookQueryMsg::Mailbox {})); + let _checked: QueryMsg = msg_checker( + QuoteDispatchMsg { + metadata: HexBinary::default(), + message: HexBinary::default(), + } + .request(), + ); + } +} diff --git a/packages/interface/src/hook/mod.rs b/packages/interface/src/hook/mod.rs index 0c368b0b..1c76d998 100644 --- a/packages/interface/src/hook/mod.rs +++ b/packages/interface/src/hook/mod.rs @@ -4,6 +4,7 @@ pub mod pausable; pub mod routing; pub mod routing_custom; pub mod routing_fallback; +pub mod fee; use cosmwasm_schema::{cw_serde, QueryResponses}; use cosmwasm_std::{ From 6bd5efaa5c40713d77ceadb5672b7df42b6905a5 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 16 Jan 2024 13:01:06 -0500 Subject: [PATCH 11/17] Make merkle hook not ownable --- contracts/hooks/merkle/src/lib.rs | 14 +++----------- packages/interface/src/hook/merkle.rs | 5 ----- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/contracts/hooks/merkle/src/lib.rs b/contracts/hooks/merkle/src/lib.rs index e837cb83..88d6defe 100644 --- a/contracts/hooks/merkle/src/lib.rs +++ b/contracts/hooks/merkle/src/lib.rs @@ -59,18 +59,14 @@ pub fn instantiate( ) -> Result { cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; - let owner = deps.api.addr_validate(&msg.owner)?; let mailbox = deps.api.addr_validate(&msg.mailbox)?; - hpl_ownable::initialize(deps.storage, &owner)?; - MAILBOX.save(deps.storage, &mailbox)?; MESSAGE_TREE.save(deps.storage, &MerkleTree::default())?; Ok(Response::new().add_event( new_event("initialize") .add_attribute("sender", info.sender) - .add_attribute("owner", owner) .add_attribute("mailbox", mailbox), )) } @@ -78,12 +74,11 @@ pub fn instantiate( #[cfg_attr(not(feature = "library"), entry_point)] pub fn execute( deps: DepsMut, - env: Env, - info: MessageInfo, + _env: Env, + _info: MessageInfo, msg: ExecuteMsg, ) -> Result { match msg { - ExecuteMsg::Ownable(msg) => Ok(hpl_ownable::handle(deps, env, info, msg)?), ExecuteMsg::PostDispatch(PostDispatchMsg { message, .. }) => { let mailbox = MAILBOX.load(deps.storage)?; @@ -123,11 +118,10 @@ pub fn execute( } #[cfg_attr(not(feature = "library"), entry_point)] -pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result { +pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> Result { use MerkleHookQueryMsg::*; match msg { - QueryMsg::Ownable(msg) => Ok(hpl_ownable::handle_query(deps, env, msg)?), QueryMsg::Hook(msg) => match msg { HookQueryMsg::Mailbox {} => to_binary(get_mailbox(deps)), HookQueryMsg::QuoteDispatch(_) => to_binary(quote_dispatch()), @@ -228,7 +222,6 @@ mod test { #[fixture] fn deps( #[default(Addr::unchecked("deployer"))] sender: Addr, - #[default(Addr::unchecked("owner"))] owner: Addr, #[default(Addr::unchecked("mailbox"))] mailbox: Addr, ) -> TestDeps { let mut deps = mock_dependencies(); @@ -238,7 +231,6 @@ mod test { mock_env(), mock_info(sender.as_str(), &[]), InstantiateMsg { - owner: owner.to_string(), mailbox: mailbox.to_string(), }, ) diff --git a/packages/interface/src/hook/merkle.rs b/packages/interface/src/hook/merkle.rs index 007ac839..85d6180d 100644 --- a/packages/interface/src/hook/merkle.rs +++ b/packages/interface/src/hook/merkle.rs @@ -1,21 +1,17 @@ use cosmwasm_schema::{cw_serde, QueryResponses}; use cosmwasm_std::HexBinary; -use crate::ownable::{OwnableMsg, OwnableQueryMsg}; - use super::{HookQueryMsg, PostDispatchMsg}; pub const TREE_DEPTH: usize = 32; #[cw_serde] pub struct InstantiateMsg { - pub owner: String, pub mailbox: String, } #[cw_serde] pub enum ExecuteMsg { - Ownable(OwnableMsg), PostDispatch(PostDispatchMsg), } @@ -23,7 +19,6 @@ pub enum ExecuteMsg { #[derive(QueryResponses)] #[query_responses(nested)] pub enum QueryMsg { - Ownable(OwnableQueryMsg), Hook(HookQueryMsg), MerkleHook(MerkleHookQueryMsg), } From 9e335f45baa24a5b3b9c36ebb4f44a74f9f45c23 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 16 Jan 2024 16:22:06 -0500 Subject: [PATCH 12/17] Address pr comments --- contracts/hooks/fees/src/lib.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/contracts/hooks/fees/src/lib.rs b/contracts/hooks/fees/src/lib.rs index adc42c16..298c9911 100644 --- a/contracts/hooks/fees/src/lib.rs +++ b/contracts/hooks/fees/src/lib.rs @@ -129,7 +129,7 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result Ok(hpl_ownable::handle_query(deps, env, msg)?), QueryMsg::Hook(msg) => match msg { HookQueryMsg::Mailbox {} => to_binary(get_mailbox(deps)), - HookQueryMsg::QuoteDispatch(_) => to_binary(quote_dispatch()), + HookQueryMsg::QuoteDispatch(_) => to_binary(quote_dispatch(deps)), }, QueryMsg::FeeHook(FeeHookQueryMsg::Fee {}) => to_binary(get_fee(deps)), } @@ -141,8 +141,9 @@ fn get_mailbox(_deps: Deps) -> Result { }) } -fn quote_dispatch() -> Result { - Ok(QuoteDispatchResponse { fees: vec![] }) +fn quote_dispatch(deps: Deps) -> Result { + let fee = COIN_FEE.load(deps.storage)?; + Ok(QuoteDispatchResponse { fees: vec![fee] }) } #[cfg(test)] @@ -151,7 +152,7 @@ mod test { use cosmwasm_std::{ from_json, testing::{mock_dependencies, mock_env, mock_info, MockApi, MockQuerier, MockStorage}, - to_json_binary, Addr, HexBinary, OwnedDeps, Uint128, + to_json_binary, Addr, HexBinary, OwnedDeps, coin, }; use hpl_interface::hook::{PostDispatchMsg, QuoteDispatchMsg}; use hpl_ownable::get_owner; @@ -172,7 +173,7 @@ mod test { fn deps( #[default(addr("deployer"))] sender: Addr, #[default(addr("owner"))] owner: Addr, - #[default(false)] paused: bool, + #[default(coin(100, "uusd"))] fee: Coin, ) -> TestDeps { let mut deps = mock_dependencies(); @@ -182,10 +183,7 @@ mod test { mock_info(sender.as_str(), &[]), InstantiateMsg { owner: owner.to_string(), - fee: Coin { - denom: "uusd".to_string(), - amount: Uint128::new(100), - }, + fee }, ) .unwrap(); @@ -230,6 +228,6 @@ mod test { deps.as_ref(), QueryMsg::Hook(HookQueryMsg::QuoteDispatch(QuoteDispatchMsg::default())), ); - assert_eq!(res.fees, vec![]); + assert_eq!(res.fees, vec![coin(100, "uusd")]); } } From 1b0e3c9c02be35e74303c283992b524bf7a1437b Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 16 Jan 2024 16:27:04 -0500 Subject: [PATCH 13/17] Fix unit tests --- contracts/hooks/merkle/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/hooks/merkle/src/lib.rs b/contracts/hooks/merkle/src/lib.rs index 88d6defe..be13388a 100644 --- a/contracts/hooks/merkle/src/lib.rs +++ b/contracts/hooks/merkle/src/lib.rs @@ -208,7 +208,6 @@ mod test { use hpl_interface::{ build_test_executor, build_test_querier, core::mailbox, hook::QuoteDispatchMsg, }; - use hpl_ownable::get_owner; use ibcx_test_utils::hex; use rstest::{fixture, rstest}; @@ -241,7 +240,6 @@ mod test { #[rstest] fn test_init(deps: TestDeps) { - assert_eq!("owner", get_owner(deps.as_ref().storage).unwrap().as_str()); assert_eq!( "mailbox", MAILBOX.load(deps.as_ref().storage).unwrap().as_str() From d65a08ad93926c713ad4bffc2be34715dcce0ede Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 16 Jan 2024 23:02:28 -0500 Subject: [PATCH 14/17] Fix fee hook tests --- Cargo.toml | 1 + contracts/hooks/{fees => fee}/Cargo.toml | 0 contracts/hooks/{fees => fee}/src/lib.rs | 16 ++++++++-------- 3 files changed, 9 insertions(+), 8 deletions(-) rename contracts/hooks/{fees => fee}/Cargo.toml (100%) rename contracts/hooks/{fees => fee}/src/lib.rs (94%) diff --git a/Cargo.toml b/Cargo.toml index 643fe642..9ef3ffa3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ hpl-mailbox = { path = "./contracts/core/mailbox" } hpl-validator-announce = { path = "./contracts/core/va" } hpl-hook-merkle = { path = "./contracts/hooks/merkle" } +hpl-hook-fee = { path = "./contracts/hooks/fee" } hpl-hook-pausable = { path = "./contracts/hooks/pausable" } hpl-hook-routing = { path = "./contracts/hooks/routing" } hpl-hook-routing-custom = { path = "./contracts/hooks/routing-custom" } diff --git a/contracts/hooks/fees/Cargo.toml b/contracts/hooks/fee/Cargo.toml similarity index 100% rename from contracts/hooks/fees/Cargo.toml rename to contracts/hooks/fee/Cargo.toml diff --git a/contracts/hooks/fees/src/lib.rs b/contracts/hooks/fee/src/lib.rs similarity index 94% rename from contracts/hooks/fees/src/lib.rs rename to contracts/hooks/fee/src/lib.rs index 298c9911..939fd115 100644 --- a/contracts/hooks/fees/src/lib.rs +++ b/contracts/hooks/fee/src/lib.rs @@ -111,7 +111,8 @@ pub fn execute( ensure!( supplied.u128() >= fee.amount.u128(), - StdError::generic_err("insufficient fee") + // TODO: improve error + StdError::generic_err("insufficient funds") ); Ok(Response::new().add_event( @@ -195,21 +196,20 @@ mod test { fn test_init(deps: TestDeps) { assert_eq!( "uusd", - query::<_, MailboxResponse>(deps.as_ref(), QueryMsg::Hook(HookQueryMsg::Mailbox {})) - .mailbox + get_fee(deps.as_ref()).unwrap().fee.denom.as_str() ); assert_eq!("owner", get_owner(deps.as_ref().storage).unwrap().as_str()); } #[rstest] - #[case("owner")] - #[should_panic(expected = "hook paused")] - #[case("owner")] - fn test_post_dispatch(mut deps: TestDeps, #[case] sender: &str) { + #[case(&[coin(100, "uusd")])] + #[should_panic(expected = "Generic error: insufficient funds")] + #[case(&[coin(99, "uusd")])] + fn test_post_dispatch(mut deps: TestDeps, #[case] funds: &[Coin]) { execute( deps.as_mut(), mock_env(), - mock_info(sender, &[]), + mock_info("owner", funds), ExecuteMsg::PostDispatch(PostDispatchMsg { metadata: HexBinary::default(), message: gen_bz(100), From 189ee156cfbd731df72c7002b233b09f920963a4 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Tue, 16 Jan 2024 23:06:23 -0500 Subject: [PATCH 15/17] Make set fee only owner --- contracts/hooks/fee/src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/contracts/hooks/fee/src/lib.rs b/contracts/hooks/fee/src/lib.rs index 939fd115..241ba6f1 100644 --- a/contracts/hooks/fee/src/lib.rs +++ b/contracts/hooks/fee/src/lib.rs @@ -79,6 +79,9 @@ pub fn execute( ExecuteMsg::Ownable(msg) => Ok(hpl_ownable::handle(deps, env, info, msg)?), ExecuteMsg::FeeHook(msg) => match msg { FeeHookMsg::SetFee { fee } => { + let owner = hpl_ownable::get_owner(deps.storage)?; + ensure_eq!(owner, info.sender, StdError::generic_err("unauthorized")); + COIN_FEE.save(deps.storage, &fee)?; Ok(Response::new().add_event( @@ -230,4 +233,14 @@ mod test { ); assert_eq!(res.fees, vec![coin(100, "uusd")]); } + + #[rstest] + fn test_set_fee(_deps: TestDeps) { + unimplemented!("TODO"); + } + + #[rstest] + fn test_claim(_deps: TestDeps) { + unimplemented!("TODO"); + } } From 317ef61b8dae43f4daf533ec12c6db08ff4b05e8 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Fri, 19 Jan 2024 12:36:22 -0500 Subject: [PATCH 16/17] Implement remaining unit tests --- contracts/hooks/fee/src/lib.rs | 55 ++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/contracts/hooks/fee/src/lib.rs b/contracts/hooks/fee/src/lib.rs index 241ba6f1..743d6e79 100644 --- a/contracts/hooks/fee/src/lib.rs +++ b/contracts/hooks/fee/src/lib.rs @@ -1,8 +1,8 @@ #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - ensure, ensure_eq, BankMsg, Coin, CosmosMsg, Deps, DepsMut, Env, Event, - MessageInfo, QueryResponse, Response, StdError, + ensure, ensure_eq, BankMsg, Coin, CosmosMsg, Deps, DepsMut, Env, Event, MessageInfo, + QueryResponse, Response, StdError, }; use cw_storage_plus::Item; use hpl_interface::{ @@ -154,9 +154,9 @@ fn quote_dispatch(deps: Deps) -> Result { mod test { use cosmwasm_schema::serde::{de::DeserializeOwned, Serialize}; use cosmwasm_std::{ - from_json, + coin, from_json, testing::{mock_dependencies, mock_env, mock_info, MockApi, MockQuerier, MockStorage}, - to_json_binary, Addr, HexBinary, OwnedDeps, coin, + to_json_binary, Addr, HexBinary, OwnedDeps, }; use hpl_interface::hook::{PostDispatchMsg, QuoteDispatchMsg}; use hpl_ownable::get_owner; @@ -187,7 +187,7 @@ mod test { mock_info(sender.as_str(), &[]), InstantiateMsg { owner: owner.to_string(), - fee + fee, }, ) .unwrap(); @@ -197,10 +197,7 @@ mod test { #[rstest] fn test_init(deps: TestDeps) { - assert_eq!( - "uusd", - get_fee(deps.as_ref()).unwrap().fee.denom.as_str() - ); + assert_eq!("uusd", get_fee(deps.as_ref()).unwrap().fee.denom.as_str()); assert_eq!("owner", get_owner(deps.as_ref().storage).unwrap().as_str()); } @@ -235,12 +232,44 @@ mod test { } #[rstest] - fn test_set_fee(_deps: TestDeps) { - unimplemented!("TODO"); + #[case(addr("owner"), coin(200, "uusd"))] + #[should_panic(expected = "unauthorized")] + #[case(addr("deployer"), coin(200, "uusd"))] + fn test_set_fee(mut deps: TestDeps, #[case] sender: Addr, #[case] fee: Coin) { + execute( + deps.as_mut(), + mock_env(), + mock_info(sender.as_str(), &[]), + ExecuteMsg::FeeHook(FeeHookMsg::SetFee { fee: fee.clone() }), + ) + .map_err(|e| e.to_string()) + .unwrap(); + + assert_eq!(fee, get_fee(deps.as_ref()).unwrap().fee); } #[rstest] - fn test_claim(_deps: TestDeps) { - unimplemented!("TODO"); + #[case(addr("owner"), Some(addr("deployer")))] + #[case(addr("owner"), None)] + #[should_panic(expected = "unauthorized")] + #[case(addr("deployer"), None)] + fn test_claim(mut deps: TestDeps, #[case] sender: Addr, #[case] recipient: Option) { + let res = execute( + deps.as_mut(), + mock_env(), + mock_info(sender.as_str(), &[]), + ExecuteMsg::FeeHook(FeeHookMsg::Claim { recipient: recipient.clone() }), + ) + .map_err(|e| e.to_string()) + .unwrap(); + + assert_eq!( + CosmosMsg::Bank(BankMsg::Send { + to_address: recipient.unwrap_or_else(|| addr("owner")).into_string(), + amount: vec![], + }), + res.messages[0].msg + ); + println!("{:?}", res); } } From 5b82038eef7e409ed7be80c1e6c0e0be8812a9b3 Mon Sep 17 00:00:00 2001 From: Yorke Rhodes Date: Fri, 19 Jan 2024 12:43:00 -0500 Subject: [PATCH 17/17] Fix merkle integration test use --- integration-test/tests/contracts/cw/hook.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-test/tests/contracts/cw/hook.rs b/integration-test/tests/contracts/cw/hook.rs index c7d4cfb3..413c8cd2 100644 --- a/integration-test/tests/contracts/cw/hook.rs +++ b/integration-test/tests/contracts/cw/hook.rs @@ -99,7 +99,6 @@ impl Hook { .instantiate( codes.hook_merkle, &hook::merkle::InstantiateMsg { - owner: owner.address(), mailbox, }, Some(deployer.address().as_str()),