diff --git a/contracts/FullyBackedSortitionPool.sol b/contracts/FullyBackedSortitionPool.sol index 18d7f49b..a26d7c84 100644 --- a/contracts/FullyBackedSortitionPool.sol +++ b/contracts/FullyBackedSortitionPool.sol @@ -56,6 +56,10 @@ contract FullyBackedSortitionPool is AbstractSortitionPool { PoolParams poolParams; + // Banned operator addresses. Banned operators are removed from the pool; they + // won't be selected to groups and won't be able to join the pool. + mapping(address => bool) public bannedOperators; + constructor( IFullyBackedBonding _bondingContract, uint256 _initialMinimumBondableValue, @@ -87,9 +91,9 @@ contract FullyBackedSortitionPool is AbstractSortitionPool { uint256 groupSize, bytes32 seed, uint256 bondValue - ) public returns (address[] memory) { + ) public onlyOwner() returns (address[] memory) { PoolParams memory params = initializeSelectionParams(bondValue); - require(msg.sender == params.owner, "Only owner may select groups"); + uint256 paramsPtr; // solium-disable-next-line security/no-inline-assembly assembly { @@ -119,6 +123,18 @@ contract FullyBackedSortitionPool is AbstractSortitionPool { return poolParams.minimumBondableValue; } + /// @notice Bans an operator in the pool. The operator is added to banned + /// operators list. If the operator is already registered in the pool it gets + /// removed. Only onwer of the pool can call this function. + /// @param operator An operator address. + function ban(address operator) public onlyOwner() { + bannedOperators[operator] = true; + + if (isOperatorRegistered(operator)) { + removeOperator(operator); + } + } + function initializeSelectionParams(uint256 bondValue) internal returns (PoolParams memory params) @@ -136,6 +152,11 @@ contract FullyBackedSortitionPool is AbstractSortitionPool { // which may differ from the weight in the pool. // Return 0 if ineligible. function getEligibleWeight(address operator) internal view returns (uint256) { + // Check if the operator has been banned. + if (bannedOperators[operator]) { + return 0; + } + address ownerAddress = poolParams.owner; // Get the amount of bondable value available for this pool. // We only care that this covers one single bond @@ -234,4 +255,9 @@ contract FullyBackedSortitionPool is AbstractSortitionPool { // but reasonable to begin with. return Fate(Decision.UpdateSelect, postWeight); } + + modifier onlyOwner() { + require(msg.sender == poolParams.owner, "Caller is not the owner"); + _; + } } diff --git a/test/fullyBackedSortitionPoolTest.js b/test/fullyBackedSortitionPoolTest.js index f29272f9..730bce88 100644 --- a/test/fullyBackedSortitionPoolTest.js +++ b/test/fullyBackedSortitionPoolTest.js @@ -94,6 +94,19 @@ contract("FullyBackedSortitionPool", (accounts) => { assert.isTrue(await pool.isOperatorInitialized(alice)) }) + + it("reverts when operator gets banned in the pool", async () => { + await prepareOperator(alice, bond) + + await mineBlocks(operatorInitBlocks.addn(1)) + + await pool.ban(alice, {from: owner}) + + expectRevert( + pool.isOperatorInitialized(alice), + "Operator is not in the pool", + ) + }) }) describe("selectSetGroup", async () => { @@ -160,7 +173,7 @@ contract("FullyBackedSortitionPool", (accounts) => { await expectRevert( pool.selectSetGroup(3, seed, minimumBondableValue, {from: alice}), - "Only owner may select groups", + "Caller is not the owner", ) }) @@ -221,6 +234,34 @@ contract("FullyBackedSortitionPool", (accounts) => { await pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner}) }) + it("reverts when operator gets banned in the sortition pool", async () => { + // Register three operators. + await prepareOperator(alice, bond) + await prepareOperator(bob, bond) + await prepareOperator(carol, bond) + + assert.equal(await pool.operatorsInPool(), 3) + + // Initialization period passed. + await mineBlocks(operatorInitBlocks.addn(1)) + + await pool.selectSetGroup(2, seed, minimumBondableValue, {from: owner}) + + // Ban an operator. + await pool.ban(carol, {from: owner}) + + assert.equal( + await pool.operatorsInPool(), + 2, + "incorrect number of operators after operator ban", + ) + + await expectRevert( + pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner}), + "Not enough operators in pool", + ) + }) + it("removes minimum-bond-ineligible operators and still works afterwards", async () => { await prepareOperator(alice, bond) await prepareOperator(bob, bond) @@ -382,5 +423,57 @@ contract("FullyBackedSortitionPool", (accounts) => { "Operator is already registered in the pool", ) }) + + it("fails for banned operator", async () => { + await pool.ban(alice, {from: owner}) + + await bonding.setBondableValue(alice, minimumBondableValue) + await bonding.setInitialized(alice, true) + + await expectRevert(pool.joinPool(alice), "Operator not eligible") + }) + }) + + describe("ban", async () => { + it("adds operator to banned operators", async () => { + expect(await pool.bannedOperators(alice)).to.be.false + + await pool.ban(alice, {from: owner}) + + expect(await pool.bannedOperators(alice)).to.be.true + }) + + it("does not revert when called multiple times", async () => { + expect(await pool.bannedOperators(alice)).to.be.false + + await pool.ban(alice, {from: owner}) + await pool.ban(alice, {from: owner}) + + expect(await pool.bannedOperators(alice)).to.be.true + }) + + it("does not revert when operator is not registered", async () => { + expect(await pool.isOperatorRegistered(alice)).to.be.false + + await pool.ban(alice, {from: owner}) + }) + + it("removes operator from the pool", async () => { + await bonding.setBondableValue(alice, minimumBondableValue) + await bonding.setInitialized(alice, true) + await pool.joinPool(alice) + + expect(await pool.isOperatorRegistered(alice)).to.be.true + + await pool.ban(alice, {from: owner}) + + expect(await pool.isOperatorRegistered(alice)).to.be.false + expect(await pool.isOperatorInPool(alice)).to.be.false + expect(await pool.getPoolWeight(alice)).to.eq.BN(0) + }) + + it("reverts when called by non-owner", async () => { + await expectRevert(pool.ban(alice), "Caller is not the owner") + }) }) })