-
Notifications
You must be signed in to change notification settings - Fork 241
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: mock data for IVC #9893
feat: mock data for IVC #9893
Conversation
{ | ||
size_t total_size = 0; |
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.
I think this being 0 was just a bug that never showed up in any tests since +1 never made a difference in the dyadic size
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.
interesting. I guess we need more tests for borderline circuit sizes
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.
yeah I only caught it because I accidentally propagated it to Ultra where we do just happen to have a case that's right on the line
for (auto block : this->get()) { | ||
total_size += block.get_fixed_size(); | ||
} | ||
return total_size; | ||
|
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.
just an update to allow this method to compute the dyadic size itself since this is how it was always being used
@@ -75,39 +76,11 @@ class IvcRecursionConstraintTest : public ::testing::Test { | |||
}; | |||
} | |||
|
|||
/** |
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.
Removing this test because it was a pain to keep updating and is also no longer relevant because the kernel circuits will not do anything with public inputs anymore since the introduction of the databus.
Changes to public function bytecode sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
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.
not fully understanding what's going on here, but seems fine.
// Construct kernel consisting only of the kernel completion logic | ||
AcirProgram program = construct_mock_kernel_program(ivc.verification_queue, { num_app_public_inputs }); | ||
Builder kernel = acir_format::create_kernel_circuit(program.constraints, ivc); | ||
// WORKTODO: this would normally happen in accumulate() |
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.
unresolved
{ | ||
size_t total_size = 0; |
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.
interesting. I guess we need more tests for borderline circuit sizes
Builder app_circuit = construct_mock_app_circuit(ivc); | ||
ivc.accumulate(app_circuit); | ||
// Test generation of "init" kernel VK via dummy IVC data | ||
TEST_F(IvcRecursionConstraintTest, GenerateVKFromConstraints) |
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.
what's missing from this test?
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.
not sure what you mean - I wouldn't say anything is missing, its just that right now I'm only supporting the mocking of the IVC state after accumulation of the first app. All that's needed I suppose is functionality to mock fold proofs which shouldn't be hard
* master: git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg feat: mock data for IVC (#9893) refactor: final token cleanup (#9864) chore: delete accidentally added file (#9912)
Incremental work towards a write_vk flow for kernel circuits. kernel circuits are generated from acir via the method
acir_format::create_kernel_circuit()
which takes as input the raw acir data plus a ClientIvc instance containing the proofs to be recursively verified in that circuit (Oink/PG + merge). In the context of VK generation, those proofs are not yet known, so the IVC state has to be mocked. This PR adds such functionality but only for the oink case (i.e. the state of the IVC after accumulating the first app which only calls the pink prover). Equivalent logic for mocking the state after subsequent accumulations will be handled in a follow on.