-
Notifications
You must be signed in to change notification settings - Fork 4
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
Str-760: re-enable prover fn-tests #599
base: main
Are you sure you want to change the base?
Conversation
Commit: c59807d SP1 Performance Test Results
|
… db and reuse those if any.
…n native mode and are blazing fast.
1d718aa
to
d0c606b
Compare
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.
Good, this feels cleaner now even if it was largely reorganization.
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.
minor nit
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.
Great work on also cleaning up the trait. It was looking a bit clunky.
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.
ACK 54dad8d
Description
Re-enable prover fn-tests.
Type of Change
Notes to Reviewers
During the testing, it was discovered that ProverOp impls have a bug, so it required some refactoring in
prover-client
first.The essense of the bug: when dependency task is present (for example, when running
cl_stf
on some params and then runningcl_agg
that includescl_stf
with the same params), the implementation throws an error because it tries to write to db an existing entry.It's been decided to always check for dependencies tasks first, and reuse if any.
Checklist
Related Issues