Add --attestation-committee-count CLI parameter by pablodeymo · Pull Request #141 · lambdaclass/ethlambda

pablodeymo · 2026-02-23T19:07:01Z

Motivation

ATTESTATION_COMMITTEE_COUNT was hardcoded to 1 in crates/blockchain/src/lib.rs, meaning all validators shared a single attestation subnet. Future devnets will use multiple committees (other clients like Zeam and Lantern already support this parameter), so the value needs to be configurable at runtime.

Description

Add a --attestation-committee-count CLI argument (default 1) that controls how many attestation subnets exist. Validators are assigned to subnets via validator_id % attestation_committee_count.

Changes:

bin/ethlambda/src/main.rs: Add --attestation-committee-count CLI argument with default value 1, pass it to start_p2p()
crates/blockchain/src/lib.rs: Remove the ATTESTATION_COMMITTEE_COUNT constant (no longer needed since the value comes from CLI)
crates/net/p2p/src/lib.rs: Accept attestation_committee_count as a function parameter instead of importing the constant, use it for subnet ID calculation

The hardcoded constant is removed entirely since clap provides the default. The blockchain crate doesn't store the value because it has no internal use for it — only the P2P layer needs it for subnet subscription.

How to test

Run with explicit value: --attestation-committee-count 1 (should behave identically to before)
Run without the flag (default 1, same behavior)
make lint and make test pass with no warnings

Updates the pinned leanSpec commit hash to pick up the latest spec changes.

leanSpec commit 0340cc1 changed XMSS Signature serialization from structured JSON (path/rho/hashes sub-objects) to hex-encoded SSZ bytes ("0x..."). Replace the old ProposerSignature struct and its SSZ reconstruction logic with a simple hex deserializer (deser_xmss_hex), matching the existing deser_pubkey_hex pattern.

…lidation Port two validation rules from leanSpec 8b7636b: reject attestations where the head checkpoint is older than the target, and reject attestations where the head checkpoint slot doesn't match the actual block slot. Well-formed attestations already satisfy these, but without them we'd accept malformed ones.

At interval 3, the migration step (interval 4) hasn't run yet, so attestations that entered "known" directly (proposer's own attestation in block body, node's self-attestation) were invisible to the safe target calculation. Merge both pools to avoid undercounting support. No double-counting risk since extract_attestations_from_aggregated_payloads deduplicates by validator ID.

After aggregating committee signatures at interval 2, broadcast the resulting SignedAggregatedAttestation messages to the network. aggregate_committee_signatures and on_tick now return the new aggregates, and BlockChainServer::on_tick publishes them via P2PMessage::PublishAggregatedAttestation. The variant already existed but was never sent from the aggregation path.

… attestation subnets configurable. Previously hardcoded to 1, this allows future devnets to use multiple committees. The value is passed to the P2P layer for subnet subscription (validator_id % committee_count).

github-actions · 2026-02-23T19:07:45Z

🤖 Kimi Code Review

Review Summary

The PR makes attestation_committee_count configurable via CLI instead of using a hardcoded constant. This is a good change for testnet flexibility, but there are a few issues:

1. Division by Zero Risk (Critical)

File: crates/net/p2p/src/lib.rs:180

let subnet_id = validator_id.map(|vid| vid % attestation_committee_count);

If attestation_committee_count is set to 0 via CLI, this will panic with division by zero. Add validation:

if attestation_committee_count == 0 {
    tracing::error!("attestation_committee_count must be > 0");
    return;
}

2. CLI Validation Missing

File: bin/ethlambda/src/main.rs:58
The CLI argument accepts any u64 value without validation. Add a validator:

#[arg(long, default_value = "1", value_parser = clap::value_parser!(u64).range(1..))]
attestation_committee_count: u64,

3. Default Value Consistency

The hardcoded constant was 1, and the CLI default is also 1. This is consistent, but consider documenting why this specific value is the default.

4. Code Style

The #[allow(clippy::too_many_arguments)] on line 59 is justified given the function signature complexity.

5. Documentation

Consider adding documentation to the CLI argument explaining that this affects subnet calculation for validators.

File: bin/ethlambda/src/main.rs:56-58

/// Number of attestation committees (subnets) per slot
/// Validators will be assigned to a subnet based on validator_id % attestation_committee_count
#[arg(long, default_value = "1", value_parser = clap::value_parser!(u64).range(1..))]
attestation_committee_count: u64,

The changes are otherwise correct and follow Rust best practices.

Automated review by Kimi (Moonshot AI) · custom prompt

greptile-apps · 2026-02-23T19:09:16Z

Greptile Summary

This PR successfully makes the attestation committee count configurable via the --attestation-committee-count CLI parameter. The hardcoded constant has been cleanly removed and replaced with runtime configuration.

Key changes:

Added CLI parameter with proper validation (minimum value of 1) to prevent division by zero
Removed hardcoded ATTESTATION_COMMITTEE_COUNT constant from crates/blockchain/src/lib.rs
Updated P2P layer to accept committee count as a function parameter
Subnet assignment logic validator_id % attestation_committee_count now uses the configurable value

Minor note:

One comment in crates/blockchain/src/store.rs:411 still references the old constant name (non-blocking)

Confidence Score: 5/5

This PR is safe to merge with no risk
The changes are well-structured, minimal, and focused. The CLI validation prevents the division-by-zero issue. All file modifications are straightforward refactoring with no logic changes beyond making the value configurable. The default behavior (value=1) preserves existing functionality.
No files require special attention

Important Files Changed

Filename	Overview
bin/ethlambda/src/main.rs	Added `--attestation-committee-count` CLI parameter with proper validation (range 1..) to prevent division by zero. Parameter is correctly passed to `start_p2p()`.
crates/blockchain/src/lib.rs	Removed hardcoded `ATTESTATION_COMMITTEE_COUNT` constant as it's now configurable via CLI. Clean removal with no side effects.
crates/net/p2p/src/lib.rs	Updated `start_p2p()` to accept `attestation_committee_count` parameter and use it for subnet ID calculation. Properly replaces the hardcoded constant import.

_{Last reviewed commit: ea37b6d}

greptile-apps

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T19:09:20Z

crates/net/p2p/src/lib.rs

-    // ATTESTATION_COMMITTEE_COUNT is 1 for devnet-3 but will increase in future devnets.
-    #[allow(clippy::modulo_one)]
-    let subnet_id = validator_id.map(|vid| vid % ATTESTATION_COMMITTEE_COUNT);
+    let subnet_id = validator_id.map(|vid| vid % attestation_committee_count);


modulo by zero will panic if attestation_committee_count is 0

Suggested change

let subnet_id = validator_id.map(|vid| vid % attestation_committee_count);

let subnet_id = validator_id.and_then(|vid| {

if attestation_committee_count == 0 {

None

} else {

Some(vid % attestation_committee_count)

}

});

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/net/p2p/src/lib.rs Line: 180 Comment: modulo by zero will panic if `attestation_committee_count` is 0 ```suggestion let subnet_id = validator_id.and_then(|vid| { if attestation_committee_count == 0 { None } else { Some(vid % attestation_committee_count) } }); ``` How can I resolve this? If you propose a fix, please make it concise.

…ivision by zero in subnet calculation

pablodeymo · 2026-02-23T19:13:46Z

@greptile-apps review it again please

greptile-apps

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T19:15:56Z

Additional Comments (1)

crates/blockchain/src/store.rs
outdated comment references ATTESTATION_COMMITTEE_COUNT constant which was removed

        // When attestation_committee_count=1, all validators are in the same subnet.

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/blockchain/src/store.rs
Line: 411

Comment:
outdated comment references `ATTESTATION_COMMITTEE_COUNT` constant which was removed

```suggestion
        // When attestation_committee_count=1, all validators are in the same subnet.
```

How can I resolve this? If you propose a fix, please make it concise.

… by CLI validation

MegaRedHand · 2026-02-25T17:53:35Z

crates/net/p2p/src/lib.rs

    pub(crate) last_peer: Option<PeerId>,
 }

+#[allow(clippy::too_many_arguments)]


We should move the arguments into some P2PConfig struct.

pablodeymo added 9 commits February 18, 2026 16:27

Bump leanSpec commit to 8b7636b

ce8572e

Updates the pinned leanSpec commit hash to pick up the latest spec changes.

Run cargo fmt

e0b9aaf

Fix gossip config

ff04494

changing devnet3 to devnet0 for network in gossip messages

a10a4d4

greptile-apps bot reviewed Feb 23, 2026

View reviewed changes

Reject --attestation-committee-count 0 at CLI parse time to prevent d…

ea37b6d

…ivision by zero in subnet calculation

greptile-apps bot reviewed Feb 23, 2026

View reviewed changes

Add comment clarifying attestation_committee_count >= 1 is guaranteed…

a6f6a5e

… by CLI validation

Base automatically changed from fix-gossip to devnet-3 February 25, 2026 17:51

Merge branch 'devnet-3' into add-attestation-committee-count-cli

c35a5a6

MegaRedHand approved these changes Feb 25, 2026

View reviewed changes

MegaRedHand reviewed Feb 25, 2026

View reviewed changes

MegaRedHand merged commit c32eab7 into devnet-3 Feb 25, 2026
2 checks passed

MegaRedHand deleted the add-attestation-committee-count-cli branch February 25, 2026 17:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --attestation-committee-count CLI parameter#141

Add --attestation-committee-count CLI parameter#141
MegaRedHand merged 12 commits intodevnet-3from
add-attestation-committee-count-cli

pablodeymo commented Feb 23, 2026

Uh oh!

github-actions bot commented Feb 23, 2026

Uh oh!

greptile-apps bot commented Feb 23, 2026 •

edited

Loading

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 23, 2026

Uh oh!

pablodeymo commented Feb 23, 2026

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot commented Feb 23, 2026

Uh oh!

MegaRedHand Feb 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    let subnet_id = validator_id.map(|vid| vid % attestation_committee_count);
+    let subnet_id = validator_id.and_then(|vid| {
+        if attestation_committee_count == 0 {
+            None
+        } else {
+            Some(vid % attestation_committee_count)
+        }
+    });

Conversation

pablodeymo commented Feb 23, 2026

Motivation

Description

How to test

Uh oh!

github-actions bot commented Feb 23, 2026

🤖 Kimi Code Review

Review Summary

1. Division by Zero Risk (Critical)

2. CLI Validation Missing

3. Default Value Consistency

4. Code Style

5. Documentation

Uh oh!

greptile-apps bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

pablodeymo commented Feb 23, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 23, 2026

Uh oh!

MegaRedHand Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps bot commented Feb 23, 2026 •

edited

Loading