Skip to content

fix(statetracker): return error from NewStateTracker instead of nil-deref#2283

Open
SAY-5 wants to merge 2 commits into
lavanet:mainfrom
SAY-5:say5/state-tracker-nil-2282
Open

fix(statetracker): return error from NewStateTracker instead of nil-deref#2283
SAY-5 wants to merge 2 commits into
lavanet:mainfrom
SAY-5:say5/state-tracker-nil-2282

Conversation

@SAY-5

@SAY-5 SAY-5 commented May 2, 2026

Copy link
Copy Markdown

Fixes #2282.

NewStateTracker ignored the error from chaintracker.NewChainTracker and immediately called StartAndServe on the returned tracker. When NewChainTracker fails (e.g., bad node URL), cst.chainTracker is nil and the next line panics during lavap startup. Return the error instead.

…ing nil chainTracker

Signed-off-by: SAY-5 <say.apm35@gmail.com>
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Return error from NewStateTracker instead of nil dereference

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Return error from NewStateTracker when chaintracker.NewChainTracker fails
• Prevent nil pointer dereference on chainTracker.StartAndServe call
• Fix panic during lavap startup with invalid node URL
Diagram
flowchart LR
  A["NewChainTracker fails"] -->|"error returned"| B["Check error immediately"]
  B -->|"error exists"| C["Return nil, error"]
  B -->|"no error"| D["Call StartAndServe safely"]
  D --> E["Return cst, nil"]
Loading

Grey Divider

File Changes

1. protocol/statetracker/state_tracker.go 🐞 Bug fix +4/-1

Add error handling for chainTracker initialization

• Added error check after chaintracker.NewChainTracker call
• Return early with error if NewChainTracker fails
• Changed final return from return cst, err to return cst, nil
• Prevents calling StartAndServe on nil chainTracker

protocol/statetracker/state_tracker.go


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Ignored StartAndServe error🐞 Bug ☼ Reliability
Description
NewStateTracker calls cst.chainTracker.StartAndServe(ctx) but ignores its returned error. If chain
tracker initialization fails (e.g., initial latest-block fetch fails), NewStateTracker still returns
(cst, nil), leaving a StateTracker that won’t receive block updates and fails silently downstream.
Code

protocol/statetracker/state_tracker.go[R289-291]

  cst.chainTracker.StartAndServe(ctx)
  cst.chainTracker.RegisterForBlockTimeUpdates(cst) // registering for block time updates.
-	return cst, err
+	return cst, nil
Evidence
StartAndServe is defined to return an error and will return non-nil when initialization fails
(ct.start -> fetchInitDataWithRetry). NewStateTracker currently discards that error and returns
success anyway.

protocol/statetracker/state_tracker.go[285-291]
protocol/chaintracker/chain_tracker.go[28-44]
protocol/chaintracker/chain_tracker.go[625-633]
protocol/chaintracker/chain_tracker.go[409-421]
protocol/chaintracker/chain_tracker.go[482-497]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`NewStateTracker` calls `cst.chainTracker.StartAndServe(ctx)` but does not check the returned error. If chain tracker initialization fails, `NewStateTracker` still returns `(cst, nil)`.
### Issue Context
`chaintracker.IChainTracker.StartAndServe(ctx)` returns an error when initialization fails (e.g., initial latest-block fetch fails). Failing to propagate this error can create a `StateTracker` that never receives block updates.
### Fix Focus Areas
- protocol/statetracker/state_tracker.go[248-292]
- protocol/chaintracker/chain_tracker.go[625-633]
### Suggested fix
Capture and handle the error:
- `if err := cst.chainTracker.StartAndServe(ctx); err != nil { return nil, <wrapped err> }`
- Keep `RegisterForBlockTimeUpdates` only after successful start.
- Wrap with context (e.g., `utils.LavaFormatError("failed starting chain tracker", err)`) so startup logs are actionable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Missing error context wrapping🐞 Bug ◔ Observability
Description
NewStateTracker returns the raw error from chaintracker.NewChainTracker without adding a descriptive
message. This makes startup failures harder to triage compared to other call sites that wrap the
same error with context.
Code

protocol/statetracker/state_tracker.go[R285-288]

  cst.chainTracker, err = chaintracker.NewChainTracker(ctx, chainFetcher, chainTrackerConfig)
+	if err != nil {
+		return nil, err
+	}
Evidence
NewStateTracker returns err directly, while a nearby/related statetracker flow wraps the same
constructor failure with a contextual message. Keeping errors consistent improves diagnosability of
configuration/spec problems (like Invalid average block time).

protocol/statetracker/state_tracker.go[284-288]
protocol/statetracker/events.go[121-125]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`NewStateTracker` returns the raw error from `chaintracker.NewChainTracker` without wrapping it in a higher-level message.
### Issue Context
Other call sites in `protocol/statetracker` wrap `NewChainTracker` failures with a descriptive message (e.g., "failed setting up chain tracker"). Consistent wrapping helps operators quickly identify where/why startup failed.
### Fix Focus Areas
- protocol/statetracker/state_tracker.go[275-291]
- protocol/statetracker/events.go[121-125]
### Suggested fix
When `NewChainTracker` returns an error, wrap it with context, e.g.:
- `return nil, utils.LavaFormatError("failed setting up chain tracker", err)`
Optionally include useful attributes (config values) if that’s a common pattern in this repo.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread protocol/statetracker/state_tracker.go Outdated
@SAY-5 SAY-5 changed the title statetracker: return error from NewStateTracker instead of nil-deref fix(statetracker): return error from NewStateTracker instead of nil-deref May 2, 2026
…errors

Signed-off-by: SAY-5 <say.apm35@gmail.com>
@SAY-5

SAY-5 commented May 3, 2026

Copy link
Copy Markdown
Author

Addressed in 63bea24: now check StartAndServe error and wrap both chaintracker errors with utils.LavaFormatError for actionable startup logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: lavap crashes on startup for NEART provider

1 participant