Merge the develop branch to the master branch, preparation to v6.0.0-rc1

This set of changes includes the following improvements and fixes: * [Improvement] EIP2612 permit (omni#618) * [Fix] Make _sendMessage internal (omni#613) * [Other] Fix coverage (omni#606) * [Other] Remove unused upgrade dir (omni#339 * [Other] Bump package version before 6.0.0-rc1 (omni#619)
gnosischain · Aug 6, 2021 · b3511bf · b3511bf
2 parents 004d466 + fb66e2a
commit b3511bf
Show file tree

Hide file tree

Showing 41 changed files with 14,011 additions and 24,361 deletions.
diff --git a/.dockerignore b/.dockerignore
@@ -11,6 +11,4 @@ Dockerfile
 flats
 contracts.sublime-project
 contracts.sublime-workspace
-upgrade/*.env*
-!upgrade/.env.example
 build/
diff --git a/.eslintignore b/.eslintignore
@@ -1,3 +1,3 @@
 node_modules
 deploy
-upgrade
+coverage
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -53,7 +53,7 @@ jobs:
           CC_SECRET: ${{ secrets.CC_SECRET }}
   coverage:
     runs-on: ubuntu-latest
-    if: github.ref == 'refs/heads/master' || github.ref == 'refs/heads/develop' || startsWith(github.ref, 'refs/tags')
+    if: github.ref == 'refs/heads/master' || github.ref == 'refs/heads/develop' || startsWith(github.ref, 'refs/tags')  || contains(github.event.head_commit.message, 'coverage')
     steps:
       - uses: actions/setup-node@v1
         with:

diff --git a/.gitignore b/.gitignore
@@ -4,11 +4,10 @@ flats
 .node*
 .idea
 coverage
+coverage.json
 *.sublime-*
 .0x-artifacts
 deploy/*.config
 deploy/*.env*
 !deploy/.env.example
-upgrade/*.env*
-!upgrade/.env.example
 .vscode
diff --git a/.solcover.js b/.solcover.js
@@ -0,0 +1,39 @@
+const Web3 = require('web3')
+
+async function waitCompoundDeploy(config) {
+  const web3 = new Web3(config.provider)
+  const abi = [{
+    inputs: [{ name: "", type: "address"}],
+    outputs: [{ name: "", type: "uint256" }],
+    name: "balanceOf",
+    stateMutability: "view",
+    type: "function"
+  }]
+  const cDai = new web3.eth.Contract(abi, '0x615cba17EE82De39162BB87dBA9BcfD6E8BcF298')
+  const faucet = (await web3.eth.getAccounts())[6]
+  while (true) {
+    try {
+      if (await cDai.methods.balanceOf(faucet).call() !== '0') break
+    } catch (e) {
+      await new Promise(res => setTimeout(res, 1000))
+    }
+  }
+}
+
+module.exports = {
+  skipFiles: [
+    'Migrations.sol',
+    'mocks',
+    'interfaces',
+    'helpers'
+  ],
+  providerOptions: {
+    port: 8545,
+    _chainId: 1337,
+    network_id: 1337,
+    seed: 'TestRPC is awesome!',
+    default_balance_ether: 1000000
+  },
+  onServerReady: waitCompoundDeploy,
+  istanbulReporter: ['lcov']
+}
diff --git a/Dockerfile b/Dockerfile
@@ -23,11 +23,6 @@ COPY ./deploy/package.json ./deploy/
 COPY ./deploy/package-lock.json ./deploy/
 RUN cd ./deploy; npm install --only=prod; cd ..
 
-COPY ./upgrade/package.json ./upgrade/
-COPY ./upgrade/package-lock.json ./upgrade/
-RUN cd ./upgrade; npm install --only=prod; cd ..
-
-COPY ./upgrade ./upgrade
 COPY deploy.sh deploy.sh
 COPY ./deploy ./deploy
 

diff --git a/Dockerfile.dev b/Dockerfile.dev
@@ -14,10 +14,6 @@ COPY ./deploy/package.json ./deploy/
 COPY ./deploy/package-lock.json ./deploy/
 RUN cd ./deploy; npm install; cd ..
 
-COPY ./upgrade/package.json ./upgrade/
-COPY ./upgrade/package-lock.json ./upgrade/
-RUN cd ./upgrade; npm install; cd ..
-
 COPY truffle-config.js truffle-config.js
 COPY ./contracts ./contracts
 RUN npm run compile
@@ -29,7 +25,6 @@ COPY .eslintignore .eslintignore
 COPY .eslintrc .eslintrc
 COPY .prettierrc .prettierrc
 
-COPY ./upgrade ./upgrade
 COPY deploy.sh deploy.sh
 COPY ./deploy ./deploy
 COPY .solhint.json .solhint.json

diff --git a/contracts/ERC677BridgeToken.sol b/contracts/ERC677BridgeToken.sol
@@ -46,7 +46,7 @@ contract ERC677BridgeToken is IBurnableMintableERC677Token, DetailedERC20, Burna
     }
 
     function getTokenInterfacesVersion() external pure returns (uint64 major, uint64 minor, uint64 patch) {
-        return (2, 4, 0);
+        return (2, 5, 0);
     }
 
     function superTransfer(address _to, uint256 _value) internal returns (bool) {

diff --git a/contracts/PermittableToken.sol b/contracts/PermittableToken.sol
@@ -7,8 +7,10 @@ contract PermittableToken is ERC677BridgeToken {
 
     // EIP712 niceties
     bytes32 public DOMAIN_SEPARATOR;
-    // bytes32 public constant PERMIT_TYPEHASH = keccak256("Permit(address holder,address spender,uint256 nonce,uint256 expiry,bool allowed)");
-    bytes32 public constant PERMIT_TYPEHASH = 0xea2aa0a1be11a07ed86d755c93467f4f82362b452371d1ba94d1715123511acb;
+    // bytes32 public constant PERMIT_TYPEHASH_LEGACY = keccak256("Permit(address holder,address spender,uint256 nonce,uint256 expiry,bool allowed)");
+    bytes32 public constant PERMIT_TYPEHASH_LEGACY = 0xea2aa0a1be11a07ed86d755c93467f4f82362b452371d1ba94d1715123511acb;
+    // bytes32 public constant PERMIT_TYPEHASH = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
+    bytes32 public constant PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9;
 
     mapping(address => uint256) public nonces;
     mapping(address => mapping(address => uint256)) public expirations;
@@ -56,7 +58,7 @@ contract PermittableToken is ERC677BridgeToken {
             } else {
                 // If allowance is unlimited by `permit`, `approve`, or `increaseAllowance`
                 // function, don't adjust it. But the expiration date must be empty or in the future
-                require(expirations[_sender][msg.sender] == 0 || expirations[_sender][msg.sender] >= _now());
+                require(expirations[_sender][msg.sender] == 0 || expirations[_sender][msg.sender] >= now);
             }
         } else {
             // If `_sender` is `msg.sender`,
@@ -71,20 +73,16 @@ contract PermittableToken is ERC677BridgeToken {
     /// @param _to The address which will spend the funds.
     /// @param _value The amount of tokens to be spent.
     function approve(address _to, uint256 _value) public returns (bool result) {
-        result = super.approve(_to, _value);
-        if (_value == uint256(-1)) {
-            delete expirations[msg.sender][_to];
-        }
+        _approveAndResetExpirations(msg.sender, _to, _value);
+        return true;
     }
 
     /// @dev Atomically increases the allowance granted to spender by the caller.
     /// @param _to The address which will spend the funds.
     /// @param _addedValue The amount of tokens to increase the allowance by.
     function increaseAllowance(address _to, uint256 _addedValue) public returns (bool result) {
-        result = super.increaseAllowance(_to, _addedValue);
-        if (allowed[msg.sender][_to] == uint256(-1)) {
-            delete expirations[msg.sender][_to];
-        }
+        _approveAndResetExpirations(msg.sender, _to, allowed[msg.sender][_to].add(_addedValue));
+        return true;
     }
 
     /// @dev An alias for `transfer` function.
@@ -134,33 +132,107 @@ contract PermittableToken is ERC677BridgeToken {
         bytes32 _r,
         bytes32 _s
     ) external {
-        require(_holder != address(0));
-        require(_spender != address(0));
-        require(_expiry == 0 || _now() <= _expiry);
-
-        require(_v == 27 || _v == 28);
-        require(uint256(_s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0);
-
-        bytes32 digest = keccak256(
-            abi.encodePacked(
-                "\x19\x01",
-                DOMAIN_SEPARATOR,
-                keccak256(abi.encode(PERMIT_TYPEHASH, _holder, _spender, _nonce, _expiry, _allowed))
-            )
-        );
+        require(_expiry == 0 || now <= _expiry);
 
-        require(_holder == ecrecover(digest, _v, _r, _s));
+        bytes32 digest = _digest(abi.encode(PERMIT_TYPEHASH_LEGACY, _holder, _spender, _nonce, _expiry, _allowed));
+
+        require(_holder == _recover(digest, _v, _r, _s));
         require(_nonce == nonces[_holder]++);
 
         uint256 amount = _allowed ? uint256(-1) : 0;
 
-        allowed[_holder][_spender] = amount;
         expirations[_holder][_spender] = _allowed ? _expiry : 0;
 
-        emit Approval(_holder, _spender, amount);
+        _approve(_holder, _spender, amount);
     }
 
-    function _now() internal view returns (uint256) {
-        return now;
+    /** @dev Allows to spend holder's unlimited amount by the specified spender according to EIP2612.
+     * The function can be called by anyone, but requires having allowance parameters
+     * signed by the holder according to EIP712.
+     * @param _holder The holder's address.
+     * @param _spender The spender's address.
+     * @param _value Allowance value to set as a result of the call.
+     * @param _deadline The deadline timestamp to call the permit function. Must be a timestamp in the future.
+     * Note that timestamps are not precise, malicious miner/validator can manipulate them to some extend.
+     * Assume that there can be a 900 seconds time delta between the desired timestamp and the actual expiration.
+     * @param _v A final byte of signature (ECDSA component).
+     * @param _r The first 32 bytes of signature (ECDSA component).
+     * @param _s The second 32 bytes of signature (ECDSA component).
+     */
+    function permit(
+        address _holder,
+        address _spender,
+        uint256 _value,
+        uint256 _deadline,
+        uint8 _v,
+        bytes32 _r,
+        bytes32 _s
+    ) external {
+        require(now <= _deadline);
+
+        uint256 nonce = nonces[_holder]++;
+        bytes32 digest = _digest(abi.encode(PERMIT_TYPEHASH, _holder, _spender, _value, nonce, _deadline));
+
+        require(_holder == _recover(digest, _v, _r, _s));
+
+        _approveAndResetExpirations(_holder, _spender, _value);
+    }
+
+    /**
+     * @dev Sets a new allowance value for the given owner and spender addresses.
+     * Resets expiration timestamp in case of unlimited approval.
+     * @param _owner address tokens holder.
+     * @param _spender address of tokens spender.
+     * @param _amount amount of approved tokens.
+     */
+    function _approveAndResetExpirations(address _owner, address _spender, uint256 _amount) internal {
+        _approve(_owner, _spender, _amount);
+
+        // it is not necessary to reset _expirations in other cases, since it is only used together with infinite allowance
+        if (_amount == uint256(-1)) {
+            delete expirations[_owner][_spender];
+        }
+    }
+
+    /**
+     * @dev Internal function for issuing an allowance.
+     * @param _owner address of the tokens owner.
+     * @param _spender address of the approved tokens spender.
+     * @param _amount amount of the approved tokens.
+     */
+    function _approve(address _owner, address _spender, uint256 _amount) internal {
+        require(_owner != address(0), "ERC20: approve from the zero address");
+        require(_spender != address(0), "ERC20: approve to the zero address");
+
+        allowed[_owner][_spender] = _amount;
+        emit Approval(_owner, _spender, _amount);
+    }
+
+    /**
+     * @dev Calculates the message digest for encoded EIP712 typed struct.
+     * @param _typedStruct encoded payload.
+     */
+    function _digest(bytes memory _typedStruct) internal view returns (bytes32) {
+        return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, keccak256(_typedStruct)));
+    }
+
+    /**
+     * @dev Derives the signer address for the given message digest and ECDSA signature params.
+     * @param _digest signed message digest.
+     * @param _v a final byte of signature (ECDSA component).
+     * @param _r the first 32 bytes of the signature (ECDSA component).
+     * @param _s the second 32 bytes of the signature (ECDSA component).
+     */
+    function _recover(bytes32 _digest, uint8 _v, bytes32 _r, bytes32 _s) internal pure returns (address) {
+        require(_v == 27 || _v == 28, "ECDSA: invalid signature 'v' value");
+        require(
+            uint256(_s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0,
+            "ECDSA: invalid signature 's' value"
+        );
+
+        address signer = ecrecover(_digest, _v, _r, _s);
+        require(signer != address(0), "ECDSA: invalid signature");
+
+        return signer;
     }
 }
diff --git a/.../gsn/token_paymaster/IUniswapRouter02.sol → ...sn/token_paymaster/IUniswapV2Router02.sol b/.../gsn/token_paymaster/IUniswapRouter02.sol → ...sn/token_paymaster/IUniswapV2Router02.sol
diff --git a/contracts/gsn/token_paymaster/TokenPaymaster.sol b/contracts/gsn/token_paymaster/TokenPaymaster.sol
@@ -4,7 +4,7 @@ pragma experimental ABIEncoderV2;
 
 import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";
 import "../BasePaymaster.sol";
-import "./IUniswapRouter02.sol";
+import "./IUniswapV2Router02.sol";
 import "../../upgradeable_contracts/GSNForeignERC20Bridge.sol";
 
 contract TokenPaymaster is BasePaymaster {

diff --git a/contracts/libraries/ArbitraryMessage.sol b/contracts/libraries/ArbitraryMessage.sol
@@ -48,11 +48,6 @@ library ArbitraryMessage {
             gasLimit := and(shr(64, blob), 0xffffffff)
 
             dataType := byte(26, blob)
-            if gt(and(dataType, 0x7f), 0) {
-                // for now, only 0x00 and 0x80 datatypes are supported - regular AMB calls
-                // other dataType values are kept reserved for future use
-                revert(0, 0)
-            }
 
             // load source chain id length
             let chainIdLength := byte(24, blob)

diff --git a/contracts/mocks/ERC677BridgeTokenRewardableMock.sol b/contracts/mocks/ERC677BridgeTokenRewardableMock.sol
@@ -19,13 +19,4 @@ contract ERC677BridgeTokenRewardableMock is ERC677BridgeTokenRewardable {
     function setStakingContractMock(address _stakingContract) public {
         stakingContract = _stakingContract;
     }
-
-    function setNow(uint256 _timestamp) public {
-        _blockTimestamp = _timestamp;
-    }
-
-    function _now() internal view returns (uint256) {
-        return _blockTimestamp != 0 ? _blockTimestamp : now;
-    }
-
 }
diff --git a/contracts/mocks/ForeignAMBWithGasTokenMock.sol b/contracts/mocks/ForeignAMBWithGasTokenMock.sol
@@ -9,7 +9,7 @@ import "../upgradeable_contracts/arbitrary_message/ForeignAMBWithGasToken.sol";
 contract ForeignAMBWithGasTokenMock is ForeignAMBWithGasToken {
     function gasToken() public pure returns (IGasToken) {
         // Address generated in unit test, also hardcoded in GasTokenMock
-        return IGasToken(0x5b1869D9A4C187F2EAa108f3062412ecf0526b24);
+        return IGasToken(0x64830eD3d58194d5b3Bc1BEa19F1ce9666AC0602);
     }
 
     function collectGasTokens() external {

diff --git a/contracts/mocks/PermittableTokenMock.sol b/contracts/mocks/PermittableTokenMock.sol
diff --git a/contracts/mocks/UniswapRouterMock.sol b/contracts/mocks/UniswapRouterMock.sol
@@ -10,6 +10,7 @@ contract UniswapRouterMock {
     }
 
     function getAmountsOut(uint256 amountIn, address[] path) external pure returns (uint256[] memory amounts) {
+        (path);
         amounts = new uint256[](2);
         amounts[0] = amountIn;
         amounts[1] = amountIn / 100;
@@ -20,6 +21,7 @@ contract UniswapRouterMock {
         external
         returns (uint256[] memory amounts)
     {
+        (amountInMax, path, to, deadline);
         uint256 ethToSend = amountOut;
         uint256 tokensToTake = ethToSend * 100;
 

diff --git a/contracts/upgradeable_contracts/arbitrary_message/MessageDelivery.sol b/contracts/upgradeable_contracts/arbitrary_message/MessageDelivery.sol
@@ -28,7 +28,7 @@ contract MessageDelivery is BasicAMB, MessageProcessor {
     * @param _gas gas limit used on the other network for executing a message
     * @param _dataType AMB message dataType to be included as a part of the header
     */
-    function _sendMessage(address _contract, bytes _data, uint256 _gas, uint256 _dataType) public returns (bytes32) {
+    function _sendMessage(address _contract, bytes _data, uint256 _gas, uint256 _dataType) internal returns (bytes32) {
         // it is not allowed to pass messages while other messages are processed
         // if other is not explicitly configured
         require(messageId() == bytes32(0) || allowReentrantRequests());

diff --git a/contracts/upgradeable_contracts/arbitrary_message/VersionableAMB.sol b/contracts/upgradeable_contracts/arbitrary_message/VersionableAMB.sol
@@ -17,6 +17,6 @@ contract VersionableAMB is VersionableBridge {
      * @return (major, minor, patch) version triple
      */
     function getBridgeInterfacesVersion() external pure returns (uint64 major, uint64 minor, uint64 patch) {
-        return (6, 0, 0);
+        return (6, 1, 0);
     }
 }