Implement no_std support #280

xoloki · 2019-07-22T20:52:09Z

This change implements no_std support for the bulletproofs crate. The changes are mostly mechanical. This is the culmination several PRs worth of work, with changes to curve25519 and merlin crates already merged and released.

It was necessary to make minor modifications to some APIs, particularly in regards to RNGs. As discussed by @isislovecruft in #275, the APIs in this change are backwards compatible for those building the crate in std mode. So current users of the bulletproofs crate will see no change. Users wishing to build bulletproofs in no_std mode will need to call the new *_with_rng prove and verfiy functions, and pass in a RNG.

This change is based on the main branch rather than the develop branch. This is because the main branch in the dalek repo has several changes that are not in the develop branch. This made rebasing my change on the upstream develop branch to be problematic.

oleganza · 2019-07-22T22:04:08Z

This looks cool. Will check in more detail tomorrow.

hdevalence · 2019-07-23T17:43:48Z

It looks like one of the CI failures is just because of rustfmt changes that haven't been applied to the codebase yet; I can make a separate change that fixes them so that this branch can be merged cleanly on top of the fixed style.

arthurgreef · 2019-07-24T20:15:19Z

Hi - I pulled the branch but I cannot bet a clean build. Can you provide me with you build command? Is is cargo build --no-default-features --features alloc? Thanks.

xoloki · 2019-07-24T20:28:38Z

Hi - I pulled the branch but I cannot bet a clean build. Can you provide me with you build command? Is is cargo build --no-default-features --features alloc? Thanks.

I pushed a change to the branch that forces the rand dependency to 0.6, which appears to fix this. Your cargo build command will now work.

I was testing the no_std support in an application that used bulletproofs in no_std mode, so it is likely that some other dependency was already requesting 0.6 and cargo's dependency resolution was never actually using a 0.5 rand.

xoloki · 2019-07-24T21:06:34Z

It looks like one of the CI failures is just because of rustfmt changes that haven't been applied to the codebase yet; I can make a separate change that fixes them so that this branch can be merged cleanly on top of the fixed style.

The badly formatted code was all in places that I changed, so I went ahead and fixed my branch @hdevalence

xoloki · 2019-07-24T21:10:27Z

All checks are now passing.

arthurgreef · 2019-07-25T01:15:44Z

We are using cargo-nono to see if the libraries reference std and are noticing that the dependencies do. I that expected? Thanks.

curve25519-dalek: FAILURE

Source code contains an explicit use std:: statement.
--> src/backend/vector/ifma/edwards.rs
Source code contains an explicit use std:: statement.
--> src/prelude.rs
clear_on_drop: SUCCESS
subtle: SUCCESS
byteorder: SUCCESS
digest: FAILURE
Source code contains an explicit use std:: statement

hdevalence · 2019-07-30T19:52:44Z

Re: curve25519-dalek failure, it might be because the version with no_std fixes hasn't been released yet, I can check and do a new release if not. There's a security audit soon-to-be-released for these libraries with some minor issues so we might batch no_std fixes into those fixes instead of doing two at once.

I updated the develop branch so there should not be any more divergence between the main and develop branches.

hdevalence · 2019-08-01T20:40:00Z

Hi all, recent changes to Rust nightlies broke the build (by changing the path root for doc(include) path specifiers), so we had to update the develop branch and release 1.0.4 which works on current nightlies. The current develop branch is now back in sync with the main branch, is ready for more changes to be merged in, and won't have any more drastic changes (except to the R1CS code).

…ropriate for std and nostd

…m where they are used

…ting default features anymore

…ve Ristretto and Scalar serialization; convert to and from byte arrays when dealing with serialized data structures

… serializing any curve points or scalars directly

…since RangeProof supports it natively with to/from_bytes()

…mode use alloc::vec::Vec

…s as _with_rng, then creating a thread_rng in the compatibility functions in std mode

…erence to the wrapped functions in the convenience wrappers' block comments

xoloki · 2019-08-07T00:10:40Z

BTW the CI failures appear to be spurious, it was just failure to connect to github:

fatal: unable to access 'https://github.com/dalek-cryptography/bulletproofs.git/': Failed to connect to github.com port 443: Connection timed out
The command "eval git clone --depth=50 https://github.com/dalek-cryptography/bulletproofs.git dalek-cryptography/bulletproofs " failed. Retrying, 2 of 3.

…out default features just works

xoloki · 2019-08-12T19:31:29Z

Okay @hdevalence I got rid of the alloc feature and just used alloc everywhere. Building without default features just works now.

Let me know if you want me to do anything else.

xoloki · 2019-08-12T19:40:40Z

All tests passing now @hdevalence

hdevalence · 2019-08-13T20:21:43Z

src/range_proof/dealer.rs

@@ -295,15 +302,19 @@ impl<'a, 'b> DealerAwaitingProofShares<'a, 'b> {
    /// performing local aggregation,
    /// [`receive_trusted_shares`](DealerAwaitingProofShares::receive_trusted_shares)
    /// saves time by skipping verification of the aggregated proof.
-    pub fn receive_shares(mut self, proof_shares: &[ProofShare]) -> Result<RangeProof, MPCError> {
+    pub fn receive_shares<T: RngCore + CryptoRng>(


I guess this needs to be a new receive_shares_with_rng function, because receive_shares is part of the public API and can't change without a major version bump.

Ok, I'll rework these so the API doesn't change in std mode.

Thanks! Sorry again for missing this on the first pass of review 😞

hdevalence · 2019-08-13T20:22:09Z

src/range_proof/party.rs

@@ -68,20 +70,18 @@ pub struct PartyAwaitingPosition<'a> {
 impl<'a> PartyAwaitingPosition<'a> {
    /// Assigns a position in the aggregated proof to this party,
    /// allowing the party to commit to the bits of their value.
-    pub fn assign_position(
+    pub fn assign_position<T: RngCore + CryptoRng>(


similarly here, assign_position is a stable API

hdevalence · 2019-08-13T20:22:31Z

src/range_proof/party.rs

@@ -155,12 +155,11 @@ pub struct PartyAwaitingBitChallenge<'a> {
 impl<'a> PartyAwaitingBitChallenge<'a> {
    /// Receive a [`BitChallenge`] from the dealer and use it to
    /// compute commitments to the party's polynomial coefficients.
-    pub fn apply_challenge(
+    pub fn apply_challenge<T: RngCore + CryptoRng>(


apply_challenge is also stable

Ok, I'll make these changes today.

hdevalence · 2019-08-13T20:27:07Z

Hey @xoloki, this looks great, I would like to merge it, thanks for all this work!

I was doing a final pass over the combined changes and I spotted something I should have noticed before, but didn't: the functions in the aggregation module that were changed to take RNG parameters are part of the public API of the crate, so they can't be changed without breaking stability. I'm sorry I didn't spot it before and that there's another round of back-and-forth, but I think that that is the only thing left.

xoloki · 2019-08-13T20:28:34Z

No worries, thanks for all your help getting this in shape.

xoloki · 2019-08-14T00:44:07Z

Ok @hdevalence I fixed the public API to be backwards compatible in std mode, and renamed the nostd public API elements _with_rng.

Let me know if you need any more changes.

oleganza · 2019-08-14T07:41:32Z

src/util.rs

@@ -64,7 +68,7 @@ pub fn exp_iter(x: Scalar) -> ScalarExp {
 pub fn add_vec(a: &[Scalar], b: &[Scalar]) -> Vec<Scalar> {
    if a.len() != b.len() {
        // throw some error
-        println!("lengths of vectors don't match for vector addition");
+        //println!("lengths of vectors don't match for vector addition");


Maybe add panic?

I think we can probably clean this up in a different PR

src/range_proof/party.rs

Co-Authored-By: Henry de Valence <[email protected]>

hdevalence · 2019-08-14T17:59:33Z

src/range_proof/mod.rs

-        singleparty_create_and_verify_helper(64, 8);
+        // XXX disable this test since it's failing with
+        //     'attempt to negate with overflow'
+        //singleparty_create_and_verify_helper(64, 8);


This seems kind of worrying (in the sense that it may point at a problem in the rest of the source code) -- is it reproducible? I tried to reproduce on my machine but I wasn't able to.

Weird, I thought I reverted this. It was failing locally, but not remotely in CI.

Reverted now.

hdevalence · 2019-08-14T18:01:38Z

I have a cherry-pick'able commit on a copy of this branch (hdevalence@4191781 ) that undoes the changes to the test code, so that the test code continues to use the previous aggregation API. This way we test that the previous API still works correctly.

this ensures that the previous API still works

xoloki · 2019-08-14T18:35:48Z

I have a cherry-pick'able commit on a copy of this branch (hdevalence@4191781 ) that undoes the changes to the test code, so that the test code continues to use the previous aggregation API. This way we test that the previous API still works correctly.

The cherry has been picked, thanks!

xoloki · 2019-09-12T17:43:47Z

Hi @hdevalence, is there anything else you need from me? It would be super helpful if we can get this merged by the end of the month, our auditors would be much happier reviewing an upstream release rather than a fork and a PR.

hdevalence · 2019-09-23T20:49:37Z

Hey @xoloki -- I was on vacation so I missed this; I'll try to get this merged before the end of the week!

xoloki · 2019-09-23T22:29:25Z

Excellent, thanks @hdevalence!

hdevalence · 2019-09-27T15:24:41Z

Planning to get this merged this afternoon!

hdevalence

LGTM!

hdevalence · 2019-09-28T00:50:11Z

Hey @xoloki, I think I'm out of time to do a release for this today but I will do it at the beginning of next week. Sorry that this took so long and thanks so much for your work on this :)

xoloki · 2019-09-28T00:59:06Z

Thanks for the merge! Next week is totally fine for a release.

hdevalence mentioned this pull request Jul 24, 2019

Merge main into develop #277

Merged

xoloki added 20 commits August 2, 2019 12:46

turn off default features in Cargo.toml, and define and select as app…

c4499ab

…ropriate for std and nostd

use dependency injection to pass RNGs around rather than creating the…

91d36eb

…m where they are used

include std/alloc explicitly for curve25519-dalek since we aren't get…

5984ec0

…ting default features anymore

use merlin from fork for nostd fixes

bb86ad1

turn off fancy serde error code to try to fix nostd errors

dd4ee75

use upstream dalek curve release, without using serde; remove all nai…

be9c393

…ve Ristretto and Scalar serialization; convert to and from byte arrays when dealing with serialized data structures

add byte buffer data structures to enable naive serialization without…

84e6924

… serializing any curve points or scalars directly

add no_std cfg_attr in base lib.rs; remove unused serialization code …

93842a7

…since RangeProof supports it natively with to/from_bytes()

only reference core directly if we're in std mode; if we're in alloc …

5f6d068

…mode use alloc::vec::Vec

try putting cfg_if in generators to pickup alloc Vec

23813f1

use cfg_if to pickup Vec in alloc mode

06f5e81

use cfg_if to pickup Vec in alloc mode

f0d37d0

use cfg_if to pickup iter and Borrow in alloc mode

9c2bc09

use core::iter; replace vec! usage

60eb7e8

remove println

d01f0c6

re-enable range proof serialzation

baa759f

use naive curve point serialization in internal data structures

de18a44

make API backward compatible in std mode, renaming templated function…

196f072

…s as _with_rng, then creating a thread_rng in the compatibility functions in std mode

remove unnecessary import and use of ThreadRng; make standardized ref…

332458f

…erence to the wrapped functions in the convenience wrappers' block comments

use version only major version for curve25519 dependency

f929674

xoloki added 3 commits August 12, 2019 12:20

remove alloc feature and just use alloc everywhere, now building with…

dc1238b

…out default features just works

pull in review changes

ee0678e

Merge branch 'develop' into nostd

c1890b1

fmt fixes

415e355

hdevalence reviewed Aug 13, 2019

View reviewed changes

keep public API backwards compatible when in std mode

b451bcc

oleganza reviewed Aug 14, 2019

View reviewed changes

hdevalence reviewed Aug 14, 2019

View reviewed changes

src/range_proof/party.rs Outdated Show resolved Hide resolved

Fix bad copypasta, remove template parameters

04533f5

Co-Authored-By: Henry de Valence <[email protected]>

hdevalence reviewed Aug 14, 2019

View reviewed changes

xoloki and others added 2 commits August 14, 2019 11:31

revert test change to disable test that was failing locally

4a523df

restore original test code

76c0e75

this ensures that the previous API still works

hdevalence approved these changes Sep 28, 2019

View reviewed changes

hdevalence merged commit 426c87a into dalek-cryptography:develop Sep 28, 2019

Implement no_std support #280

Implement no_std support #280

Conversation

xoloki commented Jul 22, 2019

oleganza commented Jul 22, 2019

hdevalence commented Jul 23, 2019

arthurgreef commented Jul 24, 2019

xoloki commented Jul 24, 2019

xoloki commented Jul 24, 2019

xoloki commented Jul 24, 2019

arthurgreef commented Jul 25, 2019 • edited Loading

hdevalence commented Jul 30, 2019

hdevalence commented Aug 1, 2019

xoloki commented Aug 7, 2019

xoloki commented Aug 12, 2019

xoloki commented Aug 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hdevalence commented Aug 13, 2019

xoloki commented Aug 13, 2019

xoloki commented Aug 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hdevalence commented Aug 14, 2019

xoloki commented Aug 14, 2019

xoloki commented Sep 12, 2019

hdevalence commented Sep 23, 2019

xoloki commented Sep 23, 2019

hdevalence commented Sep 27, 2019

hdevalence left a comment

Choose a reason for hiding this comment

hdevalence commented Sep 28, 2019

xoloki commented Sep 28, 2019

arthurgreef commented Jul 25, 2019 •

edited

Loading