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: ProverJobProcessor & circuit prover #3287

Merged
merged 8 commits into from
Nov 15, 2024
Merged

feat: ProverJobProcessor & circuit prover #3287

merged 8 commits into from
Nov 15, 2024

Conversation

EmilLuta
Copy link
Contributor

@EmilLuta EmilLuta commented Nov 15, 2024 •
edited
Loading

This is PR 2 out of 5.

Upcoming PRs:

  • refactor remaining code (circuit_prover main, keystore, etc.)
  • add tests & example for the framework
  • remove witness_vector_generator & prover_fri

How to review this PR?

What?

This PR touches 2 concepts:

  • Prover Job Processor - some sort of "framework" to make prover components more maintainable; it aims to make prover code more async & faster, provide more configurability, simplifies testing and makes writing new prover components easy
  • Circuit Prover - a complete rewrite to showcase Prover Job Processor

Why?

Check ProverJobProcessor README.md - objectives section.

Testing

Ran on local setup, L4 & T4.

@EmilLuta EmilLuta requested a review from a team as a code owner November 15, 2024 01:36
@EmilLuta
feat: ProverJobProcessor & circuit prover
5673d6e 
This is PR 2/5.

Upcoming PRs:
- refactor remaining code (circuit_prover main, keystore, etc.)
- add tests & example for the framework
- remove witness_vector_generator & prover_fri

How to review this PR?
- I'd recommend going through the README's (first prover_job_processor, then circuit_prover_service)
- Compare existing circuit prover implementation with this one

What?

This PR touches 2 concepts:
- Prover Job Processor - some sort of "framework" to make prover components more maintainable; it aims to make prover code more async & faster, provide more configurability, simplifies testing and makes writing new prover components easy
- Circuit Prover - a complete rewrite to showcase Prover Job Processor

Why?

Check ProverJobProcessor README.md - objectives sectiopn.
@EmilLuta EmilLuta force-pushed the evl-circuit-prover-rework branch from d9775f6 to 5673d6e Compare November 15, 2024 03:08
@EmilLuta EmilLuta changed the title feat: testing feat: ProverJobProcessor & circuit prover Nov 15, 2024
popzxc
popzxc reviewed Nov 15, 2024
View reviewed changes
Copy link
Member

@popzxc popzxc left a comment

Choose a reason for hiding this comment

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

Mostly looks really good!

prover/crates/bin/circuit_prover/src/main.rs Show resolved Hide resolved
core/lib/basic_types/src/prover_dal.rs Outdated Show resolved Hide resolved
prover/crates/lib/circuit_prover_service/Cargo.toml Outdated Show resolved Hide resolved
prover/crates/lib/circuit_prover_service/README.md Show resolved Hide resolved
...er/crates/lib/circuit_prover_service/src/gpu_circuit_prover/gpu_circuit_prover_job_picker.rs Outdated Show resolved Hide resolved
prover/crates/lib/circuit_prover_service/src/job_runner.rs Outdated Show resolved Hide resolved
...lib/circuit_prover_service/src/witness_vector_generator/witness_vector_generator_executor.rs Outdated Show resolved Hide resolved
...lib/circuit_prover_service/src/witness_vector_generator/witness_vector_generator_executor.rs Outdated Show resolved Hide resolved
...ib/circuit_prover_service/src/witness_vector_generator/witness_vector_generator_job_saver.rs Outdated Show resolved Hide resolved
prover/crates/lib/prover_job_processor/README.md Show resolved Hide resolved
popzxc
popzxc previously approved these changes Nov 15, 2024
View reviewed changes
Copy link
Member

@popzxc popzxc left a comment

Choose a reason for hiding this comment

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

Generally LGTM, but can't say that I was very thorough. Highly recommend getting one more approval.

prover/crates/lib/circuit_prover_service/src/job_runner.rs Show resolved Hide resolved
yorik
yorik approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@yorik yorik left a comment

Choose a reason for hiding this comment

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

Approved for CI.

Artemka374
Artemka374 approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@Artemka374 Artemka374 left a comment

Choose a reason for hiding this comment

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

Although I reviewed it pretty thourough, I really like how it looks. I mean it's really neat.

prover/crates/lib/circuit_prover_service/src/gpu_circuit_prover/gpu_circuit_prover_executor.rs Show resolved Hide resolved
prover/crates/lib/circuit_prover_service/src/metrics.rs Show resolved Hide resolved
prover/crates/lib/circuit_prover_service/src/types/circuit.rs Show resolved Hide resolved
RomanBrodetski
RomanBrodetski approved these changes Nov 15, 2024
View reviewed changes
Copy link
Collaborator

@RomanBrodetski RomanBrodetski left a comment

Choose a reason for hiding this comment

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

Overall looks good and feel free to merge and start testing on stage.

I wrote some comments, none of them are blocking. With regards to the types, please give it a thought but I may be completely off there

prover/crates/lib/prover_job_processor/README.md Show resolved Hide resolved
prover/crates/lib/prover_job_processor/README.md Show resolved Hide resolved
prover/crates/lib/prover_job_processor/README.md Show resolved Hide resolved
prover/crates/lib/prover_job_processor/README.md Show resolved Hide resolved
core/lib/basic_types/src/prover_dal.rs Show resolved Hide resolved
prover/crates/lib/prover_job_processor/src/lib.rs Show resolved Hide resolved
prover/crates/lib/prover_job_processor/src/job_runner.rs Show resolved Hide resolved
prover/crates/lib/circuit_prover_service/src/gpu_circuit_prover/gpu_circuit_prover_job_saver.rs Show resolved Hide resolved
prover/crates/lib/prover_job_processor/src/job_saver.rs Show resolved Hide resolved
prover/crates/bin/circuit_prover/src/main.rs Show resolved Hide resolved
@EmilLuta
Copy link
Contributor Author

Will resolve comments and focus on merging, but will follow-up on these on the next PR. Thanks for taking the time folks!

@EmilLuta EmilLuta added this pull request to the merge queue Nov 15, 2024
Merged via the queue into main with commit 98823f9 Nov 15, 2024
44 checks passed
@EmilLuta EmilLuta deleted the evl-circuit-prover-rework branch November 15, 2024 14:48
This was referenced Nov 15, 2024
Open
Merged
Merged
github-merge-queue bot pushed a commit that referenced this pull request Nov 18, 2024
@zksync-era-bot
chore(main): release prover 17.1.0 (#3213)
ca3c0c6 
🤖 I have created a release *beep* *boop*
---


##
[17.1.0](prover-v17.0.0...prover-v17.1.0)
(2024-11-18)


### Features

* Add min_replicas for SimpleScaler, apply_min_to_namespace config
([#3282](#3282))
([bc00c4a](bc00c4a))
* allow vm2 tracers to stop execution
([#3183](#3183))
([9dae839](9dae839))
* **contract-verifier:** Support Solidity contracts with EVM bytecode in
contract verifier
([#3225](#3225))
([8a3a82c](8a3a82c))
* **prover:** Add cluster name autodetection
([#3227](#3227))
([bd32aec](bd32aec))
* **prover:** Add queue metric to report autoscaler view of the queue.
([#3206](#3206))
([2721396](2721396))
* ProverJobProcessor & circuit prover
([#3287](#3287))
([98823f9](98823f9))
* **prover:** Move prover_autoscaler config into crate
([#3222](#3222))
([1b33b5e](1b33b5e))


### Bug Fixes

* **prover:** Remove unneeded dependencies, add default for
graceful_shutdown_timeout
([#3242](#3242))
([1bfff0e](1bfff0e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
github-merge-queue bot pushed a commit that referenced this pull request Nov 19, 2024
@zksync-era-bot
chore(main): release core 25.2.0 (#3228)
b26f939 
🤖 I have created a release *beep* *boop*
---


##
[25.2.0](core-v25.1.0...core-v25.2.0)
(2024-11-19)


### Features

* add more metrics for the tee_prover
([#3276](#3276))
([8b62434](8b62434))
* **api-server:** add `yParity` for non-legacy txs
([#3246](#3246))
([6ea36d1](6ea36d1))
* **consensus:** fallback json rpc syncing for consensus
([#3211](#3211))
([726203b](726203b))
* **contract-verifier:** Adapt contract verifier API for EVM bytecodes
([#3234](#3234))
([4509179](4509179))
* **contract-verifier:** Support Solidity contracts with EVM bytecode in
contract verifier
([#3225](#3225))
([8a3a82c](8a3a82c))
* **contract-verifier:** Support Vyper toolchain for EVM bytecodes
([#3251](#3251))
([75f7db9](75f7db9))
* **en:** Support Merkle tree recovery with pruning enabled
([#3172](#3172))
([7b8640a](7b8640a))
* ProverJobProcessor & circuit prover
([#3287](#3287))
([98823f9](98823f9))
* **prover:** Move prover_autoscaler config into crate
([#3222](#3222))
([1b33b5e](1b33b5e))
* **vm_executor:** Add new histogram metric for gas per tx in
vm_executor
([#3215](#3215))
([3606fc1](3606fc1))
* **vm:** add gateway changes to fast vm
([#3236](#3236))
([f3a2517](f3a2517))


### Bug Fixes

* **merkle-tree:** Repair stale keys for tree in background
([#3200](#3200))
([363b4f0](363b4f0))
* **tracer:** Add error to flat tracer
([#3306](#3306))
([7c93c47](7c93c47))
* use_dummy_inclusion_data condition
([#3244](#3244))
([6e3c36e](6e3c36e))
* **vm:** Do not require experimental VM config
([#3270](#3270))
([54e4b00](54e4b00))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <zksync-era-bot@users.noreply.github.com>
Deniallugo pushed a commit that referenced this pull request Nov 21, 2024
@zksync-era-bot @Deniallugo
chore(main): release core 25.2.0 (#3228)
68925bd 
🤖 I have created a release *beep* *boop*
---


##
[25.2.0](core-v25.1.0...core-v25.2.0)
(2024-11-19)


### Features

* add more metrics for the tee_prover
([#3276](#3276))
([8b62434](8b62434))
* **api-server:** add `yParity` for non-legacy txs
([#3246](#3246))
([6ea36d1](6ea36d1))
* **consensus:** fallback json rpc syncing for consensus
([#3211](#3211))
([726203b](726203b))
* **contract-verifier:** Adapt contract verifier API for EVM bytecodes
([#3234](#3234))
([4509179](4509179))
* **contract-verifier:** Support Solidity contracts with EVM bytecode in
contract verifier
([#3225](#3225))
([8a3a82c](8a3a82c))
* **contract-verifier:** Support Vyper toolchain for EVM bytecodes
([#3251](#3251))
([75f7db9](75f7db9))
* **en:** Support Merkle tree recovery with pruning enabled
([#3172](#3172))
([7b8640a](7b8640a))
* ProverJobProcessor & circuit prover
([#3287](#3287))
([98823f9](98823f9))
* **prover:** Move prover_autoscaler config into crate
([#3222](#3222))
([1b33b5e](1b33b5e))
* **vm_executor:** Add new histogram metric for gas per tx in
vm_executor
([#3215](#3215))
([3606fc1](3606fc1))
* **vm:** add gateway changes to fast vm
([#3236](#3236))
([f3a2517](f3a2517))


### Bug Fixes

* **merkle-tree:** Repair stale keys for tree in background
([#3200](#3200))
([363b4f0](363b4f0))
* **tracer:** Add error to flat tracer
([#3306](#3306))
([7c93c47](7c93c47))
* use_dummy_inclusion_data condition
([#3244](#3244))
([6e3c36e](6e3c36e))
* **vm:** Do not require experimental VM config
([#3270](#3270))
([54e4b00](54e4b00))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <zksync-era-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Artemka374 Artemka374 Artemka374 approved these changes

@popzxc popzxc popzxc approved these changes

@RomanBrodetski RomanBrodetski RomanBrodetski approved these changes

@yorik yorik yorik approved these changes

@alexandrst88 alexandrst88 Awaiting requested review from alexandrst88 alexandrst88 is a code owner automatically assigned from matter-labs/devops

@artmakh artmakh Awaiting requested review from artmakh artmakh is a code owner automatically assigned from matter-labs/devops

@hatemosphere hatemosphere Awaiting requested review from hatemosphere hatemosphere is a code owner automatically assigned from matter-labs/devops

@onyxet onyxet Awaiting requested review from onyxet onyxet is a code owner automatically assigned from matter-labs/devops

@otani88 otani88 Awaiting requested review from otani88 otani88 is a code owner automatically assigned from matter-labs/devops

@iluwaa iluwaa Awaiting requested review from iluwaa iluwaa is a code owner automatically assigned from matter-labs/devops

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

Successfully merging this pull request may close these issues.
5 participants
@EmilLuta @yorik @RomanBrodetski @popzxc @Artemka374