Create separate CI jobs (#24)

lambdaclass · Sep 18, 2023 · 3c385c7 · 3c385c7
1 parent ba90ebf
commit 3c385c7
Show file tree

Hide file tree

Showing 28 changed files with 525 additions and 237 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -1,26 +1,105 @@
 name: CI
 
-on:
-  pull_request
+on: pull_request
 
 jobs:
+  lint:
+    runs-on: ubuntu-latest
+
+    defaults:
+      run:
+        working-directory: ethereum
+
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v3
+
+      - name: Use Node.js
+        uses: actions/setup-node@v3
+        with:
+          node-version: 16.15.1
+          cache: yarn
+          cache-dependency-path: ethereum/yarn.lock
+
+      - name: Install yarn
+        run: npm install -g yarn
+
+      - name: Install dependencies
+        run: yarn install
+
+      - name: Lint
+        run: yarn lint
+
+  build:
+    runs-on: ubuntu-latest
+
+    defaults:
+      run:
+        working-directory: ethereum
+
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v3
+
+      - name: Use Node.js
+        uses: actions/setup-node@v3
+        with:
+          node-version: 16.15.1
+          cache: yarn
+          cache-dependency-path: ethereum/yarn.lock
+
+      - name: Install yarn
+        run: npm install -g yarn
+
+      - name: Install dependencies
+        run: yarn install
+
+      - name: Build artifacts
+        run: yarn build
+
+      - name: Create cache
+        uses: actions/cache/save@v3
+        with:
+          key: artifacts-${{ github.sha }}
+          path: |
+            ethereum/artifacts
+            ethereum/cache
+            ethereum/typechain
+
   test:
+    needs: [build, lint]
     runs-on: ubuntu-latest
 
+    defaults:
+      run:
+        working-directory: ethereum
+
     steps:
-      - uses: actions/checkout@v3
+      - name: Checkout the repository
+        uses: actions/checkout@v3
 
       - name: Use Node.js
         uses: actions/setup-node@v3
         with:
-          node-version: '16.15.1'
+          node-version: 16.15.1
+          cache: yarn
+          cache-dependency-path: ethereum/yarn.lock
 
       - name: Install yarn
         run: npm install -g yarn
 
       - name: Install dependencies
-        run: cd ethereum && yarn install
+        run: yarn install
+
+      - name: Restore artifacts cache
+        uses: actions/cache/restore@v3
+        with:
+          fail-on-cache-miss: true
+          key: artifacts-${{ github.sha }}
+          path: |
+            ethereum/artifacts
+            ethereum/cache
+            ethereum/typechain
 
       - name: Run tests
-        working-directory: ethereum
-        run: yarn test
+        run: yarn test --no-compile
diff --git a/ethereum/.editorconfig b/ethereum/.editorconfig
@@ -0,0 +1,9 @@
+[*]
+charset = utf-8
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+
+[*.sol]
+indent_style = space
+indent_size = 4
diff --git a/ethereum/.prettierrc b/ethereum/.prettierrc
@@ -0,0 +1,16 @@
+{
+  "plugins": ["prettier-plugin-solidity"],
+  "overrides": [
+    {
+      "files": "*.sol",
+      "options": {
+        "parser": "solidity-parse",
+        "printWidth": 120,
+        "tabWidth": 4,
+        "useTabs": false,
+        "singleQuote": false,
+        "bracketSpacing": false
+      }
+    }
+  ]
+}
diff --git a/ethereum/.solhint.json b/ethereum/.solhint.json
@@ -1,8 +1,17 @@
 {
-    "extends": "solhint:default",
-    "plugins": ["prettier"],
-    "rules": {
-      "prettier/prettier": "error",
-      "no-inline-assembly": false
-    }
+  "extends": "solhint:recommended",
+  "plugins": ["prettier"],
+  "rules": {
+    "prettier/prettier": ["error"],
+    "func-visibility": ["error", { "ignoreConstructors": true }],
+    "compiler-version": ["error", ">=0.8.0"],
+    "max-line-length": ["error", 120],
+    "var-name-mixedcase": "off",
+    "func-name-mixedcase": "off",
+    "no-inline-assembly": "off",
+    "custom-errors": "off",
+    "no-global-import": "off",
+    "no-complex-fallback": "off",
+    "immutable-vars-naming": ["warn", { "immutablesAsConstants": false }]
+  }
 }
diff --git a/ethereum/.vscode/extensions.json b/ethereum/.vscode/extensions.json
@@ -0,0 +1,7 @@
+{
+  "recommendations": [
+    "editorconfig.editorconfig",
+    "esbenp.prettier-vscode",
+    "nomicfoundation.hardhat-solidity"
+  ]
+}
diff --git a/ethereum/.vscode/settings.json b/ethereum/.vscode/settings.json
@@ -0,0 +1,6 @@
+{
+  "editor.defaultFormatter": "esbenp.prettier-vscode",
+  "editor.formatOnSave": true,
+  "prettier.documentSelectors": ["**/*.sol"],
+  "solidity.formatter": "prettier"
+}
diff --git a/ethereum/contracts/bridge/L1ERC20Bridge.sol b/ethereum/contracts/bridge/L1ERC20Bridge.sol
@@ -76,7 +76,8 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
     /// @notice At the time of the function call, it is not yet deployed in L2, but knowledge of its address
     /// @notice is necessary for determining L2 token address by L1 address, see `l2TokenAddress(address)` function
     /// @param _governor Address which can change L2 token implementation and upgrade the bridge
-    /// @param _deployBridgeImplementationFee How much of the sent value should be allocated to deploying the L2 bridge implementation
+    /// @param _deployBridgeImplementationFee How much of the sent value should be allocated to deploying the L2 bridge
+    /// implementation
     /// @param _deployBridgeProxyFee How much of the sent value should be allocated to deploying the L2 bridge proxy
     function initialize(
         bytes[] calldata _factoryDeps,
@@ -123,7 +124,8 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
             _deployBridgeProxyFee,
             l2BridgeProxyBytecodeHash,
             l2BridgeProxyConstructorData,
-            new bytes[](0) // No factory deps are needed for L2 bridge proxy, because it is already passed in previous step
+            // No factory deps are needed for L2 bridge proxy, because it is already passed in previous step
+            new bytes[](0)
         );
     }
 
@@ -136,7 +138,8 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
     /// @param _l2TxGasLimit The L2 gas limit to be used in the corresponding L2 transaction
     /// @param _l2TxGasPerPubdataByte The gasPerPubdataByteLimit to be used in the corresponding L2 transaction
     /// @return l2TxHash The L2 transaction hash of deposit finalization
-    /// NOTE: the function doesn't use `nonreentrant` and `senderCanCallFunction` modifiers, because the inner method does.
+    /// NOTE: the function doesn't use `nonreentrant` and `senderCanCallFunction` modifiers, because the inner
+    /// method does.
     function deposit(
         address _l2Receiver,
         address _l1Token,
@@ -156,14 +159,18 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
     /// @param _l2TxGasPerPubdataByte The gasPerPubdataByteLimit to be used in the corresponding L2 transaction
     /// @param _refundRecipient The address on L2 that will receive the refund for the transaction.
     /// @dev If the L2 deposit finalization transaction fails, the `_refundRecipient` will receive the `_l2Value`.
-    /// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses out of control.
+    /// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses
+    /// out of control.
     /// - If `_refundRecipient` is a contract on L1, the refund will be sent to the aliased `_refundRecipient`.
-    /// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will be sent to the `msg.sender` address.
-    /// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be sent to the aliased `msg.sender` address.
-    /// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds are controllable through the Mailbox,
-    /// since the Mailbox applies address aliasing to the from address for the L2 tx if the L1 msg.sender is a contract.
-    /// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx requests
-    /// through the Mailbox to use or withdraw the funds from L2, and the funds would be lost.
+    /// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will
+    /// be sent to the `msg.sender` address.
+    /// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be
+    /// sent to the aliased `msg.sender` address.
+    /// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds
+    /// are controllable through the Mailbox, since the Mailbox applies address aliasing to the from address for the
+    /// L2 tx if the L1 msg.sender is a contract. Without address aliasing for L1 contracts as refund recipients they
+    /// would not be able to make proper L2 tx requests through the Mailbox to use or withdraw the funds from L2, and
+    /// the funds would be lost.
     /// @return l2TxHash The L2 transaction hash of deposit finalization
     function deposit(
         address _l2Receiver,
@@ -205,11 +212,7 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
 
     /// @dev Transfers tokens from the depositor address to the smart contract address
     /// @return The difference between the contract balance before and after the transferring of funds
-    function _depositFunds(
-        address _from,
-        IERC20 _token,
-        uint256 _amount
-    ) internal returns (uint256) {
+    function _depositFunds(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) {
         uint256 balanceBefore = _token.balanceOf(address(this));
         _token.safeTransferFrom(_from, address(this), _amount);
         uint256 balanceAfter = _token.balanceOf(address(this));
@@ -316,17 +319,12 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
     }
 
     /// @dev Decode the withdraw message that came from L2
-    function _parseL2WithdrawalMessage(bytes memory _l2ToL1message)
-        internal
-        pure
-        returns (
-            address l1Receiver,
-            address l1Token,
-            uint256 amount
-        )
-    {
+    function _parseL2WithdrawalMessage(
+        bytes memory _l2ToL1message
+    ) internal pure returns (address l1Receiver, address l1Token, uint256 amount) {
         // Check that the message length is correct.
-        // It should be equal to the length of the function signature + address + address + uint256 = 4 + 20 + 20 + 32 = 76 (bytes).
+        // It should be equal to the length of the function signature + address + address + uint256 = 4 + 20 + 20 + 32 =
+        // 76 (bytes).
         require(_l2ToL1message.length == 76, "kk");
 
         (uint32 functionSignature, uint256 offset) = UnsafeBytes.readUint32(_l2ToL1message, 0);
@@ -338,12 +336,7 @@ contract L1ERC20Bridge is IL1Bridge, IL1BridgeLegacy, AllowListed, ReentrancyGua
     }
 
     /// @dev Verify the deposit limit is reached to its cap or not
-    function _verifyDepositLimit(
-        address _l1Token,
-        address _depositor,
-        uint256 _amount,
-        bool _claiming
-    ) internal {
+    function _verifyDepositLimit(address _l1Token, address _depositor, uint256 _amount, bool _claiming) internal {
         IAllowList.Deposit memory limitData = IAllowList(allowList).getTokenDepositLimitData(_l1Token);
         if (!limitData.depositLimitation) return; // no deposit limitation is placed for this token
 

diff --git a/ethereum/contracts/bridge/L1WethBridge.sol b/ethereum/contracts/bridge/L1WethBridge.sol
@@ -60,11 +60,7 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
 
     /// @dev Contract is expected to be used as proxy implementation.
     /// @dev Initialize the implementation to prevent Parity hack.
-    constructor(
-        address payable _l1WethAddress,
-        IZkSync _zkSync,
-        IAllowList _allowList
-    ) reentrancyGuardInitializer {
+    constructor(address payable _l1WethAddress, IZkSync _zkSync, IAllowList _allowList) reentrancyGuardInitializer {
         l1WethAddress = _l1WethAddress;
         zkSync = _zkSync;
         allowList = _allowList;
@@ -77,8 +73,10 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
     /// @notice _factoryDeps[1] == a raw bytecode of proxy that is used as L2 WETH bridge
     /// @param _l2WethAddress Pre-calculated address of L2 WETH token
     /// @param _governor Address which can change L2 WETH token implementation and upgrade the bridge
-    /// @param _deployBridgeImplementationFee The fee that will be paid for the L1 -> L2 transaction for deploying L2 bridge implementation
-    /// @param _deployBridgeProxyFee The fee that will be paid for the L1 -> L2 transaction for deploying L2 bridge proxy
+    /// @param _deployBridgeImplementationFee The fee that will be paid for the L1 -> L2 transaction for deploying L2
+    /// bridge implementation
+    /// @param _deployBridgeProxyFee The fee that will be paid for the L1 -> L2 transaction for deploying L2 bridge
+    /// proxy
     function initialize(
         bytes[] calldata _factoryDeps,
         address _l2WethAddress,
@@ -129,7 +127,8 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
             _deployBridgeProxyFee,
             l2WethBridgeProxyBytecodeHash,
             l2WethBridgeProxyConstructorData,
-            new bytes[](0) // No factory deps are needed for L2 bridge proxy, because it is already passed in the previous step
+            // No factory deps are needed for L2 bridge proxy, because it is already passed in the previous step
+            new bytes[](0)
         );
     }
 
@@ -142,13 +141,18 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
     /// @param _l2TxGasPerPubdataByte The gasPerPubdataByteLimit to be used in the corresponding L2 transaction
     /// @param _refundRecipient The address on L2 that will receive the refund for the transaction.
     /// @dev If the L2 deposit finalization transaction fails, the `_refundRecipient` will receive the `_l2Value`.
-    /// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses out of control.
+    /// Please note, the contract may change the refund recipient's address to eliminate sending funds to addresses
+    /// out of control.
     /// - If `_refundRecipient` is a contract on L1, the refund will be sent to the aliased `_refundRecipient`.
-    /// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will be sent to the `msg.sender` address.
-    /// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be sent to the aliased `msg.sender` address.
-    /// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds are controllable through the Mailbox,
+    /// - If `_refundRecipient` is set to `address(0)` and the sender has NO deployed bytecode on L1, the refund will
+    /// be sent to the `msg.sender` address.
+    /// - If `_refundRecipient` is set to `address(0)` and the sender has deployed bytecode on L1, the refund will be
+    /// sent to the aliased `msg.sender` address.
+    /// @dev The address aliasing of L1 contracts as refund recipient on L2 is necessary to guarantee that the funds
+    /// are controllable through the Mailbox,
     /// since the Mailbox applies address aliasing to the from address for the L2 tx if the L1 msg.sender is a contract.
-    /// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx requests
+    /// Without address aliasing for L1 contracts as refund recipients they would not be able to make proper L2 tx
+    /// requests
     /// through the Mailbox to use or withdraw the funds from L2, and the funds would be lost.
     /// @return txHash The L2 transaction hash of deposit finalization
     function deposit(
@@ -204,7 +208,8 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
     }
 
     /// @notice Withdraw funds from the initiated deposit, that failed when finalizing on L2.
-    /// Note: Refund is performed by sending an equivalent amount of ETH on L2 to the specified deposit refund recipient address.
+    /// Note: Refund is performed by sending an equivalent amount of ETH on L2 to the specified deposit refund
+    /// recipient address.
     function claimFailedDeposit(
         address, // _depositSender,
         address, // _l1Token,
@@ -219,7 +224,8 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
 
     /// @notice Finalize the withdrawal and release funds
     /// @param _l2BlockNumber The L2 block number where the ETH (WETH) withdrawal was processed
-    /// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the ETH withdrawal message containing additional data about WETH withdrawal
+    /// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the ETH
+    /// withdrawal message containing additional data about WETH withdrawal
     /// @param _l2TxNumberInBlock The L2 transaction number in a block, in which the ETH withdrawal log was sent
     /// @param _message The L2 withdraw data, stored in an L2 -> L1 message
     /// @param _merkleProof The Merkle proof of the inclusion L2 -> L1 message about withdrawal initialization
@@ -260,15 +266,15 @@ contract L1WethBridge is IL1Bridge, AllowListed, ReentrancyGuard {
         emit WithdrawalFinalized(l1WethWithdrawReceiver, l1WethAddress, amount);
     }
 
-    /// @dev Decode the ETH withdraw message with additional data about WETH withdrawal that came from L2EthToken contract
-    function _parseL2EthWithdrawalMessage(bytes memory _message)
-        internal
-        view
-        returns (address l1WethReceiver, uint256 ethAmount)
-    {
+    /// @dev Decode the ETH withdraw message with additional data about WETH withdrawal that came from L2EthToken
+    /// contract
+    function _parseL2EthWithdrawalMessage(
+        bytes memory _message
+    ) internal view returns (address l1WethReceiver, uint256 ethAmount) {
         // Check that the message length is correct.
         // additionalData (WETH withdrawal data): l2 sender address + weth receiver address = 20 + 20 = 40 (bytes)
-        // It should be equal to the length of the function signature + eth receiver address + uint256 amount + additionalData = 4 + 20 + 32 + 40 = 96 (bytes).
+        // It should be equal to the length of the function signature + eth receiver address + uint256 amount +
+        // additionalData = 4 + 20 + 32 + 40 = 96 (bytes).
         require(_message.length == 96, "Incorrect ETH message with additional data length");
 
         (uint32 functionSignature, uint256 offset) = UnsafeBytes.readUint32(_message, 0);