New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Partial Unbonding #5

Closed
dob opened this Issue May 20, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@dob
Copy link
Member

dob commented May 20, 2018

LIP: <to be assigned>
Title: Enable Partial Unbonding
Author: Doug Petkanics (@dob)
Type: Standard Track
Status: Draft
Created: 2018-05-20

Abstract

The BondingManager contract currently exposes an unbond() function which places the calling delegator into the Unbonding state, and subtracts all of their delegated stake from the transcoder. This proposal aims to allow partial unbonding, such that a delegator or transcoder can call unbond() to place a portion of their stake into the Unbonding state, but retain a portion as delegated.

Motivation

The weaknesses of the current all-or-nothing approach to unbonding are as follows:

  • There is no opportunity for transcoders to decrease their staked amount of LPT while still having a portion remained staked.
  • There is very little opportunity to prioritize decentralization, by bonding towards multiple transcoders, because doing so requires waiting the full unbonding period, splitting LPT into multiple accounts, and then rebonding. The lack of a partial unbonding period works against decentralization and leads to stagnation.
  • Community nodes who specify that they would like to use a portion of the inflationary LPT in order to grow the community or incentivize development have no means for accessing the LPT without fully resigning their position as a transcoder.

This proposal would eliminate all of these weaknesses.

The potential weakness is that it will now be easier to constantly withdraw LPT from the bonded state, and therefore it’s possible that there will be less participating stake. This shouldn’t be a concern however because it just creates more opportunity for those who remain bonded to grow their stake.

Specification

Note: this is the start of the discussion so I leave out many of the second order updates that will need to be made until we can discuss whether this approach is reasonable. Examples include calculating the delegator status and how those status' are used, and any logic related to calculating unbonding and withdrawl amounts.

In the BondingManager contract, change

function unbond() to function unbond(uint amount)

Currently the fact that a delegator is unbonding is tracked via the withdrawRound attribute on the delegator struct. I propose that there is a new:

struct Unbonding {
  uint amount;
  uint withdrawBlock;
}

and that the delegator struct removes withdrawRound and adds

uint unbondingCount.  // count that can be used to index into unbonds
Unbonding[] unbonds. // array of all the Unbonding structs

The unbond(amount) function would retain the same logic for removing a node from the transcoder pool if the amount unbonded was the total amount bonded, however in the case where this was a partial unbonding, they would just subtract the amount from the current bonded amount and transcoder’s delegated amount, and insert a new instance of Unbonding into the unbonds array.

  • The withdrawStake() function would be updated to withdrawStake(uint index) to index into the unbonds array. If the withdrawBlock has passed then allow the withdrawl, set amount = 0 and withdrawlBlock = 0.

  • Add a rebond(uint index) function, which re-applies the amount of the Unbonding struct at index in the unbonds array. Set amount and withdrawBlock to 0 to indicate that this Unbonding element is no longer active.

Specification Rationale

This design means that:

  • Anyone can expect all of the unbonding states for a delegator. It may be slightly inefficient to loop through the full array, but these are reads, and optimizations can probably be made to start at the end and loop backwards.
  • Delegators themselves can take responsibility for keeping track of their unbonds and withdrawing when they are ready 1-by-1.
  • There is not significant overhead as far as accounting. We still only allow one transcoder per address, however users can still unbond partially, split the LPT into different accounts, and bond towards other transcoders from that account, or transfer the LPT where they wish.
  • Delegators can still rebond freely if they change their mind.

I need a little more feedback on solidity implementation details to know if the nested array of structs is a good idea within the delegator struct.

I also thought about an UnbondingManager that could manage the full unbonding states separately, but then I thought it would need access to all the BondingManager state so it could update transcoder delegated stake.

Backwards Compatibility

This is a breaking change to the protocol. Clients will need to update in order to actually unbond.

Another option would be to add an overloaded unbond(amount) method, and to update the logic of unbond() to call unbond(amount) with the full staked amount. Similar with withdraw() and withdraw(index).

Implementation

TBD

Copyright

Copyright and related rights waived via CC0.

@yondonfu

This comment has been minimized.

Copy link
Member

yondonfu commented May 22, 2018

Here are some notes and additional thoughts from an offline discussion.

I'd be in favor of using a different name for the struct - in the interest of clarity and to make terminology clearer in discussions, maybe the name should be a noun such as UnbondingLock (or maybe even just Lock?). Once currentRound >= withdrawRound for a given unbonding lock, the lock is unlocked and can be used to withdraw the amount of tokens associated with the lock. I'll refer to the structs as unbonding locks in this comment, but feel free to point out if there is a better name.

Another way to manage the locks is to use a mapping instead of an array. The updated data structures would then be the following:

struct UnbondingLock {
    uint256 amount;
    uint256 withdrawRound;
}

struct Delegator {
    ...
    uint256 totalUnbondingLocks;
    mapping (uint256 => UnbondingLock) unbondingLocks;
}

Whenever a new unbonding lock is created totalUnbondingLocks would be incremented and the new lock would be assigned totalUnbondingLocks - 1as its ID.

A benefit in using a mapping instead of an array is that it allows the addition of additional variables to the UnbondingLock struct in a future upgrade if necessary. Variables of structs that are used in mappings can be modified during a delegate proxy upgrade (i.e. deploy a new target contract that the proxy contract will rely on for business logic). However, variables of structs that are used in arrays cannot be modified during a delegate proxy upgrade due to the way elements of an array are addressed in the EVM (an upgraded contract could potentially have variables of a struct in an array point to storage slots corresponding to variables of a previous struct in the array)

We should also update the Unbond and WithdrawStake stake events to

event Unbond(address indexed delegate, address indexed delegator, uint256 amount, uint256 withdrawBlock)
event WithdrawStake(address indexed delegator, uint256 unbondingLockId)

Clients will be able to use these events to keep track of unbonding locks for users. There is room for client-side optimizations here to help a user manage a set of unbonding locks.

An implication of this design is that there would no longer be any notion of a delegator being in the Unbonding state. The rest of the protocol states for a delegator will still apply such as Bonded, Unbonded and Pending. Instead, a bonded delegator can have zero, one or many unbonding locks. A delegator transitions from the Bonded state to the Unbonded state when its bondedAmount = 0. At this point, the delegator may have a non-zero number of unbonding locks which can be unlocked and used to withdraw tokens in the future without any dependence on the delegator being in a Bonded state. An additional change here is that whenever a delegator calls unbond(amount), amount should be subtracted from its bondedAmount - we can do this because the unbonding lock that is created simultaneously will track the amount that will be withdrawn in the future.

One observation made offline is that the concept of an unbonding lock can also be thought of as a contract issuing a single non-fungible token that has some denomination of LPT associated with it in addition to a withdraw round that dictates when the non-fungible token can be traded in for fungible LPT in the future. Let's call these non-fungible tokens BLPT (bonded LPT). We can take this idea one step further - the entire bonding process could be represented as delegators trading in quantities of fungible LPT into BLPT. Instead of delegators transitioning between the Bonded, Pending and Unbonded protocol states, each BLPT would transition between these states. When a BLPT is created, it is in the Bonded state, when it is unbonded it is in the Unbonding state and when it is traded int for LPT (a withdrawal) it is destroyed. While this idea is interesting, at first glance it could lead to a lot of management overhead which could be compared to the management tasks that Bitcoin clients have to do with UTXO sets. Consequently, I'll just leave this as an observation that could be interesting to discuss separately, but will leave it as a topic outside of the scope of this issue.

@dob

This comment has been minimized.

Copy link
Member

dob commented May 22, 2018

Once currentRound <= withdrawRound for a given unbonding lock, the lock is unlocked and can be used to withdraw the amount of tokens associated with the lock.

I think it's when currentRound >= withdrawRound that the lock gets unlocked.

A delegator transitions from the Bonded state to the Unbonded state when it has 0 unbonding locks and its bonded amount 0. If the delegator has 0 unbonding locks and its bonded amount > 0, it is still Bonded.

I wonder if a delegator should be Unbonded when it has 0 delegated stake, even if it has one or many Unbonding Locks? I wouldn't want the protocol to have to loop over every single unbonding lock to determine if the delegator is unbonded or not. And I actually don't think it matters whether they have any locks or not - as far as the protocol is concerned they can always withdraw from the locks separately at any time, but they are not actually bonded to anyone just because a few locks exist.

I guess this would only matter if we actually remove them from the delegators mapping when we believe them to be unbonded. We would likely have to keep them around so that they could access their unbonding locks to withdraw.

@yondonfu

This comment has been minimized.

Copy link
Member

yondonfu commented May 22, 2018

I think it's when currentRound >= withdrawRound that the lock gets unlocked.

Yep - fixed.

I wonder if a delegator should be Unbonded when it has 0 delegated stake, even if it has one or many Unbonding Locks?

Yeah I think the original state transition logic I described was wrong. Updated the description.

@yondonfu

This comment has been minimized.

Copy link
Member

yondonfu commented Jun 27, 2018

Closed by #8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment