feat: make use of custom errors (#60)

This PR follows https://soliditylang.org/blog/2021/04/21/custom-errors/ Additionally upgrades the deprecated hardhat-waffle to hardhat-chai-matchers Closes #57 --------- Co-authored-by: Braqzen <[email protected]>
FuelLabs · Sep 28, 2023 · 278cbe1 · 278cbe1
1 parent 2ace82b
commit 278cbe1
Show file tree

Hide file tree

Showing 17 changed files with 255 additions and 4,445 deletions.
diff --git a/.changeset/spicy-flies-rhyme.md b/.changeset/spicy-flies-rhyme.md
@@ -0,0 +1,5 @@
+---
+'@fuel-bridge/solidity-contracts': patch
+---
+
+Changes require statements to if-revert-custom-error for better interfacing and reduced gas costs
diff --git a/packages/integration-tests/tests/bridge_erc20.ts b/packages/integration-tests/tests/bridge_erc20.ts
@@ -17,7 +17,6 @@ import {
   getBlock,
 } from '@fuel-bridge/test-utils';
 import chai from 'chai';
-import { solidity } from 'ethereum-waffle';
 import type { BigNumber, Signer } from 'ethers';
 import { Address, BN, InputType } from 'fuels';
 import type {
@@ -29,7 +28,6 @@ import type {
 
 LOG_CONFIG.debug = false;
 
-chai.use(solidity);
 const { expect } = chai;
 
 describe('Bridging ERC20 tokens', async function () {
@@ -78,15 +76,11 @@ describe('Bridging ERC20 tokens', async function () {
     expect(await eth_testToken.decimals()).to.equal(18);
 
     // mint tokens as starting balances
-    await expect(
-      eth_testToken.mint(await env.eth.deployer.getAddress(), 10_000)
-    ).to.not.be.reverted;
-    await expect(
-      eth_testToken.mint(await env.eth.signers[0].getAddress(), 10_000)
-    ).to.not.be.reverted;
-    await expect(
-      eth_testToken.mint(await env.eth.signers[1].getAddress(), 10_000)
-    ).to.not.be.reverted;
+    await eth_testToken.mint(await env.eth.deployer.getAddress(), 10_000);
+
+    await eth_testToken.mint(await env.eth.signers[0].getAddress(), 10_000);
+
+    await eth_testToken.mint(await env.eth.signers[1].getAddress(), 10_000);
   });
 
   describe('Bridge ERC20 to Fuel', async () => {
@@ -116,11 +110,9 @@ describe('Bridging ERC20 tokens', async function () {
 
     it('Bridge ERC20 via FuelERC20Gateway', async () => {
       // approve FuelERC20Gateway to spend the tokens
-      await expect(
-        eth_testToken
-          .connect(ethereumTokenSender)
-          .approve(env.eth.fuelERC20Gateway.address, NUM_TOKENS)
-      ).to.not.be.reverted;
+      await eth_testToken
+        .connect(ethereumTokenSender)
+        .approve(env.eth.fuelERC20Gateway.address, NUM_TOKENS);
 
       // use the FuelERC20Gateway to deposit test tokens and receive equivalent tokens on Fuel
       const tx = await env.eth.fuelERC20Gateway
@@ -264,15 +256,13 @@ describe('Bridging ERC20 tokens', async function () {
       const relayMessageParams = createRelayMessageParams(withdrawMessageProof);
 
       // relay message
-      await expect(
-        env.eth.fuelMessagePortal.relayMessage(
-          relayMessageParams.message,
-          relayMessageParams.rootBlockHeader,
-          relayMessageParams.blockHeader,
-          relayMessageParams.blockInHistoryProof,
-          relayMessageParams.messageInBlockProof
-        )
-      ).to.not.be.reverted;
+      await env.eth.fuelMessagePortal.relayMessage(
+        relayMessageParams.message,
+        relayMessageParams.rootBlockHeader,
+        relayMessageParams.blockHeader,
+        relayMessageParams.blockInHistoryProof,
+        relayMessageParams.messageInBlockProof
+      );
     });
 
     it('Check ERC20 arrived on Ethereum', async () => {

diff --git a/packages/integration-tests/tests/bridge_erc721.ts b/packages/integration-tests/tests/bridge_erc721.ts
@@ -17,7 +17,6 @@ import {
   getOrDeployERC721Contract,
 } from '@fuel-bridge/test-utils';
 import chai from 'chai';
-import { solidity } from 'ethereum-waffle';
 import type { Wallet } from 'ethers';
 import { BigNumber, utils } from 'ethers';
 import { Address, BN, InputType } from 'fuels';
@@ -30,7 +29,6 @@ import type {
 
 LOG_CONFIG.debug = false;
 
-chai.use(solidity);
 const { expect } = chai;
 
 const signerToHexTokenId = (signer: { address: string }) => {

diff --git a/packages/integration-tests/tests/transfer_eth.ts b/packages/integration-tests/tests/transfer_eth.ts
@@ -11,7 +11,6 @@ import {
   getBlock,
 } from '@fuel-bridge/test-utils';
 import chai from 'chai';
-import { solidity } from 'ethereum-waffle';
 import type { BigNumber, Signer } from 'ethers';
 import { parseEther } from 'ethers/lib/utils';
 import { Address, BN, BaseAssetId } from 'fuels';
@@ -21,7 +20,6 @@ import type {
   MessageProof,
 } from 'fuels';
 
-chai.use(solidity);
 const { expect } = chai;
 
 describe('Transferring ETH', async function () {
@@ -179,15 +177,13 @@ describe('Transferring ETH', async function () {
       const relayMessageParams = createRelayMessageParams(withdrawMessageProof);
 
       // relay message
-      await expect(
-        env.eth.fuelMessagePortal.relayMessage(
-          relayMessageParams.message,
-          relayMessageParams.rootBlockHeader,
-          relayMessageParams.blockHeader,
-          relayMessageParams.blockInHistoryProof,
-          relayMessageParams.messageInBlockProof
-        )
-      ).to.not.be.reverted;
+      await env.eth.fuelMessagePortal.relayMessage(
+        relayMessageParams.message,
+        relayMessageParams.rootBlockHeader,
+        relayMessageParams.blockHeader,
+        relayMessageParams.blockInHistoryProof,
+        relayMessageParams.messageInBlockProof
+      );
     });
 
     it('Check ETH arrived on Ethereum', async () => {

diff --git a/packages/solidity-contracts/contracts/fuelchain/FuelChainState.sol b/packages/solidity-contracts/contracts/fuelchain/FuelChainState.sol
@@ -45,6 +45,12 @@ contract FuelChainState is Initializable, PausableUpgradeable, AccessControlUpgr
     /// @dev Emitted when a commit is first submitted
     event CommitSubmitted(uint256 indexed commitHeight, bytes32 blockHash);
 
+    ////////////
+    // Errors //
+    ////////////
+
+    error UnknownBlock();
+
     /////////////
     // Storage //
     /////////////
@@ -117,7 +123,7 @@ contract FuelChainState is Initializable, PausableUpgradeable, AccessControlUpgr
         // TODO This division could be done offchain, or at least also could be assembly'ed to avoid non-zero division check
         uint256 commitHeight = blockHeight / BLOCKS_PER_COMMIT_INTERVAL;
         Commit storage commitSlot = _commitSlots[commitHeight % NUM_COMMIT_SLOTS];
-        require(commitSlot.blockHash == blockHash, "Unknown block");
+        if (commitSlot.blockHash != blockHash) revert UnknownBlock();
 
         return block.timestamp >= uint256(commitSlot.timestamp) + TIME_TO_FINALIZE;
     }

diff --git a/packages/solidity-contracts/contracts/fuelchain/FuelMessagePortal.sol b/packages/solidity-contracts/contracts/fuelchain/FuelMessagePortal.sol
@@ -61,6 +61,19 @@ contract FuelMessagePortal is
     /// @dev Emitted when a message is successfully relayed to Ethereum from Fuel
     event MessageRelayed(bytes32 indexed messageId, bytes32 indexed sender, bytes32 indexed recipient, uint64 amount);
 
+    ////////////
+    // Errors //
+    ////////////
+
+    error UnfinalizedBlock();
+    error InvalidBlockInHistoryProof();
+    error InvalidMessageInBlockProof();
+    error CurrentMessageSenderNotSet();
+    error MessageDataTooLarge();
+    error AmountPrecisionIncompatibility();
+    error AmountTooBig();
+    error AlreadyRelayed();
+
     ///////////////
     // Constants //
     ///////////////
@@ -71,6 +84,7 @@ contract FuelMessagePortal is
     /// @dev The number of decimals that the base Fuel asset uses
     uint256 public constant FUEL_BASE_ASSET_DECIMALS = 9;
     uint256 public constant ETH_DECIMALS = 18;
+    uint256 public constant PRECISION = 10 ** (ETH_DECIMALS - FUEL_BASE_ASSET_DECIMALS);
 
     /// @dev The max message data size in bytes
     uint256 public constant MAX_MESSAGE_DATA_SIZE = 2 ** 16;
@@ -179,37 +193,34 @@ contract FuelMessagePortal is
         MerkleProof calldata messageInBlockProof
     ) external payable whenNotPaused {
         //verify root block header
-        require(
-            _fuelChainState.finalized(rootBlockHeader.computeConsensusHeaderHash(), rootBlockHeader.height),
-            "Unfinalized root block"
-        );
+         if (!_fuelChainState.finalized(rootBlockHeader.computeConsensusHeaderHash(), rootBlockHeader.height))
+            revert UnfinalizedBlock();
 
         //verify block in history
-        require(
-            verifyBinaryTree(
+        if(
+            !verifyBinaryTree(
                 rootBlockHeader.prevRoot,
                 abi.encodePacked(blockHeader.computeConsensusHeaderHash()),
                 blockInHistoryProof.proof,
                 blockInHistoryProof.key,
                 rootBlockHeader.height
-            ),
-            "Invalid block in history proof"
-        );
+            )
+
+        ) revert InvalidBlockInHistoryProof();
 
         //verify message in block
         bytes32 messageId = CryptographyLib.hash(
             abi.encodePacked(message.sender, message.recipient, message.nonce, message.amount, message.data)
         );
-        require(
-            verifyBinaryTree(
+        if(
+            !verifyBinaryTree(
                 blockHeader.outputMessagesRoot,
                 abi.encodePacked(messageId),
                 messageInBlockProof.proof,
                 messageInBlockProof.key,
                 blockHeader.outputMessagesCount
-            ),
-            "Invalid message in block proof"
-        );
+            )
+        ) revert InvalidMessageInBlockProof();
 
         //execute message
         _executeMessage(messageId, message);
@@ -225,7 +236,7 @@ contract FuelMessagePortal is
     /// @notice Used by message receiving contracts to get the address on Fuel that sent the message
     /// @return sender the address of the sender on Fuel
     function messageSender() external view returns (bytes32) {
-        require(_incomingMessageSender != NULL_MESSAGE_SENDER, "Current message sender not set");
+        if (_incomingMessageSender == NULL_MESSAGE_SENDER) revert CurrentMessageSenderNotSet();
         return _incomingMessageSender;
     }
 
@@ -257,14 +268,14 @@ contract FuelMessagePortal is
         bytes32 sender = bytes32(uint256(uint160(msg.sender)));
         unchecked {
             //make sure data size is not too large
-            require(data.length < MAX_MESSAGE_DATA_SIZE, "message-data-too-large");
+            if (data.length >= MAX_MESSAGE_DATA_SIZE) revert MessageDataTooLarge();
 
             //make sure amount fits into the Fuel base asset decimal level
             uint256 precision = 10 ** (ETH_DECIMALS - FUEL_BASE_ASSET_DECIMALS);
             uint256 amount = msg.value / precision;
             if (msg.value > 0) {
-                require(amount * precision == msg.value, "amount-precision-incompatability");
-                require(amount <= ((2 ** 64) - 1), "amount-precision-incompatability");
+                if (amount * PRECISION != msg.value) revert AmountPrecisionIncompatibility();
+                if (amount > type(uint64).max) revert AmountTooBig();
             }
 
             // TODO: room for gas savings - double storage read for _outgoingMessageNonce
@@ -280,7 +291,7 @@ contract FuelMessagePortal is
     /// @param messageId The id of message to execute
     /// @param message The message to execute
     function _executeMessage(bytes32 messageId, Message calldata message) private nonReentrant {
-        require(!_incomingMessageSuccessful[messageId], "Already relayed");
+        if (_incomingMessageSuccessful[messageId]) revert AlreadyRelayed();
 
         //set message sender for receiving contract to reference
         _incomingMessageSender = message.sender;

diff --git a/packages/solidity-contracts/contracts/messaging/FuelMessagesEnabled.sol b/packages/solidity-contracts/contracts/messaging/FuelMessagesEnabled.sol
@@ -6,6 +6,13 @@ import {FuelMessagePortal} from "../fuelchain/FuelMessagePortal.sol";
 /// @title FuelMessagesEnabled
 /// @notice Helper contract for contracts sending and receiving messages from Fuel
 abstract contract FuelMessagesEnabled {
+    ////////////
+    // Errors //
+    ////////////
+
+    error CallerIsNotPortal();
+    error InvalidMessageSender();
+
     /////////////
     // Storage //
     /////////////
@@ -19,15 +26,15 @@ abstract contract FuelMessagesEnabled {
 
     /// @notice Enforces that the modified function is only callable by the Fuel message portal
     modifier onlyFromPortal() {
-        require(msg.sender == address(_fuelMessagePortal), "Caller is not the portal");
+        if (msg.sender != address(_fuelMessagePortal)) revert CallerIsNotPortal();
         _;
     }
 
     /// @notice Enforces that the modified function is only callable by the portal and a specific Fuel account
     /// @param fuelSender The only sender on Fuel which is authenticated to call this function
     modifier onlyFromFuelSender(bytes32 fuelSender) {
-        require(msg.sender == address(_fuelMessagePortal), "Caller is not the portal");
-        require(_fuelMessagePortal.messageSender() == fuelSender, "Invalid message sender");
+        if (msg.sender != address(_fuelMessagePortal)) revert CallerIsNotPortal();
+        if (_fuelMessagePortal.messageSender() != fuelSender) revert InvalidMessageSender();
         _;
     }
 

diff --git a/packages/solidity-contracts/hardhat.config.ts b/packages/solidity-contracts/hardhat.config.ts
@@ -1,6 +1,6 @@
 import { config as dotEnvConfig } from 'dotenv';
 import type { HardhatUserConfig } from 'hardhat/types';
-import '@nomiclabs/hardhat-waffle';
+import '@nomicfoundation/hardhat-chai-matchers';
 import '@nomiclabs/hardhat-etherscan';
 import '@openzeppelin/hardhat-upgrades';
 import 'hardhat-typechain';

diff --git a/packages/solidity-contracts/package.json b/packages/solidity-contracts/package.json
@@ -40,6 +40,7 @@
     "@ethersproject/bytes": "^5.7.0",
     "@ethersproject/providers": "^5.7.0",
     "@fuel-contracts/merkle-sol": "^0.1.4",
+    "@nomicfoundation/hardhat-chai-matchers": "^1.0.6",
     "@nomiclabs/hardhat-ethers": "^2.2.1",
     "@nomiclabs/hardhat-etherscan": "^3.1.2",
     "@openzeppelin/contracts": "^4.8.0",
@@ -52,7 +53,6 @@
     "@ethersproject/bytes": "^5.7.0",
     "@ethersproject/providers": "^5.7.0",
     "@fuel-ts/merkle": "^0.21.2",
-    "@nomiclabs/hardhat-waffle": "^2.0.3",
     "@typechain/ethers-v5": "^6.0.5",
     "@types/chai": "^4.3.4",
     "@types/express": "^4.17.14",
@@ -62,7 +62,6 @@
     "@typescript-eslint/parser": "^5.43.0",
     "chai": "^4.3.7",
     "dotenv": "^16.0.3",
-    "ethereum-waffle": "^3.4.4",
     "ethers": "^5.7.2",
     "express": "^4.18.2",
     "hardhat": "^2.12.2",

diff --git a/packages/solidity-contracts/test/ecdsa.ts b/packages/solidity-contracts/test/ecdsa.ts
@@ -1,13 +1,11 @@
 import chai from 'chai';
-import { solidity } from 'ethereum-waffle';
 import type { Contract } from 'ethers';
 import { BigNumber as BN } from 'ethers';
 import { SigningKey } from 'ethers/lib/utils';
 import { ethers } from 'hardhat';
 
 import { componentSign } from '../protocol/validators';
 
-chai.use(solidity);
 const { expect } = chai;
 
 const SECP256K1N = BN.from(

diff --git a/packages/solidity-contracts/test/erc20Gateway.ts b/packages/solidity-contracts/test/erc20Gateway.ts
@@ -1,7 +1,6 @@
 import type { Provider } from '@ethersproject/abstract-provider';
 import { constructTree, calcRoot, getProof } from '@fuel-ts/merkle';
 import chai from 'chai';
-import { solidity } from 'ethereum-waffle';
 import { BigNumber as BN, constants, utils } from 'ethers';
 import { ethers } from 'hardhat';
 
@@ -17,7 +16,6 @@ import type { HarnessObject } from '../protocol/harness';
 import Message, { computeMessageId } from '../protocol/message';
 import { randomAddress, randomBytes32, tai64Time } from '../protocol/utils';
 
-chai.use(solidity);
 const { expect } = chai;
 
 // Merkle tree node structure
@@ -777,7 +775,10 @@ describe('ERC20 Gateway', async () => {
           BN.from(100),
           ethers.constants.HashZero
         )
-      ).to.be.revertedWith('Caller is not the portal');
+      ).to.be.revertedWithCustomError(
+        env.fuelERC20Gateway,
+        'CallerIsNotPortal'
+      );
     });
 
     it('Should be able to finalize valid withdrawal through portal', async () => {