fix(l1): validate incoming payloads even when the node is syncing. #2426
Conversation
Lines of code reportTotal lines added: Detailed view |
| match apply_fork_choice( | ||
| &context.storage, | ||
| fork_choice_state.head_block_hash, | ||
| fork_choice_state.safe_block_hash, | ||
| fork_choice_state.finalized_block_hash, | ||
| ) | ||
| .await | ||
| { |
There was a problem hiding this comment.
We shouldn't apply the fork choice if we are currently syncing, this can lead to false positives if snap sync is enabled
There was a problem hiding this comment.
Well, we need to be able to do it, since it's part of the spec.
There was a problem hiding this comment.
During an active snap sync we cannot rely on the current state (any block we have in the store may not have its state available), we cannot fetch a block from the store and set it as head if we have no state for it. Essentially, during a snap sync we "have no state"
There was a problem hiding this comment.
Ok, added a check specifically for snap sync.
…ntly syncing." This reverts commit f0474ba.
…to fix-invalid-payload-tests
…to fix-invalid-payload-tests
|
|
||
| match fork_choice_res { | ||
| if context.syncer.sync_mode() == SyncMode::Snap { | ||
| warn!("Snap sync in progress, setting new head optimistically"); |
There was a problem hiding this comment.
This sounds misleading, setting new head optimistically sound like we are setting it as canonical head of the chain. Also warn is a bit too harsh for something that is regular behaviour. It is also too spammy as fcu happen quite often
There was a problem hiding this comment.
Agreed with warn, but aren't we setting the new head as the canonical head of the chain in this case?
Motivation
We should be able to do payload validations even when the node is in a sync process (except if it's snap sync).
Description