Minimum bondable value for pool and bond value for the current selection #84
Conversation
BondedSortitionPool 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 BondedSortitionPool 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.
pdyraga
force-pushed
the
constant-minimum-bond
branch
from
2dc5753 to
46d187a
Compare
|
The next steps are:
But before I go any further, I'd love to have your opinion on this change @eth-r. |
contracts/BondedSortitionPool.sol
Outdated
| uint256 minimumBondableValue; | ||
| // 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 bond value are removed from the poool. |
There was a problem hiding this comment.
| // satisfying minimum bond value are removed from the poool. | |
| // satisfying minimum bond value are removed from the pool. |
contracts/BondedSortitionPool.sol
Outdated
| // 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 selectionBond; |
There was a problem hiding this comment.
| uint256 selectionBond; | |
| uint256 requiredBond; |
There was a problem hiding this comment.
I am not sure about it. minimumBond is also a required bond. What if we do: minimumBond -> minimumBondableValue and leave selectionBond as-is since it's a bond required for the current selection?
There was a problem hiding this comment.
Please see d09133c. I have also cleaned up all comments to make the distincion about minimumBondableValue and selectionBond clear and to use minimum bondable value consistently.
There was a problem hiding this comment.
I am not sure about it.
minimumBondis also a required bond.
You're right. I should have proposed requestedBond. If you think selectionBond is better than requestedBond I'm fine with leaving it as is.
There was a problem hiding this comment.
I think they are equally fine but if you prefer requestedBond it means we do have a preference and I'll rename it 😉
There was a problem hiding this comment.
In the beginning selectionBond confused me for a while. I believe requestedBond would be easier to understand.
contracts/BondedSortitionPool.sol
Outdated
| @@ -54,6 +60,7 @@ contract BondedSortitionPool is AbstractSortitionPool { | |||
| _minimumStake, | |||
| _bondingContract, | |||
| _minimumBondableValue, | |||
| _minimumBondableValue, | |||
There was a problem hiding this comment.
Can't we leave selectionBond with the default value 0?
There was a problem hiding this comment.
There should be no technical reason we couldn't.
Cleared docs to use minimum bondable value term consistently. minimumBondableValue is required from all operators in the pool, and it is a "global" requirement. selectionBond is the bond required for the current selection and it is a "local" requirement.
This values does not matter until the first selection happens but setting it to the default value instead of minimumBondableValue like before may help to avoid confusion.
During the review, we realized the selectionBond term can be a little confusing and requestedBond is a little better.
| poolParams.minimumStake = currentMinimumStake; | ||
| if (params.minimumStake != minimumStake) { | ||
| params.minimumStake = minimumStake; | ||
| poolParams.minimumStake = minimumStake; | ||
| } |
There was a problem hiding this comment.
Do we want to add a check that requestedBond >= minimumBondableValue?
There was a problem hiding this comment.
No, because we may expect someone to have 100 ETH available (only big fishes allowed in the system) but we still want to allow for 1 ETH-backed deposits.
|
Just to confirm, we're not updating |
We do but I'd propose doing it in a separate PR when working on ETH-only staking. |
Allow the application to set pool's minimum bondable value `BondedSortitionPool` allows specifying a global minimum bondable value the operator needs to have so that it can join and stay in the pool. The responsibility for setting it to some reasonable value goes to the application. We now have two minimum bond values in the sortition pool (see keep-network/sortition-pools#84): 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. Sortition pool is created with a minimum bond of 20 ETH to avoid small operators joining and griefing future selections before the minimum bond is set to the right value by the application. Anyone can create a sortition pool for an application with the default minimum bond value but the application can change this value later, at any point. 20 ETH per operator should be more than enough to cover 1 BTC deposit in 3-operators group.
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.
Refs: keep-network/keep-ecdsa#506
BondedSortitionPoolwas updating the minimum bondable value for eachselectSetGroupcall. 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
BondedSortitionPoolcreation 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.