Skip to content

Instantly share code, notes, and snippets.

@yondonfu
Last active September 4, 2018 21:28
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save yondonfu/c1a43c1ce0bd392e1d196a3345fa0a91 to your computer and use it in GitHub Desktop.
Save yondonfu/c1a43c1ce0bd392e1d196a3345fa0a91 to your computer and use it in GitHub Desktop.

The findings below pertain to the Livepeer protocol smart contract code at Git commit hash 37da421d38a13313809c63b945953314eaaca455.

Findings

1. Delegators can earn shares of rewards and fees that are not proportional to their bonded stake relative to a transcoder's total delegated stake.

Description

When a delegator calls BondingManager.claimEarnings() with a round number N, the delegator will claim earnings for all rounds starting from lastClaimRound + 1 through round N. When claiming earnings for a particular round, the contract will look up the earnings pool for the delegator's transcoder which keeps track of the transcoder's total delegated stake, reward cut, and fee share for that round. Furthermore, the earnings pool also tracks the claimable stake left for the earnings pool, which represents the remaining stake that has not been used to claim earnings from the pool, the reward pool, which represents the remaining unclaimed LPT rewards for the round, and the fee pool, which represents the remaining unclaimed ETH fees for the round. Every time a delegator claims earnings for a round, the claimable stake will be reduced by the delegator's bonded stake during that round, the reward pool will be reduced by the delegator's reward share and the fee pool will be reduced by the delegator's fee share.

We will discuss reward share calculations below, but the description also applies to fee share calculations as well.

The reward share calculation always uses the transcoder's reward cut percentage stored by the earnings pool to deduct the transcoder's reward share from the reward pool in order to determine the amount of the reward pool that can be claimed by delegators. As a result, rather than each delegator receiving reward shares proportional to their bonded stake relative to the transcoder's total delegated stake, each delegator receives reward shares depending on how much is left in the reward pool. As more delegators claim, the portion of the reward pool reserved for the transcoder becomes smaller thus, given a set of delegators that have the same bonded stake, the delegators that claim later might receive more rewards than the delegators that claim earlier.

Consequences

Consider the following participants:

  • A is a transcoder with 10 LPT delegated to itself. A has set its reward cut to 10%.
  • B is a delegator with 30 LPT delegated to A.
  • C is a delegator with 30 LPT delegated to A.
  • D is a delegator with 30 LPT delegated to A.

In round N, A calls BondingManager.reward() which mints 100 LPT. The rewards are placed in A's earnings pool for round N. Given the described participants, the distribution of rewards should look like:

  • A receives 10 LPT as a transcoder (corresponds to a 10% reward cut) and 9 LPT as a delegator (corresponds to a 10% share of 90 LPT of delegator rewards).
  • B receives 27 LPT as a delegator (corresponds to a 30% share of 90 LPT of delegator rewards).
  • C receives 27 LPT as a delegator (corresponds to a 30% share of 90 LPT of delegator rewards).
  • D receives 27 LPT as a delegator (corresponds to a 30% share of 90 LPT of delegator rewards).

Now, let's consider what the distribution of rewards actually looks like. Let the earnings pool for round N be earningsPool with earningsPool.rewardPool = 100 and earningsPool.claimableStake = 100.

Note: integer division with truncation as this is the behavior of the EVM.

  1. B calls BondingManager.claimEarnings(N)
  • Delegator rewards = 90% of 100 = 90
  • B's reward share = (90 * 30) / 100 = 27
  • earningsPool.rewardPool = 100 - 27 = 73
  • earningsPool.claimableStake = 100 - 30 = 70
  1. C calls BondingManager.claimEarnings(N)
  • Delegator rewards = 90% of 73 = 65
  • C's reward share = (65 * 30) / 70 = 27
  • earningsPool.rewardPool = 73 - 27 = 46
  • earningsPool.claimableStake = 70 - 30 = 40
  1. D calls BondingManager.claimEarnings(N)
  • Delegator rewards = 90% of 46 = 41
  • D's reward share = (41 * 30) / 40 = 30
  • earningsPool.rewardPool = 46 - 30 = 16
  • earningsPool.claimableStake = 40 - 30 = 10

We can observe that the actual distribution of rewards for the 3 non-transcoder delegators is:

  • B receives 27 LPT
  • C receives 27 LPT
  • D receives 30 LPT

D ends up receiving a reward share that is higher than the amount that it should receive if the reward share calculation was truly based on D's bonded stake relative to the transcoder's total delegated stake.

Solution

This issue can be fixed by tracking delegator rewards and fees separately from transcoder rewards and fees within an earnings pool. When a transcoder places rewards or fees into the earnings pool, it would calculate the amount to add to the delegator reward/fee pool and the amount to add to the transcoder reward/fee pool based on the transcoder's reward cut and fee share for the earnings pool. Then, when delegators claim from the earnings pool, the reward/fee share calculation would occur without the transcoder's share since it has already been set aside previously. When a transcoder claims from the earnings pool, it will claim as a delegator and then also empty out the transcoder reward/fee pool.

This update can be implemented by with a few updates to the EarningsPool library.

We can add two additional fields to the EarningsPool.Data struct to track transcoder rewards and fees. We also introduce an additional flag which can be used to differentiate earnings pools created before and after the update.

library EarningsPool {
    ...
    struct Data {
        ...
        uint256 transcoderRewardPool;
        uint256 transcoderFeePool;
        bool hasTranscoderRewardFeePool;
    }
}

If hasTranscoderRewardFeePool = true, the EarningsPool function should assume that there are separate transcoder reward and fee pools. If hasTranscoderRewardFeePool = false, the EarningsPool function should assume that there are not separate transcoder reward and fee pools and instead calculate reward and fee shares using the old pre-update logic. The use of this flag is to ensure that reward and fee share calculations can still be executed for older earnings pools - since additional state variables are required to track information necessary to fix this issue which do not exist in the older earnings pools, we focus on allowing transcoders and delegators to still receive reward/fee shares from older earnings pools even though they are not correctly calculated. The rewards/fee share calculations will be correct for all new earnings pool created after the update.

Helper functions can be used to split rewards/fees into the delegator and transcoder reward/fee pools.

function addToRewardPool(EarningsPool.Data storage earningsPool, uint256 _rewards) internal {
    // Split _rewards
    // Calculate transcoder rewards using earningsPool.transcoderRewardCut
    // Add transcoder rewards to earningsPool.transcoderRewardPool
    // Add remaining rewards for delegators to earningsPool.rewardPool
}

function addToFeePool(EarningsPool.Data storage earningsPool, uint256 _fees) internal {
    // Split _fees
    // Calculate transcoder fees using earningsPool.transcoderFeeShare
    // Add transcoder fees to earningsPool.transcoderFeePool
    // Add remaining fees for delegators to earningsPool.feePool
}

Update earningsPool.claimShare().

function claimShare(EarningsPool.Data storage earningsPool, uint256 _stake, bool _isTranscoder) internal returns (uint256, uint256) {
    ...
    uint256 totalRewards = 0;
    uint256 totalFees = 0;
    
    // Calculate delegator rewards as (earningsPool.rewardPool * _stake) / earningsPool.claimableStake
    // Calculate delegator fees as (earningsPool.feePool * _stake) / earningsPool.claimableStake
    // Add delegator rewards to totalRewards
    // Add delegator fees to totalFees
    // Deduct delegator rewards from earningsPool.rewardPool
    // Deduct delegator fees from earningsPool.feePool
    // Deduct _stake from earningsPool.claimableStake
    
    if (_isTranscoder) {
        // Add earningsPool.transcoderRewardPool to totalRewards
        // Add earningsPool.transcoderFeePool to totalFees
        earningsPool.transcoderRewardPool = 0;
        earningsPool.transcoderFeePool = 0;
    }
    
    return (totalFees, totalRewards);
}

This solution was implemented in the following commit: 24ad91419da1eab07af6e2ddaa75e310dfdffa88.

2. Transcoders can earn shares of rewards and fees that are less than the amount that they should be entitled to based on their reward cut and fee share.

Description

The description for finding 1 also applies to this finding. As a result, of the portion of the reward pool reserved for the transcoder becoming smaller as more delegators claim, the later the transcoder claims, the less rewards it might receive relative to the amount it should be entitled to based on its reward cut.

An additional implication is that if the transcoder is not the last to claim from its earnings pool, a portion of the reward pool will be left unclaimed which effectively means those tokens are permanently bonded and delegated toward the transcoder.

Consequences

Consider the participant set described in the exploit scenario from finding 1.

  1. B calls BondingManager.claimEarnings(N)
  • Delegator rewards = 90% of 100 = 90
  • B's reward share = (90 * 30) / 100 = 27
  • earningsPool.rewardPool = 100 - 27 = 73
  • earningsPool.claimableStake = 100 - 30 = 70
  1. A calls BondingManager.claimEarnings(N)
  • Delegator rewards = 90% of 73 = 65
  • A's reward share = (65 * 10) / 70 = 9
  • Since A is a transcoder, it also gets 10% of 73 = 7
  • earningsPool.rewardPool = 73 - 9 - 7 = 57
  • earningsPool.claimableStake = 70 - 10 = 60

We can observe that the actual distribution of rewards for the transcoder is:

  • A receives 7 LPT as a transcoder and 9 LPT as a delegator

A ends up receiving a transcoder reward share that is lower than the amount that it should receive based on its reward share percentage for the round.

Solution

The solution for finding 1 fixes this issue.

Upgrade

All solutions described have been implemented and deployed to the Ethereum mainnet as of Git commit hash 1f3f61b4d5e72d41a2818039bee35b6aa492cd46.

The new version of the BondingManager target contract that includes the relevant changes in the EarningsPool library (which is pulled into the BondingManager during compilation) is deployed at 0x05C03EA0039f2e828A725A82939fc1e90de38B44 which replaces the old version deployed at 0x633101b3f15f93c5f415830d48e56b9b1f7ba584.

The upgraded BondingManager target 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 BondingManager proxy contract deployed at 0x511bc4556d823ae99630ae8de28b9b80df90ea2e.

The registration of the new BondingManager target contract was initiated by the multisig transaction 0x383bb82479747e7224741f9b2626a3e3c3ab5d9ca7e84689bcb3a2d93bc20117 and confirmed by the multisig transaction 0x878bfec145bab376795dcb406a18d411d7fcb6249c3505f69e74abb4b8a4828a.

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