ZKP fork for rust-secp256k1, adds wrappers for range proofs, pedersen commitments, etc

Background: when I perform a transaction on iOS wallet (which use grin codebase compiled for iOS), and later on I restore the wallet - each transaction maps to a weird BIP32 account:

____ Wallet Accounts ____

 Name      | Parent BIP-32 Derivation Path
-----------+-------------------------------
 account_0 | m/540140409/3298754560
 account_1 | m/543567437/4159635456

Command 'account' completed successfully

I've started to dig in and understood, that we are using a bulletproof message to store the BIP32 path for identifying the account during restore. Then, I observed that when I create a bulletproof with ProofMessage(00000000000000000000000000000000) as a message to ffi::secp256k1_bulletproof_rangeproof_prove (passed as the last parameter) and later call rewind_bullet_proof with the just created bulletproof I got a different message, like ProofMessage(800da21d66b30000c2001b3a29f0c041). So, I assume, the message I pass to ffi::secp256k1_bulletproof_rangeproof_prove for some reason get distorted.

I've tried to run the tests on iPhone using https://github.com/snipsco/dinghy, and they are failing with the following:

thread #2: tid = 0x3cd272, 0x0000000101056e18 Dinghy`secp256k1_fe_set_b32(r=0x000000016f239528, a="") at field_10x26_impl.h:324, name = 'pedersen::tests::test_bullet_proo[15/558]
ig', stop reason = EXC_BAD_ACCESS (code=1, address=0xa7974c91c050)
  frame #0: 0x0000000101056e18 Dinghy`secp256k1_fe_set_b32(r=0x000000016f239528, a="") at field_10x26_impl.h:324
  frame #1: 0x0000000101057d64 Dinghy`secp256k1_pedersen_commitment_load(ge=0x00000002833180a0, commit=0x0000a7974c91c030) at main_impl.h:31
  frame #2: 0x000000010105f110 Dinghy`secp256k1_bulletproof_rangeproof_prove(ctx=0x0000000283b00000, scratch=0x0000000283018120, gens=0x0000000280204060, proof=0x0000000000000000
, plen=0x0000000000000000, tau_x=0x0000000000000000, t_one=0x000000016f23df30, t_two=0x000000016f23df70, value=0x000000016f239b10, min_value=0x0000000000000000, blind=0x000000028
001c020, commits=0x000000028001c030, n_commits=1, value_gen=0x000000010114a3e1, nbits=64, nonce="bdnW\ #\xab\xa342\xb6.(\x93f", private_nonce="𧜒\x8cV5%\xb1K(iVƥ\x02\xa9c;\x19RT\
xb5\\\xba\xb7, extra_commit=0x0000000000000000, extra_commit_len=0, message=0x0000000000000000) at main_impl.h:252
  frame #3: 0x000000010101e538 Dinghy`secp256k1zkp::pedersen::_$LT$impl$u20$secp256k1zkp..Secp256k1$GT$::bullet_proof_multisig::h5ef5f4a8d3562320(self=0x000000016f23dce8, value=1
2345678, blind=(__0 = "\n\xbfDL?]K[\vF\x01\x1e\xac$hv\xb0\x1c\x8c\x9foMg\x02\x1f\x86$]bdnW\ #\xab\xa342\xb6.(\x93f"), nonce=(__0 = "bdnW\ #\xab\xa342\xb6.(\x93f"), extra_data_in=
Option<alloc::vec::Vec<u8>> @ 0x000000016f23f410, message=Option<secp256k1zkp::pedersen::ProofMessage> @ 0x000000016f23f428, tau_x=Option<&mut secp256k1zkp::key::SecretKey> @ 0x0
00000016f239b18, t_one=Option<&mut secp256k1zkp::key::PublicKey> @ 0x000000016f239b20, t_two=Option<&mut secp256k1zkp::key::PublicKey> @ 0x000000016f239b28, commits=Vec<secp256k1
zkp::pedersen::Commitment> @ 0x000000016f23f458, private_nonce=Option<&secp256k1zkp::key::SecretKey> @ 0x000000016f239b30, step='\x01') at pedersen.rs:876
  frame #4: 0x0000000100fe4bcc Dinghy`secp256k1zkp::pedersen::tests::test_bullet_proof_multisig::_$u7b$$u7b$closure$u7d$$u7d$::hc44612881ad44945((null)=0x000000016f248c8f, v=1234
5678, nonce=(__0 = "bdnW\ #\xab\xa342\xb6.(\x93f\t\xad\x1b3i\t\xa843;TqG\x1bv\x9d}Y\x1b\xa905T\\\x85Y\xb0b\t\x9d\r\x15\x17ۊ\x9ed\\"\x03C\x05\x82Y\x12\"\x19\x94\x15\xa1\xacV\x83Ov
\x1f\xaem\n\\AOAXK\r\xa0ވ\xb5Ǐ"), ca=(__0 = "\t\xad\x1b3i\t\xa843;TqG\x1bv\x9d}Y\x1b\xa905T\\\x85Y\xb0b\t\x9d\r\x15\x17ۊ\x9ed\\"\x03C\x05\x82Y\x12\"\x19\x94\x15\xa1\xacV\x83Ov\x8\x1f\xaem\n\\AOAXK\r\xa0ވ\xb5Ǐ"), cb=(__0 = "\t\x9d\r\x15\x17ۊ\x9ed\\"\x03C\x05\x82Y\x12\"\x19\x94\x15\xa1\xacV\x83Ov\x84\n\xbfDL?]K[\vF\x01\x1e\xac$hv\xb0\x1c\x8c\x9foMg\x02\x1f\x1f\xaem\n\\AOAXK\r\xa0ވ\xb5Ǐ"), msg=Option<secp256k1zkp::pedersen::ProofMessage> @ 0x000000016f24a248, extra=Option<alloc::vec::Vec<u8>> @ 0x000000016f24a308) at pedersen.rs:1497
  frame #5: 0x0000000100fcf8e4 Dinghy`secp256k1zkp::pedersen::tests::test_bullet_proof_multisig::hf9efca6961c2e153 at pedersen.rs:1611
  frame #6: 0x0000000100fe45e4 Dinghy`secp256k1zkp::pedersen::tests::test_bullet_proof_multisig::_$u7b$$u7b$closure$u7d$$u7d$::he12b73a0ec3b3e1c((null)=0x000000016f24e8de) at pedersen.rs:1467
error: need to add support for DW_TAG_base_type '()' encoded with DW_ATE = 0x7, bit_size = 0
  frame #7: 0x000000010103cd70 Dinghy`core::ops::function::FnOnce::call_once::hab10037076c9bf16((null)=closure @ 0x000000016f24e8de, (null)=<unavailable>) at function.rs:238
  frame #8: 0x000000010108505c Dinghy`_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h7cd9458e96c61134 [inlined] test::run_test::_$u7b$$u7b$closure$u7d$$u7d$::hcfb9d86a7c3d2c96 at lib.rs:1468 [opt]
  frame #9: 0x0000000101085058 Dinghy`_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h7cd9458e96c61134 [inlined] core::ops::function::FnOnce::call_once::h4391d3a834d88d8b at function.rs:238 [opt]
  frame #10: 0x0000000101085058 Dinghy`_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h7cd9458e96c61134 at boxed.rs:672 [opt]
  frame #11: 0x000000010113ca84 Dinghy`__rust_maybe_catch_panic at lib.rs:102 [opt]
  frame #12: 0x00000001010a1b20 Dinghy`std::sys_common::backtrace::__rust_begin_short_backtrace::hfba72662a42f7cac [inlined] std::panicking::try::h732fe36cab73cb41 at panicking.r
s:289 [opt]
  frame #13: 0x00000001010a1af8 Dinghy`std::sys_common::backtrace::__rust_begin_short_backtrace::hfba72662a42f7cac [inlined] std::panic::catch_unwind::hac4e2f50e06b27fe at panic.
rs:392 [opt]
  frame #14: 0x00000001010a1af8 Dinghy`std::sys_common::backtrace::__rust_begin_short_backtrace::hfba72662a42f7cac [inlined] test::run_test::run_test_inner::_$u7b$$u7b$closure$u7
d$$u7d$::h006e4850988c9367 at lib.rs:1423 [opt]
  frame #15: 0x00000001010a1a5c Dinghy`std::sys_common::backtrace::__rust_begin_short_backtrace::hfba72662a42f7cac at backtrace.rs:136 [opt]
  frame #16: 0x00000001010a23d4 Dinghy`std::panicking::try::do_call::h4daf77ea764d46fd [inlined] std::thread::Builder::spawn::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d
$$u7d$::hdf44bf80f31b708f at mod.rs:409 [opt]
  frame #17: 0x00000001010a23ac Dinghy`std::panicking::try::do_call::h4daf77ea764d46fd [inlined] _$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce
$LT$$LP$$RP$$GT$$GT$::call_once::hfe8230c624f93446 at panic.rs:313 [opt]
  frame #18: 0x00000001010a23ac Dinghy`std::panicking::try::do_call::h4daf77ea764d46fd at panicking.rs:310 [opt]
  frame #19: 0x000000010113ca84 Dinghy`__rust_maybe_catch_panic at lib.rs:102 [opt]
  frame #20: 0x000000010109189c Dinghy`_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h9f47ca2e7bc599f5 [inlined] std::panicking::try::h25ee5a857b5a665a at panicki
ng.rs:289 [opt]
  frame #21: 0x0000000101091840 Dinghy`_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h9f47ca2e7bc599f5 [inlined] std::panic::catch_unwind::hb4be13ae309c1f2b at panic.rs:392 [opt]
  frame #22: 0x0000000101091840 Dinghy`_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h9f47ca2e7bc599f5 [inlined] std::thread::Builder::spawn::_$u7b$$u7b$closure$u7d$$u7d$::hc8e2b8523d8822bb at mod.rs:408 [opt]
  frame #23: 0x000000010109181c Dinghy`_$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h9f47ca2e7bc599f5 at boxed.rs:672 [opt]
  frame #24: 0x0000000101132334 Dinghy`std::sys::unix::thread::Thread::new::thread_start::h53cadc7a8d993d1d [inlined] _$LT$alloc..boxed..Box$LT$$LP$dyn$u20$alloc..boxed..FnBox$LT$A$C$$u20$Output$u3d$R$GT$$u20$$u2b$$u20$$u27$a$RP$$GT$$u20$as$u20$core..ops..function..FnOnce$LT$A$GT$$GT$::call_once::h1550d5c67ef40595 at boxed.rs:682 [opt]
  frame #25: 0x000000010113232c Dinghy`std::sys::unix::thread::Thread::new::thread_start::h53cadc7a8d993d1d [inlined] std::sys_common::thread::start_thread::ha4b13f99b17bd904 at thread.rs:24 [opt]
  frame #26: 0x0000000101132320 Dinghy`std::sys::unix::thread::Thread::new::thread_start::h53cadc7a8d993d1d at thread.rs:90 [opt]
  frame #27: 0x00000001c06e825c libsystem_pthread.dylib`<redacted> + 128
  frame #28: 0x00000001c06e81bc libsystem_pthread.dylib`_pthread_start + 48
 ERROR cargo_dinghy                > Error: LLDB returned error code Some(255)

I would appreciate any clues how I can fix this, thanks in advance!

Improve documentation on how the generator points have been dervied

This PR only changes comments in the constants.rs file.

When I tried to understand if and how the generation points for Pedersen and Switch Commitments have been derived in a secure, nothing-up-my-sleeve (NUMS) way, I realized it would be helpful to have a better explanation in the comments.

As important properties of the commitment schemes only hold if these points are chosen transparently, I thought this should be better documented.

Let me know what you think about it, or if I should change something. 🙂

Calculates the modified blinding factor x + H(xG+vH | xJ). Relevant grin PR: https://github.com/mimblewimble/grin/pull/2157

I got the raw bytes of J, starting by the compressed point in https://github.com/mimblewimble/rust-secp256k1-zkp/pull/9/files#diff-13288cc06e4963042a0eeb6b457647dfR85. I loaded these bytes as a public key, then dumped the bytes. Please, someone verify that I didn't make a mistake in this procedure.

For detail, please refer to https://github.com/mimblewimble/grin/issues/1321

This is the major fix to make the rangeproof batch verification work. The main benefit is speed! For example with 1000 range proofs, this batch verification speed is near 50 times faster as the total single range proof verifications.

And this PR also include an important optimization for range proof, use a shared generators, avoid recreating every time.

Further update to CI settings and fix for warnings:

• Use ubuntu-latest, windows-latest and macos-latest images
• Fix dyn warning
• Modify all instances of mem::uninitialized() to mem::MaybeUninit::uninit().assume_init()

Grin currently generates commits that look like this - Commitment(0251fb65e6cd4ae54749b59e139935dd4cfa10f0694aa8a2645c3950fe8785b198)

But rust-secp256k1-zkp generates commits that look like this - Commitment(08032fc3685013a35124c0275457e97a7ff0046ba763fa45409b95e8b3eae53425)

ffi::secp256k1_ec_pubkey_parse calls into secp256k1_eckey_pubkey_parse which checks that a 33 byte public key starts with 0x02 or 0x03

Q) Why is rust-secp256k1-zkp creating commitments that appear to be invalid?

In response to https://github.com/mimblewimble/grin/issues/2218. Corresponding PR in Grin: https://github.com/mimblewimble/grin/pull/2851

This PR:

• Adds zeroize crate as dependency
• Adds Zeroize derive for SecretKey in a way that underlying byte array is filled with zeroes
• Removes Copy derive from SecretKey since it is impossible to implement both Drop and Copy at the same time. Implicit copying was replaced with more explicit .clone()
• Adds unit test for SecretKey zeroing

Some references: https://github.com/dalek-cryptography/curve25519-dalek/issues/11 https://github.com/rust-lang/rfcs/issues/2533 https://www.youtube.com/watch?v=cQ9wTyYCdNU

Feedback is highly appreciated!

Two relatively small changes that will enable https://github.com/mimblewimble/grin-wallet/issues/105:

• Changed message size to 20 bytes
• Allow two separate nonces during generation

After adding this feature, we don't have any tests.

    pub fn bullet_proof(
        &self,
        value: u64,
        blind: SecretKey,
        nonce: SecretKey,
        extra_data: Option<Vec<u8>>,
        message: Option<ProofMessage>,
    ) -> RangeProof {
        ...
        // TODO: expose multi-party support
        let tau_x = ptr::null_mut();
        let t_one = ptr::null_mut();
        let t_two = ptr::null_mut();
        let commits = ptr::null_mut();
        let private_nonce = ptr::null();

@jaspervdm Could you please give me some examples to use this new feature? so as me to add some tests for it.

If I understand it correctly, then the blind and nonce can be chosen independently (even if the rust api sets nonce = blind) when creating and unwinding bulletproofs. However, if nonce != blind, then only the first 32 bytes of the message can be unwinded as in the following test which can be inserted into perdersen.rs.

	#[test]
	fn test_bullet_nonce_neq_blind() {
		use std;
		use super::*;
		let secp = Secp256k1::with_caps(ContextFlag::Commit);
		let mut rng = OsRng::new().unwrap();
		let value = 1234567;
		let blind = SecretKey::new(&secp, &mut rng);
		let nonce = SecretKey::new(&secp, &mut rng);
		//let nonce = blind; // this works
		let mut msg = [0u8; 64];
		rng.fill_bytes(&mut msg);

		// ffi structures
		let n_bits = 64;
		let mut proof = [0; constants::MAX_PROOF_SIZE];
		let mut plen = constants::MAX_PROOF_SIZE as size_t;
		let extra_data = vec![];

		// create proof
		let success = unsafe {
			ffi::secp256k1_bulletproof_rangeproof_prove_single_w_scratch(
				secp.ctx,
				proof.as_mut_ptr(),
				&mut plen,
				value,
				blind.as_ptr(),
				constants::GENERATOR_H.as_ptr(),
				n_bits as size_t,
				nonce.as_ptr(),
				extra_data.as_ptr(),
				extra_data.len() as size_t,
				msg.as_ptr()
			) == 1
		};
		assert!(success);

		// unwind proof
		let mut unwinded_msg = [0u8; 64];
		let commit = secp.commit(value, blind).unwrap();
		let success = unsafe {
			ffi::secp256k1_bulletproof_rangeproof_unwind_message(
				secp.ctx,
				proof.as_ptr(),
				plen as size_t,
				commit.as_ptr(),
				n_bits as size_t,
				constants::GENERATOR_H.as_ptr(),
				extra_data.as_ptr(),
				extra_data.len() as size_t,
				nonce.as_ptr(),
				unwinded_msg.as_mut_ptr(),
			) == 1
		};
		assert!(success);

		println!("msg:     {:?}", msg.to_vec());
		println!("unwinded:{:?}", unwinded_msg.to_vec());

		for i in 0..msg.len() {
			assert_eq!(msg[i], unwinded_msg[i]);
		}
	}

I am new to rust.

after I got the repository, I run

cargo build

then I got

error: no such file or directory: 'depend/secp256k1-zkp/contrib/lax_der_parsing.c'

what should I do? Can someone please help.

Implementing the Arbitrary trait allows libFuzzer to work faster, and simplifies fuzz driver code.

Implementations are conditionally compiled, so non-fuzz builds aren't affected.

Commitment and RangeProof implement Arbitrary for use in main grin repo.

As discussed in https://github.com/rust-bitcoin/rust-secp256k1/issues/209, it'd be helpful to rename the symbols so that they don't clash if multiple versions of the crate(s) are used.

Hi, This fork is quite an old fork of upstream, We've changed a lot since then. https://github.com/rust-bitcoin/rust-secp256k1

This PR copies the build.rs from upstream (with some small changes for the zkp stuff and the fact that things aren't feature gated), the upside is proper detection of 64bit targets and passing the according preprocessor definitions

On my 64bit laptop, before this PR:

test benches::bench_sign                                ... bench:      77,019 ns/iter (+/- 2,576)
test benches::bench_verify                              ... bench:      85,931 ns/iter (+/- 1,940)
test benches::generate                                  ... bench:      41,897 ns/iter (+/- 863)
test ecdh::benches::bench_ecdh                          ... bench:      79,895 ns/iter (+/- 1,975)
test pedersen::benches::bench_bullet_proof_verification ... bench:   3,393,491 ns/iter (+/- 102,317)

After:

test benches::bench_sign                                ... bench:      42,202 ns/iter (+/- 3,486)
test benches::bench_verify                              ... bench:      44,289 ns/iter (+/- 1,587)
test benches::generate                                  ... bench:      23,601 ns/iter (+/- 626)
test ecdh::benches::bench_ecdh                          ... bench:      48,662 ns/iter (+/- 2,532)
test pedersen::benches::bench_bullet_proof_verification ... bench:   1,868,231 ns/iter (+/- 82,517)

I do suggest to merge/rebase this repository from/on upstream, or manually copy the changes you want because the library got a lot better. These are just some examples of good/important changes we've made to the library: https://github.com/rust-bitcoin/rust-secp256k1/pull/146 https://github.com/rust-bitcoin/rust-secp256k1/pull/125 https://github.com/rust-bitcoin/rust-secp256k1/pull/120 https://github.com/rust-bitcoin/rust-secp256k1/pull/115 https://github.com/rust-bitcoin/rust-secp256k1/pull/100 https://github.com/rust-bitcoin/rust-secp256k1/pull/87 https://github.com/rust-bitcoin/rust-secp256k1/pull/78 https://github.com/rust-bitcoin/rust-secp256k1/pull/64 https://github.com/rust-bitcoin/rust-secp256k1/pull/51 https://github.com/rust-bitcoin/rust-secp256k1/pull/27

A lot of functions in this repository don't take pointers in their arguments but somehow also don't take ownership of the argument (due to the usage of unsafe blocks). We should replace all these instances with pointers, so that it conforms to the regular Rust ownership rules and is clearer to use. Some examples: commit(), bullet_proof(), verify_bullet_proof()