feat(core): implement core/bcast

[iamquang95]

No description provided.

[iamquang95]

@claude review this PR

[claude]

Claude finished @iamquang95's task in 4m 6s —— View job

---

Review: feat(core): implement core/bcast

• Gather context (read PR diff & Go source)
• Review core/bcast against Charon parity
• Review eth2api/validator_duty & extensions
• Post review feedback

Summary

This is a faithful and well-tested port of Charon's core/bcast (broadcaster + recaster) plus the supporting eth2api submit/duty helpers. Duty routing, the Electra validator-index backfill workaround, PriorAttestationKnown swallowing, the per-epoch recast logic, metric names, and delay rounding all match the Go baseline. CI is green (build/test/lint/semver all pass). A few intentional deviations from Go are worth confirming and documenting; none are blocking.

Findings

• [Medium] broadcast_exits error propagation deviates from Go
  Impact: Changes the error contract for multi-exit sets.
  Evidence: crates/core/src/bcast/mod.rs:369-388
  Go reference: charon/core/bcast/bcast.go:239-257
  In Go, err is reassigned every loop iteration, so it returns the result of the last-iterated exit only (an earlier failure is lost if the last one succeeds), and it returns immediately on a type-assertion mismatch — potentially after already submitting earlier exits. The Rust version (a) pre-converts the whole set via set_to_exits(set)?, erroring before submitting anything if any item is the wrong type, and (b) keeps last_error which is never cleared, so it returns an error if any submit fails. The Rust behavior is arguably more correct (Go's is order-dependent and non-deterministic since the map order is random), but it's a deviation from the parity baseline. Recommend confirming this is intentional and adding a short comment noting the divergence. Fix this →

• [Low] broadcast_delay_seconds is observed conditionally, unlike Go
  Impact: Histogram observation is skipped if delay computation fails (overflow); the broadcast_total counter is still incremented.
  Evidence: crates/core/src/bcast/mod.rs:276-285, crates/core/src/bcast/metrics.rs:40-50
  Go reference: charon/core/bcast/bcast.go:48-52, metrics.go:53-56
  Go always observes delay together with the counter increment (its delayFunc cannot fail). Rust splits these so a successful broadcast is always counted even if delay is uncomputable. In normal operation both observe identically; only pathological arithmetic overflow diverges. This appears intentional (commit 6efe177). No action needed beyond confirming.

• [Low] resolve_active_validators_indices drops Go's explicit nil-validator check
  Impact: Different error surfaced for malformed validator data.
  Evidence: crates/core/src/bcast/mod.rs:587-613
  Go reference: charon/core/bcast/bcast.go:462-465
  Go returns "validator data cannot be nil"; Rust instead errors via InvalidValidatorField only when activation_epoch fails to parse. Functionally equivalent given Rust's non-nullable types — note only.

• [Low/Info] Electra-vs-pre-electra request shape is decided from the first element only
  Impact: Assumes homogeneous batches.
  Evidence: crates/eth2api/src/validator_duty.rs:333-335, 545-547
attestations_request_body and aggregate_and_proofs_request_body choose the wire shape based on attestations.first(). A mixed-version batch would then hit the explicit "pre-electra payload in electra request" / "electra payload in pre-electra request" error in the loop, so it fails cleanly rather than producing bad output. This matches Go's one-duty-one-version assumption — flagging the assumption only.

Parity matrix

Component	Go	Rust	Match	Notes
Duty routing (Broadcast)	present	present	yes	Deprecated/Randao/Prepare cases match
Electra validator-index backfill	present	present	yes	warn + verify + match-by-sig
PriorAttestationKnown swallow	strings.Contains	to_string().contains	yes
Attester delay slotDuration/3	*1/3	div_duration(_,3)	yes
Agg/SyncContrib delay *2/3	*2/3	mul-then-div	yes	rounding preserved (commit 018fd26)
firstSlotInCurrentEpoch	wraps on negative age	errors on negative age	partial	safer; future-genesis only
Recaster store / older-slot skip	>=	>=	yes
Recaster FirstInEpoch gate	present	present	yes
Metric names core_bcast_*	namespace+subsystem	prefix="core_bcast"	yes	matches codebase _total convention
recast source pregen/downstream	Slot > 0	inner() > 0	yes
Exit error propagation	last-iteration wins	any-failure wins	no	see Medium finding

Tests

• Not run locally. CI is green: Build and test code, Linter check, Semver checks, and create cluster Charon parity all pass (run 27532635119).
• Strong unit coverage added: duty routing, prior-known swallow, Electra backfill, set-conversion error strings, active-validator filtering, recaster epoch/active-set behavior, and to_consensus_version / resolve_fork_version tie-break tests.

Open questions

1. Is the broadcast_exits error-propagation change (any-failure vs Go's last-iteration) intentional? If so, a one-line comment would help future parity reviews.
2. Confirm the conditional delay observation is the desired metrics behavior (it diverges slightly from Go's always-observe).
   · branch iamquang95/core-bcast