Keep execution payload during historical backfill when prune-payloads set to false

[chong-he]

Issue Addressed

• Store execution payloads during backfill if --prune-payloads false #6510

Proposed Changes

• Keep execution payload during historical backfill when --prune-payloads false is set
• Add a field in the historical backfill debug log to indicate if execution payload is kept
• Add a test to check historical blocks has execution payload when `--prune-payloads false is set
• Very minor typo correction that I notice when working on this

Additional Info

• I have done manual testing that when the flag is set to false, we see that the number of keys returned:
  lighthouse db inspect --column exp --output sizes --datadir path is around the range of: head_slot - oldest_block_slot, so I think it works

Thank you @michaelsproul for the hints in testing

[michaelsproul]

Looks good, just one suggested change to the test

[michaelsproul]

I think we should remove this if, and make it so that these tests always this check

We can set prune_payloads: false in the default config here:

lighthouse/beacon_node/beacon_chain/tests/store_tests.rs

Lines 75 to 85 in beefb92

	fn get_harness(
	store: Arc<HotColdDB<E, LevelDB<E>, LevelDB<E>>>,
	validator_count: usize,
	) -> TestHarness {
	// Most tests expect to retain historic states, so we use this as the default.
	let chain_config = ChainConfig {
	reconstruct_historic_states: true,
	..ChainConfig::default()
	};
	get_harness_generic(store, validator_count, chain_config)
	}

[chong-he]

Fixed in 42fb4d5, thank you for your help!

Setting prune_payloads default to false in the test makes the tests that calling check_chain_dump to fail:
https://github.com/sigp/lighthouse/actions/runs/13171847366/job/36763510649

From my understanding, this is because when prune_payloads is set to false, the execution payloads are kept for all slots in the test, not only after the split slot, so the second argument in assert_eq! is false.

lighthouse/beacon_node/beacon_chain/tests/store_tests.rs

Lines 3535 to 3546 in 029b4f2

	if harness.chain.spec.bellatrix_fork_epoch.is_some() {
	assert_eq!(
	harness
	.chain
	.store
	.execution_payload_exists(&checkpoint.beacon_block_root)
	.unwrap(),
	checkpoint.beacon_block.slot() >= split_slot,
	"incorrect payload storage for block at slot {}: {:?}",
	checkpoint.beacon_block.slot(),
	checkpoint.beacon_block_root,
	);

I make the assert_eq! to become assert! in 5db1b78 to have the tests pass, however it makes the tests "less robust", so not sure if there are better ways to do this?

[mergify]

This pull request has merge conflicts. Could you please resolve them @chong-he? 🙏

[mergify]

This pull request has been removed from the queue for the following reason: pull request manually updated.

The pull request #6766 has been manually updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue