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

Non-'static second phase #323

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

rubdos
Copy link
Contributor

@rubdos rubdos commented May 20, 2020

Continuation of a discussion at #276: the second phase challenges can only use FnOnce + 'static since #276.

This effectively breaks the API (and apparently, rightfully, annoys @oleganza)! We have to pass a generic lifetime to the RandomizableConstraintSystem trait in order to constrain the lifetime to the container in Prover and Verifier. I don't think there's another way, except maybe with GATs (which would bring different API breaks anyhow).

Example of breaking API in Spacesuit

How about upgrading from Fn to FnOnce in this PR, but keeping the 'static requirement, and then separately explore if moving to generic lifetime bound is worth it? Right now i have to propagate 'a in a ton of places in ZkVM, because where I had a vm::Verifier, now I have vm::Verifier<'a>, and it leaks to PrecomputedTx that wraps it, etc. I could probably slap 'static at some point inside ZkVM/Spacesuit, but that would push the burden of justification on my codebase and I'm just... ugh... not ready for it right now :-P

  • Maybe this needs an additional interesting test-case for multiple different lifetimes and borrows?
  • Not specifying an explicit lifetime on &mut self might get people into trouble with late-bound lifetimes. I've had that while experimenting on it, but haven't come to fix it yet. I think it should be possible to add it later though, without breaking API...

Depends on #244/#276 (I'll rebase on develop when #276 is merged, too much conflicts to handle otherwise)

It allows for the challenge phase to accept closures that take some more
context, e.g. moving non-Clone values in the challenge closure.

This works in stable since Rust 1.35; cfr.
rust-lang/rust#28796.

This closes issue dalek-cryptography#244.
This adds a general lifetime for the second-phase constraints on
RandomizeableConstraintSystem, which should cover the lifetime of each
closure given to specify_randomized_constraints().  The latter can
receive shorter lifetimes.
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

Successfully merging this pull request may close these issues.

None yet

1 participant