Conversation
📝 WalkthroughWalkthroughThe change adds defensive nil-check guards to the reorg detection service functions, validating that latestBlock and per-block fetch results are not nil before processing, with explicit error returns when nil values are encountered. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tx-submitter/services/reorg.go (1)
56-58: Consider centralizing block fetch + nil validation.The same fetch-and-nil-check logic appears in multiple places; a small helper would reduce duplication and keep error messages uniform.
♻️ Optional refactor sketch
+func (r *ReorgDetector) blockByNumberOrError(ctx context.Context, num *big.Int, label string) (*types.Block, error) { + block, err := r.l1Client.BlockByNumber(ctx, num) + if err != nil { + return nil, fmt.Errorf("failed to get %s: %w", label, err) + } + if block == nil { + return nil, fmt.Errorf("%s is nil", label) + } + return block, nil +}Also applies to: 67-69, 101-103, 117-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tx-submitter/services/reorg.go` around lines 56 - 58, Duplicate fetch-and-nil-check logic around latestBlock (e.g., the checks using latestBlock == nil at the shown location and also at the occurrences around lines 67-69, 101-103, 117-119) should be centralized: add a helper function (e.g., fetchLatestBlockOrErr or ensureLatestBlock) that performs the block fetch and returns (block, error) with a uniform error message when nil, then replace all inline fetch + nil checks with calls to that helper (update callers in reorg.go to use the helper and handle its error). Ensure the helper name appears in imports/signatures where used so callers are easy to find and the error text is consistent across all replaced sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tx-submitter/services/reorg.go`:
- Around line 56-58: Duplicate fetch-and-nil-check logic around latestBlock
(e.g., the checks using latestBlock == nil at the shown location and also at the
occurrences around lines 67-69, 101-103, 117-119) should be centralized: add a
helper function (e.g., fetchLatestBlockOrErr or ensureLatestBlock) that performs
the block fetch and returns (block, error) with a uniform error message when
nil, then replace all inline fetch + nil checks with calls to that helper
(update callers in reorg.go to use the helper and handle its error). Ensure the
helper name appears in imports/signatures where used so callers are easy to find
and the error text is consistent across all replaced sites.
Summary by CodeRabbit