Merge pull request #89 from keep-network/fully-backed-tweaks

Fully Backed Sortition Pools tweaks

We removed interfaces for SortitionPools that were not
implemented by the pools, so they were outdated. The
interfaces are not being used in keep-ecdsa anyways as
we're using the implementation of the contract.

This PR contains also a couple of tweaks to Fully Backed
Sortition Pools:
- added minimum bondable value and current selection as 
we already did in Bonded Sortition Pools (#84),
- defined IFullyBackedBonding interface that should be 
implemented by bonding contract dedicated for
 FullyBackedSortitionPool usage
- the IFullyBackedBonding interface defines an isInitialized 
function to check if the operator delegation in a bonding 
contract has passed initialization period,
- we check if the bonding initialization period has passed if
not don't let the operator register in the pool,
- updated existing unit tests and added new ones.

Author: pdyraga

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
@@ -24,6 +23,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 +36,15 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
   // this value can be set to equal the most recent request's bondValue.
 
   struct PoolParams {
-    IBonding bondingContract;
-    uint256 minimumAvailableBond;
+    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.
+    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,16 +57,17 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
   PoolParams poolParams;
 
   constructor(
-    IBonding _bondingContract,
-    uint256 _initialMinimumStake,
+    IFullyBackedBonding _bondingContract,
+    uint256 _initialMinimumBondableValue,
     uint256 _bondWeightDivisor,
     address _poolOwner
   ) public {
     require(_bondWeightDivisor > 0, "Weight divisor must be nonzero");
 
     poolParams = PoolParams(
       _bondingContract,
-      _initialMinimumStake,
+      _initialMinimumBondableValue,
+      0,
       _bondWeightDivisor,
       _poolOwner
     );
@@ -89,15 +98,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 +147,23 @@ contract FullyBackedSortitionPool is AbstractSortitionPool {
     );
 
     // Don't query stake if bond is insufficient.
-    if (bondableValue < poolParams.minimumAvailableBond) {
+    if (bondableValue < poolParams.minimumBondableValue) {
+      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
-    // If eligibleStake < minimumStake, return 0 = ineligible.
     return (bondableValue / poolParams.bondWeightDivisor);
   }
 
@@ -155,14 +194,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.

contracts/FullyBackedSortitionPoolFactory.sol

@@ -1,29 +1,29 @@
 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 Keep Bonding contract reference.
-  /// @param minimumStake Minimum stake value making the operator eligible to
-  /// join the network.
+  /// @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(
-    IBonding bondingContract,
-    uint256 minimumStake,
+    IFullyBackedBonding bondingContract,
+    uint256 minimumBondableValue,
     uint256 bondWeightDivisor
   ) public returns (address) {
     return
       address(
         new FullyBackedSortitionPool(
           bondingContract,
-          minimumStake,
+          minimumBondableValue,
           bondWeightDivisor,
           msg.sender
         )

contracts/api/IBondedSortitionPool.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);
+}

contracts/api/IFullyBackedBonding.sol

@@ -47,6 +47,8 @@
     "eslint": "^6.8.0",
     "eslint-config-keep": "github:keep-network/eslint-config-keep#0.3.0",
     "eth-gas-reporter": "^0.1.12",
+    "bn-chai": "^1.0.1",
+    "chai": "^4.2.0",
     "ethlint": "^1.2.5",
     "prettier": "^2.0.2",
     "prettier-plugin-solidity": "^1.0.0-alpha.47",

contracts/api/ISortitionPool.sol

@@ -1,17 +1,19 @@
 pragma solidity 0.5.17;
 
-contract BondingContractStub {
-    mapping(address => uint) public unbondedValue;
+import "../../contracts/api/IBonding.sol";
 
-    function setBondableValue(address operator, uint256 value) public {
-        unbondedValue[operator] = value;
-    }
+contract BondingContractStub is IBonding {
+  mapping(address => uint256) public unbondedValue;
 
-    function availableUnbondedValue(
-        address operator,
-        address, // bondCreator,
-        address // additionalAuthorizedContract
-    ) external view returns (uint256) {
-        return unbondedValue[operator];
-    }
+  function setBondableValue(address operator, uint256 value) public {
+    unbondedValue[operator] = value;
+  }
+
+  function availableUnbondedValue(
+    address operator,
+    address, // bondCreator,
+    address // additionalAuthorizedContract
+  ) external view returns (uint256) {
+    return unbondedValue[operator];
+  }
 }

package-lock.json

@@ -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];
+  }
+}

package.json

@@ -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)