Make miner slightly more cautious about state

helpers/test-helper.js

@@ -10,6 +10,7 @@ const { UINT256_MAX, MIN_STAKE, MINING_CYCLE_DURATION, DEFAULT_STAKE, CHALLENGE_
 
 const IColony = artifacts.require("IColony");
 const IMetaColony = artifacts.require("IMetaColony");
+const IColonyNetwork = artifacts.require("IColonyNetwork");
 const ITokenLocking = artifacts.require("ITokenLocking");
 const Token = artifacts.require("Token");
 const IReputationMiningCycle = artifacts.require("IReputationMiningCycle");
@@ -991,7 +992,15 @@ exports.getColonyEditable = async function getColonyEditable(colony, colonyNetwo
 exports.getWaitForNSubmissionsPromise = async function getWaitForNSubmissionsPromise(repCycleEthers, rootHash, nLeaves, jrh, n) {
   return new Promise(function (resolve, reject) {
     repCycleEthers.on("ReputationRootHashSubmitted", async (_miner, _hash, _nLeaves, _jrh, _entryIndex, event) => {
-      const nSubmissions = await repCycleEthers.getNSubmissionsForHash(rootHash, nLeaves, jrh);
+      let nSubmissions;
+      // We want to see when our hash hits N submissions
+      // If we've passed in our hash, we check how many submissions that hash has
+      // If not, we're waiting for N submissions from any hash
+      if (rootHash) {
+        nSubmissions = await repCycleEthers.getNSubmissionsForHash(rootHash, nLeaves, jrh);
+      } else {
+        nSubmissions = await repCycleEthers.getNSubmissionsForHash(_hash, _nLeaves, _jrh);
+      }
       if (nSubmissions.toNumber() >= n) {
         event.removeListener();
         resolve();
@@ -1007,6 +1016,24 @@ exports.getWaitForNSubmissionsPromise = async function getWaitForNSubmissionsPro
   });
 };
 
+exports.getMiningCycleCompletePromise = async function getMiningCycleCompletePromise(colonyNetworkEthers, oldHash, expectedHash) {
+  return new Promise(function (resolve, reject) {
+    colonyNetworkEthers.on("ReputationMiningCycleComplete", async (_hash, _nLeaves, event) => {
+      const colonyNetwork = await IColonyNetwork.at(colonyNetworkEthers.address);
+      const newHash = await colonyNetwork.getReputationRootHash();
+      expect(newHash).to.not.equal(oldHash, "The old and new hashes are the same");
+      expect(newHash).to.equal(expectedHash, "The network root hash doens't match the one submitted");
+      event.removeListener();
+      resolve();
+    });
+
+    // After 30s, we throw a timeout error
+    setTimeout(() => {
+      reject(new Error("ERROR: timeout while waiting for confirming hash"));
+    }, 30000);
+  });
+};
+
 exports.encodeTxData = async function encodeTxData(colony, functionName, args) {
   const convertedArgs = [];
   args.forEach((arg) => {

packages/reputation-miner/ReputationMinerClient.js

@@ -223,6 +223,7 @@ class ReputationMinerClient {
    */
   async initialise(colonyNetworkAddress, startingBlock) {
     this.resolveBlockChecksFinished = undefined;
+    this.startingBlock = startingBlock;
     await this._miner.initialise(colonyNetworkAddress);
 
     let resumedSuccessfully = false;
@@ -277,7 +278,7 @@ class ReputationMinerClient {
       await this._miner.loadState(latestConfirmedReputationHash);
       if (this._miner.nReputations.eq(0)) {
         this._adapter.log("Latest state not found - need to sync");
-        await this._miner.sync(startingBlock, true);
+        await this._miner.sync(this.startingBlock, true);
       }
       // Initial call to process the existing log from the cycle we're currently in
       await this.processReputationLog();
@@ -388,6 +389,15 @@ class ReputationMinerClient {
           return;
         }
 
+        // First check is the confirmed cycle the one we expect?
+        // Note no blocktags in these calls - we care if we're up-to-date, not the historical state (here)
+        // If we're not, we resync and stop here for this block.
+        const syncCheck = await this.ensureSynced();
+        if (!syncCheck) {
+          this.endDoBlockChecks();
+          return;
+        }
+
         await this._miner.updatePeriodLength(repCycle);
         await this.processReputationLog();
 
@@ -426,6 +436,8 @@ class ReputationMinerClient {
             const gasPrice = await updateGasEstimate("average", this.chainId, this._adapter);
             await this._miner.setGasPrice(gasPrice);
             await this.submitEntry(entryIndex);
+            this.endDoBlockChecks();
+            return;
           }
         }
       }
@@ -453,7 +465,10 @@ class ReputationMinerClient {
 
         if (oppSubmission.proposedNewRootHash === ethers.constants.AddressZero){
           const responsePossible = await repCycle.getResponsePossible(disputeStages.INVALIDATE_HASH, entry.lastResponseTimestamp);
-          if (!responsePossible) { return; }
+          if (!responsePossible) {
+            this.endDoBlockChecks();
+            return;
+          }
           // Then we don't have an opponent
           if (round.eq(0)) {
             // We can only advance if the window is closed
@@ -645,9 +660,21 @@ class ReputationMinerClient {
 
   }
 
-  async processReputationLog() {
+  async ensureSynced() {
+    const nLeaves = await this._miner.colonyNetwork.getReputationRootHashNNodes();
+    const currentRootHash = await this._miner.colonyNetwork.getReputationRootHash();
+    const localRootHash = await this._miner.reputationTree.getRootHash();
+    if (!nLeaves.eq(this._miner.nReputations) || localRootHash !== currentRootHash) {
+      this._adapter.log(`Unexpected confirmed hash seen on colonyNetwork. Let's resync.`)
+      await this._miner.sync(this.startingBlock, true);
+      return false;
+    }
+    return true;
+  }
+
+  async processReputationLog(blockNumber) {
     this._adapter.log("📁 Processing reputation update log");
-    await this._miner.addLogContentsToReputationTree();
+    await this._miner.addLogContentsToReputationTree(blockNumber);
     this._adapter.log("💾 Writing new reputation state to database");
     await this._miner.saveCurrentState();
     this._adapter.log("💾 Caching justification tree to disk");

packages/reputation-miner/test/ReputationMinerLongTransactionMined.js

@@ -0,0 +1,37 @@
+const ethers = require("ethers");
+const ReputationMinerTestWrapper = require("./ReputationMinerTestWrapper");
+
+class ReputationMinerLongTransactionMined extends ReputationMinerTestWrapper {
+  // Only difference between this and the 'real' client should be that submitRootHash
+  // doesn't resolve until we tell it to, via resolveSubmission()
+
+  async submitRootHash(entryIndex) {
+    const hash = await this.getRootHash();
+    const nLeaves = await this.getRootHashNLeaves();
+    const jrh = await this.justificationTree.getRootHash();
+    const repCycle = await this.getActiveRepCycle();
+
+    if (!entryIndex) {
+      entryIndex = await this.getEntryIndex(); // eslint-disable-line no-param-reassign
+    }
+    let gasEstimate = ethers.BigNumber.from(1000000);
+    try {
+      gasEstimate = await repCycle.estimate.submitRootHash(hash, nLeaves, jrh, entryIndex);
+    } catch (err) { // eslint-disable-line no-empty
+
+    }
+
+    // Submit that entry
+    this.p = new Promise((resolve) => {
+      this.result = repCycle.submitRootHash(hash, nLeaves, jrh, entryIndex, { gasLimit: gasEstimate, gasPrice: this.gasPrice });
+      this.resolve = resolve;
+    })
+    return this.p;
+  }
+
+  async resolveSubmission() {
+    this.resolve(this.result);
+  }
+}
+
+module.exports = ReputationMinerLongTransactionMined;

test/reputation-system/reputation-mining-client/client-auto-functionality.js

@@ -18,6 +18,7 @@ const {
   finishReputationMiningCycle,
   currentBlock,
   getWaitForNSubmissionsPromise,
+  getMiningCycleCompletePromise,
   TestAdapter,
 } = require("../../../helpers/test-helper");
 const {
@@ -30,6 +31,7 @@ const {
 const ReputationMinerClient = require("../../../packages/reputation-miner/ReputationMinerClient");
 const ReputationMinerTestWrapper = require("../../../packages/reputation-miner/test/ReputationMinerTestWrapper");
 const MaliciousReputationMinerExtraRep = require("../../../packages/reputation-miner/test/MaliciousReputationMinerExtraRep");
+const ReputationMinerLongTransactionMined = require("../../../packages/reputation-miner/test/ReputationMinerLongTransactionMined");
 
 const { expect } = chai;
 chai.use(bnChai(web3.utils.BN));
@@ -104,7 +106,7 @@ process.env.SOLIDITY_COVERAGE
       afterEach(async function () {
         await reputationMinerClient.close();
         const reputationMiningGotClean = await finishReputationMiningCycle(colonyNetwork, this);
-        if (!reputationMiningGotClean) await setupNewNetworkInstance(MINER1, MINER2);
+        if (!reputationMiningGotClean) await setupNewNetworkInstance(MINER1, MINER2, MINER3);
       });
 
       describe("core functionality", function () {
@@ -122,21 +124,7 @@ process.env.SOLIDITY_COVERAGE
           await forwardTime(MINING_CYCLE_DURATION * 0.9, this);
           await receive12Submissions;
 
-          const colonyNetworkEthers = await reputationMinerClient._miner.colonyNetwork;
-          const miningCycleComplete = new Promise(function (resolve, reject) {
-            colonyNetworkEthers.on("ReputationMiningCycleComplete", async (_hash, _nLeaves, event) => {
-              const newHash = await colonyNetwork.getReputationRootHash();
-              expect(newHash).to.not.equal(oldHash, "The old and new hashes are the same");
-              expect(newHash).to.equal(rootHash, "The network root hash doens't match the one submitted");
-              event.removeListener();
-              resolve();
-            });
-
-            // After 30s, we throw a timeout error
-            setTimeout(() => {
-              reject(new Error("ERROR: timeout while waiting for confirming hash"));
-            }, 30000);
-          });
+          const miningCycleComplete = getMiningCycleCompletePromise(reputationMinerClient._miner.colonyNetwork, oldHash, rootHash);
 
           // Forward time to the end of the mining cycle and since we are the only miner, check the client confirmed our hash correctly
           await forwardTime(MINING_CYCLE_DURATION * 0.1 + CHALLENGE_RESPONSE_WINDOW_DURATION + 1, this);
@@ -169,21 +157,7 @@ process.env.SOLIDITY_COVERAGE
           await forwardTime(MINING_CYCLE_DURATION * 0.9, this);
           await receive12Submissions;
 
-          const colonyNetworkEthers = await reputationMinerClient._miner.colonyNetwork;
-          const miningCycleComplete = new Promise(function (resolve, reject) {
-            colonyNetworkEthers.on("ReputationMiningCycleComplete", async (_hash, _nLeaves, event) => {
-              const newHash = await colonyNetwork.getReputationRootHash();
-              expect(newHash).to.not.equal(oldHash, "The old and new hashes are the same");
-              expect(newHash).to.equal(rootHash, "The network root hash doens't match the one submitted");
-              event.removeListener();
-              resolve();
-            });
-
-            // After 30s, we throw a timeout error
-            setTimeout(() => {
-              reject(new Error("ERROR: timeout while waiting for confirming hash"));
-            }, 30000);
-          });
+          const miningCycleComplete = getMiningCycleCompletePromise(reputationMinerClient._miner.colonyNetwork, oldHash, rootHash);
 
           let oracleCheckInterval;
 
@@ -253,21 +227,7 @@ process.env.SOLIDITY_COVERAGE
           await mineBlock();
           await receive12Submissions;
 
-          const colonyNetworkEthers = await reputationMinerClient._miner.colonyNetwork;
-          const miningCycleComplete = new Promise(function (resolve, reject) {
-            colonyNetworkEthers.on("ReputationMiningCycleComplete", async (_hash, _nLeaves, event) => {
-              const newHash = await colonyNetwork.getReputationRootHash();
-              expect(newHash).to.not.equal(oldHash, "The old and new hashes are the same");
-              expect(newHash).to.equal(rootHash, "The network root hash doens't match the one submitted");
-              event.removeListener();
-              resolve();
-            });
-
-            // After 30s, we throw a timeout error
-            setTimeout(() => {
-              reject(new Error("ERROR: timeout while waiting for confirming hash"));
-            }, 30000);
-          });
+          const miningCycleComplete = getMiningCycleCompletePromise(reputationMinerClient._miner.colonyNetwork, oldHash, rootHash);
 
           // Forward time to the end of the mining cycle and since we are the only miner, check the client confirmed our hash correctly
           await forwardTime(MINING_CYCLE_DURATION * 0.6 + MINING_CYCLE_DURATION, this);
@@ -293,21 +253,7 @@ process.env.SOLIDITY_COVERAGE
           await checkSuccessEthers(goodClient.submitRootHash());
           await receive12Submissions;
 
-          const colonyNetworkEthers = await reputationMinerClient._miner.colonyNetwork;
-          const miningCycleComplete = new Promise(function (resolve, reject) {
-            colonyNetworkEthers.on("ReputationMiningCycleComplete", async (_hash, _nLeaves, event) => {
-              const newHash = await colonyNetwork.getReputationRootHash();
-              expect(newHash).to.not.equal(oldHash, "The old and new hashes are the same");
-              expect(newHash).to.equal(rootHash, "The network root hash doens't match the one submitted");
-              event.removeListener();
-              resolve();
-            });
-
-            // After 30s, we throw a timeout error
-            setTimeout(() => {
-              reject(new Error("ERROR: timeout while waiting for confirming hash"));
-            }, 60000);
-          });
+          const miningCycleComplete = getMiningCycleCompletePromise(reputationMinerClient._miner.colonyNetwork, oldHash, rootHash);
 
           // Forward time to the end of the mining cycle and since we are the only miner, check the client confirmed our hash correctly
           await forwardTime(MINING_CYCLE_DURATION / 2 + CHALLENGE_RESPONSE_WINDOW_DURATION + 1, this);
@@ -769,6 +715,87 @@ process.env.SOLIDITY_COVERAGE
           await reputationMinerClient2.close();
         });
 
+        it(`should continue to mine successfully even if the submission hash takes a long time to be mined
+          (e.g. because it ran out of funds)`, async function () {
+          let repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle();
+          // Advance through a reputation cycle
+          let rootHash = await reputationMinerClient._miner.getRootHash();
+
+          let receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12);
+
+          const delayedReputationMinerClient = new ReputationMinerClient({
+            loader,
+            realProviderPort,
+            minerAddress: MINER2,
+            useJsTree: true,
+            auto: true,
+            oracle: false,
+          });
+          // That client is fine until we give it an awkward miner
+          delayedReputationMinerClient._miner = new ReputationMinerLongTransactionMined({
+            minerAddress: MINER2,
+            loader,
+            realProviderPort,
+            useJsTree: true,
+          });
+
+          await delayedReputationMinerClient.initialise(colonyNetwork.address, startingBlockNumber);
+
+          // Forward through most of the cycle duration and wait for the clients to submit all 12 allowed entries
+          await forwardTime(MINING_CYCLE_DURATION * 0.9, this);
+          await receive12Submissions;
+
+          let oldHash = await colonyNetwork.getReputationRootHash();
+
+          let miningCycleComplete = getMiningCycleCompletePromise(reputationMinerClient._miner.colonyNetwork, oldHash, rootHash);
+
+          // Forward time to the end of the mining cycle and since we are the only miner, check the client confirmed our hash correctly
+          await forwardTime(MINING_CYCLE_DURATION * 0.1 + CHALLENGE_RESPONSE_WINDOW_DURATION + 1, this);
+          await miningCycleComplete;
+
+          // Advance through another - the client should still be waiting for the first transaction to return.
+          repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle();
+
+          reputationMinerClient.blocksSinceCycleCompleted = 10;
+          await mineBlock();
+
+          receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12);
+
+          await forwardTime(MINING_CYCLE_DURATION * 0.9, this);
+          await receive12Submissions;
+
+          rootHash = await reputationMinerClient._miner.getRootHash();
+          oldHash = await colonyNetwork.getReputationRootHash();
+          miningCycleComplete = getMiningCycleCompletePromise(reputationMinerClient._miner.colonyNetwork, oldHash, rootHash);
+
+          await forwardTime(MINING_CYCLE_DURATION * 0.1 + CHALLENGE_RESPONSE_WINDOW_DURATION + 1, this);
+          await miningCycleComplete;
+
+          // We now resolve the original
+          delayedReputationMinerClient._miner.resolveSubmission();
+
+          // And then we get both clients to process the newest cycle. The good miner will update normally.
+          // In the case of the delayedReputationMinerClient, we expect it to recognise something has gone
+          // wrong, and resync.
+          delayedReputationMinerClient.blocksSinceCycleCompleted = 10;
+          reputationMinerClient.blocksSinceCycleCompleted = 10;
+          await mineBlock();
+          repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle();
+
+          receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12);
+
+          await forwardTime(MINING_CYCLE_DURATION * 0.9, this);
+          await receive12Submissions;
+
+          // check delayed miner and good miner have ended up in the same state.
+          const rootHash2 = await reputationMinerClient._miner.getRootHash();
+          const rootHash3 = await delayedReputationMinerClient._miner.getRootHash();
+          expect(rootHash2).to.equal(rootHash3);
+
+          delayedReputationMinerClient._miner.resolveSubmission();
+          await delayedReputationMinerClient.close();
+        });
+
         function noEventSeen(contract, event) {
           return new Promise(function (resolve, reject) {
             contract.on(event, async () => {