Skip to content

feat(cli): default --attestation-committee-count from validator-config.yaml #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MegaRedHand wants to merge 4 commits into
base: main
Choose a base branch
from
+147 −25
Open

feat(cli): default --attestation-committee-count from validator-config.yaml #337

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
abc0b75
feat(cli): default attestation_committee_count from validator-config.…
MegaRedHand May 1, 2026
6490483
test(cli): cover validator-config.yaml committee-count fallback
MegaRedHand May 1, 2026
8785a32
refactor: simplify code a bit
MegaRedHand May 1, 2026
421cbba
chore: fmt
MegaRedHand May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 147 additions & 25 deletions bin/ethlambda/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,13 @@ struct CliOptions {
/// use the admin endpoint to rotate duties (hot-standby model).
#[arg(long, default_value = "false")]
is_aggregator: bool,
/// Number of attestation committees (subnets) per slot
#[arg(long, default_value = "1", value_parser = clap::value_parser!(u64).range(1..))]
attestation_committee_count: u64,
/// Number of attestation committees (subnets) per slot.
///
/// If unset, falls back to `config.attestation_committee_count` from
/// `validator-config.yaml` in the network config dir, or `1` if that
/// field is also absent.
#[arg(long, value_parser = clap::value_parser!(u64).range(1..))]
attestation_committee_count: Option<u64>,
/// Subnet IDs this aggregator should subscribe to (comma-separated).
/// Requires --is-aggregator. Defaults to the subnets of the node's validators.
#[arg(long, value_delimiter = ',', requires = "is_aggregator")]
Expand All @@ -102,9 +106,6 @@ async fn main() -> eyre::Result<()> {
ethlambda_blockchain::metrics::init();
ethlambda_blockchain::metrics::set_node_info("ethlambda", version::CLIENT_VERSION);
ethlambda_blockchain::metrics::set_node_start_time();
ethlambda_blockchain::metrics::set_attestation_committee_count(
options.attestation_committee_count,
);

let api_socket = SocketAddr::new(options.http_address, options.api_port);
let metrics_socket = SocketAddr::new(options.http_address, options.metrics_port);
Expand Down Expand Up @@ -141,7 +142,20 @@ async fn main() -> eyre::Result<()> {
"Loaded genesis configuration"
);

populate_name_registry(&validator_config);
let validator_config_file = read_validator_config_file(&validator_config);
populate_name_registry(&validator_config_file);

// Resolve attestation_committee_count: CLI flag > validator-config.yaml > 1.
let attestation_committee_count = options
.attestation_committee_count
.or(validator_config_file.config.attestation_committee_count)
.unwrap_or(1);
info!(
attestation_committee_count,
"Loaded attestation committee count"
);
ethlambda_blockchain::metrics::set_attestation_committee_count(attestation_committee_count);

let bootnodes = read_bootnodes(&bootnodes_path);

let validator_keys =
Expand Down Expand Up @@ -181,7 +195,7 @@ async fn main() -> eyre::Result<()> {
bootnodes,
listening_socket: p2p_socket,
validator_ids,
attestation_committee_count: options.attestation_committee_count,
attestation_committee_count,
is_aggregator: options.is_aggregator,
aggregate_subnet_ids: options.aggregate_subnet_ids,
})
Expand Down Expand Up @@ -223,25 +237,40 @@ async fn main() -> eyre::Result<()> {
Ok(())
}

fn populate_name_registry(validator_config: impl AsRef<Path>) {
#[derive(Deserialize)]
struct Validator {
name: String,
privkey: H256,
}
#[derive(Deserialize)]
struct Config {
validators: Vec<Validator>,
}
let config_yaml =
std::fs::read_to_string(&validator_config).expect("Failed to read validator config file");
let config: Config =
serde_yaml_ng::from_str(&config_yaml).expect("Failed to parse validator config file");
/// Subset of `validator-config.yaml` consumed by ethlambda.
///
/// The `config` block is a network-wide settings bag shared across clients;
/// only fields ethlambda actually reads are deserialized. The `validators`
/// list feeds the metrics name registry.
#[derive(Debug, Deserialize)]
struct ValidatorConfigFile {
#[serde(default)]
config: ValidatorConfigBlock,
validators: Vec<ValidatorConfigEntry>,
}

#[derive(Debug, Default, Deserialize)]
struct ValidatorConfigBlock {
#[serde(default)]
attestation_committee_count: Option<u64>,
}

#[derive(Debug, Deserialize)]
struct ValidatorConfigEntry {
name: String,
privkey: H256,
}

fn read_validator_config_file(path: impl AsRef<Path>) -> ValidatorConfigFile {
let yaml = std::fs::read_to_string(&path).expect("Failed to read validator config file");
serde_yaml_ng::from_str(&yaml).expect("Failed to parse validator config file")
}

let names_and_privkeys = config
fn populate_name_registry(file: &ValidatorConfigFile) {
let names_and_privkeys = file
.validators
.into_iter()
.map(|v| (v.name, v.privkey))
.iter()
.map(|v| (v.name.clone(), v.privkey))
Comment on lines +269 to +273
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 YAML path bypasses the range(1..) lower-bound validator

When attestation_committee_count is supplied via the CLI, clap rejects any value below 1. When it comes from validator-config.yaml, there is no equivalent guard. A YAML entry of attestation_committee_count: 0 is silently accepted, reaches set_attestation_committee_count(0), and is forwarded into SwarmConfig — producing the same invalid state that the CLI validator was added to prevent.

Consider adding an assert!(val >= 1) (or returning an error) after resolving the value, so the invariant is enforced regardless of the source.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: bin/ethlambda/src/main.rs
Line: 266-270

Comment:
**YAML path bypasses the `range(1..)` lower-bound validator**

When `attestation_committee_count` is supplied via the CLI, clap rejects any value below 1. When it comes from `validator-config.yaml`, there is no equivalent guard. A YAML entry of `attestation_committee_count: 0` is silently accepted, reaches `set_attestation_committee_count(0)`, and is forwarded into `SwarmConfig` — producing the same invalid state that the CLI validator was added to prevent.

Consider adding an `assert!(val >= 1)` (or returning an error) after resolving the value, so the invariant is enforced regardless of the source.

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

.collect();

// Populates a dictionary used for labeling metrics with node names
Expand Down Expand Up @@ -478,3 +507,96 @@ async fn fetch_initial_state(
// Store the anchor state and header, without body
Ok(Store::from_anchor_state(backend, state))
}

#[cfg(test)]
mod tests {
use super::*;

/// Validator-config snippet matching `lean-quickstart`'s ansible-devnet
/// (devnet-4) where networks share a non-default committee count.
const VC_WITH_COMMITTEE_COUNT: &str = r#"
shuffle: roundrobin
deployment_mode: ansible
config:
activeEpoch: 18
keyType: "hash-sig"
attestation_committee_count: 2
validators:
- name: "ethlambda_0"
privkey: "299550529a79bc2dce003747c52fb0639465c893e00b0440ac66144d625e066a"
enrFields:
ip: "127.0.0.1"
quic: 9001
metricsPort: 9095
apiPort: 5055
subnet: 0
isAggregator: false
count: 1
"#;

/// Local-devnet snippet without the optional field — committee count is
/// expected to fall back to the binary default.
const VC_WITHOUT_COMMITTEE_COUNT: &str = r#"
shuffle: roundrobin
deployment_mode: local
config:
activeEpoch: 18
keyType: "hash-sig"
validators:
- name: "ethlambda_0"
privkey: "299550529a79bc2dce003747c52fb0639465c893e00b0440ac66144d625e066a"
enrFields:
ip: "127.0.0.1"
quic: 9001
metricsPort: 8087
apiPort: 5055
isAggregator: false
count: 1
"#;

#[test]
fn parses_committee_count_when_present() {
let file: ValidatorConfigFile = serde_yaml_ng::from_str(VC_WITH_COMMITTEE_COUNT).unwrap();
assert_eq!(file.config.attestation_committee_count, Some(2));
assert_eq!(file.validators.len(), 1);
assert_eq!(file.validators[0].name, "ethlambda_0");
}

#[test]
fn defaults_to_none_when_field_absent() {
let file: ValidatorConfigFile =
serde_yaml_ng::from_str(VC_WITHOUT_COMMITTEE_COUNT).unwrap();
assert_eq!(file.config.attestation_committee_count, None);
}

#[test]
fn cli_overrides_file_value() {
let file: ValidatorConfigFile = serde_yaml_ng::from_str(VC_WITH_COMMITTEE_COUNT).unwrap();
let cli_override: Option<u64> = Some(5);
let resolved = cli_override
.or(file.config.attestation_committee_count)
.unwrap_or(1);
assert_eq!(resolved, 5);
}

#[test]
fn falls_back_to_file_when_cli_absent() {
let file: ValidatorConfigFile = serde_yaml_ng::from_str(VC_WITH_COMMITTEE_COUNT).unwrap();
let cli_override: Option<u64> = None;
let resolved = cli_override
.or(file.config.attestation_committee_count)
.unwrap_or(1);
assert_eq!(resolved, 2);
}

#[test]
fn falls_back_to_default_when_neither_set() {
let file: ValidatorConfigFile =
serde_yaml_ng::from_str(VC_WITHOUT_COMMITTEE_COUNT).unwrap();
let cli_override: Option<u64> = None;
let resolved = cli_override
.or(file.config.attestation_committee_count)
.unwrap_or(1);
assert_eq!(resolved, 1);
}
}
Loading