Metalayer Contracts - Caldera

The MetalayerRouter contract inherits from OpenZeppelin's AccessControlUpgradeable library, relying on the DEFAULT_ADMIN_ROLE to manage critical functions. However, this role can be easily renounced, potentially leaving the contract without a designated administrator and rendering privileged functions (such as role assignment and system configurations) unusable. Additionally, the contract lacks a two-step process for securely transferring the DEFAULT_ADMIN_ROLE to a new admin, increasing the risk of role mismanagement or accidental loss of control.

Without sufficient safeguards, this setup risks accidental privilege renouncement or unauthorized role transfers.

As recommended by OpenZeppelin, consider using AccessControlDefaultAdminRulesUpgradeable instead for an extra layer of security. This contract implements the following risk mitigations on top of AccessControlUpgradeable:

• Only one account holds the DEFAULT_ADMIN_ROLE since deployment until it’s potentially renounced.

• Enforces a 2-step process to transfer the DEFAULT_ADMIN_ROLE to another account.

• Enforces a configurable delay between the two steps, with the ability to cancel before the transfer is accepted.

• The delay can be changed by scheduling, see changeDefaultAdminDelay.

• It is not possible to use another role to manage the DEFAULT_ADMIN_ROLE.

This approach ensures that only one account holds the DEFAULT_ADMIN_ROLE at a time and adds a security buffer for role transitions, reducing the risk of mismanagement and maintaining the integrity of privileged operations.

SOLVED: The Caldera team fixed this finding in commit 3474eac by switching to AccessControlDefaultAdminRulesUpgradeable as recommended.

The version() function within the MetalayerMessage library incorrectly parses the version byte from the Metalayer message payload. Current implementation:

library MetalayerMessage {
  using TypeCasts for bytes32;

  uint256 private constant VERSION_OFFSET = 0;
  uint256 private constant FINALITY_STATE_FLAG_OFFSET = 1;
  uint256 private constant NONCE_OFFSET = 2;
...

/**
  * @notice Returns the message version
  * @param _message ABI encoded Metalayer message
  * @return Version of `_message`
  */
function version(bytes calldata _message) internal pure returns (uint8) {
  return uint8(bytes1(_message[VERSION_OFFSET:NONCE_OFFSET]));
}

The slicing operation _message[VERSION_OFFSET:NONCE_OFFSET] incorrectly spans from byte offset 0 to byte offset 2 (2 bytes total), instead of only 1 byte intended for the version field. This could lead to incorrect interpretation of the message version. The intended version extraction should only take a single byte from the message payload.

Update the slicing operation in the version() function to correctly extract exactly one byte:

/**
  * @notice Returns the message version
  * @param _message ABI encoded Metalayer message
  * @return Version of `_message`
  */
function version(bytes calldata _message) internal pure returns (uint8) {
  return uint8(bytes1(_message[VERSION_OFFSET:FINALITY_STATE_FLAG_OFFSET]));
}

This change ensures that the version() function returns the correct message version byte, preventing potential decoding and handling errors.

SOLVED: The Caldera team fixed this finding in commit 3474eac by updating the slicing operation in the version() function as recommended.

The verify() function in MetalayerIsm assumes that the _metadata parameter is structured with a 65-byte ECDSA signature followed by additional data (a 4-byte length and the associated payload). However, the function does not enforce any validation of this structure beyond slicing off the first 65 bytes. This means that almost any _metadata input with a length of at least 65 bytes will be processed, potentially accepting malformed or unintended data formats.

See the current logic of the function:

function verify(bytes calldata _metadata, bytes calldata _message) external returns (bool verified) {
    address signer = offchainSigners[_message.origin()];
    require(signer != address(0), "No signer configured for origin");

    // The signature should be the first 65 bytes of the metadata.
    // We concat the metadata and the hyperlane message here to avoid replaying an api call to another message.
    verified = keccak256(bytes.concat(_metadata[65:], _message)).recover(_metadata[0:65]) == signer;

    if (verified) {
        responseMap[keccak256(_message.body())] = _metadata[65:];
    }
}

Consider adding explicit validation within verify() to enforce that _metadata conforms to the expected structure, e.g., ensuring it is at least 69 bytes long and that the length field correctly describes the size of the appended data. This improvement would help ensure that only properly formatted metadata is accepted, enhancing the integrity of subsequent processing.

SOLVED: The Caldera team fixed this finding in commit 4a069d5 by adding explicit validations within verify() as recommended.

The MetalayerRouter contract is exceeding the EVM's bytecode size limit (24,576 Bytes). See the output when running the command forge build --sizes --force, the MetalayerRouter contract is marked as too big:

╭-----------------------------+------------------+-------------------+--------------------+---------------------╮
| Contract                    | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) |
+===============================================================================================================+
...
|-----------------------------+------------------+-------------------+--------------------+---------------------|
| MetalayerRouter             | 28,402           | 29,119            | -3,826             | 20,033              |
|-----------------------------+------------------+-------------------+--------------------+---------------------|
...

To mitigate the issue, consider:

• Refactor Contract Design: Consider splitting the contract into smaller, composable modules or using external libraries. This can reduce bytecode duplication and improve maintainability.

• Optimize Compilation Settings: Adjust Solidity optimizer settings (e.g., --optimizer-runs) to explore different trade-offs between bytecode size and runtime efficiency.

• Use a Proxy Pattern: If the contract is intended to be upgradeable, a proxy pattern allows deploying minimal bytecode on-chain while storing the bulk of the logic in a separate implementation contract.

• Continuously Monitor Size: Regularly check contract size throughout development, using tooling like Foundry or Hardhat to catch and address growth early.

Addressing this issue is necessary to ensure successful deployment and maintain scalability of the system.

ACKNOWLEDGED: The Caldera team acknowledged the risk of this finding by not implementing any fix.

During the security assessment, it was identified that one function in the smart contracts may lack proper input validation, allowing a critical parameter to be set to arbitrary values. This can lead to potential vulnerabilities, unexpected behavior, or erroneous states within the contract.

Instances in MetalayerRouter.sol:

• The functions initialize(), dispatch() and setMetalayerIsm(), setMultisigIsm(), setRouter() and setIgp().

This list is not exhaustive. It is recommended to conduct a comprehensive review of the codebase to identify and assess other functions that may require additional input validation. Ensuring appropriate checks are in place for critical parameters will enhance the overall reliability, security, and predictability of the contracts.

To mitigate these issues, implement input validation in all constructor functions and other critical functions to ensure that inputs meet expected criteria. This can prevent unexpected behaviors and potential vulnerabilities.

SOLVED: The Caldera team fixed this finding in commit 3474eac by adding multiple input validations as recommended.

It was identified that the MetalayerIsm contract in scope inherit from OpenZeppelin's OwnableUpgradeable library. Ownership of the contracts that are inherited from the OwnableUpgradeable module can be lost, as the ownership is transferred in a single-step process.

The address that the ownership is changed to should be verified to be active or willing to act as the owner. Ownable2StepUpgradeable is safer than OwnableUpgradeable for smart contracts because the owner cannot accidentally transfer smart contract ownership to a mistyped address. Rather than directly transferring to the new owner, the transfer only completes when the new owner accepts ownership.

To mitigate the risks associated with single-step ownership transitions and enhance contract security, it is recommended to adopt a two-step ownership transition mechanism, such as OpenZeppelin's Ownable2StepUpgradeable. This approach introduces an additional step in the ownership transfer process, requiring the new owner to accept ownership before the transition is finalized. The process typically involves the current owner calling a function to nominate a new owner, and the nominee then calling another function to accept ownership.

Implementing Ownable2StepUpgradeable provides several benefits:

1. Reduces Risk of Accidental Loss of Ownership: By requiring explicit acceptance of ownership, the risk of accidentally transferring ownership to an incorrect or zero address is significantly reduced.

2. Enhanced Security: It adds another layer of security by ensuring that the new owner is prepared and willing to take over the responsibilities associated with contract ownership.

3. Flexibility in Ownership Transitions: Allows for a smoother transition of ownership, as the nominee has the opportunity to prepare for the acceptance of their new role.

By adopting Ownable2StepUpgradeable, contract administrators can ensure a more secure and controlled process for transferring ownership, safeguarding against the risks associated with accidental or unauthorized ownership changes.

SOLVED: The Caldera team fixed this finding in commit 4a069d5 by adopting the Ownable2StepUpgradeable module as recommended.

It was identified that the MetalayerIsm contract in scope inherits from OpenZeppelin's OwnableUpgradeable library. In the OwnableUpgradeable contracts, the renounceOwnership() function is used to renounce the Owner permission. Renouncing ownership before transferring would result in the contract having no owner, eliminating the ability to call privileged functions.

/**
 * @dev Leaves the contract without owner. It will not be possible to call
 * `onlyOwner` functions. Can only be called by the current owner.
 *
 * NOTE: Renouncing ownership will leave the contract without an owner,
 * thereby disabling any functionality that is only available to the owner.
 */
function renounceOwnership() public virtual onlyOwner {
    _transferOwnership(address(0));
}

It is recommended that the Owner cannot call renounceOwnership() without first transferring ownership to another address. In addition, if a multi-signature wallet is used, the call to the renounceOwnership() function should be confirmed for two or more users.

SOLVED: The Caldera team fixed this finding in commit 4a069d5 by overriding the renounceOwnership() function making it revert with CannotRenounceOwnership().

The MetalayerRouter contract currently inherits from both OwnableUpgradeable and AccessControlUpgradeable. However, no function within the contract makes use of the onlyOwner modifier provided by OwnableUpgradeable. Therefore, the inheritance from OwnableUpgradeable is redundant and constitutes legacy code.

Additionally, the explicit inheritance from Initializable is unnecessary because it is already inherited through AccessControlUpgradeable.

Current implementation snippet:

contract MetalayerRouter is
  Initializable,
  IMetalayerRouter,
  AbstractAggregationIsm,
  IMessageRecipient,
  ISpecifiesInterchainSecurityModule,
  OwnableUpgradeable,
  AccessControlUpgradeable
{

Redundant inheritance increases complexity and reduces clarity in the codebase, potentially leading to confusion or maintenance overhead.

Remove both the OwnableUpgradeable and Initializable inheritances from the MetalayerRouter contract declaration to simplify the contract and improve readability and maintainability. The updated contract declaration should look like:

contract MetalayerRouter is
  IMetalayerRouter,
  AbstractAggregationIsm,
  IMessageRecipient,
  ISpecifiesInterchainSecurityModule,
  AccessControlUpgradeable
{

SOLVED: The Caldera team fixed this finding in commit 3474eac by removing the OwnableUpgradeable and Initializable inheritances as recommended.

The MetalayerRouter contract inherits from AccessControlUpgradeable. The initialize() function, used to initialize the contract, includes calls to _setupRole() to assign roles such as DEFAULT_ADMIN_ROLE, ADMIN_ROLE, ISM_MANAGER_ROLE, ROUTER_MANAGER_ROLE and IGP_MANAGER_ROLE. However, according to OpenZeppelin's official module code, _setupRole() is deprecated in favor of _grantRole().

Using deprecated functions like _setupRole() can lead to potential issues with maintainability, compatibility with future versions of OpenZeppelin contracts, and adherence to current best practices. Deprecation indicates that the function may no longer be supported or could be removed in future library updates, which could lead to potential technical debt or security concerns.

Replace instances of _setupRole() with _grantRole() to ensure that the code aligns with OpenZeppelin's current best practices and remains compatible with future versions of the library.

SOLVED: The Caldera team fixed this finding in commit 3474eac by replacing instances of _setupRole() with _grantRole() as recommended.

The MetalayerIsm and MetalayerRouter smart contracts in scope are marked as unlicensed, as indicated by the SPDX license identifier in the file:

// SPDX-License-Identifier: UNLICENSED

Using unlicensed contracts can lead to legal uncertainties and potential conflicts regarding the usage, modification and distribution rights of the code. This may deter other developers from using or contributing to the project and could potentially lead to legal issues in the future.

Furthermore, it is worth mentioning that the mentioned files declare the License identifier in line 3 instead of the standard line 1.

Finally, it was also noted that the IMetalayerRouter.sol file in use (although out of scope), was missing a license declaration.

It is strongly recommended to apply the appropriate open-source license to the unlicensed smart contract and make sure the licensing is always at the top of the files.

SOLVED: The Caldera team fixed this finding in commit 3474eac by declaring a proper licensing as recommended.

The contracts in scope currently use a pragma version ^0.8.13, which means that the code can be compiled by any compiler version that is greater than or equal to 0.8.13 and less than 0.9.0. It is recommended that contracts should be deployed with the same compiler version and flags used during development and testing. Locking the pragma helps to ensure that contracts do not accidentally get deployed using another pragma. For example, an outdated pragma version might introduce bugs that affect the contract system negatively.

Lock the pragma version of all files to the same version used during development and testing.

SOLVED: The Caldera team fixed this finding in commit 3474eac by fixing the Solidity version to a recent version as recommended.

In programming, "magic numbers" refer to the use of hard-coded numerical or string values directly in the code without clear indication of their purpose or origin. The use of magic numbers can lead to confusion and make the codebase more difficult to understand, maintain, and update.

Throughout the contracts in scope, there are several instances where magic numbers are used. Magic numbers are direct numerical or string values used in the code without any explanation or context. This practice can lead to code maintainability issues, potential errors, and difficulty in understanding the code's logic.

Example code full of magic numbers in the MetalayerMessage library:

/**
  * @notice Returns all read operations from the message. Use this instead of getRead repeatedly if you want all operations as that is O(n) to find one read, O(n^2) total.
  * @param _message ABI encoded Metalayer message
  * @return Array of all read operations
  */
function reads(bytes calldata _message) internal pure returns (ReadOperation[] memory) {
  uint256 numReads = readCount(_message);
  ReadOperation[] memory tempReads = new ReadOperation[](numReads);

  bytes calldata messageBody = body(_message);
  uint256 currentOffset = READS_OFFSET;

  for (uint256 i = 0; i < numReads; i++) {
    uint32 sourceChainId = uint32(bytes4(messageBody[currentOffset:currentOffset + 4]));
    address sourceContract = address(bytes20(messageBody[currentOffset + 4:currentOffset + 24]));
    uint256 callDataLength = uint32(bytes4(messageBody[currentOffset + 24:currentOffset + 28]));
    bytes calldata callData = messageBody[currentOffset + 28:currentOffset + 28 + callDataLength];

    tempReads[i] = ReadOperation({domain: sourceChainId, target: sourceContract, callData: callData});

    currentOffset += 28 + callDataLength; // 4 (domain) + 20 (contract) + 4 (length) + callDataLength
  }

  return tempReads;
}

/**
  * @notice Returns the write call data from the message
  * @param _message ABI encoded Metalayer message
  * @return The write call data
  */
function writeCallData(bytes calldata _message) internal pure returns (bytes calldata) {
  bytes calldata messageBody = body(_message);
  uint256 currentOffset = READS_OFFSET;

  uint256 numReads = readCount(_message);
  for (uint256 i = 0; i < numReads; i++) {
    uint256 callDataLength = uint32(bytes4(messageBody[currentOffset + 24:currentOffset + 28]));
    currentOffset += 28 + callDataLength;
  }

  return messageBody[currentOffset:];
}

To improve the code's readability and facilitate refactoring, consider defining a constant for every magic number, giving it a clear and self-explanatory name. Consider adding an inline comment explaining how the magic numbers are calculated or why they are chosen for complex values.

ACKNOWLEDGED: The Caldera team acknowledged the risk of this finding.

As per the foundry.toml file, the project uses the Solidity compiler version 0.8.26, which supports named mappings. Using named mappings can improve the readability and maintainability of the code by making the purpose of each mapping clearer. This practice helps developers and auditors understand the mappings' intent more easily.

Consider refactoring the mappings to use named arguments, which will enhance code readability and make the purpose of each mapping more explicit. For example:

mapping(uint32 chainDomainID => address routerAddress) public routerAddresses;

SOLVED: The Caldera team fixed this finding in commit 3474eac by using named mappings as recommended.

In Solidity smart contract development, replacing hard-coded revert message strings with the Error() syntax is an optimization strategy that can significantly reduce gas costs. Hard-coded strings, stored on the blockchain, increase the size and cost of deploying and executing contracts.

The Error() syntax allows for the definition of reusable, parameterized custom errors, leading to a more efficient use of storage and reduced gas consumption. This approach not only optimizes gas usage during deployment and interaction with the contract but also enhances code maintainability and readability by providing clearer, context-specific error information.

It is recommended to replace hard-coded revert strings in require statements for custom errors, which can be done following the logic below:

1. Standard require statement (to be replaced):

require(condition, "Condition not met");

2. Declare the error definition to state:

error ConditionNotMet();

3. As currently is not possible to use custom errors in combination with require statements, the standard syntax is:

if (!condition) revert ConditionNotMet();

More information about this topic in the official Solidity documentation.

ACKNOWLEDGED: The Caldera team acknowledged the risk of this finding by keeping some errors with hard-coded strings.

The project codebase contains several stylistic inconsistencies and deviations from Solidity best practices, which, while not directly impacting functionality, reduce code readability, maintainability, and adherence to standard conventions. Addressing these inconsistencies can enhance the clarity and professionalism of the code.

Examples:

• Unused named return variables: Notice the following example is declaring a fee variable as a named return that is not used.

function quoteDispatch(
    uint32 destinationDomain,
    bytes32 recipientAddress,
    bytes calldata messageBody,
    bytes memory defaultHookMetadata
) external view returns (uint256 fee) {
    return quoteDispatch(destinationDomain, recipientAddress, messageBody, defaultHookMetadata, igp);
}

• Use of public Where external Could Be Used: The initialize() function is declared as public even though it could be declared as external to potentially save gas and adhere to best practices:

function initialize(
    IMailbox _mailbox,
    address _multisigIsm,
    address _multisigIsmFinalized,
    MetalayerIsm _metalayerIsm,
    uint32[] memory chains,
    address[] memory routers,
    address _owner,
    IPostDispatchHook _igp
) public initializer {

Consider implementing the style improvements highlighted above to enhance the readability and consistency of the project.

SOLVED: The Caldera team fixed this finding in commit 3474eac by implementing the style recommendations.

It has been observed that some functionalities are missing emitting events. Events are a method of informing the transaction initiator about the actions taken by the called function. It logs its emitted parameters in a specific log history, which can be accessed outside of the contract using some filter parameters. Events help non-contract tools to track changes, and events prevent users from being surprised by changes.

Instances in MetalayerIsm.sol:

• The functions initialize() and verify(), setOffchainSigner() and setOffchainApiUrls().

Instances in MetalayerRouter.sol:

• The functions initialize(), dispatch() and setMetalayerIsm(), setMultisigIsm(), setRouter() and setIgp().

This list is not exhaustive, and it is recommended to review the entire codebase to identify additional instances where event emissions may be beneficial. A thorough analysis should be conducted to determine whether adding event emissions aligns with the intended design and provides value in tracking state changes.

All functions updating important parameters should emit events.

SOLVED: The Caldera team fixed this finding in commit 3474eac by emitting events as recommended.

It is best practice to set the visibility of state variables and constants explicitly. In the MetalayerRouter contract, a global state variable does not have the visibility set:

IPostDispatchHook igp;

Consider explicitly setting the visibility of all state variables and constants. More specifically, the code could be updated to:

IPostDispatchHook public igp;

SOLVED: The Caldera team fixed this finding in commit 3474eac by adding visibility attributes as recommended.

The MetalayerIsm.sol file is importing a file that is actually not needed:

import {MetalayerMessage} from "./lib/MetalayerMessage.sol";

Consider removing any redundant import to simplify the codebase.

SOLVED: The Caldera team fixed this finding in commit 4a069d5 by removing the unused import as recommended.

Some functions in the contracts in scope use memory arrays as arguments, even though the array is not mutated in the external function itself. This results in unnecessary gas overhead when copying data from calldata to memory during the abi.decode() process.

Using calldata directly for such arguments bypasses the copying loop, reducing gas costs, especially for larger arrays. Optimizing arrays as calldata instead of memory can reduce gas costs for external calls. The savings grow with the size of the input array. Example: The the _origins, _signers and _offchainApiUrls arguments of initialize() in MetalayerIsm.

A thorough review of the entire codebase is recommended to identify additional instances where memory arguments can be changed to calldata to save gas.

Consider updating the function signatures as follows:

function initialize(
  IMailbox _mailbox,
  uint32[] calldata _origins,
  address[] calldata _signers,
  string[] calldata _offchainApiUrls,
  address _owner
) public initializer {

By switching the argument type to calldata, the _origins, _signers and _offchainApiUrls arguments are read directly from the transaction's calldata, eliminating unnecessary memory allocations and reducing gas costs.

SOLVED: The Caldera team fixed this finding in commit 4a069d5 by using calldata whenever possible as recommended.

Although much of the code under review maintains a generally consistent NatSpec standard, there are notable gaps and inconsistencies in the documentation. Specifically:

In the MetalayerRouter contract. The processedAt() function has a wrong @notice information, which is actually from the function above, processor():

/**
  * @notice Returns the account that processed the message.
  * @param _id The message ID to check.
  * @return The number of the block that the message was processed at.
  */
function processedAt(bytes32 _id) external view returns (uint48) {
  return deliveries[_id].blockNumber;
}

Another instance of a misleading NatSpec comment is in the handle() function. The _origin parameter currently reads:

/**
  * @notice Handles a message from the Hyperlane mailbox and forwards it to the actual recipient.
  * @param _origin Domain of destination chain
  * @param _sender Sender of the message (should be a metalayer router)
  * @param _message The metalayer message
  */
function handle(

In the MetalayerMessage library, the NatSpec of senderAddress() and recipientAddress() say they return an address, but they are actually returning a bytes32:

/**
  * @notice Returns the message sender as address
  * @param _message ABI encoded Metalayer message
  * @return Sender of `_message` as address
  */
function senderAddress(bytes calldata _message) internal pure returns (bytes32) {
  return bytes32(_message[SENDER_OFFSET:DESTINATION_OFFSET]);
}

...

/**
  * @notice Returns the message recipient as address. We only support evm chains for now, so address only.
  * @param _message ABI encoded Metalayer message
  * @return Recipient of `_message` as address
  */
function recipientAddress(bytes calldata _message) internal pure returns (bytes32) {
  return bytes32(_message[RECIPIENT_OFFSET:BODY_OFFSET]);
}

Finally, the functions setOffchainSigner(), setOffchainApiUrls(), process() in MetalayerIsm are missing the NatSpec documentation.

Ensure all functions and parameters have complete and accurate NatSpec documentation, following a consistent style throughout the codebase.

SOLVED: The Caldera team fixed this finding in commit 4a069d5 by improving the NatSpec as recommended.

An empty revert() statement was found (without error message) in the MetalayerRouter contract:

receive() external payable {
    revert();
}

It is recommended to always use descriptive reason strings or custom errors for revert paths. For this particular instance, consider if the affected functionality is necessary at all.

SOLVED: The Caldera team fixed this finding in commit 3474eac by removing the function with the empty revert statement.

In the function _buildWrappedMessage(), a named return variable message is declared in the function signature and then shadowed by a local variable with the same name within the function body. This creates confusion about the scope and lifetime of the variable, potentially leading to misunderstanding or maintenance errors.

Current implementation:

function _buildWrappedMessage(uint32 destinationDomain, bytes32 recipientAddress, bytes calldata messageBody)
    internal
    view
    returns (bytes memory message, bytes32 messageId)
{
    bytes memory message = _buildMessage(destinationDomain, recipientAddress, messageBody);
    return (
        MetalayerMessage.formatMessageWithReads(
            WRAPPER_VERSION,
            FinalityState.INSTANT,
            nonce,
            mailbox.localDomain(),
            msg.sender.addressToBytes32(),
            destinationDomain,
            recipientAddress,
            new ReadOperation[](0),
            message
        ),
        Message.id(message)
    );
}

Shadowing named return variables with local variables reduces code clarity, making the code harder to read and potentially prone to errors during maintenance or future updates.

Consider removing the use of named returns in the affected function or rename the local variable to avoid shadowing. A clear and recommended approach is to eliminate the named return, simplifying the code:

function _buildWrappedMessage(uint32 destinationDomain, bytes32 recipientAddress, bytes calldata messageBody)
    internal
    view
    returns (bytes memory, bytes32)
{
    bytes memory message = _buildMessage(destinationDomain, recipientAddress, messageBody);
    return (
        MetalayerMessage.formatMessageWithReads(
            WRAPPER_VERSION,
            FinalityState.INSTANT,
            nonce,
            mailbox.localDomain(),
            msg.sender.addressToBytes32(),
            destinationDomain,
            recipientAddress,
            new ReadOperation[](0),
            message
        ),
        Message.id(message)
    );
}

SOLVED: The Caldera team fixed this finding in commit 3474eac by removing the use of named returns to avoid the local variable shadowing as recommended.

During the security assessment, a commented out line of code in the quoteDispatch() function was found (presumably from old versions of the code).

function quoteDispatch(
    uint32 destinationDomain,
    bytes32 recipientAddress,
    bytes calldata messageBody,
    bytes memory metadata,
    IPostDispatchHook hook
) public view returns (uint256 fee) {
  ...
  return hook
    // Omit the required hook here as it will always be a fee-free one for the ones we deploy
@>  //requiredHook.quoteDispatch(metadata, message) +
    .quoteDispatch(metadata, message);
}

It is recommended to remove any commented out code that does not need to go in production.

SOLVED: The Caldera team fixed this finding in commit 3474eac by removing the commented out code as recommended.

During the security assessment, a typographical error was found in a comment.

* @notice Computes quote for dipatching a message to the destination domain & recipient

The word dipatching should be dispatching instead.

To maintain clarity and trustworthiness, it is essential to rectify any typographical errors present within the contracts. Correcting such errors minimizes the likelihood of confusion and reinforces confidence in the accuracy and integrity of the documentation.

ACKNOWLEDGED: The Caldera team acknowledged the risk of this finding.

The readCount() function in the MetalayerMessage library casts a 4-byte segment from the message body to a uint32, but the function itself returns a uint256. This inconsistency in casting and return type can lead to confusion and potential misinterpretation of the code. Current implementation:

/**
  * @notice Returns the number of read operations in the message
  * @param _message ABI encoded Metalayer message
  * @return Number of read operations
  */
function readCount(bytes calldata _message) internal pure returns (uint256) {
  return uint32(bytes4(body(_message)[READ_COUNT_OFFSET:READS_OFFSET]));
}

Here, the return type is uint256, but the actual value extracted is explicitly a uint32. While this compiles and functions correctly in Solidity (due to implicit widening), it introduces inconsistency that reduces code clarity.

To improve code clarity and consistency, consider one of the following changes:

Option 1 – Match return type with the value being extracted:

function readCount(bytes calldata _message) internal pure returns (uint32) {
  return uint32(bytes4(body(_message)[READ_COUNT_OFFSET:READS_OFFSET]));
}

Option 2 – Explicitly cast the uint32 to uint256 for clarity:

function readCount(bytes calldata _message) internal pure returns (uint256) {
  return uint256(uint32(bytes4(body(_message)[READ_COUNT_OFFSET:READS_OFFSET])));
}

Either option improves consistency and reduces ambiguity, with Option 1 being preferred to match other functions from the library and reduce one casting (to uint256).

SOLVED: The Caldera team fixed this finding in commit 3474eac by returning a uint32 instead of uint256.

When the length of an array is not cached outside of a loop, the Solidity compiler reads the length of the array during each iteration. For storage arrays, this results in an extra sload operation (100 additional gas for each iteration except the first). For memory arrays, this results in an extra mload operation (3 additional gas for each iteration except the first).

Detecting loops that use the length member of a storage array in their loop condition without modifying it can reveal opportunities for optimization. See the following example in MetalayerMessage.sol:

function formatMessageWithReads(
    uint8 _version,
    FinalityState _finalityState,
    uint32 _nonce,
    uint32 _originDomain,
    bytes32 _sender,
    uint32 _destinationDomain,
    bytes32 _recipient,
    ReadOperation[] memory _reads,
    bytes memory _writeCallData
) internal pure returns (bytes memory) {
    bytes memory messageBody = abi.encodePacked(uint32(_reads.length));

    // this is n^2 -> optimize later by just preallocating the appropriate size
    // can keep slow for test
    for (uint256 i = 0; i < _reads.length; i++) {

Cache the length of the (storage and memory) arrays in a local variable outside the loop to optimize gas usage. This reduces the number of read operations required during each iteration of the loop. See the example fix below:

function formatMessageWithReads(
  uint8 _version,
  FinalityState _finalityState,
  uint32 _nonce,
  uint32 _originDomain,
  bytes32 _sender,
  uint32 _destinationDomain,
  bytes32 _recipient,
  ReadOperation[] memory _reads,
  bytes memory _writeCallData
) internal pure returns (bytes memory) {
  uint256 _readsLength = _reads.length;
  bytes memory messageBody = abi.encodePacked(uint32(_readsLength));

  // this is n^2 -> optimize later by just preallocating the appropriate size
  // can keep slow for test
  for (uint256 i = 0; i < _readsLength; i++) {

SOLVED: The Caldera team fixed this finding in commit 3474eac by caching the lengths of the storage and memory arrays as recommended.

The MetalayerRouter contract records the processing of messages by storing block.number as part of the delivery record. While this provides an effective measure of ordering on a given chain, it is important to note that the meaning of block.number can vary significantly between Layer 2 networks.

• On some L2s, such as Optimism, block.number reflects the L2-specific block height, whereas on others (e.g., Arbitrum) it corresponds to an L1 value.

• In cross-chain contexts, this discrepancy might lead to inconsistencies if block.number is later used as a time or progress indicator across multiple chains.

• Currently, the recorded block number primarily serves to log the order of processed messages on a single chain rather than as an absolute measure of time, minimizing immediate risk.

This behavior should be acknowledged and documented for future integrations. Teams planning on leveraging block numbers for any timing or comparative purposes across different chains should consider the network-specific semantics of block.number and evaluate whether an alternative metric (like block.timestamp alongside proper safeguards) would also be appropriate.

Overall, while there is no immediate security risk, the implications of using block.number across diverse chain environments should be clearly understood and factored into any future design decisions.

ACKNOWLEDGED: The Caldera team acknowledged this finding.

The verify() function in MetalayerIsm only supports signature recovery using plain ECDSA (via recover()) and does not accommodate multisig smart contract wallets following the ERC-1271 standard.

Details:

• The verification logic extracts the first 65 bytes from the _metadata parameter and recovers the signer using OpenZeppelin’s ECDSA.recover().

• This approach works for standard externally owned accounts (EOAs) but does not support contract-based wallets that use ERC-1271 for signature validation.

• As a result, messages signed by multisig wallets or other contract wallets implementing ERC-1271 may be erroneously rejected.

Consider extending the verify() functionality to support ERC-1271 signature verification. This enhancement would allow the contract to verify signatures from multisignature wallets and other contract-based accounts, increasing the flexibility and robustness of the system.

ACKNOWLEDGED: The Caldera team acknowledged the risk of this finding.