Kernel internal lib #16
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When calculating a median fee for a confirmation target at a particular threshold, we analyse buckets in ranges rather than individually in case some buckets have very little data. This patch ensures the breaks between ranges are independent of the the confirmation target.
Constructs like ```cpp AssertLockNotHeld(m); LOCK(m); ``` are equivalent to ```cpp LOCK(m); ``` for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases.
Starting bitcoin-qt with non-dash ("-") arguments causes it to
silently ignore any later valid options. This change makes the
client exit with an error message if any such "loose" arguments
are encountered.
However, allow BIP-21 'bitcoin:' URIs only if no other options
follow.
clarifying when the .cookie file is generated
There is no need to have it stuck on the previous one. This is needed for the next commit.
da2679f to
bc424be
Compare
…cted to Bitcoind nodes send getaddr msgs only to outbound nodes (and ignore those received by outgoing connections). The python p2p node should mirror this behavior by not sending a getaddr message when it is not the initiator of the connection.
Co-authored-by: pablomartin4btc <pablomartin4btc@gmail.com>
-BEGIN VERIFY SCRIPT- sed -i 's|WithParams(\([a-zA-Z:._]\+\), |\1(|g' $( git grep -l WithParams ) -END VERIFY SCRIPT-
This reverts commit 057750c. It is not needed anymore in the GHA CI. This change will make the code much simpler in the following commit.
This variable is required for the `retry` script.
bc424be to
a89fe6e
Compare
In `AddTransactionsToBlock` description comment we have the asuumption that callers will never pass multiple transactions with the same txid We are asserting to assume that does not happen.
1bb79fc to
942c89b
Compare
With `queuedTx` owning the `CTransactionRef` shared ptrs, they (and the managed objects) are entirely allocated on the heap. In `DisconnectedBlockTransactions::DynamicMemoryUsage`, we account for the 2 pointers that make up the shared_ptr, but not for the managed object (CTransaction) or the control block. Prior to this commit, by calculating the `RecursiveDynamicUsage` on a `CTransaction` whenever modifying `cachedInnerUsage`, we account for the dynamic usage of the `CTransaction`, i.e. the `vins` and `vouts` vectors, but we do not account for the `CTransaction` object itself, nor for the `CTransactionRef` control block. This means prior to this commit, `DynamicMemoryUsage` underestimates dynamic memory usage by not including the `CTransaction` objects and the shared ptr control blocks. Fix this by calculating `RecursiveDynamicUsage` on the `CTransactionRef` instead of the `CTransaction` whenever modifying `cachedInnerUsage`.
This commits fixes a crash bug that can be caused with the following steps: - change to the "Transactions" view - right-click on an arbitrary transaction -> "Show transaction details" - close the transaction detail window again - select "Settings" -> "Mask values" The problem is that the list of opened dialogs, tracked in the member variable `m_opened_dialogs`, is only ever appended with newly opened transaction detail dialog pointers, but never removed. This leads to dangling pointers in the list, and if the "Mask values" menu item is selected, a crash is caused in the course of trying to close the opened transaction detail dialogs (see `closeOpenedDialogs()` method). Fix this by removing the pointer from the list if the corresponding widget is destroyed.
…, use SER_PARAMS_OPFUNC 9999019 Remove WithParams serialization helper (MarcoFalke) ffffb4a scripted-diff: Use ser params operator (MarcoFalke) fae9054 test: Use SER_PARAMS_OPFUNC in serialize_tests.cpp (MarcoFalke) Pull request description: Every serialization parameter struct already has the `SER_PARAMS_OPFUNC`, except for one in the tests. For consistency, and to remove verbose code, convert the test to `SER_PARAMS_OPFUNC`, and use it everywhere, then remove the `WithParams` helper. ACKs for top commit: ajtowns: reACK 9999019 TheCharlatan: Re-ACK 9999019 Tree-SHA512: be9cae4225a502486fe8d552aaf4b2cd2904a9f73cce9d931c6b7c757594ff1982fcc2c30d00d012cd12b0a9531fd609f8bcd7c94b811e965ac087eb8a3589d3
Having the link check in the header check loop means we get `-lminiupnpc -lminiupnpc -lminiupnpc` on the link line. This is unnecessary, and results in warnings, i.e: ```bash ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc' ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc' ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc' ``` These warnings have been occurring since the new linker released with Xcode 15, and also came up in hebasto#34.
Consolidate the lib checking logic to be the same as miniupnpc.
815eaef to
4052c2d
Compare
The zip for codesigned MacOS distribution needs to have all files have the same timestamp. These files also need to be included in the zip as zip is not automatically recursive. We use the same pattern for zip as is done for the other zip files produced by guix.
…e to SOURCE_DATE_EPOCH f6f18ee guix: Zip needs to include all files with time as SOURCE_DATE_EPOCH (Andrew Chow) Pull request description: The zip for codesigned MacOS distribution needs to have all files included and have their timestamps set to the same value (`SOURCE_DATE_EPOCH`). This uses the same pattern for zip as is done for the other zip files produced by guix. ACKs for top commit: hebasto: ACK f6f18ee. TheCharlatan: ACK f6f18ee Tree-SHA512: 569ff0d8bfe76b9b111a2454478523eeb514b44b691be8b57b61415db88356c683582550ea67ebd5fb392b4f486be170a925067b507979090535ca41cbc7351b
79539fb guix: update signapple (fanquake) Pull request description: Fixes bitcoin#28449, and removes the need to boostrap Rust, by avoiding the `python-requests` dependency. Comparing a `--no-substitutes` build of this PR, to master, signapple requires ~1350 _less_ packages to boostrap: Master derivation - https://gist.github.com/fanquake/dbf69a62c9a78b7ae8c183a160e6d58d PR derivation - https://gist.github.com/fanquake/0aa2d8eddaba861ba489ed3d936f727d ACKs for top commit: achow101: ACK 79539fb Tree-SHA512: 341ddcae27e53c31d114465cb5173573dcc9e1c0874ee160715630f686da6f69255f6080ec0181ffeffc26efbdb545599d667784b1cd17dfa7e3da0998ec9bd6
…ransaction view e26e665 gui: fix crash on selecting "Mask values" in transaction view (Sebastian Falbesoner) Pull request description: This PR fixes a crash bug that can be caused with the following steps: - change to the "Transactions" view - right-click on an arbitrary transaction -> "Show transaction details" - close the transaction detail window again - select menu item "Settings" -> "Mask values" The problem is that the list of opened dialogs, tracked in the member variable `m_opened_dialogs` (introduced in bitcoin-core/gui#708, commit 4492de1), is only ever appended with newly opened transaction detail dialog pointers, but never removed. This leads to dangling pointers in the list, and if the "Mask values" menu item is selected, a crash is caused in the course of trying to close the opened transaction detail dialogs (see `closeOpenedDialogs()` method). Fix this by removing a pointer of the list if the corresponding widget is destroyed. ACKs for top commit: achow101: ACK e26e665 pablomartin4btc: tACK e26e665 furszy: utACK e26e665 hebasto: ACK e26e665, tested on Ubuntu 22.04. Tree-SHA512: 37885c22abae0ab065b4878bae46fd362f41b09609d081fd59e26bb05474f427b98771ee73f5480526afaef04e016c5ba62c956e0e85a57b6a0f44a905b68a83
b74e449 build: remove potential for duplciate natpmp linking (fanquake) 4e95096 build: remove duplicate -lminiupnpc linking (fanquake) Pull request description: Having the link check in the header check loop means we get `-lminiupnpc -lminiupnpc -lminiupnpc` on the link line. This is unnecessary, and results in warnings, i.e: ```bash ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc' ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc' ld: warning: ignoring duplicate libraries: '-levent', '-lminiupnpc' ``` These warnings have been occurring since the new macOS linker released with Xcode 15, and also came up in hebasto#34. There are other duplicate lib issues, i.e with `-levent` + `-levent_pthreads -levent`, but those are less straight forward to solve, and won't be included here. ACKs for top commit: jonatack: ACK b74e449 hebasto: ACK b74e449, it fixes one issue mentioned in hebasto#34 (comment). TheCharlatan: ACK b74e449 theuni: ACK b74e449 Tree-SHA512: 987a56ef17cbaf273cb672c41016f3f615b16889317325a9e88135d0c41f01af3840ad44a6f811a7df97f5873c9cd957e60aaa1b99bd408b17b4b1ffe2c68f36
02a4f1a addrman: log AS only when using asmap (brunoerg) Pull request description: This PR changes the log to just print the ASN when using asmap, same logic presented in other logs: https://github.com/bitcoin/bitcoin/blob/afa081a39bde77d3959aa35b6e6c75a2fe988d68/src/net_processing.cpp#L3552-L3556 https://github.com/bitcoin/bitcoin/blob/afa081a39bde77d3959aa35b6e6c75a2fe988d68/src/net_processing.cpp#L3598-L3604 ACKs for top commit: naumenkogs: ACK 02a4f1a mzumsande: Code Review ACK 02a4f1a Tree-SHA512: adad5904ab163660d47554b32dc2dc3dfdff8dd64b94e5320ad11706381264d1e338654fa8239430eed4ccbebc8f6670698b4278895794055c37fc4bcefe71bc
…ng connections 9cfc1c9 test: check that we don't send a getaddr msg to an inbound peer (Martin Zumsande) 88c33c6 test: make python p2p not send getaddr messages when it's being connected to (Martin Zumsande) Pull request description: `bitcoind` nodes send `getaddr` messages only to outbound nodes (and ignore `getaddr` received by outgoing connections). The python p2p node should mirror this behavior by not sending a `getaddr` message when it is not the initiator of the connection. This is currently causing several unnecessary messages being sent and then ignored (`Ignoring "getaddr" from outbound-full-relay connection.`) in tests like `p2p_add_connections.py`. ACKs for top commit: pinheadmz: concept ACK 9cfc1c9 pablomartin4btc: re ACK 9cfc1c9 BrandonOdiwuor: re ACK 9cfc1c9 Tree-SHA512: 812bec5d8a4828b4384d4cdd4362d6eec09acb2363e888f2b3e3bf8b925e0e17f15e13dc297d6b616c68b93ace9ede7245b07b405d3f5f8eada98350f74230dc
…ncy from validation.cpp fa7d319 refactor: Remove unused circular include dependency from validation.cpp (MarcoFalke) Pull request description: Also, sort includes ACKs for top commit: ismaelsadeeq: ACK fa7d319 hebasto: ACK fa7d319 TheCharlatan: ACK fa7d319 Tree-SHA512: ea5e0001644d70ecfbccf87e27b393786a0eda79af4923ff68a0096d4d5b910cf6eeed8667ecbf55f3a164f500d3f5aeaf9d81bb190296c30ce0cc93c165717d
8047bb6 build: Update `qt` package up to 5.15.11 (Hennadii Stepanov) Pull request description: In the light of bitcoin#28622, we probably have to patch Qt. It seems reasonable to update it up to the latest available version before doing that. ACKs for top commit: TheCharlatan: ACK 8047bb6 Tree-SHA512: b4d7df2ff059b8f58c3202d913237c0d39a962748658f1ce853884dca095fbda5f56d4d68f73a1bc8da2f295e96a20927306e148b41a9f4afc42c8edb11c3729
…write followups 9b3da70 [test] DisconnectedBlockTransactions::DynamicMemoryUsage (glozow) b2d0447 bugfix: correct DisconnectedBlockTransactions memory usage (stickies-v) f4254e2 assume duplicate transactions are not added to `iters_by_txid` (ismaelsadeeq) 29eb219 move only: move implementation code to disconnected_transactions.cpp (ismaelsadeeq) 81dfedd refactor: update `MAX_DISCONNECTED_TX_POOL` from kb to bytes (ismaelsadeeq) Pull request description: This PR is a follow-up to fix review comments and a bugfix from bitcoin#28385 The PR - Updated `DisconnectedBlockTransactions`'s `MAX_DISCONNECTED_TX_POOL` from kb to bytes. - Moved `DisconnectedBlockTransactions` implementation code to `kernel/disconnected_transactions.cpp`. - `AddTransactionsFromBlock` now assume duplicate transactions are not passed by asserting after inserting each transaction to `iters_by_txid`. - Included a Bug fix: In the current master we are underestimating the memory usage of `DisconnectedBlockTransactions`. * When adding and subtracting `cachedInnerUsage` we call `RecursiveDynamicUsage` with `CTransaction` which invokes this [`RecursiveDynamicUsage(const CTransaction& tx)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L32) version of `RecursiveDynamicUsage`, the output of that call only account for the memory usage of the inputs and outputs of the `CTransaction`, this omits the memory usage of the `CTransaction` object and the control block. * This PR fixes this bug by calling `RecursiveDynamicUsage` with `CTransactionRef` when adding and subtracting `cachedInnerUsage` which invokes [`RecursiveDynamicUsage(const std::shared_ptr<X>& p)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L67) version of `RecursiveDynamicUsage` the output of the calculation accounts for the` CTransaction` object, the control blocks, inputs and outputs memory usage. * see [comment ](bitcoin#28385 (comment)) - Added test for DisconnectedBlockTransactions memory limit. ACKs for top commit: stickies-v: ACK 9b3da70 - nice work! BrandonOdiwuor: re ACK 9b3da70 glozow: ACK 9b3da70 Tree-SHA512: 69b9595d09f4d0209038f97081d790cea92ccf63efb94e9e372749979fcbe527f7f17a8e454720cedd12021be0c8e11cf99874625d3dafd9ec602b12dbeb4098
a5e39d3 Fee estimation: extend bucket ranges consistently (Anthony Towns) Pull request description: When calculating a median fee for a confirmation target at a particular threshold, we analyse buckets in ranges rather than individually in case some buckets have very little data. This patch ensures the breaks between ranges are independent of the the confirmation target. Fixes bitcoin#20725 ACKs for top commit: ismaelsadeeq: Code review ACK a5e39d3 glozow: btw what I meant by [this](bitcoin#21161 (review)) was ACK a5e39d3 jonatack: Initial ACK a5e39d3 Tree-SHA512: 0edf4e56717c4ab8d4ab0bc0f1d7ab36a13b99de12f689e55c9142c6b81691367ffd8df2e8260c5e14335310b1a51770c6c22995db31109976239befcb558ef8
Move util library files that are not used by the kernel library to the common library. This is consistent with the library structure laid out in doc/design/libraries.md. Moving util/message.cpp to the common library also breaks an undocumented dependency of the util library on the consensus library's CPubKey::RecoverCompact symbol.
The crypto library should not have any dependencies on the util library. With this move the cleanse file can be removed from the explicit libbitcoinconsensus dependency list, since it is now linked in through the crypto library.
The files should not be duplicated between the consensus and util libraries. Instead split off the part that is required by the consensus library into a new file that is compiled as part of the crypto library.
See doc/design/libraries.md for an overview of the internal libraries.
The external kernel library should use the same file list that the internal kernel library uses.
4052c2d to
1a58933
Compare
sedited
pushed a commit
that referenced
this pull request
Nov 30, 2023
…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
sedited
pushed a commit
that referenced
this pull request
Jan 31, 2024
…when no change pos d55fdb1 Move TRACEx parameters to seperate lines (Richard Myers) 2d58629 wallet: fix coin selection tracing to return -1 when no change pos (Richard Myers) Pull request description: This is a bugfix for from when [optional was introduced](bitcoin@758501b) for `change_pos` in the wallet. When optional `change_pos` is unset, we should return -1 and not 0. I added two new checks to the `test/functional/interface_usdt_coinselection.py` which adds coverage for the situations when `normal_create_tx_internal` and `aps_create_tx_internal` events occur with no change. You can reproduce this bug using the coin-selection-simulation scripts as described in [issue #16](achow101/coin-selection-simulation#16). You can also run the `interface_usdt_coinselection.py` test without the changes to `wallet/spend.cpp`. ACKs for top commit: 0xB10C: ACK d55fdb1 achow101: ACK d55fdb1 murchandamus: ACK d55fdb1 Tree-SHA512: 6efac3b756bdf51debbcb759dc3c4b7a4304626bc047b70025cec02f3a04937ace7712e9558ac71e560fd136005a98c518ac5bb4b90c3282d776beccd0de9749
sedited
pushed a commit
that referenced
this pull request
Jul 6, 2025
Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and:
```bash
export CC=clang
export CXX=clang++
cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address
cmake --build build
export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan"
ctest --test-dir build
```
```bash
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms
********* Finished testing of AddressBookTests *********
=================================================================
==21869==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 88 byte(s) in 1 object(s) allocated from:
#0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
#1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
#2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
#3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18
#4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5
#5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75
#6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13
#7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5
#8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5
#9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
#10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
#11 0xffff8cf57c54 (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#12 0xffff8cf5fa18 (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#13 0xffff8cf6067c (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#14 0xffff8cf610a4 (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30
#18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
#19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
#20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
```
This happens when building using depends:
```bash
Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#1 0xfbff97f8c164 (<unknown module>)
#2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o
#15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s).
```
sedited
pushed a commit
that referenced
this pull request
Jul 6, 2025
5be31b2 lsan: add more Qt suppressions (fanquake) Pull request description: Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and: ```bash export CC=clang export CXX=clang++ cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address cmake --build build export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan" ctest --test-dir build ``` ```bash Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms ********* Finished testing of AddressBookTests ********* ================================================================= ==21869==ERROR: LeakSanitizer: detected memory leaks Direct leak of 88 byte(s) in 1 object(s) allocated from: #0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) #1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18 #4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5 #5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75 #6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13 #7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5 #8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5 #9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) #10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) #11 0xffff8cf57c54 (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #12 0xffff8cf5fa18 (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #13 0xffff8cf6067c (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #14 0xffff8cf610a4 (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30 #18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) #19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) #20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) ``` This happens when building using depends: ```bash Indirect leak of 24 byte(s) in 1 object(s) allocated from: #0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #1 0xfbff97f8c164 (<unknown module>) #2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o #15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s). ``` ACKs for top commit: maflcko: lgtm ACK 5be31b2 Tree-SHA512: 0c33661c7ec83ea9b874c1ee4ee2de513131690287363e216a88560dfb31a59ef563a50af756c86a991583aa64a600a74e20fd5d6a104cf4c0a27532de8d2211
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Patches needed for adding the libbitcoinkernel to our internal tree of libraries.