diff --git a/contracts/KeysManager.sol b/contracts/KeysManager.sol index e6a1591..79d111a 100644 --- a/contracts/KeysManager.sol +++ b/contracts/KeysManager.sol @@ -30,6 +30,7 @@ contract KeysManager is EternalStorage, IKeysManager { bytes32 internal constant IS_MINING_ACTIVE = "isMiningActive"; bytes32 internal constant IS_PAYOUT_ACTIVE = "isPayoutActive"; bytes32 internal constant IS_VOTING_ACTIVE = "isVotingActive"; + bytes32 internal constant HAS_MINING_KEY_BEEN_REMOVED = "hasMiningKeyBeenRemoved"; bytes32 internal constant MINING_KEY_BY_PAYOUT = "miningKeyByPayout"; bytes32 internal constant MINING_KEY_BY_VOTING = "miningKeyByVoting"; bytes32 internal constant MINING_KEY_HISTORY = "miningKeyHistory"; @@ -84,6 +85,10 @@ contract KeysManager is EternalStorage, IKeysManager { return true; } + if (hasMiningKeyBeenRemoved(_newKey)) { + return true; + } + if (_currentKey == address(0)) { return false; } @@ -346,6 +351,12 @@ contract KeysManager is EternalStorage, IKeysManager { ]; } + function hasMiningKeyBeenRemoved(address _key) public view returns(bool) { + return boolStorage[ + keccak256(abi.encode(HAS_MINING_KEY_BEEN_REMOVED, _key)) + ]; + } + function getMiningKeyHistory(address _miningKey) public view returns(address) { return miningKeyHistory(_miningKey); } @@ -357,6 +368,9 @@ contract KeysManager is EternalStorage, IKeysManager { function addMiningKey(address _key) public onlyVotingToChangeKeys returns(bool) { if (!_withinTotalLimit()) return false; if (!initDisabled()) return false; + if (hasMiningKeyBeenRemoved(_key)) { + return false; + } if (!IPoaNetworkConsensus(poaNetworkConsensus()).addValidator(_key, true)) { return false; } @@ -402,6 +416,7 @@ contract KeysManager is EternalStorage, IKeysManager { _setMiningKeyByPayout(payoutKey, address(0)); _clearMiningKey(_key); _getValidatorMetadata().clearMetadata(_key); + _setHasMiningKeyBeenRemoved(_key); emit MiningKeyChanged(_key, "removed"); if (votingKey != address(0)) { emit VotingKeyChanged(votingKey, _key, "removed"); @@ -435,6 +450,9 @@ contract KeysManager is EternalStorage, IKeysManager { { if (_key == _oldMiningKey) return false; if (!isMiningActive(_oldMiningKey)) return false; + if (hasMiningKeyBeenRemoved(_key)) { + return false; + } if (!IPoaNetworkConsensus(poaNetworkConsensus()).swapValidatorKey(_key, _oldMiningKey)) { return false; } @@ -450,6 +468,7 @@ contract KeysManager is EternalStorage, IKeysManager { _setMiningKeyByVoting(votingKey, _key); _setMiningKeyByPayout(payoutKey, _key); _getValidatorMetadata().moveMetadata(_oldMiningKey, _key); + _setHasMiningKeyBeenRemoved(_oldMiningKey); emit MiningKeyChanged(_key, "swapped"); return true; } @@ -599,6 +618,12 @@ contract KeysManager is EternalStorage, IKeysManager { ] = _success; } + function _setHasMiningKeyBeenRemoved(address _key) private { + boolStorage[ + keccak256(abi.encode(HAS_MINING_KEY_BEEN_REMOVED, _key)) + ] = true; + } + function _withinTotalLimit() private view returns(bool) { IPoaNetworkConsensus poa = IPoaNetworkConsensus(poaNetworkConsensus()); return poa.getCurrentValidatorsLength() < maxLimitValidators(); diff --git a/contracts/interfaces/IKeysManager.sol b/contracts/interfaces/IKeysManager.sol index 7e58172..a0480d2 100644 --- a/contracts/interfaces/IKeysManager.sol +++ b/contracts/interfaces/IKeysManager.sol @@ -20,6 +20,7 @@ interface IKeysManager { function isMiningActive(address) external view returns(bool); function isVotingActive(address) external view returns(bool); function isPayoutActive(address) external view returns(bool); + function hasMiningKeyBeenRemoved(address) external view returns(bool); function getVotingByMining(address) external view returns(address); function getPayoutByMining(address) external view returns(address); function getTime() external view returns(uint256); diff --git a/test/keys_manager_test.js b/test/keys_manager_test.js index b78bdf8..ac325b0 100644 --- a/test/keys_manager_test.js +++ b/test/keys_manager_test.js @@ -289,6 +289,21 @@ contract('KeysManager [all features]', function (accounts) { logs[0].args.key.should.be.equal(accounts[2]); logs[0].args.action.should.be.equal('added'); }) + it('cannot add removed mining key', async () => { + let data; + const key = accounts[3]; + + data = await keysManager.addMiningKey(key).should.be.fulfilled; + data.logs[0].event.should.be.equal('MiningKeyChanged'); + data.logs[0].args.action.should.be.equal('added'); + + data = await keysManager.removeMiningKey(key).should.be.fulfilled; + data.logs[0].event.should.be.equal('MiningKeyChanged'); + data.logs[0].args.action.should.be.equal('removed'); + + data = await keysManager.addMiningKey(key).should.be.fulfilled; + (data.logs[0] === undefined).should.be.equal(true); + }) }) describe('#addVotingKey', async () => { @@ -693,6 +708,11 @@ contract('KeysManager [all features]', function (accounts) { false ]); }); + it('cannot swap to removed mining key', async () => { + await addMiningKey(accounts[1], true); + await swapMiningKey(accounts[2], accounts[1], true); + await swapMiningKey(accounts[1], accounts[2], false); + }) it('should swap MoC', async () => { (await keysManager.masterOfCeremony.call()).should.be.equal(masterOfCeremony); (await poaNetworkConsensusMock.masterOfCeremony.call()).should.be.equal(masterOfCeremony); diff --git a/test/keys_manager_upgrade_test.js b/test/keys_manager_upgrade_test.js index 0eee2aa..cfb5d36 100644 --- a/test/keys_manager_upgrade_test.js +++ b/test/keys_manager_upgrade_test.js @@ -296,6 +296,21 @@ contract('KeysManager upgraded [all features]', function (accounts) { logs[0].args.key.should.be.equal(accounts[2]); logs[0].args.action.should.be.equal('added'); }) + it('cannot add removed mining key', async () => { + let data; + const key = accounts[3]; + + data = await keysManager.addMiningKey(key).should.be.fulfilled; + data.logs[0].event.should.be.equal('MiningKeyChanged'); + data.logs[0].args.action.should.be.equal('added'); + + data = await keysManager.removeMiningKey(key).should.be.fulfilled; + data.logs[0].event.should.be.equal('MiningKeyChanged'); + data.logs[0].args.action.should.be.equal('removed'); + + data = await keysManager.addMiningKey(key).should.be.fulfilled; + (data.logs[0] === undefined).should.be.equal(true); + }) }) describe('#addVotingKey', async () => { @@ -700,6 +715,11 @@ contract('KeysManager upgraded [all features]', function (accounts) { false ]); }); + it('cannot swap to removed mining key', async () => { + await addMiningKey(accounts[1], true); + await swapMiningKey(accounts[2], accounts[1], true); + await swapMiningKey(accounts[1], accounts[2], false); + }) it('should swap MoC', async () => { (await keysManager.masterOfCeremony.call()).should.be.equal(masterOfCeremony); (await poaNetworkConsensusMock.masterOfCeremony.call()).should.be.equal(masterOfCeremony); @@ -1009,4 +1029,4 @@ async function removePayoutKey(_miningKey, _shouldBeSuccessful, options) { } else { result.logs.length.should.be.equal(0); } -} \ No newline at end of file +} diff --git a/test/reward_by_time_test.js b/test/reward_by_time_test.js index db0ca05..cfab9ef 100644 --- a/test/reward_by_time_test.js +++ b/test/reward_by_time_test.js @@ -261,9 +261,10 @@ contract('RewardByTime [all features]', function (accounts) { result.logs[0].args.rewards[3].toString().should.be.equal((emissionFundsAmount * 3).toString()); (await rewardByTime.lastTime.call()).should.be.bignumber.equal(100 + threshold * 7); (await rewardByTime.keyIndex.call()).should.be.bignumber.equal(1); - - await addMiningKey(miningKey2); - await addPayoutKey(payoutKey2, miningKey2); + + const miningKey4 = '0x0000000000000000000000000000000000000001'; + await addMiningKey(miningKey4); + await addPayoutKey(payoutKey2, miningKey4); await poaNetworkConsensus.setSystemAddress(coinbase); await poaNetworkConsensus.finalizeChange().should.be.fulfilled; await poaNetworkConsensus.setSystemAddress('0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE'); diff --git a/test/reward_by_time_upgrade_test.js b/test/reward_by_time_upgrade_test.js index f3278ed..471de57 100644 --- a/test/reward_by_time_upgrade_test.js +++ b/test/reward_by_time_upgrade_test.js @@ -267,8 +267,9 @@ contract('RewardByTime upgraded [all features]', function (accounts) { (await rewardByTime.lastTime.call()).should.be.bignumber.equal(100 + threshold * 7); (await rewardByTime.keyIndex.call()).should.be.bignumber.equal(1); - await addMiningKey(miningKey2); - await addPayoutKey(payoutKey2, miningKey2); + const miningKey4 = '0x0000000000000000000000000000000000000001'; + await addMiningKey(miningKey4); + await addPayoutKey(payoutKey2, miningKey4); await poaNetworkConsensus.setSystemAddress(coinbase); await poaNetworkConsensus.finalizeChange().should.be.fulfilled; await poaNetworkConsensus.setSystemAddress('0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE'); diff --git a/test/voting_to_change_keys_test.js b/test/voting_to_change_keys_test.js index 817582d..711ce5f 100644 --- a/test/voting_to_change_keys_test.js +++ b/test/voting_to_change_keys_test.js @@ -290,6 +290,74 @@ contract('Voting to change keys [all features]', function (accounts) { {from: votingKey} ).should.be.rejectedWith(ERROR_MSG) }) + it('cannot create a ballot for adding removed mining key', async () => { + await proxyStorageMock.setVotingContractMock(accounts[0]); + VOTING_START_DATE = moment.utc().add(20, 'seconds').unix(); + VOTING_END_DATE = moment.utc().add(10, 'days').unix(); + + await addMiningKey(accounts[1]); + await addVotingKey(votingKey, accounts[1]); + + await addMiningKey(accounts[2]); + const {logs} = await keysManager.removeMiningKey(accounts[2]); + logs[0].event.should.be.equal("MiningKeyChanged"); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 1, // _ballotType (KeyAdding) + 1, + "memo", + accounts[2], // _affectedKey + '0x0000000000000000000000000000000000000000', + {from: votingKey} + ).should.be.rejectedWith(ERROR_MSG); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 1, // _ballotType (KeyAdding) + 1, + "memo", + accounts[3], // _affectedKey + '0x0000000000000000000000000000000000000000', + {from: votingKey} + ).should.be.fulfilled; + }) + it('cannot create a ballot for swapping to removed mining key', async () => { + await proxyStorageMock.setVotingContractMock(accounts[0]); + VOTING_START_DATE = moment.utc().add(20, 'seconds').unix(); + VOTING_END_DATE = moment.utc().add(10, 'days').unix(); + + await addMiningKey(accounts[1]); + await addVotingKey(votingKey, accounts[1]); + + await addMiningKey(accounts[2]); + const {logs} = await keysManager.removeMiningKey(accounts[2]); + logs[0].event.should.be.equal("MiningKeyChanged"); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 3, // _ballotType (KeySwap) + 1, + "memo", + accounts[2], // _affectedKey + accounts[1], + {from: votingKey} + ).should.be.rejectedWith(ERROR_MSG); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 3, // _ballotType (KeySwap) + 1, + "memo", + accounts[3], // _affectedKey + accounts[1], + {from: votingKey} + ).should.be.fulfilled; + }) }) describe('#createBallotToAddNewValidator', async () => { @@ -511,6 +579,42 @@ contract('Voting to change keys [all features]', function (accounts) { await poaNetworkConsensusMock.finalizeChange().should.be.fulfilled; (await poaNetworkConsensusMock.getCurrentValidatorsLength.call()).should.be.bignumber.equal(4); }); + + it('cannot create a ballot with earlier removed mining key', async () => { + await proxyStorageMock.setVotingContractMock(accounts[0]); + let data = await keysManager.removePayoutKey(miningKeyForVotingKey); + data.logs[0].event.should.be.equal("PayoutKeyChanged"); + + VOTING_START_DATE = moment.utc().add(20, 'seconds').unix(); + VOTING_END_DATE = moment.utc().add(10, 'days').unix(); + + await addMiningKey(accounts[3]); + await addVotingKey(accounts[4], accounts[3]); + + await addMiningKey(accounts[5]); + data = await keysManager.removeMiningKey(accounts[5]); + data.logs[0].event.should.be.equal("MiningKeyChanged"); + + await voting.createBallotToAddNewValidator( + VOTING_START_DATE, // _startTime + VOTING_END_DATE, // _endTime + "memo", // _memo + accounts[5], // _newMiningKey + accounts[6], // _newVotingKey + accounts[7], // _newPayoutKey + {from: votingKey} + ).should.be.rejectedWith(ERROR_MSG); + + await voting.createBallotToAddNewValidator( + VOTING_START_DATE, // _startTime + VOTING_END_DATE, // _endTime + "memo", // _memo + accounts[8], // _newMiningKey + accounts[6], // _newVotingKey + accounts[7], // _newPayoutKey + {from: votingKey} + ).should.be.fulfilled; + }); }); describe('#vote', async() => { diff --git a/test/voting_to_change_keys_upgrade_test.js b/test/voting_to_change_keys_upgrade_test.js index 5638f38..3448ccc 100644 --- a/test/voting_to_change_keys_upgrade_test.js +++ b/test/voting_to_change_keys_upgrade_test.js @@ -297,6 +297,74 @@ contract('Voting to change keys upgraded [all features]', function (accounts) { {from: votingKey} ).should.be.rejectedWith(ERROR_MSG) }) + it('cannot create a ballot for adding removed mining key', async () => { + await proxyStorageMock.setVotingContractMock(accounts[0]); + VOTING_START_DATE = moment.utc().add(20, 'seconds').unix(); + VOTING_END_DATE = moment.utc().add(10, 'days').unix(); + + await addMiningKey(accounts[1]); + await addVotingKey(votingKey, accounts[1]); + + await addMiningKey(accounts[2]); + const {logs} = await keysManager.removeMiningKey(accounts[2]); + logs[0].event.should.be.equal("MiningKeyChanged"); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 1, // _ballotType (KeyAdding) + 1, + "memo", + accounts[2], // _affectedKey + '0x0000000000000000000000000000000000000000', + {from: votingKey} + ).should.be.rejectedWith(ERROR_MSG); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 1, // _ballotType (KeyAdding) + 1, + "memo", + accounts[3], // _affectedKey + '0x0000000000000000000000000000000000000000', + {from: votingKey} + ).should.be.fulfilled; + }) + it('cannot create a ballot for swapping to removed mining key', async () => { + await proxyStorageMock.setVotingContractMock(accounts[0]); + VOTING_START_DATE = moment.utc().add(20, 'seconds').unix(); + VOTING_END_DATE = moment.utc().add(10, 'days').unix(); + + await addMiningKey(accounts[1]); + await addVotingKey(votingKey, accounts[1]); + + await addMiningKey(accounts[2]); + const {logs} = await keysManager.removeMiningKey(accounts[2]); + logs[0].event.should.be.equal("MiningKeyChanged"); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 3, // _ballotType (KeySwap) + 1, + "memo", + accounts[2], // _affectedKey + accounts[1], + {from: votingKey} + ).should.be.rejectedWith(ERROR_MSG); + + await voting.createBallot( + VOTING_START_DATE, + VOTING_END_DATE, + 3, // _ballotType (KeySwap) + 1, + "memo", + accounts[3], // _affectedKey + accounts[1], + {from: votingKey} + ).should.be.fulfilled; + }) }) describe('#createBallotToAddNewValidator', async () => { @@ -518,6 +586,42 @@ contract('Voting to change keys upgraded [all features]', function (accounts) { await poaNetworkConsensusMock.finalizeChange().should.be.fulfilled; (await poaNetworkConsensusMock.getCurrentValidatorsLength.call()).should.be.bignumber.equal(4); }); + + it('cannot create a ballot with earlier removed mining key', async () => { + await proxyStorageMock.setVotingContractMock(accounts[0]); + let data = await keysManager.removePayoutKey(miningKeyForVotingKey); + data.logs[0].event.should.be.equal("PayoutKeyChanged"); + + VOTING_START_DATE = moment.utc().add(20, 'seconds').unix(); + VOTING_END_DATE = moment.utc().add(10, 'days').unix(); + + await addMiningKey(accounts[3]); + await addVotingKey(accounts[4], accounts[3]); + + await addMiningKey(accounts[5]); + data = await keysManager.removeMiningKey(accounts[5]); + data.logs[0].event.should.be.equal("MiningKeyChanged"); + + await voting.createBallotToAddNewValidator( + VOTING_START_DATE, // _startTime + VOTING_END_DATE, // _endTime + "memo", // _memo + accounts[5], // _newMiningKey + accounts[6], // _newVotingKey + accounts[7], // _newPayoutKey + {from: votingKey} + ).should.be.rejectedWith(ERROR_MSG); + + await voting.createBallotToAddNewValidator( + VOTING_START_DATE, // _startTime + VOTING_END_DATE, // _endTime + "memo", // _memo + accounts[8], // _newMiningKey + accounts[6], // _newVotingKey + accounts[7], // _newPayoutKey + {from: votingKey} + ).should.be.fulfilled; + }); }); describe('#vote', async() => { @@ -1320,4 +1424,4 @@ async function finalize(_id, _shouldBeSuccessful, options) { } else { result.logs.length.should.be.equal(0); } -} \ No newline at end of file +} diff --git a/test/voting_to_manage_emission_funds_test.js b/test/voting_to_manage_emission_funds_test.js index cc70362..024a239 100644 --- a/test/voting_to_manage_emission_funds_test.js +++ b/test/voting_to_manage_emission_funds_test.js @@ -656,7 +656,7 @@ contract('VotingToManageEmissionFunds [all features]', function (accounts) { await voting.vote(id, choice.send, {from: votingKey3}).should.be.rejectedWith(ERROR_MSG); }); - it('should not let vote with old miningKey', async () => { + it('should not let the same user to re-vote with another miningKey', async () => { await addValidator(votingKey2, miningKey2); await voting.setTime(VOTING_START_DATE); @@ -693,9 +693,9 @@ contract('VotingToManageEmissionFunds [all features]', function (accounts) { false.should.be.equal((await voting.getBallotInfo.call(id))[4]); // isFinalized await proxyStorage.setVotingContractMock(coinbase); - let result = await keysManager.swapMiningKey(miningKey, miningKey3); + let result = await keysManager.swapMiningKey(miningKey4, miningKey3); result.logs[0].event.should.equal("MiningKeyChanged"); - await swapVotingKey(votingKey, miningKey); + await swapVotingKey(votingKey, miningKey4); await proxyStorage.setVotingContractMock(votingForKeysEternalStorage.address); await poaNetworkConsensus.setSystemAddress(coinbase); await poaNetworkConsensus.finalizeChange().should.be.fulfilled; diff --git a/test/voting_to_manage_emission_funds_upgrade_test.js b/test/voting_to_manage_emission_funds_upgrade_test.js index 83bd678..0c3b722 100644 --- a/test/voting_to_manage_emission_funds_upgrade_test.js +++ b/test/voting_to_manage_emission_funds_upgrade_test.js @@ -663,7 +663,7 @@ contract('VotingToManageEmissionFunds upgraded [all features]', function (accoun await voting.vote(id, choice.send, {from: votingKey3}).should.be.rejectedWith(ERROR_MSG); }); - it('should not let vote with old miningKey', async () => { + it('should not let the same user to re-vote with another miningKey', async () => { await addValidator(votingKey2, miningKey2); await voting.setTime(VOTING_START_DATE); @@ -700,9 +700,9 @@ contract('VotingToManageEmissionFunds upgraded [all features]', function (accoun false.should.be.equal((await voting.getBallotInfo.call(id))[4]); // isFinalized await proxyStorage.setVotingContractMock(coinbase); - let result = await keysManager.swapMiningKey(miningKey, miningKey3); + let result = await keysManager.swapMiningKey(miningKey4, miningKey3); result.logs[0].event.should.equal("MiningKeyChanged"); - await swapVotingKey(votingKey, miningKey); + await swapVotingKey(votingKey, miningKey4); await proxyStorage.setVotingContractMock(votingForKeysEternalStorage.address); await poaNetworkConsensus.setSystemAddress(coinbase); await poaNetworkConsensus.finalizeChange().should.be.fulfilled; @@ -1048,4 +1048,4 @@ async function addValidator(_votingKey, _miningKey) { async function swapVotingKey(_key, _miningKey) { const {logs} = await keysManager.swapVotingKey(_key, _miningKey); logs[0].event.should.be.equal("VotingKeyChanged"); -} \ No newline at end of file +}