Skip to content

Commit 781398d

Browse files
authored
refactor(blockchain): group on_tick conditionals by interval (lambdaclass#450)
Split out of lambdaclass#445 (proposer pre-build). **Stacked on lambdaclass#449** (`gate_proposer` removal) — review/merge that first; this PR's base retargets to `main` once lambdaclass#449 lands. `on_tick` ran its per-interval blocks out of order (interval-4 snapshot, then tick, then 2, 0, 1). This regroups them into ascending interval order behind `==== interval N ====` markers so the slot timeline reads top-to-bottom and lines up with the duty schedule. **Pure reorder, behavior unchanged.** ### One block intentionally does *not* move The interval-4 `new_payloads` snapshot stays **ahead** of `store::on_tick`. At interval 4 the tick runs `accept_new_attestations → promote_new_aggregated_payloads()`, which drains `new_payloads`; snapshotting after the tick would capture an already-drained set and silently break the post-block coverage report's "timely" cohort. A comment now pins that ordering constraint. > Note: commit `7474c15` on lambdaclass#445 currently moves this snapshot *into* the post-tick interval-4 group, which hits exactly that regression (observability-only, so devnet doesn't surface it). lambdaclass#445 should adopt this PR's placement when it rebases.
1 parent 1439b2d commit 781398d

1 file changed

Lines changed: 37 additions & 31 deletions

File tree

crates/blockchain/src/lib.rs

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -230,20 +230,7 @@ impl BlockChainServer {
230230
let is_aggregator = self.aggregator.is_enabled();
231231
metrics::set_is_aggregator(is_aggregator);
232232

233-
// At interval 0, check if we will propose (but don't build the block yet).
234-
// Tick forkchoice first to accept attestations, then build the block
235-
// using the freshly-accepted attestations.
236-
let scheduled_proposer = (interval == 0 && slot > 0)
237-
.then(|| self.get_our_proposer(slot))
238-
.flatten();
239-
let proposer_validator_id =
240-
scheduled_proposer.filter(|_| self.sync_status.duties_allowed());
241-
242-
if let Some(validator_id) = scheduled_proposer
243-
&& proposer_validator_id.is_none()
244-
{
245-
info!(%slot, %validator_id, "Skipping block proposal while syncing");
246-
}
233+
// ==== interval 4 (pre-tick) ====
247234

248235
// Snapshot the pre-merge `new_payloads` set at the end-of-slot promote
249236
// (interval 4), so the post-block report for this round sees its
@@ -262,29 +249,26 @@ impl BlockChainServer {
262249
self.pre_merge_coverage = Some(snapshot);
263250
}
264251

252+
let scheduled_proposer = (interval == 0 && slot > 0)
253+
.then(|| self.get_our_proposer(slot))
254+
.flatten();
255+
let is_proposer = scheduled_proposer.is_some();
256+
265257
// Tick the store first - this accepts attestations at interval 0 if we have a proposal
266-
store::on_tick(
267-
&mut self.store,
268-
timestamp_ms,
269-
proposer_validator_id.is_some(),
270-
);
258+
store::on_tick(&mut self.store, timestamp_ms, is_proposer);
271259

272-
if interval == 2 {
273-
if is_aggregator {
274-
coverage::emit_agg_start_new_coverage(
275-
&self.store,
276-
self.attestation_committee_count,
277-
);
278-
self.start_aggregation_session(slot, ctx).await;
260+
// ==== interval 0 ====
261+
262+
// Now build and publish the block (after attestations have been accepted)
263+
if let Some(validator_id) = scheduled_proposer {
264+
if self.sync_status.duties_allowed() {
265+
self.propose_block(slot, validator_id);
279266
} else {
280-
metrics::inc_aggregator_skipped_not_aggregator();
267+
info!(%slot, %validator_id, "Skipping block proposal while syncing");
281268
}
282269
}
283270

284-
// Now build and publish the block (after attestations have been accepted)
285-
if let Some(validator_id) = proposer_validator_id {
286-
self.propose_block(slot, validator_id);
287-
}
271+
// ==== interval 1 ====
288272

289273
// Produce attestations at interval 1 (all validators including proposer).
290274
// Reuse the same snapshot so self-delivery decisions match the rest
@@ -309,6 +293,28 @@ impl BlockChainServer {
309293
}
310294
}
311295

296+
// ==== interval 2 ====
297+
298+
if interval == 2 {
299+
if is_aggregator {
300+
coverage::emit_agg_start_new_coverage(
301+
&self.store,
302+
self.attestation_committee_count,
303+
);
304+
self.start_aggregation_session(slot, ctx).await;
305+
} else {
306+
metrics::inc_aggregator_skipped_not_aggregator();
307+
}
308+
}
309+
310+
// ==== interval 3 ====
311+
312+
// Interval 3 (safe-target update) is handled inside `store::on_tick`.
313+
314+
// ==== interval 4 ====
315+
316+
// Handled by the pre-tick snapshot above.
317+
312318
// Update safe target slot metric (updated by store.on_tick at interval 3)
313319
metrics::update_safe_target_slot(self.store.safe_target_slot());
314320
// Update head slot metric (head may change when attestations are promoted at intervals 0/4)

0 commit comments

Comments
 (0)