-
Notifications
You must be signed in to change notification settings - Fork 184
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: handle consecutive kernels in IVC #8924
Conversation
@@ -148,9 +148,7 @@ void ClientIVC::complete_kernel_circuit_logic(ClientCircuit& circuit) | |||
*/ | |||
void ClientIVC::accumulate(ClientCircuit& circuit, const std::shared_ptr<VerificationKey>& precomputed_vk, bool mock_vk) | |||
{ | |||
is_kernel = !is_kernel; // toggle on each call (every even circuit is a kernel) |
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 the main point of this PR. IVC will non longer internally assume that every other circuit is a kernel. Instead it will check circuit.databus_propagation_data.is_kernel
. This tag will be set to true automatically in ACIR land when the corresponding noir program uses calldata (since only kernels use calldata). Everywhere else (e.g. random test suites), the tag must be set manually for now.
ivc.accumulate(circuit_0); | ||
} | ||
// Initialize the IVC with an arbitrary circuit | ||
Builder circuit_0 = create_mock_circuit(ivc, /*is_kernel=*/false); |
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.
Couldn't you have is_kernel = false
by default for mock circuits so that you only need a second parameter to this function when the circuit is acctually a kernel and you need to set it to true
?
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 could but the method already has a 3rd argument with a default value and I try to avoid having methods with multiple default arguments when possible. This whole test suite will be deleted soon enough anyway
barretenberg/cpp/src/barretenberg/client_ivc/mock_circuit_producer.hpp
Outdated
Show resolved
Hide resolved
…ucer.hpp Co-authored-by: maramihali <mara@aztecprotocol.com>
…c-packages into lde/trigger_is_kernel
Prior to this PR, the IVC scheme made the assumption that every other circuit was a kernel, i.e. app, kernel, app, kernel, ... etc. In practice, however, it will be common to have two or more consecutive kernels without a corresponding app, e.g. an inner kernel followed immediately by a reset kernel. This PR updates the IVC so that whether or not a circuit is treated as a kernel is determined by the the already existing tag
circuit.databus_propagation_data.is_kernel
.When constructing circuits from noir programs, the above flag is set to true if and only if the circuit has calldata (which apps never do). This allows us to reinstate the full set of circuits in the ivc integration test suite which contains 3 consecutive kernels (inner, reset, tail).
In accordance with this change I had to add explicit setting of
is_kernel
to various test suites and flows which previously utilized the default assumption that every other circuit was a kernel. (Many of these cases will soon go away once we are ready to do away with theauto_verify_mode
version of IVC which exists for testing convenience and does not have any practical use case).Closes AztecProtocol/barretenberg#1111