compound-finance / compound-protocol

The Compound On-Chain Protocol
https://compound.finance/developers
BSD 3-Clause "New" or "Revised" License
1.89k stars 1.21k forks source link

Split COMP rewards distribution #144

Closed TylerEther closed 3 years ago

TylerEther commented 3 years ago

This pull request is for RFP 16: Dynamic COMP reward distribution, authored by me and Arr00.

Changes:

I've run all of the tests to ensure none fail; the changes work as expected.

The upgrade hook has yet to be tested... after these changes pass reviews, I'll deploy the contract and write an upgrade simulation.

TylerEther commented 3 years ago

Nice work, feeling a lot better about this now. Still hard to remember how this code works though

If I'm understanding it correctly, index is a cumulative sum of the COMP per CToken, so updateComp[Borrow/Supply]Index accrues COMP for the whole market on liquidity changes and speed changes, and distribute[Supplier/Borrower]Comp calculates an account's index delta and multiplies that by the amount of cTokens they are moving and distributes this amount of COMP..

:nerd_face:

jflatow commented 3 years ago

Nice work, feeling a lot better about this now. Still hard to remember how this code works though

If I'm understanding it correctly, index is a cumulative sum of the COMP per CToken, so updateComp[Borrow/Supply]Index accrues COMP for the whole market on liquidity changes and speed changes, and distribute[Supplier/Borrower]Comp calculates an account's index delta and multiplies that by the amount of cTokens they are moving and distributes this amount of COMP..

🤓

Yeah the math is the easy part :) The initialization logic scares me since that's where we had the 'quirk' and all I remember is it was so complicated we couldn't even explain it to OZ

TylerEther commented 3 years ago

Nice work, feeling a lot better about this now. Still hard to remember how this code works though

If I'm understanding it correctly, index is a cumulative sum of the COMP per CToken, so updateComp[Borrow/Supply]Index accrues COMP for the whole market on liquidity changes and speed changes, and distribute[Supplier/Borrower]Comp calculates an account's index delta and multiplies that by the amount of cTokens they are moving and distributes this amount of COMP.. nerd_face

Yeah the math is the easy part :) The initialization logic scares me since that's where we had the 'quirk' and all I remember is it was so complicated we couldn't even explain it to OZ

Ah yes. The problem was that state.block is updated every liquidity event whereas state.index is only updated when a distribution speed is set. So the prior initialization code was never called as it expected state.block to be 0 which is not the case when a market has activity. :slightly_smiling_face:

ghswFTUSD commented 3 years ago

it seems the implementation is still not actived?

hayesgm commented 3 years ago

This was merged in a separate PR as it's on main-net.