-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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(prover): Update witness generator to zkevm_test_harness 0.150.6 #3029
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very WIP state, but I love the direction. Not having to deal with partial circuits is neat. What I don't understand, how did we manage to compact it, without blowing up the memory?
P.S. We'll have to adjust circuit prover as well.
@@ -470,6 +470,7 @@ impl Keystore { | |||
} | |||
|
|||
/// Async loads mapping of all circuits to setup key, if successful | |||
#[cfg(feature = "gpu")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, was this a miss on my end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a separate note, did you manage to run this?
let artifacts_receiver_handle = thread::spawn(move || { | ||
let span = tracing::info_span!(parent: make_circuits_span_copy, "make_circuits_blocking"); | ||
|
||
while let Ok(artifact) = artifacts_receiver.recv() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine, but shouldn't we do something if we can't receive? Maybe log the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we can have only RecvError
, which occurs if the sender is disconnected. This is the expected state after test_harness has finished its work, I don't think we want to log this.
This process will happen in two steps: in the first step (this PR) we simply fill unnecessary fields with zeros to maintain backward compatibility. The next step is to change the input structures of the circuits, from which we will remove unnecessary fields. We will do this during the protocol update.
Yes. But we need to update Shivini, which is part of another monorepo, so that the prover can be compiled in CI
I'm not sure about it. I'll check, but at this stage it shouldn't affect it in any way. |
What ❔
Why ❔
Checklist
zk_supervisor fmt
andzk_supervisor lint
.