diff --git a/contracts/FullyBackedSortitionPool.sol b/contracts/FullyBackedSortitionPool.sol index 28108ddc..18d7f49b 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 @@ -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,8 +57,8 @@ contract FullyBackedSortitionPool is AbstractSortitionPool { PoolParams poolParams; constructor( - IBonding _bondingContract, - uint256 _initialMinimumStake, + IFullyBackedBonding _bondingContract, + uint256 _initialMinimumBondableValue, uint256 _bondWeightDivisor, address _poolOwner ) public { @@ -58,7 +66,8 @@ contract FullyBackedSortitionPool is AbstractSortitionPool { 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. diff --git a/contracts/FullyBackedSortitionPoolFactory.sol b/contracts/FullyBackedSortitionPoolFactory.sol index c561ae58..6c1668db 100644 --- a/contracts/FullyBackedSortitionPoolFactory.sol +++ b/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 ) 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/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); +} 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); -} diff --git a/package-lock.json b/package-lock.json index 8b472339..c47d9ece 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1921,6 +1921,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 52d02add..2201cee2 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/test/contracts/BondingContractStub.sol b/test/contracts/BondingContractStub.sol index 6bd4df1c..30cad103 100644 --- a/test/contracts/BondingContractStub.sol +++ b/test/contracts/BondingContractStub.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]; + } } 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]; + } +} 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..f29272f9 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.muln(100)) + 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", + ) }) }) }) 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) }