Skip to content

cl/beacon: fix nil pointer panics in GetEthV1ValidatorAttestationData - #19783

Merged
lystopad merged 1 commit into
mainfrom
feature/lystopad/fix-attestation_data-http-missed-check
Mar 10, 2026
Merged

lystopad merged 1 commit into
mainfrom
feature/lystopad/fix-attestation_data-http-missed-check

Conversation

@lystopad

Copy link
Copy Markdown
Member

Summary

  • SyncedDataManager.CommitteeCount (synced_data.go): added accessLock.RLock() + nil check on headState, consistent with every other accessor in the same file. Fixes a panic when a validator client polls /eth/v1/validator/attestation_data before Caplin has synced to head. Also closes a check-then-act race in pool.go and committee_subscription.go where Syncing() is checked before CommitteeCount is called.
  • Debug-log defer (block_production.go): guard against nil committeeIndex in the deferred log closure, which is nil on two early-return paths — pre-Electra requests missing the committee_index query param, and Electra cache-hit returns (the committeeIndex = &zero assignment is bypassed by the cache-hit early return at line 158).

Reproduction

Start Erigon with a fresh caplin/ directory (or right after node restart) while a Lighthouse VC is actively polling. The VC calls GET /eth/v1/validator/attestation_data before Caplin reaches head → panic in HTTP handler goroutine with runtime error: invalid memory address or nil pointer dereference at CachingBeaconState.CommitteeCount(0x0, ...).

Test plan

  • go test ./cl/beacon/synced_data/... ./cl/beacon/handler/... -short passes
  • make lint clean
  • Observed panic no longer reproducible after fix

Generated with Claude.

Two nil dereferences during node startup (head state not yet available):

1. SyncedDataManager.CommitteeCount called s.headState.CommitteeCount
   without a nil check, despite every other headState accessor in the
   same file guarding with accessLock.RLock + nil check. Fixes the
   panic seen when a VC polls /eth/v1/validator/attestation_data before
   Caplin has synced to head. As a bonus, also closes a check-then-act
   race in pool.go and committee_subscription.go where Syncing() is
   checked before CommitteeCount is called.

2. The debug-log defer in GetEthV1ValidatorAttestationData
   unconditionally dereferenced committeeIndex, which is nil on two
   early-return paths: pre-Electra requests missing the committee_index
   query param, and Electra cache-hit returns (the committeeIndex = &zero
   assignment is skipped by the cache-hit early return).
   Guard the defer so it is a no-op when committeeIndex is nil.

Co-Authored-By: Claude
@lystopad lystopad self-assigned this Mar 10, 2026
@lystopad
lystopad enabled auto-merge (squash) March 10, 2026 19:05
@lystopad
lystopad merged commit 0f3624a into main Mar 10, 2026
37 checks passed
@lystopad
lystopad deleted the feature/lystopad/fix-attestation_data-http-missed-check branch March 10, 2026 21:55
lystopad added a commit that referenced this pull request Apr 16, 2026
…#19783)

## Summary

- **`SyncedDataManager.CommitteeCount`** (`synced_data.go`): added
`accessLock.RLock()` + nil check on `headState`, consistent with every
other accessor in the same file. Fixes a panic when a validator client
polls `/eth/v1/validator/attestation_data` before Caplin has synced to
head. Also closes a check-then-act race in `pool.go` and
`committee_subscription.go` where `Syncing()` is checked before
`CommitteeCount` is called.
- **Debug-log defer** (`block_production.go`): guard against nil
`committeeIndex` in the deferred log closure, which is nil on two
early-return paths — pre-Electra requests missing the `committee_index`
query param, and Electra cache-hit returns (the `committeeIndex = &zero`
assignment is bypassed by the cache-hit early return at line 158).

## Reproduction

Start Erigon with a fresh `caplin/` directory (or right after node
restart) while a Lighthouse VC is actively polling. The VC calls `GET
/eth/v1/validator/attestation_data` before Caplin reaches head → panic
in HTTP handler goroutine with `runtime error: invalid memory address or
nil pointer dereference` at `CachingBeaconState.CommitteeCount(0x0,
...)`.

## Test plan

- [x] `go test ./cl/beacon/synced_data/... ./cl/beacon/handler/...
-short` passes
- [x] `make lint` clean
- [x] Observed panic no longer reproducible after fix

Generated with Claude.
lystopad added a commit that referenced this pull request Apr 16, 2026
…ionData (#20600)

## Summary

Cherry-pick of #19783 from `main` to `release/3.4`.

Fixes a panic observed on `alex/collation_race_fix_34` (and
`release/3.4`) when a validator client polls
`/eth/v1/validator/attestation_data` before Caplin has synced to head.

- **`SyncedDataManager.CommitteeCount`** (`synced_data.go`): added
`accessLock.RLock()` + nil check on `headState`, consistent with every
other accessor in the same file.
- **Debug-log defer** (`block_production.go`): guard against nil
`committeeIndex` in the deferred log closure.

## Reproduction

Start Erigon on `release/3.4` while a validator client is actively
polling. The VC calls `GET /eth/v1/validator/attestation_data` before
Caplin reaches head → panic in HTTP handler goroutine:
```
panic: runtime error: invalid memory address or nil pointer dereference
github.com/erigontech/erigon/cl/phase1/core/state.(*CachingBeaconState).CommitteeCount(0x0, ...)
github.com/erigontech/erigon/cl/beacon/synced_data.(*SyncedDataManager).CommitteeCount(...)
github.com/erigontech/erigon/cl/beacon/handler.(*ApiHandler).GetEthV1ValidatorAttestationData.func1()
```

## Test plan

- [x] Clean cherry-pick from `main` (commit `0f3624a17b`)
- [x] `go test ./cl/beacon/synced_data/... ./cl/beacon/handler/...
-short` passes on main

Generated with Claude
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.

2 participants