-
Notifications
You must be signed in to change notification settings - Fork 1.4k
VeBlop: Update span index lookup and add more unit tests #16729
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
Conversation
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.
Pull Request Overview
This PR updates the span index lookup algorithm to handle span rotations where StartBlock values are not monotonically increasing. The previous implementation relied on the assumption that spans would always have increasing start blocks, but this fails when Heimdall announces future spans before span rotations occur.
Key changes:
- Modified lookup algorithm to walk backwards from the seek position while tracking the highest span ID in range
- Added comprehensive unit tests covering complex span rotation scenarios
- Fixed assertion parameter order in existing tests
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| polygon/heimdall/span_range_index.go | Updated lookup algorithm to handle non-monotonic StartBlock values by walking backwards and finding highest span ID |
| polygon/heimdall/span_range_index_test.go | Added extensive test cases for span rotations and fixed assertion parameter order |
| polygon/heimdall/snapshot_store_test.go | Fixed assertion parameter order and updated test data to match new span rotation scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if isInRange && prevSpanId > lastSpanIdInRange { // a span in range with higher span id was found | ||
| lastSpanIdInRange = prevSpanId | ||
| } | ||
| } |
Copilot
AI
Aug 19, 2025
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.
The function returns true even when lastSpanIdInRange is 0 (the initial value), which indicates no valid span was found. This should return false when no span is found in range.
| } | |
| } | |
| if lastSpanIdInRange == 0 { | |
| return 0, false, nil | |
| } |
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.
If there was no span truly in range, then this part of the code would not even be reached, instead the code above it dealing with cursor.Last() would be triggered instead.
Follow-up to : #16683 The previous PR relied on the assumption that `span.StartBlock` is monotonically increasing, but it turns out that assumption is not true due to a future span being announced in advance from Heimdall and a span rotation taking place before the announced span. e.g. ```Go var spans = []Span{ Span{ Id: 0, StartBlock: 0, EndBlock: 999, }, Span{ Id: 1, // new span announced StartBlock: 1000, EndBlock: 1999, }, Span{ Id: 2, // span rotation StartBlock: 5, EndBlock: 1999, }, } ``` Therefore, this PR adapts the lookup algorithm by starting the search from `entry := cursor.Seek(blockNum)` and walking the `BorSpans` table backwards using `cursor.Prev()` while recording the highest span id which is in range, until an out-of-range span is reached, at which point we break and return the highest span id seen. This lookup algorithm relies on the assumption that there won't ever be more than 2 consecutive spans for which `span1.Id < span2.Id` and `span1.StartBlock > span2.StartBlock` . If this assumption were ever to be broken, we would have to fall back on brute-force linear search. Also, another detail to take into account is that due to the span rotations multiple spans with the same `StartBlock` could be produced, therefore overwriting the value of the index table at that block number. This is not a problem, because if this happens, then the previous span value will become entirely obsolete and no block number could have the old span associated with it. --------- Co-authored-by: antonis19 <[email protected]>
Cherry-picks for : - #16404 - #16683 - #16729 --------- Co-authored-by: antonis19 <[email protected]>
Follow-up to : #16683 The previous PR relied on the assumption that `span.StartBlock` is monotonically increasing, but it turns out that assumption is not true due to a future span being announced in advance from Heimdall and a span rotation taking place before the announced span. e.g. ```Go var spans = []Span{ Span{ Id: 0, StartBlock: 0, EndBlock: 999, }, Span{ Id: 1, // new span announced StartBlock: 1000, EndBlock: 1999, }, Span{ Id: 2, // span rotation StartBlock: 5, EndBlock: 1999, }, } ``` Therefore, this PR adapts the lookup algorithm by starting the search from `entry := cursor.Seek(blockNum)` and walking the `BorSpans` table backwards using `cursor.Prev()` while recording the highest span id which is in range, until an out-of-range span is reached, at which point we break and return the highest span id seen. This lookup algorithm relies on the assumption that there won't ever be more than 2 consecutive spans for which `span1.Id < span2.Id` and `span1.StartBlock > span2.StartBlock` . If this assumption were ever to be broken, we would have to fall back on brute-force linear search. Also, another detail to take into account is that due to the span rotations multiple spans with the same `StartBlock` could be produced, therefore overwriting the value of the index table at that block number. This is not a problem, because if this happens, then the previous span value will become entirely obsolete and no block number could have the old span associated with it. --------- Co-authored-by: antonis19 <[email protected]>
…17095) Cherry-picks of : #16683 #16729 #16803 #16819 #16880 #16951 #17014 --------- Co-authored-by: antonis19 <[email protected]> Co-authored-by: Mark Holt <[email protected]>
Follow-up to : #16683 The previous PR relied on the assumption that `span.StartBlock` is monotonically increasing, but it turns out that assumption is not true due to a future span being announced in advance from Heimdall and a span rotation taking place before the announced span. e.g. ```Go var spans = []Span{ Span{ Id: 0, StartBlock: 0, EndBlock: 999, }, Span{ Id: 1, // new span announced StartBlock: 1000, EndBlock: 1999, }, Span{ Id: 2, // span rotation StartBlock: 5, EndBlock: 1999, }, } ``` Therefore, this PR adapts the lookup algorithm by starting the search from `entry := cursor.Seek(blockNum)` and walking the `BorSpans` table backwards using `cursor.Prev()` while recording the highest span id which is in range, until an out-of-range span is reached, at which point we break and return the highest span id seen. This lookup algorithm relies on the assumption that there won't ever be more than 2 consecutive spans for which `span1.Id < span2.Id` and `span1.StartBlock > span2.StartBlock` . If this assumption were ever to be broken, we would have to fall back on brute-force linear search. Also, another detail to take into account is that due to the span rotations multiple spans with the same `StartBlock` could be produced, therefore overwriting the value of the index table at that block number. This is not a problem, because if this happens, then the previous span value will become entirely obsolete and no block number could have the old span associated with it. --------- Co-authored-by: antonis19 <[email protected]>
Follow-up to : #16683
The previous PR relied on the assumption that
span.StartBlockis monotonically increasing, but it turns out that assumption is not true due to a future span being announced in advance from Heimdall and a span rotation taking place before the announced span.e.g.
Therefore, this PR adapts the lookup algorithm by starting the search from
entry := cursor.Seek(blockNum)and walking theBorSpanstable backwards usingcursor.Prev()while recording the highest span id which is in range, until an out-of-range span is reached, at which point we break and return the highest span id seen.This lookup algorithm relies on the assumption that there won't ever be more than 2 consecutive spans for which
span1.Id < span2.Idandspan1.StartBlock > span2.StartBlock. If this assumption were ever to be broken, we would have to fall back on brute-force linear search.Also, another detail to take into account is that due to the span rotations multiple spans with the same
StartBlockcould be produced, therefore overwriting the value of the index table at that block number. This is not a problem, because if this happens, then the previous span value will become entirely obsolete and no block number could have the old span associated with it.