doc: internal audit 7

README.md

@@ -158,6 +158,9 @@ The description of bridge-related deployment procedure is very similar to the or
 - [bridges-polygon](https://github.com/valory-xyz/autonolas-governance/blob/main/scripts/deployment/bridges/polygon);
 - [bridges-gnosis](https://github.com/valory-xyz/autonolas-governance/blob/main/scripts/deployment/bridges/gnosis).
 
+The description of ERC20 token bridging deployment between Polygon and Ethereum can be found here:
+[deployment](https://github.com/valory-xyz/autonolas-governance/blob/main/scripts/deployment).
+
 ## Documents
 All the project-related documents are located here: [docs](https://github.com/valory-xyz/autonolas-governance/blob/main/docs).
 

audits/README.md

@@ -17,7 +17,7 @@ An internal audit with a focus on `GovernorOLAS update to latest OZ version` is
 
 An internal audit with a focus on `Guard for Community Multisig (CM)` is located in this folder: [internal audit 6](https://github.com/valory-xyz/autonolas-governance/blob/main/audits/internal6).
 
-
+An internal audit with a focus on `ERC20 bridging via fx-tunnel` is located in this folder: [internal audit 7](https://github.com/valory-xyz/autonolas-governance/blob/main/audits/internal7).
 
 ### External audit
 Following the initial contracts [audit report](https://github.com/valory-xyz/autonolas-governance/blob/main/audits/Valory%20Review%20Final.pdf),

audits/internal7/README.md

@@ -0,0 +1,96 @@
+# autonolas-governance-audit
+The review has been performed based on the contract code in the following repository:<br>
+`https://github.com/valory-xyz/autonolas-governance` <br>
+commit: `93c03a22e1dabfc6cbe70c050d804ea2f903d5c2` or `tag: v1.1.7-pre-internal-audit` <br> 
+
+Update: 27-11-2023  <br>
+
+## Objectives
+The audit focused on ERC20 bridging. <BR>
+
+### Flatten version
+Flatten version of contracts. [contracts](https://github.com/valory-xyz/autonolas-governance/blob/main/audits/internal7/analysis/contracts)
+
+### Coverage
+Hardhat coverage has been performed before the audit and can be found here:
+```sh
+ contracts/bridges/      |    93.42 |      100 |       95 |    95.24 |                |
+  BridgedERC20.sol       |      100 |      100 |      100 |      100 |                |
+  FxERC20ChildTunnel.sol |      100 |      100 |      100 |      100 |                |
+  FxERC20RootTunnel.sol  |    66.67 |      100 |       80 |    71.43 |... 70,72,82,84 |
+```
+Notes: Pay attention to coverage FxERC20RootTunnel.sol
+
+### Security issues
+Details in [slither_full](https://github.com/valory-xyz/autonolas-governance/blob/main/audits/internal7/analysis/slither_full.txt) <br>
+
+#### Bug in constructor: 
+```solidity
+Compilation warnings/errors on ./BridgedERC20-flatten.sol:
+Warning: Unused function parameter. Remove or comment out the variable name to silence this warning.
+   --> BridgedERC20-flatten.sol:231:38:
+    |
+231 |     constructor(string memory _name, string memory _symbol) ERC20(_name, symbol, 18) {
+
+as result of typo
+constructor(string memory _name, string memory _symbol) ERC20(_name, symbol, 18) {
+
+=> ERC20(_name, symbol, 18)
+correct
+ERC20(_name, _symbol, 18)
+
+decimals (18) as constant is OK?
+```
+[x] fixed
+
+#### Reentrancy (not critical since the token is trusted, but it’s better to fix the problem)
+```solidity
+function depositTo(address to, uint256 amount) external {
+        // Check for the address to deposit tokens to
+        if (to == address(0)) {
+            revert ZeroAddress();
+        }
+
+        _deposit(to, amount);
+    }
+    if (amount == 0) {
+            revert ZeroValue();
+        }
+
+        // Deposit tokens on an L2 bridge contract (lock)
+        IERC20(childToken).transferFrom(msg.sender, address(this), amount); --> reentrancy
+
+```
+Notes: It is better to use explicit protection like reentrancy_guard_variable than CEI. <br>
+[x] fixed
+
+#### Non-checked return
+```solidity
+unchecked-transfer
+        // Transfer decoded amount of tokens to a specified address
+        IERC20(childToken).transfer(to, amount);
+
+```
+[x] fixed
+
+#### Non-checked return
+```solidity
+unchecked-transfer
+        // Transfer tokens from sender to this contract address
+        IERC20(rootToken).transferFrom(msg.sender, address(this), amount);
+
+```
+[x] fixed
+
+#### Overengineering
+```solidity
+Unnecessarily overcomplicated part with cast to uint96 + abi.encodePacked It is not recommended to use unless absolutely necessary.
+https://docs.soliditylang.org/en/latest/abi-spec.html#non-standard-packed-mode
+
+bytes memory message = abi.encodePacked(msg.sender, to, uint96(amount));
+https://github.com/pessimistic-io/slitherin/blob/master/docs/dubious_typecast.md
++
+no-inline-assembly
+I'm not sure that it is necessary to parse using assembler
+```
+[x] fixed

audits/internal7/analysis/contracts/BridgedERC20-flatten.sol

@@ -0,0 +1,274 @@
+// Sources flattened with hardhat v2.17.1 https://hardhat.org
+
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.21;
+
+/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation.
+/// @author Solmate (https://github.com/Rari-Capital/solmate/blob/main/src/tokens/ERC20.sol)
+/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol)
+/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it.
+abstract contract ERC20 {
+    /*//////////////////////////////////////////////////////////////
+                                 EVENTS
+    //////////////////////////////////////////////////////////////*/
+
+    event Transfer(address indexed from, address indexed to, uint256 amount);
+
+    event Approval(address indexed owner, address indexed spender, uint256 amount);
+
+    /*//////////////////////////////////////////////////////////////
+                            METADATA STORAGE
+    //////////////////////////////////////////////////////////////*/
+
+    string public name;
+
+    string public symbol;
+
+    uint8 public immutable decimals;
+
+    /*//////////////////////////////////////////////////////////////
+                              ERC20 STORAGE
+    //////////////////////////////////////////////////////////////*/
+
+    uint256 public totalSupply;
+
+    mapping(address => uint256) public balanceOf;
+
+    mapping(address => mapping(address => uint256)) public allowance;
+
+    /*//////////////////////////////////////////////////////////////
+                            EIP-2612 STORAGE
+    //////////////////////////////////////////////////////////////*/
+
+    uint256 internal immutable INITIAL_CHAIN_ID;
+
+    bytes32 internal immutable INITIAL_DOMAIN_SEPARATOR;
+
+    mapping(address => uint256) public nonces;
+
+    /*//////////////////////////////////////////////////////////////
+                               CONSTRUCTOR
+    //////////////////////////////////////////////////////////////*/
+
+    constructor(
+        string memory _name,
+        string memory _symbol,
+        uint8 _decimals
+    ) {
+        name = _name;
+        symbol = _symbol;
+        decimals = _decimals;
+
+        INITIAL_CHAIN_ID = block.chainid;
+        INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator();
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                               ERC20 LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    function approve(address spender, uint256 amount) public virtual returns (bool) {
+        allowance[msg.sender][spender] = amount;
+
+        emit Approval(msg.sender, spender, amount);
+
+        return true;
+    }
+
+    function transfer(address to, uint256 amount) public virtual returns (bool) {
+        balanceOf[msg.sender] -= amount;
+
+        // Cannot overflow because the sum of all user
+        // balances can't exceed the max uint256 value.
+        unchecked {
+            balanceOf[to] += amount;
+        }
+
+        emit Transfer(msg.sender, to, amount);
+
+        return true;
+    }
+
+    function transferFrom(
+        address from,
+        address to,
+        uint256 amount
+    ) public virtual returns (bool) {
+        uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals.
+
+        if (allowed != type(uint256).max) allowance[from][msg.sender] = allowed - amount;
+
+        balanceOf[from] -= amount;
+
+        // Cannot overflow because the sum of all user
+        // balances can't exceed the max uint256 value.
+        unchecked {
+            balanceOf[to] += amount;
+        }
+
+        emit Transfer(from, to, amount);
+
+        return true;
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                             EIP-2612 LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    function permit(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual {
+        require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED");
+
+        // Unchecked because the only math done is incrementing
+        // the owner's nonce which cannot realistically overflow.
+        unchecked {
+            address recoveredAddress = ecrecover(
+                keccak256(
+                    abi.encodePacked(
+                        "\x19\x01",
+                        DOMAIN_SEPARATOR(),
+                        keccak256(
+                            abi.encode(
+                                keccak256(
+                                    "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"
+                                ),
+                                owner,
+                                spender,
+                                value,
+                                nonces[owner]++,
+                                deadline
+                            )
+                        )
+                    )
+                ),
+                v,
+                r,
+                s
+            );
+
+            require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");
+
+            allowance[recoveredAddress][spender] = value;
+        }
+
+        emit Approval(owner, spender, value);
+    }
+
+    function DOMAIN_SEPARATOR() public view virtual returns (bytes32) {
+        return block.chainid == INITIAL_CHAIN_ID ? INITIAL_DOMAIN_SEPARATOR : computeDomainSeparator();
+    }
+
+    function computeDomainSeparator() internal view virtual returns (bytes32) {
+        return
+            keccak256(
+                abi.encode(
+                    keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
+                    keccak256(bytes(name)),
+                    keccak256("1"),
+                    block.chainid,
+                    address(this)
+                )
+            );
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                        INTERNAL MINT/BURN LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    function _mint(address to, uint256 amount) internal virtual {
+        totalSupply += amount;
+
+        // Cannot overflow because the sum of all user
+        // balances can't exceed the max uint256 value.
+        unchecked {
+            balanceOf[to] += amount;
+        }
+
+        emit Transfer(address(0), to, amount);
+    }
+
+    function _burn(address from, uint256 amount) internal virtual {
+        balanceOf[from] -= amount;
+
+        // Cannot underflow because a user's balance
+        // will never be larger than the total supply.
+        unchecked {
+            totalSupply -= amount;
+        }
+
+        emit Transfer(from, address(0), amount);
+    }
+}
+
+
+// File contracts/bridges/BridgedERC20.sol
+/// @dev Only `owner` has a privilege, but the `sender` was provided.
+/// @param sender Sender address.
+/// @param owner Required sender address as an owner.
+error OwnerOnly(address sender, address owner);
+
+/// @dev Provided zero address.
+error ZeroAddress();
+
+
+/// @title BridgedERC20 - Smart contract for bridged ERC20 token
+/// @dev Bridged token contract is owned by the bridge mediator contract, and thus the token representation from
+///      another chain must be minted and burned solely by the bridge mediator contract.
+contract BridgedERC20 is ERC20 {
+    event OwnerUpdated(address indexed owner);
+
+    // Bridged token owner
+    address public owner;
+
+    constructor(string memory _name, string memory _symbol) ERC20(_name, symbol, 18) {
+        owner = msg.sender;
+    }
+
+    /// @dev Changes the owner address.
+    /// @param newOwner Address of a new owner.
+    function changeOwner(address newOwner) external {
+        // Only the contract owner is allowed to change the owner
+        if (msg.sender != owner) {
+            revert OwnerOnly(msg.sender, owner);
+        }
+
+        // Zero address check
+        if (newOwner == address(0)) {
+            revert ZeroAddress();
+        }
+
+        owner = newOwner;
+        emit OwnerUpdated(newOwner);
+    }
+
+    /// @dev Mints bridged tokens.
+    /// @param account Account address.
+    /// @param amount Bridged token amount.
+    function mint(address account, uint256 amount) external {
+        // Only the contract owner is allowed to mint
+        if (msg.sender != owner) {
+            revert OwnerOnly(msg.sender, owner);
+        }
+
+        _mint(account, amount);
+    }
+
+    /// @dev Burns bridged tokens.
+    /// @param amount Bridged token amount to burn.
+    function burn(uint256 amount) external {
+        // Only the contract owner is allowed to burn
+        if (msg.sender != owner) {
+            revert OwnerOnly(msg.sender, owner);
+        }
+
+        _burn(msg.sender, amount);
+    }
+}