Send fcU before fully importing block

[michaelsproul]

The Problem

Presently Lighthouse doesn't send fcU until after fully importing the block, here:

lighthouse/beacon_node/network/src/network_beacon_processor/gossip_methods.rs

Lines 1499 to 1505 in cfb1f73

	debug!(
	?block_root,
	%peer_id,
	"Gossipsub block processed"
	);
	self.chain.recompute_head_at_current_slot().await;

The call to the EL happens from within that function, here-ish:

lighthouse/beacon_node/beacon_chain/src/canonical_head.rs

Lines 1136 to 1142 in cfb1f73

	if let Err(e) = chain
	.update_execution_engine_forkchoice(
	current_slot,
	forkchoice_update_params,
	OverrideForkchoiceUpdate::Yes,
	)
	.await

The problem is, even with recent optimisations, these calls are happening after state diff computations and disk writes, which add 1s+ of latency.

Possible Solutions

Background thread for writes

We could add a dedicated background thread for finishing writes of BeaconStates to disk. This would allow import_block to complete sooner, and for the post-processing (like calling fork choice) to begin. This seems like it would be relatively simple to implement.

The added benefit is that we could also de-couple in-memory states from on-disk states, which would benefit the state advance timer as well. See:

• Optimise the state advance timer for tree-states #4412

Reorder fork choice update within import

Another option is to kick off a fork choice update at the point where we currently update the early attestation cache:

lighthouse/beacon_node/beacon_chain/src/beacon_chain.rs

Lines 3910 to 3913 in cfb1f73

	// If the block is recent enough and it was not optimistically imported, check to see if it
	// becomes the head block. If so, apply it to the early attester cache. This will allow
	// attestations to the block without waiting for the block and state to be inserted to the
	// database.

That code is kind of messy and this might be a good opportunity to refactor it and clean it up.

[michaelsproul]

I don't think Reorder fork choice update within import is particularly straight-forward. The problem is, if we try to call something like recompute_head_at_slot_internal where we previously updated the attester cache (i.e. in import_block before disk writes), then the logic that updates the canonical_head will error because the block & state won't have been written to disk (nor will they be available in the cache). I.e. we will almost always error here:

lighthouse/beacon_node/beacon_chain/src/canonical_head.rs

Lines 644 to 656 in cfb1f73

	let beacon_block = self
	.store
	.get_full_block(&new_view.head_block_root)?
	.ok_or(Error::MissingBeaconBlock(new_view.head_block_root))?;
	let (_, beacon_state) = self
	.store
	.get_advanced_hot_state(
	new_view.head_block_root,
	current_slot,
	beacon_block.state_root(),
	)?
	.ok_or(Error::MissingBeaconState(beacon_block.state_root()))?;

I don't see any easy way around this, other than introducing a disconnect between blocks & states cached in memory, and those stored on disk, which is more similar to idea 1. I guess an alternative would be to pass the block and state from import_block right through to recompute_head so they can be enshrined as the canonical_head?