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.

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

Fix how we handle public inputs in Tube circuit #1048

Open
lucasxia01 opened this issue Jul 5, 2024 · 5 comments
Open

Fix how we handle public inputs in Tube circuit #1048

lucasxia01 opened this issue Jul 5, 2024 · 5 comments

Comments

@lucasxia01
Copy link
Contributor

We currently just add some (correct amount of) public inputs as new variables to the builder when constructing the Tube circuit. However, we need to set witnesses (from the folding proof) to be public inputs instead.

@ludamad
Copy link
Collaborator

ludamad commented Jul 5, 2024

@Rumata888 can you add more details here? I recall you thinking this was tricky

@Rumata888
Copy link
Contributor

There are two problems right now:

  1. Recursive verifiers take a vector of field elements as inputs, construct the transcript from them and only after that recursively verify the proof. The current architecture doesn't allow checking the correctness of those field elements (that the transcript in the decider corresponds to the Translator transcript, for example). We don't have access to those elements in circuit field form, so we can't set that as public input
  2. The concept of the public input of the circuit and the context of results of the folding proof differ. It is ambiguous right now in the protocol what we want the Tube to present as public inputs. If the public inputs are just the public inputs of the last folding proof then it can become a problem, because technically two folding proofs can have the same public inputs and if we export just those it might not be possible to correctly pick the original ClientIVC proof that produced those. So technically what we need to export is probably at least the folding proof transcript. Maybe we need to show everything that we are recursively verifying as public inputs.

@ledwards2225
Copy link
Collaborator

Aside from the security caveats above, this is a basic description of what needs to be done to pipe the right public inputs into the base rollup:

The base rollup circuit needs to perform some checks on the public inputs of the tail private kernel. With the Honk integration / introduction of the Tube circuit, the public inputs connection was broken. What needs to happen is roughly the following:

  • a ClientIvc Proof contains a FoldProof which contains the explicit public inputs of the final circuit in the IVC (kernel tail or whatever)
  • These public inputs are extracted in the ProtoGalaxyRecursiveVerifier (part of the ClientIvc recursive verifier, aka the tube) where they become private witnesses by default
  • These witnesses must be explicitly converted to public inputs in that circuit (e.g. builder->set_public_input(pub_input.get_witness_index()) )
  • This will result in those public inputs being explicitly added to the Tube circuit proof which gets sent to the base rollup circuit
  • The base rollup circuit extracts the public inputs from the proof and uses them for Azteky checks as needed

ludamad added a commit to AztecProtocol/aztec-packages that referenced this issue Jul 11, 2024
**THE AWESOME:**
PXE uses ClientIVC. Public kernels, rollup, and parity circuits use
vanilla recursion-based IVC using UltraHonk. 🎉
Linear time proofs without FFTs, better constraint counts for poseidon
(eventually) and no recursion in the private kernel circuits

**THE CAVEATS:**
This merges with many TODOs to reinstate functionality, improve security
and performance.

- If you are lead to believe honk proofs are constant-sized, you have
been fed lies

We are rounding up to a proof size that allows 2**28 gates. Please take
effort to NOT rely on this assumption, as it is just a stop-gap. It is
OK to temporarily rely on this to simplify kernel development in
particular, but we should not hard-code this assumption into e.g.
typescript in the future
- Things are a bit slower now, not faster

We prioritized correctness and any slowdowns will be investigated soon.
Some of it comes from the larger constant size proof. As well, we will
soon be able to drop constraint count in poisedon with UH

- Transcript hashing is not yet in place
- #7410

Alvaro's recent work with the VK tree had to be partially undone (aka
the parts that made it really validate in kernels, so most of it for
now) because our tube VK currently cannot be written ahead of time.
What's more, correctly handling log-sized proofs will mean many tube and
kernel VKs in the future. This will need a pass.

- #7374

This test used a lot of mocking and was stubbed out in the interest of
time. It's not necessarily hard to bring back.

- #7373

We need one soon. The relevant deploying functionality has been
commented out for now with this issue linked.

- #7372

This needs to be double-checked for current kernel proof, but we will do
this post-merge with a pass on observability

- #7371 

We need to figure out how to get this program stack bincode-encoded in
the future

- #7370

Right now redundant VKs are serialized with the client ivc proofs

- #7369

Related to #7410, currently tube proving is too unlike the rest of the
pattern

- #7368

It's currently a bit of a mess of grabbing bytecode artifacts with
ad-hoc logic

- AztecProtocol/barretenberg#1048

Inputs should be passed to the tube circuit correctly, but they are NOT
currently used. Fixing this should be contained within bb

---------

Co-authored-by: codygunton <[email protected]>
Co-authored-by: ledwards2225 <[email protected]>
Co-authored-by: lucasxia01 <[email protected]>
Co-authored-by: maramihali <[email protected]>
AztecBot pushed a commit that referenced this issue Jul 11, 2024
**THE AWESOME:**
PXE uses ClientIVC. Public kernels, rollup, and parity circuits use
vanilla recursion-based IVC using UltraHonk. 🎉
Linear time proofs without FFTs, better constraint counts for poseidon
(eventually) and no recursion in the private kernel circuits

**THE CAVEATS:**
This merges with many TODOs to reinstate functionality, improve security
and performance.

- If you are lead to believe honk proofs are constant-sized, you have
been fed lies

We are rounding up to a proof size that allows 2**28 gates. Please take
effort to NOT rely on this assumption, as it is just a stop-gap. It is
OK to temporarily rely on this to simplify kernel development in
particular, but we should not hard-code this assumption into e.g.
typescript in the future
- Things are a bit slower now, not faster

We prioritized correctness and any slowdowns will be investigated soon.
Some of it comes from the larger constant size proof. As well, we will
soon be able to drop constraint count in poisedon with UH

- Transcript hashing is not yet in place
- #7410

Alvaro's recent work with the VK tree had to be partially undone (aka
the parts that made it really validate in kernels, so most of it for
now) because our tube VK currently cannot be written ahead of time.
What's more, correctly handling log-sized proofs will mean many tube and
kernel VKs in the future. This will need a pass.

- #7374

This test used a lot of mocking and was stubbed out in the interest of
time. It's not necessarily hard to bring back.

- #7373

We need one soon. The relevant deploying functionality has been
commented out for now with this issue linked.

- #7372

This needs to be double-checked for current kernel proof, but we will do
this post-merge with a pass on observability

- #7371 

We need to figure out how to get this program stack bincode-encoded in
the future

- #7370

Right now redundant VKs are serialized with the client ivc proofs

- #7369

Related to #7410, currently tube proving is too unlike the rest of the
pattern

- #7368

It's currently a bit of a mess of grabbing bytecode artifacts with
ad-hoc logic

- #1048

Inputs should be passed to the tube circuit correctly, but they are NOT
currently used. Fixing this should be contained within bb

---------

Co-authored-by: codygunton <[email protected]>
Co-authored-by: ledwards2225 <[email protected]>
Co-authored-by: lucasxia01 <[email protected]>
Co-authored-by: maramihali <[email protected]>
@codygunton codygunton self-assigned this Jul 16, 2024
@codygunton codygunton removed their assignment Sep 10, 2024
@codygunton
Copy link
Collaborator

@ledwards2225 this will be closed by an upcoming PR of @sirasistant right? Or will that PR only pave the way for a final PR to fix this issue?

@ledwards2225
Copy link
Collaborator

@ledwards2225 this will be closed by an upcoming PR of @sirasistant right? Or will that PR only pave the way for a final PR to fix this issue?

This is unrelated to any ongoing work - I think you're thinking of the linking of the public inputs between noir witnesses and the witnesses in the recursive verifiers which is indeed completed by Alvaros recent merge but incidentally was never needed in the first place since the databus replaced the pub inputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants