Skip to content

Commit 20941bc

Browse files
committed
Fix off-by-one in block packing lcli (#2878)
## Issue Addressed The current `lcli` block packing code has an off-by-one where it would include an extra slot (the oldest slot) of attestations as "available" (this means there would be 33 slots of "available" attestations instead of 32). There is typically only single-digit attestations remaining from that slot and as such does not cause a significant change to the results although every efficiency will have been very slightly under-reported. ## Proposed Changes Prune the `available_attestation_set` before writing out the data instead of after. ## Additional Info This `lcli` code will soon be deprecated by a Lighthouse API (#2879) which will run significantly faster and will be used to hook into our upcoming monitoring platform #2873.
1 parent 6684778 commit 20941bc

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

lcli/src/etl/block_efficiency.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ pub async fn run<T: EthSpec>(matches: &ArgMatches<'_>) -> Result<(), String> {
274274
// Add them to the set.
275275
included_attestations_set.extend(attestations_in_block.clone());
276276

277+
// Remove expired available attestations.
278+
available_attestations_set.retain(|x| x.slot >= (slot.as_u64().saturating_sub(32)));
279+
277280
// Don't write data from the initialization epoch.
278281
if epoch != initialization_epoch {
279282
let included = attestations_in_block.len();
@@ -309,9 +312,6 @@ pub async fn run<T: EthSpec>(matches: &ArgMatches<'_>) -> Result<(), String> {
309312
}
310313
}
311314
}
312-
313-
// Remove expired available attestations.
314-
available_attestations_set.retain(|x| x.slot >= (slot.as_u64().saturating_sub(32)));
315315
}
316316

317317
let mut offline = "None".to_string();

0 commit comments

Comments
 (0)