feat: Domain Routing Hook documentation

README.md

@@ -26,7 +26,7 @@ The implementation guidelines can be found [here](https://docs.hyperlane.xyz/doc
 | Merkle Tree Hook       | ✅         |
 | Protocol Fee Hook      | ✅         |
 | Aggregation Hook       | ❌         |
-| Routing Hook           | ❌         |
+| Routing Hook           | ✅ (unaudited)         |
 | Pausable Hook          | ❌         |
 | Multisig ISM           | ✅         |
 | Pausable ISM           | ✅         |

cairo/crates/contracts/src/hooks/domain_routing_hook.cairo

@@ -1,3 +1,5 @@
+/// WARNING: THIS CONTRACT IS NOT AUDITED
+
 #[starknet::contract]
 pub mod domain_routing_hook {
     use alexandria_bytes::{Bytes, BytesTrait, BytesStore};
@@ -27,8 +29,9 @@ pub mod domain_routing_hook {
 
     #[storage]
     struct Storage {
+        /// Mapping of domain IDs to their corresponding post-dispatch hooks
         hooks: LegacyMap<u32, IPostDispatchHookDispatcher>,
-        domains: LegacyMap<u32, u32>,
+        /// The ERC20 token address used for paying routing fees
         fee_token: ContractAddress,
         #[substorage(v0)]
         mailboxclient: MailboxclientComponent::Storage,
@@ -40,10 +43,15 @@ pub mod domain_routing_hook {
 
 
     mod Errors {
+        /// Error when no hooks are configured for a destination domain
         pub const INVALID_DESTINATION: felt252 = 'Destination has no hooks';
+        /// Error when user has insufficient token balance
         pub const INSUFFICIENT_BALANCE: felt252 = 'Insufficient balance';
+        /// Error when fee amount is zero
         pub const ZERO_FEE: felt252 = 'Zero fee amount';
+        /// Error when user has insufficient token allowance
         pub const INSUFFICIENT_ALLOWANCE: felt252 = 'Insufficient allowance';
+        /// Error when provided fee does not cover the hook quote
         pub const AMOUNT_DOES_NOT_COVER_HOOK_QUOTE: felt252 = 'Amount does not cover quote fee';
     }
 
@@ -58,6 +66,14 @@ pub mod domain_routing_hook {
         MailboxclientEvent: MailboxclientComponent::Event,
     }
 
+
+    /// Constructor of the contract
+    /// 
+    /// # Arguments
+    ///
+    /// * `_mailbox` - The address of the mailbox contract
+    /// * `_owner` - The owner of the contract
+    /// * `_fee_token_address` - The address of the ERC20 token used for routing fees
     #[constructor]
     fn constructor(
         ref self: ContractState,
@@ -72,19 +88,40 @@ pub mod domain_routing_hook {
 
     #[abi(embed_v0)]
     impl IPostDispatchHookImpl of IPostDispatchHook<ContractState> {
+        /// Returns the type of hook (routing)
         fn hook_type(self: @ContractState) -> Types {
             Types::ROUTING(())
         }
 
+        /// Always returns true to support all metadata
+        /// 
+        /// # Arguments
+        /// 
+        /// * `_metadata` - Metadata for the hook
+        /// 
+        /// # Returns
+        /// 
+        /// boolean - whether the hook supports the metadata
         fn supports_metadata(self: @ContractState, _metadata: Bytes) -> bool {
             true
         }
 
+        /// Post-dispatch action for routing hooks
+        /// dev: the provided fee amount must not be zero, 
+        /// cover the quote dispatch of the associated hook.
+        /// 
+        /// # Arguments
+        /// 
+        /// * `_metadata` - Metadata for the hook
+        /// * `_message` - The message being dispatched
+        /// * `_fee_amount` - The fee amount provided for routing
+        ///  
+        /// # Errors
+        /// 
+        /// Reverts with `AMOUNT_DOES_NOT_COVER_HOOK_QUOTE` if the fee amount does not cover the hook quote
         fn post_dispatch(
             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
 
@@ -96,27 +133,51 @@ pub mod domain_routing_hook {
                 ._get_configured_hook(_message.clone())
                 .contract_address;
 
-            // 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. 
+            if (required_amount > 0) {
+                self
+                    ._transfer_routing_fee_to_hook(
+                        caller, configured_hook_address, required_amount
+                    );
+            };
             self
                 ._get_configured_hook(_message.clone())
-                .post_dispatch(_metadata, _message, _fee_amount);
-            self._transfer_routing_fee_to_hook(caller, configured_hook_address, required_amount);
+                .post_dispatch(_metadata, _message, required_amount);
         }
 
+        /// Quotes the dispatch fee for a given message. The hook to be selected will be based on 
+        /// the destination of the message input
+        /// 
+        /// # Arguments
+        /// 
+        /// * `_metadata` - Metadata for the hook
+        /// * `_message` - The message being dispatched
+        /// 
+        /// # Returns
+        /// 
+        /// u256 - The quoted fee for dispatching the 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<ContractState> {
+        /// Sets a hook for a specific destination domain
+        /// 
+        /// # Arguments
+        /// 
+        /// * `_destination` - The destination domain ID
+        /// * `_hook` - The address of the hook contract for this domain
         fn set_hook(ref self: ContractState, _destination: u32, _hook: ContractAddress) {
             self.ownable.assert_only_owner();
             self.hooks.write(_destination, IPostDispatchHookDispatcher { contract_address: _hook });
         }
+
+        /// Sets multiple hooks for different destination domains in a single call
+        /// 
+        /// # Arguments
+        /// 
+        /// * `configs` - An array of domain routing hook configurations
         fn set_hooks(ref self: ContractState, configs: Array<DomainRoutingHookConfig>) {
             self.ownable.assert_only_owner();
             let mut configs_span = configs.span();
@@ -127,13 +188,36 @@ pub mod domain_routing_hook {
                 };
             };
         }
+
+        /// Retrieves the hook address for a specific domain
+        /// 
+        /// # Arguments
+        /// 
+        /// * `domain` - The domain ID
+        /// 
+        /// # Returns
+        /// 
+        /// ContractAddress - The address of the hook for the specified domain
         fn get_hook(self: @ContractState, domain: u32) -> ContractAddress {
             self.hooks.read(domain).contract_address
         }
     }
 
     #[generate_trait]
     impl InternalImpl of InternalTrait {
+        /// Retrieves the configured hook for a given message's destination
+        /// 
+        /// # Arguments
+        /// 
+        /// * `_message` - The message to route
+        /// 
+        /// # Returns
+        /// 
+        /// IPostDispatchHookDispatcher - The dispatcher for the configured hook
+        /// 
+        /// # Errors
+        /// 
+        /// Reverts with `INVALID_DESTINATION` if no hook is configured for the destination
         fn _get_configured_hook(
             self: @ContractState, _message: Message
         ) -> IPostDispatchHookDispatcher {
@@ -145,6 +229,18 @@ pub mod domain_routing_hook {
             dispatcher_instance
         }
 
+        /// Transfers routing fees from the caller to the destination hook
+        /// 
+        /// # Arguments
+        /// 
+        /// * `from` - The address sending the fees
+        /// * `to` - The address receiving the fees
+        /// * `amount` - The amount of fees to transfer
+        /// 
+        /// # Errors
+        /// 
+        /// Reverts with `INSUFFICIENT_BALANCE` or `INSUFFICIENT_ALLOWANCE` respectively if the user balance/allowance 
+        /// does not mathc requirements
         fn _transfer_routing_fee_to_hook(
             ref self: ContractState, from: ContractAddress, to: ContractAddress, amount: u256
         ) {

cairo/crates/contracts/tests/hooks/test_domain_routing_hook.cairo

@@ -190,25 +190,6 @@ fn hook_set_for_destination_post_dispatch() {
 }
 
 
-#[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() {