Skip to content

fix nil pointer panic in check_consensus_sync_status#157

Open
yperbasis wants to merge 1 commit intoethpandaops:masterfrom
yperbasis:fix/nil-sync-status
Open

fix nil pointer panic in check_consensus_sync_status#157
yperbasis wants to merge 1 commit intoethpandaops:masterfrom
yperbasis:fix/nil-sync-status

Conversation

@yperbasis
Copy link
Copy Markdown

Summary

  • Fix nil pointer dereference in check_consensus_sync_status when GetNodeSyncStatus returns an error
  • When the RPC call fails (e.g. during network bootstrap before genesis), syncStatus is nil, but getClientInfo dereferences it unconditionally — causing a panic
  • The analogous check_execution_sync_status task already has this nil guard; this applies the same pattern

Panic trace from CI

panic: runtime error: invalid memory address or nil pointer dereference

goroutine 172 [running]:
github.com/ethpandaops/assertoor/pkg/tasks/check_consensus_sync_status.(*Task).getClientInfo(...)
    pkg/tasks/check_consensus_sync_status/task.go:219
github.com/ethpandaops/assertoor/pkg/tasks/check_consensus_sync_status.(*Task).processCheck(...)
    pkg/tasks/check_consensus_sync_status/task.go:142

Observed in Erigon CI glamsterdam Kurtosis tests: https://github.com/erigontech/erigon/actions/runs/24228480466/job/70734695754

Test plan

  • Verify check_consensus_sync_status no longer panics when a CL client is unreachable during startup
  • Verify ClientInfo for failed clients has Name populated and sync fields zeroed

When GetNodeSyncStatus returns an error, syncStatus is nil, but
getClientInfo dereferences it unconditionally. This causes a panic
when a consensus client is temporarily unreachable (e.g. during
initial network bootstrap before genesis).

The execution sync status task already handles this correctly with
a nil guard; apply the same pattern here.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Optimistic: syncStatus.IsOptimistic,
SyncHead: syncStatus.HeadSlot,
SyncDistance: syncStatus.SyncDistance,
Name: client.Config.Name,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider setting variables explicitly rather than letting golang use defaults here:

       if syncStatus == nil {                                                                                                                                                                                                                                   
           // RPC failed - return skeletal info without dereferencing                                                                                                                                                                                           
           return &ClientInfo{                                                                                                                                                                                                                                  
               Name:          client.Config.Name,                                                                                                                                                                                                               
               Synchronizing: true,  // Sentinel: we don't know, assume unhealthy                                                                                                                                                                               
               SyncHead:      0,                                                                                                                                                                                                                                
               SyncDistance:  0,                                                                                                                                                                                                                                
           }                                                                                                                                                                                                                                                    
       }  

@qu0b qu0b force-pushed the master branch 2 times, most recently from 1d5b002 to 4321d7d Compare April 11, 2026 11:01
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