From df27ade8ababee0acbbcde1336bd05e122c1a8b3 Mon Sep 17 00:00:00 2001 From: Xavek Date: Mon, 17 Jun 2024 16:46:27 +0545 Subject: [PATCH 01/13] add domain routing hook --- .../contracts/hooks/domain_routing_hook.cairo | 108 ++++++++++++++++++ contracts/src/interfaces.cairo | 12 ++ contracts/src/lib.cairo | 1 + 3 files changed, 121 insertions(+) create mode 100644 contracts/src/contracts/hooks/domain_routing_hook.cairo diff --git a/contracts/src/contracts/hooks/domain_routing_hook.cairo b/contracts/src/contracts/hooks/domain_routing_hook.cairo new file mode 100644 index 00000000..c544885c --- /dev/null +++ b/contracts/src/contracts/hooks/domain_routing_hook.cairo @@ -0,0 +1,108 @@ +#[starknet::contract] +pub mod domain_routing_hook { + use alexandria_bytes::{Bytes, BytesTrait, BytesStore}; + use hyperlane_starknet::contracts::client::mailboxclient_component::{ + MailboxclientComponent, MailboxclientComponent::MailboxClientInternalImpl + }; + use hyperlane_starknet::contracts::client::{mailboxclient}; + use hyperlane_starknet::contracts::libs::message::Message; + use hyperlane_starknet::interfaces::{ + IPostDispatchHook, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, + DomainRoutingHookConfig, IDomainRoutingHook, Types + }; + use openzeppelin::access::ownable::OwnableComponent; + use openzeppelin::upgrades::{interface::IUpgradeable, upgradeable::UpgradeableComponent}; + use starknet::{ContractAddress, contract_address_const}; + component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); + component!(path: MailboxclientComponent, storage: mailboxclient, event: MailboxclientEvent); + component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent); + #[abi(embed_v0)] + impl OwnableImpl = OwnableComponent::OwnableImpl; + impl OwnableInternalImpl = OwnableComponent::InternalImpl; + impl UpgradeableInternalImpl = UpgradeableComponent::InternalImpl; + + + #[storage] + struct Storage { + hooks: LegacyMap, + #[substorage(v0)] + mailboxclient: MailboxclientComponent::Storage, + #[substorage(v0)] + ownable: OwnableComponent::Storage, + #[substorage(v0)] + upgradeable: UpgradeableComponent::Storage, + } + + + mod Errors { + pub const INVALID_DESTINATION: felt252 = 'Destination has no hooks'; + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + OwnableEvent: OwnableComponent::Event, + #[flat] + UpgradeableEvent: UpgradeableComponent::Event, + #[flat] + MailboxclientEvent: MailboxclientComponent::Event, + } + + #[constructor] + fn constructor(ref self: ContractState, _mailbox: ContractAddress, _owner: ContractAddress) { + self.mailboxclient.initialize(_mailbox); + self.ownable.initializer(_owner); + } + + #[abi(embed_v0)] + impl IPostDispatchHookImpl of IPostDispatchHook { + fn hook_type(self: @ContractState) -> Types { + Types::ROUTING(()) + } + + fn supports_metadata(self: @ContractState, _metadata: Bytes) -> bool { + true + } + + fn post_dispatch(ref self: ContractState, _metadata: Bytes, _message: Message) { + self._get_configured_hook(_message.clone()).post_dispatch(_metadata, _message); + } + + fn quote_dispatch(ref self: ContractState, _metadata: Bytes, _message: Message) -> u256 { + self._get_configured_hook(_message.clone()).quote_dispatch(_metadata, _message) + } + } + + #[abi(embed_v0)] + impl IDomainRoutingHookImpl of IDomainRoutingHook { + fn setHook(ref self: ContractState, _destination: u32, _hook: ContractAddress) { + self.ownable.assert_only_owner(); + self.hooks.write(_destination, IPostDispatchHookDispatcher { contract_address: _hook }); + } + fn setHooks(ref self: ContractState, configs: Array) { + self.ownable.assert_only_owner(); + let mut configs_span = configs.span(); + loop { + match configs_span.pop_front() { + Option::Some(config) => { self.setHook(*config.destination, *config.hook) }, + Option::None(_) => { break; }, + }; + }; + } + } + + #[generate_trait] + impl InternalImpl of InternalTrait { + fn _get_configured_hook( + ref self: ContractState, _message: Message + ) -> IPostDispatchHookDispatcher { + let dispatcher_instance = self.hooks.read(_message.destination); + assert( + dispatcher_instance.contract_address != contract_address_const::<0>(), + Errors::INVALID_DESTINATION + ); + dispatcher_instance + } + } +} diff --git a/contracts/src/interfaces.cairo b/contracts/src/interfaces.cairo index 0b3dfac3..a8c95556 100644 --- a/contracts/src/interfaces.cairo +++ b/contracts/src/interfaces.cairo @@ -346,3 +346,15 @@ pub trait IProtocolFee { pub trait IRoutingIsm { fn route(self: @TContractState, _message: Message) -> ContractAddress; } + +#[derive(Drop, Serde)] +pub struct DomainRoutingHookConfig { + pub destination: u32, + pub hook: ContractAddress +} + +#[starknet::interface] +pub trait IDomainRoutingHook { + fn setHook(ref self: TContractState, _destination: u32, _hook: ContractAddress); + fn setHooks(ref self: TContractState, configs: Array); +} diff --git a/contracts/src/lib.cairo b/contracts/src/lib.cairo index fde580f0..da07e9f9 100644 --- a/contracts/src/lib.cairo +++ b/contracts/src/lib.cairo @@ -12,6 +12,7 @@ mod contracts { } } pub mod hooks { + pub mod domain_routing_hook; pub mod merkle_tree_hook; pub mod protocol_fee; pub mod libs { From 3aea33b711006b88ec8a97e4018a2f85709af1c8 Mon Sep 17 00:00:00 2001 From: Xavek Date: Tue, 18 Jun 2024 08:41:32 +0545 Subject: [PATCH 02/13] address rewiew comments minor --- contracts/src/contracts/hooks/domain_routing_hook.cairo | 8 ++++---- contracts/src/interfaces.cairo | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/src/contracts/hooks/domain_routing_hook.cairo b/contracts/src/contracts/hooks/domain_routing_hook.cairo index c544885c..3affd58b 100644 --- a/contracts/src/contracts/hooks/domain_routing_hook.cairo +++ b/contracts/src/contracts/hooks/domain_routing_hook.cairo @@ -76,16 +76,16 @@ pub mod domain_routing_hook { #[abi(embed_v0)] impl IDomainRoutingHookImpl of IDomainRoutingHook { - fn setHook(ref self: ContractState, _destination: u32, _hook: ContractAddress) { + fn set_hook(ref self: ContractState, _destination: u32, _hook: ContractAddress) { self.ownable.assert_only_owner(); self.hooks.write(_destination, IPostDispatchHookDispatcher { contract_address: _hook }); } - fn setHooks(ref self: ContractState, configs: Array) { + fn set_hooks(ref self: ContractState, configs: Array) { self.ownable.assert_only_owner(); let mut configs_span = configs.span(); loop { match configs_span.pop_front() { - Option::Some(config) => { self.setHook(*config.destination, *config.hook) }, + Option::Some(config) => { self.set_hook(*config.destination, *config.hook) }, Option::None(_) => { break; }, }; }; @@ -95,7 +95,7 @@ pub mod domain_routing_hook { #[generate_trait] impl InternalImpl of InternalTrait { fn _get_configured_hook( - ref self: ContractState, _message: Message + self: @ContractState, _message: Message ) -> IPostDispatchHookDispatcher { let dispatcher_instance = self.hooks.read(_message.destination); assert( diff --git a/contracts/src/interfaces.cairo b/contracts/src/interfaces.cairo index a8c95556..2df526dc 100644 --- a/contracts/src/interfaces.cairo +++ b/contracts/src/interfaces.cairo @@ -355,6 +355,6 @@ pub struct DomainRoutingHookConfig { #[starknet::interface] pub trait IDomainRoutingHook { - fn setHook(ref self: TContractState, _destination: u32, _hook: ContractAddress); - fn setHooks(ref self: TContractState, configs: Array); + fn set_hook(ref self: TContractState, _destination: u32, _hook: ContractAddress); + fn set_hooks(ref self: TContractState, configs: Array); } From 7587489f018ba4b53c1ce05b1888364837510ef9 Mon Sep 17 00:00:00 2001 From: Xavek Date: Thu, 20 Jun 2024 17:25:53 +0545 Subject: [PATCH 03/13] add test for domain routing hook --- contracts/src/lib.cairo | 1 + .../hooks/test_domain_routing_hook.cairo | 145 ++++++++++++++++++ contracts/src/tests/setup.cairo | 17 +- 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 contracts/src/tests/hooks/test_domain_routing_hook.cairo diff --git a/contracts/src/lib.cairo b/contracts/src/lib.cairo index da07e9f9..ebb4da5d 100644 --- a/contracts/src/lib.cairo +++ b/contracts/src/lib.cairo @@ -65,6 +65,7 @@ mod tests { pub mod test_messageid_multisig; } pub mod hooks { + pub mod test_domain_routing_hook; pub mod test_merkle_tree_hook; pub mod test_protocol_fee; } diff --git a/contracts/src/tests/hooks/test_domain_routing_hook.cairo b/contracts/src/tests/hooks/test_domain_routing_hook.cairo new file mode 100644 index 00000000..a0434a5b --- /dev/null +++ b/contracts/src/tests/hooks/test_domain_routing_hook.cairo @@ -0,0 +1,145 @@ +use alexandria_bytes::{Bytes, BytesTrait, BytesStore}; +use hyperlane_starknet::contracts::libs::message::{Message, MessageTrait, HYPERLANE_VERSION}; +use hyperlane_starknet::interfaces::{ + Types, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, + IDomainRoutingHookDispatcher, IDomainRoutingHookDispatcherTrait, DomainRoutingHookConfig +}; +use hyperlane_starknet::tests::setup::{setup_domain_routing_hook, setup_mock_hook, OWNER}; +use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; + +use snforge_std::{start_prank, CheatTarget}; +use starknet::{get_caller_address, contract_address_const, ContractAddress}; + + +#[test] +fn test_domain_routing_hook_type() { + let (routing_hook_addrs, _) = setup_domain_routing_hook(); + assert_eq!(routing_hook_addrs.hook_type(), Types::ROUTING(())); +} + +#[test] +fn test_supports_metadata_for_domain_routing_hook() { + let (routing_hook_addrs, _) = setup_domain_routing_hook(); + + let metadata = BytesTrait::new_empty(); + assert_eq!(routing_hook_addrs.supports_metadata(metadata), true); +} + +#[test] +fn test_domain_rounting_set_hook() { + let (_, set_routing_hook_addrs) = setup_domain_routing_hook(); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + let destination: u32 = 12; + let hook: ContractAddress = contract_address_const::<1>(); + set_routing_hook_addrs.set_hook(destination, hook); +} + +#[test] +#[should_panic(expected: ('Caller is not the owner',))] +fn test_set_hook_fails_if_not_owner() { + let (_, set_routing_hook_addrs) = setup_domain_routing_hook(); + let destination: u32 = 12; + let hook: ContractAddress = contract_address_const::<1>(); + set_routing_hook_addrs.set_hook(destination, hook); +} + +#[test] +fn test_domain_rounting_set_hook_array() { + let (_, set_routing_hook_addrs) = setup_domain_routing_hook(); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + let mut hook_config_arr = ArrayTrait::::new(); + hook_config_arr + .append(DomainRoutingHookConfig { destination: 1, hook: contract_address_const::<2>() }); + hook_config_arr + .append(DomainRoutingHookConfig { destination: 2, hook: contract_address_const::<3>() }); + hook_config_arr + .append(DomainRoutingHookConfig { destination: 3, hook: contract_address_const::<4>() }); + set_routing_hook_addrs.set_hooks(hook_config_arr); +} + + +#[test] +#[should_panic(expected: ('Caller is not the owner',))] +fn test_set_hook_array_fails_if_not_owner() { + let (_, set_routing_hook_addrs) = setup_domain_routing_hook(); + let mut hook_config_arr = ArrayTrait::::new(); + hook_config_arr + .append(DomainRoutingHookConfig { destination: 1, hook: contract_address_const::<2>() }); + hook_config_arr + .append(DomainRoutingHookConfig { destination: 2, hook: contract_address_const::<3>() }); + set_routing_hook_addrs.set_hooks(hook_config_arr); +} + + +#[test] +#[should_panic(expected: ('Destination has no hooks',))] +fn hook_not_set_for_destination_should_fail() { + let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + let destination: u32 = 12; + let hook: ContractAddress = contract_address_const::<1>(); + set_routing_hook_addrs.set_hook(destination, hook); + + let message = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: contract_address_const::<0>(), + destination: destination - 1, + recipient: contract_address_const::<0>(), + body: BytesTrait::new_empty(), + }; + let metadata = BytesTrait::new_empty(); + routing_hook_addrs.post_dispatch(metadata, message); +} + +// Note: Test fails with msg('Result::unwrap failed') +#[ignore] +#[test] +fn hook_set_for_destination_post_dispatch() { + let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + let destination: u32 = 18; + let hook: ContractAddress = setup_mock_hook().contract_address; + set_routing_hook_addrs.set_hook(destination, hook); + + let message = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: contract_address_const::<0>(), + destination: destination, + recipient: contract_address_const::<0>(), + body: BytesTrait::new_empty(), + }; + let metadata = BytesTrait::new_empty(); + routing_hook_addrs.post_dispatch(metadata, message); +} + +// Note: Test fails with msg('Result::unwrap failed') +#[ignore] +#[test] +fn hook_set_for_destination_quote_dispatch() { + let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + let destination: u32 = 18; + let hook: ContractAddress = setup_mock_hook().contract_address; + set_routing_hook_addrs.set_hook(destination, hook); + + let message = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: contract_address_const::<0>(), + destination: destination, + recipient: contract_address_const::<0>(), + body: BytesTrait::new_empty(), + }; + let metadata = BytesTrait::new_empty(); + routing_hook_addrs.quote_dispatch(metadata, message); +} diff --git a/contracts/src/tests/setup.cairo b/contracts/src/tests/setup.cairo index c9fb34a9..0f673a4d 100644 --- a/contracts/src/tests/setup.cairo +++ b/contracts/src/tests/setup.cairo @@ -13,7 +13,8 @@ use hyperlane_starknet::interfaces::{ IPostDispatchHookDispatcherTrait, IProtocolFeeDispatcherTrait, IMockValidatorAnnounceDispatcher, ISpecifiesInterchainSecurityModuleDispatcher, ISpecifiesInterchainSecurityModuleDispatcherTrait, IRoutingIsmDispatcher, IRoutingIsmDispatcherTrait, IDomainRoutingIsmDispatcher, - IDomainRoutingIsmDispatcherTrait + IDomainRoutingIsmDispatcherTrait, IDomainRoutingHookDispatcher, + IDomainRoutingHookDispatcherTrait }; use openzeppelin::account::utils::signature::EthSignature; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; @@ -439,3 +440,17 @@ pub fn setup_protocol_fee() -> ( IPostDispatchHookDispatcher { contract_address: protocol_fee_addr } ) } + +pub fn setup_domain_routing_hook() -> (IPostDispatchHookDispatcher, IDomainRoutingHookDispatcher) { + let (mailbox, _, _, _) = setup(); + + let domain_routing_hook_class = declare("domain_routing_hook").unwrap(); + let (domain_routing_hook_addrs, _) = domain_routing_hook_class + .deploy(@array![mailbox.contract_address.into(), OWNER().into()]) + .unwrap(); + ( + IPostDispatchHookDispatcher { contract_address: domain_routing_hook_addrs }, + IDomainRoutingHookDispatcher { contract_address: domain_routing_hook_addrs } + ) +} + From 304f17e7d9998e9af0000f7e12a613858341c5a8 Mon Sep 17 00:00:00 2001 From: Xavek Date: Wed, 26 Jun 2024 14:01:47 +0545 Subject: [PATCH 04/13] add fee impl for domain routing hook --- .../contracts/hooks/domain_routing_hook.cairo | 41 ++++++++++++++++--- contracts/src/interfaces.cairo | 13 ++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/contracts/src/contracts/hooks/domain_routing_hook.cairo b/contracts/src/contracts/hooks/domain_routing_hook.cairo index 3affd58b..39032cbd 100644 --- a/contracts/src/contracts/hooks/domain_routing_hook.cairo +++ b/contracts/src/contracts/hooks/domain_routing_hook.cairo @@ -8,11 +8,12 @@ pub mod domain_routing_hook { use hyperlane_starknet::contracts::libs::message::Message; use hyperlane_starknet::interfaces::{ IPostDispatchHook, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, - DomainRoutingHookConfig, IDomainRoutingHook, Types + DomainRoutingHookConfig, IDomainRoutingHook, Types, IPostDispatchHookForDomainRoutingHook, }; use openzeppelin::access::ownable::OwnableComponent; + use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; use openzeppelin::upgrades::{interface::IUpgradeable, upgradeable::UpgradeableComponent}; - use starknet::{ContractAddress, contract_address_const}; + use starknet::{ContractAddress, contract_address_const, get_caller_address}; component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); component!(path: MailboxclientComponent, storage: mailboxclient, event: MailboxclientEvent); component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent); @@ -25,6 +26,7 @@ pub mod domain_routing_hook { #[storage] struct Storage { hooks: LegacyMap, + fee_token: ContractAddress, #[substorage(v0)] mailboxclient: MailboxclientComponent::Storage, #[substorage(v0)] @@ -36,6 +38,9 @@ pub mod domain_routing_hook { mod Errors { pub const INVALID_DESTINATION: felt252 = 'Destination has no hooks'; + pub const INSUFFICIENT_BALANCE: felt252 = 'Insufficient balance'; + pub const FEE_AMOUNT_TRANSFER_FAILED: felt252 = 'Hook fee transfer failed'; + pub const ZERO_FEE: felt252 = 'Zero fee amount'; } #[event] @@ -50,13 +55,21 @@ pub mod domain_routing_hook { } #[constructor] - fn constructor(ref self: ContractState, _mailbox: ContractAddress, _owner: ContractAddress) { + fn constructor( + ref self: ContractState, + _mailbox: ContractAddress, + _owner: ContractAddress, + _fee_token_address: ContractAddress + ) { self.mailboxclient.initialize(_mailbox); self.ownable.initializer(_owner); + self.fee_token.write(_fee_token_address); } #[abi(embed_v0)] - impl IPostDispatchHookImpl of IPostDispatchHook { + impl IPostDispatchHookForDomainRoutingHookImpl of IPostDispatchHookForDomainRoutingHook< + ContractState + > { fn hook_type(self: @ContractState) -> Types { Types::ROUTING(()) } @@ -65,7 +78,15 @@ pub mod domain_routing_hook { true } - fn post_dispatch(ref self: ContractState, _metadata: Bytes, _message: Message) { + fn post_dispatch( + ref self: ContractState, _metadata: Bytes, _message: Message, _fee_amount: u256 + ) { + assert(_fee_amount > 0, Errors::ZERO_FEE); + let caller = get_caller_address(); + let configured_hook_address: ContractAddress = self + ._get_configured_hook(_message.clone()) + .contract_address; + self._transfer_routing_fee_to_hook(caller, configured_hook_address, _fee_amount); self._get_configured_hook(_message.clone()).post_dispatch(_metadata, _message); } @@ -104,5 +125,15 @@ pub mod domain_routing_hook { ); dispatcher_instance } + + fn _transfer_routing_fee_to_hook( + ref self: ContractState, from: ContractAddress, to: ContractAddress, amount: u256 + ) { + let token_dispatcher = IERC20Dispatcher { contract_address: self.fee_token.read() }; + let user_balance = token_dispatcher.balance_of(from); + assert(user_balance >= amount, Errors::INSUFFICIENT_BALANCE); + let transfer_flag: bool = token_dispatcher.transfer_from(from, to, amount); + assert(transfer_flag, Errors::FEE_AMOUNT_TRANSFER_FAILED); + } } } diff --git a/contracts/src/interfaces.cairo b/contracts/src/interfaces.cairo index 2df526dc..1ed22666 100644 --- a/contracts/src/interfaces.cairo +++ b/contracts/src/interfaces.cairo @@ -358,3 +358,16 @@ pub trait IDomainRoutingHook { fn set_hook(ref self: TContractState, _destination: u32, _hook: ContractAddress); fn set_hooks(ref self: TContractState, configs: Array); } + +#[starknet::interface] +pub trait IPostDispatchHookForDomainRoutingHook { + fn hook_type(self: @TContractState) -> Types; + + fn supports_metadata(self: @TContractState, _metadata: Bytes) -> bool; + + fn post_dispatch( + ref self: TContractState, _metadata: Bytes, _message: Message, _fee_amount: u256 + ); + + fn quote_dispatch(ref self: TContractState, _metadata: Bytes, _message: Message) -> u256; +} From 29d1e44694622a3c04f1de11edb805818a4c831b Mon Sep 17 00:00:00 2001 From: Xavek Date: Wed, 26 Jun 2024 14:09:19 +0545 Subject: [PATCH 05/13] routing hook fee transfer minor update --- contracts/src/contracts/hooks/domain_routing_hook.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/src/contracts/hooks/domain_routing_hook.cairo b/contracts/src/contracts/hooks/domain_routing_hook.cairo index 39032cbd..ef71e15f 100644 --- a/contracts/src/contracts/hooks/domain_routing_hook.cairo +++ b/contracts/src/contracts/hooks/domain_routing_hook.cairo @@ -133,7 +133,7 @@ pub mod domain_routing_hook { let user_balance = token_dispatcher.balance_of(from); assert(user_balance >= amount, Errors::INSUFFICIENT_BALANCE); let transfer_flag: bool = token_dispatcher.transfer_from(from, to, amount); - assert(transfer_flag, Errors::FEE_AMOUNT_TRANSFER_FAILED); + assert(transfer_flag == false, Errors::FEE_AMOUNT_TRANSFER_FAILED); } } } From bafa8d294559f342fada09539785e36814b7a831 Mon Sep 17 00:00:00 2001 From: Xavek Date: Fri, 28 Jun 2024 10:16:20 +0545 Subject: [PATCH 06/13] add fee impl erc20 allow check --- .../src/contracts/hooks/domain_routing_hook.cairo | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/contracts/src/contracts/hooks/domain_routing_hook.cairo b/contracts/src/contracts/hooks/domain_routing_hook.cairo index ef71e15f..fc64f1d7 100644 --- a/contracts/src/contracts/hooks/domain_routing_hook.cairo +++ b/contracts/src/contracts/hooks/domain_routing_hook.cairo @@ -13,7 +13,9 @@ pub mod domain_routing_hook { use openzeppelin::access::ownable::OwnableComponent; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; use openzeppelin::upgrades::{interface::IUpgradeable, upgradeable::UpgradeableComponent}; - use starknet::{ContractAddress, contract_address_const, get_caller_address}; + use starknet::{ + ContractAddress, contract_address_const, get_caller_address, get_contract_address + }; component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); component!(path: MailboxclientComponent, storage: mailboxclient, event: MailboxclientEvent); component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent); @@ -41,6 +43,7 @@ pub mod domain_routing_hook { pub const INSUFFICIENT_BALANCE: felt252 = 'Insufficient balance'; pub const FEE_AMOUNT_TRANSFER_FAILED: felt252 = 'Hook fee transfer failed'; pub const ZERO_FEE: felt252 = 'Zero fee amount'; + pub const INSUFFICIENT_ALLOWANCE: felt252 = 'Insufficient allowance'; } #[event] @@ -132,6 +135,11 @@ pub mod domain_routing_hook { let token_dispatcher = IERC20Dispatcher { contract_address: self.fee_token.read() }; let user_balance = token_dispatcher.balance_of(from); assert(user_balance >= amount, Errors::INSUFFICIENT_BALANCE); + let contract_address = get_contract_address(); + assert( + token_dispatcher.allowance(from, contract_address) >= amount, + Errors::INSUFFICIENT_ALLOWANCE + ); let transfer_flag: bool = token_dispatcher.transfer_from(from, to, amount); assert(transfer_flag == false, Errors::FEE_AMOUNT_TRANSFER_FAILED); } From 4327d75fd02b01a72d30bd03e9de169dac14d957 Mon Sep 17 00:00:00 2001 From: Xavek Date: Mon, 1 Jul 2024 22:19:44 +0545 Subject: [PATCH 07/13] address rewiew and test file update --- .../contracts/hooks/domain_routing_hook.cairo | 14 ++++----- contracts/src/interfaces.cairo | 13 -------- .../hooks/test_domain_routing_hook.cairo | 31 +++++++++++++------ contracts/src/tests/setup.cairo | 11 +++---- 4 files changed, 33 insertions(+), 36 deletions(-) diff --git a/contracts/src/contracts/hooks/domain_routing_hook.cairo b/contracts/src/contracts/hooks/domain_routing_hook.cairo index fc64f1d7..2f218ac5 100644 --- a/contracts/src/contracts/hooks/domain_routing_hook.cairo +++ b/contracts/src/contracts/hooks/domain_routing_hook.cairo @@ -8,7 +8,7 @@ pub mod domain_routing_hook { use hyperlane_starknet::contracts::libs::message::Message; use hyperlane_starknet::interfaces::{ IPostDispatchHook, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, - DomainRoutingHookConfig, IDomainRoutingHook, Types, IPostDispatchHookForDomainRoutingHook, + DomainRoutingHookConfig, IDomainRoutingHook, Types }; use openzeppelin::access::ownable::OwnableComponent; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; @@ -70,9 +70,7 @@ pub mod domain_routing_hook { } #[abi(embed_v0)] - impl IPostDispatchHookForDomainRoutingHookImpl of IPostDispatchHookForDomainRoutingHook< - ContractState - > { + impl IPostDispatchHookImpl of IPostDispatchHook { fn hook_type(self: @ContractState) -> Types { Types::ROUTING(()) } @@ -90,7 +88,9 @@ pub mod domain_routing_hook { ._get_configured_hook(_message.clone()) .contract_address; self._transfer_routing_fee_to_hook(caller, configured_hook_address, _fee_amount); - self._get_configured_hook(_message.clone()).post_dispatch(_metadata, _message); + self + ._get_configured_hook(_message.clone()) + .post_dispatch(_metadata, _message, _fee_amount); } fn quote_dispatch(ref self: ContractState, _metadata: Bytes, _message: Message) -> u256 { @@ -135,9 +135,9 @@ pub mod domain_routing_hook { let token_dispatcher = IERC20Dispatcher { contract_address: self.fee_token.read() }; let user_balance = token_dispatcher.balance_of(from); assert(user_balance >= amount, Errors::INSUFFICIENT_BALANCE); - let contract_address = get_contract_address(); + let this_contract_address = get_contract_address(); assert( - token_dispatcher.allowance(from, contract_address) >= amount, + token_dispatcher.allowance(from, this_contract_address) >= amount, Errors::INSUFFICIENT_ALLOWANCE ); let transfer_flag: bool = token_dispatcher.transfer_from(from, to, amount); diff --git a/contracts/src/interfaces.cairo b/contracts/src/interfaces.cairo index 7a38b051..8b7b1980 100644 --- a/contracts/src/interfaces.cairo +++ b/contracts/src/interfaces.cairo @@ -334,16 +334,3 @@ pub trait IDomainRoutingHook { fn set_hook(ref self: TContractState, _destination: u32, _hook: ContractAddress); fn set_hooks(ref self: TContractState, configs: Array); } - -#[starknet::interface] -pub trait IPostDispatchHookForDomainRoutingHook { - fn hook_type(self: @TContractState) -> Types; - - fn supports_metadata(self: @TContractState, _metadata: Bytes) -> bool; - - fn post_dispatch( - ref self: TContractState, _metadata: Bytes, _message: Message, _fee_amount: u256 - ); - - fn quote_dispatch(ref self: TContractState, _metadata: Bytes, _message: Message) -> u256; -} diff --git a/contracts/src/tests/hooks/test_domain_routing_hook.cairo b/contracts/src/tests/hooks/test_domain_routing_hook.cairo index a0434a5b..b42d0435 100644 --- a/contracts/src/tests/hooks/test_domain_routing_hook.cairo +++ b/contracts/src/tests/hooks/test_domain_routing_hook.cairo @@ -2,12 +2,13 @@ use alexandria_bytes::{Bytes, BytesTrait, BytesStore}; use hyperlane_starknet::contracts::libs::message::{Message, MessageTrait, HYPERLANE_VERSION}; use hyperlane_starknet::interfaces::{ Types, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, - IDomainRoutingHookDispatcher, IDomainRoutingHookDispatcherTrait, DomainRoutingHookConfig + IDomainRoutingHookDispatcher, IDomainRoutingHookDispatcherTrait, DomainRoutingHookConfig, + ETH_ADDRESS }; -use hyperlane_starknet::tests::setup::{setup_domain_routing_hook, setup_mock_hook, OWNER}; +use hyperlane_starknet::tests::setup::{setup_domain_routing_hook, OWNER, PROTOCOL_FEE, NEW_OWNER}; use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; - -use snforge_std::{start_prank, CheatTarget}; +use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; +use snforge_std::{start_prank, CheatTarget, stop_prank}; use starknet::{get_caller_address, contract_address_const, ContractAddress}; @@ -93,18 +94,19 @@ fn hook_not_set_for_destination_should_fail() { body: BytesTrait::new_empty(), }; let metadata = BytesTrait::new_empty(); - routing_hook_addrs.post_dispatch(metadata, message); + let protocol_fee = 12_u256; + routing_hook_addrs.post_dispatch(metadata, message, protocol_fee); } -// Note: Test fails with msg('Result::unwrap failed') #[ignore] #[test] fn hook_set_for_destination_post_dispatch() { let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); + let fee_token_instance = IERC20Dispatcher { contract_address: ETH_ADDRESS() }; let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); let destination: u32 = 18; - let hook: ContractAddress = setup_mock_hook().contract_address; + let hook: ContractAddress = contract_address_const::<12>(); set_routing_hook_addrs.set_hook(destination, hook); let message = Message { @@ -117,10 +119,19 @@ fn hook_set_for_destination_post_dispatch() { body: BytesTrait::new_empty(), }; let metadata = BytesTrait::new_empty(); - routing_hook_addrs.post_dispatch(metadata, message); + stop_prank(CheatTarget::One(ownable.contract_address)); + + let erc20Ownable = IOwnableDispatcher { contract_address: ETH_ADDRESS() }; + start_prank(CheatTarget::One(erc20Ownable.contract_address), NEW_OWNER()); + fee_token_instance.transfer(ownable.contract_address, PROTOCOL_FEE); + stop_prank(CheatTarget::One(erc20Ownable.contract_address)); + + assert_eq!(fee_token_instance.balance_of(ownable.contract_address), PROTOCOL_FEE); + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + fee_token_instance.approve(routing_hook_addrs.contract_address, PROTOCOL_FEE); + routing_hook_addrs.post_dispatch(metadata, message, PROTOCOL_FEE); } -// Note: Test fails with msg('Result::unwrap failed') #[ignore] #[test] fn hook_set_for_destination_quote_dispatch() { @@ -128,7 +139,7 @@ fn hook_set_for_destination_quote_dispatch() { let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); let destination: u32 = 18; - let hook: ContractAddress = setup_mock_hook().contract_address; + let hook: ContractAddress = contract_address_const::<1212>(); set_routing_hook_addrs.set_hook(destination, hook); let message = Message { diff --git a/contracts/src/tests/setup.cairo b/contracts/src/tests/setup.cairo index 0f404792..c953c2ce 100644 --- a/contracts/src/tests/setup.cairo +++ b/contracts/src/tests/setup.cairo @@ -12,9 +12,8 @@ use hyperlane_starknet::interfaces::{ ISpecifiesInterchainSecurityModuleDispatcher, ISpecifiesInterchainSecurityModuleDispatcherTrait, IRoutingIsmDispatcher, IRoutingIsmDispatcherTrait, IDomainRoutingIsmDispatcher, IDomainRoutingIsmDispatcherTrait, IDomainRoutingHookDispatcher, - IDomainRoutingHookDispatcherTrait - IDomainRoutingIsmDispatcherTrait, IPausableIsmDispatcher, IPausableIsmDispatcherTrait, - ETH_ADDRESS + IDomainRoutingHookDispatcherTrait, IPausableIsmDispatcher, IPausableIsmDispatcherTrait, + ETH_ADDRESS, }; use openzeppelin::account::utils::signature::EthSignature; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; @@ -521,11 +520,11 @@ pub fn setup_protocol_fee() -> (IProtocolFeeDispatcher, IPostDispatchHookDispatc } pub fn setup_domain_routing_hook() -> (IPostDispatchHookDispatcher, IDomainRoutingHookDispatcher) { - let (mailbox, _, _, _) = setup(); - + let (mailbox, _, _, _) = setup_mailbox(MAILBOX(), Option::None, Option::None); let domain_routing_hook_class = declare("domain_routing_hook").unwrap(); + let (domain_routing_hook_addrs, _) = domain_routing_hook_class - .deploy(@array![mailbox.contract_address.into(), OWNER().into()]) + .deploy(@array![mailbox.contract_address.into(), OWNER().into(), ETH_ADDRESS().into()]) .unwrap(); ( IPostDispatchHookDispatcher { contract_address: domain_routing_hook_addrs }, From 09631dedfd9199622fa1959e111ced1dbfa124e3 Mon Sep 17 00:00:00 2001 From: JordyRo1 Date: Wed, 11 Dec 2024 10:13:49 +0100 Subject: [PATCH 08/13] fix: main merge --- .../src/hooks/domain_routing_hook.cairo | 10 ++--- cairo/crates/contracts/src/lib.cairo | 1 + .../hooks/test_domain_routing_hook.cairo | 38 +++++++++++-------- cairo/crates/contracts/tests/lib.cairo | 1 + cairo/crates/contracts/tests/setup.cairo | 9 +++-- 5 files changed, 34 insertions(+), 25 deletions(-) diff --git a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo index 2f218ac5..7eb1447c 100644 --- a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo +++ b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo @@ -1,12 +1,12 @@ #[starknet::contract] pub mod domain_routing_hook { use alexandria_bytes::{Bytes, BytesTrait, BytesStore}; - use hyperlane_starknet::contracts::client::mailboxclient_component::{ + use contracts::client::mailboxclient_component::{ MailboxclientComponent, MailboxclientComponent::MailboxClientInternalImpl }; - use hyperlane_starknet::contracts::client::{mailboxclient}; - use hyperlane_starknet::contracts::libs::message::Message; - use hyperlane_starknet::interfaces::{ + use contracts::client::{mailboxclient}; + use contracts::libs::message::Message; + use contracts::interfaces::{ IPostDispatchHook, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, DomainRoutingHookConfig, IDomainRoutingHook, Types }; @@ -64,7 +64,7 @@ pub mod domain_routing_hook { _owner: ContractAddress, _fee_token_address: ContractAddress ) { - self.mailboxclient.initialize(_mailbox); + self.mailboxclient.initialize(_mailbox, Option::None, Option::None); self.ownable.initializer(_owner); self.fee_token.write(_fee_token_address); } diff --git a/cairo/crates/contracts/src/lib.cairo b/cairo/crates/contracts/src/lib.cairo index e419646f..3d6aeca3 100644 --- a/cairo/crates/contracts/src/lib.cairo +++ b/cairo/crates/contracts/src/lib.cairo @@ -30,6 +30,7 @@ pub mod isms { } } pub mod hooks { + pub mod domain_routing_hook; pub mod merkle_tree_hook; pub mod protocol_fee; pub mod libs { diff --git a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo index b42d0435..058f38f3 100644 --- a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo +++ b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo @@ -1,15 +1,21 @@ use alexandria_bytes::{Bytes, BytesTrait, BytesStore}; -use hyperlane_starknet::contracts::libs::message::{Message, MessageTrait, HYPERLANE_VERSION}; -use hyperlane_starknet::interfaces::{ + +use contracts::interfaces::{ Types, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, IDomainRoutingHookDispatcher, IDomainRoutingHookDispatcherTrait, DomainRoutingHookConfig, ETH_ADDRESS }; -use hyperlane_starknet::tests::setup::{setup_domain_routing_hook, OWNER, PROTOCOL_FEE, NEW_OWNER}; + +use contracts::utils::utils::U256TryIntoContractAddress; +use contracts::libs::message::{Message, MessageTrait, HYPERLANE_VERSION}; + use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; + use snforge_std::{start_prank, CheatTarget, stop_prank}; + use starknet::{get_caller_address, contract_address_const, ContractAddress}; +use super::super::setup::{setup_domain_routing_hook, OWNER, PROTOCOL_FEE, NEW_OWNER}; #[test] @@ -30,7 +36,7 @@ fn test_supports_metadata_for_domain_routing_hook() { fn test_domain_rounting_set_hook() { let (_, set_routing_hook_addrs) = setup_domain_routing_hook(); let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let destination: u32 = 12; let hook: ContractAddress = contract_address_const::<1>(); set_routing_hook_addrs.set_hook(destination, hook); @@ -49,7 +55,7 @@ fn test_set_hook_fails_if_not_owner() { fn test_domain_rounting_set_hook_array() { let (_, set_routing_hook_addrs) = setup_domain_routing_hook(); let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let mut hook_config_arr = ArrayTrait::::new(); hook_config_arr .append(DomainRoutingHookConfig { destination: 1, hook: contract_address_const::<2>() }); @@ -79,7 +85,7 @@ fn test_set_hook_array_fails_if_not_owner() { fn hook_not_set_for_destination_should_fail() { let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let destination: u32 = 12; let hook: ContractAddress = contract_address_const::<1>(); set_routing_hook_addrs.set_hook(destination, hook); @@ -88,9 +94,9 @@ fn hook_not_set_for_destination_should_fail() { version: HYPERLANE_VERSION, nonce: 0_u32, origin: 0_u32, - sender: contract_address_const::<0>(), + sender: 0, destination: destination - 1, - recipient: contract_address_const::<0>(), + recipient: 0, body: BytesTrait::new_empty(), }; let metadata = BytesTrait::new_empty(); @@ -104,7 +110,7 @@ fn hook_set_for_destination_post_dispatch() { let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); let fee_token_instance = IERC20Dispatcher { contract_address: ETH_ADDRESS() }; let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let destination: u32 = 18; let hook: ContractAddress = contract_address_const::<12>(); set_routing_hook_addrs.set_hook(destination, hook); @@ -113,21 +119,21 @@ fn hook_set_for_destination_post_dispatch() { version: HYPERLANE_VERSION, nonce: 0_u32, origin: 0_u32, - sender: contract_address_const::<0>(), + sender: 0, destination: destination, - recipient: contract_address_const::<0>(), + recipient: 0, body: BytesTrait::new_empty(), }; let metadata = BytesTrait::new_empty(); stop_prank(CheatTarget::One(ownable.contract_address)); let erc20Ownable = IOwnableDispatcher { contract_address: ETH_ADDRESS() }; - start_prank(CheatTarget::One(erc20Ownable.contract_address), NEW_OWNER()); + start_prank(CheatTarget::One(erc20Ownable.contract_address), NEW_OWNER().try_into().unwrap()); fee_token_instance.transfer(ownable.contract_address, PROTOCOL_FEE); stop_prank(CheatTarget::One(erc20Ownable.contract_address)); assert_eq!(fee_token_instance.balance_of(ownable.contract_address), PROTOCOL_FEE); - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); fee_token_instance.approve(routing_hook_addrs.contract_address, PROTOCOL_FEE); routing_hook_addrs.post_dispatch(metadata, message, PROTOCOL_FEE); } @@ -137,7 +143,7 @@ fn hook_set_for_destination_post_dispatch() { fn hook_set_for_destination_quote_dispatch() { let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let destination: u32 = 18; let hook: ContractAddress = contract_address_const::<1212>(); set_routing_hook_addrs.set_hook(destination, hook); @@ -146,9 +152,9 @@ fn hook_set_for_destination_quote_dispatch() { version: HYPERLANE_VERSION, nonce: 0_u32, origin: 0_u32, - sender: contract_address_const::<0>(), + sender: 0, destination: destination, - recipient: contract_address_const::<0>(), + recipient: 0, body: BytesTrait::new_empty(), }; let metadata = BytesTrait::new_empty(); diff --git a/cairo/crates/contracts/tests/lib.cairo b/cairo/crates/contracts/tests/lib.cairo index e22f79f4..48ad4f3d 100644 --- a/cairo/crates/contracts/tests/lib.cairo +++ b/cairo/crates/contracts/tests/lib.cairo @@ -8,6 +8,7 @@ pub mod isms { pub mod test_messageid_multisig; } pub mod hooks { + pub mod test_domain_routing_hook; pub mod test_merkle_tree_hook; pub mod test_protocol_fee; } diff --git a/cairo/crates/contracts/tests/setup.cairo b/cairo/crates/contracts/tests/setup.cairo index 3ad87d2f..ccc06404 100644 --- a/cairo/crates/contracts/tests/setup.cairo +++ b/cairo/crates/contracts/tests/setup.cairo @@ -588,10 +588,11 @@ pub fn setup_protocol_fee() -> (IProtocolFeeDispatcher, IPostDispatchHookDispatc pub fn setup_domain_routing_hook() -> (IPostDispatchHookDispatcher, IDomainRoutingHookDispatcher) { let (mailbox, _, _, _) = setup_mailbox(MAILBOX(), Option::None, Option::None); let domain_routing_hook_class = declare("domain_routing_hook").unwrap(); - - let (domain_routing_hook_addrs, _) = domain_routing_hook_class - .deploy(@array![mailbox.contract_address.into(), OWNER().into(), ETH_ADDRESS().into()]) - .unwrap(); + let u256_owner: felt252 = OWNER().try_into().unwrap(); + let args: Array = array![ + mailbox.contract_address.into(), u256_owner, ETH_ADDRESS().into() + ]; + let (domain_routing_hook_addrs, _) = domain_routing_hook_class.deploy(@args).unwrap(); ( IPostDispatchHookDispatcher { contract_address: domain_routing_hook_addrs }, IDomainRoutingHookDispatcher { contract_address: domain_routing_hook_addrs } From 20036cf140696f330f5affa62fc1aad2f50430e1 Mon Sep 17 00:00:00 2001 From: JordyRo1 Date: Wed, 11 Dec 2024 10:34:34 +0100 Subject: [PATCH 09/13] fix: fmt --- cairo/crates/contracts/src/hooks/domain_routing_hook.cairo | 2 +- .../crates/contracts/tests/hooks/test_domain_routing_hook.cairo | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo index 7eb1447c..197ed9fd 100644 --- a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo +++ b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo @@ -5,11 +5,11 @@ pub mod domain_routing_hook { MailboxclientComponent, MailboxclientComponent::MailboxClientInternalImpl }; use contracts::client::{mailboxclient}; - use contracts::libs::message::Message; use contracts::interfaces::{ IPostDispatchHook, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, DomainRoutingHookConfig, IDomainRoutingHook, Types }; + use contracts::libs::message::Message; use openzeppelin::access::ownable::OwnableComponent; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; use openzeppelin::upgrades::{interface::IUpgradeable, upgradeable::UpgradeableComponent}; diff --git a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo index 058f38f3..8dfac1d3 100644 --- a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo +++ b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo @@ -5,9 +5,9 @@ use contracts::interfaces::{ IDomainRoutingHookDispatcher, IDomainRoutingHookDispatcherTrait, DomainRoutingHookConfig, ETH_ADDRESS }; +use contracts::libs::message::{Message, MessageTrait, HYPERLANE_VERSION}; use contracts::utils::utils::U256TryIntoContractAddress; -use contracts::libs::message::{Message, MessageTrait, HYPERLANE_VERSION}; use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; From e1b88b4b597455cdf8666a6a62de024e1722714c Mon Sep 17 00:00:00 2001 From: JordyRo1 Date: Wed, 11 Dec 2024 15:23:34 +0100 Subject: [PATCH 10/13] fix: fee management in hook --- .../src/hooks/domain_routing_hook.cairo | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo index 197ed9fd..8c72c3de 100644 --- a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo +++ b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo @@ -44,6 +44,7 @@ pub mod domain_routing_hook { pub const FEE_AMOUNT_TRANSFER_FAILED: felt252 = 'Hook fee transfer failed'; pub const ZERO_FEE: felt252 = 'Zero fee amount'; pub const INSUFFICIENT_ALLOWANCE: felt252 = 'Insufficient allowance'; + pub const AMOUNT_DOES_NOT_COVER_HOOK_QUOTE: felt252 = 'Amount does not cover quote fee'; } #[event] @@ -83,14 +84,27 @@ pub mod domain_routing_hook { ref self: ContractState, _metadata: Bytes, _message: Message, _fee_amount: u256 ) { assert(_fee_amount > 0, Errors::ZERO_FEE); + + // We should check that the fee_amount is enough for the desired hook to work before actually send the amount + // We assume that the fee token is the same across the hooks + + let required_amount = self.quote_dispatch(_metadata.clone(), _message.clone()); + assert(_fee_amount >= required_amount, Errors::AMOUNT_DOES_NOT_COVER_HOOK_QUOTE); + let caller = get_caller_address(); let configured_hook_address: ContractAddress = self ._get_configured_hook(_message.clone()) .contract_address; - self._transfer_routing_fee_to_hook(caller, configured_hook_address, _fee_amount); + + // Tricky here: if the destination hook does operations with the transfered fee, we need to send it before + // the operation. However, if we send the fee before and for an unexpected reason the destination hook reverts, + // it will have to send back the token to the caller. For now, we assume that the destination hook does not + // do anything with the fee, so we can send it after the `_post_dispatch` call. self - ._get_configured_hook(_message.clone()) - .post_dispatch(_metadata, _message, _fee_amount); + ._get_configured_hook(_message.clone()) + .post_dispatch(_metadata, _message, _fee_amount); + self._transfer_routing_fee_to_hook(caller, configured_hook_address, required_amount); + } fn quote_dispatch(ref self: ContractState, _metadata: Bytes, _message: Message) -> u256 { @@ -140,8 +154,7 @@ pub mod domain_routing_hook { token_dispatcher.allowance(from, this_contract_address) >= amount, Errors::INSUFFICIENT_ALLOWANCE ); - let transfer_flag: bool = token_dispatcher.transfer_from(from, to, amount); - assert(transfer_flag == false, Errors::FEE_AMOUNT_TRANSFER_FAILED); + token_dispatcher.transfer_from(from, to, amount); } } } From e8546aa1a798b5b750694f6fe0bf77c15b4ea982 Mon Sep 17 00:00:00 2001 From: JordyRo1 Date: Wed, 11 Dec 2024 20:12:15 +0100 Subject: [PATCH 11/13] feat: test coverage --- .../src/hooks/domain_routing_hook.cairo | 6 +- .../hooks/test_domain_routing_hook.cairo | 257 ++++++++++++++++-- .../tests/hooks/test_protocol_fee.cairo | 24 +- cairo/crates/contracts/tests/setup.cairo | 18 +- .../crates/contracts/tests/test_mailbox.cairo | 12 +- 5 files changed, 272 insertions(+), 45 deletions(-) diff --git a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo index 8c72c3de..7555eea8 100644 --- a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo +++ b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo @@ -41,7 +41,6 @@ pub mod domain_routing_hook { mod Errors { pub const INVALID_DESTINATION: felt252 = 'Destination has no hooks'; pub const INSUFFICIENT_BALANCE: felt252 = 'Insufficient balance'; - pub const FEE_AMOUNT_TRANSFER_FAILED: felt252 = 'Hook fee transfer failed'; pub const ZERO_FEE: felt252 = 'Zero fee amount'; pub const INSUFFICIENT_ALLOWANCE: felt252 = 'Insufficient allowance'; pub const AMOUNT_DOES_NOT_COVER_HOOK_QUOTE: felt252 = 'Amount does not cover quote fee'; @@ -101,10 +100,9 @@ pub mod domain_routing_hook { // it will have to send back the token to the caller. For now, we assume that the destination hook does not // do anything with the fee, so we can send it after the `_post_dispatch` call. self - ._get_configured_hook(_message.clone()) - .post_dispatch(_metadata, _message, _fee_amount); + ._get_configured_hook(_message.clone()) + .post_dispatch(_metadata, _message, _fee_amount); self._transfer_routing_fee_to_hook(caller, configured_hook_address, required_amount); - } fn quote_dispatch(ref self: ContractState, _metadata: Bytes, _message: Message) -> u256 { diff --git a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo index 8dfac1d3..cd85c5a2 100644 --- a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo +++ b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo @@ -3,20 +3,21 @@ use alexandria_bytes::{Bytes, BytesTrait, BytesStore}; use contracts::interfaces::{ Types, IPostDispatchHookDispatcher, IPostDispatchHookDispatcherTrait, IDomainRoutingHookDispatcher, IDomainRoutingHookDispatcherTrait, DomainRoutingHookConfig, - ETH_ADDRESS + IProtocolFeeDispatcher, IProtocolFeeDispatcherTrait, ETH_ADDRESS }; use contracts::libs::message::{Message, MessageTrait, HYPERLANE_VERSION}; use contracts::utils::utils::U256TryIntoContractAddress; use openzeppelin::access::ownable::interface::{IOwnableDispatcher, IOwnableDispatcherTrait}; -use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; +use openzeppelin::token::erc20::interface::{ERC20ABIDispatcher, ERC20ABIDispatcherTrait}; -use snforge_std::{start_prank, CheatTarget, stop_prank}; +use snforge_std::{start_prank, get_class_hash, ContractClass, CheatTarget, stop_prank}; use starknet::{get_caller_address, contract_address_const, ContractAddress}; -use super::super::setup::{setup_domain_routing_hook, OWNER, PROTOCOL_FEE, NEW_OWNER}; - +use super::super::setup::{ + setup_domain_routing_hook, OWNER, setup_mock_token, setup_protocol_fee, PROTOCOL_FEE, NEW_OWNER +}; #[test] fn test_domain_routing_hook_type() { @@ -104,18 +105,36 @@ fn hook_not_set_for_destination_should_fail() { routing_hook_addrs.post_dispatch(metadata, message, protocol_fee); } -#[ignore] #[test] fn hook_set_for_destination_post_dispatch() { let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); - let fee_token_instance = IERC20Dispatcher { contract_address: ETH_ADDRESS() }; + let fee_token_instance = ERC20ABIDispatcher { contract_address: ETH_ADDRESS() }; let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); + + // We define a first destination let destination: u32 = 18; - let hook: ContractAddress = contract_address_const::<12>(); - set_routing_hook_addrs.set_hook(destination, hook); + let (protocol_fee, _) = setup_protocol_fee(Option::None); + set_routing_hook_addrs.set_hook(destination, protocol_fee.contract_address); - let message = Message { + let protocol_fee_class_hash = get_class_hash(protocol_fee.contract_address); + let protocol_fee_contract_class = ContractClass { class_hash: protocol_fee_class_hash }; + + // We define a second destination + let second_destination: u32 = 32; + let (second_protocol_fee, _) = setup_protocol_fee(Option::Some(protocol_fee_contract_class)); + let new_protocol_fee = 3 * PROTOCOL_FEE; + // We change the configuration + let protocol_ownable = IOwnableDispatcher { + contract_address: second_protocol_fee.contract_address + }; + start_prank(CheatTarget::One(protocol_ownable.contract_address), OWNER().try_into().unwrap()); + second_protocol_fee.set_protocol_fee(new_protocol_fee); + stop_prank(CheatTarget::One(protocol_ownable.contract_address)); + + set_routing_hook_addrs.set_hook(second_destination, second_protocol_fee.contract_address); + + let message_1 = Message { version: HYPERLANE_VERSION, nonce: 0_u32, origin: 0_u32, @@ -124,31 +143,221 @@ fn hook_set_for_destination_post_dispatch() { recipient: 0, body: BytesTrait::new_empty(), }; + let message_2 = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: 0, + destination: second_destination, + recipient: 0, + body: BytesTrait::new_empty(), + }; let metadata = BytesTrait::new_empty(); - stop_prank(CheatTarget::One(ownable.contract_address)); let erc20Ownable = IOwnableDispatcher { contract_address: ETH_ADDRESS() }; + start_prank(CheatTarget::One(erc20Ownable.contract_address), OWNER().try_into().unwrap()); + fee_token_instance.transfer(NEW_OWNER().try_into().unwrap(), PROTOCOL_FEE + new_protocol_fee); + stop_prank(CheatTarget::One(erc20Ownable.contract_address)); + + assert_eq!( + fee_token_instance.balance_of(NEW_OWNER().try_into().unwrap()), + PROTOCOL_FEE + new_protocol_fee + ); start_prank(CheatTarget::One(erc20Ownable.contract_address), NEW_OWNER().try_into().unwrap()); - fee_token_instance.transfer(ownable.contract_address, PROTOCOL_FEE); + fee_token_instance + .approve(routing_hook_addrs.contract_address, PROTOCOL_FEE + new_protocol_fee); + assert( + fee_token_instance + .allowance( + NEW_OWNER().try_into().unwrap(), routing_hook_addrs.contract_address + ) == PROTOCOL_FEE + + new_protocol_fee, + 'Approve failed' + ); stop_prank(CheatTarget::One(erc20Ownable.contract_address)); + stop_prank(CheatTarget::One(ownable.contract_address)); + start_prank(CheatTarget::One(ownable.contract_address), NEW_OWNER().try_into().unwrap()); + + routing_hook_addrs.post_dispatch(metadata.clone(), message_1, PROTOCOL_FEE); + assert_eq!(fee_token_instance.balance_of(NEW_OWNER().try_into().unwrap()), new_protocol_fee); + + routing_hook_addrs.post_dispatch(metadata, message_2, new_protocol_fee); + assert_eq!(fee_token_instance.balance_of(NEW_OWNER().try_into().unwrap()), 0); +} + + +#[test] +#[should_panic(expected: 'Zero fee amount',)] +fn test_post_dispatch_zero_fee() { + let (routing_hook_addrs, _) = setup_domain_routing_hook(); + let message = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: 0, + destination: 18, + recipient: 0, + body: BytesTrait::new_empty(), + }; + let metadata = BytesTrait::new_empty(); + + // This should panic with 'Zero fee amount' + routing_hook_addrs.post_dispatch(metadata, message, 0); +} + +#[test] +#[should_panic(expected: 'Amount does not cover quote fee',)] +fn test_post_dispatch_insufficient_fee() { + let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; - assert_eq!(fee_token_instance.balance_of(ownable.contract_address), PROTOCOL_FEE); start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); + + // Set up a destination with a specific hook + let destination: u32 = 18; + let (protocol_fee, _) = setup_protocol_fee(Option::None); + set_routing_hook_addrs.set_hook(destination, protocol_fee.contract_address); + + let message = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: 0, + destination: destination, + recipient: 0, + body: BytesTrait::new_empty(), + }; + let metadata = BytesTrait::new_empty(); + + // This should panic with 'Amount does not cover quote fee' + // Assuming PROTOCOL_FEE is smaller than the required quote + routing_hook_addrs.post_dispatch(metadata, message, PROTOCOL_FEE / 2); +} + +#[test] +#[should_panic(expected: 'Insufficient balance',)] +fn test_transfer_routing_fee_insufficient_balance() { + let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; + + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); + // We define a first destination + let destination: u32 = 18; + let (protocol_fee, _) = setup_protocol_fee(Option::None); + set_routing_hook_addrs.set_hook(destination, protocol_fee.contract_address); + + let fee_token_instance = ERC20ABIDispatcher { contract_address: ETH_ADDRESS() }; + + let message_1 = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: 0, + destination: destination, + recipient: 0, + body: BytesTrait::new_empty(), + }; + + let metadata = BytesTrait::new_empty(); + + let erc20Ownable = IOwnableDispatcher { contract_address: ETH_ADDRESS() }; + start_prank(CheatTarget::One(erc20Ownable.contract_address), OWNER().try_into().unwrap()); + // We transfer insufficient amount + fee_token_instance.transfer(NEW_OWNER().try_into().unwrap(), PROTOCOL_FEE / 2); + stop_prank(CheatTarget::One(erc20Ownable.contract_address)); + + start_prank(CheatTarget::One(erc20Ownable.contract_address), NEW_OWNER().try_into().unwrap()); fee_token_instance.approve(routing_hook_addrs.contract_address, PROTOCOL_FEE); - routing_hook_addrs.post_dispatch(metadata, message, PROTOCOL_FEE); + assert( + fee_token_instance + .allowance( + NEW_OWNER().try_into().unwrap(), routing_hook_addrs.contract_address + ) == PROTOCOL_FEE, + 'Approve failed' + ); + stop_prank(CheatTarget::One(erc20Ownable.contract_address)); + stop_prank(CheatTarget::One(ownable.contract_address)); + start_prank(CheatTarget::One(ownable.contract_address), NEW_OWNER().try_into().unwrap()); + + routing_hook_addrs.post_dispatch(metadata.clone(), message_1, PROTOCOL_FEE); +} + +#[test] +#[should_panic(expected: 'Insufficient allowance',)] +fn test_transfer_routing_fee_insufficient_allowance() { + let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); + // We define a first destination + let destination: u32 = 18; + let (protocol_fee, _) = setup_protocol_fee(Option::None); + let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); + set_routing_hook_addrs.set_hook(destination, protocol_fee.contract_address); + + let fee_token_instance = ERC20ABIDispatcher { contract_address: ETH_ADDRESS() }; + + let message_1 = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: 0, + destination: destination, + recipient: 0, + body: BytesTrait::new_empty(), + }; + + let metadata = BytesTrait::new_empty(); + + let erc20Ownable = IOwnableDispatcher { contract_address: ETH_ADDRESS() }; + start_prank(CheatTarget::One(erc20Ownable.contract_address), OWNER().try_into().unwrap()); + // We transfer insufficient amount + fee_token_instance.transfer(NEW_OWNER().try_into().unwrap(), PROTOCOL_FEE); + stop_prank(CheatTarget::One(erc20Ownable.contract_address)); + + start_prank(CheatTarget::One(erc20Ownable.contract_address), NEW_OWNER().try_into().unwrap()); + fee_token_instance.approve(routing_hook_addrs.contract_address, PROTOCOL_FEE / 2); + // Insufficient allowance for dispatch + assert( + fee_token_instance + .allowance( + NEW_OWNER().try_into().unwrap(), routing_hook_addrs.contract_address + ) == PROTOCOL_FEE + / 2, + 'Approve failed' + ); + stop_prank(CheatTarget::One(erc20Ownable.contract_address)); + stop_prank(CheatTarget::One(ownable.contract_address)); + start_prank(CheatTarget::One(ownable.contract_address), NEW_OWNER().try_into().unwrap()); + + routing_hook_addrs.post_dispatch(metadata.clone(), message_1, PROTOCOL_FEE); } -#[ignore] #[test] fn hook_set_for_destination_quote_dispatch() { let (routing_hook_addrs, set_routing_hook_addrs) = setup_domain_routing_hook(); let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); + + // We define a first destination let destination: u32 = 18; - let hook: ContractAddress = contract_address_const::<1212>(); - set_routing_hook_addrs.set_hook(destination, hook); + let (protocol_fee, _) = setup_protocol_fee(Option::None); + set_routing_hook_addrs.set_hook(destination, protocol_fee.contract_address); - let message = Message { + let protocol_fee_class_hash = get_class_hash(protocol_fee.contract_address); + let protocol_fee_contract_class = ContractClass { class_hash: protocol_fee_class_hash }; + // We define a second destination + let second_destination: u32 = 32; + let (second_protocol_fee, _) = setup_protocol_fee(Option::Some(protocol_fee_contract_class)); + let new_protocol_fee = 3 * PROTOCOL_FEE; + // We change the configuration + let protocol_ownable = IOwnableDispatcher { + contract_address: second_protocol_fee.contract_address + }; + start_prank(CheatTarget::One(protocol_ownable.contract_address), OWNER().try_into().unwrap()); + second_protocol_fee.set_protocol_fee(new_protocol_fee); + stop_prank(CheatTarget::One(protocol_ownable.contract_address)); + set_routing_hook_addrs.set_hook(second_destination, second_protocol_fee.contract_address); + + let message_1 = Message { version: HYPERLANE_VERSION, nonce: 0_u32, origin: 0_u32, @@ -157,6 +366,16 @@ fn hook_set_for_destination_quote_dispatch() { recipient: 0, body: BytesTrait::new_empty(), }; + let message_2 = Message { + version: HYPERLANE_VERSION, + nonce: 0_u32, + origin: 0_u32, + sender: 0, + destination: second_destination, + recipient: 0, + body: BytesTrait::new_empty(), + }; let metadata = BytesTrait::new_empty(); - routing_hook_addrs.quote_dispatch(metadata, message); + assert_eq!(routing_hook_addrs.quote_dispatch(metadata.clone(), message_1), PROTOCOL_FEE); + assert_eq!(routing_hook_addrs.quote_dispatch(metadata, message_2), new_protocol_fee); } diff --git a/cairo/crates/contracts/tests/hooks/test_protocol_fee.cairo b/cairo/crates/contracts/tests/hooks/test_protocol_fee.cairo index 659c424e..95459712 100644 --- a/cairo/crates/contracts/tests/hooks/test_protocol_fee.cairo +++ b/cairo/crates/contracts/tests/hooks/test_protocol_fee.cairo @@ -16,13 +16,13 @@ use super::super::setup::{ #[test] fn test_hook_type() { - let (_, protocol_fee) = setup_protocol_fee(); + let (_, protocol_fee) = setup_protocol_fee(Option::None); assert_eq!(protocol_fee.hook_type(), Types::PROTOCOL_FEE(())); } #[test] fn test_set_protocol_fee() { - let (protocol_fee, _) = setup_protocol_fee(); + let (protocol_fee, _) = setup_protocol_fee(Option::None); let ownable = IOwnableDispatcher { contract_address: protocol_fee.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let new_protocol_fee = 20000; @@ -34,7 +34,7 @@ fn test_set_protocol_fee() { #[test] #[should_panic(expected: ('Caller is not the owner',))] fn test_set_protocol_fee_fails_if_not_owner() { - let (protocol_fee, _) = setup_protocol_fee(); + let (protocol_fee, _) = setup_protocol_fee(Option::None); let new_protocol_fee = 20000; protocol_fee.set_protocol_fee(new_protocol_fee); } @@ -42,7 +42,7 @@ fn test_set_protocol_fee_fails_if_not_owner() { #[test] #[should_panic(expected: ('Exceeds max protocol fee',))] fn test_set_protocol_fee_fails_if_higher_than_max() { - let (protocol_fee, _) = setup_protocol_fee(); + let (protocol_fee, _) = setup_protocol_fee(Option::None); let ownable = IOwnableDispatcher { contract_address: protocol_fee.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let new_protocol_fee = MAX_PROTOCOL_FEE + 1; @@ -53,7 +53,7 @@ fn test_set_protocol_fee_fails_if_higher_than_max() { #[test] fn test_set_beneficiary() { - let (protocol_fee, _) = setup_protocol_fee(); + let (protocol_fee, _) = setup_protocol_fee(Option::None); let ownable = IOwnableDispatcher { contract_address: protocol_fee.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let new_beneficiary = 'NEW_BENEFICIARY'.try_into().unwrap(); @@ -65,7 +65,7 @@ fn test_set_beneficiary() { #[test] #[should_panic(expected: ('Caller is not the owner',))] fn test_set_beneficiary_fails_if_not_owner() { - let (protocol_fee, _) = setup_protocol_fee(); + let (protocol_fee, _) = setup_protocol_fee(Option::None); let new_beneficiary = 'NEW_BENEFICIARY'.try_into().unwrap(); protocol_fee.set_beneficiary(new_beneficiary); } @@ -74,7 +74,7 @@ fn test_set_beneficiary_fails_if_not_owner() { #[test] fn test_collect_protocol_fee() { let fee_token = setup_mock_token(); - let (protocol_fee, _) = setup_protocol_fee(); + let (protocol_fee, _) = setup_protocol_fee(Option::None); let ownable = IOwnableDispatcher { contract_address: fee_token.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); @@ -92,7 +92,7 @@ fn test_collect_protocol_fee() { #[should_panic(expected: ('Insufficient balance',))] fn test_collect_protocol_fee_fails_if_insufficient_balance() { setup_mock_token(); - let (protocol_fee, _) = setup_protocol_fee(); + let (protocol_fee, _) = setup_protocol_fee(Option::None); protocol_fee.collect_protocol_fees(); } @@ -100,7 +100,7 @@ fn test_collect_protocol_fee_fails_if_insufficient_balance() { #[test] fn test_supports_metadata() { let mut metadata = BytesTrait::new_empty(); - let (_, post_dispatch_hook) = setup_protocol_fee(); + let (_, post_dispatch_hook) = setup_protocol_fee(Option::None); assert_eq!(post_dispatch_hook.supports_metadata(metadata.clone()), true); let variant = 1; metadata.append_u16(variant); @@ -115,7 +115,7 @@ fn test_supports_metadata() { #[should_panic(expected: ('Invalid metadata variant',))] fn test_post_dispatch_fails_if_invalid_variant() { let fee_token = ERC20ABIDispatcher { contract_address: ETH_ADDRESS() }; - let (_, post_dispatch_hook) = setup_protocol_fee(); + let (_, post_dispatch_hook) = setup_protocol_fee(Option::None); let ownable = IOwnableDispatcher { contract_address: fee_token.contract_address }; let mut metadata = BytesTrait::new_empty(); let variant = 2; @@ -130,7 +130,7 @@ fn test_post_dispatch_fails_if_invalid_variant() { #[test] fn test_quote_dispatch() { - let (_, post_dispatch_hook) = setup_protocol_fee(); + let (_, post_dispatch_hook) = setup_protocol_fee(Option::None); let mut metadata = BytesTrait::new_empty(); let variant = 1; let message = MessageTrait::default(); @@ -142,7 +142,7 @@ fn test_quote_dispatch() { #[test] #[should_panic(expected: ('Invalid metadata variant',))] fn test_quote_dispatch_fails_if_invalid_variant() { - let (_, post_dispatch_hook) = setup_protocol_fee(); + let (_, post_dispatch_hook) = setup_protocol_fee(Option::None); let mut metadata = BytesTrait::new_empty(); let variant = 2; metadata.append_u16(variant); diff --git a/cairo/crates/contracts/tests/setup.cairo b/cairo/crates/contracts/tests/setup.cairo index ccc06404..0a32db68 100644 --- a/cairo/crates/contracts/tests/setup.cairo +++ b/cairo/crates/contracts/tests/setup.cairo @@ -17,6 +17,7 @@ use contracts::interfaces::{ use contracts::libs::multisig::merkleroot_ism_metadata::merkleroot_ism_metadata::MERKLE_PROOF_ITERATION; use openzeppelin::account::utils::signature::EthSignature; use openzeppelin::token::erc20::interface::{ERC20ABI, ERC20ABIDispatcher, ERC20ABIDispatcherTrait}; +use snforge_std::cheatcodes::contract_class::ContractClass; use snforge_std::{ declare, ContractClassTrait, CheatTarget, EventSpy, EventAssertions, spy_events, SpyOn }; @@ -564,8 +565,13 @@ pub fn setup_mock_token() -> ERC20ABIDispatcher { ERC20ABIDispatcher { contract_address: fee_token_addr } } -pub fn setup_protocol_fee() -> (IProtocolFeeDispatcher, IPostDispatchHookDispatcher) { - let protocol_fee_class = declare("protocol_fee").unwrap(); +pub fn setup_protocol_fee( + class_hash: Option:: +) -> (IProtocolFeeDispatcher, IPostDispatchHookDispatcher) { + let protocol_fee_class = match class_hash { + Option::Some(hash) => hash, + Option::None => declare("protocol_fee").unwrap() + }; let (protocol_fee_addr, _) = protocol_fee_class .deploy( @array![ @@ -588,11 +594,15 @@ pub fn setup_protocol_fee() -> (IProtocolFeeDispatcher, IPostDispatchHookDispatc pub fn setup_domain_routing_hook() -> (IPostDispatchHookDispatcher, IDomainRoutingHookDispatcher) { let (mailbox, _, _, _) = setup_mailbox(MAILBOX(), Option::None, Option::None); let domain_routing_hook_class = declare("domain_routing_hook").unwrap(); - let u256_owner: felt252 = OWNER().try_into().unwrap(); + let u256_owner: felt252 = OWNER().try_into().expect('Failed to convert owner'); let args: Array = array![ mailbox.contract_address.into(), u256_owner, ETH_ADDRESS().into() ]; - let (domain_routing_hook_addrs, _) = domain_routing_hook_class.deploy(@args).unwrap(); + let deployment_result = domain_routing_hook_class.deploy(@args); + if deployment_result.is_err() { + panic(deployment_result.unwrap_err()); + } + let (domain_routing_hook_addrs, _) = deployment_result.unwrap(); ( IPostDispatchHookDispatcher { contract_address: domain_routing_hook_addrs }, IDomainRoutingHookDispatcher { contract_address: domain_routing_hook_addrs } diff --git a/cairo/crates/contracts/tests/test_mailbox.cairo b/cairo/crates/contracts/tests/test_mailbox.cairo index 9651433c..d9ed5b4f 100644 --- a/cairo/crates/contracts/tests/test_mailbox.cairo +++ b/cairo/crates/contracts/tests/test_mailbox.cairo @@ -173,7 +173,7 @@ fn test_dispatch() { #[test] fn test_dispatch_with_protocol_fee_hook() { - let (_, protocol_fee_hook) = setup_protocol_fee(); + let (_, protocol_fee_hook) = setup_protocol_fee(Option::None); let mock_hook = setup_mock_hook(); let (mailbox, mut spy, _, _) = setup_mailbox( MAILBOX(), @@ -242,7 +242,7 @@ fn test_dispatch_with_protocol_fee_hook() { #[test] fn test_dispatch_with_two_fee_hook() { - let (_, protocol_fee_hook) = setup_protocol_fee(); + let (_, protocol_fee_hook) = setup_protocol_fee(Option::None); let mock_hook = setup_mock_fee_hook(); let (mailbox, mut spy, _, _) = setup_mailbox( MAILBOX(), @@ -312,7 +312,7 @@ fn test_dispatch_with_two_fee_hook() { #[test] #[should_panic(expected: ('Provided fee < needed fee',))] fn test_dispatch_with_two_fee_hook_fails_if_greater_than_required_and_lower_than_default() { - let (_, protocol_fee_hook) = setup_protocol_fee(); + let (_, protocol_fee_hook) = setup_protocol_fee(Option::None); let mock_hook = setup_mock_fee_hook(); let (mailbox, mut spy, _, _) = setup_mailbox( MAILBOX(), @@ -381,7 +381,7 @@ fn test_dispatch_with_two_fee_hook_fails_if_greater_than_required_and_lower_than #[test] #[should_panic(expected: ('Provided fee < needed fee',))] fn test_dispatch_with_protocol_fee_hook_fails_if_provided_fee_lower_than_required_fee() { - let (_, protocol_fee_hook) = setup_protocol_fee(); + let (_, protocol_fee_hook) = setup_protocol_fee(Option::None); let mock_hook = setup_mock_hook(); let (mailbox, _, _, _) = setup_mailbox( @@ -421,7 +421,7 @@ fn test_dispatch_with_protocol_fee_hook_fails_if_provided_fee_lower_than_require #[test] #[should_panic(expected: ('Insufficient balance',))] fn test_dispatch_with_protocol_fee_hook_fails_if_user_balance_lower_than_fee_amount() { - let (_, protocol_fee_hook) = setup_protocol_fee(); + let (_, protocol_fee_hook) = setup_protocol_fee(Option::None); let mock_hook = setup_mock_hook(); let (mailbox, _, _, _) = setup_mailbox( @@ -463,7 +463,7 @@ fn test_dispatch_with_protocol_fee_hook_fails_if_user_balance_lower_than_fee_amo #[test] #[should_panic(expected: ('Insufficient allowance',))] fn test_dispatch_with_protocol_fee_hook_fails_if_insufficient_allowance() { - let (_, protocol_fee_hook) = setup_protocol_fee(); + let (_, protocol_fee_hook) = setup_protocol_fee(Option::None); let mock_hook = setup_mock_hook(); let (mailbox, _, _, _) = setup_mailbox( From 5547df692fe4a7e5c327082a3420d59e609612a7 Mon Sep 17 00:00:00 2001 From: JordyRo1 Date: Wed, 11 Dec 2024 20:15:19 +0100 Subject: [PATCH 12/13] feat: update ts deployment config --- scripts/configs/pragma_devnet.json | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/scripts/configs/pragma_devnet.json b/scripts/configs/pragma_devnet.json index 8bca64f7..38f7a1af 100644 --- a/scripts/configs/pragma_devnet.json +++ b/scripts/configs/pragma_devnet.json @@ -192,6 +192,23 @@ "value": "$OWNER_ADDRESS" } } + }, + "domain_routing_hook": { + "name": "domain_routing_hook", + "constructor": { + "mailbox": { + "type": "ContractAddress", + "value": "$mailbox" + }, + "owner": { + "type": "ContractAddress", + "value": "$OWNER_ADDRESS" + }, + "token_address": { + "type": "ContractAddress", + "value": "0x049D36570D4e46f48e99674bd3fcc84644DdD6b96F7C741B1562B82f9e004dC7" + } + } } }, "deploymentOrder": [ @@ -207,6 +224,7 @@ "merkle_tree_hook", "default_fallback_routing_ism", "trusted_relayer_ism", - "validator_announce" + "validator_announce", + "domain_routing_hook" ] } \ No newline at end of file From b8f6157911c1b6bd81cdac63e94e2a0a4ccc838d Mon Sep 17 00:00:00 2001 From: JordyRo1 Date: Thu, 12 Dec 2024 19:09:32 +0100 Subject: [PATCH 13/13] feat: adding a a getter for hook --- .../src/hooks/domain_routing_hook.cairo | 4 ++++ cairo/crates/contracts/src/interfaces.cairo | 3 ++- .../tests/hooks/test_domain_routing_hook.cairo | 16 ++++++++++------ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo index 7555eea8..7a630020 100644 --- a/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo +++ b/cairo/crates/contracts/src/hooks/domain_routing_hook.cairo @@ -28,6 +28,7 @@ pub mod domain_routing_hook { #[storage] struct Storage { hooks: LegacyMap, + domains: LegacyMap, fee_token: ContractAddress, #[substorage(v0)] mailboxclient: MailboxclientComponent::Storage, @@ -126,6 +127,9 @@ pub mod domain_routing_hook { }; }; } + fn get_hook(self: @ContractState, domain: u32) -> ContractAddress { + self.hooks.read(domain).contract_address + } } #[generate_trait] diff --git a/cairo/crates/contracts/src/interfaces.cairo b/cairo/crates/contracts/src/interfaces.cairo index 1785c335..beff9737 100644 --- a/cairo/crates/contracts/src/interfaces.cairo +++ b/cairo/crates/contracts/src/interfaces.cairo @@ -294,7 +294,7 @@ pub trait IRoutingIsm { fn route(self: @TContractState, _message: Message) -> ContractAddress; } -#[derive(Drop, Serde)] +#[derive(Drop, Serde, Copy)] pub struct DomainRoutingHookConfig { pub destination: u32, pub hook: ContractAddress @@ -304,4 +304,5 @@ pub struct DomainRoutingHookConfig { pub trait IDomainRoutingHook { fn set_hook(ref self: TContractState, _destination: u32, _hook: ContractAddress); fn set_hooks(ref self: TContractState, configs: Array); + fn get_hook(self: @TContractState, domain: u32) -> ContractAddress; } diff --git a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo index cd85c5a2..83520c66 100644 --- a/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo +++ b/cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo @@ -41,6 +41,7 @@ fn test_domain_rounting_set_hook() { let destination: u32 = 12; let hook: ContractAddress = contract_address_const::<1>(); set_routing_hook_addrs.set_hook(destination, hook); + assert_eq!(set_routing_hook_addrs.get_hook(destination), hook); } #[test] @@ -58,13 +59,16 @@ fn test_domain_rounting_set_hook_array() { let ownable = IOwnableDispatcher { contract_address: set_routing_hook_addrs.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER().try_into().unwrap()); let mut hook_config_arr = ArrayTrait::::new(); - hook_config_arr - .append(DomainRoutingHookConfig { destination: 1, hook: contract_address_const::<2>() }); - hook_config_arr - .append(DomainRoutingHookConfig { destination: 2, hook: contract_address_const::<3>() }); - hook_config_arr - .append(DomainRoutingHookConfig { destination: 3, hook: contract_address_const::<4>() }); + let config_1 = DomainRoutingHookConfig { destination: 1, hook: contract_address_const::<2>() }; + let config_2 = DomainRoutingHookConfig { destination: 2, hook: contract_address_const::<3>() }; + let config_3 = DomainRoutingHookConfig { destination: 3, hook: contract_address_const::<4>() }; + hook_config_arr.append(config_1); + hook_config_arr.append(config_2); + hook_config_arr.append(config_3); set_routing_hook_addrs.set_hooks(hook_config_arr); + assert_eq!(set_routing_hook_addrs.get_hook(config_1.destination), config_1.hook); + assert_eq!(set_routing_hook_addrs.get_hook(config_2.destination), config_2.hook); + assert_eq!(set_routing_hook_addrs.get_hook(config_3.destination), config_3.hook); }