search

livepeer / protocol

Livepeer protocol
MIT License
152 stars 45 forks source link

bonding: move EarningsClaimed event #361

Closed kyriediculous closed 5 years ago

kyriediculous commented 5 years ago

This PR moves emitting the EarningsClaimed event in updateDelegatorWithearnings() outside of the conditional so that it gets fired even if the delegator has no delegate.

This prevents indexers from ending up with stale data.

Fixes #360

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 1272

Totals Coverage Status
Change from base Build 1269: 0.0%
Covered Lines: 689
Relevant Lines: 689
💛 - Coveralls
kyriediculous commented 5 years ago

fixed linting error in 15ef024

kyriediculous commented 5 years ago

rebased !

kyriediculous commented 5 years ago

I opted for going with if (lastClaimRound >= _endRound) return; at the beginning of updateDelegatorWithEarnings instead. 265f50d

This statement will only ever cause the function to return if it's called from autoClaimEarnings() where lastClaimRound >= currentRound since it passes in currentRound as the _endRound argument to updateDelegatorWithEarnings

The require statements in claimEarnings will always make sure currentRound > lastClaimRound since they require lastClaimRound < _endRound && _endRound <= currentRound --> lastClaimRound < currentRound.

kyriediculous commented 5 years ago

good point, fixed in 0af8c5f

On Thu, Oct 31, 2019 at 7:57 PM Yondon Fu notifications@github.com wrote:

@yondonfu commented on this pull request.

In contracts/bonding/BondingManager.sol https://github.com/livepeer/protocol/pull/361#discussion_r341313653:

@@ -1087,8 +1092,11 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {

  • @param _delegator Delegator address
  • @param _endRound The last round for which to update a delegator's stake with token pools shares */
  • function updateDelegatorWithEarnings(address _delegator, uint256 _endRound) internal {
  • function updateDelegatorWithEarnings(address _delegator, uint256 _endRound, uint256 _lastClaimRound) internal {

Can we update the function comment with the _lastClaimRound param?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/livepeer/protocol/pull/361?email_source=notifications&email_token=AFJZZWVLKDCGMYIVXP34UCDQRMTANA5CNFSM4JF6OZCKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCJ57BWY#pullrequestreview-310112475, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFJZZWVYHXHTBJXWCR6GHI3QRMTANANCNFSM4JF6OZCA .

kyriediculous commented 5 years ago

rebased !