Skip to content
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

396: Don't skip any blocks if the head of the chain advances by more than one block. #397

Merged
merged 7 commits into from
Jul 6, 2023

Conversation

telackey
Copy link

@telackey telackey commented Jun 30, 2023

As #396 shows, we can get scenarios when the chain advances by more than a single block, but the event which is seen by the WriteLoop will only be for the new head block--however many blocks in advance it is--allowing gaps to open in the statediffing.

This code will detect and trigger statediffing for the skipped blocks. The functionality is similar to the previous "head gap" filling code. Similar enough that we are able to remove that code and allow this to cover both cases.

@telackey telackey self-assigned this Jun 30, 2023
@telackey telackey changed the title WIP: 396 - don't skip any blocks if the head of the chain advances by… WIP: 396 - don't skip any blocks if the head of the chain advances by more than one block. Jun 30, 2023
@telackey telackey marked this pull request as ready for review July 5, 2023 20:48
@telackey telackey changed the title WIP: 396 - don't skip any blocks if the head of the chain advances by more than one block. 396 - don't skip any blocks if the head of the chain advances by more than one block. Jul 5, 2023
@telackey telackey changed the title 396 - don't skip any blocks if the head of the chain advances by more than one block. 396: Don't skip any blocks if the head of the chain advances by more than one block. Jul 5, 2023
Copy link
Collaborator

@i-norden i-norden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

defaultStatediffMetrics.lastEventHeight.Update(int64(nextHeight))
}
} else {
log.Warn("WriteLoop: received unexpected block from the past", "block height", nextHeight, "last height", lastHeight)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be interesting to see if/how often this occurs.

@@ -438,6 +471,8 @@ func (sds *Service) Loop(chainEventCh chan core.ChainEvent) {
select {
//Notify chain event channel of events
case chainEvent := <-chainEventCh:
// TODO: Do we need to track the last streamed block as we do for the WriteLoop so that we can detect
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For correctness we should, but nothing currently uses the streaming functionality (and won't in the foreseeable future) so we don't need to. We should consider removing the statediff streaming during the pluggeth port @roysc.

@telackey telackey merged commit e2e2e85 into v1.11.6-statediff-v5 Jul 6, 2023
5 checks passed
@telackey telackey deleted the telackey/396 branch July 6, 2023 17:01
github-cerc-io pushed a commit to cerc-io/plugeth-statediff that referenced this pull request Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants