Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement basic L1 support with commitment #575

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

itegulov
Copy link
Contributor

@itegulov itegulov commented Feb 3, 2025

What 💻

First part of #536

This PR implement L1 sidecar: an independent service that can initialize, run and query an L1 network. I tried to design it with some extendibility in mind but for now it can only spawn a built-in anvil with precomputed state.

L1 sidecar can:

  • generate commitments (with dummy values but in theory the only blocker here is zksync_kzg crate using unstable Rust)
  • encode data to ABI as expected by L1 contracts (powered by alloy-sol macro)
  • send commitments to L1

To try this out run cargo run --release -- --l1-anvil-port 8012 and then commit batch like this:

curl http://localhost:8011 \
  -X POST \
  -H "Content-Type: application/json" \
  --data '{"method":"anvil_commitBatch","params":[1],"id":1,"jsonrpc":"2.0"}

Additionally I updated our branch of era-contracts: disabled system log validation and DA input validation (only for commitment, will extend it further as need be).

Needs popzxc/alloy-zksync#47 for testing. Tests are pretty bare bones for now but I am hoping to do more on that front further down the line.

Why ✋

First step towards having L1-L2 communication

@itegulov itegulov requested a review from a team as a code owner February 3, 2025 06:54
@itegulov itegulov force-pushed the daniyar/basic-l1-support branch from 2bea19e to dec25a4 Compare February 3, 2025 07:45
@@ -0,0 +1,44 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to include portal config? I'd rather not have it included.

@@ -0,0 +1,15 @@
tokens:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to include this config? I am not opposed to have pre-deployed mock contracts available but there equivalent would need to be available on L2.

server_url: postgres://postgres:notsecurepassword@localhost:5433/zksync_local_test
prover_url: postgres://postgres:notsecurepassword@localhost:5433/prover_local_test
db:
state_keeper_db_path: /home/itegulov/Work/matterlabs/anvil-zksync/l1-setup/./chains/anvil/db/main/state_keeper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the file paths should be changed.

Comment on lines +192 to +200
let mut node_service_tasks: Vec<Pin<Box<dyn Future<Output = anyhow::Result<()>>>>> = Vec::new();
let l1_sidecar = match config.l1_anvil_port {
Some(l1_anvil_port) => {
let (l1_sidecar, l1_sidecar_runner) = L1Sidecar::builtin(l1_anvil_port).await?;
node_service_tasks.push(Box::pin(l1_sidecar_runner.run()));
l1_sidecar
}
None => L1Sidecar::none(),
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want L1-L2 support enabled by default rather then be toggled via l1-port option? Perhaps this is intentional right now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about something --with-l1 (that will be false by default until the feature is polished), and then --l1-port etc to affect how anvil is launched.

@dutterbutter
Copy link
Collaborator

@itegulov awesome work good sir 💪 !

Understandably this will happen iteratively but devs wont be required to manually send a anvil_commitBatch to start using anvil-zksync with L1/L2 support, correct? That is we would make that request automatically during start up by default in the future?

@@ -133,6 +133,8 @@ pub struct TestNodeConfig {
pub preserve_historical_states: bool,
/// State to load
pub load_state: Option<PathBuf>,
/// Port the spawned L1 anvil node will listen on
pub l1_anvil_port: Option<u16>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a print statement to print that indicates what port L1 is running at.

Comment on lines +192 to +200
let mut node_service_tasks: Vec<Pin<Box<dyn Future<Output = anyhow::Result<()>>>>> = Vec::new();
let l1_sidecar = match config.l1_anvil_port {
Some(l1_anvil_port) => {
let (l1_sidecar, l1_sidecar_runner) = L1Sidecar::builtin(l1_anvil_port).await?;
node_service_tasks.push(Box::pin(l1_sidecar_runner.run()));
l1_sidecar
}
None => L1Sidecar::none(),
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about something --with-l1 (that will be false by default until the feature is polished), and then --l1-port etc to affect how anvil is launched.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing 80mb in the repository seems like a bad idea :(
I dunno, the longer I think about it, the more I'm leaning towards just biting the bullet and adding anvil as a dependency.

@@ -0,0 +1,388 @@
postgres:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should find a way to not include the configs into the repository.
Instead, we probably can use zkstack dev config-writer to apply overrides only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in gitignore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants