See parts 3 & 4 of this design doc for more info.
DerivationActor can and should expose a single inbound request channel and a clear enum of requests it accepts (see this PR for an example).
Actors that send DerivationActor information, directly or indirectly, should be injected with a DerivationActorClient trait, over which they are made to be generic.
For instance EngineActor sends DerivationActor updates over derivation_signal_rx, el_sync_complete_rx, and engine_l2_safe_head. Instead,
- These can each be a request type that
DerivationActor accepts
EngineActor can directly call derivation actor via an EngineDerivationClient trait
- All uses of
watch::Receiver.changed() and watch::Receiver.has_changed() can become clearer and race-free
For 3, we use the changed state of engine_l2_safe_head for external storage of state that can and should live in DerivationActor. The goal appears to be to prevent more derivation loops when the L1 block has changed but the engine hasn't acknowledged the last safe head update. If we store the required state, we can make this check more explicit and prevent race conditions (i.e. the call to engine_l2_safe_head.borrow_and_update() that might be marking an unseen safe head as seen if it has changed since last read).
We can avoid these race conditions entirely and prevent missing any updates by making each case of an update a request on the DerivationActor's inbound queue. This may require us to ignore stale requests (i.e. a stale safe head update immediately after a reset), but that should be easy if we're storing the pending safe head and checking the acknowledged one against it.
See parts 3 & 4 of this design doc for more info.
DerivationActorcan and should expose a single inbound request channel and a clear enum of requests it accepts (see this PR for an example).Actors that send
DerivationActorinformation, directly or indirectly, should be injected with aDerivationActorClienttrait, over which they are made to be generic.For instance
EngineActorsendsDerivationActorupdates overderivation_signal_rx,el_sync_complete_rx, andengine_l2_safe_head. Instead,DerivationActoracceptsEngineActorcan directly call derivation actor via anEngineDerivationClienttraitwatch::Receiver.changed()andwatch::Receiver.has_changed()can become clearer and race-freeFor 3, we use the
changedstate ofengine_l2_safe_headfor external storage of state that can and should live inDerivationActor. The goal appears to be to prevent more derivation loops when the L1 block has changed but the engine hasn't acknowledged the last safe head update. If we store the required state, we can make this check more explicit and prevent race conditions (i.e. the call toengine_l2_safe_head.borrow_and_update()that might be marking an unseen safe head as seen if it has changed since last read).We can avoid these race conditions entirely and prevent missing any updates by making each case of an update a request on the
DerivationActor's inbound queue. This may require us to ignore stale requests (i.e. a stale safe head update immediately after a reset), but that should be easy if we're storing the pending safe head and checking the acknowledged one against it.