[Audit] Some audit fixes (#494)

* fix: some audit fixes * regenerate ABIs and binary with strict version solc 0.8.7 * feat: push erc20custody compiled and remove contracts * fix: update json * fix go generate * fix: filter deposited * fix: bring back ERC20Custody contract * fix --------- Co-authored-by: brewmaster012 <>
zeta-chain · Apr 7, 2023 · a1163f8 · a1163f8
1 parent 2863c6a
commit a1163f8
Show file tree

Hide file tree

Showing 26 changed files with 398 additions and 104 deletions.
diff --git a/contracts/evm/erc20custody/ERC20Custody.abi b/contracts/evm/erc20custody/ERC20Custody.abi
@@ -16,6 +16,11 @@
         "name": "_zetaFee",
         "type": "uint256"
       },
+      {
+        "internalType": "uint256",
+        "name": "_zetaMaxFee",
+        "type": "uint256"
+      },
       {
         "internalType": "contract IERC20",
         "name": "_zeta",
@@ -60,6 +65,11 @@
     "name": "ZeroFee",
     "type": "error"
   },
+  {
+    "inputs": [],
+    "name": "ZetaMaxFeeExceeded",
+    "type": "error"
+  },
   {
     "anonymous": false,
     "inputs": [
@@ -70,7 +80,7 @@
         "type": "bytes"
       },
       {
-        "indexed": false,
+        "indexed": true,
         "internalType": "contract IERC20",
         "name": "asset",
         "type": "address"
@@ -121,7 +131,7 @@
     "anonymous": false,
     "inputs": [
       {
-        "indexed": false,
+        "indexed": true,
         "internalType": "contract IERC20",
         "name": "asset",
         "type": "address"
@@ -134,7 +144,7 @@
     "anonymous": false,
     "inputs": [
       {
-        "indexed": false,
+        "indexed": true,
         "internalType": "contract IERC20",
         "name": "asset",
         "type": "address"
@@ -147,13 +157,13 @@
     "anonymous": false,
     "inputs": [
       {
-        "indexed": false,
+        "indexed": true,
         "internalType": "address",
         "name": "recipient",
         "type": "address"
       },
       {
-        "indexed": false,
+        "indexed": true,
         "internalType": "contract IERC20",
         "name": "asset",
         "type": "address"
@@ -375,5 +385,18 @@
     ],
     "stateMutability": "view",
     "type": "function"
+  },
+  {
+    "inputs": [],
+    "name": "zetaMaxFee",
+    "outputs": [
+      {
+        "internalType": "uint256",
+        "name": "",
+        "type": "uint256"
+      }
+    ],
+    "stateMutability": "view",
+    "type": "function"
   }
 ]
diff --git a/contracts/evm/erc20custody/ERC20Custody.bin b/contracts/evm/erc20custody/ERC20Custody.bin
diff --git a/contracts/evm/erc20custody/ERC20Custody.go b/contracts/evm/erc20custody/ERC20Custody.go
diff --git a/contracts/evm/erc20custody/ERC20Custody.json b/contracts/evm/erc20custody/ERC20Custody.json
diff --git a/contracts/evm/erc20custody/ERC20Custody.sol b/contracts/evm/erc20custody/ERC20Custody.sol
@@ -1,54 +1,59 @@
 // SPDX-License-Identifier: MIT
 // v1.0, 2023-01-10
-pragma solidity ^0.8.7;
+pragma solidity 0.8.7;
 
-// This ERC20 interface comes from OpenZeppelin
-// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol
-interface IERC20 {
-    function transfer(address to, uint256 amount) external returns (bool);
-    function allowance(address owner, address spender) external view returns (uint256);
-    function approve(address spender, uint256 amount) external returns (bool);
-    function transferFrom( address from, address to, uint256 amount) external returns (bool);
-    function balanceOf(address account) external view returns (uint256);
-}
+import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 
+/// @title ERC20Custody.
+/// @notice ERC20Custody for depositing ERC20 assets into ZetaChain and making operations with them.
 contract ERC20Custody {
+    using SafeERC20 for IERC20;
+
     error NotWhitelisted();
     error NotPaused();
     error InvalidSender();
     error InvalidTSSUpdater();
     error ZeroAddress();
     error IsPaused();
+    error ZetaMaxFeeExceeded();
     error ZeroFee();
 
+    /// @notice If custody operations are paused.
     bool public paused;
-
-    // TSSAddress is the TSS address collectively possessed by Zeta blockchain validators. 
-    // Threshold Signature Scheme (TSS) [GG20] is a multi-sig ECDSA/EdDSA protocol. 
+    /// @notice TSSAddress is the TSS address collectively possessed by Zeta blockchain validators. 
     address public TSSAddress; 
+    /// @notice Threshold Signature Scheme (TSS) [GG20] is a multi-sig ECDSA/EdDSA protocol. 
     address public TSSAddressUpdater;
+    /// @notice Current zeta fee for depositing funds into ZetaChain.
     uint256 public zetaFee;
+    /// @notice Maximum zeta fee for transaction.
+    uint256 immutable public zetaMaxFee;
+    /// @notice Zeta ERC20 token 
     IERC20 immutable public zeta;
-
-    // Mapping of whitelisted token => true/false
+    /// @notice Mapping of whitelisted token => true/false
     mapping(IERC20 => bool) public whitelisted;
 
     event Paused(address sender);
     event Unpaused(address sender);
-    event Whitelisted(IERC20 asset);
-    event Unwhitelisted(IERC20 asset);
-    event Deposited(bytes recipient, IERC20 asset, uint256 amount, bytes message);
-    event Withdrawn(address recipient, IERC20 asset, uint256 amount);
+    event Whitelisted(IERC20 indexed asset);
+    event Unwhitelisted(IERC20 indexed asset);
+    event Deposited(bytes recipient, IERC20 indexed asset, uint256 amount, bytes message);
+    event Withdrawn(address indexed recipient, IERC20 indexed asset, uint256 amount);
 
-    constructor(address _TSSAddress, address _TSSAddressUpdater, uint256 _zetaFee, IERC20 _zeta) {       
+    constructor(address _TSSAddress, address _TSSAddressUpdater, uint256 _zetaFee, uint256 _zetaMaxFee, IERC20 _zeta) {       
         TSSAddress = _TSSAddress; 
         TSSAddressUpdater = _TSSAddressUpdater;
         zetaFee = _zetaFee;
         zeta = _zeta;
+        zetaMaxFee = _zetaMaxFee;
         paused = false;
     }
 
-    // update the TSSAddress in case of Zeta blockchain validator nodes churn
+    /**
+     * @dev Update the TSSAddress in case of Zeta blockchain validator nodes churn
+     * @param _address, new tss address
+     */
     function updateTSSAddress(address _address) external {
         if (msg.sender != TSSAddressUpdater) {
             revert InvalidTSSUpdater();
@@ -59,19 +64,27 @@ contract ERC20Custody {
         TSSAddress = _address;
     }
 
-    // update zetaFee
+    /**
+     * @dev Update zeta fee
+     * @param _zetaFee, new zeta fee
+     */
     function updateZetaFee(uint256 _zetaFee) external {
         if (msg.sender != TSSAddress) {
             revert InvalidSender();
         }
         if (_zetaFee == 0) {
             revert ZeroFee();
         }
+        if (_zetaFee > zetaMaxFee) {
+            revert ZetaMaxFeeExceeded();
+        }
         zetaFee = _zetaFee;
     }
 
-    // Change the ownership of TSSAddressUpdater to the Zeta blockchain TSS nodes. 
-    // Effectively, only Zeta blockchain validators collectively can update TSSAddress afterwards. 
+    /**
+     * @dev Change the ownership of TSSAddressUpdater to the Zeta blockchain TSS nodes.
+     * Effectively, only Zeta blockchain validators collectively can update TSSAddress afterwards.
+     */
     function renounceTSSAddressUpdater() external {
         if (msg.sender != TSSAddressUpdater) {
             revert InvalidTSSUpdater();
@@ -82,6 +95,9 @@ contract ERC20Custody {
         TSSAddressUpdater = TSSAddress;
     }
 
+    /**
+     * @dev Pause custody operations.
+     */
     function pause() external {
         if (paused) {
             revert IsPaused();
@@ -96,6 +112,9 @@ contract ERC20Custody {
         emit Paused(msg.sender);
     }
 
+    /**
+     * @dev Unpause custody operations.
+     */
     function unpause() external {
         if (!paused) {
             revert NotPaused();
@@ -107,6 +126,10 @@ contract ERC20Custody {
         emit Unpaused(msg.sender);
     }
 
+    /**
+     * @dev Whitelist asset
+     * @param asset, ERC20 asset
+     */
     function whitelist(IERC20 asset) external {
         if (msg.sender != TSSAddress) {
             revert InvalidSender();
@@ -115,6 +138,10 @@ contract ERC20Custody {
         emit Whitelisted(asset);
     }
 
+    /**
+     * @dev Unwhitelist asset
+     * @param asset, ERC20 asset
+     */
     function unwhitelist(IERC20 asset) external {
         if (msg.sender != TSSAddress) {
             revert InvalidSender();
@@ -123,20 +150,33 @@ contract ERC20Custody {
         emit Unwhitelisted(asset);
     }
 
+    /**
+     * @dev Deposit asset amount to recipient with message that encodes additional zetachain evm call or message.
+     * @param recipient, recipient address
+     * @param asset, ERC20 asset
+     * @param amount, asset amount
+     * @param message, bytes message or encoded zetechain call.
+     */
     function deposit(bytes calldata recipient, IERC20 asset, uint256 amount, bytes calldata message) external {
         if (paused) {
             revert IsPaused();
         }
         if (!whitelisted[asset]) {
             revert NotWhitelisted();
         }
-        if (address(zeta) != address(0)) {
+        if (zetaFee != 0 && address(zeta) != address(0)) {
             zeta.transferFrom(msg.sender, TSSAddress, zetaFee);
         }
         asset.transferFrom(msg.sender, address(this), amount);
         emit Deposited(recipient, asset, amount, message);
     }
 
+    /**
+     * @dev Withdraw asset amount to recipient by custody TSS owner.
+     * @param recipient, recipient address
+     * @param asset, ERC20 asset
+     * @param amount, asset amount
+     */
     function withdraw(address recipient, IERC20 asset, uint256 amount) external {
         if (paused) {
             revert IsPaused();

diff --git a/contracts/evm/erc20custody/bindings.go b/contracts/evm/erc20custody/bindings.go
@@ -1,6 +1,5 @@
-//go:generate sh -c "solc  ERC20Custody.sol --combined-json abi,bin | jq '.contracts.\"ERC20Custody.sol:ERC20Custody\"'  > ERC20Custody.json"
 //go:generate sh -c "cat ERC20Custody.json | jq .abi > ERC20Custody.abi"
-//go:generate sh -c "cat ERC20Custody.json | jq .bin | tr -d '\"' > ERC20Custody.bin"
+//go:generate sh -c "cat ERC20Custody.json | jq .bytecode | tr -d '\"' > ERC20Custody.bin"
 //go:generate sh -c "abigen --abi ERC20Custody.abi --bin ERC20Custody.bin --pkg erc20custody --type ERC20Custody --out ERC20Custody.go"
 
 package erc20custody

diff --git a/contracts/zevm/ConnectorZEVM.go b/contracts/zevm/ConnectorZEVM.go