Skip to content

Fix lookups of the block at oldest_block_slot #7693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 2, 2025
Merged

Fix lookups of the block at oldest_block_slot #7693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions beacon_node/beacon_chain/src/beacon_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,14 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
return Ok(root_opt);
}

// Do not try to access the previous slot if it's older than the oldest block root
// stored in the database. Instead, load just the block root at `oldest_block_slot`,
// under the assumption that the `oldest_block_slot` *is not* a skipped slot (should be
// true because it is set by the oldest *block*).
if request_slot == self.store.get_anchor_info().oldest_block_slot {
return self.block_root_at_slot_skips_prev(request_slot);
}

if let Some(((prev_root, _), (curr_root, curr_slot))) = process_results(
self.forwards_iter_block_roots_until(prev_slot, request_slot)?,
|iter| iter.tuple_windows().next(),
Expand Down
91 changes: 65 additions & 26 deletions beacon_node/beacon_chain/tests/store_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2238,7 +2238,15 @@ async fn weak_subjectivity_sync_easy() {
let num_initial_slots = E::slots_per_epoch() * 11;
let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9);
let slots = (1..num_initial_slots).map(Slot::new).collect();
weak_subjectivity_sync_test(slots, checkpoint_slot).await
weak_subjectivity_sync_test(slots, checkpoint_slot, None).await
}

#[tokio::test]
async fn weak_subjectivity_sync_single_block_batches() {
let num_initial_slots = E::slots_per_epoch() * 11;
let checkpoint_slot = Slot::new(E::slots_per_epoch() * 9);
let slots = (1..num_initial_slots).map(Slot::new).collect();
weak_subjectivity_sync_test(slots, checkpoint_slot, Some(1)).await
}

#[tokio::test]
Expand All @@ -2252,7 +2260,7 @@ async fn weak_subjectivity_sync_unaligned_advanced_checkpoint() {
slot <= checkpoint_slot - 3 || slot > checkpoint_slot
})
.collect();
weak_subjectivity_sync_test(slots, checkpoint_slot).await
weak_subjectivity_sync_test(slots, checkpoint_slot, None).await
}

#[tokio::test]
Expand All @@ -2266,7 +2274,7 @@ async fn weak_subjectivity_sync_unaligned_unadvanced_checkpoint() {
slot <= checkpoint_slot || slot > checkpoint_slot + 3
})
.collect();
weak_subjectivity_sync_test(slots, checkpoint_slot).await
weak_subjectivity_sync_test(slots, checkpoint_slot, None).await
}

// Regression test for https://github.com/sigp/lighthouse/issues/4817
Expand All @@ -2278,7 +2286,7 @@ async fn weak_subjectivity_sync_skips_at_genesis() {
let end_slot = E::slots_per_epoch() * 4;
let slots = (start_slot..end_slot).map(Slot::new).collect();
let checkpoint_slot = Slot::new(E::slots_per_epoch() * 2);
weak_subjectivity_sync_test(slots, checkpoint_slot).await
weak_subjectivity_sync_test(slots, checkpoint_slot, None).await
}

// Checkpoint sync from the genesis state.
Expand All @@ -2291,10 +2299,14 @@ async fn weak_subjectivity_sync_from_genesis() {
let end_slot = E::slots_per_epoch() * 2;
let slots = (start_slot..end_slot).map(Slot::new).collect();
let checkpoint_slot = Slot::new(0);
weak_subjectivity_sync_test(slots, checkpoint_slot).await
weak_subjectivity_sync_test(slots, checkpoint_slot, None).await
}

async fn weak_subjectivity_sync_test(slots: Vec<Slot>, checkpoint_slot: Slot) {
async fn weak_subjectivity_sync_test(
slots: Vec<Slot>,
checkpoint_slot: Slot,
backfill_batch_size: Option<usize>,
) {
// Build an initial chain on one harness, representing a synced node with full history.
let num_final_blocks = E::slots_per_epoch() * 2;

Expand Down Expand Up @@ -2557,30 +2569,57 @@ async fn weak_subjectivity_sync_test(slots: Vec<Slot>, checkpoint_slot: Slot) {
.unwrap_err(),
HistoricalBlockError::InvalidSignature
));
assert_eq!(beacon_chain.store.get_oldest_block_slot(), wss_block.slot());

let available_blocks_slots = available_blocks
.iter()
.map(|block| (block.block().slot(), block.block().canonical_root()))
.collect::<Vec<_>>();
info!(
?available_blocks_slots,
"wss_block_slot" = wss_block.slot().as_usize(),
"Importing historical block batch"
);
let batch_size = backfill_batch_size.unwrap_or(available_blocks.len());

for batch in available_blocks.rchunks(batch_size) {
let available_blocks_slots = batch
.iter()
.map(|block| (block.block().slot(), block.block().canonical_root()))
.collect::<Vec<_>>();
info!(
?available_blocks_slots,
"wss_block_slot" = wss_block.slot().as_usize(),
"Importing historical block batch"
);

// Importing the batch with valid signatures should succeed.
let available_blocks_dup = available_blocks.iter().map(clone_block).collect::<Vec<_>>();
assert_eq!(beacon_chain.store.get_oldest_block_slot(), wss_block.slot());
beacon_chain
.import_historical_block_batch(available_blocks_dup)
.unwrap();
assert_eq!(beacon_chain.store.get_oldest_block_slot(), 0);
// Importing the batch with valid signatures should succeed.
let available_blocks_batch1 = batch.iter().map(clone_block).collect::<Vec<_>>();
beacon_chain
.import_historical_block_batch(available_blocks_batch1)
.unwrap();

// Resupplying the blocks should not fail, they can be safely ignored.
beacon_chain
.import_historical_block_batch(available_blocks)
.unwrap();
// We should be able to load the block root at the `oldest_block_slot`.
//
// This is a regression test for: https://github.com/sigp/lighthouse/issues/7690
let oldest_block_imported = &batch[0];
let (oldest_block_slot, oldest_block_root) =
if oldest_block_imported.block().parent_root() == beacon_chain.genesis_block_root {
(Slot::new(0), beacon_chain.genesis_block_root)
} else {
available_blocks_slots[0]
};
assert_eq!(
beacon_chain.store.get_oldest_block_slot(),
oldest_block_slot
);
assert_eq!(
beacon_chain
.block_root_at_slot(oldest_block_slot, WhenSlotSkipped::None)
.unwrap()
.unwrap(),
oldest_block_root
);

// Resupplying the blocks should not fail, they can be safely ignored.
let available_blocks_batch2 = batch.iter().map(clone_block).collect::<Vec<_>>();
beacon_chain
.import_historical_block_batch(available_blocks_batch2)
.unwrap();
}
}
assert_eq!(beacon_chain.store.get_oldest_block_slot(), 0);

// Sanity check for non-aligned WSS starts, to make sure the WSS block is persisted properly
if wss_block_slot != wss_state_slot {
Expand Down