Merge branch 'develop' into arbitrary-message-bridging-#73

# Conflicts: # contracts/upgradeable_contracts/BasicBridge.sol # contracts/upgradeable_contracts/ERC677Bridge.sol # contracts/upgradeable_contracts/erc20_to_erc20/BasicForeignBridgeErcToErc.sol # contracts/upgradeable_contracts/erc20_to_erc20/HomeBridgeErcToErc.sol # contracts/upgradeable_contracts/erc20_to_native/ForeignBridgeErcToNative.sol # contracts/upgradeable_contracts/erc20_to_native/HomeBridgeErcToNative.sol # contracts/upgradeable_contracts/native_to_erc20/ForeignBridgeNativeToErc.sol # contracts/upgradeable_contracts/native_to_erc20/HomeBridgeNativeToErc.sol # package-lock.json # package.json
omni · Jul 15, 2019 · 2c50fcb · 2c50fcb
2 parents 3c18fa8 + df1fa67
commit 2c50fcb
Show file tree

Hide file tree

Showing 69 changed files with 18,755 additions and 2,957 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,4 @@ build
 flats
 .node*
 .idea
+coverage
diff --git a/.node-xmlhttprequest-sync-27493 b/.node-xmlhttprequest-sync-27493
diff --git a/.solcover.js b/.solcover.js
diff --git a/.travis.yml b/.travis.yml
@@ -9,10 +9,6 @@ cache:
 env:
   - SOLIDITY_COVERAGE=false
   - SOLIDITY_COVERAGE=true
-matrix:
-  fast_finish: true
-  allow_failures:
-    - env: SOLIDITY_COVERAGE=true
 script:
   - yarn lint:js
   - yarn test
diff --git a/Dockerfile b/Dockerfile
@@ -1,5 +1,9 @@
 FROM node:10
 
+RUN apt-get update
+RUN apt-get install -y netcat
+RUN apt-get clean
+
 WORKDIR /contracts
 
 COPY package.json .

diff --git a/README.md b/README.md
@@ -1,5 +1,6 @@
 [![Join the chat at https://gitter.im/poanetwork/poa-bridge](https://badges.gitter.im/poanetwork/poa-bridge.svg)](https://gitter.im/poanetwork/poa-bridge?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
 [![Build Status](https://travis-ci.org/poanetwork/poa-parity-bridge-contracts.svg?branch=master)](https://travis-ci.org/poanetwork/poa-parity-bridge-contracts)
+[![Coverage Status](https://coveralls.io/repos/github/poanetwork/poa-bridge-contracts/badge.svg?branch=master)](https://coveralls.io/github/poanetwork/poa-bridge-contracts?branch=master)
 
 # POA Bridge Smart Contracts
 These contracts provide the core functionality for the POA bridge. They implement the logic to relay assests between two EVM-based blockchain networks. The contracts collect bridge validator's signatures to approve and facilitate relay operations. 
@@ -81,6 +82,13 @@ Please the [README.md](deploy/README.md) in the `deploy` folder for instructions
 npm test
 ```
 
+#### Run coverage tests
+```bash
+npm run coverage
+```
+
+The results can be found in the `coverage` directory.
+
 #### Flatten
 Fattened contracts can be used to verify the contract code in a block explorer like BlockScout or Etherscan.
 The following command will prepare flattened version of the contracts:
@@ -121,6 +129,13 @@ _Note: The container must be rebuilt every time the code in a contract or deploy
    docker cp name-of-your-container:/contracts/flats ./
    ```
 
+#### Test contract and run coverage (if needed)
+```bash
+$ docker-compose run bridge-contracts bash
+$ npm test
+$ npm run coverage
+```
+
 #### Shutdown the container
 If the container is no longer needed, it can be shutdown:
 ```bash

diff --git a/REWARD_MANAGEMENT.md b/REWARD_MANAGEMENT.md
@@ -28,11 +28,12 @@ Fees are calculated and distributed on Home network. Validators will receive nat
 Fees are calculated and distributed on Home network. Validators will receive native coins.
 ![native-erc-homefee-foreigntohome](https://user-images.githubusercontent.com/4614574/53118176-4b9da800-352b-11e9-8118-123f30e37d61.png)
 
-## ERC-TO-NATIVE
+## ERC-TO-NATIVE - Fees distributed among bridge validators
 Configuration:
 ```
 HOME_REWARDABLE=BOTH_DIRECTIONS
 FOREIGN_REWARDABLE=false
+HOME_FEE_MANAGER_TYPE=BRIDGE_VALIDATORS_REWARD
 ```
 ### Foreign to Home transfer
 Fees are calculated and distributed on Home network. Validators will receive native coins.
@@ -42,12 +43,32 @@ Fees are calculated and distributed on Home network. Validators will receive nat
 Fees are calculated and distributed on Home network. Validators will receive native coins.
 ![erc-native-hometoforeign](https://user-images.githubusercontent.com/4614574/51607508-96f47480-1ef3-11e9-93a1-0f1111793f2a.png)
 
-## ERC-TO-ERC
+## ERC-TO-NATIVE - Fees distributed among network validators
+Configuration:
+```
+HOME_REWARDABLE=BOTH_DIRECTIONS
+FOREIGN_REWARDABLE=false
+HOME_FEE_MANAGER_TYPE=POSDAO_REWARD
+```
+### Foreign to Home transfer
+Fees are calculated and distributed on Home network. Validators will receive native coins.
+![POSDAO-ERC-Native-ForeignToHome](https://user-images.githubusercontent.com/4614574/59941961-ebfdcd80-9434-11e9-8c9a-433f75bd2c09.png)
+
+### Home to Foreign transfer
+Fees are calculated and distributed on Home network. Validators will receive native coins.
+![POSDAO-ERC-Native-HomeToForeign](https://user-images.githubusercontent.com/4614574/59941982-f750f900-9434-11e9-8557-a90d9dfa6799.png)
 
+## ERC-TO-ERC
+Configuration:
+```
+HOME_REWARDABLE=BOTH_DIRECTIONS
+FOREIGN_REWARDABLE=false
+```
 ### Foreign to Home transfer
 Fees are calculated and distributed on Home network. Validators will receive ERC20 tokens.
-![ERC-ERC-ForeignToHome (1)](https://user-images.githubusercontent.com/4614574/56502412-98c8d680-64e8-11e9-8eea-5bcd545d74d9.png)
+![ERC-ERC-ForeignToHome](https://user-images.githubusercontent.com/4614574/59939650-016ff900-942f-11e9-9593-9861455c7b62.png)
 
 ### Home to Foreign transfer
 Fees are calculated and distributed on Home network. Validators will receive ERC20 tokens.
-![ERC-ERC-HomeToForeign (1)](https://user-images.githubusercontent.com/4614574/56502454-b8f89580-64e8-11e9-84ae-d9a1c229e0c4.png)
+![ERC-ERC-HomeToForeign](https://user-images.githubusercontent.com/4614574/59939670-0cc32480-942f-11e9-9693-727125555c97.png)
+
diff --git a/contracts/ERC677BridgeToken.sol b/contracts/ERC677BridgeToken.sol
@@ -3,15 +3,15 @@ pragma solidity 0.4.24;
 import "openzeppelin-solidity/contracts/token/ERC20/BurnableToken.sol";
 import "openzeppelin-solidity/contracts/token/ERC20/MintableToken.sol";
 import "openzeppelin-solidity/contracts/token/ERC20/DetailedERC20.sol";
-import "./IBurnableMintableERC677Token.sol";
-import "./ERC677Receiver.sol";
-
+import "./interfaces/IBurnableMintableERC677Token.sol";
+import "./upgradeable_contracts/Claimable.sol";
 
 contract ERC677BridgeToken is
     IBurnableMintableERC677Token,
     DetailedERC20,
     BurnableToken,
-    MintableToken {
+    MintableToken,
+    Claimable {
 
     address public bridgeContract;
 
@@ -24,7 +24,7 @@ contract ERC677BridgeToken is
     public DetailedERC20(_name, _symbol, _decimals) {}
 
     function setBridgeContract(address _bridgeContract) onlyOwner public {
-        require(_bridgeContract != address(0) && isContract(_bridgeContract));
+        require(isContract(_bridgeContract));
         bridgeContract = _bridgeContract;
     }
 
@@ -40,13 +40,13 @@ contract ERC677BridgeToken is
         emit Transfer(msg.sender, _to, _value, _data);
 
         if (isContract(_to)) {
-            require(contractFallback(_to, _value, _data));
+            require(contractFallback(msg.sender, _to, _value, _data));
         }
         return true;
     }
 
     function getTokenInterfacesVersion() public pure returns(uint64 major, uint64 minor, uint64 patch) {
-        return (2, 0, 0);
+        return (2, 1, 0);
     }
 
     function superTransfer(address _to, uint256 _value) internal returns(bool)
@@ -57,21 +57,28 @@ contract ERC677BridgeToken is
     function transfer(address _to, uint256 _value) public returns (bool)
     {
         require(superTransfer(_to, _value));
-        if (isContract(_to) && !contractFallback(_to, _value, new bytes(0))) {
+        callAfterTransfer(msg.sender, _to, _value);
+        return true;
+    }
+
+    function transferFrom(address _from, address _to, uint256 _value) public returns (bool) {
+        require(super.transferFrom(_from, _to, _value));
+        callAfterTransfer(_from, _to, _value);
+        return true;
+    }
+
+    function callAfterTransfer(address _from, address _to, uint256 _value) internal {
+        if (isContract(_to) && !contractFallback(_from, _to, _value, new bytes(0))) {
             if (_to == bridgeContract) {
                 revert();
             } else {
-                emit ContractFallbackCallFailed(msg.sender, _to, _value);
+                emit ContractFallbackCallFailed(_from, _to, _value);
             }
         }
-        return true;
     }
 
-    function contractFallback(address _to, uint _value, bytes _data)
-        private
-        returns(bool)
-    {
-        return _to.call(abi.encodeWithSignature("onTokenTransfer(address,uint256,bytes)",  msg.sender, _value, _data));
+    function contractFallback(address _from, address _to, uint _value, bytes _data) private returns(bool) {
+        return _to.call(abi.encodeWithSignature("onTokenTransfer(address,uint256,bytes)",  _from, _value, _data));
     }
 
     function isContract(address _addr)
@@ -92,17 +99,7 @@ contract ERC677BridgeToken is
         revert();
     }
 
-    function claimTokens(address _token, address _to) public onlyOwner {
-        require(_to != address(0));
-        if (_token == address(0)) {
-            _to.transfer(address(this).balance);
-            return;
-        }
-
-        DetailedERC20 token = DetailedERC20(_token);
-        uint256 balance = token.balanceOf(address(this));
-        require(token.transfer(_to, balance));
+    function claimTokens(address _token, address _to) public onlyOwner validAddress(_to) {
+        claimValues(_token, _to);
     }
-
-
 }
diff --git a/contracts/ERC677BridgeTokenRewardable.sol b/contracts/ERC677BridgeTokenRewardable.sol
@@ -15,12 +15,12 @@ contract ERC677BridgeTokenRewardable is ERC677BridgeToken {
     ) public ERC677BridgeToken(_name, _symbol, _decimals) {}
 
     function setBlockRewardContract(address _blockRewardContract) onlyOwner public {
-        require(_blockRewardContract != address(0) && isContract(_blockRewardContract));
+        require(isContract(_blockRewardContract));
         blockRewardContract = _blockRewardContract;
     }
 
     function setStakingContract(address _stakingContract) onlyOwner public {
-        require(_stakingContract != address(0) && isContract(_stakingContract));
+        require(isContract(_stakingContract));
         stakingContract = _stakingContract;
     }
 

diff --git a/contracts/IOwnedUpgradeabilityProxy.sol b/contracts/IOwnedUpgradeabilityProxy.sol
diff --git a/contracts/ERC677.sol → contracts/interfaces/ERC677.sol b/contracts/ERC677.sol → contracts/interfaces/ERC677.sol
diff --git a/contracts/ERC677Receiver.sol → contracts/interfaces/ERC677Receiver.sol b/contracts/ERC677Receiver.sol → contracts/interfaces/ERC677Receiver.sol
diff --git a/contracts/IBlockReward.sol → contracts/interfaces/IBlockReward.sol b/contracts/IBlockReward.sol → contracts/interfaces/IBlockReward.sol
@@ -8,4 +8,5 @@ interface IBlockReward {
     function bridgesAllowedLength() external view returns(uint256);
     function addBridgeTokenFeeReceivers(uint256 _amount) external;
     function addBridgeNativeFeeReceivers(uint256 _amount) external;
+    function blockRewardContractId() public pure returns(bytes4);
 }
diff --git a/contracts/IBridgeValidators.sol → contracts/interfaces/IBridgeValidators.sol b/contracts/IBridgeValidators.sol → contracts/interfaces/IBridgeValidators.sol
diff --git a/contracts/IBurnableMintableERC677Token.sol → ...terfaces/IBurnableMintableERC677Token.sol b/contracts/IBurnableMintableERC677Token.sol → ...terfaces/IBurnableMintableERC677Token.sol
@@ -1,5 +1,5 @@
 pragma solidity 0.4.24;
-import "./ERC677.sol";
+import "../interfaces/ERC677.sol";
 
 
 contract IBurnableMintableERC677Token is ERC677 {

diff --git a/contracts/IRewardableValidators.sol → ...acts/interfaces/IRewardableValidators.sol b/contracts/IRewardableValidators.sol → ...acts/interfaces/IRewardableValidators.sol
diff --git a/contracts/interfaces/IUpgradeabilityOwnerStorage.sol b/contracts/interfaces/IUpgradeabilityOwnerStorage.sol
@@ -0,0 +1,6 @@
+pragma solidity 0.4.24;
+
+
+interface IUpgradeabilityOwnerStorage {
+    function upgradeabilityOwner() public view returns (address);
+}
diff --git a/contracts/libraries/Message.sol b/contracts/libraries/Message.sol
@@ -1,5 +1,5 @@
 pragma solidity 0.4.24;
-import "../IBridgeValidators.sol";
+import "../interfaces/IBridgeValidators.sol";
 
 
 library Message {
@@ -38,11 +38,10 @@ library Message {
     // offset 84: 32 bytes :: bytes32 - transaction hash
     // offset 104: 20 bytes :: address - contract address to prevent double spending
 
-    // bytes 1 to 32 are 0 because message length is stored as little endian.
     // mload always reads 32 bytes.
     // so we can and have to start reading recipient at offset 20 instead of 32.
     // if we were to read at 32 the address would contain part of value and be corrupted.
-    // when reading from offset 20 mload will read 12 zero bytes followed
+    // when reading from offset 20 mload will read 12 bytes (most of them zeros) followed
     // by the 20 recipient address bytes and correctly convert it into an address.
     // this saves some storage/gas over the alternative solution
     // which is padding address to 32 bytes and reading recipient at offset 32.
@@ -55,7 +54,7 @@ library Message {
     {
         require(isMessageValid(message));
         assembly {
-            recipient := and(mload(add(message, 20)), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
+            recipient := mload(add(message, 20))
             amount := mload(add(message, 52))
             txHash := mload(add(message, 84))
             contractAddress := mload(add(message, 104))
@@ -86,7 +85,6 @@ library Message {
 
     function hashMessage(bytes message) internal pure returns (bytes32) {
         bytes memory prefix = "\x19Ethereum Signed Message:\n";
-        // message is always 84 length
         string memory msgLength = "104";
         return keccak256(abi.encodePacked(prefix, msgLength, message));
     }
@@ -99,6 +97,9 @@ library Message {
         IBridgeValidators _validatorContract) internal view {
         require(isMessageValid(_message));
         uint256 requiredSignatures = _validatorContract.requiredSignatures();
+        // It is not necessary to check that arrays have the same length since it will be handled
+        // during attempt to access to the corresponding elements in the loop and the call will be reverted.
+        // It will save gas for the rational validators actions and still be safe enough from security point of view
         require(_vs.length >= requiredSignatures);
         bytes32 hash = hashMessage(_message);
         address[] memory encounteredAddresses = new address[](requiredSignatures);

diff --git a/contracts/libraries/SafeMath.sol b/contracts/libraries/SafeMath.sol
diff --git a/contracts/test/BlockReward.sol b/contracts/test/BlockReward.sol
@@ -1,10 +1,10 @@
 pragma solidity 0.4.24;
 
-import "../IBlockReward.sol";
-import "../libraries/SafeMath.sol";
+import "../interfaces/IBlockReward.sol";
+import "openzeppelin-solidity/contracts/math/SafeMath.sol";
 
 
-contract BlockReward is IBlockReward {
+contract BlockReward {
     using SafeMath for uint256;
 
     address[] public validatorList;
@@ -17,10 +17,6 @@ contract BlockReward is IBlockReward {
     function () external payable {
     }
 
-    function bridgesAllowedLength() external view returns(uint256) {
-        return 3;
-    }
-
     function addExtraReceiver(uint256 _amount, address _receiver) external {
         require(_amount > 0);
         require(_receiver != address(0));
@@ -98,4 +94,8 @@ contract BlockReward is IBlockReward {
     function random(uint256 _count) public view returns(uint256) {
         return uint256(blockhash(block.number.sub(1))) % _count;
     }
+
+    function blockRewardContractId() public pure returns(bytes4) {
+        return bytes4(keccak256("blockReward"));
+    }
 }