Try and harden a test, fixing an inconsistency

contracts/reputationMiningCycle/ReputationMiningCycle.sol

@@ -505,78 +505,6 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon {
   // Internal functions
   /////////////////////////
 
-  function processBinaryChallengeSearchResponse(
-    uint256 round,
-    uint256 idx,
-    bytes memory jhIntermediateValue,
-    bytes32[2] memory lastSiblings
-  ) internal
-  {
-    disputeRounds[round][idx].lastResponseTimestamp = now;
-    disputeRounds[round][idx].challengeStepCompleted += 1;
-    // Save our intermediate hash
-    bytes32 intermediateReputationHash;
-    uint256 intermediateReputationNNodes;
-    assembly {
-      intermediateReputationHash := mload(add(jhIntermediateValue, 0x20))
-      intermediateReputationNNodes := mload(add(jhIntermediateValue, 0x40))
-    }
-    disputeRounds[round][idx].intermediateReputationHash = intermediateReputationHash;
-    disputeRounds[round][idx].intermediateReputationNNodes = intermediateReputationNNodes;
-
-    disputeRounds[round][idx].hash1 = lastSiblings[0];
-    disputeRounds[round][idx].hash2 = lastSiblings[1];
-
-    uint256 opponentIdx = getOpponentIdx(idx);
-    if (disputeRounds[round][opponentIdx].challengeStepCompleted == disputeRounds[round][idx].challengeStepCompleted ) {
-      // Our opponent answered this challenge already.
-      // Compare our intermediateReputationHash to theirs to establish how to move the bounds.
-      processBinaryChallengeSearchStep(round, idx);
-    }
-  }
-
-  function processBinaryChallengeSearchStep(uint256 round, uint256 idx) internal {
-    uint256 opponentIdx = getOpponentIdx(idx);
-    uint256 searchWidth = (disputeRounds[round][idx].upperBound - disputeRounds[round][idx].lowerBound) + 1;
-    uint256 searchWidthNextPowerOfTwo = nextPowerOfTwoInclusive(searchWidth);
-    if (
-      disputeRounds[round][opponentIdx].hash1 == disputeRounds[round][idx].hash1
-      )
-    {
-      disputeRounds[round][idx].lowerBound += searchWidthNextPowerOfTwo/2;
-      disputeRounds[round][opponentIdx].lowerBound += searchWidthNextPowerOfTwo/2;
-      disputeRounds[round][idx].targetHashDuringSearch = disputeRounds[round][idx].hash2;
-      disputeRounds[round][opponentIdx].targetHashDuringSearch = disputeRounds[round][opponentIdx].hash2;
-    } else {
-      disputeRounds[round][idx].upperBound -= (searchWidth - searchWidthNextPowerOfTwo/2);
-      disputeRounds[round][opponentIdx].upperBound -= (searchWidth - searchWidthNextPowerOfTwo/2);
-      disputeRounds[round][idx].targetHashDuringSearch = disputeRounds[round][idx].hash1;
-      disputeRounds[round][opponentIdx].targetHashDuringSearch = disputeRounds[round][opponentIdx].hash1;
-    }
-    // We need to keep the intermediate hashes so that we can figure out what type of dispute we are resolving later
-    // If the number of nodes in the reputation state are different, then we are disagreeing on whether this log entry
-    // corresponds to an existing reputation entry or not.
-    // If the hashes are different, then it's a calculation error.
-    // However, the intermediate hashes saved might not be the ones that correspond to the first disagreement, based on how exactly the last
-    // step of the binary challenge came to be.
-
-    // If complete, mark that the binary search is completed (but the intermediate hashes may or may not be correct) by setting
-    // challengeStepCompleted to the maximum it could be for the number of nodes we had to search through, plus one to indicate
-    // they've submitted their jrh
-    Submission storage submission = reputationHashSubmissions[disputeRounds[round][idx].firstSubmitter];
-    if (disputeRounds[round][idx].lowerBound == disputeRounds[round][idx].upperBound) {
-      if (2**(disputeRounds[round][idx].challengeStepCompleted-1) < submission.jrhNNodes) {
-        disputeRounds[round][idx].challengeStepCompleted += 1;
-        disputeRounds[round][opponentIdx].challengeStepCompleted += 1;
-      }
-    }
-
-    // Our opponent responded to this step of the challenge before we did, so we should
-    // reset their 'last response' time to now, as they aren't able to respond
-    // to the next challenge before they know what it is!
-    disputeRounds[round][opponentIdx].lastResponseTimestamp = now;
-  }
-
   function checkJRHProof1(bytes32 jrh, uint256 branchMask1, bytes32[] memory siblings1, uint256 reputationRootHashNNodes) internal view {
     // Proof 1 needs to prove that they started with the current reputation root hash
     bytes32 reputationRootHash = IColonyNetwork(colonyNetworkAddress).getReputationRootHash();

contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol

@@ -52,7 +52,6 @@ contract ReputationMiningCycleCommon is ReputationMiningCycleStorage, PatriciaTr
   }
 
   function disputeRewardSize() internal returns (uint256) {
-    // TODO: Is this worth calculating once, and then saving? Seems quite likely.
     uint256 nLogEntries = reputationUpdateLog.length;
 
     // If there's no log, it must be one of the first two reputation cycles - no reward.
@@ -132,7 +131,7 @@ contract ReputationMiningCycleCommon is ReputationMiningCycleStorage, PatriciaTr
 
   function responsePossible(disputeStages stage, uint256 since) internal view returns (bool) {
     uint256 delta = sub(now, since); // I don't believe this should ever be possible to underflow...
-    if (delta < SUBMITTER_ONLY_WINDOW) {
+    if (delta <= SUBMITTER_ONLY_WINDOW) {
       // require user made a submission
       if (reputationHashSubmissions[msg.sender].proposedNewRootHash == bytes32(0x00)) {
         return false;

docs/_Interface_IColonyNetwork.md

@@ -116,19 +116,15 @@ Used by a user to claim any mining rewards due to them. This will place them in
 
 ### `createColony`
 
-Overload of the simpler `createColony` -- creates a new colony in the network with a variety of options
+Creates a new colony in the network, at version 3
 
-*Note: For the colony to mint tokens, token ownership must be transferred to the new colony*
+*Note: This is now deprecated and will be removed in a future version*
 
 **Parameters**
 
 |Name|Type|Description|
 |---|---|---|
-|_tokenAddress|address|Address of an ERC20 token to serve as the colony token
-|_version|uint256|The version of colony to deploy (pass 0 for the current version)
-|_colonyName|string|The label to register (if null, no label is registered)
-|_orbitdb|string|The path of the orbitDB database associated with the user profile
-|_useExtensionManager|bool|If true, give the ExtensionManager the root role in the colony
+|_tokenAddress|address|Address of an ERC20 token to serve as the colony token.
 
 **Return Parameters**
 
@@ -138,15 +134,19 @@ Overload of the simpler `createColony` -- creates a new colony in the network wi
 
 ### `createColony`
 
-Creates a new colony in the network, at version 3
+Overload of the simpler `createColony` -- creates a new colony in the network with a variety of options
 
-*Note: This is now deprecated and will be removed in a future version*
+*Note: For the colony to mint tokens, token ownership must be transferred to the new colony*
 
 **Parameters**
 
 |Name|Type|Description|
 |---|---|---|
-|_tokenAddress|address|Address of an ERC20 token to serve as the colony token.
+|_tokenAddress|address|Address of an ERC20 token to serve as the colony token
+|_version|uint256|The version of colony to deploy (pass 0 for the current version)
+|_colonyName|string|The label to register (if null, no label is registered)
+|_orbitdb|string|The path of the orbitDB database associated with the user profile
+|_useExtensionManager|bool|If true, give the ExtensionManager the root role in the colony
 
 **Return Parameters**
 

docs/_Interface_ITokenLocking.md

@@ -308,7 +308,7 @@ Increments the lock counter to `_lockId` for the `_user` if user's lock count is
 
 ### `withdraw`
 
-Withdraw `_amount` of deposited tokens. Can only be called if user tokens are not locked.
+DEPRECATED Withdraw `_amount` of deposited tokens. Can only be called if user tokens are not locked.
 
 
 **Parameters**
@@ -317,17 +317,17 @@ Withdraw `_amount` of deposited tokens. Can only be called if user tokens are no
 |---|---|---|
 |_token|address|Address of the token to withdraw from
 |_amount|uint256|Amount to withdraw
-|_force|bool|Pass true to forcibly unlock the token
 
 
 ### `withdraw`
 
-DEPRECATED Withdraw `_amount` of deposited tokens. Can only be called if user tokens are not locked.
+Withdraw `_amount` of deposited tokens. Can only be called if user tokens are not locked.
 
 
 **Parameters**
 
 |Name|Type|Description|
 |---|---|---|
 |_token|address|Address of the token to withdraw from
-|_amount|uint256|Amount to withdraw
+|_amount|uint256|Amount to withdraw
+|_force|bool|Pass true to forcibly unlock the token

helpers/test-helper.js

@@ -307,12 +307,50 @@ export async function forwardTimeTo(_timestamp, test) {
   await forwardTime(amountToForward.toNumber(), test);
 }
 
-export async function mineBlock() {
+export async function mineBlock(timestamp) {
   return new Promise((resolve, reject) => {
     web3.currentProvider.send(
       {
         jsonrpc: "2.0",
         method: "evm_mine",
+        params: timestamp ? [timestamp] : [],
+        id: new Date().getTime(),
+      },
+      (err) => {
+        if (err) {
+          return reject(err);
+        }
+        return resolve();
+      }
+    );
+  });
+}
+
+export async function stopMining() {
+  return new Promise((resolve, reject) => {
+    web3.currentProvider.send(
+      {
+        jsonrpc: "2.0",
+        method: "miner_stop",
+        params: [],
+        id: new Date().getTime(),
+      },
+      (err) => {
+        if (err) {
+          return reject(err);
+        }
+        return resolve();
+      }
+    );
+  });
+}
+
+export async function startMining() {
+  return new Promise((resolve, reject) => {
+    web3.currentProvider.send(
+      {
+        jsonrpc: "2.0",
+        method: "miner_start",
         params: [],
         id: new Date().getTime(),
       },
@@ -326,6 +364,18 @@ export async function mineBlock() {
   });
 }
 
+export async function makeTxAtTimestamp(f, args, timestamp, test) {
+  const client = await web3GetClient();
+  if (client.indexOf("TestRPC") === -1) {
+    test.skip();
+  }
+  await stopMining();
+  const promise = f(...args);
+  await mineBlock(timestamp);
+  await startMining();
+  return promise;
+}
+
 export function bnSqrt(bn, isGreater) {
   let a = bn.addn(1).divn(2);
   let b = bn;
@@ -443,7 +493,7 @@ export async function getActiveRepCycle(colonyNetwork) {
 }
 
 export async function advanceMiningCycleNoContest({ colonyNetwork, client, minerAddress, test }) {
-  await forwardTime(MINING_CYCLE_DURATION + SUBMITTER_ONLY_WINDOW, test);
+  await forwardTime(MINING_CYCLE_DURATION + SUBMITTER_ONLY_WINDOW + 1, test);
   const repCycle = await getActiveRepCycle(colonyNetwork);
 
   if (client !== undefined) {
@@ -647,7 +697,13 @@ export async function finishReputationMiningCycle(colonyNetwork, test) {
 
   if (nUniqueSubmittedHashes.gtn(0)) {
     const reputationMiningWindowOpenTimestamp = await repCycle.getReputationMiningWindowOpenTimestamp();
-    await forwardTimeTo(reputationMiningWindowOpenTimestamp.addn(MINING_CYCLE_DURATION).addn(SUBMITTER_ONLY_WINDOW).toNumber(), test);
+    await forwardTimeTo(
+      reputationMiningWindowOpenTimestamp
+        .addn(MINING_CYCLE_DURATION)
+        .addn(SUBMITTER_ONLY_WINDOW + 1)
+        .toNumber(),
+      test
+    );
     const nInvalidatedHashes = await repCycle.getNInvalidatedHashes();
     if (nUniqueSubmittedHashes.sub(nInvalidatedHashes).eqn(1)) {
       await repCycle.confirmNewHash(nUniqueSubmittedHashes.eqn(1) ? 0 : 1); // Not a general solution - only works for one or two submissions.

test/contracts-network/reputation-basic-functionality.js

@@ -171,7 +171,7 @@ contract("Reputation mining - basic functionality", (accounts) => {
       expect(nSubmissionsForHash).to.eq.BN(1);
 
       // Cleanup
-      await forwardTime(SUBMITTER_ONLY_WINDOW, this);
+      await forwardTime(SUBMITTER_ONLY_WINDOW + 1, this);
       await repCycle.confirmNewHash(0);
     });
   });

test/extensions/funding-queue.js

@@ -132,7 +132,7 @@ contract("Funding Queues", (accounts) => {
 
     const rootHash = await reputationTree.getRootHash();
     const repCycle = await getActiveRepCycle(colonyNetwork);
-    await forwardTime(MINING_CYCLE_DURATION + SUBMITTER_ONLY_WINDOW, this);
+    await forwardTime(MINING_CYCLE_DURATION + SUBMITTER_ONLY_WINDOW + 1, this);
     await repCycle.submitRootHash(rootHash, 0, "0x00", 10, { from: MINER });
     await repCycle.confirmNewHash(0);
   });