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

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

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

    <!--021abf342d371248e50ceaed478a90ca-->

    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: contributor

    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.

    valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection
==243376== Memcheck, a memory error detector
==243376== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==243376== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==243376== Command: ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection
==243376==
==243376== Use of uninitialised value of size 8
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x7FABE9: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x7FAACD: is_direct (prevector.h:170)
==243376==    by 0x7FAACD: size (prevector.h:292)
==243376==    by 0x7FAACD: IsUnspendable (script.h:554)
==243376==    by 0x7FAACD: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:42)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x7FAB88: IsUnspendable (script.h:554)
==243376==    by 0x7FAB88: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:42)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x8F8364: is_direct (prevector.h:170)
==243376==    by 0x8F8364: size (prevector.h:292)
==243376==    by 0x8F8364: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:228)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x8F83AB: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:231)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x8F83BF: item_ptr (prevector.h:167)
==243376==    by 0x8F83BF: operator[] (prevector.h:322)
==243376==    by 0x8F83BF: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:234)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x8F840B: is_direct (prevector.h:170)
==243376==    by 0x8F840B: size (prevector.h:292)
==243376==    by 0x8F840B: end (prevector.h:302)
==243376==    by 0x8F840B: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Conditional jump or move depends on uninitialised value(s)
==243376==    at 0x8F844B: _S_check_init_len (stl_vector.h:1769)
==243376==    by 0x8F844B: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1582)
==243376==    by 0x8F844B: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
==243376==    by 0x8F844B: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x8F8470: __copy_m<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:385)
==243376==    by 0x8F8470: __copy_move_a2<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:495)
==243376==    by 0x8F8470: __copy_move_a1<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:522)
==243376==    by 0x8F8470: __copy_move_a<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:529)
==243376==    by 0x8F8470: copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:620)
==243376==    by 0x8F8470: __uninit_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:110)
==243376==    by 0x8F8470: uninitialized_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:151)
==243376==    by 0x8F8470: __uninitialized_copy_a<prevector<28, unsigned char>::const_iterator, unsigned char*, unsigned char> (stl_uninitialized.h:333)
==243376==    by 0x8F8470: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1585)
==243376==    by 0x8F8470: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
==243376==    by 0x8F8470: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Use of uninitialised value of size 8
==243376==    at 0x8F8482: __copy_m<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:383)
==243376==    by 0x8F8482: __copy_move_a2<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:495)
==243376==    by 0x8F8482: __copy_move_a1<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:522)
==243376==    by 0x8F8482: __copy_move_a<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:529)
==243376==    by 0x8F8482: copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:620)
==243376==    by 0x8F8482: __uninit_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:110)
==243376==    by 0x8F8482: uninitialized_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:151)
==243376==    by 0x8F8482: __uninitialized_copy_a<prevector<28, unsigned char>::const_iterator, unsigned char*, unsigned char> (stl_uninitialized.h:333)
==243376==    by 0x8F8482: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1585)
==243376==    by 0x8F8482: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
==243376==    by 0x8F8482: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==
==243376== Conditional jump or move depends on uninitialised value(s)
==243376==    at 0x8F8482: __copy_m<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:383)
==243376==    by 0x8F8482: __copy_move_a2<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:495)
==243376==    by 0x8F8482: __copy_move_a1<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:522)
==243376==    by 0x8F8482: __copy_move_a<false, prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:529)
==243376==    by 0x8F8482: copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_algobase.h:620)
==243376==    by 0x8F8482: __uninit_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:110)
==243376==    by 0x8F8482: uninitialized_copy<prevector<28, unsigned char>::const_iterator, unsigned char*> (stl_uninitialized.h:151)
==243376==    by 0x8F8482: __uninitialized_copy_a<prevector<28, unsigned char>::const_iterator, unsigned char*, unsigned char> (stl_uninitialized.h:333)
==243376==    by 0x8F8482: _M_range_initialize<prevector<28, unsigned char>::const_iterator> (stl_vector.h:1585)
==243376==    by 0x8F8482: vector<prevector<28, unsigned char>::const_iterator> (stl_vector.h:657)
==243376==    by 0x8F8482: CScript::IsWitnessProgram(int&, std::vector<unsigned char, std::allocator<unsigned char> >&) const (script.cpp:236)
==243376==    by 0x7FAB1F: GetDustThreshold(CTxOut const&, CFeeRate const&) (policy.cpp:54)
==243376==    by 0x7FAC00: IsDust(CTxOut const&, CFeeRate const&) (policy.cpp:67)
==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)
==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)
==243376==    by 0x29843E: operator() (wallet_create_tx.cpp:132)
==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)
==243376==    by 0x29A381: WalletCreateTx(ankerl::nanobench::Bench&, OutputType, bool, std::optional<PreSelectInputs>) [clone .constprop.0] [clone .isra.0] (wallet_create_tx.cpp:130)
==243376==    by 0x21BA5A: operator() (std_function.h:590)
==243376==    by 0x21BA5A: benchmark::BenchRunner::RunAll(benchmark::Args const&) (bench.cpp:119)
==243376==    by 0x1F84B6: main (bench_bitcoin.cpp:132)
==243376==



|               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
|    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`
==243376==
==243376== HEAP SUMMARY:
==243376==     in use at exit: 1,241 bytes in 7 blocks
==243376==   total heap usage: 23,470,975 allocs, 23,470,968 frees, 6,267,436,104 bytes allocated
==243376==
==243376== LEAK SUMMARY:
==243376==    definitely lost: 0 bytes in 0 blocks
==243376==    indirectly lost: 0 bytes in 0 blocks
==243376==      possibly lost: 0 bytes in 0 blocks
==243376==    still reachable: 1,241 bytes in 7 blocks
==243376==         suppressed: 0 bytes in 0 blocks
==243376== Rerun with --leak-check=full to see details of leaked memory
==243376==
==243376== Use --track-origins=yes to see where uninitialised values come from
==243376== For lists of detected and suppressed errors, rerun with: -s
==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: contributor

    Unable to replicate this, so probably a separate issue.

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

    Ubuntu 22.04.3 LTS
g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
valgrind-3.18.1

Using : 
./configure --enable-bench --disable-tests --disable-gui --disable-zmq --disable-fuzz --enable-fuzz-binary=no
valgrind ./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++?

    # git log -1 && valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection 
commit 9f0f83d6509a214b827f5110c0f857b494ae854c (HEAD, origin/master, origin/HEAD)
Merge: 019ec8a 37c75c5
Author: Ava Chow <github@achow101.com>
Date:   Wed Dec 13 12:38:11 2023 -0500

    Merge bitcoin/bitcoin#29065: bench: wallet, fix change position out of range error
    
    37c75c58202f89b752500f76c872d7f8caf6bdb3 test: wallet, fix change position out of range error (furszy)
    
    Pull request description:
    
      Fixes [#29061](/bitcoin-bitcoin/29061/). Only the benchmark is affected.
    
      Since [#25273](/bitcoin-bitcoin/25273/), the behavior of 'inserting change at a random position'
      is instructed by passing <C2><B4>std::nullopt<C2><B4> instead of -1.
    
      Also, added missing documentation about the meaning of
      'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'
    
    ACKs for top commit:
      achow101:
        ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3
      kevkevinpal:
        ACK [37c75c5](https://github.com/bitcoin/bitcoin/pull/29065/commits/37c75c58202f89b752500f76c872d7f8caf6bdb3)
      BrandonOdiwuor:
        ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3
    
    Tree-SHA512: d9a8d8533540455716a5090fcf407573cad9f0d0018a05f903f89e51620302f9b256318db6f7338b85c047f7fab372d724e916b1721d7ed302dbf3d845b08734
==45269== Memcheck, a memory error detector
==45269== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==45269== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==45269== Command: ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection
==45269== 
Warning, results might be unstable:
* CPU frequency scaling enabled: CPU 0 between 800.0 and 2,800.0 MHz
* CPU governor is 'powersave' but should be 'performance'

Recommendations
* Use 'pyperf system tune' before benchmarking. See https://github.com/psf/pyperf

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|    2,889,646,053.20 |                0.35 |    0.6% |    159.99 | `WalletCreateTxUsePresetInputsAndCoinSelection`
==45269== 
==45269== HEAP SUMMARY:
==45269==     in use at exit: 1,241 bytes in 7 blocks
==45269==   total heap usage: 23,469,947 allocs, 23,469,940 frees, 6,267,372,825 bytes allocated
==45269== 
==45269== LEAK SUMMARY:
==45269==    definitely lost: 0 bytes in 0 blocks
==45269==    indirectly lost: 0 bytes in 0 blocks
==45269==      possibly lost: 0 bytes in 0 blocks
==45269==    still reachable: 1,241 bytes in 7 blocks
==45269==         suppressed: 0 bytes in 0 blocks
==45269== Rerun with --leak-check=full to see details of leaked memory
==45269== 
==45269== For lists of detected and suppressed errors, rerun with: -s
==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: contributor

    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

  21. bitcoin locked this on Dec 12, 2024
Contributors
furszyDrahtBotachow101aureleouleskevkevinpalBrandonOdiwuormaflcko


aureleoules

Labels
Tests
Linked (view graph)
#25273 wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction#29061 Benchmark WalletCreateTxUsePresetInputsAndCoinSelection crashes due to #25273
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: 2026-05-02 03:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me