Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.61%. Comparing base (a311f8c) to head (f8b1983).
⚠️ Report is 4 commits behind head on develop.

@@             Coverage Diff             @@
##           develop    #2029      +/-   ##
===========================================
+ Coverage    49.57%   49.61%   +0.03%     
===========================================
  Files          873      873              
  Lines       150507   150679     +172     
===========================================
+ Hits         74615    74759     +144     
- Misses       70860    70887      +27     
- Partials      5032     5033       +1     

Looks good overall! What long does it take to preload mainnet with the default throttling setting?

@cffls - with 500KB/s on our mainnet devnode, for a cache of 4096MB, it takes around 1h30m.

I think this 500KB/s default could be a bit too optimistic for the BPs, so we can bump the default values a bit, I'll check the logs and confirm.

Code Review

Issue found: Documentation discrepancy in core/blockchain.go line 203 and eth/ethconfig/config.go line 167.

The comments say default = 5MB/s but the actual implementation default is 500KB/s (see defaultPreloadRateLimit = 500 * 1024 in triedb/pathdb/config.go:47-49).

This is a 10x difference that could cause confusion for users configuring the node.

Suggested fix: Update the comments to say default = 500KB/s to match the actual implementation.

Checked for bugs and CLAUDE.md compliance.

@claude addressed your comments.
Review this PR again.

Claude finished @pratikspatil024's task in 1m 6s —— View job

Re-reviewing PR

• Gather context on the changes
• Review the updated code for the documentation discrepancy fix
• Check for any other issues
• Provide final review feedback

Review Summary

The previous documentation discrepancy has been fixed. The PR now consistently uses 1MB/s as the default rate limit across all files:

• core/blockchain.go:203 - ✅ Updated to default = 1MB/s
• eth/ethconfig/config.go:167 - ✅ Updated to default = 1MB/s
• triedb/pathdb/config.go:47-49 - ✅ Updated to defaultPreloadRateLimit = 1024 * 1024 (1MB/s)
• internal/cli/server/config.go:684 - ✅ Documents Default: 1MB/s
• internal/cli/server/flags.go:564 - ✅ Documents Default: 1MB
• Tests in config_test.go - ✅ Verify default of 1024*1024 bytes

Code Quality Assessment

Strengths:

1. Comprehensive rate limiting implementation - The rate.Limiter from golang.org/x/time/rate is used correctly with appropriate burst size (64KB)
2. Graceful shutdown handling - Context cancellation properly interrupts rate-limited operations via limiter.WaitN(c.ctx, len(nodeData))
3. Good test coverage - Tests cover rate limit creation, throttling behavior, interruption scenarios, and edge cases
4. Thread safety - Proper mutex usage protects concurrent access during async preloading

Implementation Details:

• Rate limiting is applied per-address during cache preloading
• The burst size of 64KB allows for smoother throttling
• Cache preloads up to 66.6% of capacity, leaving room for hot paths
• BFS traversal prioritizes shallow (frequently accessed) nodes

No issues found in this re-review.

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.5% Duplication on New Code

See analysis details on SonarQube Cloud