Closed victorges closed 1 year ago
@yondonfu @0xcadams Ok I think NOW this one is ready to go! :)
Ran the gas report on confluence
branch (current
) compared to this (checkpoints
). Here are the reports, it seems to have increased around 200k gas on the bond/unbond functions due to the checkpoints that have been made. The checkpoint initialization itself also costs around 170k gas, which should still be relatively cheap on Arbitrum, but I haven't found a way to reliably calculate how it would cost considering the calldata cost on L1.
OK @yondonfu @0xcadams I've addressed the feedback on your comments and this is ready for another review round. I have included some other changes as well that seemed necessary after the rounds of testing with the testnet and some discussions.
I recommend reviewing commit by commit, I avoided making a force push here not to lose history, and also isolated the less trivial changes in their own commits. As an overview of each:
BondingCheckpointsVotes
contract from https://github.com/livepeer/protocol/pull/615 into the BondingCheckpoints
contract here (renamed to just BondingVotes
). This is mainly to allow it to send the ERC5805
events on changes.nextRoundTotalActiveStake
when querying for the next round to the current one.
votingDelay
of 1 round the voting would be delayed in 1-2 rounds, but users would only really have 0-1 rounds to make any staking changes. Any staking changes made on the round before the voting actually starts would not be effective, since we would take the active stake from the start of that round for the voting power. Now we take it from the end (start of next round), and things work a bit more as expected.BondingVotes
contract to avoid reverting queries that could simply return a 0 value. There is only 1 specific case now where we will revert instead of returning zero, which is when there is no earnings pool on a transcoder's last reward round, which is an illegal state from the way that BondingManager
works. This also fixed a lot of the redundant reverts
with the SortedArrays
implementation.
What does this pull request do? Explain your changes. (required) This is to introduce a checkpointing logic in bonding manager so that we can lookup historical information about any point in time. We need this in order to support an on-chain governance system with support for OpenZeppelin Governor abstractions. That is why it also implements the
IERC5808
interface to plug into OZ Governor framework.It is implemented as "external" to
BondingManager
as possible in a separateBondingVotes
contract, but there were a couple places whereBondingManager
needed changes to start checkpointing its state.Specific updates (required)
BondingVotes
contractBondingManager
call it to checkpoint the state where it needs toHow did you test each of these updates (required)
yarn test
Does this pull request close any open issues? Implements PRO-28
Checklist:
yarn test
pass