livepeer / LIPs

Livepeer Improvement Proposals
9 stars 13 forks source link

Partial unbonding #8

Closed dob closed 6 years ago

dob commented 6 years ago

Allow users to partially unbond from a transcoder, without having to unbond their full staked amount.

yondonfu commented 6 years ago

One scenario that needs to be addressed is when a transcoder is slashed while having a non-zero number of unbonding locks. At the moment, a transcoder will be penalized based upon its remaining bonded amount, but not the amounts tracked by its unbonding locks.

For example:

Alice is a transcoder with bonded amount of 100. She calls BondingManager.unbond() once to unbond 50 and another time to unbond 25. As a result, she now has a bonded amount of 25, one unbonding lock with 50 and one unbonding lock with 25. Alice then double claims a segment and is slashed. Lets say the slashing percentage for a double claim is 40%. Alice should lose 40% of 100 because her slashable stake should include both her bonded amount and any amounts tracked by unbonding locks that have not been used to withdraw yet. Instead, Alice only loses 40% of 25 = 10 because the slashing penalty is only calculated based on her bonded amount and not the amounts tracked by her unbonding locks.

We can make sure transcoders with a non-zero number of unbonding locks are properly penalized when slashed by introducing a slashedPercAmount field:

struct Transcoder {
     ...
    uint256 slashedPercAmount;
}

Under normal circumstances, slashedPercAmount = 0. If a transcoder is slashed using the function BondingManager.slashTranscoder(_transcoder, _finder, _slashAmount, _finderFee), the value of slashedPercAmount will be set to _slashAmount (which reflects the slashing percentage penalty for either double segment claims, missed verification or failed verification). BondingManager.slashTranscoder() will still apply the slashing penalty to whatever the transcoder's bonded amount is at the time of the function call.

Whenever, the transcoder calls BondingManager.withdrawStake() or BondingManager.rebond(), the contract will check if the caller is a transcoder with a non-zero slashedPercAmount. If slashedPercAmount is non-zero, then the stored value will be used to apply the slashing penalty to the amount tracked by the relevant unbonding lock. The remaining amount after applying the slashing penalty will be the withdrawn in the case of BondingManager.withdrawStake() and rebonded (added to the caller's bonded amount) in the case of BondingManager.rebond().

We can run through the original example scenario with this new scheme:

Alice is a transcoder with bonded amount of 100. She calls BondingManager.unbond() once to unbond 50 and another time to unbond 25. As a result, she now has a bonded amount of 25, one unbonding lock with 50 and one unbonding lock with 25. Alice then double claims a segment and is slashed. Lets say the slashing percentage for a double claim is 40%. The slashing penalty for Alice's bonded amount is 40% of 25 = 10. Alice then withdraws using the unbonding lock with an amount of 50 - the slashing penalty for this unbonding lock amount is 40% of 50 = 20 and Alice withdraws the remaining 30. Alice then rebonds using the unbonding lock with an amount of 25 - the slashing penalty for this unbonding lock amount is 40% of 25 = 10 and the remaining 15 is added back to Alice's bonded amount. The total slashing penalty across Alice's bonded amount and her unbonding locks is thus 10 + 20 + 10 = 40 which is also 40% of 100 (Alice's total slashable stake).

A remaining question with this scheme is when to set slashedPercAmount back to 0 for a transcoder? One option is to keep track of the number of unbonding locks and when that number is 0, slashedPercAmount can be set to 0 because that means the slashing penalty has been applied to all unbonding locks. With this option, it might be necessary to also disallow any further bonding actions from the relevant account until all unbonding locks are used to either withdraw or rebond. Another option is to replace slashedPercAmount with a mapping and add a creationBlock field to UnbondingLock:

slashedPercAtBlock[blockNumber] = 0 if the transcoder was not slashed in that block. If slashedPercAtBlock[blockNumber] > 0, the transcoder was slashed in that block. If unbondingLock.creationBlock <= slashedPercAtBlock[blockNumber] && slashedPercAtBlock[blockNumber] > 0 then the slashing penalty stored as slashedPercAtBlock[blockNumber] should be applied to the unbonding lock amount when withdrawing or rebonding.

EDIT: Mistyped - the last solution doesn't work, need to rethink

yondonfu commented 6 years ago

Suppose we make the following assumptions:

  1. A delegator's unbonding lock is only subject to slashing penalties for the delegator's transcoder at the time of the unbonding lock's creation
  2. A delegator's unbonding lock is only subject to slashing penalties incurred by a transcoder during the unbonding period aka from the lock's creation round up until the lock's withdraw round
  3. We are also using the delegator slashing mechanism described in https://github.com/livepeer/LIPs/issues/10#issuecomment-398893934

1 could be an acceptable assumption because the tokens associated with the lock amount would never have contributed to the total stake of another transcoder after its creation.

2 could be an acceptable assumption because the protocol could consider the tokens associated with a lock amount after the unbonding period as unbonded and not subject to slashing penalties unless they are rebonded later on.

struct UnbondingLock {
    uint256 amount;
    uint256 creationRound;
    uint256 withdrawRound;
    address delegateAddress;
}

We make sure unbonding locks keep track of the delegator's delegate at the time of creation in addition to the creation round and withdraw round.

The implementation (unoptimized) for BondingManager#withdrawStake() might look like this:

function withdrawStake(uint256 _unbondingLockId) external whenSystemNotPaused currentRoundInitialized {
    ...

    UnbondingLock storage lock = del.unbondingLocks[_unbondingLockId];

    // If the delegator was slashed during `lock.creationRound` we know that slashing penalties were properly accounted for
    // during that round so we can start from `lock.creationRound + 1`
    // If the delegator was not slashed during `lock.creationRound` there is a possibility that slashing penalties were not
    // all properly accounted for during that round, so we start from `lock.creationRound` and make sure slashing penalties
    // from that round are accounted for first
    uint256 firstRound = lock.creationRound == del.lastSlashedRound ? lock.creationRound + 1 : lock.creationRound
    uint256 currentPenalty = 0;

    for (uint256 i = firstRound; i < lock.withdrawRound; i++) {
        uint256 transcoderSlashedPerc = transcoders[lock.delegateAddress].slashedPercInRound[i];

        if (transcoderSlashedPerc > 0) {
            uint256 penalty = calcPenalty(lock.amount.sub(currentPenalty), transcoderSlashedPerc);

            currentPenalty = currentPenalty.add(penalty);
        }
    }

    // Burn penalty
    if (currentPenalty > 0) {
        minter().trustedBurnTokens(currentPenalty);
    }

    // Transfer remaining lock amount to caller
    minter().trustedTransferTokens(msg.sender, lock.amount.sub(currentPenalty));
}

The implementation for BondingManager#rebond() would use the same mechanism for processing an unbonding lock and applying slashing penalties before using the remaining lock amount for rebonding.

The loop for applying slashing penalties to the lock amount is bound by the number of rounds in the unbonding period and makes sure that if the transcoder is slashed multiple times during the unbonding period the penalties are all applied to the lock amount.

With assumption 3, we want to make sure that if a delegator already had slashing penalties for a round applied to its bonded amount because it claimed earnings through that round that the slashing penalty for that round is not applied to an unbonding lock created that round. However, if the delegator did not have the slashing penalty for a round applied to its bonded amount (either because it did not claim earnings through the round yet OR because it claimed earnings through the round and its transcoder was then slashed in the same round after the creation of the unbonding lock), then the slashing penalty for the round should be applied to the unbonding lock.

An edge case to watch out for is the creation of unbonding locks after a delegator changes its transcoder within a round because the unbonding lock should only be subject to slashing penalties for the original transcoder based on assumption 1 and not for the new transcoder. This should be covered by the fact that when a delegator changes its delegation it enters the Pending state and BondingManager#unbond() can only be invoked while in the Bonded state.

dob commented 6 years ago

Hold on a second - I am under the belief that token that are currently unbonding are no longer delegated, and therefore are no longer subjected to slashing. I thought this was always the case...and should remain the case with the unbondingLock construct.

On another note, I could see how this could potentially open up an attack where a transcoder unbonds, commits a fault that they would get slashed for, then immediately rebonds in order to avoid getting slashed. Let's first address whether we were under the same assumption, then address this potential attack.

yondonfu commented 6 years ago

I am under the belief that token that are currently unbonding are no longer delegated, and therefore are no longer subjected to slashing.

That is not the case right now - if a transcoder unbonds, its staked tokens can still be slashed while the transcoder is waiting through the unbonding period.

I also believe that it should be the case that unbonding tokens are slashed if evidence of a fault is submitted during the unbonding period - one of the intended benefits of an unbonding period is to provide enough time for evidence of a fault to be submitted since there is no strict time bound on when the evidence could be submitted. Without slashing during the unbonding period, the attack you mentioned becomes possible in addition to others - for example: a transcoder could purposefully fail verification, immediately unbond and as a result by the time the external verification system submits evidence of a fault on-chain, the transcoder would be in the unbonding period and protected from penalties.

dob commented 6 years ago

Is this attack really mitigated in the current setup? If stake is at risk for slashing during the unbonding period, then a delegator or transcoder should just bond towards an inactive address immediately before unbonding. This way they would not be at risk for being slashed during the unbonding process.

I guess in this scenario transcoders are treated slightly differently than regular delegators because they are prevented from bonding towards another node. So they can't take the above action and remain subject to slashing - which as you described is what we want in the case of a transcoder.

But a delegator who is not a transcoder could execute the above action in order to prevent being slashed while unbonding. This also seems reasonable - I'm just picturing us extending the unbonding period to 1 month, or 4 months like casper, and it seems kind of crazy that a delegator would be stuck getting slashed because of the transcoder they previously delegated towards.

yondonfu commented 6 years ago

If stake is at risk for slashing during the unbonding period, then a delegator or transcoder should just bond towards an inactive address immediately before unbonding. This way they would not be at risk for being slashed during the unbonding process.

it seems kind of crazy that a delegator would be stuck getting slashed because of the transcoder they previously delegated towards.

Both of these highlighted points might be addressed with some slashing mechanism changes described in https://github.com/livepeer/LIPs/issues/10#issuecomment-400471832.

However, in the interest of keeping this LIP focused on partial unbonding and to avoid having it also encompass potentially large changes to the slashing mechanisms, I think we can push forward with this draft proposal as is, move the rest of the discussion around covering all the slashing edge cases to the delegator slashing issue and acknowledge the following implications:

Solutions for all the slashing edge cases can be contained in their own LIP that might even be included in the same upgrade as this LIP, but I don't think they should be blockers for this feature.

dob commented 6 years ago

Yes, keeping the slashing mechanics independent of the partial unbonding mechanics sounds right. Think we're all clear to go ahead and merge.