Skip to content

Conversation

@eth-r
Copy link
Contributor

@eth-r eth-r commented Dec 1, 2021

Authorizations can be changed at various times and the sortition pool may be locked. Add data structures and functions for handling some of these cases by queuing updates when the pool is locked and processing the queue automatically when later updates are made.

Authorizations can be changed at various times and the sortition pool may be
locked. Add data structures and functions for handling some of these cases by
queuing updates when the pool is locked and processing the queue automatically
when later updates are made.

/// @notice Initializes the sortitionPool parameter. Can be performed only once.
/// @param _sortitionPool Value of the parameter.
function initSortitionPool(Data storage self, SortitionPool _sortitionPool)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider passing sortition pool as a parameter. This could let us save on the contract size of RandomBeacon.

struct Queue {
uint128 length;
uint128 start;
mapping(uint256 => Update) updates;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we do the following:

  • For RandonBeacon.authorizationIncreased, if the sortition pool is locked, revert transaction.
  • For RandomBeacon.approveAuthorizationDecrease, if the sortition pool is locked, revert transaction.

I think that if we document it right and address it in the UI by first checking the state of the sortition pool, this is acceptable.

Then:

  • For RandomBeacon.involuntaryAuthorizationDecrease, if the sortition pool is locked, we registered the involuntary authorization decrease request as pending and give the operator N seconds/blocks (N should be governable) to process this request. If they do not, anyone can process it and operator's stake will be seized (seize amount should be governable).

This way, we do not need another pool of rewards because it's in the best interest of the operator to process the pending decrease request and if they don't, the notifier receives the reward from the operator's stake.

// solhint-disable-next-line not-rely-on-time
uint64(block.timestamp)
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say we are creating a new group every month and that the group lifetime is 2 months. Say we have group a active and groups b will soon be created. Let's say that the operator requests authorization decrease at x moment of time.

|---------a-----------|
   x      |---------b----------|
                     |---------c-----------|

If we do not remove operator from the sortition pool at x, it may happen that the operator gets selected to group b and they might be able to approve authorization decrease before b ends its life. This is not ideal and it generally feels wrong not to remove the operator from the sortition pool immediately if we know that operator will be unwinding their node.

If we remove the operator from the pool at x they stop earning rewards and given that the authorization decrease delay needs to be long enough given the long group lifetime, this situation is less than ideal.

We should rework rewards so that it is possible to remove someone from the pool while still giving them a chance to earn rewards. But it still does not solve all problems.

We need to find a way to have the operator call RandomBeacon.approveAuthorizationDecrease function as soon as their authorization decrease delay ends to stop them from earning rewards for nothing.

One option I see is to reduce their reward weight drastically. They would still earn rewards but these rewards would be small and it would make sense to exit with the stake (capital) as soon as possible to have better ROI somewhere else. This strategy though is not very friendly for deauthorizing operators and I imagine details could be tricky - is this ROI small enough given this is risk-free money?

Another option I see is to let them earn rewards with their current weight but capture the expected authorization decrease approval timestamp and if they exceeded timeout, slash them proportionally to the delay. It is something we can deal with inside Authorizations library but the disadvantage of this approach is that it makes sense only when the slashed amount is higher than what they have earned during the time after the timeout.

Yet one option - and so far my favorite one - would be to make SortitionPool.withdrawRewards onlyOwner and proxy it through RandomBeacon contract. This will let us eliminate IStaking from SortitionPool and keep dependency graph clean: TokenStaking <-> RandomBeacon <-> SortitionPool. Moreover, RandomBeacon would be able to reduce the amount of reward withdrawn proportionally to the delay and send this amount back to the sortition pool or to some governable reward funder account.

I was also considering strategies where we capture eligibleUntil next to ineligibleUntil but this complicates Rewards given we do not have enough space in OperatorRewards struct and I think would require some untrivial work in Rewards contract.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strategies above describe situations when someone is decreasing authorization to 0, but we may also have a situation when someone is deauthorizing to non-0 value. I think in this scenario, as long as the value to which we are decreasing is above the current minimum authorization amount, we should decrease the weight in the sortition pool and weight for rewards immediately and let them approve authorization decrease at any time after the delay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|---------a-----------|
   ex     |---------b----------|
                     |---------c-----------|

One more tricky situation I see is when someone enters the pool at e and then requests authorization decrease at x. They are removed from the pool but they still earn rewards. There was no chance they could be selected for work so the rewards are risk-free.

I think we should have some minimum time before which authorization decrease request is not possible. This time could be governable and equal to the delay between DKGs.

Add staking stub implementing `IStaking` functionality, and fix obvious issues.
Tests currently can't be run because of `RandomBeaconStub` contract size.
@pdyraga
Copy link
Member

pdyraga commented Apr 6, 2022

Replaced by #2903.

@pdyraga pdyraga closed this Apr 6, 2022
@pdyraga pdyraga deleted the auth-sync branch April 6, 2022 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants