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

Supernova: better slices handling #360

Open
tchataigner opened this issue Mar 8, 2024 · 2 comments
Open

Supernova: better slices handling #360

tchataigner opened this issue Mar 8, 2024 · 2 comments
Labels
good first issue Good for newcomers

Comments

@tchataigner
Copy link
Contributor

While trying to further my understanding of how to handle the program counter I have encountered a case where we have a panic in supernova::snark::CompressedSNARK::verify.

It happened a conversation I had with @adr1anh about what value should the program counter of the last StepCircuit of a NIVC computation. After the discussion, I tried to set the pc to -F::ONE and I ended up with a panic as I was trying to access an index out of bounds.

This is due to the fact that in the supernova codebase there are a few places (1 , 2, ...) where we try to access indexes without ensuring they exist.

I suggest to use a .get method to return a proper error.

@tchataigner tchataigner added the good first issue Good for newcomers label Mar 8, 2024
@wwared
Copy link
Contributor

wwared commented Mar 8, 2024

Worth mentioning: returning a dummy out-of-bounds PC value might turn out to be an anti-pattern that we might not want to necessarily support (depending on the nature of the panics we're getting); a possible alternative would be including an additional "terminal" circuit that exists but is always unsatisfiable, and return the PC value as the index for this terminal circuit

@porcuquine
Copy link
Contributor

I fully support panicking in the case of a programmer error that should never survive into a finished application. I think it's reasonable to give a more instructive message when so panicking, though.

The point is: that it is categorically wrong for a circuit to supply an invalid next program counter, so the only question is how gently we want to let the programmer down. I don't think it needs to be gentle at all, but there's no harm in being instructive.

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

No branches or pull requests

3 participants