Deterministic state syncs #2177
Open
Claude / Claude Code Review
completed
Apr 4, 2026 in 17m 52s
Code review found 2 important issues
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 2 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | consensus/bor/heimdall/failover_client.go:121-129 |
hasDeadline fix ineffective: timeout must be set before callWithFailover |
| 🔴 Important | consensus/bor/heimdall/failover_client.go:121-129 |
hasDeadline fix is ineffective: paginated timeout set after callWithFailover already caps it |
Annotations
Check failure on line 129 in consensus/bor/heimdall/failover_client.go
claude / Claude Code Review
hasDeadline fix ineffective: timeout must be set before callWithFailover
The `hasDeadline` fix in `callWithFailover` is ineffective because paginated methods set their 1-minute timeout **inside** the lambda (after `callWithFailover` has already applied its 30s cap), not before calling `callWithFailover`. The comment at line 192-195 is factually wrong: `CommitStates` passes `c.ctx` with no deadline, so `callWithFailover` always applies the 30s `subCtx`, and the inner `context.WithTimeout(subCtx, 1*time.Minute)` inside `HeimdallClient.StateSyncEventsByTime` cannot exte
Check failure on line 129 in consensus/bor/heimdall/failover_client.go
claude / Claude Code Review
hasDeadline fix is ineffective: paginated timeout set after callWithFailover already caps it
The hasDeadline fix in callWithFailover is logically inverted and ineffective: the 1-minute pagination timeout is set inside the transport implementation (within the lambda), after callWithFailover has already applied the 30-second attemptTimeout cap to the context. Because Go contexts can only narrow deadlines, the effective timeout for StateSyncEventsByTime (and StateSyncEventsAtHeight) on any multi-endpoint (MultiHeimdallClient) validator is 30 seconds, not the intended 60 seconds. The fix re
Loading