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.

Sign up for GitHub

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

Jump to bottom

feat: [zkos] witness RPC and larger config #548

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

feat: [zkos] witness RPC and larger config #548

wants to merge 9 commits into from

Conversation

mm-zk
Copy link
Collaborator

@mm-zk mm-zk commented Jan 17, 2025

What 💻

  • Moved zkos flags into a single config - and passing this around
  • added RPC that can return a zkos witness for a given batch

Why ✋

  • Witness allows us to send data to the proving system

Evidence 📷

image

mm-zk and others added 7 commits January 15, 2025 11:54
@mm-zk
re-added zkos
39ea23f
@mm-zk
added chain id, and proper gas refund (#540)
b0370e4
@mm-zk
feat: [zkos] compute and return the witness (#545)
92dad63 
* ugly hacks to return the witness

* compute witness correctly

* passing zkconfig along

* moved zkos into zkos_config

* moved zkos namespace to proper place

* added docs

* put dependency back on main branch
@mm-zk
changed from vec<u8> to hex string (#546)
b38e6dc
@mm-zk @itegulov
@nbaztec @dutterbutter
chore: merge main (#547)
daa0b80 
* refactor: reduce usage of `ForkStorage` outside `inner` module (#539)

* reduce usage of `ForkStorage` outside `inner` module

* adapt for zkos

* fix destination path (#541)

* fix: anvil-zksync able to start even if the port is busy (#542)

* fix: closes #512 - start instance even if port is busy

* chore: apply make lint:fix

* chore: refactor test to make use of anvil-zksync instance

* chore: remove no longer needed deps

* chore: remove no longer needed deps

* chore: bump actions/upload-artifact: v3 to v4 due to CI fail

* chore: bump actions/upload-artifact / download: v3 to v4 due to CI fail

* compilation fixes

---------

Co-authored-by: Daniyar Itegulov <ditegulov@gmail.com>
Co-authored-by: Nisheeth Barthwal <nbaztec@gmail.com>
Co-authored-by: Dustin Brickwood <dustinbrickwood204@gmail.com>
@mm-zk
Merge remote-tracking branch 'origin/main' into zkos-dev
717fbeb
@mm-zk
reverting stuff
1facec4
@mm-zk mm-zk requested a review from itegulov January 17, 2025 07:44
@mm-zk mm-zk requested a review from a team as a code owner January 17, 2025 07:44
itegulov
itegulov approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@itegulov itegulov left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of nits.

Generally we are working on expanding the anvil_ namespace with new methods for managing batches deterministically (i.e. commitBatch, proveBatch, finalizeBatch). getWitness might fit there too.

crates/api_decl/src/namespaces/zkos.rs

/// API bindings for the `zkos` experimental namespace.
#[rpc(server, namespace = "zkos")]
pub trait ZKOSNamespace {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: acronyms are usually treated as one word in camel case, so ZkOsNamespace instead of ZKOSNamespace

crates/config/src/types/zkos.rs

/// Genesis
#[derive(Deserialize, Clone, Debug, Parser, Default)]
pub struct ZKOSConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: same thing here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itegulov itegulov itegulov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mm-zk @itegulov @dutterbutter