Changes post review

contracts/bridging/IColonyBridge.sol

@@ -31,7 +31,7 @@ interface IColonyBridge {
 
   /// @notice Function to get the colony network address that the bridge is interacting with
   /// @return address The address of the colony network
-  function getColonyNetworkAddress() external view returns (address);
+  function colonyNetwork() external view returns (address);
 
   /// @notice Function to set the address of the instance of this contract on other chains, that
   /// this contract will expect to receive messages from

contracts/bridging/WormholeBridgeForColony.sol

@@ -25,7 +25,7 @@ import { CallWithGuards } from "../common/CallWithGuards.sol";
 import { DSAuth } from "../../lib/dappsys/auth.sol";
 
 contract WormholeBridgeForColony is DSAuth, IColonyBridge, CallWithGuards {
-  address colonyNetwork;
+  address public colonyNetwork;
   IWormhole public wormhole;
 
   // ChainId => colonyBridge
@@ -64,10 +64,6 @@ contract WormholeBridgeForColony is DSAuth, IColonyBridge, CallWithGuards {
     colonyNetwork = _colonyNetwork;
   }
 
-  function getColonyNetworkAddress() public view returns (address) {
-    return colonyNetwork;
-  }
-
   function setColonyBridgeAddress(uint256 _evmChainId, address _bridgeAddress) public auth {
     require(_evmChainId <= type(uint128).max, "colony-bridge-chainid-too-large");
     uint16 requestedWormholeChainId = evmChainIdToWormholeChainId[_evmChainId];

contracts/colonyNetwork/ColonyNetworkDataTypes.sol

@@ -71,15 +71,15 @@ interface ColonyNetworkDataTypes {
   /// @param skillId The skillId that failed to bridge
   event SkillCreationStored(uint256 skillId);
 
-  /// @notice Event logged when a skill is successfully added from a bridge.
-  /// @param skillId The skillId of the skill that was bridged
-  event SkillAddedFromBridge(uint256 skillId);
-
   /// @notice Event logged when a skill is received from a bridge, but can't yet be
   /// added to the skill tree.
   /// @param skillId The skillId of the skill that was bridged
   event SkillStoredFromBridge(uint256 skillId);
 
+  /// @notice Event logged when a skill is successfully added from a bridge.
+  /// @param skillId The skillId of the skill that was bridged
+  event SkillAddedFromBridge(uint256 skillId);
+
   /// @notice Event logged when a new auction is created and started
   /// @dev Emitted from `IColonyNetwork.startTokenAuction` function
   /// @param auction Address of the created auction contract
@@ -174,18 +174,18 @@ interface ColonyNetworkDataTypes {
   /// @param count The number of the reputation update trying to be bridged in that colony
   event ReputationUpdateSentToBridge(address colony, uint256 count);
 
-  /// @notice Event logged when a reputation update is successfully bridged.
+  /// @notice Event logged when a reputation update is received from a bridge, but can't be
+  /// added to the reputation update log due to being bridged out of order or the skill not existing.
   /// @param chainId The chainId of the chain the bridge is associated with
   /// @param colony The address of the colony where reputation is being emitted
   /// @param updateNumber The number of the reputation update bridged in that colony
-  event ReputationUpdateAddedFromBridge(uint256 chainId, address colony, uint256 updateNumber);
+  event ReputationUpdateStoredFromBridge(uint256 chainId, address colony, uint256 updateNumber);
 
-  /// @notice Event logged when a reputation update is received from a bridge, but can't be
-  /// added to the reputation update log due to being bridged out of order or the skill not existing.
+  /// @notice Event logged when a reputation update is successfully bridged.
   /// @param chainId The chainId of the chain the bridge is associated with
   /// @param colony The address of the colony where reputation is being emitted
   /// @param updateNumber The number of the reputation update bridged in that colony
-  event ReputationUpdateStoredFromBridge(uint256 chainId, address colony, uint256 updateNumber);
+  event ReputationUpdateAddedFromBridge(uint256 chainId, address colony, uint256 updateNumber);
 
   struct Skill {
     // total number of parent skills

contracts/colonyNetwork/ColonyNetworkMining.sol

@@ -174,7 +174,9 @@ contract ColonyNetworkMining is ColonyNetworkStorage {
     address clnyToken = IMetaColony(metaColony).getToken();
     require(clnyToken != address(0x0), "colony-reputation-mining-clny-token-invalid-address");
 
-    // Add the special mining skill
+    // Add the special mining skill. Note that if moving away from this chain,
+    // and then back, a new mining skill will be created. It is an open question
+    // whether this is desireable behaviour.
     ColonyDataTypes.Domain memory d = IMetaColony(metaColony).getDomain(1);
 
     reputationMiningSkillId = IColonyNetwork(address(this)).addSkill(d.skillId);

contracts/colonyNetwork/ColonyNetworkStorage.sol

@@ -111,22 +111,23 @@ contract ColonyNetworkStorage is ColonyNetworkDataTypes, DSMath, CommonStorage,
 
   address colonyBridgeAddress; // Storage slot 44
 
+  mapping(uint256 => uint256) bridgeCurrentRootHashNonces; // Storage slot 45
+
   // A mapping that maps chain id -> skill count
-  mapping(uint256 => uint256) networkSkillCounts; // Storage slot 45
+  mapping(uint256 => uint256) networkSkillCounts; // Storage slot 46
+
   // A mapping that stores pending bridged skill additions that have been bridged out-of-order
   // chainId -> skillCount -> parentSkillId
-  mapping(uint256 => mapping(uint256 => uint256)) pendingSkillAdditions; // Storage slot 46
+  mapping(uint256 => mapping(uint256 => uint256)) pendingSkillAdditions; // Storage slot 47
 
   // A mapping that stores the latest reputation update received from a colony on a particular chain
   // chainId -> colonyAddress -> updateCount
-  mapping(uint256 => mapping(address => uint256)) reputationUpdateCount; // Storage slot 47
+  mapping(uint256 => mapping(address => uint256)) reputationUpdateCount; // Storage slot 48
 
   // A mapping that stores reputation updates that haven't been added to the log yet, either because they've been
   // received out of order, or because the skill in question hasn't been bridged yet.
   // networkId -> colonyAddress -> updateCount -> update
-  mapping(uint256 => mapping(address => mapping(uint256 => PendingReputationUpdate))) pendingReputationUpdates; // Storage slot 48
-
-  mapping(uint256 => uint256) bridgeCurrentRootHashNonces; // Storage slot 49
+  mapping(uint256 => mapping(address => mapping(uint256 => PendingReputationUpdate))) pendingReputationUpdates; // Storage slot 49
 
   // Modifiers
 

helpers/test-helper.js

@@ -1,5 +1,4 @@
 /* globals artifacts, hre */
-const ChainId = artifacts.require("ChainId");
 const shortid = require("shortid");
 const chai = require("chai");
 const { asciiToHex, isBN } = require("web3-utils");
@@ -31,6 +30,7 @@ const Resolver = artifacts.require("Resolver");
 const ContractEditing = artifacts.require("ContractEditing");
 const ColonyDomains = artifacts.require("ColonyDomains");
 const EtherRouter = artifacts.require("EtherRouter");
+const ChainId = artifacts.require("ChainId");
 
 const { expect } = chai;
 
@@ -1226,7 +1226,7 @@ exports.sleep = function sleep(ms) {
   });
 };
 
-exports.getMultichainSkillId = function multichainSkillId(chainId, skillId) {
+exports.getMultichainSkillId = function getMultichainSkillId(chainId, skillId) {
   if (chainId === XDAI_CHAINID || chainId === FORKED_XDAI_CHAINID) {
     return skillId;
   }

migrations/6_setup_mining_cycle_resolver.js

@@ -15,11 +15,7 @@ module.exports = async function (deployer) {
   // Check chain id
   // If not a mining chain, then skip
   const chainId = await getChainId();
-  let miningChainId = parseInt(process.env.MINING_CHAIN_ID, 10);
-  console.log(chainId, miningChainId);
-  if (!miningChainId) {
-    miningChainId = chainId;
-  }
+  const miningChainId = parseInt(process.env.MINING_CHAIN_ID, 10) || chainId;
 
   if (chainId !== miningChainId) {
     console.log("Not mining chain, skipping setting up mining cycle resolver");

migrations/8_setup_meta_colony.js

@@ -47,11 +47,8 @@ module.exports = async function (deployer, network, accounts) {
   // Check chain id
   // If not a mining chain, then skip setting up mining
   const chainId = await getChainId();
-  let miningChainId = parseInt(process.env.MINING_CHAIN_ID, 10);
-  if (!miningChainId) {
-    miningChainId = chainId;
-  }
-  console.log(miningChainId, chainId);
+  const miningChainId = parseInt(process.env.MINING_CHAIN_ID, 10) || chainId;
+
   if (miningChainId === chainId) {
     // These commands add MAIN_ACCOUNT as a reputation miner.
     // This is necessary because the first miner must have staked before the mining cycle begins.

test/cross-chain/cross-chain.js

@@ -262,7 +262,7 @@ contract("Cross-chain", (accounts) => {
       const bridgeAddress = await homeColonyNetwork.getColonyBridgeAddress();
       expect(bridgeAddress).to.equal(homeColonyBridge.address);
 
-      const networkAddress = await homeColonyBridge.getColonyNetworkAddress();
+      const networkAddress = await homeColonyBridge.colonyNetwork();
       expect(networkAddress).to.equal(homeColonyNetwork.address);
 
       const foreignColonyBridgeAddress = await homeColonyBridge.getColonyBridgeAddress(foreignChainId);