diff --git a/solidity/contracts/Mailbox.sol b/solidity/contracts/Mailbox.sol index 6cfec22af4..59b0d63b18 100644 --- a/solidity/contracts/Mailbox.sol +++ b/solidity/contracts/Mailbox.sol @@ -392,20 +392,23 @@ contract Mailbox is IMailbox, Indexed, Versioned, OwnableUpgradeable { view returns (IInterchainSecurityModule) { - // Use a default interchainSecurityModule if one is not specified by the - // recipient. - // This is useful for backwards compatibility and for convenience as - // recipients are not mandated to specify an ISM. - try - ISpecifiesInterchainSecurityModule(_recipient) - .interchainSecurityModule() - returns (IInterchainSecurityModule _val) { - // If the recipient specifies a zero address, use the default ISM. - if (address(_val) != address(0)) { - return _val; + // use low-level staticcall in case of revert or empty return data + (bool success, bytes memory returnData) = _recipient.staticcall( + abi.encodeCall( + ISpecifiesInterchainSecurityModule.interchainSecurityModule, + () + ) + ); + // check if call was successful and returned data + if (success && returnData.length != 0) { + // check if returnData is a valid address + address ism = abi.decode(returnData, (address)); + // check if the ISM is a contract + if (ism != address(0)) { + return IInterchainSecurityModule(ism); } - // solhint-disable-next-line no-empty-blocks - } catch {} + } + // Use the default if a valid one is not specified by the recipient. return defaultIsm; } diff --git a/solidity/test/Mailbox.t.sol b/solidity/test/Mailbox.t.sol index e408b4b01d..fe50aa279b 100644 --- a/solidity/test/Mailbox.t.sol +++ b/solidity/test/Mailbox.t.sol @@ -13,6 +13,12 @@ import "../contracts/hooks/MerkleTreeHook.sol"; import {StandardHookMetadata} from "../contracts/hooks/libs/StandardHookMetadata.sol"; import {TypeCasts} from "../contracts/libs/TypeCasts.sol"; +contract Empty {} + +contract EmptyFallback { + fallback() external {} +} + contract MailboxTest is Test, Versioned { using StandardHookMetadata for bytes; using TypeCasts for address; @@ -78,7 +84,24 @@ contract MailboxTest is Test, Versioned { IInterchainSecurityModule ism = mailbox.recipientIsm( address(recipient) ); - assertEq(address(mailbox.defaultIsm()), address(ism)); + assertEq(address(defaultIsm), address(ism)); + + // check no ism function returns default + Empty empty = new Empty(); + ism = mailbox.recipientIsm(address(empty)); + assertEq(address(defaultIsm), address(ism)); + + // check empty fallback returns default + EmptyFallback emptyFallback = new EmptyFallback(); + ism = mailbox.recipientIsm(address(emptyFallback)); + assertEq(address(defaultIsm), address(ism)); + + // check zero address returns default + recipient.setInterchainSecurityModule(address(0)); + ism = mailbox.recipientIsm(address(recipient)); + assertEq(address(defaultIsm), address(ism)); + + // check recipient override is used TestIsm newIsm = new TestIsm(); recipient.setInterchainSecurityModule(address(newIsm)); ism = mailbox.recipientIsm(address(recipient));