More events. Remove unnecessary bool return values #127
Conversation
|
What about events for
|
There was a problem hiding this comment.
Looking pretty good. Just a couple questions.
The current round was used pretty consistently throughout the events, but it seemed to be absent from the job/claim/verify loop events. Intentional? I don't have a strong opinion either way about whether it should be in every event...especially if it can be inferred from the block that the event was included in.
| @@ -10,15 +10,15 @@ contract Migrations { | |||
| _; | |||
| } | |||
|
|
|||
| function Migrations() { | |||
| function Migrations() public { | |||
There was a problem hiding this comment.
These methods that are marked public...they're public by default and we're just adding this to be explicit, right? Good idea.
There was a problem hiding this comment.
Yep, the compiler also emits warnings if you don't add explicit visibility declarations - a non-functionality related todo is to do clean up any compiler warnings and make the CI fail if we have any warnings
contracts/bonding/BondingManager.sol
Outdated
| @@ -282,7 +292,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { | |||
| } | |||
| } | |||
|
|
|||
| return true; | |||
| Bond(_to, msg.sender, currentRound); | |||
There was a problem hiding this comment.
Are we including currentRound in all the events for convenience and searchability on the frontend? Presumably the client would know which block or round it occured in based upon the event metadata itself, right...but this would just allow us to filter or easily view a timeline?
There was a problem hiding this comment.
Yeah good point - the current round can always be computed from the block number during which an event is received
|
Removed round values from events as they can always be recalculated using the block number associated with an event. Also added |
Current list of events (excluding LivepeerVerifier and LivepeerToken):
ParameterUpdate
SetContract
SetController
TranscoderUpdate
TranscoderEvicted
TranscoderResigned
Bond
Unbond
Reward
NewRound
NewJob
NewClaim
Verify
DistributeFees
ReceivedVerification
NewInflation
SetCurrentRewardTokens
Let me know if there are any additional events in particular that anyone thinks might be particularly helpful. Maybe @jozanza has some thoughts relevant for the SDK?
Closes #31