Optimise state_root_at_slot for finalized slot

[michaelsproul]

Proposed Changes

This is an optimisation targeted at Fulu networks in non-finality.

While debugging on Holesky, we found that state_root_at_slot was being called from prepare_beacon_proposer a lot, for the finalized state:

lighthouse/beacon_node/http_api/src/lib.rs

Lines 3860 to 3861 in 2c9b670

	let (finalized_beacon_state, _, _) =
	StateId(CoreStateId::Finalized).state(&chain)?;

This was causing prepare_beacon_proposer calls to take upwards of 5 seconds, sometimes 10 seconds, because it would trigger multiple beacon state loads in order to iterate back to the finalized slot. Ideally, loading the finalized state should be quick because we keep it cached in the state cache (technically we keep the split state, but they usually coincide). Instead we are computing the finalized state root separately (slow), and then loading the state from the cache (fast).

Although it would be possible to make the API faster by removing the state_root_at_slot call, I believe it's simpler to change state_root_at_slot itself and remove the footgun. Devs rightly expect operations involving the finalized state to be fast.

Additional Info

Speaking of footgun removal:

• Abolish iterators that load states #7184

[michaelsproul]

This introduces a new read of the store.split RwLock in this function, prior to reading the canonical_head RwLock.

The risk here is:

Some other thread is holding the split lock exclusively (i.e. the DB finalization migration is running), and this blocks our read. Counter point: this will happen anyway (and with other downsides) if we miss the fast_lookup on the head state and need to load states from disk.

Maybe we could sequence the split read after the head read, as we mostly care about optimising the case where the finalized_slot is out of range of the head but known to the DB. Conversely, maybe using the split over the head is preferable, as there is more write-contention for the head (the head changes every slot whereas the split only changes at most once per epoch).

I suspect the effect sizes of both these phenomena are small. What we have now seems to be performing very well on Holesky, so I'm inclined to just leave it as-is pending further benchmarking.

[dapplion]

Why not use a try_read here to prevent the deadlock risk?

[michaelsproul]

There's no deadlock risk. I figure try_read isn't worth it because the alternative is doing state reconstruction. We may as well wait for the split lock, as it will be less work in total