From 20941bc0f7869533c714ca796679cfd33007e4d9 Mon Sep 17 00:00:00 2001 From: Mac L Date: Fri, 7 Jan 2022 05:32:29 +0000 Subject: [PATCH] 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. --- lcli/src/etl/block_efficiency.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lcli/src/etl/block_efficiency.rs b/lcli/src/etl/block_efficiency.rs index 45452735d..87175ace8 100644 --- a/lcli/src/etl/block_efficiency.rs +++ b/lcli/src/etl/block_efficiency.rs @@ -274,6 +274,9 @@ pub async fn run(matches: &ArgMatches<'_>) -> Result<(), String> { // Add them to the set. included_attestations_set.extend(attestations_in_block.clone()); + // Remove expired available attestations. + available_attestations_set.retain(|x| x.slot >= (slot.as_u64().saturating_sub(32))); + // Don't write data from the initialization epoch. if epoch != initialization_epoch { let included = attestations_in_block.len(); @@ -309,9 +312,6 @@ pub async fn run(matches: &ArgMatches<'_>) -> Result<(), String> { } } } - - // Remove expired available attestations. - available_attestations_set.retain(|x| x.slot >= (slot.as_u64().saturating_sub(32))); } let mut offline = "None".to_string();