livepeer / LIPs

Livepeer Improvement Proposals
9 stars 14 forks source link

LIP-52: Snapshot for claiming earnings #52

Closed dob closed 3 years ago

dob commented 4 years ago

lip: \<to be assigned> title: PendingStake Snapshot and ClaimPastEarnings author: Doug Petkanics (@dob) type: Standard Track status: Draft created: 2020-08-12 discussions-to: http://github.com/livepeer/LIPs/issues/52

Abstract

This issue proposes snapshotting the pendingStake values for each delegator account at a specific block height (and round), anchoring proof of this snapshot on chain, and updating the claimEarnings() transaction to reference the snapshot, eliminating the need to claim earnings for each round previous to the snapshot. The net effect will be dramatically lowered gas costs.

Motivation

Gas costs on the Ethereum network have skyrocketed, sometimes reaching as high as 200 gwei/gas. This has resulted in any bonding related action, such as bonding, unbonding, rebonding, or claiming earnings costing a prohibitive amount relative to the value that is being accessed in LPT. As an example, users looking to withdraw $20 worth of LPT may be faced with $80 in gas costs to do so.

Part of the reason this is the case is because Livepeer's accounting architecture requires users to pay the gas costs to incrementally calculate their earnings in every single round. As Livepeer has been live for over two years, some users have to calculate 600-700 rounds worth of earnings, leading to very high costs.

While #35 proposes changes that can be applied going forward to reduce these costs, this proposal suggests a change that the community could approve, which would eliminate the majority of the costs to claim earnings going back in time. The proposal is for the community to review a snapshot of the pendingStake for each account, along with an onchain update which would reference the snapshot in order to allow users to instantaneously claim past earnings without having to calculate the earnings from each round. This would make claiming an O(1) operation instead of O(n) where n in the number of rounds. In the above example of $80 for a user to claim many rounds, this cost could be reduced to a ballpark of $1 or less.

The benefits are:

The drawbacks are:

To briefly address these drawbacks however, one could argue that this state change is no more arbitrary than any agreed upon protocol update, and this is what the governance process is for. And that prioritizing practicality, usability, and a growing community and user base is probably more important than standing on principles of mathematical accounting cleanliness.

Specification

The specification is TBD depending on the discussion. The following outlines what I think needs to be done at a high level and offers two different technical approaches for doing so. A spec will follow pending discussion.

What needs to be done

Approach 1 - MerkleTree

In this approach the snapshot is represented on chain by the root of a MerkleTree where the leaves are the account + pendingStake pairs of each account in the set sorted from low to high value of account address. The updated claim transaction flow would have to take the account balance and merkle proof as arguments.

The benefit of this approach is that it puts very little state on chain, and the protocol update is smaller. But the drawback is that it puts a new burden on the client software which implements bonding/staking actions to calculate and submit this data. It would also have to reference offchain state in order to calculate the merkle proof.

Approach 2 - On chain mapping

In this approach the protocol update would actually include the data to populate a mapping from account address to pendingStake balance. There are approximately ~3000 accounts which would need to be included in this mapping.

This has the reverse benefits and drawbacks - it makes the update large and stateful, but puts very little burden on the clients and reduces any reliance on offchain state.

Are there other good approaches?

Backwards Compatibility

I think it's TBD whether this protocol update would support backwards compatible claiming, or whether it would eliminate it in favor of the updated transactions and schema. More feedback/discussion is needed.

Test Cases

TBD

Implementation

TBD

Copyright

Copyright and related rights waived via CC0.

yondonfu commented 4 years ago

Calculate a snapshot of pendingStake for every staked user at a given blockheight.

Since the snapshot is also associated with a specific round, the snapshot could be calculated at the start of that round (i.e. upon initialization) which would remove the need to specify a block number (it would be implied by the round). Let's call this round LIP_52_SNAPSHOT_ROUND. Then, any delegator could use the snapshot to claim all earnings up to (but not including) LIP_52_SNAPSHOT_ROUND.

This round would also need to be in the past. If LIP-36 is accepted and deployed, then LIP_52_SNAPSHOT_ROUND could be set to LIP_36_UPGRADE_ROUND. The benefit of doing so would be that after using the snapshot to claim all earnings up to LIP_36_UPGRADE_ROUND a delegator could immediately use the more gas efficient LIP-36 earnings claiming algorithm going forward.

be it a MerkleRoot or readable Mapping

I would use a Merkle root. I think the extra work that a client would need to do to generate Merkle proofs is a reasonable tradeoff to make in order to minimize the amount of on-chain state required. If needed, a third party could also provide the proofs to a client - since the client can efficiently verify the proof locally before submitting it on-chain (simply through a transaction dry run) there would be minimal risk in outsourcing proof generation.

kyriediculous commented 4 years ago

I really like this proposal and it can work well complementary to LIP-36. A few comments:

  1. pending fees would also have to be considered in scope for the snapshot and in case of a merkle root would have to be part of the proof.

  2. The submitted proof to claimEarnings can be an optional argument. If it's not provided we could still default to the old looping workflow up until the LIP-36 upgrade round.

  3. I actually feel setting the snapshot round to the round before the LIP-36 round is the only viable option. The current LIP-36 Implementation actually accommodates for this because it will short circuit the loop claiming algorithm for rounds >= LIP_36_UPGRADE_ROUND which in itself is part of the BondingManager state after the LIP-36 upgrade round. This provides a clear non-contentious point at which the snapshot should be taken due to implementation constraints of LIP-36.

  4. While there would be more code and it would feel less clean this would be true for the old algorithm as well (pre-LIP36) that code path would also still exists even if everyone has claimed for those rounds in the future. It is what it is.

dob commented 4 years ago

@kyriediculous

that code path would also still exists even if everyone has claimed for those rounds in the future

Are you suggesting that the existing function signature and code for claiming would continue to live and be supported? If so this solves the backwards compatibility issue with any existing clients that wrap the current claim path.

kyriediculous commented 4 years ago

The signature would likely take additional arguments (the proof , which can be a single struct argument) but the code path would very much still exist.

Clients would have to do a minor update to support that.

Alternatively we could just create a separate function that allows claiming up until (and only up until) the LIP_36_ROUND/LIP_52_SNAPSHOT_ROUND through a merkle proof and keep the existing function as-is.

Prevention of double claiming using the merkle proof as well as the old approach would be prevented by setting the lastClaimRound for a delegator depending on the method first used by the end-user.

chrishobcroft commented 4 years ago

I have only one question: how will a Delegator validate whether the value in the snapshot is accurate per the pendingStake onchain, so that they know which way to vote in the poll?

kyriediculous commented 4 years ago

By either

(1) reconstructing the merkle tree itself and checking merkle roots match

Involves calling pendingStake(delegator, snapshotRound) and pendingFees(delegator, snapshotRound) for each delegator

(2) reconstructing its own leaf and required proof leafs to see that the proof is valid for the given merkle root in the proposal

A delegator should do (2) either way before submitting the transaction for its own leaf and generated proof on-chain

yondonfu commented 4 years ago

A draft LIP has been created containing updates to the original post. The contents of the draft LIP should be used as the basis for further discussion.

yondonfu commented 4 years ago

I don't think the following is a strict requirement, but I was thinking about ways to improve the UX for a delegator and it would be nice if 1) claiming from the snapshot does not need to be a distinct action and a delegator can just do it when it also needs to perform a staking action 2) claiming from the snapshot and performing a staking action could be done with a single tx. If both of these properties are achieved then a client that supports LIP-52 would not need to expose snapshot claiming to its user and the snapshot claiming would happen in a tx that also performs a staking action requested by the user.

One way that these properties might be achieved could be to pass an optional tx data param for invoking another function after the snapshot claiming is complete. A rough example:

function claimSnapshotEarningsAndCall(
    uint256 _rewards,
    uint256 _fees,
    bytes32[] memory _earningsProof,
    bytes _data
)
    external
    ...
{
    // Other code
    ...

    if (_data.length > 0) {
         // Delegatecall so msg.sender and the current call context is preserved
         // This could be a bond, unbond, rebond, etc.
        (bool success, bytes res) = address(this).delegatecall(_data);
        ...
    }
}

If achieving the aforementioned properties requires complex changes then I don't think we need to push for them, but if the change is simple (i.e. something along the lines of the example above) then it might be worth considering.

kyriediculous commented 4 years ago

I don't think we need the additional field, we can just call updateDelegatorWithEarnings internally with _lastClaimRound being the snapShotRound (i.e. roundsManager.LIPUpgradeRound(52) ) and _endRound being currentRound.sub(1) (or we can add an additional _endRound argument to this method).

yondonfu commented 4 years ago

we can just call updateDelegatorWithEarnings internally

updateDelegatorWithEarnings() would need to have access to the earnings proof in order to claim from the snapshot. This would require all staking functions (i.e. bond, unbond, etc.) to accept the earnings proof as a parameter if we want to support claiming from the snapshot + executing a staking function in a single tx.

kyriediculous commented 4 years ago

I think we have a mismatch in user expectations here, I imagined a case where users just could click a claim earnings button to claim all their historical earnings and not incorporate staking actions into it.

I still prefer a UI solution over arbitrary code execution through that function. There would be no difference for those users as it is now, they would also have to separately claim their earnings before being able to do a staking action because for most it's been over 100 rounds. Even though that restriction gets lifted the user flow would remain similar.

EDIT: after discussing this with @adamsoffer being able to arbitrarily execute any staking action within the claimSnapshotEarnings call might actually be the path of least resistance and should also eliminate confusion around claiming earnings.

yondonfu commented 3 years ago

Closed by #66