Fix issue handling bucketTake events #30
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EdNoepel
changed the title
| @@ -313,9 +313,8 @@ export function handleBucketBankruptcy(event: BucketBankruptcyEvent): void { | |||
| } | |||
|
|
|||
| export function handleBucketTake(event: BucketTakeEvent): void { | |||
| const bucketTake = new BucketTake( | |||
There was a problem hiding this comment.
Do we need to assume the next log index at all? Shouldn't the transaction hash and block number concat still work for the ID?
There was a problem hiding this comment.
Not sure how block number helps us. A transaction hash may contain multiple bucketTake actions. As such, we need the log index. The LP award should always be followed by the bucket take event.
There was a problem hiding this comment.
nvm, I had the issue backwards - this makes sense
src/utils/liquidation.ts
Outdated
| } | ||
| return bucketTake; | ||
| }1 |
There was a problem hiding this comment.
nit: trailing 1 and no extra line
|
Example query screenshot: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a chicken-and-egg problem handling
bucketTake.BucketTakeLPAwardedEventis currently emitted beforeBucketTakeEvent. As such, when the LP award looked up theBucketTakeentity, it was unexpectedly null.Because
BucketTakeLPAwardedEventdoes not have enough information to build theBucketTake, we now create a placeholder which references the LP award event. When theBucketTakeEventoccurs (on the very next log index), it looks up the LP award event and updates state of all related entities. We associate these events by assuming the take event will always be the subsequent log index, which a cursory review of the code reveals is the case. Reviewer should confirm.This should handle the case where multiple
bucketTakes occur in a single transaction, but that remains untested.Tested on
parvati, which is able to sync up to current block height on goerli (9336296).