From 5f6eb4a1974524ae248f797c37d3cb19b2f11a48 Mon Sep 17 00:00:00 2001
From: Jakub Nowakowski <jakub.nowakowski@akena.co>
Date: Wed, 19 Aug 2020 16:52:09 +0200
Subject: [PATCH 1/8] Removed interfaces for SortitionPools

Removed interffaces that were defined for BondedSortitionPool and
SortitionPool. The interfaces are not being used anywhere and they are
not up-to-date. In keep-ecdsa we are using the contracts directly.
---
 contracts/api/IBondedSortitionPool.sol | 41 --------------------------
 contracts/api/ISortitionPool.sol       | 37 -----------------------
 2 files changed, 78 deletions(-)
 delete mode 100644 contracts/api/IBondedSortitionPool.sol
 delete mode 100644 contracts/api/ISortitionPool.sol

diff --git a/contracts/api/IBondedSortitionPool.sol b/contracts/api/IBondedSortitionPool.sol
deleted file mode 100644
index 617ca718..00000000
--- a/contracts/api/IBondedSortitionPool.sol
+++ /dev/null
@@ -1,41 +0,0 @@
-pragma solidity 0.5.17;
-
-interface IBondedSortitionPool {
-    /// @notice Selects a new group of operators of the provided size based on
-    /// the provided pseudo-random seed and bonding requirements. All operators
-    /// in the group are unique.
-    ///
-    /// If there are not enough operators in a pool to form a group or not
-    /// enough operators are eligible for work selection given the bonding
-    /// requirements, the function fails.
-    /// @param groupSize Size of the requested group
-    /// @param seed Pseudo-random number used to select operators to group
-    /// @param bondValue Size of the requested bond per operator
-    /// @return selected Members of the selected group
-    function selectSetGroup(uint256 groupSize, bytes32 seed, uint256 bondValue)
-        external returns (address[] memory selected);
-
-    // Return whether the operator is eligible for the pool.
-    // Checks that the operator has sufficient staked tokens and bondable value,
-    // and the required authorizations.
-    function isOperatorEligible(address operator) external view returns (bool);
-
-    // Return whether the operator is present in the pool.
-    function isOperatorInPool(address operator) external view returns (bool);
-
-    // Return whether the operator is up to date in the pool,
-    // i.e. whether its eligible weight matches its current weight in the pool.
-    // If the operator is eligible but not present, return False.
-    function isOperatorUpToDate(address operator) external view returns (bool);
-
-    // Add an operator to the pool,
-    // reverting if the operator is already present.
-    function joinPool(address operator) external;
-
-    // Update the operator's weight if present and eligible,
-    // or remove from the pool if present and ineligible.
-    function updateOperatorStatus(address operator) external;
-
-    // Returns the total weight of the pool.
-    function totalWeight() external view returns (uint256);
-}
diff --git a/contracts/api/ISortitionPool.sol b/contracts/api/ISortitionPool.sol
deleted file mode 100644
index 972cdc53..00000000
--- a/contracts/api/ISortitionPool.sol
+++ /dev/null
@@ -1,37 +0,0 @@
-pragma solidity 0.5.17;
-
-interface ISortitionPool {
-    /// @notice Selects a new group of operators of the provided size based on
-    /// the provided pseudo-random seed. At least one operator has to be
-    /// registered in the pool, otherwise the function fails reverting the
-    /// transaction.
-    /// @param groupSize Size of the requested group
-    /// @param seed Pseudo-random number used to select operators to group
-    /// @return selected Members of the selected group
-    function selectGroup(uint256 groupSize, bytes32 seed)
-        external returns (address[] memory selected);
-
-    // Return whether the operator is eligible for the pool.
-    // Checks that the operator has sufficient staked tokens and bondable value,
-    // and the required authorizations.
-    function isOperatorEligible(address operator) external view returns (bool);
-
-    // Return whether the operator is present in the pool.
-    function isOperatorInPool(address operator) external view returns (bool);
-
-    // Return whether the operator is up to date in the pool,
-    // i.e. whether its eligible weight matches its current weight in the pool.
-    // If the operator is eligible but not present, return False.
-    function isOperatorUpToDate(address operator) external view returns (bool);
-
-    // Add an operator to the pool,
-    // reverting if the operator is already present.
-    function joinPool(address operator) external;
-
-    // Update the operator's weight if present and eligible,
-    // or remove from the pool if present and ineligible.
-    function updateOperatorStatus(address operator) external;
-
-    // Returns the total weight of the pool.
-    function totalWeight() external view returns (uint256);
-}

From 2e06928819993e51cf9ccfecb9e4eaec52f971c1 Mon Sep 17 00:00:00 2001
From: Jakub Nowakowski <jakub.nowakowski@akena.co>
Date: Thu, 20 Aug 2020 08:00:28 +0200
Subject: [PATCH 2/8] Minimum bondable value for Fully Backed Sortition Pool

The pool was updating the minimum bondable value for each
selectSetGroup call. The idea was to automatically remove from the pool
operators that are no longer able to satisfy application needs about the
minimum available bondable value and prevent operators with too low value
griefing signer selection.

We now have two minimum bond values: one defining the minimum unbonded
value the operator needs to have so that it can join and stay in the pool,
and another defining the required bond value for the current selection.
If the given operator has enough unbonded value to stay in the pool but it
does not have enough unbonded value for the current group selection, it is
skipped.

We assume a reasonable minimum bondable value is set on the pool
creation and it can be later updated by the pool owner (application).
We move the responsibility of keeping this value sane to the application.
If the application defines allowed lot sizes, just like tBTC, the application
may set the minimum bondable value to the minimum lot size and later skip
operators that are not eligible to satisfy bond for the current selection
instead of removing them from the pool. All operators without at least the
minimum unbonded value are removed from the pool during the group selection.

The change was originally implememnted to BondedSortitionPool in 46d187a.
---
 contracts/FullyBackedSortitionPool.sol        | 60 +++++++++++++++----
 contracts/FullyBackedSortitionPoolFactory.sol | 10 ++--
 2 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/contracts/FullyBackedSortitionPool.sol b/contracts/FullyBackedSortitionPool.sol
index 28108ddc..9b7b46a5 100644
--- a/contracts/FullyBackedSortitionPool.sol
+++ b/contracts/FullyBackedSortitionPool.sol
@@ -24,6 +24,8 @@ import "./DynamicArray.sol";
 /// If the changes would be detrimental to the operator,
 /// the operator selection is performed again with the updated input
 /// to ensure correctness.
+/// The pool should specify a reasonable minimum bondable value for operators
+/// trying to join the pool, to prevent griefing the selection.
 contract FullyBackedSortitionPool is AbstractSortitionPool {
   using DynamicArray for DynamicArray.UintArray;
   using DynamicArray for DynamicArray.AddressArray;
@@ -35,8 +37,14 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
   // this value can be set to equal the most recent request's bondValue.
 
   struct PoolParams {
-    IBonding bondingContract;
-    uint256 minimumAvailableBond;
+    // Defines the minimum unbounded value the operator needs to have to be
+    // eligible to join and stay in the sortition pool. Operators not
+    // satisfying minimum bondable value are removed from the pool.
+    uint256 minimumBondableValue;
+    // Bond required from each operator for the currently pending group
+    // selection. If operator does not have at least this unbounded value,
+    // it is skipped during the selection.
+    uint256 requestedBond;
     // Because the minimum available bond may fluctuate,
     // we use a constant pool weight divisor.
     // When we receive the available bond,
@@ -49,8 +57,7 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
   PoolParams poolParams;
 
   constructor(
-    IBonding _bondingContract,
-    uint256 _initialMinimumStake,
+    uint256 _initialMinimumBondableValue,
     uint256 _bondWeightDivisor,
     address _poolOwner
   ) public {
@@ -58,7 +65,8 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
 
     poolParams = PoolParams(
       _bondingContract,
-      _initialMinimumStake,
+      _initialMinimumBondableValue,
+      0,
       _bondWeightDivisor,
       _poolOwner
     );
@@ -89,15 +97,35 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
     return generalizedSelectGroup(groupSize, seed, paramsPtr, true);
   }
 
+  /// @notice Sets the minimum bondable value required from the operator
+  /// so that it is eligible to be in the pool. The pool should specify
+  /// a reasonable minimum requirement for operators trying to join the pool
+  /// to prevent griefing group selection.
+  /// @param minimumBondableValue The minimum bondable value required from the
+  /// operator.
+  function setMinimumBondableValue(uint256 minimumBondableValue) public {
+    require(
+      msg.sender == poolParams.owner,
+      "Only owner may update minimum bond value"
+    );
+
+    poolParams.minimumBondableValue = minimumBondableValue;
+  }
+
+  /// @notice Returns the minimum bondable value required from the operator
+  /// so that it is eligible to be in the pool.
+  function getMinimumBondableValue() public view returns (uint256) {
+    return poolParams.minimumBondableValue;
+  }
+
   function initializeSelectionParams(uint256 bondValue)
     internal
     returns (PoolParams memory params)
   {
     params = poolParams;
 
-    if (params.minimumAvailableBond != bondValue) {
-      params.minimumAvailableBond = bondValue;
-      poolParams.minimumAvailableBond = bondValue;
+    if (params.requestedBond != bondValue) {
+      params.requestedBond = bondValue;
     }
 
     return params;
@@ -118,13 +146,12 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
     );
 
     // Don't query stake if bond is insufficient.
-    if (bondableValue < poolParams.minimumAvailableBond) {
+    if (bondableValue < poolParams.minimumBondableValue) {
       return 0;
     }
 
     // Weight = floor(eligibleStake / mimimumStake)
     // Ethereum uint256 division performs implicit floor
-    // If eligibleStake < minimumStake, return 0 = ineligible.
     return (bondableValue / poolParams.bondWeightDivisor);
   }
 
@@ -155,14 +182,21 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
       address(this)
     );
 
-    // Don't proceed further if bond is insufficient.
-    if (preStake < params.minimumAvailableBond) {
+    // If unbonded value is insufficient for the operator to be in the pool,
+    // delete the operator.
+    if (preStake < params.minimumBondableValue) {
       return Fate(Decision.Delete, 0);
     }
 
+    // If unbonded value is sufficient for the operator to be in the pool
+    // but it is not sufficient for the current selection, skip the operator.
+    if (preStake < params.requestedBond) {
+      return Fate(Decision.Skip, 0);
+    }
+
     // Calculate the bond-stake that would be left after selection
     // Doesn't underflow because preStake >= minimum
-    uint256 postStake = preStake - params.minimumAvailableBond;
+    uint256 postStake = preStake - params.minimumBondableValue;
 
     // Calculate the eligible pre-selection weight
     // based on the constant weight divisor.
diff --git a/contracts/FullyBackedSortitionPoolFactory.sol b/contracts/FullyBackedSortitionPoolFactory.sol
index c561ae58..80460eb8 100644
--- a/contracts/FullyBackedSortitionPoolFactory.sol
+++ b/contracts/FullyBackedSortitionPoolFactory.sol
@@ -8,22 +8,20 @@ import "./api/IStaking.sol";
 /// @notice Factory for the creation of fully-backed sortition pools.
 contract FullyBackedSortitionPoolFactory {
   /// @notice Creates a new fully-backed sortition pool instance.
-  /// @param bondingContract Keep Bonding contract reference.
-  /// @param minimumStake Minimum stake value making the operator eligible to
-  /// join the network.
+  /// @param minimumBondableValue Minimum unbonded value making the operator
+  /// eligible to join the network.
   /// @param bondWeightDivisor Constant divisor for the available bond used to
   /// evalate the applicable weight.
   /// @return Address of the new fully-backed sortition pool contract instance.
   function createSortitionPool(
-    IBonding bondingContract,
-    uint256 minimumStake,
+    uint256 minimumBondableValue,
     uint256 bondWeightDivisor
   ) public returns (address) {
     return
       address(
         new FullyBackedSortitionPool(
           bondingContract,
-          minimumStake,
+          minimumBondableValue,
           bondWeightDivisor,
           msg.sender
         )

From 059b09350205d4080da11fd824b735beb40d84c9 Mon Sep 17 00:00:00 2001
From: Jakub Nowakowski <jakub.nowakowski@akena.co>
Date: Thu, 20 Aug 2020 08:06:59 +0200
Subject: [PATCH 3/8] Added bonding contract interface for fully backed bonding

We created a dedicated interface for the fully backed bonding as there
will be specific rules to verify for the group selection.
For regular BondedSortitionPool we had a staking contract reference
holding informations like eligibleStake, here we don't use staking
contract but all required informations are received from bonding
contract.
---
 contracts/FullyBackedSortitionPool.sol        |  5 +++--
 contracts/FullyBackedSortitionPoolFactory.sol |  4 +++-
 contracts/api/IFullyBackedBonding.sol         | 19 +++++++++++++++++++
 3 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 contracts/api/IFullyBackedBonding.sol

diff --git a/contracts/FullyBackedSortitionPool.sol b/contracts/FullyBackedSortitionPool.sol
index 9b7b46a5..dbc9b8d0 100644
--- a/contracts/FullyBackedSortitionPool.sol
+++ b/contracts/FullyBackedSortitionPool.sol
@@ -2,9 +2,8 @@ pragma solidity 0.5.17;
 
 import "./AbstractSortitionPool.sol";
 import "./RNG.sol";
-import "./api/IStaking.sol";
-import "./api/IBonding.sol";
 import "./DynamicArray.sol";
+import "./api/IFullyBackedBonding.sol";
 
 /// @title Fully Backed Sortition Pool
 /// @notice A logarithmic data structure
@@ -37,6 +36,7 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
   // this value can be set to equal the most recent request's bondValue.
 
   struct PoolParams {
+    IFullyBackedBonding bondingContract;
     // Defines the minimum unbounded value the operator needs to have to be
     // eligible to join and stay in the sortition pool. Operators not
     // satisfying minimum bondable value are removed from the pool.
@@ -57,6 +57,7 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
   PoolParams poolParams;
 
   constructor(
+    IFullyBackedBonding _bondingContract,
     uint256 _initialMinimumBondableValue,
     uint256 _bondWeightDivisor,
     address _poolOwner
diff --git a/contracts/FullyBackedSortitionPoolFactory.sol b/contracts/FullyBackedSortitionPoolFactory.sol
index 80460eb8..6c1668db 100644
--- a/contracts/FullyBackedSortitionPoolFactory.sol
+++ b/contracts/FullyBackedSortitionPoolFactory.sol
@@ -1,19 +1,21 @@
 pragma solidity 0.5.17;
 
 import "./FullyBackedSortitionPool.sol";
-import "./api/IBonding.sol";
+import "./api/IFullyBackedBonding.sol";
 import "./api/IStaking.sol";
 
 /// @title Fully-Backed Sortition Pool Factory
 /// @notice Factory for the creation of fully-backed sortition pools.
 contract FullyBackedSortitionPoolFactory {
   /// @notice Creates a new fully-backed sortition pool instance.
+  /// @param bondingContract Fully Backed Bonding contract reference.
   /// @param minimumBondableValue Minimum unbonded value making the operator
   /// eligible to join the network.
   /// @param bondWeightDivisor Constant divisor for the available bond used to
   /// evalate the applicable weight.
   /// @return Address of the new fully-backed sortition pool contract instance.
   function createSortitionPool(
+    IFullyBackedBonding bondingContract,
     uint256 minimumBondableValue,
     uint256 bondWeightDivisor
   ) public returns (address) {
diff --git a/contracts/api/IFullyBackedBonding.sol b/contracts/api/IFullyBackedBonding.sol
new file mode 100644
index 00000000..2a6aa24b
--- /dev/null
+++ b/contracts/api/IFullyBackedBonding.sol
@@ -0,0 +1,19 @@
+pragma solidity 0.5.17;
+
+import "./IBonding.sol";
+
+/// @title Fully Backed Bonding contract interface.
+/// @notice The interface should be implemented by a bonding contract used for
+/// Fully Backed Sortition Pool.
+contract IFullyBackedBonding is IBonding {
+  /// @notice Checks if the operator for the given bond creator contract
+  /// has passed the initialization period.
+  /// @param operator The operator address.
+  /// @param bondCreator The bond creator contract address.
+  /// @return True if operator has passed initialization period for given
+  /// bond creator contract, false otherwise.
+  function isInitialized(address operator, address bondCreator)
+    public
+    view
+    returns (bool);
+}

From fe4c1b86b08c321e70ebfdc7a0021d03b8cb3770 Mon Sep 17 00:00:00 2001
From: Jakub Nowakowski <jakub.nowakowski@akena.co>
Date: Thu, 20 Aug 2020 08:11:10 +0200
Subject: [PATCH 4/8] Check if bonding delegation is initialized

When a bonding relationship gets delegated in the bonding contract it
has to pass some initialization period. In getEligibleWeight that is
invoked on operator registration in the pool we are checking if the
initialization period has passed. If not we're not letting the operator
to register in the pool.
---
 contracts/FullyBackedSortitionPool.sol | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/contracts/FullyBackedSortitionPool.sol b/contracts/FullyBackedSortitionPool.sol
index dbc9b8d0..18d7f49b 100644
--- a/contracts/FullyBackedSortitionPool.sol
+++ b/contracts/FullyBackedSortitionPool.sol
@@ -151,6 +151,17 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
       return 0;
     }
 
+    // Check if a bonding delegation is initialized.
+    bool isBondingInitialized = poolParams.bondingContract.isInitialized(
+      operator,
+      ownerAddress
+    );
+
+    // If a delegation is not yet initialized return 0 = ineligible.
+    if (!isBondingInitialized) {
+      return 0;
+    }
+
     // Weight = floor(eligibleStake / mimimumStake)
     // Ethereum uint256 division performs implicit floor
     return (bondableValue / poolParams.bondWeightDivisor);

From 1cac71ffbf8ad33a79a2c61061d0d68e270128bd Mon Sep 17 00:00:00 2001
From: Jakub Nowakowski <jakub.nowakowski@akena.co>
Date: Thu, 20 Aug 2020 08:16:30 +0200
Subject: [PATCH 5/8] Updated bonding contract stubs

Updated BondingContractStub to implement IBonding interface.
Added stub for FullyBackedBonding that implements IFullyBackedBonding
interface and extends BondingContractStub.
---
 test/contracts/BondingContractStub.sol    |  6 ++++--
 test/contracts/FullyBackedBondingStub.sol | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)
 create mode 100644 test/contracts/FullyBackedBondingStub.sol

diff --git a/test/contracts/BondingContractStub.sol b/test/contracts/BondingContractStub.sol
index 9c09654a..cbbe0273 100644
--- a/test/contracts/BondingContractStub.sol
+++ b/test/contracts/BondingContractStub.sol
@@ -1,7 +1,9 @@
 pragma solidity 0.5.17;
 
-contract BondingContractStub {
-    mapping(address => uint) unbondedValue;
+import "../../contracts/api/IBonding.sol";
+
+contract BondingContractStub is IBonding {
+    mapping(address => uint256) unbondedValue;
 
     function setBondableValue(address operator, uint256 value) public {
         unbondedValue[operator] = value;
diff --git a/test/contracts/FullyBackedBondingStub.sol b/test/contracts/FullyBackedBondingStub.sol
new file mode 100644
index 00000000..5cfe52f4
--- /dev/null
+++ b/test/contracts/FullyBackedBondingStub.sol
@@ -0,0 +1,19 @@
+pragma solidity 0.5.17;
+
+import "../../contracts/api/IFullyBackedBonding.sol";
+import "./BondingContractStub.sol";
+
+contract FullyBackedBondingStub is IFullyBackedBonding, BondingContractStub {
+  mapping(address => bool) initialized;
+
+  function setInitialized(address operator, bool value) public {
+    initialized[operator] = value;
+  }
+
+  function isInitialized(
+    address operator,
+    address // bondCreator
+  ) public view returns (bool) {
+    return initialized[operator];
+  }
+}

From e815c91851e702cac01b5f56410da547980bf4d4 Mon Sep 17 00:00:00 2001
From: Jakub Nowakowski <jakub.nowakowski@akena.co>
Date: Thu, 20 Aug 2020 08:23:42 +0200
Subject: [PATCH 6/8] Updated unit tests for Fully Backed Sortition Pools

fullyBackedFactoryTest.js:
- use correct bonding contract
- added awaits
- set operator initialization in bonding stub

fullyBackedSortitionPoolTest.js:
- use correct bonding contract
- added awaits
- set operator initialization in bonding stub
- use bn-chai to verify BN values
- use bond value instead of weighted value for operator preparation
- get operator initialiation period from contract and use it for blocks
mining
- updated tests for group selection
- added tests for minimum bondable value configuration
- added tests for joinPool (getEligibleWeight).
---
 package-lock.json                    |   6 +
 package.json                         |   2 +
 test/fullyBackedFactoryTest.js       |  17 +-
 test/fullyBackedSortitionPoolTest.js | 379 +++++++++++++++++++--------
 4 files changed, 290 insertions(+), 114 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index 4f9f89f1..a18b88e3 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1166,6 +1166,12 @@
       "integrity": "sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg==",
       "dev": true
     },
+    "bn-chai": {
+      "version": "1.0.1",
+      "resolved": "https://registry.npmjs.org/bn-chai/-/bn-chai-1.0.1.tgz",
+      "integrity": "sha512-7rJXt21DwYiLLpvzLaACixBBoUGkRV1iuFD3wElEhw8Ji9IiY/QsJRtvW+c7ChRgEOyLQkGaSGFUUqBKm21SNA==",
+      "dev": true
+    },
     "bn.js": {
       "version": "4.11.8",
       "resolved": "https://registry.npmjs.org/bn.js/-/bn.js-4.11.8.tgz",
diff --git a/package.json b/package.json
index ac06a422..f2d11b80 100644
--- a/package.json
+++ b/package.json
@@ -46,6 +46,8 @@
     "ethlint": "^1.2.5",
     "eth-gas-reporter": "^0.1.12",
     "ganache-cli": "^6.4.3",
+    "bn-chai": "^1.0.1",
+    "chai": "^4.2.0",
     "solc": "0.5.17",
     "solium": "^1.2.5",
     "solium-config-keep": "github:keep-network/solium-config-keep#0.1.2",
diff --git a/test/fullyBackedFactoryTest.js b/test/fullyBackedFactoryTest.js
index d9cecc42..26f17605 100644
--- a/test/fullyBackedFactoryTest.js
+++ b/test/fullyBackedFactoryTest.js
@@ -1,10 +1,8 @@
 const FullyBackedSortitionPoolFactory = artifacts.require(
-  "./contracts/FullyBackedSortitionPoolFactory.sol",
+  "FullyBackedSortitionPoolFactory",
 )
-const FullyBackedSortitionPool = artifacts.require(
-  "./contracts/FullyBackedSortitionPool.sol",
-)
-const BondingContractStub = artifacts.require("BondingContractStub.sol")
+const FullyBackedSortitionPool = artifacts.require("FullyBackedSortitionPool")
+const FullyBackedBondingStub = artifacts.require("FullyBackedBondingStub")
 
 contract("FullyBackedSortitionPoolFactory", (accounts) => {
   let bondingContract
@@ -15,7 +13,7 @@ contract("FullyBackedSortitionPoolFactory", (accounts) => {
 
   before(async () => {
     factory = await FullyBackedSortitionPoolFactory.deployed()
-    bondingContract = await BondingContractStub.new()
+    bondingContract = await FullyBackedBondingStub.new()
   })
 
   describe("createSortitionPool()", async () => {
@@ -46,8 +44,11 @@ contract("FullyBackedSortitionPoolFactory", (accounts) => {
 
       assert.notEqual(pool1Address, pool2Address)
 
-      bondingContract.setBondableValue(accounts[1], 100)
-      bondingContract.setBondableValue(accounts[2], 200)
+      await bondingContract.setBondableValue(accounts[1], 100)
+      await bondingContract.setBondableValue(accounts[2], 200)
+
+      await bondingContract.setInitialized(accounts[1], true)
+      await bondingContract.setInitialized(accounts[2], true)
 
       assert.equal(await pool1.operatorsInPool(), 0)
       assert.equal(await pool2.operatorsInPool(), 0)
diff --git a/test/fullyBackedSortitionPoolTest.js b/test/fullyBackedSortitionPoolTest.js
index ddeee2c8..dc0743bc 100644
--- a/test/fullyBackedSortitionPoolTest.js
+++ b/test/fullyBackedSortitionPoolTest.js
@@ -2,48 +2,61 @@ const Branch = artifacts.require("Branch")
 const Position = artifacts.require("Position")
 const StackLib = artifacts.require("StackLib")
 const Leaf = artifacts.require("Leaf")
-const FullyBackedSortitionPool = artifacts.require(
-  "./contracts/FullyBackedSortitionPool.sol",
-)
+const FullyBackedSortitionPool = artifacts.require("FullyBackedSortitionPool")
 
-const BondingContractStub = artifacts.require("BondingContractStub.sol")
+const FullyBackedBondingStub = artifacts.require("FullyBackedBondingStub")
 
 const {mineBlocks} = require("./mineBlocks")
 
 const {expectRevert} = require("@openzeppelin/test-helpers")
 
+const BN = web3.utils.BN
+
+const chai = require("chai")
+chai.use(require("bn-chai")(BN))
+const expect = chai.expect
+
 contract("FullyBackedSortitionPool", (accounts) => {
   const seed = "0xff39d6cca87853892d2854566e883008bc"
-  const bond = 2000
-  const weightDivisor = 1000
-  const alice = accounts[0]
-  const bob = accounts[1]
-  const carol = accounts[2]
-  const david = accounts[3]
-  const emily = accounts[4]
+
+  const weightDivisor = web3.utils.toBN(1000)
+  const minimumBondableValue = weightDivisor.muln(5)
+  const bond = minimumBondableValue.muln(2)
+
+  const alice = accounts[1]
+  const bob = accounts[2]
+  const carol = accounts[3]
+  const david = accounts[4]
+  const emily = accounts[5]
   const owner = accounts[9]
+
   let pool
   let bonding
   let prepareOperator
+  let operatorInitBlocks
 
   beforeEach(async () => {
-    FullyBackedSortitionPool.link(Branch)
-    FullyBackedSortitionPool.link(Position)
-    FullyBackedSortitionPool.link(StackLib)
-    FullyBackedSortitionPool.link(Leaf)
-    bonding = await BondingContractStub.new()
-
-    prepareOperator = async (address, weight) => {
-      await bonding.setBondableValue(address, weight * weightDivisor)
+    await FullyBackedSortitionPool.link(Branch)
+    await FullyBackedSortitionPool.link(Position)
+    await FullyBackedSortitionPool.link(StackLib)
+    await FullyBackedSortitionPool.link(Leaf)
+    bonding = await FullyBackedBondingStub.new()
+
+    prepareOperator = async (address, bondableValue) => {
+      await bonding.setBondableValue(address, bondableValue)
+      await bonding.setInitialized(address, true)
+
       await pool.joinPool(address)
     }
 
     pool = await FullyBackedSortitionPool.new(
       bonding.address,
-      bond,
+      minimumBondableValue,
       weightDivisor,
       owner,
     )
+
+    operatorInitBlocks = await pool.operatorInitBlocks()
   })
 
   describe("isOperatorInitialized", async () => {
@@ -55,7 +68,7 @@ contract("FullyBackedSortitionPool", (accounts) => {
     })
 
     it("returns false at the beginning of the initialization period", async () => {
-      await prepareOperator(alice, 10)
+      await prepareOperator(alice, bond)
 
       assert.isFalse(
         await pool.isOperatorInitialized(alice),
@@ -64,9 +77,9 @@ contract("FullyBackedSortitionPool", (accounts) => {
     })
 
     it("returns false when the initialization period is almost passed", async () => {
-      await prepareOperator(alice, 10)
+      await prepareOperator(alice, bond)
 
-      await mineBlocks(await pool.operatorInitBlocks())
+      await mineBlocks(operatorInitBlocks)
 
       assert.isFalse(
         await pool.isOperatorInitialized(alice),
@@ -75,9 +88,9 @@ contract("FullyBackedSortitionPool", (accounts) => {
     })
 
     it("returns true when initialization period passed", async () => {
-      await prepareOperator(alice, 10)
+      await prepareOperator(alice, bond)
 
-      await mineBlocks((await pool.operatorInitBlocks()).addn(1))
+      await mineBlocks(operatorInitBlocks.addn(1))
 
       assert.isTrue(await pool.isOperatorInitialized(alice))
     })
@@ -85,135 +98,289 @@ contract("FullyBackedSortitionPool", (accounts) => {
 
   describe("selectSetGroup", async () => {
     it("returns group of expected size with unique members", async () => {
-      await prepareOperator(alice, 10)
-      await prepareOperator(bob, 11)
-      await prepareOperator(carol, 12)
-      await prepareOperator(david, 13)
-      await prepareOperator(emily, 14)
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond)
+      await prepareOperator(carol, bond)
+      await prepareOperator(david, bond)
+      await prepareOperator(emily, bond)
 
-      await mineBlocks(11)
+      await mineBlocks(operatorInitBlocks.addn(1))
 
       let group
 
-      group = await pool.selectSetGroup.call(3, seed, bond, {from: owner})
-      await pool.selectSetGroup(3, seed, bond, {from: owner})
+      group = await pool.selectSetGroup.call(3, seed, minimumBondableValue, {
+        from: owner,
+      })
+      await pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner})
       assert.equal(group.length, 3)
       assert.isFalse(hasDuplicates(group))
 
-      group = await pool.selectSetGroup.call(5, seed, bond, {from: owner})
-      await pool.selectSetGroup(5, seed, bond, {from: owner})
+      group = await pool.selectSetGroup.call(5, seed, minimumBondableValue, {
+        from: owner,
+      })
+      await pool.selectSetGroup(5, seed, minimumBondableValue, {from: owner})
       assert.equal(group.length, 5)
       assert.isFalse(hasDuplicates(group))
     })
 
     it("updates operators' weight", async () => {
-      await prepareOperator(alice, 10)
-      await prepareOperator(bob, 11)
-      await prepareOperator(carol, 12)
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond.muln(2))
+      await prepareOperator(carol, bond.muln(3))
 
-      await mineBlocks(11)
+      await mineBlocks(operatorInitBlocks.addn(1))
 
-      assert.equal(await pool.getPoolWeight(alice), 10)
-      assert.equal(await pool.getPoolWeight(bob), 11)
-      assert.equal(await pool.getPoolWeight(carol), 12)
+      expect(await pool.getPoolWeight(alice)).to.eq.BN(bond.div(weightDivisor))
+      expect(await pool.getPoolWeight(bob)).to.eq.BN(
+        bond.muln(2).div(weightDivisor),
+      )
+      expect(await pool.getPoolWeight(carol)).to.eq.BN(
+        bond.muln(3).div(weightDivisor),
+      )
 
-      await pool.selectSetGroup(3, seed, bond, {from: owner})
+      await pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner})
 
-      assert.equal(await pool.getPoolWeight(alice), 8)
-      assert.equal(await pool.getPoolWeight(bob), 9)
-      assert.equal(await pool.getPoolWeight(carol), 10)
+      expect(await pool.getPoolWeight(alice)).to.eq.BN(
+        bond.sub(minimumBondableValue).div(weightDivisor),
+      )
+      expect(await pool.getPoolWeight(bob)).to.eq.BN(
+        bond.muln(2).sub(minimumBondableValue).div(weightDivisor),
+      )
+      expect(await pool.getPoolWeight(carol)).to.eq.BN(
+        bond.muln(3).sub(minimumBondableValue).div(weightDivisor),
+      )
     })
 
-    function hasDuplicates(array) {
-      return new Set(array).size !== array.length
-    }
-
     it("reverts when called by non-owner", async () => {
-      await prepareOperator(alice, 10)
-      await prepareOperator(bob, 11)
-      await prepareOperator(carol, 12)
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond)
+      await prepareOperator(carol, bond)
 
-      await mineBlocks(11)
+      await mineBlocks(operatorInitBlocks.addn(1))
 
-      try {
-        await pool.selectSetGroup(3, seed, bond, {from: alice})
-      } catch (error) {
-        assert.include(error.message, "Only owner may select groups")
-        return
-      }
-
-      assert.fail("Expected throw not received")
+      await expectRevert(
+        pool.selectSetGroup(3, seed, minimumBondableValue, {from: alice}),
+        "Only owner may select groups",
+      )
     })
 
     it("reverts when there are no operators in pool", async () => {
-      try {
-        await pool.selectSetGroup(3, seed, bond, {from: owner})
-      } catch (error) {
-        assert.include(error.message, "Not enough operators in pool")
-        return
-      }
-
-      assert.fail("Expected throw not received")
+      await expectRevert(
+        pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner}),
+        "Not enough operators in pool",
+      )
     })
 
     it("reverts when there are not enough operators in pool", async () => {
-      await prepareOperator(alice, 10)
-      await prepareOperator(bob, 11)
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond)
+
+      await mineBlocks(operatorInitBlocks.addn(1))
+
+      await expectRevert(
+        pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner}),
+        "Not enough operators in pool",
+      )
+    })
+
+    it("reverts when operator is not initialized in the sortition pool", async () => {
+      // Register two operators.
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond)
+
+      assert.equal(await pool.operatorsInPool(), 2)
+
+      // Initialization period not passed.
+      await expectRevert(
+        pool.selectSetGroup(2, seed, minimumBondableValue, {from: owner}),
+        "Not enough operators in pool",
+      )
 
-      await mineBlocks(11)
+      // Initialization period passed.
+      await mineBlocks(operatorInitBlocks.addn(1))
 
-      try {
-        await pool.selectSetGroup(3, seed, bond, {from: owner})
-      } catch (error) {
-        assert.include(error.message, "Not enough operators in pool")
-        return
-      }
+      await pool.selectSetGroup(2, seed, minimumBondableValue, {from: owner})
 
-      assert.fail("Expected throw not received")
+      // Register third operator.
+      await prepareOperator(carol, bond)
+
+      assert.equal(
+        await pool.operatorsInPool(),
+        3,
+        "incorrect number of operators after second registration",
+      )
+
+      await expectRevert(
+        pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner}),
+        "Not enough operators in pool",
+        "unexpected result for the third group selection",
+      )
+
+      await mineBlocks(operatorInitBlocks.addn(1))
+
+      await pool.selectSetGroup(3, seed, minimumBondableValue, {from: owner})
     })
 
-    it("removes ineligible operators and still works afterwards", async () => {
-      await prepareOperator(alice, 10)
-      await prepareOperator(bob, 11)
-      await prepareOperator(carol, 12)
-      await prepareOperator(david, 5)
+    it("removes minimum-bond-ineligible operators and still works afterwards", async () => {
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond)
+      await prepareOperator(carol, bond)
+      await prepareOperator(david, bond)
 
-      await mineBlocks(11)
+      await mineBlocks(operatorInitBlocks.addn(1))
 
-      await bonding.setBondableValue(carol, 1 * weightDivisor)
+      await bonding.setBondableValue(carol, minimumBondableValue.subn(1))
 
-      try {
-        await pool.selectSetGroup(4, seed, bond, {from: owner})
-      } catch (error) {
-        assert.include(error.message, "Not enough operators in pool")
+      // all 4 operators in the pool
+      assert.equal(await pool.operatorsInPool(), 4)
 
-        group = await pool.selectSetGroup.call(3, seed, bond, {from: owner})
+      // should select group and remove carol
+      await pool.selectSetGroup(3, seed, bond, {from: owner})
 
-        assert.equal(group.length, 3)
-        assert.isFalse(hasDuplicates(group))
+      assert.equal(await pool.operatorsInPool(), 3) // carol removed
 
-        return
-      }
+      // should have only 3 operators in the pool now
+      group = await pool.selectSetGroup.call(3, seed, bond, {
+        from: owner,
+      })
 
-      assert.fail("Expected throw not received")
+      assert.equal(group.length, 3)
+      assert.sameMembers(group, [alice, bob, david])
+      assert.equal(await pool.operatorsInPool(), 3)
     })
 
-    it("updates operators whose weight has increased", async () => {
-      await prepareOperator(alice, 10)
-      await prepareOperator(bob, 11)
-      await prepareOperator(carol, 12)
+    it("skips selection-bond-ineligible operators and still works afterwards", async () => {
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond.subn(1))
+      await prepareOperator(carol, bond)
+      await prepareOperator(david, bond)
 
-      await mineBlocks(11)
+      await mineBlocks(operatorInitBlocks.addn(1))
 
-      await bonding.setBondableValue(carol, 15 * weightDivisor)
+      // all 4 operators in the pool
+      assert.equal(await pool.operatorsInPool(), 4)
 
-      group = await pool.selectSetGroup.call(3, seed, bond, {from: owner})
+      // should select group and skip bob (do not remove it!)
       await pool.selectSetGroup(3, seed, bond, {from: owner})
+
+      // all 4 operators still in the pool
+      assert.equal(await pool.operatorsInPool(), 4)
+
+      group = await pool.selectSetGroup.call(
+        3,
+        seed,
+        minimumBondableValue.muln(2),
+        {
+          from: owner,
+        },
+      )
+
       assert.equal(group.length, 3)
+      assert.sameMembers(group, [alice, carol, david])
+      assert.equal(await pool.operatorsInPool(), 4)
+    })
+
+    it("updates operators whose weight has increased", async () => {
+      await prepareOperator(alice, bond)
+      await prepareOperator(bob, bond)
+      await prepareOperator(carol, bond)
+      await prepareOperator(david, bond)
+
+      await mineBlocks(operatorInitBlocks.addn(1))
+
+      // Insignificant changes less than the weight divisor, resulting in no weight update.
+      await bonding.setBondableValue(alice, bond.addn(1))
+      await bonding.setBondableValue(bob, bond.add(weightDivisor).subn(1))
+      // The first value resulting in the weight update.
+      await bonding.setBondableValue(carol, bond.add(weightDivisor))
+      // Value increasing the weight for couple units.
+      await bonding.setBondableValue(david, bond.add(weightDivisor.muln(7)))
+
+      group = await pool.selectSetGroup.call(4, seed, minimumBondableValue, {
+        from: owner,
+      })
+      await pool.selectSetGroup(4, seed, minimumBondableValue, {from: owner})
+      assert.equal(group.length, 4)
       assert.isFalse(hasDuplicates(group))
 
-      const postWeight = await pool.getPoolWeight(carol)
-      assert.equal(postWeight, 13)
+      expect(await pool.getPoolWeight(alice)).to.eq.BN(
+        bond.sub(minimumBondableValue).div(weightDivisor),
+      )
+      expect(await pool.getPoolWeight(bob)).to.eq.BN(
+        bond.sub(minimumBondableValue).div(weightDivisor),
+      )
+      expect(await pool.getPoolWeight(carol)).to.eq.BN(
+        bond.sub(minimumBondableValue).div(weightDivisor).addn(1),
+      )
+      expect(await pool.getPoolWeight(david)).to.eq.BN(
+        bond.sub(minimumBondableValue).div(weightDivisor).addn(7),
+      )
+    })
+
+    function hasDuplicates(array) {
+      return new Set(array).size !== array.length
+    }
+  })
+
+  describe("setMinimumBondableValue", async () => {
+    it("default value set in the pool constructor", async () => {
+      expect(await pool.getMinimumBondableValue()).to.eq.BN(
+        minimumBondableValue,
+      )
+    })
+
+    it("can only be called by the owner", async () => {
+      await expectRevert(
+        pool.setMinimumBondableValue(1, {from: accounts[0]}),
+        "Only owner may update minimum bond value",
+      )
+    })
+
+    it("updates the minimum bondable value", async () => {
+      await pool.setMinimumBondableValue(1, {from: owner})
+      expect(await pool.getMinimumBondableValue()).to.eq.BN(1)
+
+      await pool.setMinimumBondableValue(6, {from: owner})
+      expect(await pool.getMinimumBondableValue()).to.eq.BN(6)
+    })
+  })
+
+  describe("joinPool", async () => {
+    it("succeeds with minimum bond value", async () => {
+      await bonding.setBondableValue(alice, minimumBondableValue)
+      await bonding.setInitialized(alice, true)
+
+      await pool.joinPool(alice)
+
+      expect(await pool.getPoolWeight(alice)).to.eq.BN(
+        minimumBondableValue.div(weightDivisor),
+      )
+    })
+
+    it("fails with less than minimum bond value", async () => {
+      await bonding.setBondableValue(alice, minimumBondableValue.subn(1))
+      await bonding.setInitialized(alice, true)
+
+      await expectRevert(pool.joinPool(alice), "Operator not eligible")
+    })
+
+    it("reverts if the operator is not yet initialized in the bonding contract", async () => {
+      await bonding.setBondableValue(alice, minimumBondableValue)
+      await bonding.setInitialized(alice, false)
+
+      await expectRevert(pool.joinPool(alice), "Operator not eligible")
+    })
+
+    it("reverts if operator is already registered", async () => {
+      await bonding.setBondableValue(alice, minimumBondableValue)
+      await bonding.setInitialized(alice, true)
+
+      await pool.joinPool(alice)
+
+      await expectRevert(
+        pool.joinPool(alice),
+        "Operator is already registered in the pool",
+      )
     })
   })
 })

From eb2d54f9be30bb883f0117dc574411c9b7125c89 Mon Sep 17 00:00:00 2001
From: Promethea Raschke <promethea.raschke@keep.network>
Date: Tue, 13 Oct 2020 14:45:49 +0100
Subject: [PATCH 7/8] Increase Carol's weight in minimum bond test to improve
 reliability

Because operators are selected randomly,
there was a 1/4 possibility that Carol might not be picked,
causing the check for the removal of ineligible operators to fail.
By increasing Carol's initial bond to 100x that of the others,
the probability of Carol being skipped becomes roughly 1 in a million.
---
 test/fullyBackedSortitionPoolTest.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/fullyBackedSortitionPoolTest.js b/test/fullyBackedSortitionPoolTest.js
index dc0743bc..f29272f9 100644
--- a/test/fullyBackedSortitionPoolTest.js
+++ b/test/fullyBackedSortitionPoolTest.js
@@ -224,7 +224,7 @@ contract("FullyBackedSortitionPool", (accounts) => {
     it("removes minimum-bond-ineligible operators and still works afterwards", async () => {
       await prepareOperator(alice, bond)
       await prepareOperator(bob, bond)
-      await prepareOperator(carol, bond)
+      await prepareOperator(carol, bond.muln(100))
       await prepareOperator(david, bond)
 
       await mineBlocks(operatorInitBlocks.addn(1))

From 949fd0984e98edd5b40896492f94c98f1ba5080c Mon Sep 17 00:00:00 2001
From: Promethea Raschke <promethea.raschke@keep.network>
Date: Tue, 13 Oct 2020 15:25:34 +0100
Subject: [PATCH 8/8] Use correct bonding contract stub in fully backed tests

BondingContractStub doesn't implement initialization status functions.
This caused the work selection distribution tests to fail.
Switch to FullyBackedBondingStub
and set initialization status when preparing operators.
---
 test/workDistributionTest.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/workDistributionTest.js b/test/workDistributionTest.js
index 0ab5350f..ed1a2f43 100644
--- a/test/workDistributionTest.js
+++ b/test/workDistributionTest.js
@@ -11,6 +11,7 @@ const FullyBackedSortitionPool = artifacts.require(
 
 const StakingContractStub = artifacts.require("StakingContractStub.sol")
 const BondingContractStub = artifacts.require("BondingContractStub.sol")
+const FullyBackedBondingStub = artifacts.require("FullyBackedBondingStub.sol")
 
 const {mineBlocks} = require("./mineBlocks")
 
@@ -219,7 +220,7 @@ contract("FullyBackedSortitionPool", (accounts) => {
     FullyBackedSortitionPool.link(Position)
     FullyBackedSortitionPool.link(StackLib)
     FullyBackedSortitionPool.link(Leaf)
-    keepBonding = await BondingContractStub.new()
+    keepBonding = await FullyBackedBondingStub.new()
 
     pool = await FullyBackedSortitionPool.new(
       keepBonding.address,
@@ -307,6 +308,7 @@ contract("FullyBackedSortitionPool", (accounts) => {
       address,
       web3.utils.toBN(bond).mul(tokenDecimalMultiplier),
     )
+    await keepBonding.setInitialized(address, true)
     await pool.joinPool(address)
   }