feat(blockchain): cap attestations packed per built block

[MegaRedHand]

Summary

Adds a --max-attestations-per-block CLI flag (default 3) that bounds how many distinct AttestationData entries a proposer packs into a block it builds.

This is a proposer-side self-limit only. The consensus cap for accepting peers' blocks is unchanged (store.rs still rejects > MAX_ATTESTATIONS_DATA), so interop is preserved: other clients may still legally send up to MAX_ATTESTATIONS_DATA entries and we accept them. The configured value is clamped to MAX_ATTESTATIONS_DATA during selection, so we never build a block that peers would reject.

Since compaction collapses proofs to one entry per AttestationData, the limit equals the number of attestations in the built block (default 3).

Changes

File	Change
bin/ethlambda/src/cli.rs	New --max-attestations-per-block flag (default 3)
bin/ethlambda/src/main.rs	Thread it into BlockChain::spawn
crates/blockchain/src/lib.rs	spawn param, BlockChainServer field, pass at proposal time
crates/blockchain/src/store.rs	produce_block_with_signatures builds a ProposerConfig
crates/blockchain/src/block_builder.rs	New ProposerConfig struct; selection loop capped at N.min(MAX_ATTESTATIONS_DATA); new regression test

ProposerConfig groups the new limit with the existing enable_proposer_aggregation flag: without it, build_block exceeds clippy's too_many_arguments (8/7) limit.

Testing

• cargo clippy -p ethlambda-blockchain -p ethlambda --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• cargo test -p ethlambda-blockchain --lib — 40/40 pass, including the new build_block_respects_configured_attestation_limit (proves the limit binds below MAX_ATTESTATIONS_DATA)

Spec-test suites (forkchoice_spectests, etc.) were not run locally: they require the leanSpec/fixtures directory, which is not present in this worktree. They are unaffected by this change.

[github-actions]

🤖 Kimi Code Review

The PR is well-structured and correct. The proposer-side self-limit is safely implemented and cannot affect consensus.

Code Quality & Correctness

• Safe parameter propagation: The refactor from enable_proposer_aggregation: bool to ProposerConfig in block_builder.rs (lines 46-55, 77-89) is clean and maintains type safety.
• Correct clamping: The limit clamping at line 208-210 (max_attestations_per_block.min(MAX_ATTESTATIONS_DATA)) ensures the proposer never attempts to exceed the consensus-defined maximum, preventing invalid block production.
• Documentation: Doc comments clearly distinguish between the proposer-side limit and the consensus cap (on_block enforcement).

Testing

• Adequate coverage: The test build_block_respects_configured_attestation_limit (lines 1059-1149) correctly verifies that:
  1. The configured limit caps the output when the pool is larger (assertion line 1143)
  2. The limit is the binding constraint, not pool exhaustion (assertion line 1147)

Minor Observations

1. Default value consideration: The CLI default of 3 (cli.rs:107) is quite conservative for Ethereum consensus (mainnet blocks typically carry 64-128+ attestations). While appropriate for a minimalist client, operators should be aware this may reduce their block's reward and chain participation unless overridden. This is not a correctness issue.

2. Edge case handling: If max_attestations_per_block is set to 0, the proposer will pack no attestations. This is valid behavior (the block will still be accepted by peers if it meets other consensus rules), though suboptimal. Consider adding a CLI validation warning if 0 is unintended, but the current logic is safe.

Security Assessment

• Consensus-safe: Since this only affects build_block (local block production) and the value is clamped to MAX_ATTESTATIONS_DATA before use, it cannot cause the node to produce invalid blocks or reject valid ones.
• No overflow risks: The min operation between two usize values is safe.

Verdict: Approved. The implementation correctly adds a proposer-side configuration without touching consensus validation logic.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Codex Code Review

Findings

1. High: the new default silently reduces proposer packing from the consensus cap of 8 to 3. The flag is introduced with default_value = "3" in cli.rs, and selection now hard-stops at that limit in block_builder.rs. Since MAX_ATTESTATIONS_DATA is 8 in block.rs, every node that does not explicitly set the new flag will start proposing blocks with materially fewer attestations than before. That does not break block validity, but it does weaken fork-choice signal and can delay justification/finalization under load. If this is meant to be a tuning knob, the default should preserve existing behavior (MAX_ATTESTATIONS_DATA / 8) and only lower inclusion when explicitly configured.

2. Medium: --max-attestations-per-block is unvalidated, so 0 is accepted and produces empty-attestation blocks. The CLI field is a plain usize with no range check in cli.rs, and the builder turns that into for _round in 0..max_rounds in block_builder.rs. If 0 is not an intentional escape hatch, add clap::value_parser!(usize).range(1..); otherwise document explicitly that 0 disables attestation inclusion and is unsafe for normal operation.

The rest of the plumbing from CLI to build_block looks straightforward, and I did not spot a new memory-safety, signature-verification, or SSZ correctness issue in the patch itself.

I could not run the targeted Rust test locally because cargo/rustup tried to write under /home/runner/.rustup, which is read-only in this environment.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[greptile-apps]

Greptile Summary

This PR adds a proposer-side cap for attestations packed into locally built blocks. The main changes are:

• New --max-attestations-per-block CLI option.
• Plumbing from CLI startup into the blockchain server.
• ProposerConfig passed into block building.
• Selection loop capped below the consensus maximum.
• Regression coverage for a configured cap below MAX_ATTESTATIONS_DATA.

Confidence Score: 4/5

The changed flow is mostly safe, but the new CLI option allows a zero value that disables attestation packing.

• The new cap is threaded consistently through the proposer path.
• Values above the consensus maximum are clamped during selection.
• A zero value makes the selection loop skip all eligible attestations.

bin/ethlambda/src/cli.rs

Important Files Changed

Filename	Overview
bin/ethlambda/src/cli.rs	Adds the new CLI flag, but leaves the lower bound unconstrained.
bin/ethlambda/src/main.rs	Passes the parsed attestation cap into blockchain startup.
crates/blockchain/src/lib.rs	Stores the proposer cap and forwards it when proposing blocks.
crates/blockchain/src/store.rs	Builds ProposerConfig and passes it into block construction.
crates/blockchain/src/block_builder.rs	Applies the configured cap to attestation selection and adds regression coverage.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
bin/ethlambda/src/cli.rs:108-109
**Zero Limit Drops Attestations**

When the node is started with `--max-attestations-per-block 0`, this unconstrained `usize` reaches `select_attestations` and makes the new loop bound `0..0`. The proposer then signs and publishes a valid block with no attestations even when eligible payloads exist, silently removing all local attestation contribution from that slot.

```suggestion
    #[arg(long, default_value = "3", value_parser = clap::value_parser!(usize).range(1..))]
    pub(crate) max_attestations_per_block: usize,
```

_{Reviews (1): Last reviewed commit: "feat(blockchain): cap attestations packe..." | Re-trigger Greptile}

[github-actions]

🤖 Claude Code Review

Review: feat(blockchain): cap attestations packed per built block (PR 486)

Summary

Small, well-scoped change: adds a --max-attestations-per-block CLI flag (default 3) that limits how many distinct AttestationData entries a proposer packs into blocks it builds, while leaving the consensus-level acceptance cap (MAX_ATTESTATIONS_DATA) untouched. Overall the implementation is correct and the design decisions are sound.

Correctness

• The clamp max_attestations_per_block.min(MAX_ATTESTATIONS_DATA) at block_builder.rs:213 correctly guarantees a proposer can never build a block that its own peers (or itself, via on_block) would reject — the invariant the PR claims to preserve holds.
• max_attestations_per_block = 0 degrades gracefully: max_rounds becomes 0, the selection loop never executes, and an empty attestation list is produced without panicking. Worth confirming this is the desired behavior for a mistakenly-configured 0 (a validator that proposes empty-attestation blocks indefinitely), but it's not a correctness bug — just a config foot-gun. A value_parser with a minimum of 1 on the clap arg (cli.rs:108) would make misconfiguration fail fast at startup rather than silently producing degenerate blocks, but this is a minor nice-to-have, not a blocker.
• Threading of the new parameter through main.rs → BlockChain::spawn → BlockChainServer → produce_block_with_signatures → build_block → select_attestations is consistent; argument order at each call site matches the corresponding signature.
• Grouping the new field into ProposerConfig alongside enable_proposer_aggregation is a reasonable response to clippy's too_many_arguments, and keeps the two proposer-only knobs together conceptually.

Consensus-layer considerations

• No change to store.rs's block-acceptance path (on_block / MAX_ATTESTATIONS_DATA check) — confirmed unmodified apart from threading the new parameter into produce_block_with_signatures. This preserves interop: peers running unmodified nodes can still send up to MAX_ATTESTATIONS_DATA entries and this node will accept them.
• Fork-choice and STF paths (process_slots, process_block) are untouched; the limit only affects proposer-side attestation selection before the STF is simulated in build_block, so no new source of state-root mismatches is introduced.

Tests

• build_block_respects_configured_attestation_limit (block_builder.rs:1059-1191) is a solid regression test: it constructs a pool with more selectable entries (10) than the configured limit (3), and asserts the limited build stays within 1..=CONFIGURED_LIMIT while the unlimited build strictly exceeds it — this proves the limit is the binding constraint rather than candidate exhaustion. Good use of a comparative baseline instead of just asserting a static count.
• Existing call sites updated to pass MAX_ATTESTATIONS_DATA as the limit preserve prior test semantics (no behavior change for those tests).

Minor / nitpicks

• cli.rs:100-109 and block_builder.rs:55-58 both carry near-duplicate explanations of the proposer-side-limit-vs-consensus-cap distinction. Not wrong, just repeated across three or four doc comments (CLI flag, ProposerConfig field, build_block doc, select_attestations doc) — a minor documentation-maintenance burden if the semantics ever change, since all four would need updating in lockstep.
• Default value of 3 is asserted in the PR description as intentional but isn't explained in-code (why 3 specifically, e.g. relation to committee count or gossip size). Not a blocking concern, just something a future reader might wonder about.

No security, memory-safety, or performance concerns — this is a pure selection-loop bound with no unsafe code, no new allocations of consequence, and no change to hot-path signature verification or aggregation logic.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt