Skip to content

Commit 10db765

Browse files
committed
refactor: drop incidental churn from the heartbeat change
A pass over the branch diff for changes not required by the heartbeat work. All are behavior-preserving; the point is to shrink what a reviewer reads. - `build_block` keeps main's positional signature. The `BlockTarget` parameter object wrapped five existing arguments in order to add one, so `heartbeat_committee_size` moves to `ProposerConfig` instead, which every call site already builds. It is read once at startup: the persisted value is authoritative from first boot and cannot change at runtime. - `SlotInterval::HeadUpdate` reverts to `EndOfSlot`, which was also colliding with the pre-existing `store::HeadUpdate` struct. - `lean_fast_head_slot` is gone. It was set from the same expression as `lean_head_slot` two lines above, so it carried no information of its own; `lean_lagging_head_slot` stays. - `lagging_head` drops off the fork-choice RPC response and the Hive driver snapshot, and `RLMD_LOOKBACK_LIMIT` off `/config/spec`; nothing read them. `HEARTBEAT_COMMITTEE_SIZE` stays, being the cross-client agreement check. - The `DEFAULT_`/`MAX_HEARTBEAT_COMMITTEE_SIZE` re-export from `ethlambda-state-transition` is gone. `MAX_` was never reached through it and `DEFAULT_` only by two tests, both in crates already depending on `ethlambda-types`. - Dead `seed_covered` parameter on `select_fold_children`, whose sole caller passed an empty set. - An `on_stopped` doc comment reflowed to identical text, a stray blank line, and a `try_finalize` test covering logic this branch does not touch. `JobSource` stays despite looking like more of the same: it is the only thing routing prover time into `lean_heartbeat_fold_time_seconds`, the metric that says whether a chosen `K` still fits inside its interval.
1 parent 11ee711 commit 10db765

12 files changed

Lines changed: 64 additions & 177 deletions

File tree

bin/ethlambda/src/main.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,9 @@ async fn main() -> eyre::Result<()> {
262262
proposer_config: ProposerConfig {
263263
enable_proposer_aggregation: options.enable_proposer_aggregation,
264264
max_attestations_per_block: options.max_attestations_per_block,
265+
// Read once here rather than per build: the persisted value is
266+
// authoritative from first boot and cannot change at runtime.
267+
heartbeat_committee_size: store.heartbeat_committee_size(),
265268
},
266269
};
267270

crates/blockchain/src/block_builder.rs

Lines changed: 38 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,10 @@ pub struct ProposerConfig {
5656
/// Proposer-side self-limit only; clamped to `MAX_ATTESTATIONS_DATA` during
5757
/// selection so the block never exceeds the cap `on_block` enforces.
5858
pub max_attestations_per_block: usize,
59-
}
60-
61-
/// Identity and chain context of the block being built: everything that names
62-
/// *which* block this is, as opposed to *what* goes in it (the candidate payloads)
63-
/// or *how much* goes in it (the [`ProposerConfig`] policy).
64-
///
65-
/// Grouped rather than passed positionally because `slot` is load-bearing in two
66-
/// places at once — it is the block's own slot and, minus one, the slot whose
67-
/// committee the heartbeat tier packs — and because the three fields describing
68-
/// the parent are only meaningful together.
69-
pub(crate) struct BlockTarget<'a> {
70-
pub(crate) head_state: &'a State,
71-
pub(crate) slot: u64,
72-
pub(crate) proposer_index: u64,
73-
pub(crate) parent_root: H256,
74-
pub(crate) known_block_roots: &'a HashSet<H256>,
7559
/// The network's `K`. Used only to classify entries into
7660
/// [`Tier::Heartbeat`], never to validate anything, so a stale value builds a
7761
/// worse block rather than an invalid one.
78-
pub(crate) heartbeat_committee_size: u64,
62+
pub heartbeat_committee_size: u64,
7963
}
8064

8165
/// Build a valid block on top of this state.
@@ -103,18 +87,15 @@ pub(crate) struct BlockTarget<'a> {
10387
/// clamped to `MAX_ATTESTATIONS_DATA` so the block never exceeds the cap
10488
/// `on_block` enforces on incoming blocks.
10589
pub(crate) fn build_block(
106-
target: BlockTarget<'_>,
90+
head_state: &State,
91+
slot: u64,
92+
proposer_index: u64,
93+
parent_root: H256,
94+
known_block_roots: &HashSet<H256>,
10795
aggregated_payloads: &HashMap<H256, (AttestationData, Vec<SingleMessageAggregate>)>,
10896
config: ProposerConfig,
10997
) -> Result<(Block, Vec<SingleMessageAggregate>, PostBlockCheckpoints), StoreError> {
110-
let BlockTarget {
111-
head_state,
112-
slot,
113-
proposer_index,
114-
parent_root,
115-
known_block_roots,
116-
heartbeat_committee_size,
117-
} = target;
98+
let heartbeat_committee_size = config.heartbeat_committee_size;
11899
info!(slot, proposer_index, "Building block");
119100

120101
let select_start = Instant::now();
@@ -1065,7 +1046,7 @@ fn trace_skipped_attestation(reason: &'static str, att: &AttestationData, data_r
10651046
#[cfg(test)]
10661047
mod tests {
10671048
use super::*;
1068-
use ethlambda_state_transition::DEFAULT_HEARTBEAT_COMMITTEE_SIZE;
1049+
use ethlambda_types::constants::DEFAULT_HEARTBEAT_COMMITTEE_SIZE;
10691050
use ethlambda_types::{
10701051
attestation::{AggregatedAttestation, AggregationBits, AttestationData},
10711052
block::{ByteList512KiB, MultiMessageAggregate, SignedBlock, SingleMessageAggregate},
@@ -1629,18 +1610,16 @@ mod tests {
16291610

16301611
// Build the block; this should succeed (the bug: no size guard)
16311612
let (block, signatures, _post_checkpoints) = build_block(
1632-
BlockTarget {
1633-
head_state: &head_state,
1634-
slot,
1635-
proposer_index,
1636-
parent_root,
1637-
known_block_roots: &known_block_roots,
1638-
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
1639-
},
1613+
&head_state,
1614+
slot,
1615+
proposer_index,
1616+
parent_root,
1617+
&known_block_roots,
16401618
&aggregated_payloads,
16411619
ProposerConfig {
16421620
enable_proposer_aggregation: true,
16431621
max_attestations_per_block: MAX_ATTESTATIONS_DATA,
1622+
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
16441623
},
16451624
)
16461625
.expect("build_block should succeed");
@@ -1778,18 +1757,16 @@ mod tests {
17781757

17791758
let build = |limit: usize| {
17801759
build_block(
1781-
BlockTarget {
1782-
head_state: &head_state,
1783-
slot,
1784-
proposer_index,
1785-
parent_root,
1786-
known_block_roots: &known_block_roots,
1787-
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
1788-
},
1760+
&head_state,
1761+
slot,
1762+
proposer_index,
1763+
parent_root,
1764+
&known_block_roots,
17891765
&aggregated_payloads,
17901766
ProposerConfig {
17911767
enable_proposer_aggregation: false,
17921768
max_attestations_per_block: limit,
1769+
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
17931770
},
17941771
)
17951772
.expect("build_block should succeed")
@@ -1907,18 +1884,16 @@ mod tests {
19071884
aggregated_payloads.insert(data_root, (att_data.clone(), proofs));
19081885

19091886
let (block, signatures, _post_checkpoints) = build_block(
1910-
BlockTarget {
1911-
head_state: &head_state,
1912-
slot,
1913-
proposer_index,
1914-
parent_root,
1915-
known_block_roots: &known_block_roots,
1916-
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
1917-
},
1887+
&head_state,
1888+
slot,
1889+
proposer_index,
1890+
parent_root,
1891+
&known_block_roots,
19181892
&aggregated_payloads,
19191893
ProposerConfig {
19201894
enable_proposer_aggregation: false,
19211895
max_attestations_per_block: MAX_ATTESTATIONS_DATA,
1896+
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
19221897
},
19231898
)
19241899
.expect("build_block should succeed");
@@ -2216,18 +2191,16 @@ mod tests {
22162191
known_block_roots.insert(hashes[0]);
22172192

22182193
let (block, _signatures, post_checkpoints) = build_block(
2219-
BlockTarget {
2220-
head_state: &head_state,
2221-
slot,
2222-
proposer_index,
2223-
parent_root,
2224-
known_block_roots: &known_block_roots,
2225-
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
2226-
},
2194+
&head_state,
2195+
slot,
2196+
proposer_index,
2197+
parent_root,
2198+
&known_block_roots,
22272199
&aggregated_payloads,
22282200
ProposerConfig {
22292201
enable_proposer_aggregation: true,
22302202
max_attestations_per_block: MAX_ATTESTATIONS_DATA,
2203+
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
22312204
},
22322205
)
22332206
.expect("build_block should succeed");
@@ -2355,18 +2328,16 @@ mod tests {
23552328
known_block_roots.insert(hashes[0]);
23562329

23572330
let (block, _signatures, post_checkpoints) = build_block(
2358-
BlockTarget {
2359-
head_state: &head_state,
2360-
slot,
2361-
proposer_index,
2362-
parent_root,
2363-
known_block_roots: &known_block_roots,
2364-
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
2365-
},
2331+
&head_state,
2332+
slot,
2333+
proposer_index,
2334+
parent_root,
2335+
&known_block_roots,
23662336
&aggregated_payloads,
23672337
ProposerConfig {
23682338
enable_proposer_aggregation: true,
23692339
max_attestations_per_block: MAX_ATTESTATIONS_DATA,
2340+
heartbeat_committee_size: DEFAULT_HEARTBEAT_COMMITTEE_SIZE,
23702341
},
23712342
)
23722343
.expect("build_block should succeed");

crates/blockchain/src/heartbeat_fold.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,7 @@ fn resolve_fold_job(
125125
known_proofs: &[SingleMessageAggregate],
126126
validators: &[Validator],
127127
) -> Option<AggregationJob> {
128-
let (children, accepted_child_ids) =
129-
select_fold_children(new_proofs, known_proofs, HashSet::new(), validators);
128+
let (children, accepted_child_ids) = select_fold_children(new_proofs, known_proofs, validators);
130129
let covered: HashSet<u64> = accepted_child_ids.iter().copied().collect();
131130

132131
// B \ A, in ascending validator order (XMSS aggregation requires it, which
@@ -187,10 +186,9 @@ fn resolve_fold_job(
187186
fn select_fold_children(
188187
new_proofs: &[SingleMessageAggregate],
189188
known_proofs: &[SingleMessageAggregate],
190-
seed_covered: HashSet<u64>,
191189
validators: &[Validator],
192190
) -> (Vec<(Vec<ValidatorPublicKey>, ByteList512KiB)>, Vec<u64>) {
193-
let mut covered = seed_covered;
191+
let mut covered: HashSet<u64> = HashSet::new();
194192
let mut children = Vec::new();
195193
let mut child_ids: Vec<u64> = Vec::new();
196194

crates/blockchain/src/lib.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,9 @@ pub const GOSSIP_DISPARITY_INTERVALS: u64 = 1;
9292
/// The four ticks inside a slot.
9393
///
9494
/// Declared in wall-clock order; the discriminant is the interval index within
95-
/// the slot. `Aggregation` and `EndOfSlot` from the 5-interval grid are gone:
96-
/// committee aggregation is anchored to the interval-2 boundary rather than
97-
/// occupying a tick of its own, and the end-of-slot duties (promote payloads,
98-
/// log the tree, build the next block) fold into [`Self::HeadUpdate`].
95+
/// the slot. The 5-interval grid's `Aggregation` tick is gone: committee
96+
/// aggregation is now anchored to the interval-2 boundary rather than occupying
97+
/// a tick of its own, so its duties fold into [`Self::SafeTargetUpdate`].
9998
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
10099
pub(crate) enum SlotInterval {
101100
/// 0 ms. The slot's block arrives; import merges committee bits into the
@@ -109,7 +108,7 @@ pub(crate) enum SlotInterval {
109108
SafeTargetUpdate,
110109
/// 3000 ms. `lagging_head` then `fast_head`; promote payloads, log the tree,
111110
/// and build + publish the next slot's block aligned to its slot boundary.
112-
HeadUpdate,
111+
EndOfSlot,
113112
}
114113

115114
impl SlotInterval {
@@ -122,7 +121,7 @@ impl SlotInterval {
122121
0 => Self::BlockPublication,
123122
1 => Self::AttestationProduction,
124123
2 => Self::SafeTargetUpdate,
125-
3 => Self::HeadUpdate,
124+
3 => Self::EndOfSlot,
126125
_ => unreachable!("slots only have {INTERVALS_PER_SLOT} intervals"),
127126
}
128127
}
@@ -354,7 +353,7 @@ impl BlockChainServer {
354353
// needs (those stragglers surface in the `late` section instead). Skip
355354
// empty snapshots so a missed round keeps the last set we saw. Pure
356355
// observability.
357-
if interval == SlotInterval::HeadUpdate
356+
if interval == SlotInterval::EndOfSlot
358357
&& let Some(snapshot) = coverage::snapshot_new_payloads(&self.store)
359358
{
360359
self.pre_merge_coverage = Some(snapshot);
@@ -469,7 +468,7 @@ impl BlockChainServer {
469468
// `on_tick` skips the interval-0 tick whenever this build overruns
470469
// its interval. The head update itself runs inside `store::on_tick`
471470
// above, so the build sees this slot's `fast_head` as its parent.
472-
SlotInterval::HeadUpdate => {
471+
SlotInterval::EndOfSlot => {
473472
let next_slot = slot + 1;
474473
let next_proposer = self
475474
.get_our_proposer(next_slot)
@@ -485,7 +484,6 @@ impl BlockChainServer {
485484
metrics::update_safe_target_slot(self.store.safe_target_slot());
486485
// Update head slot metrics (head may change when attestations are promoted at intervals 0/3)
487486
metrics::update_head_slot(self.store.head_slot());
488-
metrics::update_fast_head_slot(self.store.head_slot());
489487
metrics::update_lagging_head_slot(self.store.lagging_head_slot());
490488

491489
// Advance XMSS keys for next slot so the signing paths don't have to
@@ -1479,9 +1477,9 @@ impl BlockChainServer {
14791477
}
14801478

14811479
/// Actor lifecycle hook: wait for any in-flight aggregation worker to exit
1482-
/// before the actor is fully stopped. We cancel the session's token and wait up
1483-
/// to PRIOR_WORKER_JOIN_TIMEOUT for the worker's current `aggregate_job` call
1484-
/// to finish (the proof itself cannot be interrupted).
1480+
/// before the actor is fully stopped. We cancel the session's token and
1481+
/// wait up to PRIOR_WORKER_JOIN_TIMEOUT for the worker's current
1482+
/// `aggregate_job` call to finish (the proof itself cannot be interrupted).
14851483
#[stopped]
14861484
async fn on_stopped(&mut self, _ctx: &Context<Self>) {
14871485
let Some(session) = self.current_aggregation.take() else {

crates/blockchain/src/metrics.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -700,18 +700,6 @@ pub fn update_safe_target_slot(slot: u64) {
700700

701701
// --- Heartbeat / two-tier fork choice ---
702702

703-
/// Set the fast-head gauge.
704-
///
705-
/// Tracks the same value as `lean_head_slot` (the fast head *is* the store head);
706-
/// exported under its own name so dashboards can pair it with
707-
/// `lean_lagging_head_slot` without relying on that equivalence holding forever.
708-
pub fn update_fast_head_slot(slot: u64) {
709-
static LEAN_FAST_HEAD_SLOT: std::sync::LazyLock<IntGauge> = std::sync::LazyLock::new(|| {
710-
register_int_gauge!("lean_fast_head_slot", "Fast head slot (GHOST-Eph)").unwrap()
711-
});
712-
LEAN_FAST_HEAD_SLOT.set(slot.try_into().unwrap());
713-
}
714-
715703
/// Set the lagging-head gauge: the RLMD-window tree base.
716704
///
717705
/// A frozen lagging head under a moving fast head is the RLMD-window failure

crates/blockchain/src/store.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use tracing::{info, trace, warn};
2222
use crate::{
2323
GOSSIP_DISPARITY_INTERVALS, INTERVALS_PER_SLOT, MAX_ATTESTATIONS_DATA,
2424
MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT, RLMD_LOOKBACK_LIMIT, SlotInterval,
25-
block_builder::{BlockTarget, PostBlockCheckpoints, ProposerConfig, build_block},
25+
block_builder::{PostBlockCheckpoints, ProposerConfig, build_block},
2626
metrics,
2727
};
2828

@@ -569,7 +569,7 @@ pub fn on_tick(store: &mut Store, timestamp_ms: u64, has_proposal: bool) {
569569
// Update safe target for validators
570570
update_safe_target(store);
571571
}
572-
SlotInterval::HeadUpdate => {
572+
SlotInterval::EndOfSlot => {
573573
// Recompute the tree base from the RLMD window, then the fast
574574
// head on top of it, before promoting this slot's payloads and
575575
// logging the resulting tree.
@@ -1103,7 +1103,6 @@ pub fn get_attestation_target_with_checkpoints(
11031103
.expect("parent block exists")
11041104
.unwrap();
11051105
}
1106-
11071106
// Guard: clamp target to justified (not in the spec).
11081107
//
11091108
// The spec's walk-back has no lower bound, so it can produce attestations
@@ -1249,14 +1248,11 @@ pub fn produce_block_with_signatures(
12491248
let (block, signatures, post_checkpoints) = {
12501249
let _timing = metrics::time_block_building_payload_aggregation();
12511250
build_block(
1252-
BlockTarget {
1253-
head_state: &head_state,
1254-
slot,
1255-
proposer_index: validator_index,
1256-
parent_root: head_root,
1257-
known_block_roots: &known_block_roots,
1258-
heartbeat_committee_size: store.heartbeat_committee_size(),
1259-
},
1251+
&head_state,
1252+
slot,
1253+
validator_index,
1254+
head_root,
1255+
&known_block_roots,
12601256
&aggregated_payloads,
12611257
config,
12621258
)?

0 commit comments

Comments
 (0)