Update setBlockRewardContract method validations (#197)

* Update setBlockReward checks
omni · Jun 19, 2019 · 942e6c4 · 942e6c4
1 parent 50656eb
commit 942e6c4
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 7 deletions.
diff --git a/contracts/IBlockReward.sol b/contracts/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/test/BlockReward.sol b/contracts/test/BlockReward.sol
@@ -4,7 +4,7 @@ import "../IBlockReward.sol";
 import "../libraries/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"));
+    }
 }
diff --git a/contracts/upgradeable_contracts/erc20_to_erc20/FeeManagerErcToErcPOSDAO.sol b/contracts/upgradeable_contracts/erc20_to_erc20/FeeManagerErcToErcPOSDAO.sol
@@ -13,7 +13,17 @@ contract FeeManagerErcToErcPOSDAO is BlockRewardFeeManager {
     }
 
     function setBlockRewardContract(address _blockReward) external {
-        require(_blockReward != address(0) && isContract(_blockReward) && (IBlockReward(_blockReward).bridgesAllowedLength() != 0));
+        require(_blockReward != address(0) && isContract(_blockReward));
+
+        // Before store the contract we need to make sure that it is the block reward contract in actual fact,
+        // call a specific method from the contract that should return a specific value
+        bool isBlockRewardContract = false;
+        if (_blockReward.call(abi.encodeWithSignature("blockRewardContractId()"))) {
+            isBlockRewardContract = IBlockReward(_blockReward).blockRewardContractId() == bytes4(keccak256("blockReward"));
+        } else if (_blockReward.call(abi.encodeWithSignature("bridgesAllowedLength()"))) {
+            isBlockRewardContract = IBlockReward(_blockReward).bridgesAllowedLength() != 0;
+        }
+        require(isBlockRewardContract);
         addressStorage[keccak256(abi.encodePacked("blockRewardContract"))] = _blockReward;
     }
 

diff --git a/contracts/upgradeable_contracts/erc20_to_native/HomeBridgeErcToNative.sol b/contracts/upgradeable_contracts/erc20_to_native/HomeBridgeErcToNative.sol
@@ -116,7 +116,17 @@ contract HomeBridgeErcToNative is EternalStorage, BasicBridge, BasicHomeBridge,
     }
 
     function setBlockRewardContract(address _blockReward) public onlyOwner {
-        require(_blockReward != address(0) && isContract(_blockReward) && (IBlockReward(_blockReward).bridgesAllowedLength() != 0));
+        require(_blockReward != address(0) && isContract(_blockReward));
+
+        // Before store the contract we need to make sure that it is the block reward contract in actual fact,
+        // call a specific method from the contract that should return a specific value
+        bool isBlockRewardContract = false;
+        if (_blockReward.call(abi.encodeWithSignature("blockRewardContractId()"))) {
+            isBlockRewardContract = IBlockReward(_blockReward).blockRewardContractId() == bytes4(keccak256("blockReward"));
+        } else if (_blockReward.call(abi.encodeWithSignature("bridgesAllowedLength()"))) {
+            isBlockRewardContract = IBlockReward(_blockReward).bridgesAllowedLength() != 0;
+        }
+        require(isBlockRewardContract);
         addressStorage[keccak256(abi.encodePacked("blockRewardContract"))] = _blockReward;
     }
 

diff --git a/test/erc_to_erc/home_bridge.test.js b/test/erc_to_erc/home_bridge.test.js
@@ -7,6 +7,7 @@ const ERC677BridgeTokenRewardable = artifacts.require('ERC677BridgeTokenRewardab
 const FeeManagerErcToErcPOSDAO = artifacts.require('FeeManagerErcToErcPOSDAO.sol')
 const RewardableValidators = artifacts.require('RewardableValidators.sol')
 const BlockReward = artifacts.require('BlockReward')
+const OldBlockReward = artifacts.require('oldBlockReward')
 
 const { expect } = require('chai')
 const { ERROR_MSG, ZERO_ADDRESS, toBN } = require('../setup')
@@ -1255,6 +1256,10 @@ contract('HomeBridge_ERC20_to_ERC20', async accounts => {
       // Then
       const newBlockReward = await homeBridge.blockRewardContract()
       newBlockReward.should.be.equals(newBlockRewardContract.address)
+
+      const oldBlockRewardContract = await OldBlockReward.new()
+      await homeBridge.setBlockRewardContract(oldBlockRewardContract.address).should.be.fulfilled
+      oldBlockRewardContract.address.should.be.equal(await homeBridge.blockRewardContract())
     })
   })
   describe('#onTokenTransfer', async () => {

diff --git a/test/erc_to_native/home_bridge.test.js b/test/erc_to_native/home_bridge.test.js
@@ -2,6 +2,7 @@ const HomeBridge = artifacts.require('HomeBridgeErcToNative.sol')
 const EternalStorageProxy = artifacts.require('EternalStorageProxy.sol')
 const BridgeValidators = artifacts.require('BridgeValidators.sol')
 const BlockReward = artifacts.require('BlockReward')
+const OldBlockReward = artifacts.require('oldBlockReward')
 const RewardableValidators = artifacts.require('RewardableValidators.sol')
 const FeeManagerErcToNative = artifacts.require('FeeManagerErcToNative.sol')
 const FeeManagerErcToNativePOSDAO = artifacts.require('FeeManagerErcToNativePOSDAO')
@@ -108,6 +109,10 @@ contract('HomeBridge_ERC20_to_Native', async accounts => {
 
       await homeContract.setBlockRewardContract(validatorContract.address).should.be.rejectedWith(ERROR_MSG)
       secondBlockRewardContract.address.should.be.equal(await homeContract.blockRewardContract())
+
+      const oldBlockRewardContract = await OldBlockReward.new()
+      await homeContract.setBlockRewardContract(oldBlockRewardContract.address).should.be.fulfilled
+      oldBlockRewardContract.address.should.be.equal(await homeContract.blockRewardContract())
     })
 
     it('cant set maxPerTx > dailyLimit', async () => {

diff --git a/test/testContracts/oldBlockReward.sol b/test/testContracts/oldBlockReward.sol
@@ -0,0 +1,8 @@
+pragma solidity 0.4.24;
+
+contract oldBlockReward {
+
+    function bridgesAllowedLength() external view returns(uint256) {
+        return 3;
+    }
+}