abort when blinding very large transaction

[gwillen]

Last log lines:

2020-05-29T21:13:32Z [default wallet] keypool reserve 1003
[libsecp256k1] illegal argument: n_input_tags <= SECP256K1_SURJECTIONPROOF_MAX_N_INPUTS

Stack trace:

(lldb) bt
* thread #13, name = 'bitcoin-httpworker', stop reason = signal SIGABRT
  * frame #0: 0x00007fff64a4a2c6 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff64b05bf1 libsystem_pthread.dylib`pthread_kill + 284
    frame #2: 0x00007fff649b46a6 libsystem_c.dylib`abort + 127
    frame #3: 0x000000010054c507 elementsd`secp256k1_default_illegal_callback_fn(str=<unavailable>, data=<unavailable>) at secp256k1.c:52 [opt]
    frame #4: 0x000000010057e82e elementsd`secp256k1_surjectionproof_initialize [inlined] secp256k1_callback_call(text=<unavailable>) at util.h:24 [opt]
    frame #5: 0x000000010057e828 elementsd`secp256k1_surjectionproof_initialize(ctx=<unavailable>, proof=0x0000700005971588, input_index=0x0000700005971560, fixed_input_tags=0x000000010b7b0000, n_input_tags=2617, n_input_tags_to_use=3, fixed_output_tag=0x0000700005971568, n_max_iterations=100, random_seed32="`6?\x13KT=?\x05\x1cP\x9c\" \x8fbu\x98\x1d\x92?Ն?@\x04?G?l?") at main_impl.h:222 [opt]
    frame #6: 0x0000000100427df9 elementsd`SurjectOutput(txoutwit=0x00000002054698b0, surjection_targets=size=2617, target_asset_generators=size=2617, target_asset_blinders=size=2617, asset_blindptrs=size=1, output_asset_gen=0x0000700005973880, asset=0x0000700005973860) at blind.cpp:135 [opt]
    frame #7: 0x0000000100429e85 elementsd`BlindTransaction(input_value_blinding_factors=size=0, input_asset_blinding_factors=size=0, input_assets=size=0, input_amounts=size=0, out_val_blind_factors=size=3, out_asset_blind_factors=size=3, output_pubkeys=size=3, issuance_blinding_privkey=size=0, token_blinding_privkey=size=0, tx=0x0000700005a104d0, auxiliary_generators=0x0000000000000000) at blind.cpp:547 [opt]
    frame #8: 0x00000001003df4bb elementsd`interfaces::(anonymous namespace)::WalletImpl::handleTransactionChanged(std::__1::function<void (uint256 const&, ChangeType)>) [inlined] std::__1::function<void (uint256 const&, ChangeType)>::function(this=<unavailable>, __f=<unavailable>)> const&) - 18446744069410523972 [opt]
    frame #9: 0x000000010033f847 elementsd`SendMoney(locked_chain=0x00000002052bb8d0, pwallet=0x0000000101820a00, address=0x0000700005a11050, nValue=10000000, asset=0x0000700005a11200, fSubtractFeeFromAmount=true, coin_control=0x0000700005a11220, mapValue=size=10000000, ignore_blind_fail=<unavailable>) at rpcwallet.cpp:389 [opt]
    frame #10: 0x0000000100395f6d elementsd`sendtoaddress(request=0x0000700005a11ad0) at rpcwallet.cpp:506 [opt]
    frame #11: 0x00000001001dab1a elementsd`CRPCTable::execute(this=<unavailable>, request=0x0000700005a11ad0) const at server.cpp:557 [opt]
    frame #12: 0x000000010001ed9e elementsd`HTTPReq_JSONRPC(req=0x00000001fff61720, (null)=<unavailable>) at httprpc.cpp:192 [opt]
    frame #13: 0x000000010002686b elementsd`HTTPWorkItem::operator()() [inlined] std::__1::function<bool (HTTPRequest*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)>::operator(this=<unavailable>, __arg=0x00000001fff61720, __arg=<unavailable>)(HTTPRequest*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) const at functional:1913 [opt]
    frame #14: 0x000000010002685e elementsd`HTTPWorkItem::operator(this=<unavailable>)() at httpserver.cpp:55 [opt]
    frame #15: 0x0000000100028583 elementsd`WorkQueue<HTTPClosure>::Run(this=0x0000000101600640) at httpserver.cpp:114 [opt]
    frame #16: 0x0000000100028b9a elementsd`void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(WorkQueue<HTTPClosure>*), WorkQueue<HTTPClosure>*> >(void*) [inlined] decltype(__f=<unavailable>, __args=<unavailable>)(WorkQueue<HTTPClosure>*)>(fp)(std::__1::forward<WorkQueue<HTTPClosure>*>(fp0))) std::__1::__invoke<void (*)(WorkQueue<HTTPClosure>*), WorkQueue<HTTPClosure>*>(void (*&&)(WorkQueue<HTTPClosure>*), WorkQueue<HTTPClosure>*&&) at type_traits:4428 [opt]
    frame #17: 0x0000000100028b92 elementsd`void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(WorkQueue<HTTPClosure>*), WorkQueue<HTTPClosure>*> >(void*) [inlined] void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(WorkQueue<HTTPClosure>*), WorkQueue<HTTPClosure>*, 2ul>(__t=<unavailable>)(WorkQueue<HTTPClosure>*), WorkQueue<HTTPClosure>*>&, std::__1::__tuple_indices<2ul>) at thread:342 [opt]
    frame #18: 0x0000000100028b92 elementsd`void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(WorkQueue<HTTPClosure>*), WorkQueue<HTTPClosure>*> >(__vp=0x0000000101007ca0) at thread:352 [opt]
    frame #19: 0x00007fff64b032eb libsystem_pthread.dylib`_pthread_body + 126
    frame #20: 0x00007fff64b06249 libsystem_pthread.dylib`_pthread_start + 66
    frame #21: 0x00007fff64b0240d libsystem_pthread.dylib`thread_start + 13

I'm investigating in a live debugger but I wanted to document for the record while I fiddle with it. The offending transaction has a huge number of tiny inputs. Since it's only got one output, and no inputs are blinded, it's going to fail to blind anyway, but apparently it does a lot of work before checking this.

[gwillen]

n_input_tags=2617 -- it seems like the code just assumes that it's not possible to violate this limit without hitting the transaction size limit before, and this is almost always true, not sure why it stopped being true just this one time.

EDIT: Hmm, but the limit is 256, we are WAY past it...

[gwillen]

Aha okay, new theory: this triggers any time you try to make a blinded transaction with more than 256 inputs (or something similar to that.) This failure happens before the "too large" check. I wasn't seeing it before because I wasn't making blinded transactions.

For now I will try to work around it by just not making huge transactions.

I think that probably all of the following should happen, or similar: (1) secp should document this limit in the header comments (it technically sort of does in passing, but it should formally state that this parameter must be less than this constant or we will abort); (2) elements should (in SurjectOutput) fail informatively OR do something sensible (if that's possible, I'm not sure) if the parameter would be exceeded; (3) if it's not possible to do something sensible in SurjectOutput, elements should not try to create transactions with more than 256 inputs (or whatever necessary limit), at the stage of like, coin selection or something.

[gwillen]

Addendum: we should add regression tests for the largest thing we expect to succeed without hitting the limit, at least one thing that would hit the limit (testing for correctness of whatever we have it do instead), and also a thing we expect to fail if there are any remaining cases where it's not possible to do the right thing.

[apoelstra]

There is no good reason for there to be a 256-input limit, but unfortunately it is a hard-fork to fix this.

[gwillen]

If we adjust that limit I'm sure there are other limits we'd hit. But we should fix it so we don't crash.