Update security roles on v1 contracts

contracts/IOwnedUpgradeabilityProxy.sol

@@ -0,0 +1,6 @@
+pragma solidity 0.4.23;
+
+
+interface IOwnedUpgradeabilityProxy {
+    function proxyOwner() public view returns (address);
+}

contracts/upgradeable_contracts/OwnedUpgradeability.sol

@@ -0,0 +1,17 @@
+pragma solidity 0.4.23;
+
+import "../IOwnedUpgradeabilityProxy.sol";
+
+
+contract OwnedUpgradeability {
+
+    function upgradeabilityAdmin() public view returns (address) {
+        return IOwnedUpgradeabilityProxy(this).proxyOwner();
+    }
+
+    // Avoid using onlyProxyOwner name to prevent issues with implementation from proxy contract
+    modifier onlyIfOwnerOfProxy() {
+        require(msg.sender == upgradeabilityAdmin());
+        _;
+    }
+}

contracts/upgradeable_contracts/U_BasicBridge.sol

@@ -1,24 +1,13 @@
 pragma solidity 0.4.23;
 import "../IBridgeValidators.sol";
 import "../upgradeability/EternalStorage.sol";
+import "./Validatable.sol";
+import "./Ownable.sol";
 
 
-contract BasicBridge is EternalStorage {
+contract BasicBridge is EternalStorage, Validatable, Ownable {
     event GasPriceChanged(uint256 gasPrice);
     event RequiredBlockConfirmationChanged(uint256 requiredBlockConfirmations);
-    function validatorContract() public view returns(IBridgeValidators) {
-        return IBridgeValidators(addressStorage[keccak256("validatorContract")]);
-    }
-
-    modifier onlyValidator() {
-        require(validatorContract().isValidator(msg.sender));
-        _;
-    }
-
-    modifier onlyOwner() {
-        require(validatorContract().owner() == msg.sender);
-        _;
-    }
 
     function setGasPrice(uint256 _gasPrice) public onlyOwner {
         require(_gasPrice > 0);

contracts/upgradeable_contracts/U_ForeignBridge.sol

@@ -5,9 +5,10 @@ import "./U_BasicBridge.sol";
 import "../IBurnableMintableERC677Token.sol";
 import "../ERC677Receiver.sol";
 import "openzeppelin-solidity/contracts/token/ERC20/ERC20Basic.sol";
+import "./OwnedUpgradeability.sol";
 
 
-contract ForeignBridge is ERC677Receiver, BasicBridge {
+contract ForeignBridge is ERC677Receiver, BasicBridge, OwnedUpgradeability {
     using SafeMath for uint256;
     /// triggered when relay of deposit from HomeBridge is complete
     event Deposit(address recipient, uint value, bytes32 transactionHash);
@@ -34,13 +35,15 @@ contract ForeignBridge is ERC677Receiver, BasicBridge {
         uint256 _foreignGasPrice,
         uint256 _requiredBlockConfirmations,
         uint256 _homeDailyLimit,
-        uint256 _homeMaxPerTx
+        uint256 _homeMaxPerTx,
+        address _owner
     ) public returns(bool) {
         require(!isInitialized());
         require(_validatorContract != address(0));
         require(_minPerTx > 0 && _maxPerTx > _minPerTx && _foreignDailyLimit > _maxPerTx);
         require(_homeMaxPerTx < _homeDailyLimit);
         require(_foreignGasPrice > 0);
+        require(_owner != address(0));
         addressStorage[keccak256("validatorContract")] = _validatorContract;
         setErc677token(_erc677token);
         uintStorage[keccak256("foreignDailyLimit")] = _foreignDailyLimit;
@@ -51,6 +54,7 @@ contract ForeignBridge is ERC677Receiver, BasicBridge {
         uintStorage[keccak256("requiredBlockConfirmations")] = _requiredBlockConfirmations;
         uintStorage[keccak256("homeDailyLimit")] = _homeDailyLimit;
         uintStorage[keccak256("homeMaxPerTx")] = _homeMaxPerTx;
+        setOwner(_owner);
         setInitialize(true);
         return isInitialized();
     }
@@ -79,7 +83,7 @@ contract ForeignBridge is ERC677Receiver, BasicBridge {
         uintStorage[keccak256("minPerTx")] = _minPerTx;
     }
 
-    function claimTokens(address _token, address _to) external onlyOwner {
+    function claimTokens(address _token, address _to) external onlyIfOwnerOfProxy {
         require(_to != address(0));
         if (_token == address(0)) {
             _to.transfer(address(this).balance);
@@ -91,7 +95,7 @@ contract ForeignBridge is ERC677Receiver, BasicBridge {
         require(token.transfer(_to, balance));
     }
 
-    function claimTokensFromErc677(address _token, address _to) external onlyOwner {
+    function claimTokensFromErc677(address _token, address _to) external onlyIfOwnerOfProxy {
         erc677token().claimTokens(_token, _to);
     }
 

contracts/upgradeable_contracts/U_HomeBridge.sol

@@ -3,14 +3,16 @@ import "../libraries/SafeMath.sol";
 import "../libraries/Message.sol";
 import "./U_BasicBridge.sol";
 import "../upgradeability/EternalStorage.sol";
+import "./OwnedUpgradeability.sol";
+
 
 contract Sacrifice {
     constructor(address _recipient) public payable {
         selfdestruct(_recipient);
     }
 }
 
-contract HomeBridge is EternalStorage, BasicBridge {
+contract HomeBridge is EternalStorage, BasicBridge, OwnedUpgradeability {
     using SafeMath for uint256;
     event GasConsumptionLimitsUpdated(uint256 gas);
     event Deposit (address recipient, uint256 value);
@@ -26,7 +28,8 @@ contract HomeBridge is EternalStorage, BasicBridge {
         uint256 _homeGasPrice,
         uint256 _requiredBlockConfirmations,
         uint256 _foreignDailyLimit,
-        uint256 _foreignMaxPerTx
+        uint256 _foreignMaxPerTx,
+        address _owner
     ) public
       returns(bool)
     {
@@ -36,6 +39,7 @@ contract HomeBridge is EternalStorage, BasicBridge {
         require(_requiredBlockConfirmations > 0);
         require(_minPerTx > 0 && _maxPerTx > _minPerTx && _homeDailyLimit > _maxPerTx);
         require(_foreignMaxPerTx < _foreignDailyLimit);
+        require(_owner != address(0));
         addressStorage[keccak256("validatorContract")] = _validatorContract;
         uintStorage[keccak256("deployedAtBlock")] = block.number;
         uintStorage[keccak256("homeDailyLimit")] = _homeDailyLimit;
@@ -45,6 +49,7 @@ contract HomeBridge is EternalStorage, BasicBridge {
         uintStorage[keccak256("requiredBlockConfirmations")] = _requiredBlockConfirmations;
         uintStorage[keccak256("foreignDailyLimit")] = _foreignDailyLimit;
         uintStorage[keccak256("foreignMaxPerTx")] = _foreignMaxPerTx;
+        setOwner(_owner);
         setInitialize(true);
         return isInitialized();
     }

contracts/upgradeable_contracts/Validatable.sol

@@ -0,0 +1,16 @@
+pragma solidity 0.4.23;
+import "../IBridgeValidators.sol";
+import "../upgradeability/EternalStorage.sol";
+
+
+contract Validatable is EternalStorage {
+
+    function validatorContract() public view returns(IBridgeValidators) {
+        return IBridgeValidators(addressStorage[keccak256("validatorContract")]);
+    }
+
+    modifier onlyValidator() {
+        require(validatorContract().isValidator(msg.sender));
+        _;
+    }
+}

deploy/.env.example

@@ -5,19 +5,19 @@ DEPLOYMENT_GAS_PRICE=10
 GET_RECEIPT_INTERVAL_IN_MILLISECONDS=3000
 
 HOME_RPC_URL=https://sokol.poa.network
-HOME_OWNER_MULTISIG=0x
-HOME_UPGRADEABLE_ADMIN_VALIDATORS=0x
-HOME_UPGRADEABLE_ADMIN_BRIDGE=0x
+HOME_BRIDGE_OWNER=0x
+HOME_VALIDATORS_OWNER=0x
+HOME_UPGRADEABLE_ADMIN=0x
 HOME_DAILY_LIMIT=30000000000000000000000000
 HOME_MAX_AMOUNT_PER_TX=1500000000000000000000000
 HOME_MIN_AMOUNT_PER_TX=500000000000000000
 HOME_REQUIRED_BLOCK_CONFIRMATIONS=1
 HOME_GAS_PRICE=1
 
 FOREIGN_RPC_URL=https://sokol.poa.network
-FOREIGN_OWNER_MULTISIG=0x
-FOREIGN_UPGRADEABLE_ADMIN_VALIDATORS=0x
-FOREIGN_UPGRADEABLE_ADMIN_BRIDGE=0x
+FOREIGN_BRIDGE_OWNER=0x
+FOREIGN_VALIDATORS_OWNER=0x
+FOREIGN_UPGRADEABLE_ADMIN=0x
 FOREIGN_DAILY_LIMIT=15000000000000000000000000
 FOREIGN_MAX_AMOUNT_PER_TX=750000000000000000000000
 FOREIGN_MIN_AMOUNT_PER_TX=500000000000000000

deploy/README.md

@@ -24,19 +24,19 @@ It is assumed that steps listed below are executed in `deploy` folder.
    GET_RECEIPT_INTERVAL_IN_MILLISECONDS=3000
 
    HOME_RPC_URL=https://sokol.poa.network
-   HOME_OWNER_MULTISIG=0x
-   HOME_UPGRADEABLE_ADMIN_VALIDATORS=0x
-   HOME_UPGRADEABLE_ADMIN_BRIDGE=0x
+   HOME_BRIDGE_OWNER=0x
+   HOME_VALIDATORS_OWNER=0x
+   HOME_UPGRADEABLE_ADMIN=0x
    HOME_DAILY_LIMIT=30000000000000000000000000
    HOME_MAX_AMOUNT_PER_TX=1500000000000000000000000
    HOME_MIN_AMOUNT_PER_TX=500000000000000000
    HOME_REQUIRED_BLOCK_CONFIRMATIONS=1
    HOME_GAS_PRICE=1
 
    FOREIGN_RPC_URL=https://sokol.poa.network
-   FOREIGN_OWNER_MULTISIG=0x
-   FOREIGN_UPGRADEABLE_ADMIN_VALIDATORS=0x
-   FOREIGN_UPGRADEABLE_ADMIN_BRIDGE=0x
+   FOREIGN_BRIDGE_OWNER=0x
+   FOREIGN_VALIDATORS_OWNER=0x
+   FOREIGN_UPGRADEABLE_ADMIN=0x
    FOREIGN_DAILY_LIMIT=15000000000000000000000000
    FOREIGN_MAX_AMOUNT_PER_TX=750000000000000000000000
    FOREIGN_MIN_AMOUNT_PER_TX=500000000000000000
@@ -64,18 +64,18 @@ DEPLOYMENT_GAS_LIMIT | Gas Limit to use for transactions during bridge contract
 DEPLOYMENT_GAS_PRICE | Gas Price to use for transactions during bridge contract provisioning on both networks in gwei  
 GET_RECEIPT_INTERVAL_IN_MILLISECONDS | Interval that is used to wait for tx to be mined( 3 sec in example)
 HOME_RPC_URL | Public RPC Node URL for Home Network  
-HOME_OWNER_MULTISIG | Address of Administrator role on Home network to change parameters of the bridge and validator's contract
-HOME_UPGRADEABLE_ADMIN_VALIDATORS | Address from which Validator's contract could be upgraded
-HOME_UPGRADEABLE_ADMIN_BRIDGE | Address from which HomeBridge's contract could be upgraded
+HOME_BRIDGE_OWNER | Address on Home network with permissions to change parameters of the bridge contract.
+HOME_VALIDATORS_OWNER | Address on Home network with permissions to change parameters of bridge validator contract.
+HOME_UPGRADEABLE_ADMIN | Address on Home network with permissions to upgrade the bridge contract and the bridge validator contract.
 HOME_DAILY_LIMIT | Daily Limit in Wei. Example above is `1 eth`  
 HOME_MAX_AMOUNT_PER_TX | Max limit per 1 tx in Wei. Example above is `0.1 eth`  
 HOME_MIN_AMOUNT_PER_TX | Minimum amount per 1 tx in Wei. Example above is `0.01 eth`  
 HOME_REQUIRED_BLOCK_CONFIRMATIONS | Number of blocks issued after the block with the corresponding deposit transaction to make sure that the transaction will not be rolled back
 HOME_GAS_PRICE | Gas Price to use for transactions to relay withdraws to Home Network
 FOREIGN_RPC_URL | Public RPC Node URL for Foreign Network  
-FOREIGN_OWNER_MULTISIG | Address of Administrator role on FOREIGN network to change parameters of the bridge and validator's contract
-FOREIGN_UPGRADEABLE_ADMIN_VALIDATORS | Address from which Validator's contract could be upgraded
-FOREIGN_UPGRADEABLE_ADMIN_BRIDGE | Address from which HomeBridge's contract could be upgraded
+FOREIGN_BRIDGE_OWNER | Address on Foreign network with permissions to change parameters of the bridge contract.
+FOREIGN_VALIDATORS_OWNER | Address on Foreign network with permissions to change parameters of bridge validator contract.
+FOREIGN_UPGRADEABLE_ADMIN | Address on Foreign network with permissions to upgrade the bridge contract and the bridge validator contract.
 FOREIGN_DAILY_LIMIT | Daily Limit in Wei. Example above is `1 eth`  
 FOREIGN_MAX_AMOUNT_PER_TX | Max limit per 1 tx in Wei. Example above is `0.1 eth`  
 FOREIGN_MIN_AMOUNT_PER_TX | Minimum amount per 1 tx in Wei. Example above is `0.01 eth`  

deploy/src/foreign.js

@@ -19,9 +19,9 @@ const FOREIGN_GAS_PRICE =  Web3Utils.toWei(process.env.FOREIGN_GAS_PRICE, 'gwei'
 const {
   DEPLOYMENT_ACCOUNT_ADDRESS,
   REQUIRED_NUMBER_OF_VALIDATORS,
-  FOREIGN_OWNER_MULTISIG,
-  FOREIGN_UPGRADEABLE_ADMIN_VALIDATORS,
-  FOREIGN_UPGRADEABLE_ADMIN_BRIDGE,
+  FOREIGN_BRIDGE_OWNER,
+  FOREIGN_VALIDATORS_OWNER,
+  FOREIGN_UPGRADEABLE_ADMIN,
   FOREIGN_DAILY_LIMIT,
   FOREIGN_MAX_AMOUNT_PER_TX,
   FOREIGN_MIN_AMOUNT_PER_TX,
@@ -69,7 +69,7 @@ async function deployForeign() {
   console.log(`REQUIRED_NUMBER_OF_VALIDATORS: ${REQUIRED_NUMBER_OF_VALIDATORS}, VALIDATORS: ${VALIDATORS}`)
   bridgeValidatorsForeign.options.address = storageValidatorsForeign.options.address
   const initializeForeignData = await bridgeValidatorsForeign.methods.initialize(
-    REQUIRED_NUMBER_OF_VALIDATORS, VALIDATORS, FOREIGN_OWNER_MULTISIG
+    REQUIRED_NUMBER_OF_VALIDATORS, VALIDATORS, FOREIGN_VALIDATORS_OWNER
   ).encodeABI({from: DEPLOYMENT_ACCOUNT_ADDRESS});
   const txInitializeForeign = await sendRawTx({
     data: initializeForeignData,
@@ -80,11 +80,11 @@ async function deployForeign() {
   });
   assert.equal(txInitializeForeign.status, '0x1', 'Transaction Failed');
   const validatorOwner = await bridgeValidatorsForeign.methods.owner().call();
-  assert.equal(validatorOwner.toLocaleLowerCase(), FOREIGN_OWNER_MULTISIG.toLocaleLowerCase());
+  assert.equal(validatorOwner.toLocaleLowerCase(), FOREIGN_VALIDATORS_OWNER.toLocaleLowerCase());
   foreignNonce++;
 
   console.log('\nTransferring ownership of ValidatorsProxy\n')
-  const validatorsForeignOwnershipData = await storageValidatorsForeign.methods.transferProxyOwnership(FOREIGN_UPGRADEABLE_ADMIN_VALIDATORS)
+  const validatorsForeignOwnershipData = await storageValidatorsForeign.methods.transferProxyOwnership(FOREIGN_UPGRADEABLE_ADMIN)
     .encodeABI({from: DEPLOYMENT_ACCOUNT_ADDRESS});
   const txValidatorsForeignOwnershipData = await sendRawTx({
     data: validatorsForeignOwnershipData,
@@ -96,7 +96,7 @@ async function deployForeign() {
   assert.equal(txValidatorsForeignOwnershipData.status, '0x1', 'Transaction Failed');
   foreignNonce++;
   const newProxyValidatorsOwner = await storageValidatorsForeign.methods.proxyOwner().call();
-  assert.equal(newProxyValidatorsOwner.toLowerCase(), FOREIGN_UPGRADEABLE_ADMIN_VALIDATORS.toLowerCase());
+  assert.equal(newProxyValidatorsOwner.toLowerCase(), FOREIGN_UPGRADEABLE_ADMIN.toLowerCase());
 
   console.log('\ndeploying foreignBridge storage\n')
   const foreignBridgeStorage = await deployContract(EternalStorageProxy, [], {from: DEPLOYMENT_ACCOUNT_ADDRESS, network: 'foreign', nonce: foreignNonce})
@@ -139,7 +139,8 @@ async function deployForeign() {
     FOREIGN_GAS_PRICE,
     FOREIGN_REQUIRED_BLOCK_CONFIRMATIONS,
     HOME_DAILY_LIMIT,
-    HOME_MAX_AMOUNT_PER_TX
+    HOME_MAX_AMOUNT_PER_TX,
+    FOREIGN_BRIDGE_OWNER
   ).encodeABI({from: DEPLOYMENT_ACCOUNT_ADDRESS});
   const txInitializeBridge = await sendRawTx({
     data: initializeFBridgeData,
@@ -164,7 +165,7 @@ async function deployForeign() {
   assert.equal(txOwnership.status, '0x1', 'Transaction Failed');
   foreignNonce++;
 
-  const bridgeOwnershipData = await foreignBridgeStorage.methods.transferProxyOwnership(FOREIGN_UPGRADEABLE_ADMIN_BRIDGE)
+  const bridgeOwnershipData = await foreignBridgeStorage.methods.transferProxyOwnership(FOREIGN_UPGRADEABLE_ADMIN)
     .encodeABI({from: DEPLOYMENT_ACCOUNT_ADDRESS});
   const txBridgeOwnershipData = await sendRawTx({
     data: bridgeOwnershipData,
@@ -176,7 +177,7 @@ async function deployForeign() {
   assert.equal(txBridgeOwnershipData.status, '0x1', 'Transaction Failed');
   foreignNonce++;
   const newProxyBridgeOwner = await foreignBridgeStorage.methods.proxyOwner().call();
-  assert.equal(newProxyBridgeOwner.toLowerCase(), FOREIGN_UPGRADEABLE_ADMIN_BRIDGE.toLowerCase());
+  assert.equal(newProxyBridgeOwner.toLowerCase(), FOREIGN_UPGRADEABLE_ADMIN.toLowerCase());
 
   return {
     foreignBridge:

deploy/src/home.js

@@ -18,9 +18,9 @@ const HOME_GAS_PRICE =  Web3Utils.toWei(process.env.HOME_GAS_PRICE, 'gwei');
 const {
   DEPLOYMENT_ACCOUNT_ADDRESS,
   REQUIRED_NUMBER_OF_VALIDATORS,
-  HOME_OWNER_MULTISIG,
-  HOME_UPGRADEABLE_ADMIN_VALIDATORS,
-  HOME_UPGRADEABLE_ADMIN_BRIDGE,
+  HOME_BRIDGE_OWNER,
+  HOME_VALIDATORS_OWNER,
+  HOME_UPGRADEABLE_ADMIN,
   HOME_DAILY_LIMIT,
   HOME_MAX_AMOUNT_PER_TX,
   HOME_MIN_AMOUNT_PER_TX,
@@ -59,7 +59,7 @@ async function deployHome()
   console.log(`REQUIRED_NUMBER_OF_VALIDATORS: ${REQUIRED_NUMBER_OF_VALIDATORS}, VALIDATORS: ${VALIDATORS}`)
   bridgeValidatorsHome.options.address = storageValidatorsHome.options.address
   const initializeData = await bridgeValidatorsHome.methods.initialize(
-    REQUIRED_NUMBER_OF_VALIDATORS, VALIDATORS, HOME_OWNER_MULTISIG
+    REQUIRED_NUMBER_OF_VALIDATORS, VALIDATORS, HOME_VALIDATORS_OWNER
   ).encodeABI({from: DEPLOYMENT_ACCOUNT_ADDRESS})
   const txInitialize = await sendRawTx({
     data: initializeData,
@@ -70,11 +70,11 @@ async function deployHome()
   })
   assert.equal(txInitialize.status, '0x1', 'Transaction Failed');
   const validatorOwner = await bridgeValidatorsHome.methods.owner().call();
-  assert.equal(validatorOwner.toLocaleLowerCase(), HOME_OWNER_MULTISIG.toLocaleLowerCase());
+  assert.equal(validatorOwner.toLocaleLowerCase(), HOME_VALIDATORS_OWNER.toLocaleLowerCase());
   homeNonce++;
 
   console.log('transferring proxy ownership to multisig for Validators Proxy contract');
-  const proxyDataTransfer = await storageValidatorsHome.methods.transferProxyOwnership(HOME_UPGRADEABLE_ADMIN_VALIDATORS).encodeABI();
+  const proxyDataTransfer = await storageValidatorsHome.methods.transferProxyOwnership(HOME_UPGRADEABLE_ADMIN).encodeABI();
   const txProxyDataTransfer = await sendRawTx({
     data: proxyDataTransfer,
     nonce: homeNonce,
@@ -84,7 +84,7 @@ async function deployHome()
   })
   assert.equal(txProxyDataTransfer.status, '0x1', 'Transaction Failed');
   const newProxyOwner = await storageValidatorsHome.methods.proxyOwner().call();
-  assert.equal(newProxyOwner.toLocaleLowerCase(), HOME_UPGRADEABLE_ADMIN_VALIDATORS.toLocaleLowerCase());
+  assert.equal(newProxyOwner.toLocaleLowerCase(), HOME_UPGRADEABLE_ADMIN.toLocaleLowerCase());
   homeNonce++;
 
   console.log('\ndeploying homeBridge storage\n')
@@ -128,7 +128,8 @@ async function deployHome()
     HOME_GAS_PRICE,
     HOME_REQUIRED_BLOCK_CONFIRMATIONS,
     FOREIGN_DAILY_LIMIT,
-    FOREIGN_MAX_AMOUNT_PER_TX
+    FOREIGN_MAX_AMOUNT_PER_TX,
+    HOME_BRIDGE_OWNER
   ).encodeABI({from: DEPLOYMENT_ACCOUNT_ADDRESS});
   const txInitializeHomeBridge = await sendRawTx({
     data: initializeHomeBridgeData,
@@ -141,7 +142,7 @@ async function deployHome()
   homeNonce++;
 
   console.log('transferring proxy ownership to multisig for Home bridge Proxy contract');
-  const homeBridgeProxyData = await homeBridgeStorage.methods.transferProxyOwnership(HOME_UPGRADEABLE_ADMIN_BRIDGE).encodeABI();
+  const homeBridgeProxyData = await homeBridgeStorage.methods.transferProxyOwnership(HOME_UPGRADEABLE_ADMIN).encodeABI();
   const txhomeBridgeProxyData = await sendRawTx({
     data: homeBridgeProxyData,
     nonce: homeNonce,
@@ -151,7 +152,7 @@ async function deployHome()
   })
   assert.equal(txhomeBridgeProxyData.status, '0x1', 'Transaction Failed');
   const newProxyBridgeOwner = await homeBridgeStorage.methods.proxyOwner().call();
-  assert.equal(newProxyBridgeOwner.toLocaleLowerCase(), HOME_UPGRADEABLE_ADMIN_BRIDGE.toLocaleLowerCase());
+  assert.equal(newProxyBridgeOwner.toLocaleLowerCase(), HOME_UPGRADEABLE_ADMIN.toLocaleLowerCase());
   homeNonce++;
 
   console.log('\nHome Deployment Bridge is complete\n')