livepeer / protocol

Livepeer protocol
MIT License
154 stars 45 forks source link

bonding: Add TransferBond event and fix nextUnbondingLockId #532

Closed yondonfu closed 2 years ago

yondonfu commented 2 years ago

What does this pull request do? Explain your changes. (required)

Emit a TransferBond event in BondingManager.transferBond() which can be used by an off-chain indexer to properly track state updates in the contract. Previously, an Unbond event would be emitted for the caller and a Rebond event would be emitted for the receiver. From an off-chain indexer's POV, there would be an extra unbonding lock (there would never be a corresponding WithdrawStake or Rebond event for this lock for the caller) in the former case and no pre-existing unbonding lock (there would not be an initial Unbond event for the receiver) in the latter case. The addition of TransferBond allows the indexer to be aware that the unbonding lock created by the Unbond event should be removed and that there should be an unbonding lock created for the caller that would then be removed when the Rebond event is received.

While adding a test for this sequence of events I also fixed a "bug" where the incremented value of nextUnbondingLockId was not being persisted back to storage. In practice, this wasn't that much of a problem because the unbonding lock that was created for the receiver was immediately deleted in processRebond(). However, this logic would result in the same unbondingLockId potentially showing up in events again in the future. For example, suppose the unbondingLockId in the Rebond event emitted after a transferBond() is 1. If the receiver then calls unbond(), an Unbond event with unbondingLockId = 1 would be emitted. From an indexer's POV, this is weird behavior because that implies that a unbonding lock was previously created with the ID = 1 and then created again with the ID = 1. While fixing this "bug" by persisting the incremented value to storage increases gas costs, I think it is worthwhile as it results in more readable code and more understandable behavior especially from the POV of an indexer consuming events.

Specific updates (required)

See commit history.

How did you test each of these updates (required)

Added a test case for events.

Does this pull request close any open issues?

N/A

Checklist: