Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Default wallet passphrase timeout #1

Open
wants to merge 272 commits into
base: master
Choose a base branch
from
Open

Default wallet passphrase timeout #1

wants to merge 272 commits into from

Conversation

kevkevinpal
Copy link
Owner

No description provided.

@furszy
gui: macOS, do not process dock icon actions during shutdown
e14cc8f 
As the 'QMenuBar' is created without a parent window in MacOS, the
app crashes when the user presses the shutdown button and, right
after it, triggers any action in the menu bar.

This happens because the QMenuBar is manually deleted in the
BitcoinGUI destructor but the events attached to it children
actions are not disconnected, so QActions events such us the
'QMenu::aboutToShow' could try to access null pointers.

Instead of guarding every single QAction pointer inside the
QMenu::aboutToShow slot, or manually disconnecting all
registered events in the destructor, we can check if a
shutdown was requested and discard the event.

The 'node' field is a ref whose memory is held by the
main application class, so it is safe to use here. Events
are disconnected prior destructing the main application object.

Furthermore, the 'MacDockIconHandler::dockIconClicked' signal
can make the app crash during shutdown for the very same
reason. The 'show()' call triggers the 'QApplication::focusWindowChanged'
event, which is connected to the 'minimize_action' QAction,
which is also part of the app menu bar, which could no longer exist.
kevkevinpal pushed a commit that referenced this pull request Sep 11, 2023
@fanquake
Merge bitcoin#27917: fuzz: Fix implicit-integer-sign-change in wallet…
c2316b1 
…/fees fuzz target

faa05d1 fuzz: Fix implicit-integer-sign-change in wallet/fees fuzz target (MarcoFalke)

Pull request description:

  This fixes a bug in the fuzz target.

  ```
  echo 'OiAAAPr//wAAAAAAAAA=' | base64  --decode > /tmp/a
  UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=wallet_fees ./src/test/fuzz/fuzz  /tmp/a
  ```

  ```
  wallet/fees.cpp:58:58: runtime error: implicit conversion from type 'unsigned int' of value 4294574080 (32-bit, unsigned) to type 'int' changed the value to -393216 (32-bit, signed)
      #0 0x5625ef46a094 in wallet::GetMinimumFeeRate(wallet::CWallet const&, wallet::CCoinControl const&, FeeCalculation*) src/wallet/fees.cpp:58:58
      #1 0x5625eedd467f in wallet::(anonymous namespace)::wallet_fees_fuzz_target(Span<unsigned char const>) src/wallet/test/fuzz/fees.cpp:64:11
  ...

  SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change wallet/fees.cpp:58:58 in

ACKs for top commit:
  dergoegge:
    tACK faa05d1
  brunoerg:
    ACK faa05d1

Tree-SHA512: 66a4020d6a4153a92c7023e9f94ec6279862566db7236ce3cf6951b7fbee616dc88a56fe9502de4099d74f9840439b20a984b0733fb432e43129e774bcc2a6e6
kevkevinpal pushed a commit that referenced this pull request Sep 11, 2023
@fanquake
Merge bitcoin#27922: ci: fix llvm-symbolizer in MSAN jobs
8d5b93c 
682274a ci: install llvm-symbolizer in MSAN jobs (fanquake)
96527cd ci: use LLVM 16.0.6 in MSAN jobs (fanquake)

Pull request description:

  Fixes: bitcoin#27737 (comment).

  Tested (locally) with bitcoin#27495 that it produces a symbolized backtrace:
  ```bash
  2023-06-20T17:5Uninitialized bytes in __interceptor_strlen at offset 113 inside [0x719000006908, 114)
  ==35429==WARNING: MemorySanitizer: use-of-uninitialized-value
      #0 0x56060fae8c4b in sqlite3Strlen30 /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:32670:28
      #1 0x56060fb0fcf4 in sqlite3PagerOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:57953:17
      #2 0x56060fb0f48b in sqlite3BtreeOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:68679:10
      #3 0x56060fb01384 in openDatabase /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:171911:8
      #4 0x56060fb016ca in sqlite3_open_v2 /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:172034:10
      #5 0x56060e8a94db in wallet::SQLiteDatabase::Open() src/wallet/sqlite.cpp:250:19
      #6 0x56060e8a30fd in wallet::SQLiteDatabase::SQLiteDatabase(fs::path const&, fs::path const&, wallet::DatabaseOptions const&, bool) src/wallet/sqlite.cpp:133:9
      #7 0x56060e8b78f5 in std::__1::__unique_if<wallet::SQLiteDatabase>::__unique_single std::__1::make_unique[abi:v160006]<wallet::SQLiteDatabase, std::__1::__fs::filesystem::path, fs::path&, wallet::DatabaseOptions const&>(std::__1::__fs::filesystem::path&&, fs::path&, wallet::DatabaseOptions const&) /home/ubuntu/ci_scratch/ci/scratch/msan/cxx_build/include/c++/v1/__memory/unique_ptr.h:686:30
      #8 0x56060e8b5240 in wallet::MakeSQLiteDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/sqlite.cpp:641:19
      #9 0x56060e83560b in wallet::MakeDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/walletdb.cpp:1261:16
      #10 0x56060e7546e9 in wallet::MakeWalletDatabase(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/wallet.cpp:2905:12
      #11 0x56060e4bc03f in wallet::TestLoadWallet(wallet::WalletContext&) src/wallet/test/util.cpp:68:21
      #12 0x56060e349ad4 in wallet::wallet_tests::ZapSelectTx::test_method() src/wallet/test/wallet_tests.cpp:897:19
      #13 0x56060e348598 in wallet::wallet_tests::ZapSelectTx_invoker() src/wallet/test/wallet_tests.cpp:891:1
      #14 0x56060cfec325 in boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11
      #15 0x56060ced3a7e in boost::function0<void>::operator()() const /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14
      #16 0x56060ced3a7e in boost::detail::forward::operator()() /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32
      #17 0x56060ced3a7e in boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18
      #18 0x56060cda71c2 in boost::function0<int>::operator()() const /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14
      #19 0x56060cda71c2 in int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()>>(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:301:30
      #20 0x56060cda71c2 in boost::execution_monitor::catch_signals(boost::function<int ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:903:16
      bitcoin#21 0x56060cda784a in boost::execution_monitor::execute(boost::function<int ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1301:16
      bitcoin#22 0x56060cd9ec3a in boost::execution_monitor::vexecute(boost::function<void ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1397:5
      bitcoin#23 0x56060cd9ec3a in boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9
      bitcoin#24 0x56060ce1a07b in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:815:44
      bitcoin#25 0x56060ce1ad8b in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58
      bitcoin#26 0x56060ce1ad8b in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58
      bitcoin#27 0x56060cd9b8de in boost::unit_test::framework::run(unsigned long, bool) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1722:29
      bitcoin#28 0x56060cdd4fac in boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:250:9
      bitcoin#29 0x56060cdd6094 in main /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:306:12
      bitcoin#30 0x7f7379691d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
      bitcoin#31 0x7f7379691e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
      bitcoin#32 0x56060cce2e24 in _start (/home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/test_bitcoin+0x188e24)

    Uninitialized value was created by a heap allocation
      #0 0x56060cd163f2 in malloc /ci_base_install/ci/scratch/msan/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:934:3
      #1 0x56060fc10069 in sqlite3MemMalloc /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:25163:7
      #2 0x56060fb063bc in mallocWithAlarm /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:28846:7
      #3 0x56060fae4eb9 in sqlite3Malloc /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:28876:5
      #4 0x56060faf9e19 in sqlite3DbMallocRaw /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:29176:7
      #5 0x56060fb0fc67 in sqlite3PagerOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:57938:17
      #6 0x56060fb0f48b in sqlite3BtreeOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:68679:10
      #7 0x56060fb01384 in openDatabase /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:171911:8
      #8 0x56060fb016ca in sqlite3_open_v2 /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:172034:10
      #9 0x56060e8a94db in wallet::SQLiteDatabase::Open() src/wallet/sqlite.cpp:250:19
      #10 0x56060e8a30fd in wallet::SQLiteDatabase::SQLiteDatabase(fs::path const&, fs::path const&, wallet::DatabaseOptions const&, bool) src/wallet/sqlite.cpp:133:9
      #11 0x56060e8b78f5 in std::__1::__unique_if<wallet::SQLiteDatabase>::__unique_single std::__1::make_unique[abi:v160006]<wallet::SQLiteDatabase, std::__1::__fs::filesystem::path, fs::path&, wallet::DatabaseOptions const&>(std::__1::__fs::filesystem::path&&, fs::path&, wallet::DatabaseOptions const&) /home/ubuntu/ci_scratch/ci/scratch/msan/cxx_build/include/c++/v1/__memory/unique_ptr.h:686:30
      #12 0x56060e8b5240 in wallet::MakeSQLiteDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/sqlite.cpp:641:19
      #13 0x56060e83560b in wallet::MakeDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/walletdb.cpp:1261:16
      #14 0x56060e7546e9 in wallet::MakeWalletDatabase(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/wallet.cpp:2905:12
      #15 0x56060e4bc03f in wallet::TestLoadWallet(wallet::WalletContext&) src/wallet/test/util.cpp:68:21
      #16 0x56060e349ad4 in wallet::wallet_tests::ZapSelectTx::test_method() src/wallet/test/wallet_tests.cpp:897:19
      #17 0x56060e348598 in wallet::wallet_tests::ZapSelectTx_invoker() src/wallet/test/wallet_tests.cpp:891:1
      #18 0x56060cfec325 in boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11
      #19 0x56060ced3a7e in boost::function0<void>::operator()() const /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14
      #20 0x56060ced3a7e in boost::detail::forward::operator()() /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32
      bitcoin#21 0x56060ced3a7e in boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18

  SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:32670:28 in sqlite3Strlen30
  ```

  as opposed to unsymbolized: https://cirrus-ci.com/task/6005512018329600?logs=ci#L3245.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 682274a

Tree-SHA512: 8f3e7636761c956537a472989bf07529f5afbd988c5e7e1f07ece8b2599608fa4fe9e1efdc6e302cf0f7f44dec3cf9a3c1e68b758af81a8a8b476a43d3220807
@kevkevinpal kevkevinpal force-pushed the defaultWalletPassphraseTimeout branch 6 times, most recently from 54acb38 to 1cc6b1c Compare September 12, 2023 01:56
@furszy
gui: macOS, make appMenuBar part of the main app window
bae209e 
By moving the appMenuBar destruction responsibility to the QT
framework, we ensure the disconnection of the submenus signals
prior to the destruction of the main app window.

The standalone menu bar may have served a purpose in earlier
versions when it didn't contain actions that directly open
specific screens within the main application window. However,
at present, all the actions within the appMenuBar lead to the
opening of screens within the main app window. So, the absence
of a main app window makes these actions essentially pointless.
@kevkevinpal kevkevinpal force-pushed the defaultWalletPassphraseTimeout branch 2 times, most recently from 7deb571 to a7bc5db Compare September 13, 2023 21:23
@kevkevinpal kevkevinpal force-pushed the defaultWalletPassphraseTimeout branch 5 times, most recently from 9be4f3e to 0fc8db0 Compare September 17, 2023 20:02
erikarvstedt and others added 12 commits September 19, 2023 13:45
@erikarvstedt
contrib/bash-completions: use package naming conventions
360b917 
This naming scheme supports auto-detection and on-demand loading of completions.

See
https://github.com/scop/bash-completion/blob/ba109693ee2284f6a82f8f0e1563baf071252df9/README.md#faq,
section "Where should I put it to be sure that interactive bash shells will find it and source it".

Previously, distro package maintainers had to rename these files manually.
@hebasto
Add ability to specify SHA256 implementation for benchmark purposes
5f72417
@hebasto
bench: Add SHA256 implementation specific benchmarks
ce6df7d
@hebasto
build, macos: Fix qt package build with new Xcode 15 linker
79ef528
@ryanofsky @furszy
wallet: Add TxStateString function for debugging and logging
8a553c9 
Co-authored-by: furszy <[email protected]>
@sipa
Add headerssync-params.py script to the repository
7899402
@sipa
Update parameters in headerssync.cpp
53d7d35
@sipa
Add instructions for headerssync-params.py to release-process.md
3d420d8
@glozow
[doc] parent pay for child in aggregate CheckFeeRate
b4f28cc
@glozow
[txpackages] IsChildWithParentsTree()
e32ba15 
Many edge cases exist when parents in a child-with-parents package can
spend each other. However, this pattern should also be uncommon in
normal use cases.
@glozow
[rpc] require package to be a tree in submitpackage
5b9087a
@glozow
[rpc] allow submitpackage to be called outside of regtest
7a9bb2a
fanquake and others added 21 commits October 17, 2023 10:20
@fanquake
Merge bitcoin#28652: assumeutxo: fail early if snapshot block hash do…
738ef44 
…esn't match AssumeUTXO parameters

9620cb4 assumeutxo: fail early if snapshot block hash doesn't match AssumeUTXO parameters (Sebastian Falbesoner)

Pull request description:

  Right now the `loadtxoutset` RPC call treats literally all files with a minimum size of 40 bytes (=size of metadata) as potential valid snapshot candidates and the waiting loop for seeing the metadata block hash in the headers chain is always entered, e.g.:
  ```
  $ ./src/bitcoin-cli loadtxoutset ~/.vimrc
  <wait>

  bitcoind log:
  ...
  2023-10-15T14:55:45Z [snapshot] waiting to see blockheader 626174207465730a7265626d756e207465730a656c62616e65207861746e7973 in headers chain before snapshot activation
  ...
  ```
  There is no point in doing any further action though if we already know from the start that the UTXO snapshot loading won't be successful. This PR adds an assumeutxo parameter check immediately after the metadata is read in, so we can fail immediately on a mismatch:
  ```
  $ ./src/bitcoin-cli loadtxoutset ~/.vimrc
  error code: -32603
  error message:
  Unable to load UTXO snapshot, assumeutxo block hash in snapshot metadata not recognized (626174207465730a7265626d756e207465730a656c62616e
  65207861746e7973)
  ```
  This way, users who mistakenly try to load files that are not snapshots don't have to wait 10 minutes (=the block header waiting timeout) anymore to get a negative response. If a file is loaded which is a valid snapshot (referencing to an existing block hash), but one which doesn't match the parameters, the feedback is also faster, as we don't have to wait anymore to see the hash in the headers chain before getting an error.

  This is also partially fixes bitcoin#28621.

ACKs for top commit:
  maflcko:
    lgtm ACK 9620cb4
  ryanofsky:
    Code review ACK 9620cb4. This should fix an annoyance and bad UX.
  pablomartin4btc:
    tACK 9620cb4

Tree-SHA512: f88b865e9d46254858e57c024463f389cd9d8760a7cb30c190aa1723a931e159987dfc2263a733825d700fa612e7416691e4d8aab64058f1aeb0a7fa9233ac9c
test: Add assumeutxo test for wrong hash
fa68571
@fjahr
test: assumeutxo file with unknown block hash
621db2f
@fanquake
doc: add historical release notes for 25.1
2338715
@ryanofsky
Merge bitcoin#28647: test: Add assumeutxo test for wrong hash
ff6be77 
fa68571 test: Add assumeutxo test for wrong hash (MarcoFalke)

Pull request description:

  Also:
  * Update test TODOs
  * Fix off-by-4 typo in test, remove `struct` import

ACKs for top commit:
  fjahr:
    utACK fa68571
  theStack:
    Code-review re-ACK fa68571
  pablomartin4btc:
    re ACK fa68571
  ryanofsky:
    Code review ACK fa68571

Tree-SHA512: 877653010efe4e20018827e8ec2801d036e1344457401f0c9e5d55907b817724201dd2e3f0f29505bbff619882c0c2cd731ecdcd209258bcefe11b86ff0205dd
@achow101
Merge bitcoin#28544: wallet: Add TxStateString function for debugging…
fbcf102 
… and logging

8a553c9 wallet: Add TxStateString function for debugging and logging (Ryan Ofsky)

Pull request description:

  I found this useful while debugging silent conflict between bitcoin#10102 and bitcoin#27469 recently

ACKs for top commit:
  ishaanam:
    utACK 8a553c9
  achow101:
    ACK 8a553c9
  furszy:
    Code ACK 8a553c9

Tree-SHA512: 87965c66bcb59a21e7639878bb567e583a0e624735721ff7ad1104eed6bb9fba60607d0e3de7be3304232b3a55f48bab7039ea9c26b0e81963e59f9acd94f666
@hebasto
Merge bitcoin-core/gui#766: Fix coin control input size accounting fo…
9c30f5e 
…r taproot spends

00a52e6 gui: fix coin control input size accounting for taproot spends (Sebastian Falbesoner)

Pull request description:

  If manual coin control is used in the GUI, the input size accounting for P2TR is currently overshooting, as it still assumes P2WPKH (segwitv0) spends which have a larger witness, as ECDSA signatures are longer and the pubkey also has to be provided. Fix that by adding sizes depending on the witness version. Note that the total accounting including outputs is still off and there is some weird logic involved depending on whether SFFO is used, but it's (hopefully) a first step into the right direction.

ACKs for top commit:
  maflcko:
    lgtm ACK 00a52e6
  furszy:
    utACK 00a52e6

Tree-SHA512: 9633642f8473247cc3d8e6e0ef502fd515e1dde0e2939d28d6754d0cececedd6a328df22a3d4c85eb2846fd0417cf224b92594613f6e84ada82d2d7d84fc455f
@achow101
Merge bitcoin#28651: Make miniscript GetWitnessSize accurate for taps…
c2d4e40 
…cript

b228108 miniscript: make GetWitnessSize accurate for tapscript (Pieter Wuille)
8be9851 test: add tests for miniscript GetWitnessSize (Pieter Wuille)
7ed2b2d test: remove mutable global contexts in miniscript fuzzer/test (Pieter Wuille)

Pull request description:

  So far, the same algorithm is used to compute an (upper bound on) the maximum witness size for both P2WSH and P2TR miniscript. That's unfortunate, because it means fee estimations for P2TR miniscript will miss out on the generic savings brought by P2TR witnesses (smaller signatures and public keys, specifically).

  Fix this by making the algorithm use script context specification calculations, and add tests for it. Also included is a cleanup for the tests to avoid mutable globals, as I found it hard to reason about what exactly was being tested.

ACKs for top commit:
  achow101:
    ACK b228108
  darosior:
    ACK b228108

Tree-SHA512: e4bda7376628f3e91cfc74917cefc554ca16eb5f2a0e1adddc33eb8717c4aaa071e56a40f85a2041ae74ec445a7bd0129bba48994c203e0e6e4d25af65954d9e
test: Fix failing time check in rpc_net.py
fa4c683
@glozow
Merge bitcoin#28637: doc: add release note for bitcoin#27460 (new `im…
1803fee 
…portmempool` RPC)

1b672eb doc: add release note for bitcoin#27460 (new `importmempool` RPC) (Sebastian Falbesoner)

Pull request description:

  This PR adds a missing release note for bitcoin#27460.

ACKs for top commit:
  glozow:
    ACK 1b672eb

Tree-SHA512: 89deadbfd6779e6eb19801c9fe7459a9876b920d44e09df102774c1eb8b3c0716462613dc99d1711eda4bd959ea61595b33f4528424ac02cf1af6cb4e5f1f0e9
@ryanofsky
Merge bitcoin#28666: test: assumeutxo file with unknown block hash
655dc71 
621db2f test: assumeutxo file with unknown block hash (Fabian Jahr)

Pull request description:

  Takes care of one of the open Todos in the assumeutxo functional test. Since an unknown block could be any hash, I simply chose one placeholder, it could also be a random string though.

ACKs for top commit:
  maflcko:
    lgtm ACK 621db2f
  pablomartin4btc:
    cr ACK 621db2f
  theStack:
    ACK 621db2f
  ryanofsky:
    Code review ACK 621db2f

Tree-SHA512: ee0438ce619f7348c6f88e39b0ea7ddddb8832956d9034ecc795c6033d5d905c09d11b7d0d5afc38231b2fd091ea7c1bd0a0be99d9c32c4e6357a25d76294142
@instagibbs
docs: Add reference to total.coverage report
2ce7e31
@fanquake
build: move -fstack-reuse=none to CORE_CXXFLAGS
8cfa22a 
This is not a hardening specific flag, it should be used at all times,
regardless of if hardening is enabled or not. Note that this was
still the case here, but having this exist in the hardening flags is
confusing, and may lead someone to move it inside one of the `use_hardening`
blocks, where it would become unused, with `--disable-hardening`.
@fanquake
Merge bitcoin#28671: test: Fix failing time check in rpc_net.py
f4049ea 
fa4c683 test: Fix failing time check in rpc_net.py (MarcoFalke)

Pull request description:

  This check fails on slow runners, such as s390x qemu.

  Fix it by using mocktime.

  See bitcoin#28523 (comment)

ACKs for top commit:
  0xB10C:
    ACK fa4c683
  pinheadmz:
    ACK fa4c683
  brunoerg:
    crACK fa4c683

Tree-SHA512: 83fb534682e11e97537dc89de8c16f206f38af1a892a2d5970c02684c05eaea8fc9adba3159f16b2440ca0b3871d513a0562a6f3a38f19a5574a47be0919e42f
@fanquake
Merge bitcoin#28672: build: move -fstack-reuse=none to CORE_CXXFLAGS
5eb82d5 
8cfa22a build: move -fstack-reuse=none to CORE_CXXFLAGS (fanquake)

Pull request description:

  This is not a hardening specific flag, it should be used at all times, regardless of if hardening is enabled or not. Note that this was still the case here, but having this exist in the hardening flags is confusing, and may lead someone to move it inside one of the `use_hardening` blocks, where it would become unused, with `--disable-hardening`.

  Noticed while reviewing hebasto#32 (comment).

ACKs for top commit:
  theuni:
    ACK 8cfa22a. Agree it's confusing as-is and this better matches the intent.
  hebasto:
    ACK 8cfa22a
  luke-jr:
    utACK 8cfa22a
  TheCharlatan:
    ACK 8cfa22a

Tree-SHA512: 74c3219301398361d06b1ef2257fc9ec18055b1661f8733ee909adefee61e458d70991c32adf0e0450905a7ffbddc99799f5fdac894f4896cfade19f961818df
@fanquake
Merge bitcoin#28617: test: Add Wallet Unlock Context Manager
091d29c 
004903e test: Add Wallet Unlock Context Manager (Brandon Odiwuor)

Pull request description:

  Fixes bitcoin#28601, see bitcoin#28403 (comment)

  Add Context Manager to manage the locking and unlocking of locked wallets with a passphrase during testing.

ACKs for top commit:
  kevkevinpal:
    lgtm ACK [004903e](bitcoin@004903e)
  maflcko:
    lgtm ACK 004903e

Tree-SHA512: ab234c167e71531df0d974ff9a31d444f7ce2a1d05aba5ea868cc9452f139845eeb24ca058d88f058bc02482b762adf2d99e63a6640b872cc71a57a0068abfe8
@fanquake
Merge bitcoin#28673: docs: Add reference to total.coverage report
106ab20 
2ce7e31 docs: Add reference to total.coverage report (Greg Sanders)

Pull request description:

  I couldn't get fuzz coverage to work, so couldn't verify what it would look like if made.

ACKs for top commit:
  darosior:
    ACK 2ce7e31
  dergoegge:
    ACK 2ce7e31

Tree-SHA512: c773ef28f7cc630cb094740805c0d235255e7b0559ac7fed4e8952ade4005335f3787bdd2f8786979e341e914b41c4f3eef02bb4773b15f92d618f1040f53c54
@fanquake
Merge bitcoin#22764: build: Include qt sources for parsing with extra…
9e616ba 
…ct_strings.py

b59b31a build: Drop redundant qt/bitcoin.cpp (Hennadii Stepanov)
d90ad5a build: Include qt sources for parsing with extract_strings.py (Hennadii Stepanov)

Pull request description:

  On master (4fc15d1) some strings are still untranslated.

  This PR fixes this issue.

  To verify:
  1) `./autogen.sh && ./configure && make -C src translate` _before_ applying this change
  2) apply this change
  3) `./autogen.sh && ./configure && make -C src translate` _after_ applying this change

  The result of `git diff src/qt/bitcoinstrings.cpp`:
  ```diff
  --- a/src/qt/bitcoinstrings.cpp
  +++ b/src/qt/bitcoinstrings.cpp
  @@ -126,6 +126,7 @@ QT_TRANSLATE_NOOP("bitcoin-core", ""
   "You need to rebuild the database using -reindex to go back to unpruned "
   "mode.  This will redownload the entire blockchain"),
   QT_TRANSLATE_NOOP("bitcoin-core", "%s is set very high!"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "(press q to shutdown and continue later)"),
   QT_TRANSLATE_NOOP("bitcoin-core", "-maxmempool must be at least %d MB"),
   QT_TRANSLATE_NOOP("bitcoin-core", "A fatal internal error occurred, see debug.log for details"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Cannot resolve -%s address: '%s'"),
  @@ -204,6 +205,8 @@ QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to prepare statement t
   QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to read database verification error: %s"),
   QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Unexpected application id. Expected %u, got %u"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Section [%s] is not recognized."),
  +QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be written"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Signing transaction failed"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" does not exist"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" is a relative path"),
  @@ -242,4 +245,5 @@ QT_TRANSLATE_NOOP("bitcoin-core", "User Agent comment (%s) contains unsafe chara
   QT_TRANSLATE_NOOP("bitcoin-core", "Verifying blocks…"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Verifying wallet(s)…"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Wallet needed to be rewritten: restart %s to complete"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "press q to shutdown"),
   };
  ```

ACKs for top commit:
  ryanofsky:
    Code review ACK b59b31a. Being able to use `_()` macro in qt would allow simplifying some code, for example replacing repetitive:
  TheCharlatan:
    ACK b59b31a

Tree-SHA512: 13d9d86b487a1b6e718ae96c198a0a927c881bf33df318412793ec9efba3a7e59cfa836204f73f5b53ff4c99edce778c11bffaa88138b80e37b71e36df6b816f
@fanquake
Merge bitcoin#28667: doc: add historical release notes for 25.1
6e721c9 
2338715 doc: add historical release notes for 25.1 (fanquake)

Pull request description:

  v25.1 has [been tagged](https://github.com/bitcoin/bitcoin/releases/tag/v25.1).
  For merge post-bin-availability.

ACKs for top commit:
  stickies-v:
    ACK 2338715

Tree-SHA512: 799937b8bacab7908d86c91855bfcdfd1f8f806d82df0d04cf984d957d9bf1bbdde6103975358360ef35b0cfeba3ce894bc7dce412ea6790d495b78b3cebee0d
@achow101
Merge bitcoin#27071: Handle CJDNS from LookupSubNet()
0655e9d 
0e6f6eb net: remove unused CConnman::FindNode(const CSubNet&) (Vasil Dimov)
9482cb7 netbase: possibly change the result of LookupSubNet() to CJDNS (Vasil Dimov)
53afa68 net: move MaybeFlipIPv6toCJDNS() from net to netbase (Vasil Dimov)
6e30865 net: move IsReachable() code to netbase and encapsulate it (Vasil Dimov)
c42ded3 fuzz: ConsumeNetAddr(): avoid IPv6 addresses that look like CJDNS (Vasil Dimov)
64d6f77 net: put CJDNS prefix byte in a constant (Vasil Dimov)

Pull request description:

  `LookupSubNet()` would treat addresses that start with `fc` as IPv6 even if `-cjdnsreachable` is set. This creates the following problems where it is called:

  * `NetWhitelistPermissions::TryParse()`: otherwise `-whitelist=` fails to white list CJDNS addresses: when a CJDNS peer connects to us, it will be matched against IPv6 `fc...` subnet and the match will never succeed.

  * `BanMapFromJson()`: CJDNS bans are stored as just IPv6 addresses in `banlist.json`. Upon reading from disk they have to be converted back to CJDNS, otherwise, after restart, a ban entry like (`fc00::1`, IPv6) would not match a peer (`fc00::1`, CJDNS).

  * `RPCConsole::unbanSelectedNode()`: in the GUI the ban entries go through `CSubNet::ToString()` and back via `LookupSubNet()`. Then it must match whatever is stored in `BanMan`, otherwise it is impossible to unban via the GUI.

  These were uncovered by bitcoin#26859.

  Thus, flip the result of `LookupSubNet()` to CJDNS if the network base address starts with `fc` and `-cjdnsreachable` is set. Since subnetting/masking does not make sense for CJDNS (the address is "random" bytes, like Tor and I2P, there is no hierarchy) treat `fc.../mask` as an invalid `CSubNet`.

  To achieve that, `MaybeFlipIPv6toCJDNS()` has to be moved from `net` to `netbase` and thus also `IsReachable()`. In the process of moving `IsReachable()`, `SetReachable()` and `vfLimited[]` encapsulate those in a class.

ACKs for top commit:
  jonatack:
    Code review ACK 0e6f6eb
  achow101:
    ACK 0e6f6eb
  mzumsande:
    re-ACK 0e6f6eb

Tree-SHA512: 4767a60dc882916de4c8b110ce8de208ff3f58daaa0b560e6547d72e604d07c4157e72cf98b237228310fc05c0a3922f446674492e2ba02e990a272d288bd566
@achow101
Merge bitcoin#28077: I2P: also sleep after errors in Accept() & destr…
77f0ceb 
…oy the session if we get an unexpected error

5c8e15c i2p: destroy the session if we get an unexpected error from the I2P router (Vasil Dimov)
762404a i2p: also sleep after errors in Accept() (Vasil Dimov)

Pull request description:

  ### Background

  In the `i2p::sam::Session` class:

  `Listen()` does:
  * if the session is not created yet
    * create the control socket and on it:
    * `HELLO`
    * `SESSION CREATE ID=sessid`
    * leave the control socked opened
  * create a new socket and on it:
  * `HELLO`
  * `STREAM ACCEPT ID=sessid`
  * read reply (`STREAM STATUS`), `Listen()` only succeeds if it contains `RESULT=OK`

  Then a wait starts, for a peer to connect. When connected,

  `Accept()` does:
  * on the socket from `STREAM ACCEPT` from `Listen()`: read the Base64 identification of the connecting peer

  ### Problem

  The I2P router may be in such a state that this happens in a quick succession (many times per second, see bitcoin#22759 (comment)): `Listen()`-succeeds, `Accept()`-fails.

  `Accept()` fails because the I2P router sends something that is not Base64 on the socket: `STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"`

  We only sleep after failed `Listen()` because the assumption was that if `Accept()` fails then the next `Listen()` will also fail.

  ### Solution

  Avoid filling the log with "Error accepting:" messages and sleep also after a failed `Accept()`.

  ### Extra changes

  * Reset the error waiting time after one successful connection. Otherwise the timer will remain high due to problems that have been solved long time in the past.

  * Increment the wait time less aggressively.

  * Handle the unexpected "Session was closed" message more gracefully (don't log stupid messages like `Cannot decode Base64: "STREAM STATUS...`) and destroy the session right way.

ACKs for top commit:
  achow101:
    ACK 5c8e15c
  jonatack:
    re-ACK 5c8e15c

Tree-SHA512: 1d47958c50eeae9eefcb668b8539fd092adead93328e4bf3355267819304b99ab41cbe1b5dbedbc3452c2bc389dc8330c0e27eb5ccb880e33dc46930a1592885
@kevkevinpal kevkevinpal force-pushed the defaultWalletPassphraseTimeout branch from 0fc8db0 to 7969c6b Compare October 20, 2023 01:53
@kevkevinpal
wallet: walletpassphrase timeout to be set to MAX_SLEEP_TIME
76b1c4c 
In this change we allow the timeout for walletpassphrase to be
set to MAX_SLEEP_TIME,
if set as -1 then we then use the MAX_SLEEP_TIME amount

context from PR 28403

added release notes for RPC Wallet

Also tests were modified to use the max timeout amount using timeout = -1
@kevkevinpal kevkevinpal force-pushed the defaultWalletPassphraseTimeout branch from 7969c6b to 76b1c4c Compare October 20, 2023 01:54
kevkevinpal pushed a commit that referenced this pull request Nov 7, 2023
@fanquake
Merge bitcoin#28695: net: Sanity check private keys received from SAM…
ec5116a 
… proxy

5cf4d26 [test] Test i2p private key constraints (Vasil Dimov)
cf70a8d [net] Check i2p private key constraints (dergoegge)

Pull request description:

  Not sanity checking can lead to crashes or worse:

  ```
  ==1715589==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6140000055c2 at pc 0x5622ed66e7ad bp 0x7ffee547a2c0 sp 0x7ffee547a2b8
  READ of size 2 at 0x6140000055c2 thread T0 (b-test)
      #0 0x5622ed66e7ac in memcpy include/bits/string_fortified.h:29:10
      #1 0x5622ed66e7ac in i2p::sam::Session::MyDestination() const src/i2p.cpp:362:5
      #2 0x5622ed662e46 in i2p::sam::Session::CreateIfNotCreatedAlready() src/i2p.cpp:414:40
      #3 0x5622ed6619f2 in i2p::sam::Session::Listen(i2p::Connection&) src/i2p.cpp:143:9
  ```

ACKs for top commit:
  maflcko:
    code lgtm ACK 5cf4d26
  stickies-v:
    re-ACK 5cf4d26
  vasild:
    ACK 5cf4d26

Tree-SHA512: 3de3bd396538fa619de67957b9c8a58011ab911f0f51097c387e730c13908278b7322aa3357051fb245a20b15bef34b0e9fadcb1eff8ad751139d2aa634c78ad
kevkevinpal pushed a commit that referenced this pull request Dec 31, 2023
@fanquake
Merge bitcoin#28966: test: Add missing CBlockPolicyEstimator::process…
453c9ca 
…BlockTx suppression

fa9dc92 test: Add missing CBlockPolicyEstimator::processBlockTx suppression (MarcoFalke)

Pull request description:

  Fixes bitcoin#28865 (comment)

  ```
  # FUZZ=policy_estimator UBSAN_OPTIONS="suppressions=/root/fuzz_dir/scratch/fuzz_gen/code/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/test/fuzz/fuzz /tmp/crash-154b42214e70781a9c1ad72d3f2693913dcf8c06

  ...

  policy/fees.cpp:632:27: runtime error: implicit conversion from type 'unsigned int' of value 4294574080 (32-bit, unsigned) to type 'int' changed the value to -393216 (32-bit, signed)
      #0 0x55cbbe10daee in CBlockPolicyEstimator::processBlockTx(unsigned int, CTxMemPoolEntry const*) src/policy/fees.cpp:632:27
      #1 0x55cbbe10e361 in CBlockPolicyEstimator::processBlock(unsigned int, std::vector<CTxMemPoolEntry const*, std::allocator<CTxMemPoolEntry const*>>&) src/policy/fees.cpp:680:13
      #2 0x55cbbd84af48 in policy_estimator_fuzz_target(Span<unsigned char const>)::$_1::operator()() const src/test/fuzz/policy_estimator.cpp:69:40
      #3 0x55cbbd84af48 in unsigned long CallOneOf<policy_estimator_fuzz_target(Span<unsigned char const>)::$_0, policy_estimator_fuzz_target(Span<unsigned char const>)::$_1, policy_estimator_fuzz_target(Span<unsigned char const>)::$_2, policy_estimator_fuzz_target(Span<unsigned char const>)::$_3>(FuzzedDataProvider&, policy_estimator_fuzz_target(Span<unsigned char const>)::$_0, policy_estimator_fuzz_target(Span<unsigned char const>)::$_1, policy_estimator_fuzz_target(Span<unsigned char const>)::$_2, policy_estimator_fuzz_target(Span<unsigned char const>)::$_3) src/./test/fuzz/util.h:43:27
      #4 0x55cbbd84af48 in policy_estimator_fuzz_target(Span<unsigned char const>) src/test/fuzz/policy_estimator.cpp:38:9
      #5 0x55cbbda1cc18 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
      #6 0x55cbbda1cc18 in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:178:5
      #7 0x55cbbd26a944 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x190e944) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d)
      #8 0x55cbbd253916 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x18f7916) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d)
      #9 0x55cbbd25945a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x18fd45a) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d)
      #10 0x55cbbd284026 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x1928026) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d)
      #11 0x7fe4aa8280cf  (/lib/x86_64-linux-gnu/libc.so.6+0x280cf) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
      #12 0x7fe4aa828188 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28188) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
      #13 0x55cbbd24e494 in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x18f2494) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d)

  SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change policy/fees.cpp:632:27 in
  ```

  ```
  # base64 /tmp/crash-154b42214e70781a9c1ad72d3f2693913dcf8c06
  AQEAAAAAADkFlVwAAQEAAAAAADkFlZVcACTDSSsP3746IAZrH48khwMAAQEB/QEALQAACwAAAAAA
  FgAAAAAAAQAABgAAAAAAAAAAAAAAAAAAACcQAAAAAAAAAAAAAAAAAAAAAAD6AAAAOQWVXAABAQAA
  AAAAOQWVlVwAIMNJKw/fvjogBmsfjySHAwABAQH9AQAtAAALAAAAAAAAAAABAAAGAAAAAAAAAAAA
  AAAAAAAAJxAAAAAAAAAAAAAAAAAAAAAAAPr/AAAAAAAAAAAAAAQAAAAA/wAAAAAAAAAAAAAEAAAA
  AAEBAeAIAVwBXAAA/jbSBvwBKABSKBwBYgEB2wAEkvXInHYAAAAAAAAAvgAAAAAA/9//6v8e/xIk
  MgAlAiUAOw==

ACKs for top commit:
  fanquake:
    ACK fa9dc92
  dergoegge:
    utACK fa9dc92

Tree-SHA512: 3898c17c928ecc2bcc8c7086359e9ae00da2197b4d8e10c7bf6d12415326c9bca3ef6e1d8d3b83172ccfa604ce7e7371415262ba705225f9ea4da8b1a7eb0306
kevkevinpal pushed a commit that referenced this pull request Dec 31, 2023
@fanquake
Merge bitcoin#28969: fuzz: Avoid signed-integer-overflow in wallet_no…
7bc8c53 
…tifications fuzz target

fab164f fuzz: Avoid signed-integer-overflow in wallet_notifications fuzz target (MarcoFalke)

Pull request description:

  Should avoid

  ```
  policy/feerate.cpp:29:63: runtime error: signed integer overflow: 77600710321911316 * 149 cannot be represented in type 'int64_t' (aka 'long')
      #0 0x563a1775ed66 in CFeeRate::GetFee(unsigned int) const src/policy/feerate.cpp:29:63
      #1 0x563a15913a69 in wallet::COutput::COutput(COutPoint const&, CTxOut const&, int, int, bool, bool, bool, long, bool, std::optional<CFeeRate>) src/./wallet/coinselection.h:91:57
      #2 0x563a16fa6a6d in wallet::FetchSelectedInputs(wallet::CWallet const&, wallet::CCoinControl const&, wallet::CoinSelectionParams const&) src/wallet/spend.cpp:297:17
      #3 0x563a16fc4512 in wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, int, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1105:33
      #4 0x563a16fbec74 in wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, int, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1291:16
      #5 0x563a16fcf6df in wallet::FundTransaction(wallet::CWallet&, CMutableTransaction&, long&, int&, bilingual_str&, bool, std::set<int, std::less<int>, std::allocator<int>> const&, wallet::CCoinControl) src/wallet/spend.cpp:1361:16
      #6 0x563a1597b7b9 in wallet::(anonymous namespace)::FuzzedWallet::FundTx(FuzzedDataProvider&, CMutableTransaction) src/wallet/test/fuzz/notifications.cpp:162:15
      #7 0x563a15958240 in wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0::operator()() const src/wallet/test/fuzz/notifications.cpp:228:23
      #8 0x563a15958240 in unsigned long CallOneOf<wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_1>(FuzzedDataProvider&, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_1) src/./test/fuzz/util.h:43:27
      #9 0x563a15958240 in wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>) src/wallet/test/fuzz/notifications.cpp:196:9
      #10 0x563a15fdef0c in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
      #11 0x563a15fdef0c in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:178:5
      #12 0x563a158032a4 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19822a4) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #13 0x563a15802999 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1981999) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #14 0x563a15804586 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1983586) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #15 0x563a15804aa7 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1983aa7) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #16 0x563a157f21fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19711fb) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #17 0x563a1581c766 in main (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x199b766) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)
      #18 0x7f499e17b0cf  (/lib/x86_64-linux-gnu/libc.so.6+0x280cf) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
      #19 0x7f499e17b188 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28188) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89)
      #20 0x563a157e70c4 in _start (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19660c4) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06)

  SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow policy/feerate.cpp:29:63 in
  MS: 0 ; base unit: 0000000000000000000000000000000000000000
  0x3f,0x0,0x2f,0x5f,0x5f,0x5f,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0xff,0xff,0xff,0xff,0xff,0x53,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x0,0x0,0x0,0x0,0x13,0x5e,0x5f,0x5f,0x8,0x25,0x0,0x5f,0x5f,0x5f,0x5f,0x5f,0x5f,0x8,0x25,0xca,0x7f,0x5f,0x5f,0x5f,0x13,0x13,0x5f,0x5f,0x5f,0x2,0xdb,0xca,0x0,0x0,0xe7,0xe6,0x66,0x65,0x0,0x0,0x0,0x0,0x44,0x3f,0xa,0xa,0xff,0xff,0xff,0xff,0xff,0x61,0x76,0x6f,0x69,0x0,0xb5,0x15,
  ?\000/___}}}}}}}}}}}}}}}}}}}}\377\377\377\377\377S\377\377\377\377\377\000\000\000\000\000\000\023^__\010%\000______\010%\312\177___\023\023___\002\333\312\000\000\347\346fe\000\000\000\000D?\012\012\377\377\377\377\377avoi\000\265\025
  artifact_prefix='./'; Test unit written to ./crash-4d3bac8a64d4e58b2f0943e6d28e6e1f16328d7d
  Base64: PwAvX19ffX19fX19fX19fX19fX19fX19fX3//////1P//////wAAAAAAABNeX18IJQBfX19fX18IJcp/X19fExNfX18C28oAAOfmZmUAAAAARD8KCv//////YXZvaQC1FQ==

ACKs for top commit:
  dergoegge:
    ACK fab164f
  brunoerg:
    ACK fab164f

Tree-SHA512: f416828f4394aa7303ee437f141e9bbd23c0e0f1b830e4ef3932338858249ba68a811b9837c5b7ad8c6ab871b6354996434183597c1a910a8d8e8d829693e4b2
kevkevinpal pushed a commit that referenced this pull request Aug 1, 2024
refactor: Use util::Split to avoid a harmless unsigned-integer-overflow
fac932b 
The previous commit added a test which would fail the
unsigned-integer-overflow sanitizer. The warning is harmless and can be
triggered on any commit, since the code was introduced.

For reference, the warning would happen when the separator `-` was not
present.

For example:

  GET /rest/getutxos/6a297bfa5cb8dd976ab0207a767d6cbfaa5e876f30081127ec8674c8c52b16c0_+1.json

would result in:

rest.cpp:792:77: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'size_type' (aka 'unsigned long')
    #0 0x55ad42c16931 in rest_getutxos(std::any const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) src/rest.cpp:792:77
    #1 0x55ad4319e3c0 in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    #2 0x55ad4319e3c0 in HTTPWorkItem::operator()() src/httpserver.cpp:59:9
    #3 0x55ad431a3eea in WorkQueue<HTTPClosure>::Run() src/httpserver.cpp:114:13
    #4 0x55ad4318f961 in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) src/httpserver.cpp:403:12
    #5 0x7f078ebcbbb3  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xeabb3) (BuildId: 40b9b0d17fdeebfb57331304da2b7f85e1396ef2)
    #6 0x55ad4277e01c in asan_thread_start(void*) asan_interceptors.cpp.o
    #7 0x7f078e840a93  (/lib/x86_64-linux-gnu/libc.so.6+0x9ca93) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #8 0x7f078e8cdc3b  (/lib/x86_64-linux-gnu/libc.so.6+0x129c3b) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)

SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow rest.cpp:792:77
kevkevinpal pushed a commit that referenced this pull request Nov 12, 2024
@fanquake
Merge bitcoin#31203: fuzz: fix implicit-integer-sign-change in wall…
e001dc3 
…et_create_transaction

5a26cf7 fuzz: fix `implicit-integer-sign-change` in wallet_create_transaction (brunoerg)

Pull request description:

  This PR limites the value of `m_confirm_target` to avoid `implicit-integer-sign-change`:
  ```
  /ci_container_base/src/wallet/fees.cpp:58:58: runtime error: implicit conversion from type 'unsigned int' of value 4294967292 (32-bit, unsigned) to type 'int' changed the value to -4 (32-bit, signed)
      #0 0x55b6fd26c021 in wallet::GetMinimumFeeRate(wallet::CWallet const&, wallet::CCoinControl const&, FeeCalculation*) ci/scratch/build-x86_64-pc-linux-gnu/src/wallet/./src/wallet/fees.cpp:58:58
      #1 0x55b6fd3ef5ca in wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) ci/scratch/build-x86_64-pc-linux-gnu/src/wallet/./src/wallet/spend.cpp:1101:49
      #2 0x55b6fd3ebea5 in wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) ci/scratch/build-x86_64-pc-linux-gnu/src/wallet/./src/wallet/spend.cpp:1382:16
      #3 0x55b6fccbc154 in wallet::(anonymous namespace)::wallet_create_transaction_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/./src/wallet/test/fuzz/spend.cpp:99:11
      #4 0x55b6fccda45d in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
      #5 0x55b6fccda45d in LLVMFuzzerTestOneInput ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/util/./src/test/fuzz/fuzz.cpp:211:5
      #6 0x55b6fc368484 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c8a484) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5)
      #7 0x55b6fc367b79 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c89b79) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5)
      #8 0x55b6fc369796 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c8b796) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5)
      #9 0x55b6fc369ca7 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c8bca7) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5)
      #10 0x55b6fc35719f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c7919f) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5)
      #11 0x55b6fc381826 in main (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1ca3826) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5)
      #12 0x7f934c6661c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
      #13 0x7f934c66628a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
      #14 0x55b6fc34c184 in _start (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c6e184) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5)

  SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change /ci_container_base/src/wallet/fees.cpp:58:58
  MS: 0 ; base unit: 0000000000000000000000000000000000000000
  0x2e,0x1,0xb0,0xb8,0x0,0xff,0xff,0xff,0xff,0x60,0x14,0x22,0xff,0xff,0xff,0xff,0xff,0xfd,0xff,0xff,0xff,0xff,0xff,0x7e,0xf9,0x41,0x8,0x2b,0x17,0x58,0xb,0x17,0xfc,0xff,0xff,0xff,0xff,0xff,0xff,0x7e,0xf9,0x41,0x8,0x2b,0x17,0x58,0xb,
  .\001\260\270\000\377\377\377\377`\024\"\377\377\377\377\377\375\377\377\377\377\377~\371A\010+\027X\013\027\374\377\377\377\377\377\377~\371A\010+\027X\013
  artifact_prefix='./'; Test unit written to ./crash-5627f57ffba7568a500f8379f62c3338978b43f2
  Base64: LgGwuAD/////YBQi///////9//////9++UEIKxdYCxf8////////fvlBCCsXWAs=
  ```

ACKs for top commit:
  maflcko:
    lgtm ACK 5a26cf7
  dergoegge:
    utACK 5a26cf7

Tree-SHA512: a1b129d81d42350cf85ff6fb95cd6982b6aac88467a526ee4b3c9b3313af2f7952c5dfa9886f455756faba399d8356b6c318d7ab2d6318e08fea838bee90b2fe
kevkevinpal pushed a commit that referenced this pull request Nov 27, 2024
@luke-jr
refactor: Use spanparsing::Split to avoid a harmless unsigned-integer…
728bc25 
…-overflow

The previous commit added a test which would fail the
unsigned-integer-overflow sanitizer. The warning is harmless and can be
triggered on any commit, since the code was introduced.

For reference, the warning would happen when the separator `-` was not
present.

For example:

  GET /rest/getutxos/6a297bfa5cb8dd976ab0207a767d6cbfaa5e876f30081127ec8674c8c52b16c0_+1.json

would result in:

rest.cpp:792:77: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'size_type' (aka 'unsigned long')
    #0 0x55ad42c16931 in rest_getutxos(std::any const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) src/rest.cpp:792:77
    #1 0x55ad4319e3c0 in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    #2 0x55ad4319e3c0 in HTTPWorkItem::operator()() src/httpserver.cpp:59:9
    #3 0x55ad431a3eea in WorkQueue<HTTPClosure>::Run() src/httpserver.cpp:114:13
    #4 0x55ad4318f961 in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) src/httpserver.cpp:403:12
    #5 0x7f078ebcbbb3  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xeabb3) (BuildId: 40b9b0d17fdeebfb57331304da2b7f85e1396ef2)
    #6 0x55ad4277e01c in asan_thread_start(void*) asan_interceptors.cpp.o
    #7 0x7f078e840a93  (/lib/x86_64-linux-gnu/libc.so.6+0x9ca93) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #8 0x7f078e8cdc3b  (/lib/x86_64-linux-gnu/libc.so.6+0x129c3b) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)

SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow rest.cpp:792:77

Github-Pull: bitcoin#30444
Rebased-From: fac932b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.