diff --git a/contracts/src/contracts/mailbox.cairo b/contracts/src/contracts/mailbox.cairo index bf8942f..6d2f797 100644 --- a/contracts/src/contracts/mailbox.cairo +++ b/contracts/src/contracts/mailbox.cairo @@ -118,12 +118,15 @@ pub mod mailbox { pub const UNEXPECTED_DESTINATION: felt252 = 'Unexpected destination'; pub const ALREADY_DELIVERED: felt252 = 'Mailbox: already delivered'; pub const ISM_VERIFICATION_FAILED: felt252 = 'Mailbox:ism verification failed'; + pub const ISM_CANNOT_BE_NULL: felt252 = 'ISM cannot be null'; + pub const OWNER_CANNOT_BE_NULL: felt252 = 'ISM cannot be null'; + pub const HOOK_CANNOT_BE_NULL: felt252 = 'Hook cannot be null'; pub const NO_ISM_FOUND: felt252 = 'ISM: no ISM found'; pub const NEW_OWNER_IS_ZERO: felt252 = 'Ownable: new owner cannot be 0'; pub const ALREADY_OWNER: felt252 = 'Ownable: already owner'; pub const INSUFFICIENT_BALANCE: felt252 = 'Insufficient balance'; pub const INSUFFICIENT_ALLOWANCE: felt252 = 'Insufficient allowance'; - pub const NOT_ENOUGH_FEE_PROVIDED: felt252 = 'Provided fee < required fee'; + pub const NOT_ENOUGH_FEE_PROVIDED: felt252 = 'Provided fee < needed fee'; } #[constructor] @@ -135,6 +138,10 @@ pub mod mailbox { _default_hook: ContractAddress, _required_hook: ContractAddress ) { + assert(_default_ism != contract_address_const::<0>(), Errors::ISM_CANNOT_BE_NULL); + assert(_default_hook != contract_address_const::<0>(), Errors::HOOK_CANNOT_BE_NULL); + assert(_default_hook != contract_address_const::<0>(), Errors::HOOK_CANNOT_BE_NULL); + assert(owner != contract_address_const::<0>(), Errors::OWNER_CANNOT_BE_NULL); self.local_domain.write(_local_domain); self.default_ism.write(_default_ism); self.default_hook.write(_default_hook); @@ -266,43 +273,41 @@ pub mod mailbox { // HOOKS - let caller_address = get_caller_address(); let required_hook_address = self.required_hook.read(); let required_hook = IPostDispatchHookDispatcher { contract_address: required_hook_address }; - let token_dispatcher = ERC20ABIDispatcher { contract_address: ETH_ADDRESS() }; - let mut required_fee = required_hook .quote_dispatch(hook_metadata.clone(), message.clone()); - if (required_fee > 0) { - assert(_fee_amount >= required_fee, Errors::NOT_ENOUGH_FEE_PROVIDED); - let contract_address = get_contract_address(); - let user_balance = token_dispatcher.balanceOf(caller_address); - assert(user_balance >= _fee_amount, Errors::INSUFFICIENT_BALANCE); - assert( - token_dispatcher.allowance(caller_address, contract_address) >= _fee_amount, - Errors::INSUFFICIENT_ALLOWANCE - ); + let hook_dispatcher = IPostDispatchHookDispatcher { contract_address: hook }; + let default_fee = hook_dispatcher + .quote_dispatch(hook_metadata.clone(), message.clone()); + + assert(_fee_amount >= required_fee + default_fee, Errors::NOT_ENOUGH_FEE_PROVIDED); + + let caller_address = get_caller_address(); + let contract_address = get_contract_address(); + + let token_dispatcher = ERC20ABIDispatcher { contract_address: ETH_ADDRESS() }; + let user_balance = token_dispatcher.balanceOf(caller_address); + + assert(user_balance >= required_fee + default_fee, Errors::INSUFFICIENT_BALANCE); + + assert( + token_dispatcher.allowance(caller_address, contract_address) >= _fee_amount, + Errors::INSUFFICIENT_ALLOWANCE + ); + + if (required_fee > 0) { token_dispatcher.transferFrom(caller_address, required_hook_address, required_fee); } - required_hook.post_dispatch(hook_metadata.clone(), message.clone(), required_fee); - if (hook != contract_address_const::<0>()) { - let hook_dispatcher = IPostDispatchHookDispatcher { contract_address: hook }; - let default_fee = hook_dispatcher - .quote_dispatch(hook_metadata.clone(), message.clone()); - if (default_fee == 0) { - hook_dispatcher - .post_dispatch(hook_metadata.clone(), message.clone(), default_fee); - } - if (_fee_amount - required_fee >= default_fee) { - token_dispatcher.transferFrom(caller_address, hook, default_fee); - hook_dispatcher.post_dispatch(hook_metadata, message.clone(), default_fee); - } + if (default_fee > 0) { + token_dispatcher.transferFrom(caller_address, hook, default_fee); } + hook_dispatcher.post_dispatch(hook_metadata, message.clone(), default_fee); id } diff --git a/contracts/src/tests/test_mailbox.cairo b/contracts/src/tests/test_mailbox.cairo index 4f62fdd..3cc29e9 100644 --- a/contracts/src/tests/test_mailbox.cairo +++ b/contracts/src/tests/test_mailbox.cairo @@ -302,7 +302,74 @@ fn test_dispatch_with_two_fee_hook() { } #[test] -#[should_panic(expected: ('Provided fee < required fee',))] +#[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 mock_hook = setup_mock_fee_hook(); + let (mailbox, mut spy, _, _) = setup_mailbox( + MAILBOX(), + Option::Some(protocol_fee_hook.contract_address), + Option::Some(mock_hook.contract_address) + ); + let erc20_dispatcher = ERC20ABIDispatcher { contract_address: ETH_ADDRESS() }; + let ownable = IOwnableDispatcher { contract_address: ETH_ADDRESS() }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + // (mock_fee_hook consummes 3 * PROTOCOL_FEE) + erc20_dispatcher.approve(MAILBOX(), 3 * PROTOCOL_FEE); + stop_prank(CheatTarget::One(ownable.contract_address)); + // The owner has the initial fee token supply + let ownable = IOwnableDispatcher { contract_address: mailbox.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + let array = array![ + 0x01020304050607080910111213141516, + 0x01020304050607080910111213141516, + 0x01020304050607080910000000000000 + ]; + + let message_body = BytesTrait::new(42, array); + let message = Message { + version: HYPERLANE_VERSION, + nonce: 0, + origin: LOCAL_DOMAIN, + sender: OWNER(), + destination: DESTINATION_DOMAIN, + recipient: RECIPIENT_ADDRESS(), + body: message_body.clone() + }; + let (message_id, _) = MessageTrait::format_message(message.clone()); + mailbox + .dispatch( + DESTINATION_DOMAIN, + RECIPIENT_ADDRESS(), + message_body, + 3 * PROTOCOL_FEE, + Option::None, + Option::None + ); + let expected_event = mailbox::Event::Dispatch( + mailbox::Dispatch { + sender: OWNER(), + destination_domain: DESTINATION_DOMAIN, + recipient_address: RECIPIENT_ADDRESS(), + message: message + } + ); + let expected_event_id = mailbox::Event::DispatchId(mailbox::DispatchId { id: message_id }); + + spy + .assert_emitted( + @array![ + (mailbox.contract_address, expected_event), + (mailbox.contract_address, expected_event_id) + ] + ); + + // balance check + assert_eq!(erc20_dispatcher.balance_of(OWNER()), INITIAL_SUPPLY - 4 * PROTOCOL_FEE); + assert(mailbox.get_latest_dispatched_id() == message_id, 'Failed to fetch latest id'); +} +#[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 mock_hook = setup_mock_hook(); diff --git a/contracts/src/tests/test_validator_announce.cairo b/contracts/src/tests/test_validator_announce.cairo index ad5e70b..d4da4ab 100644 --- a/contracts/src/tests/test_validator_announce.cairo +++ b/contracts/src/tests/test_validator_announce.cairo @@ -69,9 +69,7 @@ fn test_double_announce() { 180946006308525359965345158532346553211983108462325076142963585023296502126, 276191619276790668637754154763775604 ]; - let res = validator_announce - .announce(validator_address, _storage_location_2.clone(), signature); - assert_eq!(res, true); + validator_announce.announce(validator_address, _storage_location_2.clone(), signature); let validators = validator_announce.get_announced_validators(); assert(validators == array![validator_address].span(), 'validator array mismatch'); let storage_location = validator_announce.get_announced_storage_locations(validators);