Skip to content

Commit

Permalink
Merge pull request #208 from maxaleks/deny-repeatedly-adding-removed-…
Browse files Browse the repository at this point in the history 
…mining-key

(Fix) Deny repeatedly adding removed mining key
  • Loading branch information
@varasev
varasev authored Jun 21, 2019
2 parents b8d95fd + e253973 commit ea84f61
Show file tree
Hide file tree
Showing 10 changed files with 289 additions and 13 deletions.
25 changes: 25 additions & 0 deletions contracts/KeysManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -84,6 +85,10 @@ contract KeysManager is EternalStorage, IKeysManager {
return true;
}

if (hasMiningKeyBeenRemoved(_newKey)) {
return true;
}

if (_currentKey == address(0)) {
return false;
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions contracts/interfaces/IKeysManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 20 additions & 0 deletions test/keys_manager_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
Expand Down
22 changes: 21 additions & 1 deletion test/keys_manager_upgrade_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1009,4 +1029,4 @@ async function removePayoutKey(_miningKey, _shouldBeSuccessful, options) {
} else {
result.logs.length.should.be.equal(0);
}
}
}
7 changes: 4 additions & 3 deletions test/reward_by_time_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
5 changes: 3 additions & 2 deletions test/reward_by_time_upgrade_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
104 changes: 104 additions & 0 deletions test/voting_to_change_keys_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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() => {
Expand Down
Loading

0 comments on commit ea84f61

Please sign in to comment.