Skip to content

fix(store): avoid panic when seeking with an empty prefix - #528

Open
memosr wants to merge 1 commit into
canopy-network:mainfrom
memosr:fix/versioned-iterator-empty-prefix-panic
Open

fix(store): avoid panic when seeking with an empty prefix#528
memosr wants to merge 1 commit into
canopy-network:mainfrom
memosr:fix/versioned-iterator-empty-prefix-panic

Conversation

@memosr

@memosr memosr commented Aug 18, 2026

Copy link
Copy Markdown

Problem

go test -race ./store/ cannot complete. The package aborts with a panic before

most tests get to run:

--- FAIL: TestNestedTxnMergedIteration (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered, repanicked]

github.com/cockroachdb/pebble/v2.testingDisableSeekOpt(...)
pebble/v2@v2.1.6/iterator.go:1521
github.com/cockroachdb/pebble/v2.(*mergingIter).seekGE(...)
pebble/v2@v2.1.6/merging_iter.go:958
github.com/canopy-network/canopy/store.(*VersionedIterator).first()
store/versioned_store.go:369
github.com/canopy-network/canopy/store.(*VersionedIterator).Valid()
store/versioned_store.go:321
github.com/canopy-network/canopy/store.(*TxnIterator).Valid()
store/txn.go:498

Root cause

VersionedIterator.first() hands vi.prefix directly to pebble's SeekGE.

When an iterator is created with no prefix, that value is not nil.

Txn.NewIterator builds the parent prefix with lib.Append(t.prefix, prefix),

and lib.Append allocates with make([]byte, len(a)+len(b)). For two empty

inputs that returns a non-nil zero length slice:

lib.Append(nil, nil)  // nil == false, len == 0

Pebble's seek key guard checks key != nil but not len(key) > 0, so a zero

length non-nil key passes the guard and then indexes key[0] on an empty slice.

The panic only surfaces under the invariants build that -race enables, so it

does not hit a production node. The cost is that the race detector is effectively

unusable on this package, which means genuine races in store stay invisible.

Change

Normalize an empty seek key to nil before passing it to SeekGE. Semantics are

unchanged, an empty prefix already meant start from the beginning.

Verification

Before:

$ go test -race ./store/
--- FAIL: TestNestedTxnMergedIteration (0.00s)
panic: runtime error: index out of range [0] with length 0
FAIL

After:

$ go test -race ./store/
ok github.com/canopy-network/canopy/store 11.446s

The whole package now runs clean under the race detector. gofmt clean.

Notes

Only the SeekGE path is changed. The reverse path calls SeekLT(prefixEnd(vi.prefix)),

and prefixEnd appends endBytes, so its key is never empty.

CONTRIBUTING.md asks for PRs against development, but no such branch exists on

the remote and recently merged PRs target main, so this targets main.

Happy to retarget.

Related: #527 fixes a separate data race in lib/crypto. The two are independent.

VersionedIterator.first() passes vi.prefix straight to pebble's SeekGE. When an
iterator is created with no prefix, that value is not nil: Txn.NewIterator builds
it with lib.Append(t.prefix, prefix), and lib.Append always returns
make([]byte, 0) for two empty inputs, which is a non-nil zero length slice.

Pebble guards its seek key with a nil check but not a length check, so a zero
length non-nil key gets past the guard and indexes into an empty slice:

  panic: runtime error: index out of range [0] with length 0
    pebble/v2.testingDisableSeekOpt(...)
      pebble/v2@v2.1.6/iterator.go:1521
    store.(*VersionedIterator).first()
      store/versioned_store.go:369

This aborts the whole package under -race, so no store test could run with the
race detector enabled.

Normalize an empty seek key to nil before handing it to SeekGE. The semantics are
unchanged: an empty prefix already meant start from the beginning.

Before:
  $ go test -race ./store/
  --- FAIL: TestNestedTxnMergedIteration (0.00s)
  panic: runtime error: index out of range [0] with length 0
  FAIL

After:
  $ go test -race ./store/
  ok  github.com/canopy-network/canopy/store 11.446s
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.

1 participant