Closed dob closed 6 years ago
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 - 1
as 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.
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.
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.
Closed by #8
Abstract
The
BondingManager
contract currently exposes anunbond()
function which places the calling delegator into theUnbonding
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 callunbond()
to place a portion of their stake into theUnbonding
state, but retain a portion as delegated.Motivation
The weaknesses of the current all-or-nothing approach to unbonding are as follows:
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, changefunction unbond()
tofunction 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:and that the delegator struct removes
withdrawRound
and addsThe
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 theamount
from the current bonded amount and transcoder’s delegated amount, and insert a new instance ofUnbonding
into theunbonds
array.The
withdrawStake()
function would be updated towithdrawStake(uint index)
to index into theunbonds
array. If thewithdrawBlock
has passed then allow the withdrawl, setamount = 0
andwithdrawlBlock = 0
.Add a
rebond(uint index)
function, which re-applies theamount
of theUnbonding
struct atindex
in theunbonds
array. Setamount
andwithdrawBlock
to0
to indicate that thisUnbonding
element is no longer active.Specification Rationale
This design means that:
unbonds
andwithdrawing
when they are ready 1-by-1.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 ofunbond()
to callunbond(amount)
with the full staked amount. Similar withwithdraw()
andwithdraw(index)
.Implementation
TBD
Copyright
Copyright and related rights waived via CC0.