First changes following second review

.solcover.crosschain.js

@@ -3,6 +3,8 @@ const log = console.log;
 const { execSync } = require("child_process");
 const ethers  = require("ethers");
 
+const { FORKED_XDAI_CHAINID } = require("./helpers/constants");
+
 const existingCompileComplete = config.onCompileComplete;
 
 let chainId;
@@ -11,9 +13,9 @@ let chainId;
 // TODO: Actually query nodes, don't hard-code here, or work out how to get environment
 // variables in package.json to work here as I want.
 if (JSON.parse(process.env.TRUFFLE_FOREIGN)){
-  chainId = 265669101;
+  chainId = FORKED_XDAI_CHAINID + 1;
 } else {
-  chainId = 265669100;
+  chainId = FORKED_XDAI_CHAINID;
 }
 
 config.providerOptions.network_id = chainId;

contracts/colony/Colony.sol

@@ -215,11 +215,11 @@ contract Colony is BasicMetaTransaction, Multicall, ColonyStorage, PatriciaTreeP
     IColonyNetwork(colonyNetworkAddress).addColonyVersion(_version, _resolver);
   }
 
-  function setBridgeData(address bridgeAddress, bytes memory updateLogBefore, bytes memory updateLogAfter, uint256 gas, uint256 chainId, bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) external
+  function setBridgeData(address bridgeAddress, uint256 chainId, uint256 gas, bytes memory updateLogBefore, bytes memory updateLogAfter, bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) external
   stoppable
   auth
   {
-    IColonyNetwork(colonyNetworkAddress).setBridgeData(bridgeAddress, updateLogBefore, updateLogAfter, gas, chainId, skillCreationBefore, skillCreationAfter, setReputationRootHashBefore, setReputationRootHashAfter);
+    IColonyNetwork(colonyNetworkAddress).setBridgeData(bridgeAddress, chainId, gas, updateLogBefore, updateLogAfter, skillCreationBefore, skillCreationAfter, setReputationRootHashBefore, setReputationRootHashAfter);
   }
 
   function addExtensionToNetwork(bytes32 _extensionId, address _resolver)
@@ -323,7 +323,7 @@ contract Colony is BasicMetaTransaction, Multicall, ColonyStorage, PatriciaTreeP
     ColonyAuthority colonyAuthority = ColonyAuthority(address(authority));
     bytes4 sig;
 
-    sig = bytes4(keccak256("setBridgeData(address,bytes,bytes,uint256,uint256,bytes,bytes,bytes,bytes)"));
+    sig = bytes4(keccak256("setBridgeData(address,uint256,uint256,bytes,bytes,bytes,bytes,bytes,bytes)"));
     colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true);
   }
 

contracts/colony/ColonyAuthority.sol

@@ -131,7 +131,7 @@ contract ColonyAuthority is CommonAuthority {
     addRoleCapability(ARBITRATION_ROLE, "setExpenditurePayout(uint256,uint256,uint256,uint256,address,uint256)");
 
     // Added in colony vxxx
-    addRoleCapability(ROOT_ROLE, "setBridgeData(address,bytes,bytes,uint256,uint256,bytes,bytes,bytes,bytes)");
+    addRoleCapability(ROOT_ROLE, "setBridgeData(address,uint256,uint256,bytes,bytes,bytes,bytes,bytes,bytes)");
   }
 
   function addRoleCapability(uint8 role, bytes memory sig) private {

contracts/colony/IMetaColony.sol

@@ -67,13 +67,13 @@ interface IMetaColony is IColony {
 
   /// @notice Called to set the details about bridge _bridgeAddress
   /// @param _bridgeAddress The address of the bridge
+  /// @param chainId The chainId of the corresponding network
+  /// @param gas How much gas to use for a bridged transaction
   /// @param updateLogBefore The tx data before the dynamic part of the tx to bridge to the update log
   /// @param updateLogAfter The tx data after the dynamic part of the tx to bridge to the update log
-  /// @param gas How much gas to use for a bridged transaction
-  /// @param chainId The chainId of the corresponding network
   /// @param skillCreationBefore The tx data before the dynamic part of the tx to brdige skill creation
   /// @param skillCreationAfter The tx data after the dynamic part of the tx to brdige skill creation
   /// @param setReputationRootHashBefore The tx data before the dynamic part of the tx to bridge a new reputation root hash
   /// @param setReputationRootHashAfter The tx data after the dynamic part of the tx to bridge a new reputation root hash
-  function setBridgeData(address _bridgeAddress, bytes memory updateLogBefore, bytes memory updateLogAfter, uint256 gas, uint256 chainId, bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) external;
+  function setBridgeData(address _bridgeAddress, uint256 chainId, uint256 gas, bytes memory updateLogBefore, bytes memory updateLogAfter, bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) external;
 }

contracts/colonyNetwork/ColonyNetwork.sol

@@ -102,20 +102,20 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
     emit ColonyVersionAdded(_version, _resolver);
   }
 
-  function setBridgeData(address bridgeAddress, bytes memory updateLogBefore, bytes memory updateLogAfter, uint256 gas, uint256 chainId, bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) public
+  function setBridgeData(address bridgeAddress, uint256 chainId, uint256 gas, bytes memory updateLogBefore, bytes memory updateLogAfter, bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) public
   always
   calledByMetaColony
   {
     if (!isMiningChain()) {
-      miningBridgeAddress = bridgeAddress;
       require(isMiningChainId(chainId), "colony-network-can-only-set-mining-chain-bridge");
+      miningBridgeAddress = bridgeAddress;
     }
     bridgeData[bridgeAddress] = Bridge(updateLogBefore, updateLogAfter, gas, chainId, skillCreationBefore, skillCreationAfter, setReputationRootHashBefore, setReputationRootHashAfter);
     if (networkSkillCounts[chainId] == 0) {
       // Initialise the skill count to match the foreign chain
       networkSkillCounts[chainId] = chainId << 128;
     }
-    // emit BridgeDataSet
+    emit BridgeDataSet(bridgeAddress);
   }
 
   function getBridgeData(address bridgeAddress) public view returns (Bridge memory) {
@@ -238,15 +238,15 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
 
   function addSkillFromBridge(uint256 _parentSkillId, uint256 _skillId) public always onlyMiningChain() {
     // Require is a known bridge
-    Bridge storage bridge = bridgeData[msgSender()];
-    require(bridge.chainId != 0, "colony-network-not-known-bridge");
+    uint256 bridgeChainId = bridgeData[msgSender()].chainId;
+    require(bridgeChainId != 0, "colony-network-not-known-bridge");
 
     // Check skill count - if not next, then store for later.
-    if (networkSkillCounts[bridge.chainId] + 1 == _skillId){
+    if (networkSkillCounts[bridgeChainId] + 1 == _skillId){
       addSkillToChainTree(_parentSkillId, _skillId);
-      networkSkillCounts[bridge.chainId] += 1;
-    } else if (networkSkillCounts[bridge.chainId] < _skillId){
-      pendingSkillAdditions[bridge.chainId][_skillId] = _parentSkillId;
+      networkSkillCounts[bridgeChainId] += 1;
+    } else if (networkSkillCounts[bridgeChainId] < _skillId){
+      pendingSkillAdditions[bridgeChainId][_skillId] = _parentSkillId;
       // TODO: Event?
     }
   }
@@ -260,20 +260,20 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
   }
 
   function addBridgedPendingSkill(address _bridgeAddress, uint256 _skillId) public always onlyMiningChain() {
-    Bridge storage bridge = bridgeData[_bridgeAddress];
-    require(bridge.chainId != 0, "colony-network-not-known-bridge");
+    uint256 bridgeChainId = bridgeData[_bridgeAddress].chainId;
+    require(bridgeChainId != 0, "colony-network-not-known-bridge");
 
     // Require that specified skill is next
     // Note this also implicitly checks that the chainId prefix of the skill is correct
-    require(networkSkillCounts[bridge.chainId] + 1 == _skillId, "colony-network-not-next-bridged-skill");
+    require(networkSkillCounts[bridgeChainId] + 1 == _skillId, "colony-network-not-next-bridged-skill");
 
-    uint256 parentSkillId = pendingSkillAdditions[bridge.chainId][_skillId];
+    uint256 parentSkillId = pendingSkillAdditions[bridgeChainId][_skillId];
     require(parentSkillId != 0, "colony-network-no-such-bridged-skill");
     addSkillToChainTree(parentSkillId, _skillId);
-    networkSkillCounts[bridge.chainId] += 1;
+    networkSkillCounts[bridgeChainId] += 1;
 
     // Delete the pending addition
-    delete pendingSkillAdditions[bridge.chainId][_skillId];
+    delete pendingSkillAdditions[bridgeChainId][_skillId];
   }
 
   function getParentSkillId(uint _skillId, uint _parentSkillIndex) public view returns (uint256) {
@@ -314,19 +314,21 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
     return skillCount;
   }
 
-  function appendReputationUpdateLogFromBridge(address _colony, address _user, int _amount, uint _skillId, uint256 _updateNumber) public onlyMiningChain stoppable
+  function appendReputationUpdateLogFromBridge(address _colony, address _user, int256 _amount, uint256 _skillId, uint256 _updateNumber) public onlyMiningChain stoppable
   {
     // Require is a known bridge
-    require(bridgeData[msgSender()].chainId != 0, "colony-network-not-known-bridge");
+    uint256 bridgeChainId = bridgeData[msgSender()].chainId;
+
+    require(bridgeChainId != 0, "colony-network-not-known-bridge");
 
-    require(bridgeData[msgSender()].chainId == _skillId >> 128, "colony-network-invalid-skill-id-for-bridge");
+    require(bridgeChainId == _skillId >> 128, "colony-network-invalid-skill-id-for-bridge");
 
     // if next expected update, add to log
     if (
-      reputationUpdateCount[bridgeData[msgSender()].chainId][_colony] + 1 == _updateNumber && // It's the next reputation update for this colony
+      reputationUpdateCount[bridgeChainId][_colony] + 1 == _updateNumber && // It's the next reputation update for this colony
       networkSkillCounts[_skillId >> 128] >= _skillId // Skill has been bridged
     ){
-      reputationUpdateCount[bridgeData[msgSender()].chainId][_colony] += 1;
+      reputationUpdateCount[bridgeChainId][_colony] += 1;
       uint128 nParents = skills[_skillId].nParents;
       // We only update child skill reputation if the update is negative, otherwise just set nChildren to 0 to save gas
       uint128 nChildren = _amount < 0 ? skills[_skillId].nChildren : 0;
@@ -342,7 +344,7 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
 
     } else {
       // Not next update, store for later
-      pendingReputationUpdates[bridgeData[msgSender()].chainId][_colony][_updateNumber] = PendingReputationUpdate(_colony, _user, _amount, _skillId, block.timestamp);
+      pendingReputationUpdates[bridgeChainId][_colony][_updateNumber] = PendingReputationUpdate(_user, _amount, _skillId, _colony, block.timestamp);
     }
   }
 
@@ -370,13 +372,13 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
   }
 
   function addBridgedReputationUpdate(uint256 _chainId, address _colony) public stoppable onlyMiningChain {
-    uint256 nextUpdateNumber = reputationUpdateCount[_chainId][_colony] + 1;
+    uint256 mostRecentUpdateNumber = reputationUpdateCount[_chainId][_colony];
+    PendingReputationUpdate storage pendingUpdate = pendingReputationUpdates[_chainId][_colony][mostRecentUpdateNumber + 1];
+
     // Bridged update must exist
-    require(pendingReputationUpdates[_chainId][_colony][nextUpdateNumber].colony != address(0x00), "colony-network-next-update-does-not-exist");
+    require(pendingUpdate.colony != address(0x00), "colony-network-next-update-does-not-exist");
     // It should be the next one
-    assert(pendingReputationUpdates[_chainId][_colony][nextUpdateNumber - 1].colony == address(0x00));
-
-    PendingReputationUpdate storage pendingUpdate = pendingReputationUpdates[_chainId][_colony][nextUpdateNumber];
+    assert(pendingReputationUpdates[_chainId][_colony][mostRecentUpdateNumber].colony == address(0x00));
 
     // Skill creation must have been bridged
     require(networkSkillCounts[pendingUpdate.skillId >> 128] >= pendingUpdate.skillId, "colony-network-invalid-skill-id");
@@ -387,10 +389,10 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
 
     int256 updateAmount = decayReputation(pendingUpdate.amount, pendingUpdate.timestamp);
 
-    reputationUpdateCount[_chainId][_colony] +=1;
+    reputationUpdateCount[_chainId][_colony] += 1;
     address user = pendingUpdate.user;
     uint256 skillId = pendingUpdate.skillId;
-    delete pendingReputationUpdates[_chainId][_colony][nextUpdateNumber];
+    delete pendingReputationUpdates[_chainId][_colony][mostRecentUpdateNumber + 1];
 
     IReputationMiningCycle(inactiveReputationMiningCycle).appendReputationUpdateLog(
       user,
@@ -453,7 +455,7 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage, Multicall
       (bool success, ) = miningBridgeAddress.call(payload);
       if (!success || !isContract(miningBridgeAddress)) {
         // Store to resend later
-        pendingReputationUpdates[getChainId()][msgSender()][reputationUpdateCount[getChainId()][msgSender()]] = PendingReputationUpdate(msgSender(), _user, _amount, _skillId, block.timestamp);
+        pendingReputationUpdates[getChainId()][msgSender()][reputationUpdateCount[getChainId()][msgSender()]] = PendingReputationUpdate(_user, _amount, _skillId, msgSender(), block.timestamp);
       }
       // TODO: How do we emit events here?
     }

contracts/colonyNetwork/ColonyNetworkDataTypes.sol

@@ -145,6 +145,10 @@ interface ColonyNetworkDataTypes {
   /// @param tokenAuthorityAddress The address of the token authority deployed
   event TokenAuthorityDeployed(address tokenAuthorityAddress);
 
+  /// @notice Event logged when the colony network has data about a bridge contract set.
+  /// @param bridgeAddress The address of the bridge contract that will be interacted with
+  event BridgeDataSet(address bridgeAddress);
+
   struct Skill {
     // total number of parent skills
     uint128 nParents;
@@ -191,10 +195,10 @@ interface ColonyNetworkDataTypes {
   }
 
   struct PendingReputationUpdate {
-    address colony;
     address user;
     int256 amount;
     uint skillId;
+    address colony;
     uint256 timestamp;
   }
 }

contracts/colonyNetwork/ColonyNetworkMining.sol

@@ -93,14 +93,14 @@ contract ColonyNetworkMining is ColonyNetworkStorage {
     emit ReputationRootHashSet(newHash, newNLeaves, newAddressArray(), 0);
   }
 
-  function bridgeCurrentRootHash(address bridgeAddress) onlyMiningChain stoppable public {
-    require(bridgeData[bridgeAddress].chainId != 0, "colony-network-not-known-bridge");
+  function bridgeCurrentRootHash(address _bridgeAddress) onlyMiningChain stoppable public {
+    require(bridgeData[_bridgeAddress].chainId != 0, "colony-network-not-known-bridge");
     bytes memory payload = abi.encodePacked(
-      bridgeData[bridgeAddress].setReputationRootHashBefore,
+      bridgeData[_bridgeAddress].setReputationRootHashBefore,
       abi.encodeWithSignature("setReputationRootHashFromBridge(bytes32,uint256)", reputationRootHash, reputationRootHashNLeaves),
-      bridgeData[bridgeAddress].setReputationRootHashAfter
+      bridgeData[_bridgeAddress].setReputationRootHashAfter
     );
-    (bool success, ) = bridgeAddress.call(payload);
+    (bool success, ) = _bridgeAddress.call(payload);
     // TODO: Do we require success here?
     require(success, "colony-mining-bridge-call-failed");
   }

contracts/colonyNetwork/ColonyNetworkStorage.sol

@@ -116,10 +116,10 @@ contract ColonyNetworkStorage is ColonyNetworkDataTypes, DSMath, CommonStorage,
   // networkId -> skillCount -> parentSkillId
   mapping(uint256 => mapping(uint256 => uint256)) pendingSkillAdditions; // Storage slot 46
 
-  // networkId -> colonyAddress -> updateCount -> update
-  mapping(uint256 => mapping( address => mapping(uint256 => PendingReputationUpdate))) pendingReputationUpdates; // Storage slot 47
   // networkID -> colonyAddress -> updateCount
   mapping(uint256 => mapping( address => uint256)) reputationUpdateCount; // Storage slot 48
+  // networkId -> colonyAddress -> updateCount -> update
+  mapping(uint256 => mapping( address => mapping(uint256 => PendingReputationUpdate))) pendingReputationUpdates; // Storage slot 47
 
   modifier calledByColony() {
     require(_isColony[msgSender()], "colony-caller-must-be-colony");

contracts/colonyNetwork/IColonyNetwork.sol

@@ -465,15 +465,15 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery, IBasicMetaTransac
 
   /// @notice Called to set the details about bridge _bridgeAddress
   /// @param _bridgeAddress The address of the bridge
+  /// @param chainId The chainId of the corresponding network
+  /// @param gas How much gas to use for a bridged transaction
   /// @param updateLogBefore The tx data before the dynamic part of the tx to bridge to the update log
   /// @param updateLogAfter The tx data after the dynamic part of the tx to bridge to the update log
-  /// @param gas How much gas to use for a bridged transaction
-  /// @param chainId The chainId of the corresponding network
   /// @param skillCreationBefore The tx data before the dynamic part of the tx to brdige skill creation
   /// @param skillCreationAfter The tx data after the dynamic part of the tx to brdige skill creation
   /// @param setReputationRootHashBefore The tx data before the dynamic part of the tx to bridge a new reputation root hash
   /// @param setReputationRootHashAfter The tx data after the dynamic part of the tx to bridge a new reputation root hash
-  function setBridgeData(address _bridgeAddress, bytes memory updateLogBefore, bytes memory updateLogAfter, uint256 gas, uint256 chainId, bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) external;
+  function setBridgeData(address _bridgeAddress, uint256 chainId, uint256 gas, bytes memory updateLogBefore, bytes memory updateLogAfter,  bytes memory skillCreationBefore, bytes memory skillCreationAfter, bytes memory setReputationRootHashBefore, bytes memory setReputationRootHashAfter) external;
 
   /// @notice Called to get the details about known bridge _bridgeAddress
   /// @param _bridgeAddress The address of the bridge

contracts/common/MultiChain.sol

@@ -51,7 +51,7 @@ contract MultiChain {
   }
 
   function isMiningChainId(uint256 chainId) internal view returns (bool) {
-    return (chainId == 100 || chainId == 265669100);
+    return (chainId == MINING_CHAIN_ID || chainId == 265669100);
   }
 
   uint256 constant MINING_CHAIN_ID = 100;

contracts/testHelpers/BridgeMock.sol

@@ -41,16 +41,6 @@ contract BridgeMock {
         }
       }
 
-    // bool success;
-    // assembly {
-    //           // call contract at address a with input mem[in…(in+insize))
-    //           //   providing g gas and v wei and output area mem[out…(out+outsize))
-    //           //   returning 0 on error (eg. out of gas) and 1 on success
-
-    //           // call(g,         a,       v, in,              insize,      out, outsize)
-    //   success := call(_gasLimit, _target, 0, add(_data, 0x20), mload(_data), 0, 0)
-    // }
-
     emit RelayedMessage(_sender, msg.sender, _messageId, success);
   }