From e3139f46513b78d5ade56eed9d88517344dfb954 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Wed, 2 Nov 2022 12:16:56 +0000 Subject: [PATCH 01/12] Add test demonstrating issue with miner ordering --- docs/interfaces/ireputationminingcycle.md | 18 +++++ .../client-auto-functionality.js | 77 +++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/docs/interfaces/ireputationminingcycle.md b/docs/interfaces/ireputationminingcycle.md index 4133892364..f3f92417b2 100644 --- a/docs/interfaces/ireputationminingcycle.md +++ b/docs/interfaces/ireputationminingcycle.md @@ -277,6 +277,7 @@ Get the length of the ReputationUpdateLog stored on this instance of the Reputat Returns whether the caller is able to currently respond to a dispute stage. +*Note: Deprecated* **Parameters** @@ -291,6 +292,23 @@ Returns whether the caller is able to currently respond to a dispute stage. |---|---|---| |possible|bool|bool Whether the user can respond at the current time. +### ▸ `getResponsePossible(uint256 _since):bool possible` + +Returns whether the caller is able to currently respond to a dispute stage. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|_since|uint256|The timestamp the last response for the submission in the dispute in question was made at. + +**Return Parameters** + +|Name|Type|Description| +|---|---|---| +|possible|bool|bool Whether the user can respond at the current time. + ### ▸ `getSubmissionUser(bytes32 _hash, uint256 _nLeaves, bytes32 _jrh, uint256 _index):address user` Get the address that made a particular submission. diff --git a/test/reputation-system/reputation-mining-client/client-auto-functionality.js b/test/reputation-system/reputation-mining-client/client-auto-functionality.js index 9f79d8ea5d..dbc371bc0b 100644 --- a/test/reputation-system/reputation-mining-client/client-auto-functionality.js +++ b/test/reputation-system/reputation-mining-client/client-auto-functionality.js @@ -260,6 +260,83 @@ process.env.SOLIDITY_COVERAGE await miningCycleComplete; }); + it("miners should be randomised in terms of order of allowed responses each cycle", async function () { + reputationMinerClient = new ReputationMinerClient({ + loader, + realProviderPort, + minerAddress: MINER1, + useJsTree: true, + auto: true, + oracle: false, + processingDelay: 1, + }); + + const reputationMinerClient2 = new ReputationMinerClient({ + loader, + realProviderPort, + minerAddress: MINER2, + useJsTree: true, + auto: true, + oracle: false, + processingDelay: 1, + }); + await reputationMinerClient2.initialise(colonyNetwork.address, startingBlockNumber); + await reputationMinerClient.initialise(colonyNetwork.address, startingBlockNumber); + function sleep(ms) { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); + } + + let differentAddresses = false; + const completionAddresses = []; + while (!differentAddresses) { + const colonyNetworkEthers = reputationMinerClient._miner.colonyNetwork; + let completionEvent; + const repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle(); + const receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12); + + // Forward time and wait for the client to submit all 12 allowed entries + await forwardTime(MINING_CYCLE_DURATION / 2, this); + // await checkSuccessEthers(goodClient.submitRootHash()); + await receive12Submissions; + + let cycleComplete = false; + let error = false; + const miningCycleCompletePromise = new Promise(function (resolve, reject) { + colonyNetworkEthers.on("ReputationMiningCycleComplete", async (_hash, _nLeaves, event) => { + event.removeListener(); + cycleComplete = true; + completionEvent = event; + resolve(); + }); + + // After 30s, we throw a timeout error + setTimeout(() => { + error = true; + reject(new Error("ERROR: timeout while waiting for confirming hash")); + }, 30000); + }); + + while (!cycleComplete && !error) { + await forwardTime(MINING_CYCLE_DURATION / 10); + await sleep(1000); + } + + if (error) { + throw miningCycleCompletePromise; + } + + const t = await completionEvent.getTransaction(); + completionAddresses.push(t.from); + // We repeat this loop until both miners have confirmed in different cycles + if ([...new Set(completionAddresses)].length > 1) { + differentAddresses = true; + } + } + await reputationMinerClient2.close(); + }); + it("should successfully complete a dispute resolution", async function () { const badClient = new MaliciousReputationMinerExtraRep({ loader, realProviderPort, useJsTree: true, minerAddress: MINER3 }, 1, 0); await badClient.initialise(colonyNetwork.address); From 3524289e63f33cc61c712937125096f9ddd9f22c Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Wed, 2 Nov 2022 12:24:44 +0000 Subject: [PATCH 02/12] Update miner to explicitly call soon-to-be-deprecated function --- .../reputation-miner/ReputationMinerClient.js | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/reputation-miner/ReputationMinerClient.js b/packages/reputation-miner/ReputationMinerClient.js index fa103e030a..9e4f68f0e5 100644 --- a/packages/reputation-miner/ReputationMinerClient.js +++ b/packages/reputation-miner/ReputationMinerClient.js @@ -464,7 +464,7 @@ class ReputationMinerClient { const oppSubmission = await repCycle.getReputationHashSubmission(oppEntry.firstSubmitter); if (oppSubmission.proposedNewRootHash === ethers.constants.AddressZero){ - const responsePossible = await repCycle.getResponsePossible(disputeStages.INVALIDATE_HASH, entry.lastResponseTimestamp); + const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"](disputeStages.INVALIDATE_HASH, entry.lastResponseTimestamp); if (!responsePossible) { this.endDoBlockChecks(); return; @@ -497,7 +497,7 @@ class ReputationMinerClient { // Before checking if our opponent has timed out yet, check if we can respond to something // 1. Do we still need to confirm JRH? if (submission.jrhNLeaves.eq(0)) { - const responsePossible = await repCycle.getResponsePossible(disputeStages.CONFIRM_JRH, entry.lastResponseTimestamp); + const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"](disputeStages.CONFIRM_JRH, entry.lastResponseTimestamp); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -512,7 +512,10 @@ class ReputationMinerClient { // We can respond if neither of us have responded to this stage yet or // if they have responded already if (oppEntry.challengeStepCompleted.gte(entry.challengeStepCompleted)) { - const responsePossible = await repCycle.getResponsePossible(disputeStages.BINARY_SEARCH_RESPONSE, entry.lastResponseTimestamp); + const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( + disputeStages.BINARY_SEARCH_RESPONSE, + entry.lastResponseTimestamp + ); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -530,7 +533,10 @@ class ReputationMinerClient { ethers.BigNumber.from(2).pow(entry.challengeStepCompleted.sub(2)).lte(submission.jrhNLeaves) ) { - const responsePossible = await repCycle.getResponsePossible(disputeStages.BINARY_SEARCH_CONFIRM, entry.lastResponseTimestamp); + const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( + disputeStages.BINARY_SEARCH_CONFIRM, + entry.lastResponseTimestamp + ); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -548,7 +554,10 @@ class ReputationMinerClient { ethers.BigNumber.from(2).pow(entry.challengeStepCompleted.sub(3)).lte(submission.jrhNLeaves) ) { - const responsePossible = await repCycle.getResponsePossible(disputeStages.RESPOND_TO_CHALLENGE, entry.lastResponseTimestamp); + const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( + disputeStages.RESPOND_TO_CHALLENGE, + entry.lastResponseTimestamp + ); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -562,7 +571,7 @@ class ReputationMinerClient { const opponentTimeout = ethers.BigNumber.from(block.timestamp).sub(oppEntry.lastResponseTimestamp).gte(CHALLENGE_RESPONSE_WINDOW_DURATION); if (opponentTimeout){ - const responsePossible = await repCycle.getResponsePossible( + const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( disputeStages.INVALIDATE_HASH, ethers.BigNumber.from(oppEntry.lastResponseTimestamp).add(CHALLENGE_RESPONSE_WINDOW_DURATION) ); @@ -585,7 +594,7 @@ class ReputationMinerClient { const disputeRound = await repCycle.getDisputeRound(round); const entry = disputeRound[index]; - const responsePossible = await repCycle.getResponsePossible(disputeStages.CONFIRM_NEW_HASH, entry.lastResponseTimestamp); + const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"](disputeStages.CONFIRM_NEW_HASH, entry.lastResponseTimestamp); if (responsePossible){ await this.confirmEntry(); } From 083e4eadec1e998ebd620149742f1e21dcdf2ab5 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Wed, 2 Nov 2022 12:26:25 +0000 Subject: [PATCH 03/12] Correctly randomise allowed order of miner responses --- .../IReputationMiningCycle.sol | 7 +++++++ .../ReputationMiningCycle.sol | 14 +++++++++----- .../ReputationMiningCycleBinarySearch.sol | 4 ++-- .../ReputationMiningCycleCommon.sol | 4 ++-- .../ReputationMiningCycleRespond.sol | 2 +- test-smoke/colony-storage-consistent.js | 10 +++++----- .../client-auto-functionality.js | 12 ++---------- 7 files changed, 28 insertions(+), 25 deletions(-) diff --git a/contracts/reputationMiningCycle/IReputationMiningCycle.sol b/contracts/reputationMiningCycle/IReputationMiningCycle.sol index 1153377f72..047a772333 100644 --- a/contracts/reputationMiningCycle/IReputationMiningCycle.sol +++ b/contracts/reputationMiningCycle/IReputationMiningCycle.sol @@ -271,5 +271,12 @@ interface IReputationMiningCycle is ReputationMiningCycleDataTypes { /// enum in ReputationMiningCycleDataTypes /// @param _since The timestamp the last response for the submission in the dispute in question was made at. /// @return possible bool Whether the user can respond at the current time. + /// @dev Deprecated function getResponsePossible(DisputeStages _stage, uint256 _since) external view returns (bool possible); + + /// @notice Returns whether the caller is able to currently respond to a dispute stage. + /// @param _since The timestamp the last response for the submission in the dispute in question was made at. + /// @return possible bool Whether the user can respond at the current time. + function getResponsePossible(uint256 _since) external view returns (bool possible); + } diff --git a/contracts/reputationMiningCycle/ReputationMiningCycle.sol b/contracts/reputationMiningCycle/ReputationMiningCycle.sol index 1a6741c1b5..87cf092b1e 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycle.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycle.sol @@ -216,7 +216,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { require(submissionWindowClosed(), "colony-reputation-mining-submission-window-still-open"); require( - responsePossible(DisputeStages.ConfirmNewHash, disputeRounds[_roundNumber][0].lastResponseTimestamp), + responsePossible(disputeRounds[_roundNumber][0].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -256,7 +256,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { // Is the person making this call eligible to? require( - responsePossible(DisputeStages.InvalidateHash, disputeRounds[_round][opponentIdx].lastResponseTimestamp), + responsePossible(disputeRounds[_round][opponentIdx].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -292,7 +292,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { // The submission can be invalidated - now check the person invalidating is allowed to require( - responsePossible(DisputeStages.InvalidateHash, add(disputeRounds[_round][_idx].lastResponseTimestamp, CHALLENGE_RESPONSE_WINDOW_DURATION)), + responsePossible(add(disputeRounds[_round][_idx].lastResponseTimestamp, CHALLENGE_RESPONSE_WINDOW_DURATION)), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -350,7 +350,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { require(submissionWindowClosed(), "colony-reputation-mining-cycle-submissions-not-closed"); require(_index < disputeRounds[_round].length, "colony-reputation-mining-index-beyond-round-length"); require( - responsePossible(DisputeStages.ConfirmNewHash, disputeRounds[_round][_index].lastResponseTimestamp), + responsePossible(disputeRounds[_round][_index].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -500,7 +500,11 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { } function getResponsePossible(DisputeStages _stage, uint256 _since) external view returns (bool) { - return responsePossible(_stage, _since); + return responsePossible(_since); + } + + function getResponsePossible(uint256 _since) external view returns (bool) { + return responsePossible(_since); } ///////////////////////// diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol b/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol index 50b52c078a..eac32713e4 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol @@ -37,7 +37,7 @@ contract ReputationMiningCycleBinarySearch is ReputationMiningCycleCommon { require(_idx < disputeRounds[_round].length, "colony-reputation-mining-index-beyond-round-length"); require(disputeRounds[_round][_idx].lowerBound != disputeRounds[_round][_idx].upperBound, "colony-reputation-mining-challenge-not-active"); require( - responsePossible(DisputeStages.BinarySearchResponse, disputeRounds[_round][_idx].lastResponseTimestamp), + responsePossible(disputeRounds[_round][_idx].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -87,7 +87,7 @@ contract ReputationMiningCycleBinarySearch is ReputationMiningCycleCommon { "colony-reputation-binary-search-result-already-confirmed" ); require( - responsePossible(DisputeStages.BinarySearchConfirm, disputeRounds[_round][_idx].lastResponseTimestamp), + responsePossible(disputeRounds[_round][_idx].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol b/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol index 5262aa2882..9b1bdf07f3 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol @@ -146,7 +146,7 @@ contract ReputationMiningCycleCommon is ReputationMiningCycleStorage, PatriciaTr uint256 constant CHALLENGE_RESPONSE_WINDOW_DURATION = 60 * 20; uint256 constant Y = UINT256_MAX / (CHALLENGE_RESPONSE_WINDOW_DURATION - ALL_ENTRIES_ALLOWED_END_OF_WINDOW); - function responsePossible(DisputeStages _stage, uint256 _responseWindowOpened) internal view returns (bool) { + function responsePossible(uint256 _responseWindowOpened) internal view returns (bool) { if (_responseWindowOpened > block.timestamp) { // I don't think this is currently possible, but belt and braces! return false; @@ -162,7 +162,7 @@ contract ReputationMiningCycleCommon is ReputationMiningCycleStorage, PatriciaTr return false; } uint256 target = windowOpenFor * Y; - if (uint256(keccak256(abi.encodePacked(minerAddress, _stage))) > target) { + if (uint256(keccak256(abi.encodePacked(minerAddress, _responseWindowOpened))) > target) { return false; } } diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol b/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol index e83e7efd49..a4a43f910d 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol @@ -112,7 +112,7 @@ contract ReputationMiningCycleRespond is ReputationMiningCycleCommon { challengeOpen(_u[U_ROUND], _u[U_IDX]) { require( - responsePossible(DisputeStages.RespondToChallenge, disputeRounds[_u[U_ROUND]][_u[U_IDX]].lastResponseTimestamp), + responsePossible(disputeRounds[_u[U_ROUND]][_u[U_IDX]].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); diff --git a/test-smoke/colony-storage-consistent.js b/test-smoke/colony-storage-consistent.js index c72d16b4d9..b7f1ce8276 100644 --- a/test-smoke/colony-storage-consistent.js +++ b/test-smoke/colony-storage-consistent.js @@ -149,11 +149,11 @@ contract("Contract Storage", (accounts) => { console.log("miningCycleStateHash:", miningCycleStateHash); console.log("tokenLockingStateHash:", tokenLockingStateHash); - expect(colonyNetworkStateHash).to.equal("0xa9289d3025a1f5e108b7b68f335327c1c5748015db91c78978679ba9832984e1"); - expect(colonyStateHash).to.equal("0x54a0edcb2097270bd95d610dc827869cc827241d131461f58788f7c3257ca151"); - expect(metaColonyStateHash).to.equal("0x15fab25907cfb6baedeaf1fdabd68678d37584a1817a08dfe77db60db378a508"); - expect(miningCycleStateHash).to.equal("0x632d459a2197708bd2dbde87e8275c47dddcdf16d59e3efd21dcef9acb2a7366"); - expect(tokenLockingStateHash).to.equal("0x30fbcbfbe589329fe20288101faabe1f60a4610ae0c0effb15526c6b390a8e07"); + expect(colonyNetworkStateHash).to.equal("0x77e04702d554bbcff2c36be7ccce767adce92a13c135a9cd279dd3f7415b093b"); + expect(colonyStateHash).to.equal("0x4e90fcbe58b79118b2a2c09dd96c2cefe46f732b18b1d6230a361c0332133dec"); + expect(metaColonyStateHash).to.equal("0x6be6cb630afd143ac7db391fb52150d3b817443b59206497e36aa4ffbeca5c1a"); + expect(miningCycleStateHash).to.equal("0x20b0a911563ceab3018f750b12c1dda75608436d3f67abb061a1201434931028"); + expect(tokenLockingStateHash).to.equal("0xc9fa6f26cac13030e857f1c4aeb6057d65a0d196a11e69c152483aa382270a2a"); }); }); }); diff --git a/test/reputation-system/reputation-mining-client/client-auto-functionality.js b/test/reputation-system/reputation-mining-client/client-auto-functionality.js index dbc371bc0b..ea112e7cab 100644 --- a/test/reputation-system/reputation-mining-client/client-auto-functionality.js +++ b/test/reputation-system/reputation-mining-client/client-auto-functionality.js @@ -261,16 +261,7 @@ process.env.SOLIDITY_COVERAGE }); it("miners should be randomised in terms of order of allowed responses each cycle", async function () { - reputationMinerClient = new ReputationMinerClient({ - loader, - realProviderPort, - minerAddress: MINER1, - useJsTree: true, - auto: true, - oracle: false, - processingDelay: 1, - }); - + reputationMinerClient._processingDelay = 1; const reputationMinerClient2 = new ReputationMinerClient({ loader, realProviderPort, @@ -335,6 +326,7 @@ process.env.SOLIDITY_COVERAGE } } await reputationMinerClient2.close(); + reputationMinerClient._processingDelay = 10; }); it("should successfully complete a dispute resolution", async function () { From 6d130ecea024b07a56cd21ed34c2ba0e5d906959 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Tue, 8 Nov 2022 11:17:16 +0000 Subject: [PATCH 04/12] If a miner loses a race, allow it to keep mining --- helpers/test-helper.js | 26 +++- .../reputation-miner/ReputationMinerClient.js | 17 +++ .../client-auto-functionality.js | 131 +++++++++++++++++- 3 files changed, 166 insertions(+), 8 deletions(-) diff --git a/helpers/test-helper.js b/helpers/test-helper.js index 958ec9803e..953af7183c 100644 --- a/helpers/test-helper.js +++ b/helpers/test-helper.js @@ -297,6 +297,18 @@ exports.currentBlock = async function currentBlock() { return p; }; +exports.getBlock = async function getBlock(blockTag) { + const p = new Promise((resolve, reject) => { + web3.eth.getBlock(blockTag, (err, res) => { + if (err) { + return reject(err); + } + return resolve(res); + }); + }); + return p; +}; + exports.getBlockTime = async function getBlockTime(blockNumber = "latest") { const p = new Promise((resolve, reject) => { web3.eth.getBlock(blockNumber, (err, res) => { @@ -1032,8 +1044,12 @@ exports.getMiningCycleCompletePromise = async function getMiningCycleCompletePro 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"); + if (oldHash) { + expect(newHash).to.not.equal(oldHash, "The old and new hashes are the same"); + } + if (expectedHash) { + expect(newHash).to.equal(expectedHash, "The network root hash doens't match the one submitted"); + } event.removeListener(); resolve(); }); @@ -1104,6 +1120,12 @@ exports.rolesToBytes32 = function rolesToBytes32(roles) { return `0x${new BN(roles.map((role) => new BN(1).shln(role)).reduce((a, b) => a.or(b), new BN(0))).toString(16, 64)}`; }; +exports.sleep = function sleep(ms) { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +}; + class TestAdapter { constructor() { this.outputs = []; diff --git a/packages/reputation-miner/ReputationMinerClient.js b/packages/reputation-miner/ReputationMinerClient.js index 9e4f68f0e5..9a7e98486b 100644 --- a/packages/reputation-miner/ReputationMinerClient.js +++ b/packages/reputation-miner/ReputationMinerClient.js @@ -21,6 +21,16 @@ const CHALLENGE_RESPONSE_WINDOW_DURATION = 20 * 60; const cache = apicache.middleware +const racingFunctionSignatures = [ + "submitRootHash(bytes32,uint256,bytes32,uint256)", + "confirmNewHash(uint256)", + "invalidateHash(uint256, uint256)", + "respondToBinarySearchForChallenge(uint256,uint256 ,bytes,bytes32[])", + "confirmBinarySearchResult(uint256,uint256,bytes,bytes32[] )", + "respondToChallenge(uint256[26],bytes32[7],bytes32[],bytes32[],bytes32[],bytes32[],bytes32[],bytes32[])", + "confirmJustificationRootHash(uint256,uint256,bytes32[],bytes32[])" +].map(x => ethers.utils.id(x).slice(0,10)) + class ReputationMinerClient { /** * Constructor for ReputationMiner @@ -616,6 +626,13 @@ class ReputationMinerClient { return; } this._adapter.error(`Error during block checks: ${err}`); + if (racingFunctionSignatures.indexOf(err.transaction.data.slice(0, 10)) > -1){ + // An error on a function that we were 'racing' to execute failed - most likely because someone else did it. + // So let's keep mining. + console.log('Sometimes-expected transaction failure - we lost a race to submit for a stage. Continuing mining') + this.endDoBlockChecks(); + return; + } if (this._exitOnError) { this._adapter.error(`Automatically restarting`); process.exit(1); diff --git a/test/reputation-system/reputation-mining-client/client-auto-functionality.js b/test/reputation-system/reputation-mining-client/client-auto-functionality.js index ea112e7cab..14e2b863ab 100644 --- a/test/reputation-system/reputation-mining-client/client-auto-functionality.js +++ b/test/reputation-system/reputation-mining-client/client-auto-functionality.js @@ -20,6 +20,12 @@ const { getWaitForNSubmissionsPromise, getMiningCycleCompletePromise, TestAdapter, + getBlock, + web3GetTransactionReceipt, + web3GetTransaction, + sleep, + stopMining, + startMining, } = require("../../../helpers/test-helper"); const { setupColonyNetwork, @@ -271,13 +277,9 @@ process.env.SOLIDITY_COVERAGE oracle: false, processingDelay: 1, }); - await reputationMinerClient2.initialise(colonyNetwork.address, startingBlockNumber); await reputationMinerClient.initialise(colonyNetwork.address, startingBlockNumber); - function sleep(ms) { - return new Promise((resolve) => { - setTimeout(resolve, ms); - }); - } + await reputationMinerClient2.initialise(colonyNetwork.address, startingBlockNumber); + await mineBlock(); let differentAddresses = false; const completionAddresses = []; @@ -329,6 +331,123 @@ process.env.SOLIDITY_COVERAGE reputationMinerClient._processingDelay = 10; }); + it("Losing a race shouldn't prevent a miner from continuing", async function () { + reputationMinerClient._processingDelay = 1; + const reputationMinerClient2 = new ReputationMinerClient({ + loader, + realProviderPort, + minerAddress: MINER3, + useJsTree: true, + auto: true, + oracle: false, + processingDelay: 1, + }); + await reputationMinerClient2.initialise(colonyNetwork.address, startingBlockNumber); + + let lostRace = false; + // while (!lostRace) { + while (reputationMinerClient.lockedForBlockProcessing || reputationMinerClient2.lockedForBlockProcessing) { + await sleep(1000); + } + reputationMinerClient.lockedForBlockProcessing = true; + reputationMinerClient2.lockedForBlockProcessing = true; + await mineBlock(); + + let startingBlock = await currentBlock(); + startingBlockNumber = startingBlock.number; + + let repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle(); + let receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12); + + await forwardTime(MINING_CYCLE_DURATION / 2, this); + const oldHash = await colonyNetwork.getReputationRootHash(); + + await goodClient.loadState(oldHash); + await goodClient.addLogContentsToReputationTree(); + for (let i = 0; i < 11; i += 1) { + await goodClient.submitRootHash(); + } + await stopMining(); + + const submissionIndex1 = reputationMinerClient.submissionIndex; + const submissionIndex2 = reputationMinerClient2.submissionIndex; + reputationMinerClient.lockedForBlockProcessing = false; + reputationMinerClient2.lockedForBlockProcessing = false; + + await mineBlock(); + while (reputationMinerClient.submissionIndex === submissionIndex1 || reputationMinerClient2.submissionIndex === submissionIndex2) { + await sleep(1000); + } + + await startMining(); + await mineBlock(); + + await receive12Submissions; + // 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); + + await goodClient.confirmNewHash(); + let endBlock = await currentBlock(); + let endBlockNumber = endBlock.number; + // For every block... + for (let i = startingBlockNumber; i <= endBlockNumber; i += 1) { + const block = await getBlock(i); + // Check every transaction... + for (let txCount = 0; txCount < block.transactions.length; txCount += 1) { + const txHash = block.transactions[txCount]; + const txReceipt = await web3GetTransactionReceipt(txHash); + if (!txReceipt.status) { + // Was it actually a race? + const tx = await web3GetTransaction(txHash); + if (tx.input.slice(0, 10) === "0x3fcbcf0d") { + lostRace = true; + } + } + } + } + // } + + assert(lostRace, "No lostrace seen"); + + // So we've now seen a miner lose a race - let's check they can go through a cycle correctly. + repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle(); + receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12); + + startingBlock = await currentBlock(); + startingBlockNumber = startingBlock.number; + // Forward time and wait for the clients to submit all 12 allowed entries + await forwardTime(MINING_CYCLE_DURATION / 2, this); + + await receive12Submissions; + endBlock = await currentBlock(); + endBlockNumber = endBlock.number; + + const submissionAddresses = []; + + // For every block... + for (let i = startingBlockNumber; i <= endBlockNumber; i += 1) { + const block = await getBlock(i); + // Check every transaction... + for (let txCount = 0; txCount < block.transactions.length; txCount += 1) { + const txHash = block.transactions[txCount]; + const tx = await web3GetTransaction(txHash); + if (tx.input.slice(0, 10) === "0x3fcbcf0d") { + submissionAddresses.push(tx.from); + } + } + } + + // If we are locked for block processing (for example, after stopping block checks due to an error), this will hang + // Reset everything to be well behaved before testing the assertion. + reputationMinerClient2.lockedForBlockProcessing = false; + await reputationMinerClient2.close(); + reputationMinerClient._processingDelay = 10; + + if ([...new Set(submissionAddresses)].length === 1) { + assert(false, "Only one miner address seen"); + } + }); + it("should successfully complete a dispute resolution", async function () { const badClient = new MaliciousReputationMinerExtraRep({ loader, realProviderPort, useJsTree: true, minerAddress: MINER3 }, 1, 0); await badClient.initialise(colonyNetwork.address); From cce8f9a8ee930b34581d0f7248f9162cd0ae0d3a Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Tue, 8 Nov 2022 11:21:35 +0000 Subject: [PATCH 05/12] Changes following first review --- .../reputation-mining-client/client-auto-functionality.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/reputation-system/reputation-mining-client/client-auto-functionality.js b/test/reputation-system/reputation-mining-client/client-auto-functionality.js index 14e2b863ab..c3931e9504 100644 --- a/test/reputation-system/reputation-mining-client/client-auto-functionality.js +++ b/test/reputation-system/reputation-mining-client/client-auto-functionality.js @@ -284,18 +284,17 @@ process.env.SOLIDITY_COVERAGE let differentAddresses = false; const completionAddresses = []; while (!differentAddresses) { - const colonyNetworkEthers = reputationMinerClient._miner.colonyNetwork; - let completionEvent; const repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle(); const receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12); // Forward time and wait for the client to submit all 12 allowed entries await forwardTime(MINING_CYCLE_DURATION / 2, this); - // await checkSuccessEthers(goodClient.submitRootHash()); await receive12Submissions; let cycleComplete = false; let error = false; + const colonyNetworkEthers = reputationMinerClient._miner.colonyNetwork; + let completionEvent; const miningCycleCompletePromise = new Promise(function (resolve, reject) { colonyNetworkEthers.on("ReputationMiningCycleComplete", async (_hash, _nLeaves, event) => { event.removeListener(); From 7627200c83a2587e085f9e90b4e2b764d05c2a8d Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Tue, 8 Nov 2022 15:55:40 +0000 Subject: [PATCH 06/12] Do not clobber global variable in tests --- .../client-auto-functionality.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/reputation-system/reputation-mining-client/client-auto-functionality.js b/test/reputation-system/reputation-mining-client/client-auto-functionality.js index c3931e9504..4de7aff5da 100644 --- a/test/reputation-system/reputation-mining-client/client-auto-functionality.js +++ b/test/reputation-system/reputation-mining-client/client-auto-functionality.js @@ -352,8 +352,8 @@ process.env.SOLIDITY_COVERAGE reputationMinerClient2.lockedForBlockProcessing = true; await mineBlock(); - let startingBlock = await currentBlock(); - startingBlockNumber = startingBlock.number; + let latestBlock = await currentBlock(); + let firstSubmissionBlockNumber = latestBlock.number; let repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle(); let receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12); @@ -389,7 +389,7 @@ process.env.SOLIDITY_COVERAGE let endBlock = await currentBlock(); let endBlockNumber = endBlock.number; // For every block... - for (let i = startingBlockNumber; i <= endBlockNumber; i += 1) { + for (let i = firstSubmissionBlockNumber; i <= endBlockNumber; i += 1) { const block = await getBlock(i); // Check every transaction... for (let txCount = 0; txCount < block.transactions.length; txCount += 1) { @@ -412,8 +412,8 @@ process.env.SOLIDITY_COVERAGE repCycleEthers = await reputationMinerClient._miner.getActiveRepCycle(); receive12Submissions = getWaitForNSubmissionsPromise(repCycleEthers, null, null, null, 12); - startingBlock = await currentBlock(); - startingBlockNumber = startingBlock.number; + latestBlock = await currentBlock(); + firstSubmissionBlockNumber = latestBlock.number; // Forward time and wait for the clients to submit all 12 allowed entries await forwardTime(MINING_CYCLE_DURATION / 2, this); @@ -424,7 +424,7 @@ process.env.SOLIDITY_COVERAGE const submissionAddresses = []; // For every block... - for (let i = startingBlockNumber; i <= endBlockNumber; i += 1) { + for (let i = firstSubmissionBlockNumber; i <= endBlockNumber; i += 1) { const block = await getBlock(i); // Check every transaction... for (let txCount = 0; txCount < block.transactions.length; txCount += 1) { From 2bca53d6b4c1496b78f9912aad4f0075a073b33e Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Tue, 8 Nov 2022 15:56:09 +0000 Subject: [PATCH 07/12] Overlooked migration from ethers v4->v5 --- packages/reputation-miner/ReputationMiner.js | 52 +++++++++++--------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/packages/reputation-miner/ReputationMiner.js b/packages/reputation-miner/ReputationMiner.js index 147926e1c4..ad8c57ae07 100644 --- a/packages/reputation-miner/ReputationMiner.js +++ b/packages/reputation-miner/ReputationMiner.js @@ -753,11 +753,12 @@ class ReputationMiner { if (!entryIndex) { entryIndex = await this.getEntryIndex(); // eslint-disable-line no-param-reassign } - let gasEstimate = ethers.BigNumber.from(1000000); + let gasEstimate; try { - gasEstimate = await repCycle.estimate.submitRootHash(hash, nLeaves, jrh, entryIndex); - } catch (err) { // eslint-disable-line no-empty - + gasEstimate = await repCycle.estimateGas.submitRootHash(hash, nLeaves, jrh, entryIndex); + gasEstimate = gasEstimate.mul(11).div(10); + } catch (err) { + gasEstimate = ethers.BigNumber.from(1000000); } // Submit that entry @@ -1001,11 +1002,12 @@ class ReputationMiner { const [, siblings2] = await this.justificationTree.getProof(ReputationMiner.getHexString(totalnUpdates, 64)); const [round, index] = await this.getMySubmissionRoundAndIndex(); - let gasEstimate = ethers.BigNumber.from(6000000); + let gasEstimate; try { - gasEstimate = await repCycle.estimate.confirmJustificationRootHash(round, index, siblings1, siblings2); - } catch (err) { // eslint-disable-line no-empty - + gasEstimate = await repCycle.estimateGas.confirmJustificationRootHash(round, index, siblings1, siblings2); + gasEstimate = gasEstimate.mul(11).div(10) + } catch (err) { + gasEstimate = ethers.BigNumber.from(6000000); } return repCycle.confirmJustificationRootHash( @@ -1083,16 +1085,17 @@ class ReputationMiner { ); } - let gasEstimate = ethers.BigNumber.from(1000000); + let gasEstimate; try { - gasEstimate = await repCycle.estimate.respondToBinarySearchForChallenge( + gasEstimate = await repCycle.estimateGas.respondToBinarySearchForChallenge( round, index, intermediateReputationHash, siblings ); - } catch (err) { // eslint-disable-line no-empty - + gasEstimate = gasEstimate.mul(11).div(10); + } catch (err) { + gasEstimate = ethers.BigNumber.from(1000000); } return repCycle.respondToBinarySearchForChallenge( round, @@ -1121,12 +1124,13 @@ class ReputationMiner { const intermediateReputationHash = this.justificationHashes[targetLeafKeyAsHex].jhLeafValue; const [, siblings] = await this.justificationTree.getProof(targetLeafKeyAsHex); - let gasEstimate = ethers.BigNumber.from(1000000); + let gasEstimate try { - gasEstimate = await repCycle.estimate.confirmBinarySearchResult(round, index, intermediateReputationHash, siblings); - } catch (err){ // eslint-disable-line no-empty - + gasEstimate = await repCycle.estimateGas.confirmBinarySearchResult(round, index, intermediateReputationHash, siblings); + gasEstimate = gasEstimate.mul(11).div(10); + } catch (err){ + gasEstimate = ethers.BigNumber.from(1000000); } return repCycle.confirmBinarySearchResult(round, index, intermediateReputationHash, siblings, { @@ -1225,11 +1229,12 @@ class ReputationMiner { lastAgreeJustifications.childReputationProof.siblings, lastAgreeJustifications.adjacentReputationProof.siblings] - let gasEstimate = ethers.BigNumber.from(4000000); + let gasEstimate; try { - gasEstimate = await repCycle.estimate.respondToChallenge(...functionArgs); - } catch (err){ // eslint-disable-line no-empty - + gasEstimate = await repCycle.estimateGas.respondToChallenge(...functionArgs); + gasEstimate = gasEstimate.mul(11).div(10); + } catch (err){ + gasEstimate = ethers.BigNumber.from(4000000); } return repCycle.respondToChallenge(...functionArgs, @@ -1245,11 +1250,12 @@ class ReputationMiner { const repCycle = await this.getActiveRepCycle(); const [round] = await this.getMySubmissionRoundAndIndex(); - let gasEstimate = ethers.BigNumber.from(4000000); + let gasEstimate; try { gasEstimate = await repCycle.estimateGas.confirmNewHash(round); - } catch (err){ // eslint-disable-line no-empty - + gasEstimate = gasEstimate.mul(11).div(10); + } catch (err){ + gasEstimate = ethers.BigNumber.from(4000000); } return repCycle.confirmNewHash(round, { gasLimit: gasEstimate, gasPrice: this.gasPrice }); } From be1fad93523addb866df36191de72e8dca574f98 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Fri, 11 Nov 2022 13:46:32 +0000 Subject: [PATCH 08/12] Missed updates to Dockerfiles --- packages/metatransaction-broadcaster/Dockerfile | 2 ++ packages/reputation-miner/Dockerfile | 3 +++ 2 files changed, 5 insertions(+) diff --git a/packages/metatransaction-broadcaster/Dockerfile b/packages/metatransaction-broadcaster/Dockerfile index b00e1b679a..5b767f1a46 100644 --- a/packages/metatransaction-broadcaster/Dockerfile +++ b/packages/metatransaction-broadcaster/Dockerfile @@ -4,5 +4,7 @@ COPY ./package.json ./ COPY ./package-lock.json ./ COPY ./build ./build RUN npm ci +RUN cd ./packages/metatransaction-broadcaster/ && npm i +RUN cd ./packages/package-utils/ && npm i EXPOSE 3000 CMD node $NODE_ARGS packages/metatransaction-broadcaster/bin/index.js --colonyNetworkAddress $COLONYNETWORK_ADDRESS --privateKey $PRIVATE_KEY --gasLimit $GASLIMIT $ARGS diff --git a/packages/reputation-miner/Dockerfile b/packages/reputation-miner/Dockerfile index 2203791963..1ab9630f07 100644 --- a/packages/reputation-miner/Dockerfile +++ b/packages/reputation-miner/Dockerfile @@ -1,8 +1,11 @@ FROM node:14-bullseye +RUN apt-get update || : && apt-get install python -y COPY ./packages ./packages COPY ./package.json ./ COPY ./package-lock.json ./ COPY ./build ./build RUN npm install +RUN cd ./packages/reputation-miner/ && npm i +RUN cd ./packages/package-utils/ && npm i EXPOSE 3000 CMD node $NODE_ARGS packages/reputation-miner/bin/index.js --dbPath $REPUTATION_JSON_PATH --colonyNetworkAddress $COLONYNETWORK_ADDRESS --privateKey $PRIVATE_KEY --syncFrom $SYNC_FROM_BLOCK $ARGS From daaafdc204edcfb448f11fa5028d66aef1b5b1a2 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Fri, 18 Nov 2022 10:51:49 +0000 Subject: [PATCH 09/12] Changes after review --- helpers/test-helper.js | 6 +++--- packages/reputation-miner/ReputationMinerClient.js | 6 +++--- .../reputation-mining-client/client-auto-functionality.js | 1 - 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/helpers/test-helper.js b/helpers/test-helper.js index 953af7183c..ae41ac4a41 100644 --- a/helpers/test-helper.js +++ b/helpers/test-helper.js @@ -297,9 +297,9 @@ exports.currentBlock = async function currentBlock() { return p; }; -exports.getBlock = async function getBlock(blockTag) { +exports.getBlock = async function getBlock(blockNumber) { const p = new Promise((resolve, reject) => { - web3.eth.getBlock(blockTag, (err, res) => { + web3.eth.getBlock(blockNumber, (err, res) => { if (err) { return reject(err); } @@ -1048,7 +1048,7 @@ exports.getMiningCycleCompletePromise = async function getMiningCycleCompletePro expect(newHash).to.not.equal(oldHash, "The old and new hashes are the same"); } if (expectedHash) { - expect(newHash).to.equal(expectedHash, "The network root hash doens't match the one submitted"); + expect(newHash).to.equal(expectedHash, "The network root hash doesn't match the one submitted"); } event.removeListener(); resolve(); diff --git a/packages/reputation-miner/ReputationMinerClient.js b/packages/reputation-miner/ReputationMinerClient.js index 9a7e98486b..41b14b8465 100644 --- a/packages/reputation-miner/ReputationMinerClient.js +++ b/packages/reputation-miner/ReputationMinerClient.js @@ -24,9 +24,9 @@ const cache = apicache.middleware const racingFunctionSignatures = [ "submitRootHash(bytes32,uint256,bytes32,uint256)", "confirmNewHash(uint256)", - "invalidateHash(uint256, uint256)", - "respondToBinarySearchForChallenge(uint256,uint256 ,bytes,bytes32[])", - "confirmBinarySearchResult(uint256,uint256,bytes,bytes32[] )", + "invalidateHash(uint256,uint256)", + "respondToBinarySearchForChallenge(uint256,uint256,bytes,bytes32[])", + "confirmBinarySearchResult(uint256,uint256,bytes,bytes32[])", "respondToChallenge(uint256[26],bytes32[7],bytes32[],bytes32[],bytes32[],bytes32[],bytes32[],bytes32[])", "confirmJustificationRootHash(uint256,uint256,bytes32[],bytes32[])" ].map(x => ethers.utils.id(x).slice(0,10)) diff --git a/test/reputation-system/reputation-mining-client/client-auto-functionality.js b/test/reputation-system/reputation-mining-client/client-auto-functionality.js index 4de7aff5da..d4196625f2 100644 --- a/test/reputation-system/reputation-mining-client/client-auto-functionality.js +++ b/test/reputation-system/reputation-mining-client/client-auto-functionality.js @@ -344,7 +344,6 @@ process.env.SOLIDITY_COVERAGE await reputationMinerClient2.initialise(colonyNetwork.address, startingBlockNumber); let lostRace = false; - // while (!lostRace) { while (reputationMinerClient.lockedForBlockProcessing || reputationMinerClient2.lockedForBlockProcessing) { await sleep(1000); } From 486f0a31b9cbcae829dd7486c25334338813c5c7 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Wed, 23 Nov 2022 11:03:14 +0000 Subject: [PATCH 10/12] Add comments about extra gas --- packages/reputation-miner/ReputationMiner.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/reputation-miner/ReputationMiner.js b/packages/reputation-miner/ReputationMiner.js index ad8c57ae07..ec76812b68 100644 --- a/packages/reputation-miner/ReputationMiner.js +++ b/packages/reputation-miner/ReputationMiner.js @@ -756,6 +756,7 @@ class ReputationMiner { let gasEstimate; try { gasEstimate = await repCycle.estimateGas.submitRootHash(hash, nLeaves, jrh, entryIndex); + // Add some extra gas just in case the details change and a little more is needed gasEstimate = gasEstimate.mul(11).div(10); } catch (err) { gasEstimate = ethers.BigNumber.from(1000000); @@ -1005,6 +1006,7 @@ class ReputationMiner { let gasEstimate; try { gasEstimate = await repCycle.estimateGas.confirmJustificationRootHash(round, index, siblings1, siblings2); + // Add some extra gas just in case the details change and a little more is needed gasEstimate = gasEstimate.mul(11).div(10) } catch (err) { gasEstimate = ethers.BigNumber.from(6000000); @@ -1093,6 +1095,7 @@ class ReputationMiner { intermediateReputationHash, siblings ); + // Add some extra gas just in case the details change and a little more is needed gasEstimate = gasEstimate.mul(11).div(10); } catch (err) { gasEstimate = ethers.BigNumber.from(1000000); @@ -1128,6 +1131,7 @@ class ReputationMiner { try { gasEstimate = await repCycle.estimateGas.confirmBinarySearchResult(round, index, intermediateReputationHash, siblings); + // Add some extra gas just in case the details change and a little more is needed gasEstimate = gasEstimate.mul(11).div(10); } catch (err){ gasEstimate = ethers.BigNumber.from(1000000); @@ -1232,6 +1236,7 @@ class ReputationMiner { let gasEstimate; try { gasEstimate = await repCycle.estimateGas.respondToChallenge(...functionArgs); + // Add some extra gas just in case the details change and a little more is needed gasEstimate = gasEstimate.mul(11).div(10); } catch (err){ gasEstimate = ethers.BigNumber.from(4000000); @@ -1253,6 +1258,7 @@ class ReputationMiner { let gasEstimate; try { gasEstimate = await repCycle.estimateGas.confirmNewHash(round); + // Add some extra gas just in case the details change and a little more is needed gasEstimate = gasEstimate.mul(11).div(10); } catch (err){ gasEstimate = ethers.BigNumber.from(4000000); From fecec7ead3b116d5d8d7ea8f2743cd90c133ae4b Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Wed, 30 Nov 2022 17:22:37 +0000 Subject: [PATCH 11/12] Make function sig in test more explicit --- .../reputation-mining-client/client-auto-functionality.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/reputation-system/reputation-mining-client/client-auto-functionality.js b/test/reputation-system/reputation-mining-client/client-auto-functionality.js index d4196625f2..b16200d698 100644 --- a/test/reputation-system/reputation-mining-client/client-auto-functionality.js +++ b/test/reputation-system/reputation-mining-client/client-auto-functionality.js @@ -332,6 +332,8 @@ process.env.SOLIDITY_COVERAGE it("Losing a race shouldn't prevent a miner from continuing", async function () { reputationMinerClient._processingDelay = 1; + const SUBMISSION_SIG = web3.utils.soliditySha3("submitRootHash(bytes32,uint256,bytes32,uint256)").slice(0, 10); + const reputationMinerClient2 = new ReputationMinerClient({ loader, realProviderPort, @@ -397,7 +399,7 @@ process.env.SOLIDITY_COVERAGE if (!txReceipt.status) { // Was it actually a race? const tx = await web3GetTransaction(txHash); - if (tx.input.slice(0, 10) === "0x3fcbcf0d") { + if (tx.input.slice(0, 10) === SUBMISSION_SIG) { lostRace = true; } } @@ -429,7 +431,7 @@ process.env.SOLIDITY_COVERAGE for (let txCount = 0; txCount < block.transactions.length; txCount += 1) { const txHash = block.transactions[txCount]; const tx = await web3GetTransaction(txHash); - if (tx.input.slice(0, 10) === "0x3fcbcf0d") { + if (tx.input.slice(0, 10) === SUBMISSION_SIG) { submissionAddresses.push(tx.from); } } From 8bcf179c2f51f4f555b629420622751f38328b15 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Wed, 30 Nov 2022 22:01:47 +0000 Subject: [PATCH 12/12] Second attempt at fix for issue --- .../IReputationMiningCycle.sol | 7 ------ .../ReputationMiningCycle.sol | 14 ++++------- .../ReputationMiningCycleBinarySearch.sol | 4 ++-- .../ReputationMiningCycleCommon.sol | 4 ++-- .../ReputationMiningCycleRespond.sol | 2 +- docs/interfaces/ireputationminingcycle.md | 18 --------------- .../reputation-miner/ReputationMinerClient.js | 23 ++++++------------- test-smoke/colony-storage-consistent.js | 10 ++++---- 8 files changed, 22 insertions(+), 60 deletions(-) diff --git a/contracts/reputationMiningCycle/IReputationMiningCycle.sol b/contracts/reputationMiningCycle/IReputationMiningCycle.sol index 047a772333..1153377f72 100644 --- a/contracts/reputationMiningCycle/IReputationMiningCycle.sol +++ b/contracts/reputationMiningCycle/IReputationMiningCycle.sol @@ -271,12 +271,5 @@ interface IReputationMiningCycle is ReputationMiningCycleDataTypes { /// enum in ReputationMiningCycleDataTypes /// @param _since The timestamp the last response for the submission in the dispute in question was made at. /// @return possible bool Whether the user can respond at the current time. - /// @dev Deprecated function getResponsePossible(DisputeStages _stage, uint256 _since) external view returns (bool possible); - - /// @notice Returns whether the caller is able to currently respond to a dispute stage. - /// @param _since The timestamp the last response for the submission in the dispute in question was made at. - /// @return possible bool Whether the user can respond at the current time. - function getResponsePossible(uint256 _since) external view returns (bool possible); - } diff --git a/contracts/reputationMiningCycle/ReputationMiningCycle.sol b/contracts/reputationMiningCycle/ReputationMiningCycle.sol index 87cf092b1e..1a6741c1b5 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycle.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycle.sol @@ -216,7 +216,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { require(submissionWindowClosed(), "colony-reputation-mining-submission-window-still-open"); require( - responsePossible(disputeRounds[_roundNumber][0].lastResponseTimestamp), + responsePossible(DisputeStages.ConfirmNewHash, disputeRounds[_roundNumber][0].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -256,7 +256,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { // Is the person making this call eligible to? require( - responsePossible(disputeRounds[_round][opponentIdx].lastResponseTimestamp), + responsePossible(DisputeStages.InvalidateHash, disputeRounds[_round][opponentIdx].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -292,7 +292,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { // The submission can be invalidated - now check the person invalidating is allowed to require( - responsePossible(add(disputeRounds[_round][_idx].lastResponseTimestamp, CHALLENGE_RESPONSE_WINDOW_DURATION)), + responsePossible(DisputeStages.InvalidateHash, add(disputeRounds[_round][_idx].lastResponseTimestamp, CHALLENGE_RESPONSE_WINDOW_DURATION)), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -350,7 +350,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { require(submissionWindowClosed(), "colony-reputation-mining-cycle-submissions-not-closed"); require(_index < disputeRounds[_round].length, "colony-reputation-mining-index-beyond-round-length"); require( - responsePossible(disputeRounds[_round][_index].lastResponseTimestamp), + responsePossible(DisputeStages.ConfirmNewHash, disputeRounds[_round][_index].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -500,11 +500,7 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { } function getResponsePossible(DisputeStages _stage, uint256 _since) external view returns (bool) { - return responsePossible(_since); - } - - function getResponsePossible(uint256 _since) external view returns (bool) { - return responsePossible(_since); + return responsePossible(_stage, _since); } ///////////////////////// diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol b/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol index eac32713e4..50b52c078a 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleBinarySearch.sol @@ -37,7 +37,7 @@ contract ReputationMiningCycleBinarySearch is ReputationMiningCycleCommon { require(_idx < disputeRounds[_round].length, "colony-reputation-mining-index-beyond-round-length"); require(disputeRounds[_round][_idx].lowerBound != disputeRounds[_round][_idx].upperBound, "colony-reputation-mining-challenge-not-active"); require( - responsePossible(disputeRounds[_round][_idx].lastResponseTimestamp), + responsePossible(DisputeStages.BinarySearchResponse, disputeRounds[_round][_idx].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); @@ -87,7 +87,7 @@ contract ReputationMiningCycleBinarySearch is ReputationMiningCycleCommon { "colony-reputation-binary-search-result-already-confirmed" ); require( - responsePossible(disputeRounds[_round][_idx].lastResponseTimestamp), + responsePossible(DisputeStages.BinarySearchConfirm, disputeRounds[_round][_idx].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol b/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol index 9b1bdf07f3..ab324f33fb 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol @@ -146,7 +146,7 @@ contract ReputationMiningCycleCommon is ReputationMiningCycleStorage, PatriciaTr uint256 constant CHALLENGE_RESPONSE_WINDOW_DURATION = 60 * 20; uint256 constant Y = UINT256_MAX / (CHALLENGE_RESPONSE_WINDOW_DURATION - ALL_ENTRIES_ALLOWED_END_OF_WINDOW); - function responsePossible(uint256 _responseWindowOpened) internal view returns (bool) { + function responsePossible(DisputeStages _stage, uint256 _responseWindowOpened) internal view returns (bool) { if (_responseWindowOpened > block.timestamp) { // I don't think this is currently possible, but belt and braces! return false; @@ -162,7 +162,7 @@ contract ReputationMiningCycleCommon is ReputationMiningCycleStorage, PatriciaTr return false; } uint256 target = windowOpenFor * Y; - if (uint256(keccak256(abi.encodePacked(minerAddress, _responseWindowOpened))) > target) { + if (uint256(keccak256(abi.encodePacked(minerAddress, address(this), _stage))) > target) { return false; } } diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol b/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol index a4a43f910d..e83e7efd49 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleRespond.sol @@ -112,7 +112,7 @@ contract ReputationMiningCycleRespond is ReputationMiningCycleCommon { challengeOpen(_u[U_ROUND], _u[U_IDX]) { require( - responsePossible(disputeRounds[_u[U_ROUND]][_u[U_IDX]].lastResponseTimestamp), + responsePossible(DisputeStages.RespondToChallenge, disputeRounds[_u[U_ROUND]][_u[U_IDX]].lastResponseTimestamp), "colony-reputation-mining-user-ineligible-to-respond" ); diff --git a/docs/interfaces/ireputationminingcycle.md b/docs/interfaces/ireputationminingcycle.md index f3f92417b2..4133892364 100644 --- a/docs/interfaces/ireputationminingcycle.md +++ b/docs/interfaces/ireputationminingcycle.md @@ -277,7 +277,6 @@ Get the length of the ReputationUpdateLog stored on this instance of the Reputat Returns whether the caller is able to currently respond to a dispute stage. -*Note: Deprecated* **Parameters** @@ -292,23 +291,6 @@ Returns whether the caller is able to currently respond to a dispute stage. |---|---|---| |possible|bool|bool Whether the user can respond at the current time. -### ▸ `getResponsePossible(uint256 _since):bool possible` - -Returns whether the caller is able to currently respond to a dispute stage. - - -**Parameters** - -|Name|Type|Description| -|---|---|---| -|_since|uint256|The timestamp the last response for the submission in the dispute in question was made at. - -**Return Parameters** - -|Name|Type|Description| -|---|---|---| -|possible|bool|bool Whether the user can respond at the current time. - ### ▸ `getSubmissionUser(bytes32 _hash, uint256 _nLeaves, bytes32 _jrh, uint256 _index):address user` Get the address that made a particular submission. diff --git a/packages/reputation-miner/ReputationMinerClient.js b/packages/reputation-miner/ReputationMinerClient.js index 41b14b8465..c8c44a9063 100644 --- a/packages/reputation-miner/ReputationMinerClient.js +++ b/packages/reputation-miner/ReputationMinerClient.js @@ -474,7 +474,7 @@ class ReputationMinerClient { const oppSubmission = await repCycle.getReputationHashSubmission(oppEntry.firstSubmitter); if (oppSubmission.proposedNewRootHash === ethers.constants.AddressZero){ - const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"](disputeStages.INVALIDATE_HASH, entry.lastResponseTimestamp); + const responsePossible = await repCycle.getResponsePossible(disputeStages.INVALIDATE_HASH, entry.lastResponseTimestamp); if (!responsePossible) { this.endDoBlockChecks(); return; @@ -507,7 +507,7 @@ class ReputationMinerClient { // Before checking if our opponent has timed out yet, check if we can respond to something // 1. Do we still need to confirm JRH? if (submission.jrhNLeaves.eq(0)) { - const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"](disputeStages.CONFIRM_JRH, entry.lastResponseTimestamp); + const responsePossible = await repCycle.getResponsePossible(disputeStages.CONFIRM_JRH, entry.lastResponseTimestamp); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -522,10 +522,7 @@ class ReputationMinerClient { // We can respond if neither of us have responded to this stage yet or // if they have responded already if (oppEntry.challengeStepCompleted.gte(entry.challengeStepCompleted)) { - const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( - disputeStages.BINARY_SEARCH_RESPONSE, - entry.lastResponseTimestamp - ); + const responsePossible = await repCycle.getResponsePossible(disputeStages.BINARY_SEARCH_RESPONSE, entry.lastResponseTimestamp); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -543,10 +540,7 @@ class ReputationMinerClient { ethers.BigNumber.from(2).pow(entry.challengeStepCompleted.sub(2)).lte(submission.jrhNLeaves) ) { - const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( - disputeStages.BINARY_SEARCH_CONFIRM, - entry.lastResponseTimestamp - ); + const responsePossible = await repCycle.getResponsePossible(disputeStages.BINARY_SEARCH_CONFIRM, entry.lastResponseTimestamp); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -564,10 +558,7 @@ class ReputationMinerClient { ethers.BigNumber.from(2).pow(entry.challengeStepCompleted.sub(3)).lte(submission.jrhNLeaves) ) { - const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( - disputeStages.RESPOND_TO_CHALLENGE, - entry.lastResponseTimestamp - ); + const responsePossible = await repCycle.getResponsePossible(disputeStages.RESPOND_TO_CHALLENGE, entry.lastResponseTimestamp); if (responsePossible){ const gasPrice = await updateGasEstimate("fast", this.chainId, this._adapter); await this._miner.setGasPrice(gasPrice); @@ -581,7 +572,7 @@ class ReputationMinerClient { const opponentTimeout = ethers.BigNumber.from(block.timestamp).sub(oppEntry.lastResponseTimestamp).gte(CHALLENGE_RESPONSE_WINDOW_DURATION); if (opponentTimeout){ - const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"]( + const responsePossible = await repCycle.getResponsePossible( disputeStages.INVALIDATE_HASH, ethers.BigNumber.from(oppEntry.lastResponseTimestamp).add(CHALLENGE_RESPONSE_WINDOW_DURATION) ); @@ -604,7 +595,7 @@ class ReputationMinerClient { const disputeRound = await repCycle.getDisputeRound(round); const entry = disputeRound[index]; - const responsePossible = await repCycle["getResponsePossible(uint8,uint256)"](disputeStages.CONFIRM_NEW_HASH, entry.lastResponseTimestamp); + const responsePossible = await repCycle.getResponsePossible(disputeStages.CONFIRM_NEW_HASH, entry.lastResponseTimestamp); if (responsePossible){ await this.confirmEntry(); } diff --git a/test-smoke/colony-storage-consistent.js b/test-smoke/colony-storage-consistent.js index b7f1ce8276..c72d16b4d9 100644 --- a/test-smoke/colony-storage-consistent.js +++ b/test-smoke/colony-storage-consistent.js @@ -149,11 +149,11 @@ contract("Contract Storage", (accounts) => { console.log("miningCycleStateHash:", miningCycleStateHash); console.log("tokenLockingStateHash:", tokenLockingStateHash); - expect(colonyNetworkStateHash).to.equal("0x77e04702d554bbcff2c36be7ccce767adce92a13c135a9cd279dd3f7415b093b"); - expect(colonyStateHash).to.equal("0x4e90fcbe58b79118b2a2c09dd96c2cefe46f732b18b1d6230a361c0332133dec"); - expect(metaColonyStateHash).to.equal("0x6be6cb630afd143ac7db391fb52150d3b817443b59206497e36aa4ffbeca5c1a"); - expect(miningCycleStateHash).to.equal("0x20b0a911563ceab3018f750b12c1dda75608436d3f67abb061a1201434931028"); - expect(tokenLockingStateHash).to.equal("0xc9fa6f26cac13030e857f1c4aeb6057d65a0d196a11e69c152483aa382270a2a"); + expect(colonyNetworkStateHash).to.equal("0xa9289d3025a1f5e108b7b68f335327c1c5748015db91c78978679ba9832984e1"); + expect(colonyStateHash).to.equal("0x54a0edcb2097270bd95d610dc827869cc827241d131461f58788f7c3257ca151"); + expect(metaColonyStateHash).to.equal("0x15fab25907cfb6baedeaf1fdabd68678d37584a1817a08dfe77db60db378a508"); + expect(miningCycleStateHash).to.equal("0x632d459a2197708bd2dbde87e8275c47dddcdf16d59e3efd21dcef9acb2a7366"); + expect(tokenLockingStateHash).to.equal("0x30fbcbfbe589329fe20288101faabe1f60a4610ae0c0effb15526c6b390a8e07"); }); }); });