From e3cc09eba506458e837dccbf482338154ad496bc Mon Sep 17 00:00:00 2001 From: Laurent Senta Date: Tue, 6 Mar 2018 16:10:46 +0100 Subject: [PATCH 1/4] fix #173: Add reputation stacking in gas cost test --- gasCosts/gasCosts.js | 51 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/gasCosts/gasCosts.js b/gasCosts/gasCosts.js index f5e17f1362..ef0309d9ab 100644 --- a/gasCosts/gasCosts.js +++ b/gasCosts/gasCosts.js @@ -17,8 +17,11 @@ import { DELIVERABLE_HASH, SECONDS_PER_DAY } from "../helpers/constants"; -import { getTokenArgs, currentBlockTime, createSignatures } from "../helpers/test-helper"; +import { getTokenArgs, currentBlockTime, createSignatures, forwardTime } from "../helpers/test-helper"; import { setupColonyVersionResolver } from "../helpers/upgradable-contracts"; +import { giveUserCLNYTokens } from "../helpers/test-data-generator"; + +const BN = require("bn.js"); const Colony = artifacts.require("Colony"); const Token = artifacts.require("Token"); @@ -29,6 +32,7 @@ const ColonyFunding = artifacts.require("ColonyFunding"); const Resolver = artifacts.require("Resolver"); const EtherRouter = artifacts.require("EtherRouter"); const Authority = artifacts.require("Authority"); +const ReputationMiningCycle = artifacts.require("ReputationMiningCycle"); contract("All", () => { const gasPrice = 20e9; @@ -138,5 +142,50 @@ contract("All", () => { // finalizeTask await colony.finalizeTask(1); }); + + it("when working with stacking", async () => { + // TODO: Should Stackers be part of the constants? + const STACKER1 = EVALUATOR; + const STACKER2 = WORKER; + + // Load the token + const clnyAddress = await commonColony.getToken.call(); + const clny = Token.at(clnyAddress); + + // Setup the stackers balance + const bigStr = "1000000000000000000"; + const lessBigStr = "10000000000000000"; + const big = new BN(bigStr); + + await giveUserCLNYTokens(colonyNetwork, STACKER1, big); + await clny.approve(colonyNetwork.address, bigStr, { from: STACKER1 }); + + await giveUserCLNYTokens(colonyNetwork, STACKER2, big); + await clny.approve(colonyNetwork.address, bigStr, { from: STACKER2 }); + + // stack + await colonyNetwork.deposit(lessBigStr, { from: STACKER1 }); + await colonyNetwork.deposit(lessBigStr, { from: STACKER2 }); + + // Start Reputation + await colonyNetwork.startNextCycle(); + const repCycleAddr = await colonyNetwork.getReputationMiningCycle.call(); + + await forwardTime(3600, this); + + const repCycle = ReputationMiningCycle.at(repCycleAddr); + + // Submit Hash + await repCycle.submitNewHash("0x87654321", 1, 1, { from: STACKER1, gas: 600000 }); + await repCycle.submitNewHash("0x87654321", 1, 10, { from: STACKER2, gas: 600000 }); + + await forwardTime(3600, this); + + await repCycle.confirmNewHash(0); + + // withdraw + await colonyNetwork.withdraw(lessBigStr, { from: STACKER1 }); + await colonyNetwork.withdraw(lessBigStr, { from: STACKER2 }); + }); }); }); From 598114e501de3ee66740c1e854883071fad4ed91 Mon Sep 17 00:00:00 2001 From: Laurent Senta Date: Wed, 14 Mar 2018 10:28:33 +0100 Subject: [PATCH 2/4] gas cost test: add the hash submission process --- gasCosts/gasCosts.js | 58 ++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/gasCosts/gasCosts.js b/gasCosts/gasCosts.js index ef0309d9ab..6b03456fff 100644 --- a/gasCosts/gasCosts.js +++ b/gasCosts/gasCosts.js @@ -34,6 +34,8 @@ const EtherRouter = artifacts.require("EtherRouter"); const Authority = artifacts.require("Authority"); const ReputationMiningCycle = artifacts.require("ReputationMiningCycle"); +const oneHourLater = async () => forwardTime(3600, this); + contract("All", () => { const gasPrice = 20e9; @@ -143,49 +145,63 @@ contract("All", () => { await colony.finalizeTask(1); }); - it("when working with stacking", async () => { - // TODO: Should Stackers be part of the constants? - const STACKER1 = EVALUATOR; - const STACKER2 = WORKER; + it("when working with staking", async () => { + // TODO: Should stakers be part of the constants? + const STAKER1 = EVALUATOR; + const STAKER2 = WORKER; + const STAKER3 = MANAGER; // Load the token const clnyAddress = await commonColony.getToken.call(); const clny = Token.at(clnyAddress); - // Setup the stackers balance + // Setup the stakers balance const bigStr = "1000000000000000000"; const lessBigStr = "10000000000000000"; const big = new BN(bigStr); - await giveUserCLNYTokens(colonyNetwork, STACKER1, big); - await clny.approve(colonyNetwork.address, bigStr, { from: STACKER1 }); + await giveUserCLNYTokens(colonyNetwork, STAKER1, big); + await clny.approve(colonyNetwork.address, bigStr, { from: STAKER1 }); + + await giveUserCLNYTokens(colonyNetwork, STAKER2, big); + await clny.approve(colonyNetwork.address, bigStr, { from: STAKER2 }); - await giveUserCLNYTokens(colonyNetwork, STACKER2, big); - await clny.approve(colonyNetwork.address, bigStr, { from: STACKER2 }); + await giveUserCLNYTokens(colonyNetwork, STAKER3, big); + await clny.approve(colonyNetwork.address, bigStr, { from: STAKER3 }); - // stack - await colonyNetwork.deposit(lessBigStr, { from: STACKER1 }); - await colonyNetwork.deposit(lessBigStr, { from: STACKER2 }); + // stake + await colonyNetwork.deposit(lessBigStr, { from: STAKER1 }); + await colonyNetwork.deposit(lessBigStr, { from: STAKER2 }); + await colonyNetwork.deposit(lessBigStr, { from: STAKER3 }); // Start Reputation await colonyNetwork.startNextCycle(); const repCycleAddr = await colonyNetwork.getReputationMiningCycle.call(); - await forwardTime(3600, this); + await oneHourLater(); const repCycle = ReputationMiningCycle.at(repCycleAddr); // Submit Hash - await repCycle.submitNewHash("0x87654321", 1, 1, { from: STACKER1, gas: 600000 }); - await repCycle.submitNewHash("0x87654321", 1, 10, { from: STACKER2, gas: 600000 }); - - await forwardTime(3600, this); - - await repCycle.confirmNewHash(0); + await repCycle.submitNewHash("0x87654321", 1, 1, { from: STAKER1, gas: 600000 }); + await repCycle.submitNewHash("0x87654322", 2, 10, { from: STAKER2, gas: 600000 }); + await repCycle.submitNewHash("0x87654323", 3, 10, { from: STAKER3, gas: 600000 }); + + // Session of respond / invalidate between our 3 submissions + await repCycle.respondToChallenge(0, 0); + await oneHourLater(); + await repCycle.invalidateHash(0, 1); + await oneHourLater(); + await repCycle.invalidateHash(0, 3); // Invalidate the 'null' competing against submission idx=2 + await oneHourLater(); + await repCycle.respondToChallenge(1, 1); + await oneHourLater(); + await repCycle.invalidateHash(1, 0); + await oneHourLater(); + await repCycle.confirmNewHash(2); // withdraw - await colonyNetwork.withdraw(lessBigStr, { from: STACKER1 }); - await colonyNetwork.withdraw(lessBigStr, { from: STACKER2 }); + await colonyNetwork.withdraw(lessBigStr, { from: STAKER3 }); }); }); }); From ffcaa285b53774a095bf3ecb788fae6744aadec5 Mon Sep 17 00:00:00 2001 From: Laurent Senta Date: Mon, 19 Mar 2018 16:27:05 +0100 Subject: [PATCH 3/4] Workaround gas reporting issue for staking Before we wouldn't get gas costs for calls on the ReputationMiningCycle contract. Workaround is: - give it its own file - require it before the tests starts (in migrations/) related issue: https://github.com/cgewecke/eth-gas-reporter/issues/64 --- contracts/ColonyNetworkStaking.sol | 194 +------------------------- contracts/ReputationMiningCycle.sol | 204 ++++++++++++++++++++++++++++ migrations/2_deploy_contracts.js | 8 ++ scripts/check-storage.js | 3 +- 4 files changed, 216 insertions(+), 193 deletions(-) create mode 100644 contracts/ReputationMiningCycle.sol diff --git a/contracts/ColonyNetworkStaking.sol b/contracts/ColonyNetworkStaking.sol index 7a0f557028..0dbc36ef0b 100644 --- a/contracts/ColonyNetworkStaking.sol +++ b/contracts/ColonyNetworkStaking.sol @@ -9,6 +9,7 @@ import "./EtherRouter.sol"; import "./ERC20Extended.sol"; import "./ColonyNetworkStorage.sol"; import "./IColonyNetwork.sol"; +import "./ReputationMiningCycle.sol"; contract ColonyNetworkStaking is ColonyNetworkStorage, DSMath { @@ -122,195 +123,4 @@ contract ColonyNetworkStaking is ColonyNetworkStorage, DSMath { } } -// TODO: Can we handle a dispute regarding the very first hash that should be set? - - -contract ReputationMiningCycle { - address colonyNetworkAddress; - // TODO: Do we need both these mappings? - mapping (bytes32 => mapping( uint256 => address[])) public submittedHashes; - mapping (address => Submission) public reputationHashSubmissions; - uint256 reputationMiningWindowOpenTimestamp; - mapping (uint256 => Submission[]) public disputeRounds; - - // Tracks the number of submissions in each round that have completed their challenge, one way or the other. - // This might be that they passed the challenge, it might be that their opponent passed (and therefore by implication, - // they failed), or it might be that they timed out - mapping (uint256 => uint256) nHashesCompletedChallengeRound; - // A flaw with this is that if someone spams lots of nonsense transactions, then 'good' users still have to come along and - // explicitly complete the pairings. But if they get the tokens that were staked in order to make the submission, maybe - // that's okay...? - - uint256 public nSubmittedHashes = 0; - uint256 public nInvalidatedHashes = 0; - - struct Submission { - bytes32 hash; - uint256 nNodes; - uint256 lastResponseTimestamp; - uint256 challengeStepCompleted; - } - - // Records for which hashes, for which addresses, for which entries have been accepted - // Otherwise, people could keep submitting the same entry. - mapping (bytes32 => mapping(address => mapping(uint256 => bool))) submittedEntries; - - modifier onlyFinalRoundWhenComplete(uint roundNumber){ - require (nSubmittedHashes - nInvalidatedHashes == 1); - require (disputeRounds[roundNumber].length == 1); //i.e. this is the final round - // Note that even if we are passed the penultimate round, which had a length of two, and had one eliminated, - // and therefore 'delete' called in `invalidateHash`, the array still has a length of '2' - it's just that one - // element is zeroed. If this functionality of 'delete' is ever changed, this will have to change too. - _; - } - - function ReputationMiningCycle() public { - colonyNetworkAddress = msg.sender; - reputationMiningWindowOpenTimestamp = now; - } - - function respondToChallenge(uint256 round, uint256 idx) public { - // TODO: Check challenge response is valid, relating to current challenge - // TODO: Check challenge response relates to this hash - - // Assuming that the challenge response is correct... - uint256 opponentIdx = (idx % 2 == 1 ? idx-1 : idx + 1); - - disputeRounds[round][idx].lastResponseTimestamp = now; - disputeRounds[round][idx].challengeStepCompleted += 1; - // If our opponent responded to this challenge before we did, 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! - if (disputeRounds[round][idx].challengeStepCompleted == disputeRounds[round][opponentIdx].challengeStepCompleted) { - disputeRounds[round][opponentIdx].lastResponseTimestamp = now; - } - } - - function submitNewHash(bytes32 newHash, uint256 nNodes, uint256 entry) public { - //Check the ticket is an eligible one for them to claim - require(entry <= IColonyNetwork(colonyNetworkAddress).getStakedBalance(msg.sender) / 10**15); - require(entry > 0); - if (reputationHashSubmissions[msg.sender].hash != 0x0) { // If this user has submitted before during this round... - require(newHash == reputationHashSubmissions[msg.sender].hash); // ...require that they are submitting the same hash ... - require(nNodes == reputationHashSubmissions[msg.sender].nNodes); // ...require that they are submitting the same number of nodes for that hash ... - require (submittedEntries[newHash][msg.sender][entry] == false); // ... but not this exact entry - } - // TODO: Require minimum stake, that is (much) more than the cost required to defend the valid submission. - // Check the ticket is a winning one. - // TODO Figure out how to uncomment the next line, but not break tests sporadically. - // require((now-reputationMiningWindowOpenTimestamp) <= 3600); - // x = floor(uint((2**256 - 1) / 3600) - if (now-reputationMiningWindowOpenTimestamp <= 3600) { - uint256 x = 32164469232587832062103051391302196625908329073789045566515995557753647122; - uint256 target = (now - reputationMiningWindowOpenTimestamp ) * x; - require(uint256(keccak256(msg.sender, entry, newHash)) < target); - } - - // We only allow this submission if there's still room - // Check there is still room. - require (submittedHashes[newHash][nNodes].length < 12); - - // If this is a new hash, increment nSubmittedHashes as such. - if (submittedHashes[newHash][nNodes].length == 0) { - nSubmittedHashes += 1; - // And add it to the first disputeRound - // NB if no other hash is submitted, no dispute resolution will be required. - disputeRounds[0].push(Submission({hash: newHash, nNodes: nNodes, lastResponseTimestamp: 0, challengeStepCompleted: 0})); - // If we've got a pair of submissions to face off, may as well start now. - if (nSubmittedHashes % 2 == 0) { - disputeRounds[0][nSubmittedHashes-1].lastResponseTimestamp = now; - disputeRounds[0][nSubmittedHashes-2].lastResponseTimestamp = now; - } - } - - - reputationHashSubmissions[msg.sender] = Submission({hash: newHash, nNodes: nNodes, lastResponseTimestamp: 0, challengeStepCompleted: 0}); - //And add the miner to the array list of submissions here - submittedHashes[newHash][nNodes].push(msg.sender); - //Note that they submitted it. - submittedEntries[newHash][msg.sender][entry] = true; - } - - function confirmNewHash(uint256 roundNumber) public - onlyFinalRoundWhenComplete(roundNumber) - { - - // TODO: Require some amount of time to have passed (i.e. people have had a chance to submit other hashes) - Submission storage reputationRootHash = disputeRounds[roundNumber][0]; - IColonyNetwork(colonyNetworkAddress).setReputationRootHash(reputationRootHash.hash, reputationRootHash.nNodes, submittedHashes[disputeRounds[roundNumber][0].hash][disputeRounds[roundNumber][0].nNodes]); - selfdestruct(colonyNetworkAddress); - } - - function invalidateHash(uint256 round, uint256 idx) public { - // What we do depends on our opponent, so work out which index it was at in disputeRounds[round] - uint256 opponentIdx = (idx % 2 == 1 ? idx-1 : idx + 1); - uint256 nInNextRound; - - // We require either - // 1. That we actually had an opponent - can't invalidate the last hash. - // 2. This cycle had an odd number of submissions, which was larger than 1, and we're giving the last entry a bye to the next round. - if (disputeRounds[round].length % 2 == 1 && disputeRounds[round].length == idx) { - // This is option two above - note that because arrays are zero-indexed, if idx==length, then - // this is the slot after the last entry, and so our opponentIdx will be the last entry - // We just move the opponent on, and nothing else happens. - - // Ensure that the previous round is complete, and this entry wouldn't possibly get an opponent later on. - require(nHashesCompletedChallengeRound[round-1] == disputeRounds[round-1].length); - - // Prevent us invalidating the final hash - require(disputeRounds[round].length>1); - // Move opponent on to next round - disputeRounds[round+1].push(disputeRounds[round][opponentIdx]); - // Note the fact that this round has had another challenge complete - nHashesCompletedChallengeRound[round] += 1; - // TODO: DRY with the code below. - // Check if the hash we just moved to the next round is the second of a pairing that should now face off. - nInNextRound = disputeRounds[round+1].length; - - if (nInNextRound % 2 == 0) { - disputeRounds[round+1][nInNextRound-1].challengeStepCompleted = 0; - disputeRounds[round+1][nInNextRound-1].lastResponseTimestamp = now; - disputeRounds[round+1][nInNextRound-2].challengeStepCompleted = 0; - disputeRounds[round+1][nInNextRound-2].lastResponseTimestamp = now; - } - } else { - require(disputeRounds[round].length > opponentIdx); - require(disputeRounds[round][opponentIdx].hash!=""); - // Require that it has failed a challenge (i.e. failed to respond in time) - require(now - disputeRounds[round][idx].lastResponseTimestamp >= 600); //'In time' is ten minutes here. - - if (disputeRounds[round][opponentIdx].challengeStepCompleted > disputeRounds[round][idx].challengeStepCompleted) { - // If true, then the opponent completed one more challenge round than the submission being invalidated, so we - // don't know if they're valid or not yet. Move them on to the next round. - disputeRounds[round+1].push(disputeRounds[round][opponentIdx]); - delete disputeRounds[round][opponentIdx]; - // TODO Delete the hash(es) being invalidated? - nInvalidatedHashes += 1; - // Check if the hash we just moved to the next round is the second of a pairing that should now face off. - nInNextRound = disputeRounds[round+1].length; - if (nInNextRound % 2 == 0) { - disputeRounds[round+1][nInNextRound-1].challengeStepCompleted = 0; - disputeRounds[round+1][nInNextRound-1].lastResponseTimestamp = now; - disputeRounds[round+1][nInNextRound-2].challengeStepCompleted = 0; - disputeRounds[round+1][nInNextRound-2].lastResponseTimestamp = now; - } - } else { - // Our opponent completed the same number of challenge rounds, and both have now timed out. - nInvalidatedHashes += 2; - // Punish the people who proposed our opponent - IColonyNetwork(colonyNetworkAddress).punishStakers(submittedHashes[disputeRounds[round][opponentIdx].hash][disputeRounds[round][opponentIdx].nNodes]); - } - - // Note that two hashes have completed this challenge round (either one accepted for now and one rejected, or two rejected) - nHashesCompletedChallengeRound[round] += 2; - - // Punish the people who proposed the hash that was rejected - IColonyNetwork(colonyNetworkAddress).punishStakers(submittedHashes[disputeRounds[round][idx].hash][disputeRounds[round][idx].nNodes]); - - } - //TODO: Can we do some deleting to make calling this as cheap as possible for people? - } - - - -} +// TODO: Can we handle a dispute regarding the very first hash that should be set? \ No newline at end of file diff --git a/contracts/ReputationMiningCycle.sol b/contracts/ReputationMiningCycle.sol new file mode 100644 index 0000000000..e30934bdd5 --- /dev/null +++ b/contracts/ReputationMiningCycle.sol @@ -0,0 +1,204 @@ +pragma solidity ^0.4.17; +pragma experimental "v0.5.0"; +pragma experimental "ABIEncoderV2"; + +import "../lib/dappsys/auth.sol"; +import "./Authority.sol"; +import "./IColony.sol"; +import "./EtherRouter.sol"; +import "./ERC20Extended.sol"; +import "./ColonyNetworkStorage.sol"; +import "./IColonyNetwork.sol"; + +// TODO: Can we handle a dispute regarding the very first hash that should be set? + + +contract ReputationMiningCycle { + address colonyNetworkAddress; + // TODO: Do we need both these mappings? + mapping (bytes32 => mapping( uint256 => address[])) public submittedHashes; + mapping (address => Submission) public reputationHashSubmissions; + uint256 reputationMiningWindowOpenTimestamp; + mapping (uint256 => Submission[]) public disputeRounds; + + // Tracks the number of submissions in each round that have completed their challenge, one way or the other. + // This might be that they passed the challenge, it might be that their opponent passed (and therefore by implication, + // they failed), or it might be that they timed out + mapping (uint256 => uint256) nHashesCompletedChallengeRound; + // A flaw with this is that if someone spams lots of nonsense transactions, then 'good' users still have to come along and + // explicitly complete the pairings. But if they get the tokens that were staked in order to make the submission, maybe + // that's okay...? + + uint256 public nSubmittedHashes = 0; + uint256 public nInvalidatedHashes = 0; + + struct Submission { + bytes32 hash; + uint256 nNodes; + uint256 lastResponseTimestamp; + uint256 challengeStepCompleted; + } + + // Records for which hashes, for which addresses, for which entries have been accepted + // Otherwise, people could keep submitting the same entry. + mapping (bytes32 => mapping(address => mapping(uint256 => bool))) submittedEntries; + + modifier onlyFinalRoundWhenComplete(uint roundNumber){ + require (nSubmittedHashes - nInvalidatedHashes == 1); + require (disputeRounds[roundNumber].length == 1); //i.e. this is the final round + // Note that even if we are passed the penultimate round, which had a length of two, and had one eliminated, + // and therefore 'delete' called in `invalidateHash`, the array still has a length of '2' - it's just that one + // element is zeroed. If this functionality of 'delete' is ever changed, this will have to change too. + _; + } + + function ReputationMiningCycle() public { + colonyNetworkAddress = msg.sender; + reputationMiningWindowOpenTimestamp = now; + } + + function respondToChallenge(uint256 round, uint256 idx) public { + // TODO: Check challenge response is valid, relating to current challenge + // TODO: Check challenge response relates to this hash + + // Assuming that the challenge response is correct... + uint256 opponentIdx = (idx % 2 == 1 ? idx-1 : idx + 1); + + disputeRounds[round][idx].lastResponseTimestamp = now; + disputeRounds[round][idx].challengeStepCompleted += 1; + // If our opponent responded to this challenge before we did, 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! + if (disputeRounds[round][idx].challengeStepCompleted == disputeRounds[round][opponentIdx].challengeStepCompleted) { + disputeRounds[round][opponentIdx].lastResponseTimestamp = now; + } + } + + function submitNewHash(bytes32 newHash, uint256 nNodes, uint256 entry) public { + //Check the ticket is an eligible one for them to claim + require(entry <= IColonyNetwork(colonyNetworkAddress).getStakedBalance(msg.sender) / 10**15); + require(entry > 0); + if (reputationHashSubmissions[msg.sender].hash != 0x0) { // If this user has submitted before during this round... + require(newHash == reputationHashSubmissions[msg.sender].hash); // ...require that they are submitting the same hash ... + require(nNodes == reputationHashSubmissions[msg.sender].nNodes); // ...require that they are submitting the same number of nodes for that hash ... + require (submittedEntries[newHash][msg.sender][entry] == false); // ... but not this exact entry + } + // TODO: Require minimum stake, that is (much) more than the cost required to defend the valid submission. + // Check the ticket is a winning one. + // TODO Figure out how to uncomment the next line, but not break tests sporadically. + // require((now-reputationMiningWindowOpenTimestamp) <= 3600); + // x = floor(uint((2**256 - 1) / 3600) + if (now-reputationMiningWindowOpenTimestamp <= 3600) { + uint256 x = 32164469232587832062103051391302196625908329073789045566515995557753647122; + uint256 target = (now - reputationMiningWindowOpenTimestamp ) * x; + require(uint256(keccak256(msg.sender, entry, newHash)) < target); + } + + // We only allow this submission if there's still room + // Check there is still room. + require (submittedHashes[newHash][nNodes].length < 12); + + // If this is a new hash, increment nSubmittedHashes as such. + if (submittedHashes[newHash][nNodes].length == 0) { + nSubmittedHashes += 1; + // And add it to the first disputeRound + // NB if no other hash is submitted, no dispute resolution will be required. + disputeRounds[0].push(Submission({hash: newHash, nNodes: nNodes, lastResponseTimestamp: 0, challengeStepCompleted: 0})); + // If we've got a pair of submissions to face off, may as well start now. + if (nSubmittedHashes % 2 == 0) { + disputeRounds[0][nSubmittedHashes-1].lastResponseTimestamp = now; + disputeRounds[0][nSubmittedHashes-2].lastResponseTimestamp = now; + } + } + + + reputationHashSubmissions[msg.sender] = Submission({hash: newHash, nNodes: nNodes, lastResponseTimestamp: 0, challengeStepCompleted: 0}); + //And add the miner to the array list of submissions here + submittedHashes[newHash][nNodes].push(msg.sender); + //Note that they submitted it. + submittedEntries[newHash][msg.sender][entry] = true; + } + + function confirmNewHash(uint256 roundNumber) public + onlyFinalRoundWhenComplete(roundNumber) + { + + // TODO: Require some amount of time to have passed (i.e. people have had a chance to submit other hashes) + Submission storage reputationRootHash = disputeRounds[roundNumber][0]; + IColonyNetwork(colonyNetworkAddress).setReputationRootHash(reputationRootHash.hash, reputationRootHash.nNodes, submittedHashes[disputeRounds[roundNumber][0].hash][disputeRounds[roundNumber][0].nNodes]); + selfdestruct(colonyNetworkAddress); + } + + function invalidateHash(uint256 round, uint256 idx) public { + // What we do depends on our opponent, so work out which index it was at in disputeRounds[round] + uint256 opponentIdx = (idx % 2 == 1 ? idx-1 : idx + 1); + uint256 nInNextRound; + + // We require either + // 1. That we actually had an opponent - can't invalidate the last hash. + // 2. This cycle had an odd number of submissions, which was larger than 1, and we're giving the last entry a bye to the next round. + if (disputeRounds[round].length % 2 == 1 && disputeRounds[round].length == idx) { + // This is option two above - note that because arrays are zero-indexed, if idx==length, then + // this is the slot after the last entry, and so our opponentIdx will be the last entry + // We just move the opponent on, and nothing else happens. + + // Ensure that the previous round is complete, and this entry wouldn't possibly get an opponent later on. + require(nHashesCompletedChallengeRound[round-1] == disputeRounds[round-1].length); + + // Prevent us invalidating the final hash + require(disputeRounds[round].length>1); + // Move opponent on to next round + disputeRounds[round+1].push(disputeRounds[round][opponentIdx]); + // Note the fact that this round has had another challenge complete + nHashesCompletedChallengeRound[round] += 1; + // TODO: DRY with the code below. + // Check if the hash we just moved to the next round is the second of a pairing that should now face off. + nInNextRound = disputeRounds[round+1].length; + + if (nInNextRound % 2 == 0) { + disputeRounds[round+1][nInNextRound-1].challengeStepCompleted = 0; + disputeRounds[round+1][nInNextRound-1].lastResponseTimestamp = now; + disputeRounds[round+1][nInNextRound-2].challengeStepCompleted = 0; + disputeRounds[round+1][nInNextRound-2].lastResponseTimestamp = now; + } + } else { + require(disputeRounds[round].length > opponentIdx); + require(disputeRounds[round][opponentIdx].hash!=""); + // Require that it has failed a challenge (i.e. failed to respond in time) + require(now - disputeRounds[round][idx].lastResponseTimestamp >= 600); //'In time' is ten minutes here. + + if (disputeRounds[round][opponentIdx].challengeStepCompleted > disputeRounds[round][idx].challengeStepCompleted) { + // If true, then the opponent completed one more challenge round than the submission being invalidated, so we + // don't know if they're valid or not yet. Move them on to the next round. + disputeRounds[round+1].push(disputeRounds[round][opponentIdx]); + delete disputeRounds[round][opponentIdx]; + // TODO Delete the hash(es) being invalidated? + nInvalidatedHashes += 1; + // Check if the hash we just moved to the next round is the second of a pairing that should now face off. + nInNextRound = disputeRounds[round+1].length; + if (nInNextRound % 2 == 0) { + disputeRounds[round+1][nInNextRound-1].challengeStepCompleted = 0; + disputeRounds[round+1][nInNextRound-1].lastResponseTimestamp = now; + disputeRounds[round+1][nInNextRound-2].challengeStepCompleted = 0; + disputeRounds[round+1][nInNextRound-2].lastResponseTimestamp = now; + } + } else { + // Our opponent completed the same number of challenge rounds, and both have now timed out. + nInvalidatedHashes += 2; + // Punish the people who proposed our opponent + IColonyNetwork(colonyNetworkAddress).punishStakers(submittedHashes[disputeRounds[round][opponentIdx].hash][disputeRounds[round][opponentIdx].nNodes]); + } + + // Note that two hashes have completed this challenge round (either one accepted for now and one rejected, or two rejected) + nHashesCompletedChallengeRound[round] += 2; + + // Punish the people who proposed the hash that was rejected + IColonyNetwork(colonyNetworkAddress).punishStakers(submittedHashes[disputeRounds[round][idx].hash][disputeRounds[round][idx].nNodes]); + + } + //TODO: Can we do some deleting to make calling this as cheap as possible for people? + } + + + +} diff --git a/migrations/2_deploy_contracts.js b/migrations/2_deploy_contracts.js index 6371b94896..472fb3148c 100644 --- a/migrations/2_deploy_contracts.js +++ b/migrations/2_deploy_contracts.js @@ -7,6 +7,7 @@ const ColonyNetwork = artifacts.require("./ColonyNetwork"); const ColonyNetworkStaking = artifacts.require("./ColonyNetworkStaking"); const EtherRouter = artifacts.require("./EtherRouter"); const Resolver = artifacts.require("./Resolver"); +const ReputationMiningCycle = artifacts.require("./ReputationMiningCycle"); module.exports = (deployer, network) => { console.log(`## ${network} network ##`); @@ -16,4 +17,11 @@ module.exports = (deployer, network) => { deployer.deploy([ColonyNetworkStaking]); deployer.deploy([EtherRouter]); deployer.deploy([Resolver]); + + // We `require` the ReputationMiningCycle object to make sure + // it is injected in the `artifacts` variables during test + // preparation. We need this for the eth-gas-reporter. + // See https://github.com/cgewecke/eth-gas-reporter/issues/64 + // This log prevents "variable not used" warnings. + console.log(ReputationMiningCycle); }; diff --git a/scripts/check-storage.js b/scripts/check-storage.js index 0490e1551a..03aab7f751 100644 --- a/scripts/check-storage.js +++ b/scripts/check-storage.js @@ -12,7 +12,8 @@ fs.readdirSync("./contracts/").forEach(contractName => { "EtherRouter.sol", "Migrations.sol", "Resolver.sol", - "Token.sol" + "Token.sol", + "ReputationMiningCycle.sol" ].indexOf(contractName) > -1 ) { return; From 693eadf9d4a464c28bca35721107a07adc83d82f Mon Sep 17 00:00:00 2001 From: Laurent Senta Date: Tue, 20 Mar 2018 14:06:57 +0100 Subject: [PATCH 4/4] Remove duplicates todo and unecessary logging --- contracts/ColonyNetworkStaking.sol | 3 +-- contracts/ReputationMiningCycle.sol | 5 ----- migrations/2_deploy_contracts.js | 14 ++++++-------- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/contracts/ColonyNetworkStaking.sol b/contracts/ColonyNetworkStaking.sol index 0dbc36ef0b..e13551ff03 100644 --- a/contracts/ColonyNetworkStaking.sol +++ b/contracts/ColonyNetworkStaking.sol @@ -13,6 +13,7 @@ import "./ReputationMiningCycle.sol"; contract ColonyNetworkStaking is ColonyNetworkStorage, DSMath { + // TODO: Can we handle a dispute regarding the very first hash that should be set? modifier onlyReputationMiningCycle () { require(msg.sender == reputationMiningCycle); @@ -122,5 +123,3 @@ contract ColonyNetworkStaking is ColonyNetworkStorage, DSMath { } } } - -// TODO: Can we handle a dispute regarding the very first hash that should be set? \ No newline at end of file diff --git a/contracts/ReputationMiningCycle.sol b/contracts/ReputationMiningCycle.sol index e30934bdd5..5ffdf442f1 100644 --- a/contracts/ReputationMiningCycle.sol +++ b/contracts/ReputationMiningCycle.sol @@ -10,8 +10,6 @@ import "./ERC20Extended.sol"; import "./ColonyNetworkStorage.sol"; import "./IColonyNetwork.sol"; -// TODO: Can we handle a dispute regarding the very first hash that should be set? - contract ReputationMiningCycle { address colonyNetworkAddress; @@ -198,7 +196,4 @@ contract ReputationMiningCycle { } //TODO: Can we do some deleting to make calling this as cheap as possible for people? } - - - } diff --git a/migrations/2_deploy_contracts.js b/migrations/2_deploy_contracts.js index 472fb3148c..2ebdd79fc4 100644 --- a/migrations/2_deploy_contracts.js +++ b/migrations/2_deploy_contracts.js @@ -7,7 +7,12 @@ const ColonyNetwork = artifacts.require("./ColonyNetwork"); const ColonyNetworkStaking = artifacts.require("./ColonyNetworkStaking"); const EtherRouter = artifacts.require("./EtherRouter"); const Resolver = artifacts.require("./Resolver"); -const ReputationMiningCycle = artifacts.require("./ReputationMiningCycle"); + +// We `require` the ReputationMiningCycle object to make sure +// it is injected in the `artifacts` variables during test +// preparation. We need this for the eth-gas-reporter. +// See https://github.com/cgewecke/eth-gas-reporter/issues/64 +artifacts.require("./ReputationMiningCycle"); module.exports = (deployer, network) => { console.log(`## ${network} network ##`); @@ -17,11 +22,4 @@ module.exports = (deployer, network) => { deployer.deploy([ColonyNetworkStaking]); deployer.deploy([EtherRouter]); deployer.deploy([Resolver]); - - // We `require` the ReputationMiningCycle object to make sure - // it is injected in the `artifacts` variables during test - // preparation. We need this for the eth-gas-reporter. - // See https://github.com/cgewecke/eth-gas-reporter/issues/64 - // This log prevents "variable not used" warnings. - console.log(ReputationMiningCycle); };