bench: wallet, fix change position out of range error #29065

pull furszy wants to merge 1 commits into bitcoin:master from furszy:2023_bench_wallet_fix changing 6 files +8 −8
  1. furszy commented at 6:23 pm on December 12, 2023: member

    Fixes #29061. Only the benchmark is affected.

    Since #25273, the behavior of ‘inserting change at a random position’ is instructed by passing ´std::nullopt´ instead of -1.

    Also, added missing documentation about the meaning of ‘change_pos=std::nullopt’ inside ‘CWallet::CreateTransaction()’

  2. test: wallet, fix change position out of range error 
    Since #25273, the behavior of 'inserting change at a random
position' is instructed by passing std::nullopt instead of -1.

Also, added missing documentation about the meaning of
'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'
    37c75c5820
  3. DrahtBot commented at 6:23 pm on December 12, 2023: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK achow101, kevkevinpal, BrandonOdiwuor

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  4. DrahtBot added the label Tests on Dec 12, 2023
  5. achow101 commented at 8:02 pm on December 12, 2023: member
    ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3
  6. achow101 requested review from aureleoules on Dec 12, 2023
  7. aureleoules commented at 8:31 pm on December 12, 2023: member

    This pull does fix the crash but I am still getting valgrind errors when executing the benchmark, which did not happen on master before. I haven’t verified that these errors were introduced by #25273 but I would suppose so.

      0valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection
  1==243376== Memcheck, a memory error detector
  2==243376== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
  3==243376== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
  4==243376== Command: ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection
  5==243376==
  6==243376== Use of uninitialised value of size 8
  7==243376==    at 0x606AF0: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1240)
  8==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
  9==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
 10==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
 11==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
 12==243376==    by 0x21BA5A: operator() (std_function.h:590)
 13==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
 14==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
 15==243376==
 16==243376== Use of uninitialised value of size 8
 17==243376==    at 0x7FABE9: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
 18==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
 19==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
 20==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
 21==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
 22==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
 23==243376==    by 0x21BA5A: operator() (std_function.h:590)
 24==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
 25==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
 26==243376==
 27==243376== Use of uninitialised value of size 8
 28==243376==    at 0x7FAACD: is_direct (prevector.h:170)
 29==243376==    by 0x7FAACD: size (prevector.h:292)
 30==243376==    by 0x7FAACD: IsUnspendable (script.h:554)
 31==243376==    by 0x7FAACD: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:42)
 32==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
 33==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
 34==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
 35==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
 36==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
 37==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
 38==243376==    by 0x21BA5A: operator() (std_function.h:590)
 39==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
 40==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
 41==243376==
 42==243376== Use of uninitialised value of size 8
 43==243376==    at 0x7FAB88: IsUnspendable (script.h:554)
 44==243376==    by 0x7FAB88: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:42)
 45==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
 46==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
 47==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
 48==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
 49==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
 50==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
 51==243376==    by 0x21BA5A: operator() (std_function.h:590)
 52==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
 53==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
 54==243376==
 55==243376== Use of uninitialised value of size 8
 56==243376==    at 0x8F8364: is_direct (prevector.h:170)
 57==243376==    by 0x8F8364: size (prevector.h:292)
 58==243376==    by 0x8F8364: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:228)
 59==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
 60==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
 61==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
 62==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
 63==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
 64==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
 65==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
 66==243376==    by 0x21BA5A: operator() (std_function.h:590)
 67==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
 68==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
 69==243376==
 70==243376== Use of uninitialised value of size 8
 71==243376==    at 0x8F83AB: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:231)
 72==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
 73==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
 74==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
 75==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
 76==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
 77==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
 78==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
 79==243376==    by 0x21BA5A: operator() (std_function.h:590)
 80==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
 81==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
 82==243376==
 83==243376== Use of uninitialised value of size 8
 84==243376==    at 0x8F83BF: item_ptr (prevector.h:167)
 85==243376==    by 0x8F83BF: operator[] (prevector.h:322)
 86==243376==    by 0x8F83BF: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:234)
 87==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
 88==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
 89==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
 90==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
 91==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
 92==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
 93==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
 94==243376==    by 0x21BA5A: operator() (std_function.h:590)
 95==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
 96==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
 97==243376==
 98==243376== Use of uninitialised value of size 8
 99==243376==    at 0x8F840B: is_direct (prevector.h:170)
100==243376==    by 0x8F840B: size (prevector.h:292)
101==243376==    by 0x8F840B: end (prevector.h:302)
102==243376==    by 0x8F840B: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
103==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
104==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
105==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
106==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
107==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
108==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
109==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
110==243376==    by 0x21BA5A: operator() (std_function.h:590)
111==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
112==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
113==243376==
114==243376== Conditional jump or move depends on uninitialised value(s)
115==243376==    at 0x8F844B: _S_check_init_len (stl_vector.h:1769)
116==243376==    by 0x8F844B: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1582)
117==243376==    by 0x8F844B: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
118==243376==    by 0x8F844B: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
119==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
120==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
121==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
122==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
123==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
124==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
125==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
126==243376==    by 0x21BA5A: operator() (std_function.h:590)
127==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
128==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
129==243376==
130==243376== Use of uninitialised value of size 8
131==243376==    at 0x8F8470: __copy_m<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:385)
132==243376==    by 0x8F8470: __copy_move_a2<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:495)
133==243376==    by 0x8F8470: __copy_move_a1<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:522)
134==243376==    by 0x8F8470: __copy_move_a<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:529)
135==243376==    by 0x8F8470: copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:620)
136==243376==    by 0x8F8470: __uninit_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:110)
137==243376==    by 0x8F8470: uninitialized_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:151)
138==243376==    by 0x8F8470: __uninitialized_copy_a<prevector<28, unsigned char>::const_iterator, unsigned char*, unsigned char> (stl_uninitialized.h:333)
139==243376==    by 0x8F8470: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1585)
140==243376==    by 0x8F8470: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
141==243376==    by 0x8F8470: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
142==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
143==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
144==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
145==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
146==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
147==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
148==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
149==243376==    by 0x21BA5A: operator() (std_function.h:590)
150==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
151==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
152==243376==
153==243376== Use of uninitialised value of size 8
154==243376==    at 0x8F8482: __copy_m<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:383)
155==243376==    by 0x8F8482: __copy_move_a2<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:495)
156==243376==    by 0x8F8482: __copy_move_a1<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:522)
157==243376==    by 0x8F8482: __copy_move_a<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:529)
158==243376==    by 0x8F8482: copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:620)
159==243376==    by 0x8F8482: __uninit_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:110)
160==243376==    by 0x8F8482: uninitialized_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:151)
161==243376==    by 0x8F8482: __uninitialized_copy_a<prevector<28, unsigned char>::const_iterator, unsigned char*, unsigned char> (stl_uninitialized.h:333)
162==243376==    by 0x8F8482: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1585)
163==243376==    by 0x8F8482: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
164==243376==    by 0x8F8482: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
165==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
166==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
167==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
168==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
169==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
170==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
171==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
172==243376==    by 0x21BA5A: operator() (std_function.h:590)
173==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
174==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
175==243376==
176==243376== Conditional jump or move depends on uninitialised value(s)
177==243376==    at 0x8F8482: __copy_m<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:383)
178==243376==    by 0x8F8482: __copy_move_a2<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:495)
179==243376==    by 0x8F8482: __copy_move_a1<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:522)
180==243376==    by 0x8F8482: __copy_move_a<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:529)
181==243376==    by 0x8F8482: copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:620)
182==243376==    by 0x8F8482: __uninit_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:110)
183==243376==    by 0x8F8482: uninitialized_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:151)
184==243376==    by 0x8F8482: __uninitialized_copy_a<prevector<28, unsigned char>::const_iterator, unsigned char*, unsigned char> (stl_uninitialized.h:333)
185==243376==    by 0x8F8482: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1585)
186==243376==    by 0x8F8482: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
187==243376==    by 0x8F8482: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
188==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
189==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
190==243376==    by 0x606B2B: wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1249)
191==243376==    by 0x6086A6: wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient> > const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) (spend.cpp:1333)
192==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
193==243376==    by 0x29843E: ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}>(WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>)::{lambda()#3}&&) [clone .isra.0] (nanobench.h:1221)
194==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
195==243376==    by 0x21BA5A: operator() (std_function.h:590)
196==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
197==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
198==243376==
199
200
201
202|               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
203|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
204|    1,013,313,787.80 |                0.99 |    0.8% |12,459,882,645.20 |5,208,416,083.40 |  2.392 |1,982,028,973.60 |    1.1% |     55.84 | `WalletCreateTxUsePresetInputsAndCoinSelection`
205==243376==
206==243376== HEAP SUMMARY:
207==243376==     in use at exit: 1,241 bytes in 7 blocks
208==243376==   total heap usage: 23,470,975 allocs, 23,470,968 frees, 6,267,436,104 bytes allocated
209==243376==
210==243376== LEAK SUMMARY:
211==243376==    definitely lost: 0 bytes in 0 blocks
212==243376==    indirectly lost: 0 bytes in 0 blocks
213==243376==      possibly lost: 0 bytes in 0 blocks
214==243376==    still reachable: 1,241 bytes in 7 blocks
215==243376==         suppressed: 0 bytes in 0 blocks
216==243376== Rerun with --leak-check=full to see details of leaked memory
217==243376==
218==243376== Use --track-origins=yes to see where uninitialised values come from
219==243376== For lists of detected and suppressed errors, rerun with: -s
220==243376== ERROR SUMMARY: 480 errors from 12 contexts (suppressed: 0 from 0)
  8. kevkevinpal commented at 2:34 am on December 13, 2023: contributor

    ACK 37c75c5

    closes #29069

    Was able to reproduce issue and building with this change fixed it for me

  9. fanquake requested review from achow101 on Dec 13, 2023
  10. fanquake removed review request from achow101 on Dec 13, 2023
  11. BrandonOdiwuor commented at 10:30 am on December 13, 2023: contributor
    Concept ACK
  12. BrandonOdiwuor approved
  13. BrandonOdiwuor commented at 3:58 pm on December 13, 2023: contributor

    ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3

    Looks good to me

  14. achow101 commented at 5:38 pm on December 13, 2023: member

    This pull does fix the crash but I am still getting valgrind errors when executing the benchmark

    Unable to replicate this, so probably a separate issue.

  15. achow101 merged this on Dec 13, 2023
  16. achow101 closed this on Dec 13, 2023
  17. aureleoules commented at 6:02 pm on December 13, 2023: member

    Unable to replicate this, so probably a separate issue.

    I can consistently replicate the valgrind errors on a fresh new ubuntu docker container:

    0Ubuntu 22.04.3 LTS
1g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
2valgrind-3.18.1
3
4Using : 
5./configure --enable-bench --disable-tests --disable-gui --disable-zmq --disable-fuzz --enable-fuzz-binary=no
6valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection

    These errors do not appear in the commit 2d39db7aa128a948b6ad11242591ef26a342f5b1 which is the previous commit of 758501b71391136c33b525b1a0109b990d4f463e, the one that introduced the crash. This seems important to fix to me.

  18. maflcko commented at 7:52 pm on December 13, 2023: member

    This seems important to fix to me.

    I tried on current master with clang-13 and valgrind on Jammy, but it seems to pass.

    Maybe it is a bug in your g++?

     0# git log -1 && valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection 
 1commit 9f0f83d6509a214b827f5110c0f857b494ae854c (HEAD, origin/master, origin/HEAD)
 2Merge: 019ec8a 37c75c5
 3Author: Ava Chow <github@achow101.com>
 4Date:   Wed Dec 13 12:38:11 2023 -0500
 5
 6    Merge bitcoin/bitcoin#29065: bench: wallet, fix change position out of range error
 7    
 8    37c75c58202f89b752500f76c872d7f8caf6bdb3 test: wallet, fix change position out of range error (furszy)
 9    
10    Pull request description:
11    
12      Fixes [#29061](/bitcoin-bitcoin/29061/). Only the benchmark is affected.
13    
14      Since [#25273](/bitcoin-bitcoin/25273/), the behavior of 'inserting change at a random position'
15      is instructed by passing <C2><B4>std::nullopt<C2><B4> instead of -1.
16    
17      Also, added missing documentation about the meaning of
18      'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'
19    
20    ACKs for top commit:
21      achow101:
22        ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3
23      kevkevinpal:
24        ACK [37c75c5](https://github.com/bitcoin/bitcoin/pull/29065/commits/37c75c58202f89b752500f76c872d7f8caf6bdb3)
25      BrandonOdiwuor:
26        ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3
27    
28    Tree-SHA512: d9a8d8533540455716a5090fcf407573cad9f0d0018a05f903f89e51620302f9b256318db6f7338b85c047f7fab372d724e916b1721d7ed302dbf3d845b08734
29==45269== Memcheck, a memory error detector
30==45269== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
31==45269== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
32==45269== Command: ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection
33==45269== 
34Warning, results might be unstable:
35* CPU frequency scaling enabled: CPU 0 between 800.0 and 2,800.0 MHz
36* CPU governor is 'powersave' but should be 'performance'
37
38Recommendations
39* Use 'pyperf system tune' before benchmarking. See https://github.com/psf/pyperf
40
41|               ns/op |                op/s |    err% |     total | benchmark
42|--------------------:|--------------------:|--------:|----------:|:----------
43|    2,889,646,053.20 |                0.35 |    0.6% |    159.99 | `WalletCreateTxUsePresetInputsAndCoinSelection`
44==45269== 
45==45269== HEAP SUMMARY:
46==45269==     in use at exit: 1,241 bytes in 7 blocks
47==45269==   total heap usage: 23,469,947 allocs, 23,469,940 frees, 6,267,372,825 bytes allocated
48==45269== 
49==45269== LEAK SUMMARY:
50==45269==    definitely lost: 0 bytes in 0 blocks
51==45269==    indirectly lost: 0 bytes in 0 blocks
52==45269==      possibly lost: 0 bytes in 0 blocks
53==45269==    still reachable: 1,241 bytes in 7 blocks
54==45269==         suppressed: 0 bytes in 0 blocks
55==45269== Rerun with --leak-check=full to see details of leaked memory
56==45269== 
57==45269== For lists of detected and suppressed errors, rerun with: -s
58==45269== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
  19. furszy deleted the branch on Dec 13, 2023
  20. aureleoules commented at 9:07 pm on December 13, 2023: member

    Maybe it is a bug in your g++?

    Right maybe, I just tested with g++ 13.2.0 and it runs fine now. No worries then.

    Post-merge ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3


furszy DrahtBot achow101 aureleoules kevkevinpal BrandonOdiwuor maflcko


aureleoules

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-01 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me