-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix fetch state with endBlock and pending dynamic contracts #393
Conversation
let isPastEndblock = baseRegister.latestFetchedBlock.blockNumber >= endBlock | ||
switch (baseRegister.registerType, endBlock) { | ||
| (RootRegister, Some(endBlock)) => | ||
let isPastEndblock = getLatestFullyFetchedBlock(fetchState).blockNumber >= endBlock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This getLatestFullyFetchedBlock
usage is the only change related to the fix. Since we want to consider pending dynamic contracts as well.
Other changes are either renaming of fetchState
to partitionedFetchState
which is more correct, since these are two different entities. And removing endBlock from fetchState
since it's not needed there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% following the intricacies to be honest. Are you saying that before this fix, there could be pending requests to fetch more events for dynamic contracts that weren't accounted for - and 'getLatestFullyFetchedBlock' will include that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not completely following - but happy to get this in so it doesn't block you and we can test it more (don't promote to 'latest' release etc until we're confident)
We can chat it through on a call too
let isPastEndblock = baseRegister.latestFetchedBlock.blockNumber >= endBlock | ||
switch (baseRegister.registerType, endBlock) { | ||
| (RootRegister, Some(endBlock)) => | ||
let isPastEndblock = getLatestFullyFetchedBlock(fetchState).blockNumber >= endBlock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% following the intricacies to be honest. Are you saying that before this fix, there could be pending requests to fetch more events for dynamic contracts that weren't accounted for - and 'getLatestFullyFetchedBlock' will include that?
No description provided.