Skip to content

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 20, 2025
Merged

VeBlop: Update span index lookup and add more unit tests #16729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8c49a8
update span index lookup and unit tests
antonis19 Aug 19, 2025
5182932
fix comment
antonis19 Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 31 additions & 31 deletions polygon/heimdall/snapshot_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ func TestHeimdallStoreEntity(t *testing.T) {
actualSpan, ok, err := heimdallStore.spans.Entity(t.Context(), expectedSpan.RawId())
require.NoError(t, err)
require.True(t, ok)
require.Equal(t, actualSpan.Id, expectedSpan.Id)
require.Equal(t, actualSpan.StartBlock, expectedSpan.StartBlock)
require.Equal(t, actualSpan.EndBlock, expectedSpan.EndBlock)
require.Equal(t, expectedSpan.Id, actualSpan.Id)
require.Equal(t, expectedSpan.StartBlock, actualSpan.StartBlock)
require.Equal(t, expectedSpan.EndBlock, actualSpan.EndBlock)
}
}

Expand Down Expand Up @@ -157,7 +157,7 @@ func TestHeimdallStoreLastFrozenIdWithSpanRotations(t *testing.T) {
lastFrozenId, found, err := heimdallStore.spans.LastFrozenEntityId()
require.NoError(t, err)
require.True(t, found)
require.Equal(t, lastFrozenId, uint64(9))
require.Equal(t, uint64(9), lastFrozenId)
}

func TestHeimdallStoreEntityWithSpanRotations(t *testing.T) {
Expand Down Expand Up @@ -358,54 +358,54 @@ var spanDataForTesting = []Span{

// span data that is irregular, containing possible span rotations
var spanDataWithRotations = []Span{
Span{
Span{ // first span
Id: 0,
StartBlock: 0,
EndBlock: 999,
},
Span{
Span{ // new span announced
Id: 1,
StartBlock: 5,
StartBlock: 1000,
EndBlock: 1999,
},
Span{
Span{ // span rotation
Id: 2,
StartBlock: 1988,
EndBlock: 2999,
StartBlock: 4,
EndBlock: 1999,
},
Span{
Span{ // span rotation
Id: 3,
StartBlock: 3000,
EndBlock: 3999,
StartBlock: 5,
EndBlock: 1999,
},
Span{
Span{ // span rotation
Id: 4,
StartBlock: 3500,
EndBlock: 4999,
StartBlock: 6,
EndBlock: 1999,
},
Span{
Span{ // new span announced
Id: 5,
StartBlock: 5000,
EndBlock: 5999,
StartBlock: 2000,
EndBlock: 2999,
},
Span{
Span{ // span rotation
Id: 6,
StartBlock: 5500,
EndBlock: 6999,
StartBlock: 11,
EndBlock: 1999,
},
Span{
Span{ // new span announced, this will have duplicate StartBlock
Id: 7,
StartBlock: 7000,
EndBlock: 7999,
StartBlock: 2000,
EndBlock: 2999,
},
Span{
Span{ // span rotation
Id: 8,
StartBlock: 7001,
EndBlock: 8999,
StartBlock: 3100,
EndBlock: 4999,
},
Span{
Span{ // span rotation
Id: 9,
StartBlock: 7002,
EndBlock: 9999,
StartBlock: 4600,
EndBlock: 5999,
},
}
51 changes: 27 additions & 24 deletions polygon/heimdall/span_range_index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,21 @@ func (i *txSpanRangeIndex) Put(ctx context.Context, r ClosedRange, id uint64) er
return tx.Put(i.table, key[:], valuePair[:])
}

// Returns max span.Id such that span.StartBlock <= blockNum && blockNum <= span.EndBlock
func (i *txSpanRangeIndex) Lookup(ctx context.Context, blockNum uint64) (uint64, bool, error) {
cursor, err := i.tx.Cursor(i.table)
if err != nil {
return 0, false, err
}
defer cursor.Close()

// use Seek(blockNum) as the starting point for the search.
key := rangeIndexKey(blockNum)
startBlockRaw, valuePair, err := cursor.Seek(key[:])
if err != nil {
return 0, false, err
}
// seek not found, we check the last entry
// seek not found, check the last entry as the only candidate
if valuePair == nil {
// get latest then
lastStartBlockRaw, lastValuePair, err := cursor.Last()
Expand All @@ -131,33 +133,34 @@ func (i *txSpanRangeIndex) Lookup(ctx context.Context, blockNum uint64) (uint64,

}

var lastSpanIdInRange = uint64(0)
currStartBlock := rangeIndexKeyParse(startBlockRaw)
// If currStartBlock == blockNum, then this span contains blockNum, and no need to do the .Prev() below
if currStartBlock == blockNum {
currSpanId, currEndBlock := rangeIndexValuePairParse(valuePair)
// sanityCheck
isInRange := blockNumInRange(blockNum, currStartBlock, currEndBlock)
if !isInRange {
return 0, false, fmt.Errorf("SpanIndexLookup(%d) returns Span{Id:%d, StartBlock:%d, EndBlock:%d } not containing blockNum=%d", blockNum, currSpanId, currStartBlock, currEndBlock, blockNum)
}
// happy case
return currSpanId, true, nil
currSpanId, currEndBlock := rangeIndexValuePairParse(valuePair)
isInRange := blockNumInRange(blockNum, currStartBlock, currEndBlock)
if isInRange { // cursor.Seek(blockNum) is in range
lastSpanIdInRange = currSpanId
}

// Prev should contain the appropriate span containing blockNum
prevStartBlockRaw, prevValuePair, err := cursor.Prev()
if err != nil {
return 0, false, err
}
prevStartBlock := rangeIndexKeyParse(prevStartBlockRaw)
spanId, endBlock := rangeIndexValuePairParse(prevValuePair)
// sanity check
isInRange := blockNumInRange(blockNum, prevStartBlock, endBlock)
if !isInRange {
return 0, false, fmt.Errorf("SpanIndexLookup(%d) returns Span{Id:%d, StartBlock:%d, EndBlock:%d } not containing blockNum=%d", blockNum, spanId, prevStartBlock, endBlock, blockNum)
for { // from this point walk backwards the table until the blockNum is out of range
prevStartBlockRaw, prevValuePair, err := cursor.Prev()
if err != nil {
return 0, false, err
}
// this could happen if we've walked all the way to the first entry in the table, and there is no more Prev()
if prevValuePair == nil {
break
}
prevStartBlock := rangeIndexKeyParse(prevStartBlockRaw)
prevSpanId, prevBlock := rangeIndexValuePairParse(prevValuePair)
isInRange := blockNumInRange(blockNum, prevStartBlock, prevBlock)
if !isInRange {
break // we have walked out of range, break to return current known lastSpanIdInRange
}
if isInRange && prevSpanId > lastSpanIdInRange { // a span in range with higher span id was found
lastSpanIdInRange = prevSpanId
}
}
Copy link

Copilot AI Aug 19, 2025

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.

Suggested change
}
}
if lastSpanIdInRange == 0 {
return 0, false, nil
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@antonis19 antonis19 Aug 19, 2025

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.

// happy case
return spanId, true, nil
return lastSpanIdInRange, true, nil
}

// last key in the index
Expand Down
Loading
Loading