PSBT for Confidential Assets - #600
Conversation
|
could you rebase? |
instagibbs
left a comment
There was a problem hiding this comment.
concept ACK, first look seems good.
| } | ||
|
|
||
| CAmount CWalletTx::GetOutputValueOut(unsigned int output_index) const { | ||
| void CWalletTx::GetNonIssuanceBlindingData(const unsigned int output_index, CPubKey* blinding_pubkey_out, CAmount* value_out, uint256* value_factor_out, CAsset* asset_out, uint256* asset_factor_out) const { |
There was a problem hiding this comment.
this refactoring could be its own commit
| // One change script per output asset. | ||
| size_t index = 0; | ||
| for (const std::pair<CAsset, CAmount>& value : mapValue) { | ||
| for (const std::pair<const CAsset, CAmount>& value : mapValue) { |
There was a problem hiding this comment.
compiler complaint squashing could be its own commit
There was a problem hiding this comment.
I think this must have gone away in the rebase
|
|
||
| CMutableTransaction tx_tmp = tx; // We don't want to mutate the transaction in the PSBT yet, just extract blinding data | ||
|
|
||
| // TODO(gwillen): Make this do something better than fail silently if there are any issuances, reissuances, pegins, etc. |
There was a problem hiding this comment.
PSBTs can't really encode these anyways yet, I think we're safe for now.
Supporting the creation of issuance/peg-in inputs in a PSBT format is the harder part.
There was a problem hiding this comment.
Well, the PSBT can encode anything the CTransaction can encode, so I can make a PSBT of an issuance -- I just can't blind it. (Perhaps I also can't sign it -- I didn't try a fully unblinded one.)
| o.range_proof = tx_tmp.witness.vtxoutwit[i].vchRangeproof; | ||
| o.surjection_proof = tx_tmp.witness.vtxoutwit[i].vchSurjectionproof; | ||
|
|
||
| o.blinding_pubkey = CPubKey(); // Once we're done blinding, remove the pubkeys to signal that it's complete |
There was a problem hiding this comment.
I guess this makes sense. We remove partial sigs once a full sig is complete right?
There was a problem hiding this comment.
Yes, and I suspect that there's more pruning I could do with the goal of keeping the PSBTs small (which I expect is why we do that), but this specific removal is actually important as a signal to signpsbt that it's okay to sign. (Since the hack with the nonce field is gone in this flow.)
There was a problem hiding this comment.
why wouldn't the presence of the other fields show that?
There was a problem hiding this comment.
The only way to tell that an output is intended to be blinded is that it has a pubkey. Absence of blinders could mean "not going to be blinded" or "hasn't been blinded yet".
There was a problem hiding this comment.
So I guess you could indeed check for "has a pubkey and blinders" or "has no pubkey", but just checking for not having a pubkey seems easier (and is similar to what the code was doing before). But I can go either way as you like.
|
|
||
| CMutableTransaction& tx = *psbtx.tx; | ||
|
|
||
| // TODO(gwillen): Replace all this with the 'bonus output' scheme to use an OP_RETURN to balance blinders, with a rangeproof exponent of -1 (public). |
There was a problem hiding this comment.
with known blinding pubkey, right?
There was a problem hiding this comment.
I believe the scheme we talked about -- which I don't expect to fully understand before implementing it -- was to just make up a random pubkey and then throw it away after, because nobody ever needs to be able to rewind these outputs.
There was a problem hiding this comment.
that works too. Could just be the generator point G or something stupid as well.
|
Rebased, still need to split things up a bit. Also looks like I killed CI with the rebase, will investigate. EDIT: nevermind, of course CI fails -- the tests don't pass with all the extra printfs in there. That will go away on its own when I remove those. |
d7a44e5 to
2921cb4
Compare
|
apparently somehow I lost the compiler warning squashing in the rebase, but not because it was fixed upstream? So I added it back as its own commit. Also fixed tests by removing all the stray printfs, none of them appeared worthy of upgrading to LogPrintf. I think this should be ready for a real review. |
|
fixed travis complaints (coming from lints that don't run on my system because they have some python dependency I can't get to install.) |
|
Linters are hard to replicate locally for me as well. Thanks.
…On Thu, May 2, 2019, 9:42 PM gwillen ***@***.***> wrote:
fixed travis complaints (coming from lints that don't run on my system
because they have some python dependency I can't get to install.)
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#600 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABMAFU2D27IOK5VJPQOZAB3PTOJ7PANCNFSM4HJSS4DA>
.
|
|
ok looks like everything but bitcoin compatibility test for psbt is working: see: |
|
Hm, interesting. It only fails on a single platform, which makes me worry it's nondeterministic. Also, I didn't realize those compatibility tests existed, so I wasn't running them -- I will try and see what it does locally. ALSO, I think I'm still waiting for you to help me figure out where to condition on g_con_elementsmode. So assuming the compatibility tests run without that, that may fix it? |
|
@instagibbs formal reminder that I am waiting for your review on this (and guidance on where to put g_con_elementsmode checks) |
|
@stevenroose promised a review Soon(TM), I think after his next catchup PR. |
|
@stevenroose @instagibbs piiiing? :-) I would really like to get this cleaned up and in. I definitely still need help sorting out what g_con_elementsmode is for, and where it should fit in (to make the bitcoin backwards-compat tests pass, which still fail at present.) |
|
please add tests for |
44db4d8 Merge pull request ElementsProject#57 from apoelstra/2019-04-surjectionproof-stack f7e4d08 surjection proof: Reject proofs with too many used inputs in reduced mode 112edb2 allow reducing surjection proof size (to lower generation stack usage) d512d78 surjectionproof: introduce `SECP256K1_SURJECTIONPROOF_MAX_USED_INPUTS` constant and set it to 16 a118acc surjectionproof: reduce stack usage e7f4ff4 Merge pull request ElementsProject#70 from apoelstra/2019-06-surjection-count f94d46e Merge pull request ElementsProject#71 from real-or-random/fix-trailing-test 49a1e01 surjectionproof: fix malleability in surjection proof parsing 55311b0 Fix read of wrong buffer (and OOB) in surjectionproof tests 290a27b surjectionproof: add test vectors for "set padding bits" 7bc3daa surjectionproof: add fixed test vectors 6f3b0c0 Improve comments for surctionproof init+alloc/destroy funcs 250ebb3 work in progress: add _allocate_initialized/destroy funcs 4a77633 Improve explanation of key cancellation attack in whitelist.md 898c9f0 Clarify how to derive alternative generator H 15d9278 Add bench_generator and bench_rangeproof to .gitignore 86240b2 Clean up ./configure help strings (zkp extensions) 865b761 Fix a small typo in the generator parameter name cd5ba5c generator: remove `CHECK` abort calls exposed by public API ff16651 musig: add user documentation 0ad6b60 Add 3-of-3 MuSig example b61a1a9 Add MuSig module which allows creating n-of-n multisignatures and adaptor signatures. 5d5374f Add schnorrsig module which implements BIP-schnorr [0] compatible signing, verification and batch verification. a8ae6ba add chacha20 function 9a8a71e use proper types for rangeproof min/max 14769b9 rangeproof: reduce iteration count in unit tests 0593861 Enable more builds with rest of experimental flags e9fea74 Add explanation about how BIP32 unhardened derivation can be used to simplify whitelisting dec1b9c Add comment to explain effect of max_n_iterations in surjectionproof_init ea62bfe add unit test for generator and pedersen commitment roundtripping e32924f rangeproof: fix serialization of pedersen commintments 972d056 rangeproof: verify correctness of pedersen commitments when parsing 2cc4c6f generator: verify correctness of point when parsing 65ffea4 rangeproof: check that points deserialize correctly when verifying rangeproof cb786d6 rangeproof: add fixed vector test case b387ba0 Expose generator in shared library 8da4328 fix spelling in documentation 6f14fe4 Test for rejection of trailing bytes in range proofs ab4fbc1 Test for rejection of trailing bytes in surjection proofs c908c97 Reject surjection proofs with trailing garbage f723bf5 Minor bugfix. Wrong length due to NUL character. 6872069 Add whitelisting benchmark 6ceccb7 add whitelist_impl.h to include for dist a3ad4a8 generator: add API tests e93e886 generator: remove unnecessary ARG_CHECK from generate() f1d6e4b Fix generator makefile 68be611 Fix pedersen_blind_generator_blind_sum return value documentation 51fc58a Add n_keys argument to whitelist_verify 36b100c Fix checks of whitelist serialize/parse arguments c8f54e1 whitelist: fix serialize/parse API to take serialized length 56fca50 Fix include/secp256k1_rangeproof.h function argument documentation. 4617f04 rangeproof: add API tests cd4e438 surjectionproof: rename unit test functions to be more consistent with other modules 2cc7f1e surjectionproof: add API unit tests c4097f7 surjectionproof: tests_impl.h s/assert/CHECK/g 5ee6bf3 rangeproof: fix memory leak in unit tests 94e81a2 add surjection proof module a66ea35 Implement ring-signature based whitelist delegation scheme 2bb5133 rangeproof: several API changes 9b00b61 Expose generator in pedersen/rangeproof API 54fa263 Constant-time generator module 023aa86 rangeproof: expose sidechannel message field in the signing API 89e7451 [RANGEPROOF BREAK] Use quadratic residue for tie break and modularity cleanup f126331 Pedersen commitments, borromean ring signatures, and ZK range proofs. e1fb4af Add 64-bit integer utilities e541a90 Merge ElementsProject#629: Avoid calling _is_zero when _set_b32 fails. f34b0c3 Merge ElementsProject#630: Note intention of timing sidechannel freeness. 8d1563b Note intention of timing sidechannel freeness. 1669bb2 Merge ElementsProject#628: Fix ability to compile tests without -DVERIFY. ecc94ab Merge ElementsProject#627: Guard memcmp in tests against mixed size inputs. 544435f Merge ElementsProject#578: Avoid implementation-defined and undefined behavior when dealing with sizes 143dc6e Merge ElementsProject#595: Allow to use external default callbacks e49f799 Add missing #(un)defines to base-config.h 77defd2 Add secp256k1_ prefix to default callback functions 908bdce Include stdio.h and stdlib.h explicitly in secp256k1.c 5db782e Allow usage of external default callbacks 6095a86 Replace CHECKs for no_precomp ctx by ARG_CHECKs without a return cd473e0 Avoid calling secp256k1_*_is_zero when secp256k1_*_set_b32 fails. 6c36de7 Merge ElementsProject#600: scratch space: use single allocation 98836b1 scratch: replace frames with "checkpoint" system 7623cf2 scratch: save a couple bytes of unnecessarily-allocated memory a7a164f scratch: rename `max_size` to `size`, document that extra will actually be allocated 5a4bc0b scratch: unify allocations c2b028a scratch space: thread `error_callback` into all scratch space functions 0be1a4a scratch: add magic bytes to beginning of structure 92a48a7 scratch space: use single allocation 40839e2 Merge ElementsProject#592: Use trivial algorithm in ecmult_multi if scratch space is small dcf3920 Fix ability to compile tests without -DVERIFY. a484e00 Merge ElementsProject#566: Enable context creation in preallocated memory 0522caa Explain caller's obligations for preallocated memory 238305f Move _preallocated functions to separate header 695feb6 Export _preallocated functions 814cc78 Add tests for contexts in preallocated memory ba12dd0 Check arguments of _preallocated functions 5feadde Support cloning a context into preallocated memory c4fd5da Switch to a single malloc call ef020de Add size constants for preallocated memory 1bf7c05 Prepare for manual memory management in preallocated memory 248bffb Guard memcmp in tests against mixed size inputs. 36698dc Merge ElementsProject#596: Make WINDOW_G configurable a61a93f Clean up ./configure help strings 2842dc5 Make WINDOW_G configurable 1a02d6c Merge ElementsProject#626: Revert "Merge ElementsProject#620: Install headers automatically" 662918c Revert "Merge ElementsProject#620: Install headers automatically" 14c7dbd Simplify control flow in DER parsing ec8f20b Avoid out-of-bound pointers and integer overflows in size comparisons 01ee1b3 Parse DER-enconded length into a size_t instead of an int 912680e Merge ElementsProject#561: Respect LDFLAGS and #undef STATIC_PRECOMPUTATION if using basic config 91fae3a Merge ElementsProject#620: Install headers automatically 5df77a0 Merge ElementsProject#533: Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...) 975e51e Merge ElementsProject#617: Pass scalar by reference in secp256k1_wnaf_const() 735fbde Merge ElementsProject#619: Clear a copied secret key after negation 16e8615 Install headers automatically 069870d Clear a copied secret key after negation 8979ec0 Pass scalar by reference in secp256k1_wnaf_const() 84a8085 Merge ElementsProject#612: Allow field_10x26_arm.s to compile for ARMv7 architecture d4d270a Allow field_10x26_arm.s to compile for ARMv7 architecture b19c000 Merge ElementsProject#607: Use size_t shifts when computing a size_t 4d01bc2 Merge ElementsProject#606: travis: Remove unused sudo:false e6d01e9 Use size_t shifts when computing a size_t 7667532 travis: Remove unused sudo:false 248f046 Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...) 9ab96f7 Use trivial algorithm in ecmult_multi if scratch space is small ee99f12 Merge ElementsProject#599: Switch x86_64 asm to use "i" instead of "n" for immediate values. d58bc93 Switch x86_64 asm to use "i" instead of "n" for immediate values. 05362ee Merge ElementsProject#597: Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build 8348386 Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build aa15154 Merge ElementsProject#568: Fix integer overflow in ecmult_multi_var when n is large 2277af5 Fix integer overflow in ecmult_multi_var when n is large dbed75d Undefine `STATIC_PRECOMPUTATION` if using the basic config 310111e Keep LDFLAGS if `--coverage` 85d0e1b Merge ElementsProject#591: Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. 1419637 Merge ElementsProject#580: Add trivial ecmult_multi algorithm which does not require a scratch space a697d82 Add trivial ecmult_multi to the benchmark tool bade617 Add trivial ecmult_multi algorithm. It is selected when no scratch space is given and just multiplies and adds the points. 5545e13 Merge ElementsProject#584: configure: Use CFLAGS_FOR_BUILD when checking native compiler 20c5869 Merge ElementsProject#516: improvements to random seed in src/tests.c b76e45d Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. 870a977 Merge ElementsProject#562: Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse be40c4d Fixup for C90 mixed declarations. c71dd2c Merge ElementsProject#509: Fix algorithm selection in bench_ecmult 6492bf8 Merge ElementsProject#518: Summarize build options after running configure 0e9ada1 Merge ElementsProject#567: Correct order of libs returned on pkg-config --libs --static libsecp2… e96901a Merge ElementsProject#587: Make randomization of a non-signing context a noop 58df8d0 Merge ElementsProject#511: Portability fix for the configure scripts generated 2ebdad7 Merge ElementsProject#552: Make constants static: 1c131af Merge ElementsProject#551: secp256k1_fe_sqrt: Verify that the arguments don't alias. ba698f8 Merge ElementsProject#539: Assorted minor corrections 949e85b Merge ElementsProject#550: Optimize secp256k1_fe_normalize_weak calls. a34bcaa Actually pass CFLAGS_FOR_BUILD and LDFLAGS_FOR_BUILD to linker 2d5f4ce configure: Use CFLAGS_FOR_BUILD when checking native compiler b408c6a Merge ElementsProject#579: Use __GNUC_PREREQ for detecting __builtin_expect 6198375 Make randomization of a non-signing context a noop c663397 Use __GNUC_PREREQ for detecting __builtin_expect 3cb057f Fix possible integer overflow in DER parsing 89a20a8 Correct order of libs returned on pkg-config --libs --static libsecp256k1 call. d3cb1f9 Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse 496c5b4 Make constants static: static const secp256k1_ge secp256k1_ge_const_g; static const int CURVE_B; bf8b86c secp256k1_fe_sqrt: Verify that the arguments don't alias. 9bd89c8 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. 52ab96f clean dependendies in field_*_impl.h deff5ed Correct math typos in field_*.h 4efb3f8 Add check that restrict pointers don't alias with all parameters. 3965027 Summarize build options in configure script 0f05173 Fix algorithm selection in bench_ecmult 8b3841c fix bug in fread() failure check cddef0c tests: add warning message when /dev/urandom fails 270f6c8 Portability fix for the configure scripts generated REVERT: 43dd1f4 Merge pull request ElementsProject#40 from instagibbs/size_t REVERT: 6532fa0 Merge pull request ElementsProject#39 from instagibbs/more_builds REVERT: 2b2429d rangeproof: reduce iteration count in unit tests REVERT: 12b0e5d Enable more builds with rest of experimental flags REVERT: 8c444ee use proper types for rangeproof min/max REVERT: 53ad841 Add explanation about how BIP32 unhardened derivation can be used to simplify whitelisting REVERT: 71c5fe0 Add comment to explain effect of max_n_iterations in surjectionproof_init REVERT: 85fd42f add unit test for generator and pedersen commitment roundtripping REVERT: 2ccf885 rangeproof: fix serialization of pedersen commintments REVERT: 60c173b rangeproof: verify correctness of pedersen commitments when parsing REVERT: 32d7526 generator: verify correctness of point when parsing REVERT: ae14e8a rangeproof: check that points deserialize correctly when verifying rangeproof REVERT: 44fe43d rangeproof: add fixed vector test case REVERT: e065d7d Expose generator in shared library REVERT: fb1ba32 fix spelling in documentation REVERT: fb75faa Test for rejection of trailing bytes in range proofs REVERT: 9b2cf17 Test for rejection of trailing bytes in surjection proofs REVERT: a3a1800 Reject surjection proofs with trailing garbage REVERT: 0c77ae9 Minor bugfix. Wrong length due to NUL character. REVERT: b1f31bc Add whitelisting benchmark REVERT: 52a9f8f add whitelist_impl.h to include for dist REVERT: a707865 generator: add API tests REVERT: ec1ef04 generator: remove unnecessary ARG_CHECK from generate() REVERT: b0e9aa8 Fix generator makefile REVERT: 526c654 Fix pedersen_blind_generator_blind_sum return value documentation REVERT: b51886e Add n_keys argument to whitelist_verify REVERT: 37c57de Fix checks of whitelist serialize/parse arguments REVERT: 9b8a9d9 whitelist: fix serialize/parse API to take serialized length REVERT: 7f17515 Fix include/secp256k1_rangeproof.h function argument documentation. REVERT: 0d81702 rangeproof: add API tests REVERT: 417bb06 surjectionproof: rename unit test functions to be more consistent with other modules REVERT: 1e2d5c1 surjectionproof: add API unit tests REVERT: 7878a29 surjectionproof: tests_impl.h s/assert/CHECK/g REVERT: e609591 rangeproof: fix memory leak in unit tests REVERT: 0c17f79 add surjection proof module REVERT: c174f0c Implement ring-signature based whitelist delegation scheme REVERT: a2bc660 rangeproof: several API changes REVERT: 21bfb3c Expose generator in pedersen/rangeproof API REVERT: f4620de Constant-time generator module REVERT: d46fc3c rangeproof: expose sidechannel message field in the signing API REVERT: cf40b1b [RANGEPROOF BREAK] Use quadratic residue for tie break and modularity cleanup REVERT: 6d28767 Get rid of precomputed H tables (Pieter Wuille) REVERT: ae1e576 Pedersen commitments, borromean ring signatures, and ZK range proofs. REVERT: efc61dc Add 64-bit integer utilities git-subtree-dir: src/secp256k1 git-subtree-split: 44db4d801fff3cd94105136cb443d603683baad2
44db4d801 Merge pull request ElementsProject#57 from apoelstra/2019-04-surjectionproof-stack f7e4d08da surjection proof: Reject proofs with too many used inputs in reduced mode 112edb2c6 allow reducing surjection proof size (to lower generation stack usage) d512d7865 surjectionproof: introduce `SECP256K1_SURJECTIONPROOF_MAX_USED_INPUTS` constant and set it to 16 a118acc02 surjectionproof: reduce stack usage e7f4ff4c6 Merge pull request ElementsProject#70 from apoelstra/2019-06-surjection-count f94d46e65 Merge pull request ElementsProject#71 from real-or-random/fix-trailing-test 49a1e0173 surjectionproof: fix malleability in surjection proof parsing 55311b0e0 Fix read of wrong buffer (and OOB) in surjectionproof tests 290a27bb7 surjectionproof: add test vectors for "set padding bits" 7bc3daa48 surjectionproof: add fixed test vectors 6f3b0c05c Improve comments for surctionproof init+alloc/destroy funcs 250ebb364 work in progress: add _allocate_initialized/destroy funcs 4a7763361 Improve explanation of key cancellation attack in whitelist.md 898c9f05b Clarify how to derive alternative generator H 15d92782d Add bench_generator and bench_rangeproof to .gitignore 86240b207 Clean up ./configure help strings (zkp extensions) 865b76186 Fix a small typo in the generator parameter name cd5ba5c3b generator: remove `CHECK` abort calls exposed by public API ff1665127 musig: add user documentation 0ad6b6036 Add 3-of-3 MuSig example b61a1a9d9 Add MuSig module which allows creating n-of-n multisignatures and adaptor signatures. 5d5374f92 Add schnorrsig module which implements BIP-schnorr [0] compatible signing, verification and batch verification. a8ae6baff add chacha20 function 9a8a71e8b use proper types for rangeproof min/max 14769b964 rangeproof: reduce iteration count in unit tests 0593861cc Enable more builds with rest of experimental flags e9fea7427 Add explanation about how BIP32 unhardened derivation can be used to simplify whitelisting dec1b9ce2 Add comment to explain effect of max_n_iterations in surjectionproof_init ea62bfe22 add unit test for generator and pedersen commitment roundtripping e32924f0e rangeproof: fix serialization of pedersen commintments 972d056fa rangeproof: verify correctness of pedersen commitments when parsing 2cc4c6fef generator: verify correctness of point when parsing 65ffea43d rangeproof: check that points deserialize correctly when verifying rangeproof cb786d6d1 rangeproof: add fixed vector test case b387ba038 Expose generator in shared library 8da432855 fix spelling in documentation 6f14fe40d Test for rejection of trailing bytes in range proofs ab4fbc1be Test for rejection of trailing bytes in surjection proofs c908c97d6 Reject surjection proofs with trailing garbage f723bf5b3 Minor bugfix. Wrong length due to NUL character. 6872069de Add whitelisting benchmark 6ceccb75b add whitelist_impl.h to include for dist a3ad4a866 generator: add API tests e93e886cb generator: remove unnecessary ARG_CHECK from generate() f1d6e4b83 Fix generator makefile 68be61131 Fix pedersen_blind_generator_blind_sum return value documentation 51fc58ae6 Add n_keys argument to whitelist_verify 36b100c77 Fix checks of whitelist serialize/parse arguments c8f54e12e whitelist: fix serialize/parse API to take serialized length 56fca5077 Fix include/secp256k1_rangeproof.h function argument documentation. 4617f0478 rangeproof: add API tests cd4e438a3 surjectionproof: rename unit test functions to be more consistent with other modules 2cc7f1e04 surjectionproof: add API unit tests c4097f758 surjectionproof: tests_impl.h s/assert/CHECK/g 5ee6bf341 rangeproof: fix memory leak in unit tests 94e81a250 add surjection proof module a66ea3522 Implement ring-signature based whitelist delegation scheme 2bb513361 rangeproof: several API changes 9b00b61d9 Expose generator in pedersen/rangeproof API 54fa2639e Constant-time generator module 023aa86ac rangeproof: expose sidechannel message field in the signing API 89e7451d4 [RANGEPROOF BREAK] Use quadratic residue for tie break and modularity cleanup f126331bc Pedersen commitments, borromean ring signatures, and ZK range proofs. e1fb4af90 Add 64-bit integer utilities e541a90ef Merge ElementsProject#629: Avoid calling _is_zero when _set_b32 fails. f34b0c3f3 Merge ElementsProject#630: Note intention of timing sidechannel freeness. 8d1563b0f Note intention of timing sidechannel freeness. 1669bb286 Merge ElementsProject#628: Fix ability to compile tests without -DVERIFY. ecc94abcc Merge ElementsProject#627: Guard memcmp in tests against mixed size inputs. 544435fc9 Merge ElementsProject#578: Avoid implementation-defined and undefined behavior when dealing with sizes 143dc6e9e Merge ElementsProject#595: Allow to use external default callbacks e49f7991c Add missing #(un)defines to base-config.h 77defd2c3 Add secp256k1_ prefix to default callback functions 908bdce64 Include stdio.h and stdlib.h explicitly in secp256k1.c 5db782e65 Allow usage of external default callbacks 6095a863f Replace CHECKs for no_precomp ctx by ARG_CHECKs without a return cd473e02c Avoid calling secp256k1_*_is_zero when secp256k1_*_set_b32 fails. 6c36de7a3 Merge ElementsProject#600: scratch space: use single allocation 98836b11f scratch: replace frames with "checkpoint" system 7623cf2b9 scratch: save a couple bytes of unnecessarily-allocated memory a7a164f2c scratch: rename `max_size` to `size`, document that extra will actually be allocated 5a4bc0bb9 scratch: unify allocations c2b028a28 scratch space: thread `error_callback` into all scratch space functions 0be1a4ae6 scratch: add magic bytes to beginning of structure 92a48a764 scratch space: use single allocation 40839e21b Merge ElementsProject#592: Use trivial algorithm in ecmult_multi if scratch space is small dcf392027 Fix ability to compile tests without -DVERIFY. a484e0008 Merge ElementsProject#566: Enable context creation in preallocated memory 0522caac8 Explain caller's obligations for preallocated memory 238305fdb Move _preallocated functions to separate header 695feb6fb Export _preallocated functions 814cc78d7 Add tests for contexts in preallocated memory ba12dd08d Check arguments of _preallocated functions 5feadde46 Support cloning a context into preallocated memory c4fd5dab4 Switch to a single malloc call ef020de16 Add size constants for preallocated memory 1bf7c056b Prepare for manual memory management in preallocated memory 248bffb05 Guard memcmp in tests against mixed size inputs. 36698dcfe Merge ElementsProject#596: Make WINDOW_G configurable a61a93ff5 Clean up ./configure help strings 2842dc523 Make WINDOW_G configurable 1a02d6ce5 Merge ElementsProject#626: Revert "Merge ElementsProject#620: Install headers automatically" 662918cb2 Revert "Merge ElementsProject#620: Install headers automatically" 14c7dbd44 Simplify control flow in DER parsing ec8f20bab Avoid out-of-bound pointers and integer overflows in size comparisons 01ee1b3b3 Parse DER-enconded length into a size_t instead of an int 912680ed8 Merge ElementsProject#561: Respect LDFLAGS and #undef STATIC_PRECOMPUTATION if using basic config 91fae3ace Merge ElementsProject#620: Install headers automatically 5df77a0ed Merge ElementsProject#533: Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...) 975e51e0d Merge ElementsProject#617: Pass scalar by reference in secp256k1_wnaf_const() 735fbde04 Merge ElementsProject#619: Clear a copied secret key after negation 16e86150d Install headers automatically 069870d92 Clear a copied secret key after negation 8979ec0d9 Pass scalar by reference in secp256k1_wnaf_const() 84a808598 Merge ElementsProject#612: Allow field_10x26_arm.s to compile for ARMv7 architecture d4d270a59 Allow field_10x26_arm.s to compile for ARMv7 architecture b19c00006 Merge ElementsProject#607: Use size_t shifts when computing a size_t 4d01bc2d9 Merge ElementsProject#606: travis: Remove unused sudo:false e6d01e934 Use size_t shifts when computing a size_t 7667532bd travis: Remove unused sudo:false 248f04661 Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...) 9ab96f7b1 Use trivial algorithm in ecmult_multi if scratch space is small ee99f12f3 Merge ElementsProject#599: Switch x86_64 asm to use "i" instead of "n" for immediate values. d58bc93f2 Switch x86_64 asm to use "i" instead of "n" for immediate values. 05362ee04 Merge ElementsProject#597: Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build 83483869a Add $(COMMON_LIB) to exhaustive tests to fix ARM asm build aa15154a4 Merge ElementsProject#568: Fix integer overflow in ecmult_multi_var when n is large 2277af5ff Fix integer overflow in ecmult_multi_var when n is large dbed75d96 Undefine `STATIC_PRECOMPUTATION` if using the basic config 310111e09 Keep LDFLAGS if `--coverage` 85d0e1bcc Merge ElementsProject#591: Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. 14196379e Merge ElementsProject#580: Add trivial ecmult_multi algorithm which does not require a scratch space a697d82da Add trivial ecmult_multi to the benchmark tool bade61741 Add trivial ecmult_multi algorithm. It is selected when no scratch space is given and just multiplies and adds the points. 5545e13de Merge ElementsProject#584: configure: Use CFLAGS_FOR_BUILD when checking native compiler 20c5869df Merge ElementsProject#516: improvements to random seed in src/tests.c b76e45d5d Make bench_internal obey secp256k1_fe_sqrt's contract wrt aliasing. 870a97764 Merge ElementsProject#562: Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse be40c4d0b Fixup for C90 mixed declarations. c71dd2c08 Merge ElementsProject#509: Fix algorithm selection in bench_ecmult 6492bf88c Merge ElementsProject#518: Summarize build options after running configure 0e9ada194 Merge ElementsProject#567: Correct order of libs returned on pkg-config --libs --static libsecp2… e96901a4b Merge ElementsProject#587: Make randomization of a non-signing context a noop 58df8d03a Merge ElementsProject#511: Portability fix for the configure scripts generated 2ebdad772 Merge ElementsProject#552: Make constants static: 1c131affd Merge ElementsProject#551: secp256k1_fe_sqrt: Verify that the arguments don't alias. ba698f883 Merge ElementsProject#539: Assorted minor corrections 949e85b00 Merge ElementsProject#550: Optimize secp256k1_fe_normalize_weak calls. a34bcaadf Actually pass CFLAGS_FOR_BUILD and LDFLAGS_FOR_BUILD to linker 2d5f4cebd configure: Use CFLAGS_FOR_BUILD when checking native compiler b408c6a8b Merge ElementsProject#579: Use __GNUC_PREREQ for detecting __builtin_expect 619837521 Make randomization of a non-signing context a noop c663397f4 Use __GNUC_PREREQ for detecting __builtin_expect 3cb057f84 Fix possible integer overflow in DER parsing 89a20a894 Correct order of libs returned on pkg-config --libs --static libsecp256k1 call. d3cb1f95e Make use of TAG_PUBKEY constants in secp256k1_eckey_pubkey_parse 496c5b43b Make constants static: static const secp256k1_ge secp256k1_ge_const_g; static const int CURVE_B; bf8b86cc0 secp256k1_fe_sqrt: Verify that the arguments don't alias. 9bd89c836 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. 52ab96fed clean dependendies in field_*_impl.h deff5edd4 Correct math typos in field_*.h 4efb3f8dd Add check that restrict pointers don't alias with all parameters. 3965027c8 Summarize build options in configure script 0f0517369 Fix algorithm selection in bench_ecmult 8b3841c91 fix bug in fread() failure check cddef0c0b tests: add warning message when /dev/urandom fails 270f6c80d Portability fix for the configure scripts generated REVERT: 43dd1f4fe Merge pull request ElementsProject#40 from instagibbs/size_t REVERT: 6532fa0a9 Merge pull request ElementsProject#39 from instagibbs/more_builds REVERT: 2b2429dfa rangeproof: reduce iteration count in unit tests REVERT: 12b0e5dda Enable more builds with rest of experimental flags REVERT: 8c444eef6 use proper types for rangeproof min/max REVERT: 53ad841ca Add explanation about how BIP32 unhardened derivation can be used to simplify whitelisting REVERT: 71c5fe0f6 Add comment to explain effect of max_n_iterations in surjectionproof_init REVERT: 85fd42fb7 add unit test for generator and pedersen commitment roundtripping REVERT: 2ccf88541 rangeproof: fix serialization of pedersen commintments REVERT: 60c173b64 rangeproof: verify correctness of pedersen commitments when parsing REVERT: 32d7526cd generator: verify correctness of point when parsing REVERT: ae14e8a9d rangeproof: check that points deserialize correctly when verifying rangeproof REVERT: 44fe43d75 rangeproof: add fixed vector test case REVERT: e065d7df9 Expose generator in shared library REVERT: fb1ba329a fix spelling in documentation REVERT: fb75faa14 Test for rejection of trailing bytes in range proofs REVERT: 9b2cf1708 Test for rejection of trailing bytes in surjection proofs REVERT: a3a1800ba Reject surjection proofs with trailing garbage REVERT: 0c77ae9a7 Minor bugfix. Wrong length due to NUL character. REVERT: b1f31bc4b Add whitelisting benchmark REVERT: 52a9f8f8f add whitelist_impl.h to include for dist REVERT: a707865bc generator: add API tests REVERT: ec1ef040f generator: remove unnecessary ARG_CHECK from generate() REVERT: b0e9aa828 Fix generator makefile REVERT: 526c65499 Fix pedersen_blind_generator_blind_sum return value documentation REVERT: b51886e72 Add n_keys argument to whitelist_verify REVERT: 37c57de08 Fix checks of whitelist serialize/parse arguments REVERT: 9b8a9d91e whitelist: fix serialize/parse API to take serialized length REVERT: 7f1751560 Fix include/secp256k1_rangeproof.h function argument documentation. REVERT: 0d817020d rangeproof: add API tests REVERT: 417bb0643 surjectionproof: rename unit test functions to be more consistent with other modules REVERT: 1e2d5c1a2 surjectionproof: add API unit tests REVERT: 7878a298b surjectionproof: tests_impl.h s/assert/CHECK/g REVERT: e609591b6 rangeproof: fix memory leak in unit tests REVERT: 0c17f7972 add surjection proof module REVERT: c174f0c60 Implement ring-signature based whitelist delegation scheme REVERT: a2bc6604f rangeproof: several API changes REVERT: 21bfb3c91 Expose generator in pedersen/rangeproof API REVERT: f4620de04 Constant-time generator module REVERT: d46fc3c19 rangeproof: expose sidechannel message field in the signing API REVERT: cf40b1bed [RANGEPROOF BREAK] Use quadratic residue for tie break and modularity cleanup REVERT: 6d28767c7 Get rid of precomputed H tables (Pieter Wuille) REVERT: ae1e576f6 Pedersen commitments, borromean ring signatures, and ZK range proofs. REVERT: efc61dc0c Add 64-bit integer utilities git-subtree-dir: src/secp256k1 git-subtree-split: 44db4d801fff3cd94105136cb443d603683baad2
|
Ok so, belated comment on your last comment @instagibbs -- there already were tests for walletcreatefundedpsbt, but nothing is actually testing that things that should be blinded are, and nothing is testing that nonces don't show up where they shouldn't. I have fixed walletcreatefundedpsbt and converttopsbt, and tested by hand, and the tests pass; but I haven't gone back and made the tests check that expected blinding is actually happening. I expect that the code itself should be done at this point, though -- can you take another look? |
| "blindpsbt \"psbt\" ( ignoreblindfail )\n" | ||
| "\nUse the blinding data from the PSBT inputs to generate the blinding data for the PSBT outputs.\n" | ||
| "\nUse the blinding data from the PSBT inputs to generate the blinding data for the PSBT outputs.\n\n" | ||
| "TODO: Not expected to work on issuance/reissuance/peg transactions yet.\n" |
There was a problem hiding this comment.
I'd rather not have a TODO in help text :) Just telling the user it isn't supported is enough
| // At this point, if the nonce field is present it should be a smuggled | ||
| // pubkey, and not a real nonce. Convert it back to a pubkey and strip | ||
| // it out. | ||
| psbtx.outputs[i].blinding_pubkey = CPubKey(tx.vout[i].nNonce.vchCommitment); |
There was a problem hiding this comment.
do we want to sanity check it's a IsFullyValid key?
There was a problem hiding this comment.
Leaving this per discussion.
| // If we're signing, check that the transaction is not still in need of blinding | ||
| if (sign) { | ||
| for (const PSBTOutput& o : psbtx.outputs) { | ||
| if (o.blinding_pubkey.IsValid()) { |
There was a problem hiding this comment.
I usually prefer IsFullyValid()
There was a problem hiding this comment.
Leaving this per discussion.
| import json | ||
| import os | ||
|
|
||
| import IPython |
There was a problem hiding this comment.
whoop lol
was using this for debugging. Removed.
|
Addressed comments, still have to debug the issue I uncovered with the walletcreatefundedpsbt change output coming out improperly unblinded. |
|
Sigh, ok, gave up on doing createfundedpsbt in (what I felt was) the nice way. The mystery of FundTransaction runs way too deep. Instead, I let FundTransaction do its thing, then extract the pubkeys from the nonces afterwards. Still need to rebase on 0.18. |
|
@gwillen right, seems to be the least painful thing to do. We let Core-ish stuff alone if it makes rebasing upstream easier. |
|
@instagibbs Heh... I didn't even think about the ease-of-rebase issue. ^^; I am still working on the rebase onto 0.18. Currently the rpc_psbt.py test is erroring out immediately at the top of the very first test with "insufficient funds", which confuses me and I'm not sure what's up. |
5becc1e to
c4f3605
Compare
|
Rebased onto master. After a bunch of fixups, the errors left from Travis are (1) an apparent memory-exhaustion running tsan, and (2) a timeout. Not sure what's the best way to deal with those but they're not obviously something I can fix. |
|
Sounds like the builds we haven't been able to get working from upstream.
Will take a look Monday!
…On Fri, Jul 26, 2019, 11:21 PM Glenn Willen ***@***.***> wrote:
Rebased onto master.
After a bunch of fixups, the errors left from Travis are (1) an apparent
memory-exhaustion running tsan, and (2) a timeout. Not sure what's the best
way to deal with those but they're not obviously something I can fix.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#600?email_source=notifications&email_token=ABMAFU2AXMAU4OFJHVEBQB3QBO5NJA5CNFSM4HJSS4DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD26CQKQ#issuecomment-515647530>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABMAFU3IIXQMRPRUBWZZO4TQBO5NJANCNFSM4HJSS4DA>
.
|
commented
Jul 29, 2019
|
Please update the OP to list some of the common usage sequences for RPC. Also an explicit list of the new fields in the description is probably apt. |
commented
Jul 29, 2019
|
oh could you also say what the PR isn't doing in the OP text? e.g., doesn't handle issuance/peg-ins. |
commented
Jul 29, 2019
|
Updated the PR description. |
commented
Jul 31, 2019
|
Addressed all comments. PTAL? |
commented
Jul 31, 2019
|
Changes look good, one more linter: |
commented
Jul 31, 2019
|
Lol, it's really hard for me to not type semicolons at the end of lines. I blame C/C++ for this. Fixed. (One day I will get the python lints working on OS X.) |
commented
Jul 31, 2019
|
Added a sentence of explanation of output_pubkeys_out in ConstructTransaction. I did leave it in because there's still once place I did use it. |
commented
Jul 31, 2019
|
Squashed down to a few sensible commits. |
commented
Jul 31, 2019
|
ACK |
commented
Jul 31, 2019
|
Thanks @instagibbs! Been a long road for this one. |
commented
Jul 31, 2019
|
appreciate the heavy lifting! |
|
|
||
| // How many are we trying to blind? | ||
| int num_pubkeys = 0; | ||
| unsigned int keyIndex = -1; |
There was a problem hiding this comment.
Flagged by ubsan for being an implicit signedness conversion (which is not UB but ubsan lints for it). But also this is literally UB because keyIndex is never checked against this -1 value, it's just used as an array index.
There was a problem hiding this comment.
Should be fixed by #900, just noting here for posterity.
There was a problem hiding this comment.
Oh, this block appears to have been copied from rawblindrawtransaction which has had this issue since at least #515
There was a problem hiding this comment.
Oh, my bad, in both cases it is actually safe becasue keyIndex is only -1 if num_pubkeys is left at 0, and keyIndex is only used as an index when num_pubkeys is nonzero.
This PR extends the PSBT format and RPCs to handle Confidential Assets transactions.
New fields in PSBT inputs: Unblinded value, Value blinder, Unblinded asset, Asset blinder.
New fields in PSBT outputs: Recipient blinding pubkey, Value commitment, Value blinder, Asset commitment, Asset blinder, Nonce commitment, Range proof, Surjection proof.
We preserve the existing invariant that the unsigned transaction inside a PSBT never changes during the process; all updates that need to be applied to produce the final signed transaction are accumulated in the PSBT fields listed above.
The process is as follows:
converttopsbt[deprecated],walletcreatefundedpsbt,createpsbtwalletfillpsbtdata.walletprocesspsbtRPC [deprecated], which tried to both fill and sign the PSBT (which is not workable in the Confidential Assets setting.)blindpsbt.blindpsbtis called.walletsignpsbt. As with updating, this can be done by multiple wallets as necessary for the inputs being signed.finalizepsbtis used to create the final transaction in the regular transaction format, as before.sendrawtransactionis used, which will check to make sure that blinding was performed properly before sending.