Contracts: Model custom errors (#81)

Co-authored-by: Facu Spagnuolo <[email protected]>

packages/authorizer/contracts/Authorized.sol

@@ -84,7 +84,7 @@ contract Authorized is IAuthorized, Initializable, AuthorizedHelpers {
      * @param how Params to be authenticated
      */
     function _authenticate(address who, bytes4 what, uint256[] memory how) internal view {
-        require(_isAuthorized(who, what, how), 'AUTH_SENDER_NOT_ALLOWED');
+        if (!_isAuthorized(who, what, how)) revert AuthSenderNotAllowed(who, what, how);
     }
 
     /**

packages/authorizer/contracts/Authorizer.sol

@@ -164,8 +164,7 @@ contract Authorizer is IAuthorizer, AuthorizedHelpers, Initializable, Reentrancy
      */
     function authorize(address who, address where, bytes4 what, Param[] memory params) public override nonReentrant {
         uint256[] memory how = authParams(who, where, what);
-        bool allowed = isAuthorized(msg.sender, address(this), IAuthorizer.authorize.selector, how);
-        require(allowed, 'AUTHORIZER_SENDER_NOT_ALLOWED');
+        _authenticate(msg.sender, IAuthorizer.authorize.selector, how);
         _authorize(who, where, what, params);
     }
 
@@ -177,11 +176,21 @@ contract Authorizer is IAuthorizer, AuthorizedHelpers, Initializable, Reentrancy
      */
     function unauthorize(address who, address where, bytes4 what) public override nonReentrant {
         uint256[] memory how = authParams(who, where, what);
-        bool allowed = isAuthorized(msg.sender, address(this), IAuthorizer.unauthorize.selector, how);
-        require(allowed, 'AUTHORIZER_SENDER_NOT_ALLOWED');
+        _authenticate(msg.sender, IAuthorizer.unauthorize.selector, how);
         _unauthorize(who, where, what);
     }
 
+    /**
+     * @dev Validates whether `who` is authorized to call `what` with `how`
+     * @param who Address asking permission for
+     * @param what Function selector asking permission for
+     * @param how Params asking permission for
+     */
+    function _authenticate(address who, bytes4 what, uint256[] memory how) internal view {
+        bool allowed = isAuthorized(who, address(this), what, how);
+        if (!allowed) revert AuthorizerSenderNotAllowed(who, address(this), what, how);
+    }
+
     /**
      * @dev Tells whether `who` is allowed to call `what` on `where` with `how`
      * @param who Address asking permission for
@@ -255,6 +264,6 @@ contract Authorizer is IAuthorizer, AuthorizedHelpers, Initializable, Reentrancy
         if (Op(param.op) == Op.LT) return how < param.value;
         if (Op(param.op) == Op.GTE) return how >= param.value;
         if (Op(param.op) == Op.LTE) return how <= param.value;
-        revert('AUTHORIZER_INVALID_PARAM_OP');
+        revert AuthorizerInvalidParamOp(param.op);
     }
 }

packages/authorizer/contracts/interfaces/IAuthorized.sol

@@ -18,6 +18,11 @@ pragma solidity >=0.8.0;
  * @dev Authorized interface
  */
 interface IAuthorized {
+    /**
+     * @dev Sender `who` is not allowed to call `what` with `how`
+     */
+    error AuthSenderNotAllowed(address who, bytes4 what, uint256[] how);
+
     /**
      * @dev Tells the address of the authorizer reference
      */

packages/authorizer/contracts/interfaces/IAuthorizer.sol

@@ -61,6 +61,16 @@ interface IAuthorizer {
         uint248 value;
     }
 
+    /**
+     * @dev Sender is not authorized to call `what` on `where` with `how`
+     */
+    error AuthorizerSenderNotAllowed(address who, address where, bytes4 what, uint256[] how);
+
+    /**
+     * @dev The operation param is invalid
+     */
+    error AuthorizerInvalidParamOp(uint8 op);
+
     /**
      * @dev Emitted every time `who`'s permission to perform `what` on `where` is granted with `params`
      */

packages/authorizer/test/Authorizer.test.ts

@@ -385,17 +385,17 @@ describe('Authorizer', () => {
 
                 // try granting other permissions
                 await expect(authorizer.connect(who).authorize(WHO, WHERE2, WHAT, [])).to.be.revertedWith(
-                  'AUTHORIZER_SENDER_NOT_ALLOWED'
+                  'AuthorizerSenderNotAllowed'
                 )
                 await expect(authorizer.connect(who).authorize(WHO, WHERE, WHAT2, [])).to.be.revertedWith(
-                  'AUTHORIZER_SENDER_NOT_ALLOWED'
+                  'AuthorizerSenderNotAllowed'
                 )
 
                 // rollback authorization
                 await authorizer.unauthorize(who.address, authorizer.address, authorizeRole)
                 expect(await authorizer.isAuthorized(who.address, authorizer.address, authorizeRole, [])).to.be.false
                 await expect(authorizer.connect(who).authorize(WHO, WHERE, WHAT, [])).to.be.revertedWith(
-                  'AUTHORIZER_SENDER_NOT_ALLOWED'
+                  'AuthorizerSenderNotAllowed'
                 )
               })
             })
@@ -437,7 +437,7 @@ describe('Authorizer', () => {
 
     context('when the sender is not allowed', () => {
       it('reverts', async () => {
-        await expect(authorizer.authorize(WHO, WHERE, WHAT, [])).to.be.revertedWith('AUTHORIZER_SENDER_NOT_ALLOWED')
+        await expect(authorizer.authorize(WHO, WHERE, WHAT, [])).to.be.revertedWith('AuthorizerSenderNotAllowed')
       })
     })
   })
@@ -502,7 +502,7 @@ describe('Authorizer', () => {
 
     context('when the sender is not allowed', () => {
       it('reverts', async () => {
-        await expect(authorizer.unauthorize(WHO, WHERE, WHAT)).to.be.revertedWith('AUTHORIZER_SENDER_NOT_ALLOWED')
+        await expect(authorizer.unauthorize(WHO, WHERE, WHAT)).to.be.revertedWith('AuthorizerSenderNotAllowed')
       })
     })
   })
@@ -560,7 +560,7 @@ describe('Authorizer', () => {
           authorizer
             .connect(admin)
             .changePermissions([{ where: WHERE2, grants: [{ who: WHO2, what: WHAT2, params: [] }], revokes: [] }])
-        ).to.be.revertedWith('AUTHORIZER_SENDER_NOT_ALLOWED')
+        ).to.be.revertedWith('AuthorizerSenderNotAllowed')
       })
     })
   })

packages/connectors/contracts/bridge/axelar/AxelarConnector.sol

@@ -26,6 +26,26 @@ import './IAxelarGateway.sol';
  * @dev Interfaces with Axelar to bridge tokens
  */
 contract AxelarConnector {
+    /**
+     * @dev The recipient address is zero
+     */
+    error AxelarBridgeRecipientZero();
+
+    /**
+     * @dev The source and destination chains are the same
+     */
+    error AxelarBridgeSameChain(uint256 chainId);
+
+    /**
+     * @dev The chain ID is not supported
+     */
+    error AxelarBridgeUnknownChainId(uint256 chainId);
+
+    /**
+     * @dev The post token balance is lower than the previous token balance minus the amount bridged
+     */
+    error AxelarBridgeBadPostTokenBalance(uint256 postBalance, uint256 preBalance, uint256 amount);
+
     // List of chain names supported by Axelar
     string private constant ETHEREUM_NAME = 'Ethereum';
     string private constant POLYGON_NAME = 'Polygon';
@@ -61,19 +81,19 @@ contract AxelarConnector {
      * @param recipient Address that will receive the tokens on the destination chain
      */
     function execute(uint256 chainId, address token, uint256 amountIn, address recipient) external {
-        require(block.chainid != chainId, 'AXELAR_BRIDGE_SAME_CHAIN');
-        require(recipient != address(0), 'AXELAR_BRIDGE_RECIPIENT_ZERO');
+        if (block.chainid == chainId) revert AxelarBridgeSameChain(chainId);
+        if (recipient == address(0)) revert AxelarBridgeRecipientZero();
 
         string memory chainName = _getChainName(chainId);
         string memory symbol = IERC20Metadata(token).symbol();
-
         uint256 preBalanceIn = IERC20(token).balanceOf(address(this));
 
         ERC20Helpers.approve(token, address(axelarGateway), amountIn);
         axelarGateway.sendToken(chainName, Strings.toHexString(recipient), symbol, amountIn);
 
         uint256 postBalanceIn = IERC20(token).balanceOf(address(this));
-        require(postBalanceIn >= preBalanceIn - amountIn, 'AXELAR_BAD_TOKEN_IN_BALANCE');
+        bool isPostBalanceInUnexpected = postBalanceIn < preBalanceIn - amountIn;
+        if (isPostBalanceInUnexpected) revert AxelarBridgeBadPostTokenBalance(postBalanceIn, preBalanceIn, amountIn);
     }
 
     /**
@@ -88,6 +108,6 @@ contract AxelarConnector {
         else if (chainId == BSC_ID) return BSC_NAME;
         else if (chainId == FANTOM_ID) return FANTOM_NAME;
         else if (chainId == AVALANCHE_ID) return AVALANCHE_NAME;
-        else revert('AXELAR_UNKNOWN_CHAIN_ID');
+        else revert AxelarBridgeUnknownChainId(chainId);
     }
 }

packages/connectors/contracts/bridge/connext/ConnextConnector.sol

@@ -25,6 +25,36 @@ import './IConnext.sol';
  * @dev Interfaces with Connext to bridge tokens
  */
 contract ConnextConnector {
+    /**
+     * @dev The recipient address is zero
+     */
+    error ConnextBridgeRecipientZero();
+
+    /**
+     * @dev The source and destination chains are the same
+     */
+    error ConnextBridgeSameChain(uint256 chainId);
+
+    /**
+     * @dev The chain ID is not supported
+     */
+    error ConnextBridgeUnknownChainId(uint256 chainId);
+
+    /**
+     * @dev The relayer fee is greater than the amount to be bridged
+     */
+    error ConnextBridgeRelayerFeeGtAmount(uint256 relayerFee, uint256 amountIn);
+
+    /**
+     * @dev The minimum amount out is greater than the amount to be bridged minus the relayer fee
+     */
+    error ConnextBridgeMinAmountOutTooBig(uint256 minAmountOut, uint256 amountIn, uint256 relayerFee);
+
+    /**
+     * @dev The post token balance is lower than the previous token balance minus the amount bridged
+     */
+    error ConnextBridgeBadPostTokenBalance(uint256 postBalance, uint256 preBalance, uint256 amount);
+
     // List of chain domains supported by Connext
     uint32 private constant ETHEREUM_DOMAIN = 6648936;
     uint32 private constant POLYGON_DOMAIN = 1886350457;
@@ -69,10 +99,12 @@ contract ConnextConnector {
         address recipient,
         uint256 relayerFee
     ) external {
-        require(block.chainid != chainId, 'CONNEXT_BRIDGE_SAME_CHAIN');
-        require(recipient != address(0), 'CONNEXT_BRIDGE_RECIPIENT_ZERO');
-        require(relayerFee <= amountIn, 'CONNEXT_RELAYER_FEE_GT_AMOUNT_IN');
-        require(minAmountOut <= amountIn - relayerFee, 'CONNEXT_MIN_AMOUNT_OUT_TOO_BIG');
+        if (block.chainid == chainId) revert ConnextBridgeSameChain(chainId);
+        if (recipient == address(0)) revert ConnextBridgeRecipientZero();
+        if (relayerFee > amountIn) revert ConnextBridgeRelayerFeeGtAmount(relayerFee, amountIn);
+
+        bool isMinAmountTooBig = minAmountOut > amountIn - relayerFee;
+        if (isMinAmountTooBig) revert ConnextBridgeMinAmountOutTooBig(minAmountOut, amountIn, relayerFee);
 
         uint32 domain = _getChainDomain(chainId);
         uint256 amountInAfterFees = amountIn - relayerFee;
@@ -96,7 +128,8 @@ contract ConnextConnector {
         );
 
         uint256 postBalanceIn = IERC20(token).balanceOf(address(this));
-        require(postBalanceIn >= preBalanceIn - amountIn, 'CONNEXT_BAD_TOKEN_IN_BALANCE');
+        bool isPostBalanceInUnexpected = postBalanceIn < preBalanceIn - amountIn;
+        if (isPostBalanceInUnexpected) revert ConnextBridgeBadPostTokenBalance(postBalanceIn, preBalanceIn, amountIn);
     }
 
     /**
@@ -111,6 +144,6 @@ contract ConnextConnector {
         else if (chainId == OPTIMISM_ID) return OPTIMISM_DOMAIN;
         else if (chainId == GNOSIS_ID) return GNOSIS_DOMAIN;
         else if (chainId == BSC_ID) return BSC_DOMAIN;
-        else revert('CONNEXT_UNKNOWN_CHAIN_ID');
+        else revert ConnextBridgeUnknownChainId(chainId);
     }
 }

packages/connectors/contracts/bridge/hop/HopConnector.sol

@@ -33,6 +33,41 @@ contract HopConnector {
     using FixedPoint for uint256;
     using Denominations for address;
 
+    /**
+     * @dev The source and destination chains are the same
+     */
+    error HopBridgeSameChain(uint256 chainId);
+
+    /**
+     * @dev The bridge operation is not supported
+     */
+    error HopBridgeOpNotSupported();
+
+    /**
+     * @dev The recipient address is zero
+     */
+    error HopBridgeRecipientZero();
+
+    /**
+     * @dev The relayer was sent when not needed
+     */
+    error HopBridgeRelayerNotNeeded();
+
+    /**
+     * @dev The deadline was sent when not needed
+     */
+    error HopBridgeDeadlineNotNeeded();
+
+    /**
+     * @dev The deadline is in the past
+     */
+    error HopBridgePastDeadline(uint256 deadline, uint256 currentTimestamp);
+
+    /**
+     * @dev The post token balance is lower than the previous token balance minus the amount bridged
+     */
+    error HopBridgeBadPostTokenBalance(uint256 postBalance, uint256 preBalance, uint256 amount);
+
     // Ethereum mainnet chain ID = 1
     uint256 private constant MAINNET_CHAIN_ID = 1;
 
@@ -66,7 +101,7 @@ contract HopConnector {
      * @param recipient Address that will receive the tokens on the destination chain
      * @param bridge Address of the bridge component (i.e. hopBridge or hopAMM)
      * @param deadline Deadline to be used when bridging to L2 in order to swap the corresponding hToken
-     * @param relayer Only used when transfering from L1 to L2 if a 3rd party is relaying the transfer on the user's behalf
+     * @param relayer Only used when transferring from L1 to L2 if a 3rd party is relaying the transfer on the user's behalf
      * @param fee Fee to be sent to the bridge based on the source and destination chain (i.e. relayerFee or bonderFee)
      */
     function execute(
@@ -80,26 +115,26 @@ contract HopConnector {
         address relayer,
         uint256 fee
     ) external {
-        require(block.chainid != chainId, 'HOP_BRIDGE_SAME_CHAIN');
-        require(recipient != address(0), 'HOP_BRIDGE_RECIPIENT_ZERO');
+        if (block.chainid == chainId) revert HopBridgeSameChain(chainId);
+        if (recipient == address(0)) revert HopBridgeRecipientZero();
 
         bool toL2 = !_isL1(chainId);
         bool fromL1 = _isL1(block.chainid);
-
         uint256 preBalanceIn = IERC20(token).balanceOf(address(this));
 
         if (fromL1 && toL2)
             _bridgeFromL1ToL2(chainId, token, amountIn, minAmountOut, recipient, bridge, deadline, relayer, fee);
         else if (!fromL1 && toL2) {
-            require(relayer == address(0), 'HOP_RELAYER_NOT_NEEDED');
+            if (relayer != address(0)) revert HopBridgeRelayerNotNeeded();
             _bridgeFromL2ToL2(chainId, token, amountIn, minAmountOut, recipient, bridge, deadline, fee);
         } else if (!fromL1 && !toL2) {
-            require(deadline == 0, 'HOP_DEADLINE_NOT_NEEDED');
+            if (deadline != 0) revert HopBridgeDeadlineNotNeeded();
             _bridgeFromL2ToL1(chainId, token, amountIn, minAmountOut, recipient, bridge, fee);
-        } else revert('HOP_BRIDGE_OP_NOT_SUPPORTED');
+        } else revert HopBridgeOpNotSupported();
 
         uint256 postBalanceIn = IERC20(token).balanceOf(address(this));
-        require(postBalanceIn >= preBalanceIn - amountIn, 'HOP_BAD_TOKEN_IN_BALANCE');
+        bool isPostBalanceInUnexpected = postBalanceIn < preBalanceIn - amountIn;
+        if (isPostBalanceInUnexpected) revert HopBridgeBadPostTokenBalance(postBalanceIn, preBalanceIn, amountIn);
     }
 
     /**
@@ -125,7 +160,7 @@ contract HopConnector {
         address relayer,
         uint256 relayerFee
     ) internal {
-        require(deadline > block.timestamp, 'HOP_BRIDGE_INVALID_DEADLINE');
+        if (deadline <= block.timestamp) revert HopBridgePastDeadline(deadline, block.timestamp);
 
         uint256 value = _unwrapOrApproveTokens(hopBridge, token, amountIn);
         IHopL1Bridge(hopBridge).sendToL2{ value: value }(
@@ -193,7 +228,7 @@ contract HopConnector {
         uint256 deadline,
         uint256 bonderFee
     ) internal {
-        require(deadline > block.timestamp, 'HOP_BRIDGE_INVALID_DEADLINE');
+        if (deadline <= block.timestamp) revert HopBridgePastDeadline(deadline, block.timestamp);
 
         uint256 intermediateMinAmountOut = amountIn - ((amountIn - minAmountOut) / 2);
         IHopL2AMM(hopAMM).swapAndSend{ value: _unwrapOrApproveTokens(hopAMM, token, amountIn) }(