Skip to content

Add offset support to MemoryFreezer #1965

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

Open
LarryArnault45 wants to merge 8 commits into
base: develop
Choose a base branch
from
+195 −17
Open

Add offset support to MemoryFreezer #1965

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3b32cf6
Add offset support to MemoryFreezer
LarryArnault45 Jan 5, 2026
6097d13
Fix NewMemoryFreezer call sites to include offset parameter
LarryArnault45 Jan 31, 2026
f74d4ac
chore: trigger CI
LarryArnault45 Mar 20, 2026
9716b10
Fix MemoryFreezer offset handling and trienode freezer call sites
LarryArnault45 Apr 13, 2026
0cd3033
Merge develop into memory-freezer-offset
LarryArnault45 Apr 13, 2026
907e9aa
ci: skip fork-only checks and bump Go to 1.26.2
LarryArnault45 Apr 13, 2026
d917c1e
Merge develop into memory-freezer-offset
LarryArnault45 May 7, 2026
11439ca
rawdb: align memory freezer offset handling
LarryArnault45 May 15, 2026
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
4 changes: 2 additions & 2 deletions core/rawdb/ancient_scheme.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ var freezers = []string{
// state freezer.
func NewStateFreezer(ancientDir string, verkle bool, readOnly bool) (ethdb.ResettableAncientStore, error) {
if ancientDir == "" {
return NewMemoryFreezer(readOnly, stateFreezerTableConfigs), nil
return NewMemoryFreezer(readOnly, 0, stateFreezerTableConfigs), nil
}
var name string
if verkle {
Expand All @@ -140,7 +140,7 @@ func NewStateFreezer(ancientDir string, verkle bool, readOnly bool) (ethdb.Reset
// trienode freezer.
func NewTrienodeFreezer(ancientDir string, verkle bool, readOnly bool) (ethdb.ResettableAncientStore, error) {
if ancientDir == "" {
return NewMemoryFreezer(readOnly, trienodeFreezerTableConfigs), nil
return NewMemoryFreezer(readOnly, 0, trienodeFreezerTableConfigs), nil
}
var name string
if verkle {
Expand Down
2 changes: 1 addition & 1 deletion core/rawdb/chain_freezer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func NewChainFreezer(datadir string, namespace string, readonly bool, offset uin
func newChainFreezer(datadir string, eraDir string, namespace string, readonly bool, offset uint64) (*chainFreezer, error) {
if datadir == "" {
return &chainFreezer{
ancients: NewMemoryFreezer(readonly, chainFreezerTableConfigs),
ancients: NewMemoryFreezer(readonly, offset, chainFreezerTableConfigs),
quit: make(chan struct{}),
trigger: make(chan chan struct{}),
}, nil
Expand Down
54 changes: 43 additions & 11 deletions core/rawdb/freezer_memory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
"github.com/ethereum/go-ethereum/rlp"
)

var errTruncateBelowOffset = errors.New("truncation below offset")

// memoryTable is used to store a list of sequential items in memory.
type memoryTable struct {
items uint64 // Number of stored items in the table, including the deleted ones
Expand Down Expand Up @@ -149,7 +151,7 @@
b.size = make(map[string]int64)

for name, table := range freezer.tables {
b.next[name] = table.items
b.next[name] = table.items + freezer.offset.Load()
}
}

Expand Down Expand Up @@ -215,16 +217,24 @@
}

// NewMemoryFreezer initializes an in-memory freezer instance.
func NewMemoryFreezer(readonly bool, tableName map[string]freezerTableConfig) *MemoryFreezer {
func NewMemoryFreezer(readonly bool, offset uint64, tableName map[string]freezerTableConfig) *MemoryFreezer {
tables := make(map[string]*memoryTable)
for name, cfg := range tableName {
tables[name] = newMemoryTable(name, cfg)
}
return &MemoryFreezer{
freezer := &MemoryFreezer{
writeBatch: newMemoryBatch(),
readonly: readonly,
tables: tables,
}

freezer.offset.Store(offset)

// Some blocks in ancientDB may have already been frozen and been pruned, so adding the offset to
// represent the absolute number of blocks already frozen.
freezer.items += offset

return freezer
}
Comment on lines 219 to 238

// todo: @anshalshukla || @manav2401 - Check if implementation is required
Expand All @@ -240,7 +250,11 @@
f.lock.RLock()
defer f.lock.RUnlock()

return f.items - f.offset.Load(), nil
offset := f.offset.Load()
if f.items < offset {
return 0, nil
}
return f.items - offset, nil
}

// Ancient retrieves an ancient binary blob from the in-memory freezer.
Expand All @@ -249,13 +263,17 @@
defer f.lock.RUnlock()

t := f.tables[kind]
if t == nil {
return nil, errUnknownTable
}
data, err := t.retrieve(number, 1, 0)
offset := f.offset.Load()
if number < offset {
return nil, errOutOfBounds
}
data, err := t.retrieve(number-offset, 1, 0)
if err != nil {
return nil, err
}

Check failure on line 276 in core/rawdb/freezer_memory.go

View check run for this annotation

Claude / Claude Code Review

AncientBytes left non-offset-aware while Ancient/AncientRange were updated 

MemoryFreezer.AncientBytes (core/rawdb/freezer_memory.go:467-490) was not updated to be offset-aware, while this PR adds offset translation to Ancient and AncientRange. With offset > 0, AncientBytes silently disagrees with Ancient/AncientRange: e.g. AncientBytes("test", 12, 0, n) on a freezer with offset=10 and items at absolute ids 10..12 hits the t.items<=start guard and returns errOutOfBounds, while AncientBytes("test", 2, 0, n) — below the absolute start — would silently return data for what
Comment on lines 266 to 276
Copy link
Copy Markdown

@claude claude Bot May 15, 2026

Choose a reason for hiding this comment

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

🔴 MemoryFreezer.AncientBytes (core/rawdb/freezer_memory.go:467-490) was not updated to be offset-aware, while this PR adds offset translation to Ancient and AncientRange. With offset > 0, AncientBytes silently disagrees with Ancient/AncientRange: e.g. AncientBytes("test", 12, 0, n) on a freezer with offset=10 and items at absolute ids 10..12 hits the t.items<=start guard and returns errOutOfBounds, while AncientBytes("test", 2, 0, n) — below the absolute start — would silently return data for what is actually item 12. Fix: subtract f.offset.Load() from id with the same underflow guard the PR adds for Ancient/AncientRange/TruncateHead/TruncateTail, and add a TestMemoryFreezerOffset case for AncientBytes.

Extended reasoning...

The bug

This PR makes MemoryFreezer offset-aware, updating Ancient, AncientRange, TruncateHead, TruncateTail, ModifyAncients, Reset and the write-batch initializer to translate between absolute item numbers and table-relative indices via f.offset.Load(). However, the parallel reader MemoryFreezer.AncientBytes (core/rawdb/freezer_memory.go:467-490) — which is not in the diff — was not updated. It still calls table.retrieve(id, 1, 0) with the absolute id.

After this PR, AncientBytes is the only AncientReaderOp method left non-offset-aware, so a MemoryFreezer constructed with offset > 0 produces inconsistent results across its reader methods.

Step-by-step proof

Construct f := NewMemoryFreezer(false, 10, {"test": …}) and append three items at absolute ids 10, 11, 12 (matching the new TestMemoryFreezerOffset). After commit, the underlying memoryTable has t.items = 3 and t.offset = 0 (the memoryTable.items field is incremented only by commit and starts at 0 — unlike the freezer-level f.items which now includes the offset).

  1. f.Ancient("test", 12) → updated path: number=12, offset=10, calls t.retrieve(12-10, 1, 0) = t.retrieve(2, 1, 0) → returns "twelve". ✓
  2. f.AncientBytes("test", 12, 0, n) → unchanged path: calls t.retrieve(12, 1, 0) → trips the t.items <= start guard (3 <= 12) → returns errOutOfBounds. ✗ (disagrees with Ancient)
  3. f.AncientBytes("test", 2, 0, n) (an id below the freezer's absolute start of 10) → calls t.retrieve(2, 1, 0) → silently returns "twelve" (the entry at relative index 2, i.e. absolute id 12). ✗ (worse: wrong data, not even an error)

Why existing code does not prevent it

The new TestMemoryFreezerOffset exercises Ancient, AncientRange, TruncateHead, TruncateTail, and Reset against offset=10, but does not call AncientBytes, so this gap is not caught. chainFreezer.AncientBytes (core/rawdb/chain_freezer.go:451) is a direct pass-through, and newChainFreezer now wires a non-zero offset into NewMemoryFreezer for the in-memory chain freezer case, so the inconsistency is reachable through the public ethdb.AncientStore interface even if no current caller of AncientBytes happens to use an offset != 0 today (state/trienode constructors still pass 0; chain reads currently route through Ancient, not AncientBytes).

Impact

Latent today (no current production caller triggers it), but a real correctness/consistency bug introduced by this PR's offset refactor. Any future caller — or any future code path that routes chain-table reads through AncientBytes while the chain freezer is configured with a pruning offset — would either silently read the wrong item (when id < offset) or get a spurious errOutOfBounds (when id >= offset). The contract that all reader methods agree on absolute ids is broken on the same struct.

Suggested fix

Apply the same translation and underflow guard already used for Ancient and AncientRange:

func (f *MemoryFreezer) AncientBytes(kind string, id, offset, length uint64) ([]byte, error) {
    f.lock.RLock()
    defer f.lock.RUnlock()

    table := f.tables[kind]
    if table == nil {
        return nil, errUnknownTable
    }
    tableOffset := f.offset.Load()
    if id < tableOffset {
        return nil, errOutOfBounds
    }
    entries, err := table.retrieve(id-tableOffset, 1, 0)
    // … rest unchanged
}

And extend TestMemoryFreezerOffset with AncientBytes("test", 12, …) succeeding and AncientBytes("test", 9, …) returning errOutOfBounds, mirroring the existing Ancient/AncientRange assertions.

return data[0], nil
}

Expand All @@ -273,7 +291,11 @@
if t == nil {
return nil, errUnknownTable
}
return t.retrieve(start, count, maxBytes)
offset := f.offset.Load()
if start < offset {
return nil, errOutOfBounds
}
return t.retrieve(start-offset, count, maxBytes)
}

// Ancients returns the ancient item numbers in the freezer.
Expand Down Expand Up @@ -322,18 +344,20 @@
return 0, errReadOnly
}
// Roll back all tables to the starting position in case of error.
old := f.items
offset := f.offset.Load()
defer func(old uint64) {
if err == nil {
return
}
// The write operation has failed. Go back to the previous item position.
for name, table := range f.tables {
err := table.truncateHead(old)
err := table.truncateHead(old - offset)
if err != nil {
log.Error("Freezer table roll-back failed", "table", name, "index", old, "err", err)
}
}
}(f.items)
}(old)

// Modify the ancients in batch.
f.writeBatch.reset(f)
Expand Down Expand Up @@ -361,8 +385,12 @@
if old <= items {
return old, nil
}
offset := f.offset.Load()
if items < offset {
return 0, errTruncateBelowOffset
}
for _, table := range f.tables {
if err := table.truncateHead(items); err != nil {
if err := table.truncateHead(items - offset); err != nil {
return 0, err
}
}
Comment on lines 386 to 396
Expand All @@ -384,9 +412,13 @@
if old >= tail {
return old, nil
}
offset := f.offset.Load()
if tail < offset {
return 0, errTruncateBelowOffset
}
for _, table := range f.tables {
if table.config.prunable {
if err := table.truncateTail(tail); err != nil {
if err := table.truncateTail(tail - offset); err != nil {
return 0, err
}
}
Comment on lines 414 to 424
Expand Down Expand Up @@ -422,7 +454,7 @@
tables[name] = newMemoryTable(name, table.config)
}
f.tables = tables
f.items, f.tail = 0, 0
f.items, f.tail = f.offset.Load(), 0
return nil
}

Expand Down
131 changes: 129 additions & 2 deletions core/rawdb/freezer_memory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package rawdb

import (
"bytes"
"errors"
"testing"

"github.com/ethereum/go-ethereum/core/rawdb/ancienttest"
Expand All @@ -32,7 +34,7 @@
prunable: true,
}
}
return NewMemoryFreezer(false, tables)
return NewMemoryFreezer(false, 0, tables)
})
ancienttest.TestResettableAncientSuite(t, func(kinds []string) ethdb.ResettableAncientStore {
tables := make(map[string]freezerTableConfig)
Expand All @@ -42,6 +44,131 @@
prunable: true,
}
}
return NewMemoryFreezer(false, tables)
return NewMemoryFreezer(false, 0, tables)
})
}

func TestMemoryFreezerOffset(t *testing.T) {

Check failure on line 51 in core/rawdb/freezer_memory_test.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 36 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=0xPolygon_bor&issues=AZ4BHB4ZaCITuand2R_0&open=AZ4BHB4ZaCITuand2R_0&pullRequest=1965
freezer := NewMemoryFreezer(false, 10, map[string]freezerTableConfig{
"test": {noSnappy: true, prunable: true},
})
if got := freezer.AncientOffSet(); got != 10 {
t.Fatalf("AncientOffSet() = %d, want 10", got)
}
if got, err := freezer.Ancients(); err != nil {
t.Fatalf("Ancients() returned error: %v", err)
} else if got != 10 {
t.Fatalf("Ancients() = %d, want 10", got)
}
if got, err := freezer.ItemAmountInAncient(); err != nil {
t.Fatalf("ItemAmountInAncient() returned error: %v", err)
} else if got != 0 {
t.Fatalf("ItemAmountInAncient() = %d, want 0", got)
}

if _, err := freezer.ModifyAncients(func(op ethdb.AncientWriteOp) error {
return op.AppendRaw("test", 0, []byte("zero"))
}); !errors.Is(err, errOutOrderInsertion) {
t.Fatalf("AppendRaw before offset error = %v, want %v", err, errOutOrderInsertion)
}

if _, err := freezer.ModifyAncients(func(op ethdb.AncientWriteOp) error {
if err := op.AppendRaw("test", 10, []byte("ten")); err != nil {
return err
}
if err := op.AppendRaw("test", 11, []byte("eleven")); err != nil {
return err
}
return op.AppendRaw("test", 12, []byte("twelve"))
}); err != nil {
t.Fatalf("ModifyAncients() returned error: %v", err)
}

if got, err := freezer.Ancients(); err != nil {
t.Fatalf("Ancients() returned error: %v", err)
} else if got != 13 {
t.Fatalf("Ancients() = %d, want 13", got)
}
if got, err := freezer.ItemAmountInAncient(); err != nil {
t.Fatalf("ItemAmountInAncient() returned error: %v", err)
} else if got != 3 {
t.Fatalf("ItemAmountInAncient() = %d, want 3", got)
}

if blob, err := freezer.Ancient("test", 10); err != nil {
t.Fatalf("Ancient(10) returned error: %v", err)
} else if !bytes.Equal(blob, []byte("ten")) {
t.Fatalf("Ancient(10) = %q, want %q", blob, []byte("ten"))
}
if _, err := freezer.Ancient("test", 9); !errors.Is(err, errOutOfBounds) {
t.Fatalf("Ancient(9) error = %v, want %v", err, errOutOfBounds)
}

if batch, err := freezer.AncientRange("test", 10, 2, 0); err != nil {
t.Fatalf("AncientRange(10, 2, 0) returned error: %v", err)
} else if len(batch) != 2 || !bytes.Equal(batch[0], []byte("ten")) || !bytes.Equal(batch[1], []byte("eleven")) {
t.Fatalf("AncientRange(10, 2, 0) = %q, want %q", batch, [][]byte{[]byte("ten"), []byte("eleven")})
}
if _, err := freezer.AncientRange("test", 9, 1, 0); !errors.Is(err, errOutOfBounds) {
t.Fatalf("AncientRange(9, 1, 0) error = %v, want %v", err, errOutOfBounds)
}

if _, err := freezer.TruncateHead(9); !errors.Is(err, errTruncateBelowOffset) {
t.Fatalf("TruncateHead(9) error = %v, want %v", err, errTruncateBelowOffset)
}
if _, err := freezer.TruncateTail(9); !errors.Is(err, errTruncateBelowOffset) {
t.Fatalf("TruncateTail(9) error = %v, want %v", err, errTruncateBelowOffset)
}

if old, err := freezer.TruncateTail(11); err != nil {
t.Fatalf("TruncateTail(11) returned error: %v", err)
} else if old != 0 {
t.Fatalf("TruncateTail(11) old tail = %d, want 0", old)
}
if blob, err := freezer.Ancient("test", 11); err != nil {
t.Fatalf("Ancient(11) returned error: %v", err)
} else if !bytes.Equal(blob, []byte("eleven")) {
t.Fatalf("Ancient(11) = %q, want %q", blob, []byte("eleven"))
}
if _, err := freezer.Ancient("test", 10); !errors.Is(err, errOutOfBounds) {
t.Fatalf("Ancient(10) after tail truncation error = %v, want %v", err, errOutOfBounds)
}

if old, err := freezer.TruncateHead(12); err != nil {
t.Fatalf("TruncateHead(12) returned error: %v", err)
} else if old != 13 {
t.Fatalf("TruncateHead(12) old head = %d, want 13", old)
}
if got, err := freezer.Ancients(); err != nil {
t.Fatalf("Ancients() after head truncation returned error: %v", err)
} else if got != 12 {
t.Fatalf("Ancients() after head truncation = %d, want 12", got)
}
if _, err := freezer.Ancient("test", 12); !errors.Is(err, errOutOfBounds) {
t.Fatalf("Ancient(12) after head truncation error = %v, want %v", err, errOutOfBounds)
}
}

func TestMemoryFreezerResetKeepsOffset(t *testing.T) {
freezer := NewMemoryFreezer(false, 7, map[string]freezerTableConfig{
"test": {noSnappy: true, prunable: true},
})
if err := freezer.Reset(); err != nil {
t.Fatalf("Reset() returned error: %v", err)
}
if got, err := freezer.Ancients(); err != nil {
t.Fatalf("Ancients() returned error: %v", err)
} else if got != 7 {
t.Fatalf("Ancients() = %d, want 7", got)
}
if got, err := freezer.ItemAmountInAncient(); err != nil {
t.Fatalf("ItemAmountInAncient() returned error: %v", err)
} else if got != 0 {
t.Fatalf("ItemAmountInAncient() = %d, want 0", got)
}
if _, err := freezer.ModifyAncients(func(op ethdb.AncientWriteOp) error {
return op.AppendRaw("test", 7, []byte("seven"))
}); err != nil {
t.Fatalf("ModifyAncients() after reset returned error: %v", err)
}
}
Loading