From b0a540201bc998edcddec1fe0ecacf586dad98e2 Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Thu, 3 Feb 2022 11:23:39 +0000 Subject: [PATCH 1/2] Use mining stake not obligation where appropriate --- contracts/colonyNetwork/ColonyNetworkMining.sol | 5 ++--- contracts/reputationMiningCycle/ReputationMiningCycle.sol | 4 ++-- .../reputationMiningCycle/ReputationMiningCycleCommon.sol | 4 ++-- packages/reputation-miner/ReputationMinerClient.js | 6 +++--- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/contracts/colonyNetwork/ColonyNetworkMining.sol b/contracts/colonyNetwork/ColonyNetworkMining.sol index 65c8da80ed..cab5be30de 100644 --- a/contracts/colonyNetwork/ColonyNetworkMining.sol +++ b/contracts/colonyNetwork/ColonyNetworkMining.sol @@ -222,7 +222,7 @@ contract ColonyNetworkMining is ColonyNetworkStorage, MultiChain { ITokenLocking(tokenLocking).deposit(clnyToken, 0, true); // Faux deposit to clear any locks for (uint256 i = 0; i < _stakers.length; i++) { - lostStake = min(ITokenLocking(tokenLocking).getObligation(_stakers[i], clnyToken, address(this)), _amount); + lostStake = min(miningStakes[_stakers[i]].amount, _amount); ITokenLocking(tokenLocking).transferStake(_stakers[i], lostStake, clnyToken, address(this)); // TODO: Lose rep? emit ReputationMinerPenalised(_stakers[i], lostStake); @@ -243,12 +243,11 @@ contract ColonyNetworkMining is ColonyNetworkStorage, MultiChain { function stakeForMining(uint256 _amount) public stoppable { address clnyToken = IMetaColony(metaColony).getToken(); - uint256 existingObligation = ITokenLocking(tokenLocking).getObligation(msg.sender, clnyToken, address(this)); ITokenLocking(tokenLocking).approveStake(msg.sender, _amount, clnyToken); ITokenLocking(tokenLocking).obligateStake(msg.sender, _amount, clnyToken); - miningStakes[msg.sender].timestamp = getNewTimestamp(existingObligation, _amount, miningStakes[msg.sender].timestamp, block.timestamp); + miningStakes[msg.sender].timestamp = getNewTimestamp(miningStakes[msg.sender].amount, _amount, miningStakes[msg.sender].timestamp, block.timestamp); miningStakes[msg.sender].amount = add(miningStakes[msg.sender].amount, _amount); } diff --git a/contracts/reputationMiningCycle/ReputationMiningCycle.sol b/contracts/reputationMiningCycle/ReputationMiningCycle.sol index 305ccbe281..5b3b1d2e12 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycle.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycle.sol @@ -49,8 +49,8 @@ contract ReputationMiningCycle is ReputationMiningCycleCommon { /// @param _jrh The justification root hash for the application of the log being processed. /// @param _entryIndex The number of the entry the submitter hash asked us to consider. function checkEntryQualifies(address _minerAddress, bytes32 _newHash, uint256 _nLeaves, bytes32 _jrh, uint256 _entryIndex) internal { - uint256 lockBalance = ITokenLocking(tokenLockingAddress).getObligation(_minerAddress, clnyTokenAddress, colonyNetworkAddress); - require(_entryIndex <= lockBalance / MIN_STAKE, "colony-reputation-mining-stake-minimum-not-met-for-index"); + uint256 stakedForMining = IColonyNetwork(colonyNetworkAddress).getMiningStake(_minerAddress).amount; + require(_entryIndex <= stakedForMining / MIN_STAKE, "colony-reputation-mining-stake-minimum-not-met-for-index"); require(_entryIndex > 0, "colony-reputation-mining-zero-entry-index-passed"); uint256 stakeTimestamp = IColonyNetwork(colonyNetworkAddress).getMiningStake(_minerAddress).timestamp; diff --git a/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol b/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol index c44218f0d7..020ec1fbaa 100644 --- a/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol +++ b/contracts/reputationMiningCycle/ReputationMiningCycleCommon.sol @@ -34,8 +34,8 @@ contract ReputationMiningCycleCommon is ReputationMiningCycleStorage, PatriciaTr function getMinerAddressIfStaked() internal view returns (address) { // Is msg.sender a miner themselves? See if they have stake. - uint256 lockBalance = ITokenLocking(tokenLockingAddress).getObligation(msg.sender, clnyTokenAddress, colonyNetworkAddress); - if (lockBalance > 0) { + uint256 stakedForMining = IColonyNetwork(colonyNetworkAddress).getMiningStake(msg.sender).amount; + if (stakedForMining > 0) { // If so, they we don't let them mine on someone else's behalf return msg.sender; } diff --git a/packages/reputation-miner/ReputationMinerClient.js b/packages/reputation-miner/ReputationMinerClient.js index 9ef7713184..b8923119ff 100644 --- a/packages/reputation-miner/ReputationMinerClient.js +++ b/packages/reputation-miner/ReputationMinerClient.js @@ -557,12 +557,12 @@ class ReputationMinerClient { const addr = await this._miner.colonyNetwork.getReputationMiningCycle(true); const repCycle = new ethers.Contract(addr, this._miner.repCycleContractDef.abi, this._miner.realWallet); - const balance = await this._miner.tokenLocking.getObligation( + const miningStake = await this._miner.colonyNetwork.getMiningStake( this._miner.minerAddress, - this._miner.clnyAddress, - this._miner.colonyNetwork.address ); + const balance = miningStake.amount; + const reputationMiningWindowOpenTimestamp = await repCycle.getReputationMiningWindowOpenTimestamp(); const rootHash = await this._miner.getRootHash(); From 1f4d6d9d59072ad4589773d91e58a46ce0643c2d Mon Sep 17 00:00:00 2001 From: Alex Rea Date: Mon, 24 Jan 2022 15:51:13 +0000 Subject: [PATCH 2/2] Deduct mining stake on punishment --- contracts/colonyNetwork/ColonyNetworkMining.sol | 1 + test/reputation-system/root-hash-submissions.js | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/contracts/colonyNetwork/ColonyNetworkMining.sol b/contracts/colonyNetwork/ColonyNetworkMining.sol index cab5be30de..d19695a711 100644 --- a/contracts/colonyNetwork/ColonyNetworkMining.sol +++ b/contracts/colonyNetwork/ColonyNetworkMining.sol @@ -223,6 +223,7 @@ contract ColonyNetworkMining is ColonyNetworkStorage, MultiChain { ITokenLocking(tokenLocking).deposit(clnyToken, 0, true); // Faux deposit to clear any locks for (uint256 i = 0; i < _stakers.length; i++) { lostStake = min(miningStakes[_stakers[i]].amount, _amount); + miningStakes[_stakers[i]].amount = sub(miningStakes[_stakers[i]].amount, lostStake); ITokenLocking(tokenLocking).transferStake(_stakers[i], lostStake, clnyToken, address(this)); // TODO: Lose rep? emit ReputationMinerPenalised(_stakers[i], lostStake); diff --git a/test/reputation-system/root-hash-submissions.js b/test/reputation-system/root-hash-submissions.js index 41a29181bd..6a7d79c58c 100644 --- a/test/reputation-system/root-hash-submissions.js +++ b/test/reputation-system/root-hash-submissions.js @@ -618,6 +618,8 @@ contract("Reputation mining - root hash submissions", (accounts) => { const userLockMiner1Before = await tokenLocking.getUserLock(clnyToken.address, MINER1); const userLockMiner2Before = await tokenLocking.getUserLock(clnyToken.address, MINER2); const userLockMiner3Before = await tokenLocking.getUserLock(clnyToken.address, MINER3); + const user2MiningBalanceBefore = await colonyNetwork.getMiningStake(MINER2); + const user3MiningBalanceBefore = await colonyNetwork.getMiningStake(MINER3); // We want badClient2 to submit the same hash as badClient for this test. badClient2 = new MaliciousReputationMinerExtraRep({ loader, minerAddress: MINER3, realProviderPort, useJsTree }, 1, "0xfffffffff"); @@ -673,9 +675,16 @@ contract("Reputation mining - root hash submissions", (accounts) => { const userLockMiner2 = await tokenLocking.getUserLock(clnyToken.address, MINER2); expect(userLockMiner2.balance, "Account was not punished properly").to.eq.BN(new BN(userLockMiner2Before.balance).sub(miner2Loss)); + // Rewards for defences, however, aren't automatically staked, so they should have lost MIN_STAKE from mining stake + const userMiningBalance2 = await colonyNetwork.getMiningStake(MINER2); + expect(userMiningBalance2.amount, "Mining stake was not docked properly").to.eq.BN(new BN(user2MiningBalanceBefore.amount).sub(MIN_STAKE)); + const userLockMiner3 = await tokenLocking.getUserLock(clnyToken.address, MINER3); expect(userLockMiner3.balance, "Account was not punished properly").to.eq.BN(new BN(userLockMiner3Before.balance).sub(miner3Loss)); + const userMiningBalance3 = await colonyNetwork.getMiningStake(MINER3); + expect(userMiningBalance3.amount, "Mining stake was not docked properly").to.eq.BN(new BN(user3MiningBalanceBefore.amount).sub(MIN_STAKE)); + // Reset badClient2 to its default behaviour. badClient2 = new MaliciousReputationMinerExtraRep({ loader, minerAddress: MINER3, realProviderPort, useJsTree }, 1, "0xeeeeeeeee"); });