search

dalek-cryptography / bulletproofs

A pure-Rust implementation of Bulletproofs using Ristretto.
MIT License
1.02k stars 218 forks source link

Make closure of challenge phase FnOnce. #276

Closed rubdos closed 4 years ago

rubdos commented 5 years ago

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. https://github.com/rust-lang/rust/issues/28796.

This closes issue #244.

In other news: I'm not yet happy with the 'static bound on the closures. Since I'm not yet worried about that kind of performance (just hacking around currently), I'm not going to fight the borrow checker there, yet. Cfr #323

rubdos commented 5 years ago

Seems like the CI fails on the Rust version (which should be 1.35 for this to work).

rubdos commented 5 years ago

The new build failure is because of the bunch of recent deprecations in merlin.

hdevalence commented 5 years ago

Hi, those deprecations were fixed in the main branch of this crate but those changes weren't merged into the develop branch. I can update the develop branch so that this PR can work again.

rubdos commented 5 years ago

Is develop still being used then? Otherwise I'll rebase this on main.

hdevalence commented 5 years ago

Yep, it's still being used, it's just that those changes were included as part of a PR that added additional validation to the rangeproof code, so they were put into the main branch. The develop branch is still for ongoing work on the R1CS implementation, which slowed a bit but will probably pick up again soon.

rubdos commented 5 years ago

Ok, fair! So, would you mind porting those deprecation fixes to develop then, then I'll rebase.

hdevalence commented 5 years ago

Currently in-progress with #277!

rubdos commented 4 years ago

I've rebased this over develop. I'm looking into the 'static lifetime too now, so hold your horses for a few minutes :-)

rubdos commented 4 years ago

Looks like I managed to shorten the lifetime. I added a test that would fail before and succeed after the patch. It does however mean that all Provers and Verifiers, and RandomizableConstraintSystem all get some extra lifetimes (which technically would mean breaking the API). Since it's behind a yolo-feature, I assume that's not a big issue.

EDIT: feel free to cherry-pick whichever of the patches you like already!

oleganza commented 4 years ago

Does non-static lifetime work well with multiple calls to specify_randomized_constraints? I'll try this out with zkvm and spacesuit next week.

oleganza commented 4 years ago

That's an exciting update, thanks!

rubdos commented 4 years ago

Does non-static lifetime work well with multiple calls to specify_randomized_constraints? I'll try this out with zkvm and spacesuit next week.

It should! I currently have no use for it myself, but I may soon have. If it doesn't work out, feel free to add a test case and I'll take a look.

rubdos commented 4 years ago

I dropped the non-'static stuff from this PR and created #323 in favour of that. I'll rebase this on develop now.