Refactor rollup initialization #1714
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eyusufatik
requested changes
Jan 16, 2025
crates/common/src/da.rs
Outdated
Comment on lines
34
to
72
| 'block_sync: loop { | ||
| let last_finalized_l1_block_header = | ||
| match da_service.get_last_finalized_block_header().await { | ||
| Ok(header) => header, | ||
| Err(e) => { | ||
| error!("Could not fetch last finalized L1 block header: {}", e); | ||
| sleep(Duration::from_secs(2)).await; | ||
| continue; | ||
| } | ||
| }; | ||
|
|
||
| let new_l1_height = last_finalized_l1_block_header.height(); | ||
|
|
||
| for block_number in l1_height + 1..=new_l1_height { | ||
| let l1_block = | ||
| match get_da_block_at_height(&da_service, block_number, l1_block_cache.clone()) | ||
| .await | ||
| { | ||
| Ok(block) => block, | ||
| Err(e) => { | ||
| error!("Could not fetch last finalized L1 block: {}", e); | ||
| sleep(Duration::from_secs(2)).await; | ||
| continue 'block_sync; | ||
| } | ||
| }; | ||
|
|
||
| if block_number > l1_height { | ||
| l1_height = block_number; | ||
| l1_block_scan_histogram.record( | ||
| Instant::now() | ||
| .saturating_duration_since(start) | ||
| .as_secs_f64(), | ||
| ); | ||
| if let Err(e) = sender.send(l1_block).await { | ||
| error!("Could not notify about L1 block: {}", e); | ||
| continue 'block_sync; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I also want to refactor this part of the code
There was a problem hiding this comment.
What changes are you looking to have done?
- L1BlockHandler for all node types has been moved - RPC server is started for batch prover Remaining: - Update spawning RPC server for sequencer / light client
This consistently calls `build_services` which return all services that a specific node would need to spawn using the task manager.
jfldde
reviewed
Jan 20, 2025
jfldde
reviewed
Jan 20, 2025
Comment on lines
+108
to
+115
| if let Some(sequencer_config) = sequencer_config { | ||
| return Ok(NodeType::Sequencer(sequencer_config)); | ||
| } else if let Some(batch_prover_config) = batch_prover_config { | ||
| return Ok(NodeType::BatchProver(batch_prover_config)); | ||
| } else if let Some(light_client_prover_config) = light_client_prover_config { | ||
| return Ok(NodeType::LightClientProver(light_client_prover_config)); | ||
| } | ||
| Ok(NodeType::FullNode) |
There was a problem hiding this comment.
Wish we had it the other way round and passed a --kind arg or something. Would let us have a single config field (+ the rollup config) and not be implicit about which node kind is running.
We would match on the kind here and get the config accordingly.
jfldde
reviewed
Jan 20, 2025
| storage_manager, | ||
| prover_storage, | ||
| soft_confirmation_channel.0, | ||
| rpc_module, |
There was a problem hiding this comment.
Could remove it here since we return it as is
jfldde
reviewed
Jan 20, 2025
| use crate::RpcConfig; | ||
|
|
||
| /// Starts a RPC server with provided rpc methods. | ||
| pub async fn start_rpc_server( |
There was a problem hiding this comment.
IIRC the different nodes have subtle differences between them here
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem we have right now is that config along with a lot of other dependencies go through
main -> rollup -> client (sequencer, fullnode ..etc) -> da_block_handler
So introducing a component in DA block handler that requires a piece of config or a dependency as an example, would require updating the whole flow all the way to that point.
This PR changes this by running RPC & DA block handler as services along side the client (sequencer, fullnode ..etc) not from within. This makes initialization much easier.
TODO