Addresses issues raised during audit (#116)

* hyp_erc20_vaul_tests

* remove unused import

* remove unused import

* remove unused storage variable

* MailBoxClient & Router access control fix

* checks on ERC20 transfers

* HypeNative fixes

* erc20_comp removed from Hyp{Native/NativeScaled}

* HypNativeScaled transfer_from_sender_hook

* missing ERC721Enumerable initializer fix

* duplicate storage var 'wrapped_token' definition fix

* hyp_native_scaled 'transfer_remote' wrong impl embed fix'

* hyp_native_scaled 'transfer_from_sender_hook'

* warp routes missing fee collection logic fix

* HypERC721URICollateral missing transfer_to_hook fix

* removed unused imports + commented code

* removed erc20 comp from HypNative Contract

* hpy_721_uri_collateral unused storage variable removed

cairo/crates/contracts/src/client/mailboxclient_component.cairo

@@ -18,6 +18,7 @@ pub mod MailboxclientComponent {
 
     pub mod Errors {
         pub const ADDRESS_CANNOT_BE_ZERO: felt252 = 'Address cannot be zero';
+        pub const CALLER_NOT_MAILBOX: felt252 = 'Caller not mailbox';
     }
 
     #[embeddable_as(MailboxClientImpl)]
@@ -117,71 +118,6 @@ pub mod MailboxclientComponent {
             let mailbox: IMailboxDispatcher = self.mailbox.read();
             mailbox.contract_address
         }
-
-        /// Dispatches a message to the destination domain & recipient.
-        /// 
-        /// # Arguments
-        /// 
-        /// * - `_destination_domain` Domain of destination chain
-        /// * - `_recipient` Address of recipient on destination chain
-        /// * - `_message_body` Bytes content of message body
-        /// * - `_fee_amount` - the payment provided for sending the message
-        /// * - `_hook_metadata` Metadata used by the post dispatch hook
-        /// * - `_hook` Custom hook to use instead of the default
-        /// 
-        /// # Returns 
-        /// 
-        /// u256 - The message ID inserted into the Mailbox's merkle tree
-        fn _dispatch(
-            self: @ComponentState<TContractState>,
-            _destination_domain: u32,
-            _recipient: u256,
-            _message_body: Bytes,
-            _fee_amount: u256,
-            _hook_metadata: Option<Bytes>,
-            _hook: Option<ContractAddress>
-        ) -> u256 {
-            self
-                .mailbox
-                .read()
-                .dispatch(
-                    _destination_domain,
-                    _recipient,
-                    _message_body,
-                    _fee_amount,
-                    _hook_metadata,
-                    _hook
-                )
-        }
-
-        /// Computes quote for dispatching a message to the destination domain & recipient.
-        /// 
-        /// # Arguments
-        /// 
-        /// * - `_destination_domain` Domain of destination chain
-        /// * - `_recipient` Address of recipient on destination chain
-        /// * - `_message_body` Bytes content of message body
-        /// * - `_hook_metadata` Metadata used by the post dispatch hook
-        /// * - `_hook` Custom hook to use instead of the default
-        /// 
-        /// # Returns 
-        /// 
-        /// u256 - The payment required to dispatch the message
-        fn quote_dispatch(
-            self: @ComponentState<TContractState>,
-            _destination_domain: u32,
-            _recipient: u256,
-            _message_body: Bytes,
-            _hook_metadata: Option<Bytes>,
-            _hook: Option<ContractAddress>
-        ) -> u256 {
-            self
-                .mailbox
-                .read()
-                .quote_dispatch(
-                    _destination_domain, _recipient, _message_body, _hook_metadata, _hook
-                )
-        }
     }
 
     #[generate_trait]
@@ -210,5 +146,15 @@ pub mod MailboxclientComponent {
                 self.interchain_security_module.write(ism);
             }
         }
+
+        /// Panics if caller is not the 'mailbox'. 
+        /// Use this to restrict access to certain functions to the `mailbox`.
+        fn assert_only_mailbox(self: @ComponentState<TContractState>) {
+            let mailbox: IMailboxDispatcher = self.mailbox.read();
+            assert(
+                starknet::get_caller_address() == mailbox.contract_address,
+                Errors::CALLER_NOT_MAILBOX
+            );
+        }
     }
 }

cairo/crates/contracts/src/client/router_component.cairo

@@ -26,23 +26,28 @@ pub mod RouterComponent {
     use contracts::client::mailboxclient_component::{
         MailboxclientComponent, MailboxclientComponent::MailboxClientInternalImpl
     };
-    use contracts::interfaces::{IMailboxClient, IMailboxDispatcher, IMailboxDispatcherTrait};
+    use contracts::interfaces::{
+        IMailboxClient, IMailboxDispatcher, IMailboxDispatcherTrait, ETH_ADDRESS
+    };
     use contracts::libs::enumerable_map::{EnumerableMap, EnumerableMapTrait};
     use openzeppelin::access::ownable::{
         OwnableComponent, OwnableComponent::InternalImpl as OwnableInternalImpl
     };
+    use openzeppelin::token::erc20::interface::{IERC20Dispatcher, IERC20DispatcherTrait};
     use starknet::ContractAddress;
 
     #[storage]
     struct Storage {
         routers: EnumerableMap<u32, u256>,
-        gas_router: ContractAddress,
     }
 
     mod Err {
         pub fn domain_not_found(domain: u32) {
             panic!("No router enrolled for domain {}", domain);
         }
+        pub fn fee_transfer_failed() {
+            panic!("fee transfer from sender failed");
+        }
     }
 
     pub trait IMessageRecipientInternalHookTrait<TContractState> {
@@ -55,7 +60,7 @@ pub mod RouterComponent {
     pub impl Router<
         TContractState,
         +HasComponent<TContractState>,
-        +MailboxclientComponent::HasComponent<TContractState>,
+        impl MailboxClient: MailboxclientComponent::HasComponent<TContractState>,
         impl Owner: OwnableComponent::HasComponent<TContractState>,
         +Drop<TContractState>,
         impl Hook: IMessageRecipientInternalHookTrait<TContractState>
@@ -132,7 +137,9 @@ pub mod RouterComponent {
         /// # Arguments
         ///
         /// * `domains` - An array of `u32` values representing the domains for which routers are being unenrolled.
-        fn unenroll_remote_routers(ref self: ComponentState<TContractState>, domains: Array<u32>,) {
+        fn unenroll_remote_routers(ref self: ComponentState<TContractState>, domains: Array<u32>) {
+            let ownable_comp = get_dep_component!(@self, Owner);
+            ownable_comp.assert_only_owner();
             let domains_len = domains.len();
             let mut i = 0;
             while i < domains_len {
@@ -159,6 +166,8 @@ pub mod RouterComponent {
         fn handle(
             ref self: ComponentState<TContractState>, origin: u32, sender: u256, message: Bytes
         ) {
+            let mailbox_client_comp = get_dep_component!(@self, MailboxClient);
+            mailbox_client_comp.assert_only_mailbox();
             let router = self._must_have_remote_router(origin);
             assert!(router == sender, "Enrolled router does not match sender");
 
@@ -242,9 +251,19 @@ pub mod RouterComponent {
         ) -> u256 {
             let router = self._must_have_remote_router(destination_domain);
             let mut mailbox_comp = get_dep_component!(self, MailBoxClient);
-            let value = mailbox_comp
-                .mailbox
-                .read()
+
+            let mut fee_token_dispatcher = IERC20Dispatcher { contract_address: ETH_ADDRESS() };
+            if !fee_token_dispatcher
+                .transfer_from(
+                    starknet::get_caller_address(), starknet::get_contract_address(), value
+                ) {
+                Err::fee_transfer_failed();
+            }
+
+            let mailbox_dispatcher = mailbox_comp.mailbox.read();
+            fee_token_dispatcher.approve(mailbox_dispatcher.contract_address, value);
+
+            let value = mailbox_dispatcher
                 .dispatch(
                     destination_domain,
                     router,

cairo/crates/contracts/src/interfaces.cairo

@@ -156,25 +156,6 @@ pub trait IMailboxClient<TContractState> {
     fn _is_delivered(self: @TContractState, _id: u256) -> bool;
 
     fn mailbox(self: @TContractState) -> ContractAddress;
-
-    fn _dispatch(
-        self: @TContractState,
-        _destination_domain: u32,
-        _recipient: u256,
-        _message_body: Bytes,
-        _fee_amount: u256,
-        _hook_metadata: Option<Bytes>,
-        _hook: Option<ContractAddress>
-    ) -> u256;
-
-    fn quote_dispatch(
-        self: @TContractState,
-        _destination_domain: u32,
-        _recipient: u256,
-        _message_body: Bytes,
-        _hook_metadata: Option<Bytes>,
-        _hook: Option<ContractAddress>
-    ) -> u256;
 }
 
 

cairo/crates/mocks/src/erc4626_component.cairo

@@ -60,6 +60,8 @@ pub mod ERC4626Component {
         pub const EXCEEDED_MAX_MINT: felt252 = 'ERC4626: exceeded max mint';
         pub const EXCEEDED_MAX_REDEEM: felt252 = 'ERC4626: exceeded max redeem';
         pub const EXCEEDED_MAX_WITHDRAW: felt252 = 'ERC4626: exceeded max withdraw';
+        pub const ERC20_TRANSFER_FAILED: felt252 = 'ERC20 transfer failed';
+        pub const ERC20_TRANSFER_FROM_FAILED: felt252 = 'ERC20 transfer_from failed';
     }
 
     pub trait ERC4626HooksTrait<TContractState> {
@@ -442,7 +444,10 @@ pub mod ERC4626Component {
             Hooks::before_deposit(ref self, caller, receiver, assets, shares);
 
             let dispatcher = ERC20ABIDispatcher { contract_address: self.ERC4626_asset.read() };
-            dispatcher.transfer_from(caller, get_contract_address(), assets);
+            assert(
+                dispatcher.transfer_from(caller, get_contract_address(), assets),
+                Errors::ERC20_TRANSFER_FROM_FAILED
+            );
             let mut erc20_comp_mut = get_dep_component_mut!(ref self, ERC20);
             erc20_comp_mut.mint(receiver, shares);
             self.emit(Deposit { sender: caller, owner: receiver, assets, shares });
@@ -473,7 +478,7 @@ pub mod ERC4626Component {
             erc20_comp_mut.burn(owner, shares);
 
             let dispatcher = ERC20ABIDispatcher { contract_address: self.ERC4626_asset.read() };
-            dispatcher.transfer(receiver, assets);
+            assert(dispatcher.transfer(receiver, assets), Errors::ERC20_TRANSFER_FAILED);
 
             self.emit(Withdraw { sender: caller, receiver, owner, assets, shares });
 

cairo/crates/token/src/components/fast_token_router.cairo

@@ -29,18 +29,14 @@ pub mod FastTokenRouterComponent {
         MailboxclientComponent::MailboxClient
     };
     use contracts::client::router_component::{
-        RouterComponent, RouterComponent::RouterComponentInternalImpl,
-        RouterComponent::IMessageRecipientInternalHookTrait, IRouter
+        RouterComponent, RouterComponent::IMessageRecipientInternalHookTrait
     };
     use contracts::utils::utils::U256TryIntoContractAddress;
-    use openzeppelin::access::ownable::{
-        OwnableComponent, OwnableComponent::InternalImpl as OwnableInternalImpl
-    };
+    use openzeppelin::access::ownable::OwnableComponent;
     use starknet::ContractAddress;
     use token::components::token_message::TokenMessageTrait;
     use token::components::token_router::{
-        TokenRouterComponent, TokenRouterComponent::TokenRouterInternalImpl,
-        TokenRouterComponent::TokenRouterHooksTrait
+        TokenRouterComponent, TokenRouterComponent::TokenRouterHooksTrait
     };
 
     #[storage]
@@ -237,7 +233,7 @@ pub mod FastTokenRouterComponent {
 
             let filler_address = self
                 ._get_fast_transfers_key(origin, fast_transfer_id, amount, fast_fee, recipient);
-            if filler_address == 0 {
+            if filler_address != 0 {
                 return filler_address;
             }
 

cairo/crates/token/src/components/hyp_erc20_collateral_component.cairo

@@ -9,34 +9,28 @@ pub trait IHypErc20Collateral<TState> {
 #[starknet::component]
 pub mod HypErc20CollateralComponent {
     use alexandria_bytes::{Bytes, BytesTrait};
-    use contracts::client::gas_router_component::{
-        GasRouterComponent,
-        GasRouterComponent::{GasRouterInternalImpl, InternalTrait as GasRouterInternalTrait}
+    use contracts::client::{
+        gas_router_component::GasRouterComponent, router_component::RouterComponent,
+        mailboxclient_component::MailboxclientComponent
     };
-    use contracts::client::mailboxclient_component::{
-        MailboxclientComponent, MailboxclientComponent::MailboxClientImpl
-    };
-    use contracts::client::router_component::{
-        RouterComponent,
-        RouterComponent::{InternalTrait as RouterInternalTrait, RouterComponentInternalImpl}
-    };
-    use contracts::interfaces::IMailboxClient;
-    use contracts::utils::utils::{U256TryIntoContractAddress};
-
+    use contracts::utils::utils::U256TryIntoContractAddress;
     use openzeppelin::access::ownable::OwnableComponent;
     use openzeppelin::token::erc20::{ERC20ABIDispatcher, ERC20ABIDispatcherTrait};
     use starknet::ContractAddress;
-    use token::components::token_message::TokenMessageTrait;
     use token::components::token_router::{
-        TokenRouterComponent, TokenRouterComponent::TokenRouterInternalImpl,
-        TokenRouterComponent::TokenRouterHooksTrait
+        TokenRouterComponent, TokenRouterComponent::TokenRouterHooksTrait
     };
 
     #[storage]
     struct Storage {
         wrapped_token: ERC20ABIDispatcher
     }
 
+    pub mod Errors {
+        pub const ERC20_TRANSFER_FAILED: felt252 = 'ERC20 transfer failed';
+        pub const ERC20_TRANSFER_FROM_FAILED: felt252 = 'ERC20 transfer_from failed';
+    }
+
     pub impl TokenRouterHooksImpl<
         TContractState,
         +HasComponent<TContractState>,
@@ -123,20 +117,28 @@ pub mod HypErc20CollateralComponent {
         }
 
         fn _transfer_from_sender(ref self: ComponentState<TContractState>, amount: u256) -> Bytes {
-            self
-                .wrapped_token
-                .read()
-                .transfer_from(
-                    starknet::get_caller_address(), starknet::get_contract_address(), amount
-                );
+            assert(
+                self
+                    .wrapped_token
+                    .read()
+                    .transfer_from(
+                        starknet::get_caller_address(), starknet::get_contract_address(), amount
+                    ),
+                Errors::ERC20_TRANSFER_FROM_FAILED
+            );
             BytesTrait::new_empty()
         }
 
         fn _transfer_to(ref self: ComponentState<TContractState>, recipient: u256, amount: u256) {
-            self
-                .wrapped_token
-                .read()
-                .transfer(recipient.try_into().expect('u256 to ContractAddress failed'), amount);
+            assert(
+                self
+                    .wrapped_token
+                    .read()
+                    .transfer(
+                        recipient.try_into().expect('u256 to ContractAddress failed'), amount
+                    ),
+                Errors::ERC20_TRANSFER_FAILED
+            );
         }
     }
 }

cairo/crates/token/src/components/hyp_erc20_component.cairo

@@ -1,21 +1,11 @@
-use starknet::ContractAddress;
-
 #[starknet::component]
 pub mod HypErc20Component {
     use alexandria_bytes::{Bytes, BytesTrait};
-    use contracts::client::gas_router_component::{
-        GasRouterComponent,
-        GasRouterComponent::{GasRouterInternalImpl, InternalTrait as GasRouterInternalTrait}
-    };
-    use contracts::client::mailboxclient_component::{
-        MailboxclientComponent, MailboxclientComponent::MailboxClientImpl
-    };
-    use contracts::client::router_component::{
-        RouterComponent,
-        RouterComponent::{InternalTrait as RouterInternalTrait, RouterComponentInternalImpl}
+    use contracts::client::{
+        gas_router_component::GasRouterComponent, router_component::RouterComponent,
+        mailboxclient_component::MailboxclientComponent
     };
-    use contracts::interfaces::IMailboxClient;
-    use contracts::utils::utils::{U256TryIntoContractAddress};
+    use contracts::utils::utils::U256TryIntoContractAddress;
     use openzeppelin::access::ownable::OwnableComponent;
     use openzeppelin::token::erc20::ERC20Component;
     use openzeppelin::token::erc20::{
@@ -24,10 +14,8 @@ pub mod HypErc20Component {
     };
 
     use starknet::ContractAddress;
-    use token::components::token_message::TokenMessageTrait;
     use token::components::token_router::{
-        TokenRouterComponent, TokenRouterComponent::TokenRouterInternalImpl,
-        TokenRouterComponent::TokenRouterHooksTrait
+        TokenRouterComponent, TokenRouterComponent::TokenRouterHooksTrait
     };
 
     #[storage]