feat(l1): full sync #1312
feat(l1): full sync #1312
Conversation
…ethereum_rust into trie_iter_plus_listen_loop
…test" This reverts commit 356eaf0.
| @@ -117,6 +115,11 @@ async fn main() { | |||
| .get_one::<String>("datadir") | |||
| .map_or(set_datadir(DEFAULT_DATADIR), |datadir| set_datadir(datadir)); | |||
|
|
|||
| let snap_sync = is_snap_sync(&matches); | |||
| if snap_sync { | |||
| info!("snap-sync not available, defaulting to full-sync"); | |||
There was a problem hiding this comment.
Full sync will only work for a network with a post-merge genesis, right?
There was a problem hiding this comment.
For the sync itself here is no check specifically preventing it from happening, we would get an error when trying to execute pre-merge blocks as we don't support them.
| /// Only performs full-sync but will also be in charge of snap-sync in the future | ||
| #[derive(Debug)] | ||
| pub struct SyncManager { | ||
| // true: syncmode = snap, false = syncmode = full |
There was a problem hiding this comment.
nit: wouldn't it be clearer to have an enum instead of a bool?
There was a problem hiding this comment.
I thought making an enum for only two choices was a bit overkill. Maybe we could change the name to snap_enabled so we can "disable" it after the first sync
There was a problem hiding this comment.
I actually find an enum fitting for this situation, having a bool can lead to wonder what false/true means (plus the enum has type checking which warns when you're not using some varint), but this is not something critical.
|
|
||
| /// Starts a sync cycle, updating the state with all blocks between the current head and the sync head | ||
| /// TODO: only uses full sync, should also process snap sync once implemented | ||
| pub async fn start_sync(&mut self, mut current_head: H256, sync_head: H256, store: Store) { |
There was a problem hiding this comment.
so, if I understand this right, current_head is older than sync_head?
There was a problem hiding this comment.
I think eventually we will have to go backwards for syncing, but let's discuss it. Anyway this is great for a first version.
| } | ||
| } | ||
|
|
||
| /// Creates a dummy SyncManager for tests where syncing is not needed |
There was a problem hiding this comment.
should we put this behind the test feature flag?
There was a problem hiding this comment.
I would like to, but test feature flag gating doesn't work well across crates
| }; | ||
| return serde_json::to_value(fork_choice_response) | ||
| .map_err(|error| RpcErr::Internal(error.to_string())); | ||
| } |
Co-authored-by: Francisco Krause Arnim <[email protected]>
Motivation
Support syncing via fullsync
Description
Leftover work: #1317 #1318
Status:
Succesfully passes
ethereum/synctest suite when removing V3-specifc fork choice & payload checks (see commit c6d6767)This PR shares the same basis as snap-sync #1209
Closes: None, but is a good basis for snap sync