The findings below pertain to the Livepeer protocol smart contract code at Git commit hash 2889ad2d5f20507d58701acf4d47a0551e593c04.
1. If a registered transcoder A is delegated to another registered transcoder B, transcoder A will not be able to claim its reward cut as a transcoder
Whenever a transcoder calls BondingManager.reward(), the newly minted inflationary tokens are placed in a reward pool for the round in which
the tokens were minted. Both transcoders and delegators claim their reward shares for reward pools in past rounds by either explicitly
invoking BondingManager.claimEarnings() with an end round that the caller wishes to claim earnings through or
by invoking a bonding related funtion such as BondingManager.bond() or BondingManager.unbond() which will automatically claim earnings
for the caller from reward pools in past rounds through the current round.
The reward share calculation assumes that transcoders are always delegated towards themselves (transcoders are also delegators):
function updateDelegatorWithEarnings(address _delegator, uint256 _endRound) {
...
bool isTranscoder = _delegator == del.delegateAddress;
var (fees, rewards) = earningsPool.claimShare(currentBondedAmount, isTranscoder);
...
}
As a result, if a registered transcoder is not delegated toward itself, the line earningsPool.claimShare(currentBondedAmount, isTranscoder) will not
return a value for rewards that reflects the transcoder cut of the rewards. Additionally, since the registered transcoder is not delegated toward itself, it
would not be entitled to any reward shares at all, so rewards would always equal to 0.
At the same time, even though the registered transcoder is unable to claim any reward share, its delegated stake will still reflect these unclaimable reward shares. Consequently, these tokens can be considered to be permanently bonded and delegated toward the registered transcoder.
Alice delegates 10 LPT toward herself resulting in a total delegated stake of 20 LPT (including LPT that others have delegated
toward Alice) and registers as a transcoder. Alice sets her rewardCut to 50%. Alice delegates her 10 LPT toward another registered transcoder Bob. The next round passes
and Alice calls BondingManager.reward() for the round which places 100 LPT into the reward pool for the round. Alice should be
entitled to 50 LPT (50% of 100), but she is unable to claim her reward share because she is delegated toward Bob which violates
the assumption in the reward share calculation code which assumes that Alice is always delegated toward herself if she is a
transcoder.
Restricting registered transcoders from delegating towards anyone besides themselves fixes this issue. By forcing the registered transcoder to always be delegated towards itself, the assumptions made in the reward share calculation will not be violated. This update can be implemented by a restriction in BondingManager.bond() that the caller cannot change its delegate if it is a registered transcoder (it would already be delegated toward itself at this point if it had registered as a transcoder):
function bond(uint256 _amount, address _to) external whenSystemNotPaused currentRoundInitialized autoClaimEarnings {
...
} else if (del.delegateAddress != address(0) && _to != del.delegateAddress) {
// A registered transcoder cannot delegate its bonded stake toward another address
// because it can only be delegated toward itself
// In the future, if delegation towards another registered transcoder as an already
// registered transcoder becomes useful (i.e. for transitive delegation), this restriction
// could be removed
require(transcoderStatus(msg.sender) == TranscoderStatus.NotRegistered);
...
}
...
}
This update would cause BondingManager.bond() to revert if the caller is a registered transcoder that tries to change its
delegate to anyone besides itself. Note that this solution does not seek to solve the problem of reward share tokens that are permanently bonded and delegated toward a registered transcoder because it cannot claim them.
This solution was implemented in the following commit: 9563609d98e7b517ab25b41087960c0a68d0d6aa.
2. If a registered transcoder A is delegated to another registered transcoder B and proceeds to unbond, transcoder B's total delegated stake will not decrease properly
The function BondingManager.unbond() will resign the caller as a transcoder if the caller is a registered transcoder. If the
caller is not a registered transcoder, but is delegated to a registered transcoder, the delegated stake of the caller's
delegate will decrease by the caller's stake.
function unbond() {
...
if (transcoderStatus(msg.sender) == TranscoderStatus.Registered) {
// If caller is a registered transcoder, resign
// In the future, with partial unbonding there would be a check for 0 bonded stake as well
resignTranscoder(msg.sender);
} else if (transcoderStatus(del.delegateAddress) == TranscoderStatus.Registered) {
// If delegate is a registered transcoder, decrease its delegated stake
transcoderPool.updateKey(del.delegateAddress, transcoderPool.getKey(del.delegateAddress).sub(del.bondedAmount), address(0), address(0));
}
...
}
Note that if the caller is both a registered transcoder and is also delegated to another registered transcoder besides itself, the second conditional path will never be executed. As a result, in that scenario, the caller will be resigned as a transcoder, but the delegated stake of the registered transcoder that it was delegated toward will not decrease by the caller's stake.
Alice delegates 10 LPT toward herself using account A resulting in a total delegated stake of 20 LPT (including LPT that others have delegated
toward account A) and registers as a transcoder. Alice creates another account B and using account B delegates 10 LPT toward herself
and also registers account B as a transcoder. Using account A, Alice delegates 10 LPT toward account B thus increasing
account B's delegated stake to 20 LPT. In the following round, Alice uses account A to invoke BondingManager.unbond() which
resigns account A as a transcoder, but will not decrease account B's delegated stake. As a result, even though no one else is
delegated to account B besides itself, it has an additional 10 LPT credited to its delegated stake.
Similarly to the solution for finding 1, restricting registered transcoders from delegating towards anyone besides themselves prevents this issue from occuring in the future. However, an additional update is necessary to make sure any registered transcoders that are already in a state such that they are delegated towards someone besides themselves can invoke BondingManager.unbond() and properly decrease the delegated stake of the other registered transcoder. This update can be
implemented by always checking in BondingManager.unbond() if the caller is delegated to a registered transcoder aside
from itself and if so, decreasing the registered transcoder's delegated stake by the caller's stake:
function unbond() external whenSystemNotPaused currentRoundInitialized autoClaimEarnings {
...
if (transcoderStatus(msg.sender) == TranscoderStatus.Registered) {
// If caller is a registered transcoder, resign
// In the future, with partial unbonding there would be a check for 0 bonded stake as well
resignTranscoder(msg.sender);
}
if (del.delegateAddress != msg.sender && transcoderStatus(del.delegateAddress) == TranscoderStatus.Registered) {
// If delegate is not self and is a registered transcoder, decrease its delegated stake
// We do not need to decrease delegated stake if delegate is self because we would have already removed self
// from the transcoder pool
transcoderPool.updateKey(del.delegateAddress, transcoderPool.getKey(del.delegateAddress).sub(del.bondedAmount), address(0), address(0));
}
...
}
Ths update would make sure that if the caller is a registered transcoder delegated towards another registered transcoder, the caller will be resigned as a transcoder and the delegated stake of the caller's delegate will decrease by the caller's stake.
This solution was implemented in the following commit: 9563609d98e7b517ab25b41087960c0a68d0d6aa.
All solutions described have been implemented as of Git commit hash 9563609d98e7b517ab25b41087960c0a68d0d6aa.
The new version of the BondingManager is deployed at 0x5A9512826EAAF1FE4190f89443314E95A515fE24 which replaces the old version deployed at 0x81Eb0b10Ff8703905904e4d91Cf6AA575d59736f.
The upgraded BondingManager contract is registered by the Controller under the contract ID keccak256("BondingManagerTarget") and serves as the new target implementation contract containing the business logic relied upon by the proxy BondingManager contract deployed at 0x511Bc4556D823Ae99630aE8de28b9B80Df90eA2e.
The registration of the new target implementation BondingManager contract was initiated by the multisig transaction 0xc1c4227f54319316184c5d373b2e6c0a539f250a4240aa898e2447a5f8c264e7 and confirmed by the multisig transaction 0x4787560ca9e07a474f7e52876efe057d5bac2d9e87e03ca601429bdc57043864.