Feat: make block-production intervals configurable - #1032
Conversation
📝 WalkthroughWalkthroughThe PR updates Morph L2 Tendermint replacements from v0.3.8 to v0.3.9. It also adds CLI flags for sequencer block intervals and applies them during consensus node startup. ChangesTendermint and sequencer configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant NodeStartup
participant tmsequencer
CLI->>NodeStartup: Provide sequencer interval flags
NodeStartup->>tmsequencer: Call SetBlockIntervals
tmsequencer-->>NodeStartup: Return validation result
NodeStartup->>NodeStartup: Log intervals or return startup error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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)
node/flags/flags.go (1)
218-232: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused coverage for the interval configuration contract.
Test the default values, environment-variable overrides, flag registration, and invalid ordering before startup. This protects the exact names used by
node/cmd/node/main.goand prevents configuration drift.Also applies to: 425-426
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node/flags/flags.go` around lines 218 - 232, Add focused tests for SequencerBlockInterval and SequencerFastBlockInterval covering their default durations, SEQUENCER_BLOCK_INTERVAL and SEQUENCER_FAST_BLOCK_INTERVAL environment overrides, registration under the exact flag names, and rejection of configurations where the fast interval is not less than the block interval before startup. Anchor the coverage to these flag definitions and the startup validation used by node/cmd/node/main.go.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@node/flags/flags.go`:
- Around line 218-232: Add focused tests for SequencerBlockInterval and
SequencerFastBlockInterval covering their default durations,
SEQUENCER_BLOCK_INTERVAL and SEQUENCER_FAST_BLOCK_INTERVAL environment
overrides, registration under the exact flag names, and rejection of
configurations where the fast interval is not less than the block interval
before startup. Anchor the coverage to these flag definitions and the startup
validation used by node/cmd/node/main.go.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c0d4a2c-7c4d-47a4-89d5-96f983c80cb7
⛔ Files ignored due to path filters (2)
node/go.sumis excluded by!**/*.sumops/tools/go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
Makefilebindings/go.modcommon/go.modcontracts/go.modnode/cmd/node/main.gonode/flags/flags.gonode/go.modops/l2-genesis/go.modops/tools/go.modtx-submitter/go.mod
Summary by CodeRabbit
New Features
Bug Fixes