Add partial blinding test to qa/rpc-tests #510

dgpv · 2019-02-18T16:00:19Z

This PR adds fixes for witness serialization/deseralization in qa/rpc-tests/test_framework/mininode.py
and extends multisig tests in qa/rpc-tests/confidential_transactions.py with partial blinding tests

instagibbs · 2019-02-18T16:41:11Z

great contribution, thanks, will review.

instagibbs

looking good, mostly explanations required to follow flow easier.

instagibbs · 2019-03-06T15:54:17Z

qa/rpc-tests/confidential_transactions.py

 from test_framework.test_framework import BitcoinTestFramework
+from test_framework.mininode import *


please import just what is needed

This will be linted to hell in 0.17, just want to get ahead of it :)

Ok. I was just copying the style used in other tests :-)

instagibbs · 2019-03-06T16:00:26Z

qa/rpc-tests/confidential_transactions.py


-        self.nodes[0].generate(1)
+        blinded_addr = self.nodes[0].getnewaddress()


A bit of a description of the setup would help here, especially with the inter-leaved testing happening

ie I didn't really get the importance of having multiple assets, why you issued an asset first, etc.

A paragraph even describing this entire section(and mark the front/end) would not hurt!

Using multiple assets probably does not matter much, but I thought that using more functionality makes the tests cover more, and there might happen to be some corner cases.. But no specific reason why use multiple assets. I guess this doesn't hurt...

that's a valid answer :)

instagibbs · 2019-03-06T16:02:56Z

qa/rpc-tests/confidential_transactions.py

+
+        stx2 = self.nodes[1].signrawtransaction(blindtx)
+        stx = self.nodes[0].signrawtransaction(stx2['hex'])
+        self.nodes[2].sendrawtransaction(stx['hex'])


let's generate a block and make sure it has confirmations, to rule out mempool weirdness.

instagibbs · 2019-03-06T16:05:03Z

qa/rpc-tests/confidential_transactions.py

+        # Blind the first part of the transaction - we need to supply the
+        # assetcommmitments for all of the inputs, for the surjectionproof
+        # to be valid after we combine the transactions
+        blindtx = self.nodes[1].blindrawtransaction(


wow I'm surprised it does the right thing here, even when the transaction itself only has 1 input! I guess 2 years ago me was thinking ahead...

You note out of band that this is indeed broken behavior.

Could you please add this fix in BlindTransaction if it seems right:

// Needed to construct the proof itself. Generators must match final transaction to be valid std::vector<secp256k1_generator> targetAssetGenerators; surjectionTargets.resize(tx.vin.size()*3); + if (auxiliary_generators) { + // It's fine to over-count here. We truncate before generating proofs. + surjectionTargets.resize(tx.vin.size()*3 + auxiliary_generators->size()) + }

Submitted #514 - made a little refactoring to avoid extra if(auxiliary_generators), and extra resize. Please check.

dgpv · 2019-03-06T19:01:57Z

addressed the comments/suggestions.

instagibbs · 2019-03-06T19:03:35Z

Looks great, could you squash the last 4 commits?

dgpv · 2019-03-06T19:10:26Z

done

instagibbs · 2019-03-06T19:11:41Z

looks like you snuck in one of my commits during your squash...

dgpv · 2019-03-06T19:12:19Z

oops

dgpv · 2019-03-06T19:21:02Z

fixed, also rebased on top of current head of elements-0.14.1 (edit: rebase here just to make commit history more linear :)

instagibbs · 2019-03-06T19:22:08Z

force push diff is only the parts rebased onto.

utACK 6c5e075

instagibbs · 2019-03-13T20:03:42Z

@stevenroose can you ACK and port to 0.17?

instagibbs · 2019-03-14T18:43:59Z

As noted, this test actually uncovered a small OOB issue, asked for a proposed fix in this PR to not lose track of it.

instagibbs · 2019-03-25T14:39:48Z

restarted all builds, as they pretty much all failed seconds into building

6c5e075 add code to test partial blinding functionality to qa/rpc-tests (Dmitry Petukhov) d009244 fix witness serialization in qa/rpc-tests/test_framework (Dmitry Petukhov)

@dgpv

…-tests 4258f9f add code to test partial blinding functionality to qa/rpc-tests (Dmitry Petukhov) Pull request description: forward port of #510 @dgpv Tree-SHA512: 3c6b8afb63234d98f9c917ad17f08f0d426577891178cecff0cfca5787f5285607e64811119a328cbe09eacae1693201deb458ea8db0c97600dfcffd1cbf90be

53ad841 Add explanation about how BIP32 unhardened derivation can be used to simplify whitelisting 71c5fe0 Add comment to explain effect of max_n_iterations in surjectionproof_init 85fd42f add unit test for generator and pedersen commitment roundtripping 2ccf885 rangeproof: fix serialization of pedersen commintments 60c173b rangeproof: verify correctness of pedersen commitments when parsing 32d7526 generator: verify correctness of point when parsing ae14e8a rangeproof: check that points deserialize correctly when verifying rangeproof 44fe43d rangeproof: add fixed vector test case e065d7d Expose generator in shared library fb1ba32 fix spelling in documentation fb75faa Test for rejection of trailing bytes in range proofs 9b2cf17 Test for rejection of trailing bytes in surjection proofs a3a1800 Reject surjection proofs with trailing garbage 0c77ae9 Minor bugfix. Wrong length due to NUL character. b1f31bc Add whitelisting benchmark 52a9f8f add whitelist_impl.h to include for dist a707865 generator: add API tests ec1ef04 generator: remove unnecessary ARG_CHECK from generate() b0e9aa8 Fix generator makefile 526c654 Fix pedersen_blind_generator_blind_sum return value documentation b51886e Add n_keys argument to whitelist_verify 37c57de Fix checks of whitelist serialize/parse arguments 9b8a9d9 whitelist: fix serialize/parse API to take serialized length 7f17515 Fix include/secp256k1_rangeproof.h function argument documentation. 0d81702 rangeproof: add API tests 417bb06 surjectionproof: rename unit test functions to be more consistent with other modules 1e2d5c1 surjectionproof: add API unit tests 7878a29 surjectionproof: tests_impl.h s/assert/CHECK/g e609591 rangeproof: fix memory leak in unit tests 0c17f79 add surjection proof module c174f0c Implement ring-signature based whitelist delegation scheme a2bc660 rangeproof: several API changes 21bfb3c Expose generator in pedersen/rangeproof API f4620de Constant-time generator module d46fc3c rangeproof: expose sidechannel message field in the signing API cf40b1b [RANGEPROOF BREAK] Use quadratic residue for tie break and modularity cleanup 6d28767 Get rid of precomputed H tables (Pieter Wuille) ae1e576 Pedersen commitments, borromean ring signatures, and ZK range proofs. efc61dc Add 64-bit integer utilities e34ceb3 Merge ElementsProject#557: Eliminate scratch memory used when generating contexts b3bf5f9 ecmult_impl: expand comment to explain how effective affine interacts with everything efa783f Store z-ratios in the 'x' coord they'll recover ffd3b34 add `secp256k1_ge_set_all_gej_var` test which deals with many infinite points 84740ac ecmult_impl: save one fe_inv_var 4704527 ecmult_impl: eliminate scratch memory used when generating context 7f7a2ed ecmult_gen_impl: eliminate scratch memory used when generating context 314a61d Merge ElementsProject#553: add static context object which has no capabilities 1086fda Merge ElementsProject#354: [ECDH API change] Support custom hash function 40fde61 prevent attempts to modify `secp256k1_context_no_precomp` ed7c084 add static context object which has no capabilities 1e6f1f5 Merge ElementsProject#529: fix tests.c in the count == 0 case c8fbc3c [ECDH API change] Allow pass arbitrary data to hash function b00be65 [ECDH API change] Support custom hash function 95e99f1 fix tests.c in the count == 0 case 452d8e4 Merge ElementsProject#523: scratch: add stack frame support 6fe5043 scratch: add stack frame support 9bc2e26 Merge ElementsProject#522: parameterize ecmult_const over input size 7c1b91b parameterize ecmult_const over input size dbc3ddd Merge ElementsProject#513: Increase sparsity of pippenger fixed window naf representation fb9271d Merge ElementsProject#510: add a couple missing `const`s to ecmult_pippenger_wnaf cd5f602 Merge ElementsProject#515: Fix typo 09146ae Merge ElementsProject#512: secp256k1_ec_privkey_negate - fix documentation ec0a7b3 Don't touch leading zeros in wnaf_fixed. 9e36d1b Fix bug in wnaf_fixed where the wnaf array is not completely zeroed when given a 0 scalar. 96f68a0 Don't invert scalar in wnaf_fixed when it is even because a caller might intentionally give a scalar with many leading zeros. 9b7c47a Fix typo 6dbb007 Increase sparsity of pippenger fixed window naf representation 1646ace secp256k1_ec_privkey_negate - fix documentation 9b3ff03 add a couple missing `const`s to ecmult_pippenger_wnaf cd329db Merge ElementsProject#460: [build] Update ax_jni_include_dir.m4 macro 7f9c1a1 Merge ElementsProject#498: tests: Avoid calling fclose(...) with an invalid argument f99aa8d Merge ElementsProject#499: tests: Make sure we get the requested number of bytes from /dev/urandom b549d3d Merge ElementsProject#472: [build] Set --enable-jni to no by default instead of auto. d333521 Merge ElementsProject#494: Support OpenSSL versions >= 1.1 for ENABLE_OPENSSL_TESTS 2ef8ea5 Merge ElementsProject#495: Add bench_ecmult to .gitignore 82a96e4 tests: Make sure we get the requested number of bytes from /dev/urandom 5aae5b5 Avoid calling fclose(...) with an invalid argument cb32940 Add bench_ecmult to .gitignore 31abd3a Support OpenSSL versions >= 1.1 for ENABLE_OPENSSL_TESTS c95f6f1 Merge ElementsProject#487: fix tests typo, s/changed/unchanged fb46c83 Merge ElementsProject#463: Reduce usage of hardcoded size constants 02f5001 Merge ElementsProject#490: Disambiguate bench functions and types 1f46d60 Disambiguate bench functions and types f54c6c5 Merge ElementsProject#480: Enable benchmark building by default c77fc08 Merge ElementsProject#486: Add pippenger_wnaf for multi-multiplication d2f9c6b Use more precise pippenger bucket windows 4c950bb Save some additions per window in _pippenger_wnaf a58f543 Add flags for choosing algorithm in ecmult_multi benchmark 36b22c9 Use scratch space dependent batching in ecmult_multi 355a38f Add pippenger_wnaf ecmult_multi bc65aa7 Add bench_ecmult dba5471 Add ecmult_multi tests 8c1c831 Generalize Strauss to support multiple points 548de42 add resizeable scratch space API 0e96cdc fix typo, s/changed/unchanged c7680e5 Reduce usage of hardcoded size constants 6ad5cdb Merge ElementsProject#479: Get rid of reserved _t in type names 7a78f60 Print whether we're building benchmarks 4afec9f Build benchmarks by default d1dc9df Get rid of reserved _t in type names 57752d2 [build] Set --enable-jni to no by default instead of auto. e7daa9b [build] Tweak JNI macro to warn instead of error for JNI not found. 5b22977 [build] Update ax_jni_include_dir.m4 macro to deal with recent versions of macOS git-subtree-dir: src/secp256k1 git-subtree-split: 53ad841cafa3bcb94b65409aec91fd7043533cf7

d584b65 [test]decrese num of inputs on partial blind test (Akio Nakamura) 254b166 [rpc]blindrawtransaction accepts more commitments (Akio Nakamura) Pull request description: In #510 / #550 a requirement that the blinding commitments were equal in amount to the inputs was introduced. This requirement is correct for the final case, but not for partial transactions (e.g. where party 1 blinds their inputs before even handing the transaction over to party 2 to add theirs). In order to hide party 1's input amounts from party 2, blindrawtransaction must allow >= the inputs, since the commitments are shared beforehand (if not, the blinding fails; this may need further investigating). This PR restores the functionality to allow blinding commitments more than or equal to the amount of inputs, which makes the confidential assets demo https://github.com/ElementsProject/confidential-assets-demo functional again. Tree-SHA512: 11c7e1c648aea26be2f4cb47606cd8e9b40d9e0d736a7eeb1989d6fad10f8f06528fd90766cb95e304c9b2c371278b20d7ef61099cb8b6e18b281a260e5de92d

b19c000063 Merge #607: Use size_t shifts when computing a size_t 4d01bc2d9c Merge #606: travis: Remove unused sudo:false e6d01e9347 Use size_t shifts when computing a size_t 7667532bd7 travis: Remove unused sudo:false ee99f12f3d Merge #599: Switch x86_64 asm to use "i" instead of "n" for immediate values. d58bc93f2c Switch x86_64 asm to use "i" instead of "n" for immediate values. 05362ee042 Merge #597: Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build 83483869ac Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build aa15154a48 Merge #568: Fix integer overflow in ecmult_multi_var when n is large 2277af5ff0 Fix integer overflow in ecmult_multi_var when n is large 85d0e1bcce Merge #591: Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. 14196379ec Merge #580: Add trivial ecmult_multi algorithm which does not require a scratch space a697d82da9 Add trivial ecmult_multi to the benchmark tool bade617417 Add trivial ecmult_multi algorithm. It is selected when no scratch space is given and just multiplies and adds the points. 5545e13dea Merge #584: configure: Use CFLAGS_FOR_BUILD when checking native compiler 20c5869df2 Merge #516: improvements to random seed in src/tests.c b76e45d5d6 Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. 870a977644 Merge #562: Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse be40c4d0b5 Fixup for C90 mixed declarations. c71dd2c08f Merge #509: Fix algorithm selection in bench_ecmult 6492bf88cc Merge #518: Summarize build options after running configure 0e9ada1941 Merge #567: Correct order of libs returned on pkg-config --libs --static libsecp2… e96901a4b9 Merge #587: Make randomization of a non-signing context a noop 58df8d03ad Merge #511: Portability fix for the configure scripts generated 2ebdad772a Merge #552: Make constants static: 1c131affd3 Merge #551: secp256k1_fe_sqrt: Verify that the arguments don't alias. ba698f883b Merge #539: Assorted minor corrections 949e85b009 Merge #550: Optimize secp256k1_fe_normalize_weak calls. a34bcaadf1 Actually pass CFLAGS_FOR_BUILD and LDFLAGS_FOR_BUILD to linker 2d5f4cebdc configure: Use CFLAGS_FOR_BUILD when checking native compiler b408c6a8b2 Merge #579: Use __GNUC_PREREQ for detecting __builtin_expect 6198375218 Make randomization of a non-signing context a noop c663397f46 Use __GNUC_PREREQ for detecting __builtin_expect e34ceb333b Merge #557: Eliminate scratch memory used when generating contexts b3bf5f99a3 ecmult_impl: expand comment to explain how effective affine interacts with everything efa783f8f0 Store z-ratios in the 'x' coord they'll recover ffd3b346fe add `secp256k1_ge_set_all_gej_var` test which deals with many infinite points 84740acd2a ecmult_impl: save one fe_inv_var 47045270fa ecmult_impl: eliminate scratch memory used when generating context 7f7a2ed3a8 ecmult_gen_impl: eliminate scratch memory used when generating context 314a61d724 Merge #553: add static context object which has no capabilities 89a20a8945 Correct order of libs returned on pkg-config --libs --static libsecp256k1 call. 1086fda4c1 Merge #354: [ECDH API change] Support custom hash function d3cb1f95eb Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse 40fde611bd prevent attempts to modify `secp256k1_context_no_precomp` ed7c08417a add static context object which has no capabilities 496c5b43b8 Make constants static: static const secp256k1_ge secp256k1_ge_const_g; static const int CURVE_B; bf8b86cc07 secp256k1_fe_sqrt: Verify that the arguments don't alias. 9bd89c836b Optimize secp256k1_fe_normalize_weak calls. Move secp256k1_fe_normalize_weak calls out of ECMULT_TABLE_GET_GE and ECMULT_TABLE_GET_GE_STORAGE and into secp256k1_ge_globalz_set_table_gej instead. 52ab96fedb clean dependendies in field_*_impl.h deff5edd42 Correct math typos in field_*.h 4efb3f8dd1 Add check that restrict pointers don't alias with all parameters. 1e6f1f5ad5 Merge #529: fix tests.c in the count == 0 case c8fbc3c397 [ECDH API change] Allow pass arbitrary data to hash function b00be65056 [ECDH API change] Support custom hash function 95e99f196f fix tests.c in the count == 0 case 452d8e4d2a Merge #523: scratch: add stack frame support 6fe50439ae scratch: add stack frame support 9bc2e26502 Merge #522: parameterize ecmult_const over input size 7c1b91ba4b parameterize ecmult_const over input size dbc3ddd5e2 Merge #513: Increase sparsity of pippenger fixed window naf representation 3965027c81 Summarize build options in configure script 0f0517369c Fix algorithm selection in bench_ecmult fb9271dcf0 Merge #510: add a couple missing `const`s to ecmult_pippenger_wnaf cd5f6028e5 Merge #515: Fix typo 09146ae854 Merge #512: secp256k1_ec_privkey_negate - fix documentation ec0a7b3ae3 Don't touch leading zeros in wnaf_fixed. 9e36d1bfe2 Fix bug in wnaf_fixed where the wnaf array is not completely zeroed when given a 0 scalar. 96f68a0afc Don't invert scalar in wnaf_fixed when it is even because a caller might intentionally give a scalar with many leading zeros. 8b3841c91d fix bug in fread() failure check cddef0c0be tests: add warning message when /dev/urandom fails 9b7c47a21e Fix typo 6dbb007869 Increase sparsity of pippenger fixed window naf representation 1646ace4d5 secp256k1_ec_privkey_negate - fix documentation 270f6c80db Portability fix for the configure scripts generated 9b3ff0309d add a couple missing `const`s to ecmult_pippenger_wnaf cd329dbc3e Merge #460: [build] Update ax_jni_include_dir.m4 macro 7f9c1a1565 Merge #498: tests: Avoid calling fclose(...) with an invalid argument f99aa8d4d3 Merge #499: tests: Make sure we get the requested number of bytes from /dev/urandom b549d3d5f7 Merge #472: [build] Set --enable-jni to no by default instead of auto. d333521516 Merge #494: Support OpenSSL versions >= 1.1 for ENABLE_OPENSSL_TESTS 2ef8ea5d21 Merge #495: Add bench_ecmult to .gitignore 82a96e4587 tests: Make sure we get the requested number of bytes from /dev/urandom 5aae5b5bb2 Avoid calling fclose(...) with an invalid argument cb32940df3 Add bench_ecmult to .gitignore 31abd3ab8d Support OpenSSL versions >= 1.1 for ENABLE_OPENSSL_TESTS c95f6f1360 Merge #487: fix tests typo, s/changed/unchanged fb46c83881 Merge #463: Reduce usage of hardcoded size constants 02f5001dfc Merge #490: Disambiguate bench functions and types 1f46d6089e Disambiguate bench functions and types f54c6c5083 Merge #480: Enable benchmark building by default c77fc08597 Merge #486: Add pippenger_wnaf for multi-multiplication d2f9c6b5dc Use more precise pippenger bucket windows 4c950bbeaf Save some additions per window in _pippenger_wnaf a58f543f5a Add flags for choosing algorithm in ecmult_multi benchmark 36b22c9337 Use scratch space dependent batching in ecmult_multi 355a38f113 Add pippenger_wnaf ecmult_multi bc65aa794e Add bench_ecmult dba5471b69 Add ecmult_multi tests 8c1c831bdb Generalize Strauss to support multiple points 548de42ecf add resizeable scratch space API 0e96cdc6b6 fix typo, s/changed/unchanged c7680e570f Reduce usage of hardcoded size constants 6ad5cdb42a Merge #479: Get rid of reserved _t in type names 7a78f60598 Print whether we're building benchmarks 4afec9f1ae Build benchmarks by default d1dc9dfc0a Get rid of reserved _t in type names 57752d28b3 [build] Set --enable-jni to no by default instead of auto. e7daa9b3c2 [build] Tweak JNI macro to warn instead of error for JNI not found. 5b22977922 [build] Update ax_jni_include_dir.m4 macro to deal with recent versions of macOS git-subtree-dir: src/secp256k1 git-subtree-split: b19c000063be11018b4d1a6b0a85871ab9d0bdcf

dgpv force-pushed the partial_blinding_test branch from 4040586 to 0898160 Compare February 19, 2019 09:33

stevenroose added the 0.14.1 label Mar 6, 2019

instagibbs reviewed Mar 6, 2019

View reviewed changes

dgpv force-pushed the partial_blinding_test branch from f902ffc to 6240f96 Compare March 6, 2019 19:09

dgpv force-pushed the partial_blinding_test branch from 6240f96 to 6c5e075 Compare March 6, 2019 19:19

dgpv added 2 commits March 7, 2019 00:19

fix witness serialization in qa/rpc-tests/test_framework

d009244

add code to test partial blinding functionality to qa/rpc-tests

6c5e075

instagibbs added the needs port Needs backport to a different branch label Mar 14, 2019

instagibbs merged commit 6c5e075 into ElementsProject:elements-0.14.1 Mar 29, 2019

instagibbs mentioned this pull request Mar 29, 2019

add code to test partial blinding functionality to qa/rpc-tests #550

Merged

instagibbs removed the needs port Needs backport to a different branch label Jun 5, 2019

This was referenced Jul 5, 2019

[rpc]blindrawtransaction accepts more commitments AkioNak/elements#6

Closed

[rpc]blindrawtransaction accepts more commitments #674

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add partial blinding test to qa/rpc-tests #510

Add partial blinding test to qa/rpc-tests #510

dgpv commented Feb 18, 2019

instagibbs commented Feb 18, 2019

instagibbs left a comment

instagibbs Mar 6, 2019

instagibbs Mar 6, 2019

dgpv Mar 6, 2019

instagibbs Mar 6, 2019

instagibbs Mar 6, 2019

dgpv Mar 6, 2019 •

edited

Loading

instagibbs Mar 6, 2019

instagibbs Mar 6, 2019

instagibbs Mar 6, 2019

instagibbs Mar 14, 2019

dgpv Mar 15, 2019

dgpv commented Mar 6, 2019

instagibbs commented Mar 6, 2019

dgpv commented Mar 6, 2019

instagibbs commented Mar 6, 2019

dgpv commented Mar 6, 2019

dgpv commented Mar 6, 2019 •

edited

Loading

instagibbs commented Mar 6, 2019

instagibbs commented Mar 13, 2019

instagibbs commented Mar 14, 2019

instagibbs commented Mar 25, 2019

		from test_framework.test_framework import BitcoinTestFramework
		from test_framework.mininode import *


		self.nodes[0].generate(1)
		blinded_addr = self.nodes[0].getnewaddress()

Add partial blinding test to qa/rpc-tests #510

Add partial blinding test to qa/rpc-tests #510

Conversation

dgpv commented Feb 18, 2019

instagibbs commented Feb 18, 2019

instagibbs left a 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

dgpv Mar 6, 2019 • edited Loading

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

dgpv commented Mar 6, 2019

instagibbs commented Mar 6, 2019

dgpv commented Mar 6, 2019

instagibbs commented Mar 6, 2019

dgpv commented Mar 6, 2019

dgpv commented Mar 6, 2019 • edited Loading

instagibbs commented Mar 6, 2019

instagibbs commented Mar 13, 2019

instagibbs commented Mar 14, 2019

instagibbs commented Mar 25, 2019

dgpv Mar 6, 2019 •

edited

Loading

dgpv commented Mar 6, 2019 •

edited

Loading