Ban operator in FullyBackedSortitionPool

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");
+    _;
+  }
 }

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")
+    })
   })
 })