feat(vara.eth): add --batch-commitment-period node knob#5606
feat(vara.eth): add --batch-commitment-period node knob#5606grishasobol wants to merge 4 commits into
Conversation
Add a node-local `batch_commitment_period: NonZeroU32` (CLI `--batch-commitment-period`, default 1). A validator only builds/validates a batch commitment on blocks whose `height % period == 0`, so commitments can be produced every Nth block instead of every block. Default 1 reproduces the previous every-block behavior. Threaded CLI -> NodeConfig -> ethexe-consensus ValidatorConfig/ValidatorCore; gate added in `Idle::maybe_advance_to_role` (height-based, applies to both coordinator and participant). Tests default to period 1; adds one consensus unit test (idle gate, period 2) and one service integration test (ping round-trips and every committed batch lands on a multiple-of-period height). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147tTHt6e7mhirSAPwNxRc3
Move the height gate out of the shared role-dispatch path and into the coordinator branch only. Participants now always enter the Participant role and validate whatever the coordinator chooses to commit, regardless of their own period; only the elected coordinator consults its local batch_commitment_period to decide whether a block is a commitment block. This makes the knob purely coordinator-local (like commitment_delay_limit) — nodes may run different values without losing quorum. Tests: rename the coordinator-gating test; add a participant test asserting it enters the role even on a non-multiple height with a huge local period. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147tTHt6e7mhirSAPwNxRc3
Address review: add the `batch-commitment-period` knob to `.ethexe.example.toml` and `.ethexe.example.local.toml`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147tTHt6e7mhirSAPwNxRc3
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request adds a new node-local configuration knob that allows the elected coordinator to throttle batch commitments to every Nth block. This change is designed to be purely local to the coordinator, ensuring that different nodes can operate with different commitment cadences without impacting consensus or requiring cross-node agreement. The implementation updates the configuration propagation path and adds logic in the consensus layer to conditionally skip the coordinator boot process based on block height. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a coordinator-local configuration parameter, batch_commitment_period, allowing coordinators to build batch commitments only on blocks whose height is a multiple of the configured period. Feedback on the changes suggests explicitly casting the period to u64 when calling is_multiple_of to ensure type correctness and compilation, and dynamically finding even and odd blocks in the unit tests to avoid relying on brittle hardcoded indices.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Address review: select the even/odd-height blocks for the coordinator gate test dynamically instead of by fixed index, so it doesn't depend on the mock chain's genesis-height parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147tTHt6e7mhirSAPwNxRc3
What
Adds a coordinator-local knob
batch_commitment_period: NonZeroU32(CLI flag--batch-commitment-period, default1). When a node is the electedcoordinator it only builds a
BatchCommitmenton Ethereum blocks whoseheight % batch_commitment_period == 0. With the default1the behavior isunchanged (commit every block); larger values commit every Nth block (every
2nd, 5th, 10th, 100th, ...).
How
DEFAULT_BATCH_COMMITMENT_PERIOD(= 1) inethexe-common.NodeParams→service::config::NodeConfig→ethexe_consensus::ValidatorConfig→ValidatorCore.Idle::maybe_advance_to_role(ethexe/consensus/.../idle.rs),inside the coordinator branch only: the elected coordinator drops back to
Idleon a non-multiple height. Participants are unaffected — they alwaysenter the role and validate whatever the coordinator commits. This keeps the
knob purely coordinator-local (like
commitment_delay_limit): different nodesmay run different values without losing quorum.
Tests
period = 1everywhere (no behavior change in existing tests).ethexe-consensus:batch_commitment_period_gates_coordinator_by_block_height(period 2 — ascoordinator: odd height stays
Idle, even height →CoordinatorBoot).participant_enters_regardless_of_batch_commitment_period(huge period —a non-coordinator still enters
Participanton a non-multiple height).ethexe-service:batch_commitment_period_commits_only_on_multiples(period 2, single validator/coordinator — a ping still round-trips and every
committed batch lands on a multiple-of-period height).
Local:
cargo fmt --all,cargo clippy --all-targets(clean), new tests +ethexe-consensus/ethexe-common/ethexe-clisuites pass; fullcargo nextest run -p "ethexe-*"passed on the prior revision.Operational note
The period is coordinator-local, so divergent values across nodes are safe.
Keep it small enough that every era's election window still contains several
multiple-of-period blocks (validator-set rotation has an election-window
deadline), and prefer
period <= uncommitted-chain-len-thresholdso theidle-chain checkpoint still fires close to its threshold. Default
1avoidsall of this.
🤖 Generated with Claude Code