aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-07-22Merge bitcoin/bitcoin#30494: fuzz: reduce keypool size in `scriptpubkeyman` ↵merge-script
target dcb4ec944984961e3b40452425a219e15e6ab508 fuzz: reduce keypool size in scriptpubkeyman target (brunoerg) Pull request description: Fixes #30476 This PR reduces keypool size in scriptpubkeyman fuzz target to avoid spend a lot of time in `TopUp` (which is obviously called by many spkm functions). For reference: This PR: ``` INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 1845055748 INFO: Loaded 1 modules (1225616 inline 8-bit counters): 1225616 [0x106346fe0, 0x106472370), INFO: Loaded 1 PC tables (1225616 PCs): 1225616 [0x106472370,0x107725c70), ./src/test/fuzz/fuzz: Running 1 inputs 10 time(s) each. Running: ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50 Executed ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50 in 250 ms ``` Master: ``` INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 2004906948 INFO: Loaded 1 modules (1225603 inline 8-bit counters): 1225603 [0x104196f80, 0x1042c2303), INFO: Loaded 1 PC tables (1225603 PCs): 1225603 [0x1042c2308,0x105575b38), ./src/test/fuzz/fuzz: Running 1 inputs 10 time(s) each. Running: ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50 Executed ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50 in 21016 ms ``` ACKs for top commit: maflcko: review ACK dcb4ec944984961e3b40452425a219e15e6ab508 dergoegge: utACK dcb4ec944984961e3b40452425a219e15e6ab508 Tree-SHA512: d818b228d5f1dd0d5c665d8e54cf5dd8e378604039eaac114fc34366ece4420b9b2519d898f2dc2410960b873f0b91bbad4a534a35658477aed6ef48f3458137
2024-07-22Fix lint-spelling warningsLőrinc
These warnings were often polluting the CI output, e.g. https://github.com/bitcoin/bitcoin/pull/30499/checks?check_run_id=27745036545 > ./test/lint/lint-spelling.py before the change: ``` doc/design/libraries.md:100: targetted ==> targeted doc/developer-notes.md:495: dependant ==> dependent src/bench/sign_transaction.cpp:49: hashIn ==> hashing, hash in src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in src/coins.cpp:24: viewIn ==> viewing, view in src/coins.cpp:24: viewIn ==> viewing, view in src/coins.cpp:29: viewIn ==> viewing, view in src/coins.cpp:29: viewIn ==> viewing, view in src/coins.h:44: outIn ==> outing, out in src/coins.h:44: outIn ==> outing, out in src/coins.h:45: outIn ==> outing, out in src/coins.h:45: outIn ==> outing, out in src/coins.h:215: viewIn ==> viewing, view in src/coins.h:220: viewIn ==> viewing, view in src/primitives/transaction.h:37: hashIn ==> hashing, hash in src/primitives/transaction.h:37: hashIn ==> hashing, hash in src/protocol.cpp:51: hashIn ==> hashing, hash in src/protocol.cpp:51: hashIn ==> hashing, hash in src/protocol.h:497: hashIn ==> hashing, hash in src/qt/forms/optionsdialog.ui:344: incomin ==> incoming src/qt/optionsdialog.cpp:445: proxys ==> proxies src/rpc/mining.cpp:987: hashIn ==> hashing, hash in src/rpc/mining.cpp:987: hashIn ==> hashing, hash in src/script/interpreter.h:298: amountIn ==> amounting, amount in src/script/interpreter.h:298: amountIn ==> amounting, amount in src/script/interpreter.h:299: amountIn ==> amounting, amount in src/script/interpreter.h:299: amountIn ==> amounting, amount in src/script/sigcache.h:70: amountIn ==> amounting, amount in src/script/sigcache.h:70: amountIn ==> amounting, amount in src/signet.cpp:144: amountIn ==> amounting, amount in src/test/fuzz/util/net.cpp:386: occured ==> occurred src/test/fuzz/util/net.cpp:398: occured ==> occurred src/util/vecdeque.h:79: deques ==> dequeues src/util/vecdeque.h:160: deques ==> dequeues src/util/vecdeque.h:184: deques ==> dequeues src/util/vecdeque.h:194: deques ==> dequeues src/validation.cpp:2130: re-declared ==> redeclared src/validation.h:348: outIn ==> outing, out in src/validation.h:349: outIn ==> outing, out in test/functional/wallet_bumpfee.py:851: atleast ==> at least ```
2024-07-22Merge bitcoin/bitcoin#30473: fuzz: Limit parse_univalue input lengthmerge-script
fa80b16b20dffcb85b80f75fee64ed333f2062f9 fuzz: Limit parse_univalue input length (MarcoFalke) Pull request description: The new limit should be more than enough, and hopefully avoids fuzz input bloat, such as `parse_univalue/0426365704e09ddd704a058cc2add1cbf104c1a9`. C.f. https://cirrus-ci.com/task/6178647134961664?logs=ci#L3805 ``` Run parse_univalue with args ['/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/parse_univalue')]INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 572704560 INFO: Loaded 1 modules (623498 inline 8-bit counters): 623498 [0x561cba23a518, 0x561cba2d28a2), INFO: Loaded 1 PC tables (623498 PCs): 623498 [0x561cba2d28a8,0x561cbac56148), INFO: 3224 files found in /ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/parse_univalue INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1048576 bytes INFO: seed corpus: files: 3224 min: 1b max: 1050370b total: 25114084b rss: 112Mb #1024pulse cov: 10458 ft: 33444 corp: 906/32Kb exec/s: 341 rss: 154Mb #2048pulse cov: 12081 ft: 55461 corp: 1668/192Kb exec/s: 227 rss: 228Mb Slowest unit: 15 s: artifact_prefix='./'; Test unit written to ./slow-unit-9df6997f2f4726843e82b3dfde46862599904d56 Slowest unit: 309 s: artifact_prefix='./'; Test unit written to ./slow-unit-0426365704e09ddd704a058cc2add1cbf104c1a9 #3226INITED cov: 12246 ft: 66944 corp: 2358/3510Kb exec/s: 6 rss: 1610Mb #3226DONE cov: 12246 ft: 66944 corp: 2358/3510Kb lim: 282379 exec/s: 6 rss: 1610Mb Done 3226 runs in 477 second(s) ACKs for top commit: dergoegge: utACK fa80b16b20dffcb85b80f75fee64ed333f2062f9 brunoerg: utACK fa80b16b20dffcb85b80f75fee64ed333f2062f9 Tree-SHA512: b2ffbaaa4876be61be0e6c975ab65a842562d14079a13836202f8b5b5ef75e068e73df75c9bcc702379e123fcdb1dcd66951e31533fb4aaa6afe17dff160f7d0
2024-07-20fuzz: reduce keypool size in scriptpubkeyman targetbrunoerg
2024-07-19Fix MSVC warning C4273 "inconsistent dll linkage"Hennadii Stepanov
When using CMake, the user can select the MSVC runtime library to be: 1) Statically-linked (with the corresponding `x64-windows-static` vcpkg triplet) or 2) Dynamically-linked (with the corresponding `x64-windows` vcpkg triplet) In the latter case, the compiler emits the C4273 warning. As the "Necessary on some platforms" comment does not apply to MSVC, skip the declaration for MSVC.
2024-07-19fuzz: Deglobalize signature cache in sigcache testTheCharlatan
The body of the fuzz test should ideally be a pure function. If data is persisted in the cache over many iterations, and there is a crash, reproducing it from the input might be difficult.
2024-07-19fuzz: Limit parse_univalue input lengthMarcoFalke
2024-07-19log: Remove NOLINT(bitcoin-unterminated-logprintf)MarcoFalke
2024-07-19fuzz: Use BasicTestingSetup for coins_view targetTheCharlatan
2024-07-19test: Add arguments for creating a slimmer setupTheCharlatan
Adds more testing options for creating an environment without networking and a validation interface. This is useful for improving the performance of the utxo snapshot fuzz test, which constructs a new TestingSetup on each iteration.
2024-07-19logging: use std::string_viewAnthony Towns
2024-07-19logging: Apply formatting to early log messagesAnthony Towns
The formatting of log messages isn't defined until StartLogging() is called; so can't be correctly applied to early log messages from prior to that call. Instead of saving the output log message, save the inputs to the logging invocation (including time, mocktime and thread name), and format those inputs into a log message when StartLogging() is called.
2024-07-19logging: Limit early logging bufferAnthony Towns
Log messages created prior to StartLogging() being called go into a buffer. Enforce a limit on the size of this buffer.
2024-07-18Merge bitcoin/bitcoin#30245: net: Allow -proxy=[::1] on nodes with IPV6 lo onlyAva Chow
23333b7ed243071c9b4e4f04c727556d8065acbb net: Allow DNS lookups on nodes with IPV6 lo only (Max Edwards) Pull request description: This is similar to (but does not fix) https://github.com/bitcoin/bitcoin/issues/13155 which I believe is the same issue but in libevent. The issue is on a host that has IPV6 enabled but only a loopback IP address `-proxy=[::1]` will fail as `[::1]` is not considered valid by `getaddrinfo` with `AI_ADDRCONFIG` flag. I think the loopback interface should be considered valid and we have a functional test that will try to test this: `feature_proxy.py`. To replicate the issue, run `feature_proxy.py` inside a docker container that has IPV6 loopback ::1 address without specifically giving that container an external IPV6 address. This should be the default with recent versions of docker. IPV6 on loopback interface was enabled in docker engine 26 and later ([https://docs.docker.com/engine/release-notes/26.0/#bug-fixes-and-enhancements-2](https://docs.docker.com/engine/release-notes/26.0/#bug-fixes-and-enhancements-2)). `AI_ADDRCONFIG` was introduced to prevent slow DNS lookups on systems that were IPV4 only. References: Man section on `AI_ADDRCONFIG`: ``` If hints.ai_flags includes the AI_ADDRCONFIG flag, then IPv4 addresses are returned in the list pointed to by res only if the local system has at least one IPv4 address configured, and IPv6 addresses are returned only if the local system has at least one IPv6 address configured. The loopback address is not considered for this case as valid as a configured address. This flag is useful on, for ex‐ ample, IPv4-only systems, to ensure that getaddrinfo() does not return IPv6 socket addresses that would always fail in connect(2) or bind(2). ``` [AI_ADDRCONFIG considered harmful Wiki entry by Fedora](https://fedoraproject.org/wiki/QA/Networking/NameResolution/ADDRCONFIG) [Mozilla discussing slow DNS without AI_ADDRCONFIG and also localhost issues with it](https://bugzilla.mozilla.org/show_bug.cgi?id=467497) ACKs for top commit: achow101: ACK 23333b7ed243071c9b4e4f04c727556d8065acbb tdb3: ACK 23333b7ed243071c9b4e4f04c727556d8065acbb pinheadmz: ACK 23333b7ed243071c9b4e4f04c727556d8065acbb Tree-SHA512: 5ecd8c72d1e1c28e3ebff07346381d74eaddef98dca830f6d3dbf098380562fa68847d053c0d84cc8ed19a45148ceb5fb244e4820cf63dccb10ab3db53175020
2024-07-18Merge bitcoin/bitcoin#30320: assumeutxo: Don't load a snapshot if it's not ↵Ava Chow
in the best header chain 55b6d7be68a6f6c3882588ffd5b9349d885ed953 validation: Don't load a snapshot if it's not in the best header chain. (Martin Zumsande) Pull request description: This was suggested by me in the discussion of #30288, which has more context. If the snapshot is not an ancestor of the most-work header (`m_best_header`), syncing from that alternative chain leading to `m_best_header` should be prioritised. Therefore it's not useful loading the snapshot in this situation. If the other chain turns out to be invalid or the chain with the snapshot retrieves additional headers so that it's the most-work one again (see functional test), `m_best_header` will change and loading the snapshot will be possible again. Because of the work required to generate a conflicting headers chain, a situation with two conflicting chains should only be possible under extreme circumstances, such as major forks. ACKs for top commit: fjahr: re-ACK 55b6d7be68a6f6c3882588ffd5b9349d885ed953 achow101: ACK 55b6d7be68a6f6c3882588ffd5b9349d885ed953 alfonsoromanz: Re ACK 55b6d7be68a6f6c3882588ffd5b9349d885ed953 Tree-SHA512: 4fbea5ab1038ae353fc949a186041cf9b397e7ce4ac59ff36f881c9437b4f22ada922490ead5b2661389eb1ca0f3d1e7e7e6a4261057678643e71594a691ac36
2024-07-18Merge bitcoin/bitcoin#30444: rest: Reject negative outpoint index early in ↵Ava Chow
getutxos parsing fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 refactor: Use util::Split to avoid a harmless unsigned-integer-overflow (MarcoFalke) fab54db9f1d0e634f4a697480dbb87b87940dc5c rest: Reject negative outpoint index in getutxos parsing (MarcoFalke) Pull request description: In `rest_getutxos` outpoint indexes such as `+N` or `-N` are accepted. This should be harmless, because any index out of range should be treated as a non-existent utxo. However, a negative index can't exist ever, so it seems better to reject all signs, whether `+` or `-`. ACKs for top commit: achow101: ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 hodlinator: ut-ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 tdb3: re ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 danielabrozzoni: ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 brunoerg: reACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1 Tree-SHA512: 8f1a75248cb61e1c4beceded6ed170db83b07f30fbcf93a26acfffc00ec4546572366eff87907a7e1423d7d3a2a9e57a0a7a9bacb787c86463f842d7161c16bc
2024-07-18Merge bitcoin/bitcoin#30356: refactor: add coinbase constraints to ↵Ryan Ofsky
BlockAssembler::Options c504b6997b1acc9771ad1f52efaa4be2b4966c6c refactor: add coinbase constraints to BlockCreateOptions (Sjors Provoost) 6b4c817d4b978adf69738677c74855ef0675f333 refactor: pass BlockCreateOptions to createNewBlock (Sjors Provoost) 323cfed5959b25c98235ec988b408fc5e3391e3c refactor: use CHECK_NONFATAL to avoid single-use symbol (Sjors Provoost) Pull request description: When generating a block template through e.g. getblocktemplate RPC, we reserve 4000 weight units and 400 sigops. Pools use this space for their coinbase outputs. At least one pool patched their Bitcoin Core node to adjust these hardcoded values. They eventually [produced an invalid block](https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426) which exceeded the sigops limit. The existince of such patches suggests it may be useful to make this value configurable. This PR would make such a change easier. However, the main motivation is that in the Stratum v2 spec requires the pool to communicate the maximum bytes they intend to add to the coinbase outputs. Specifically the `CoinbaseOutputDataSize` message which is part of the [Template Distribution Protocol](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---server) has a field `coinbase_output_max_additional_size`. A proposed change to the spec adds the max additional sigops as well: https://github.com/stratum-mining/sv2-spec/pull/86. Whether that change makes it into the spec is not important though, as adding both to `BlockAssembler::Options` makes sense. The first commit is a test refactor followup for #30335, related to the code that's changed here, but not required. The second commit introduces BlockCreateOptions, with just `use_mempool`. The thirds commit adds `coinbase_max_additional_weight` and `coinbase_output_max_additional_sigops` to `BlockCreateOptions`. They use the originally hardcoded values, and no existing caller overrides these defaults. This changes in #29432. ACKs for top commit: itornaza: tested ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c ryanofsky: Code review ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c ismaelsadeeq: Code review ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c Tree-SHA512: de2fa085f47048c91d95524e03f909f6f27f175c1fefa3d6106445e7eb5cf5b710eda6ea5b641cf3b4704a4e4e0181a0c829003b9fd35465f2a46167e5d64487
2024-07-18Merge bitcoin/bitcoin#30464: test, refactor: Fix MSVC warning C4101 ↵merge-script
"unreferenced local variable" 44f08786f435ed4284d39dc604c2a5fcbde9e602 test: Fix MSVC warning C4101 "unreferenced local variable" (Hennadii Stepanov) 5d25a82b9a5e54f74cc066599541bc1d3da70988 univalue, refactor: Convert indentation tabs to spaces (Hennadii Stepanov) Pull request description: This PR is split from https://github.com/bitcoin/bitcoin/pull/30454 and addresses MSVC warning [C4101](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4101) "unreferenced local variable". The current MSVC build system in the master branch skips building univalue tests, so it is not affected. No behaviour changes. ACKs for top commit: kevkevinpal: utACK [44f0878](https://github.com/bitcoin/bitcoin/pull/30464/commits/44f08786f435ed4284d39dc604c2a5fcbde9e602) maflcko: ACK 44f08786f435ed4284d39dc604c2a5fcbde9e602 theuni: trivial ACK 44f08786f435ed4284d39dc604c2a5fcbde9e602. Tree-SHA512: 661d3b40ddb4f7915de7a65ccb27a24da88ae499ce03c036099007260b0597e83738f1a3a420985b51f798ee309ade32988c6d78f4ffed401099b175a0b2025b
2024-07-18[fees]: change `estimatesmartfee` default mode to `economical`ismaelsadeeq
2024-07-17Merge bitcoin/bitcoin#29523: Wallet: Add `max_tx_weight` to transaction ↵Ava Chow
funding options (take 2) 734076c6de1781f957c8bc3bf7ed6951920cfcf6 [wallet, rpc]: add `max_tx_weight` to tx funding options (ismaelsadeeq) b6fc5043c16c2467a2a6768a6ca9b18035fc400f [wallet]: update the data type of `change_output_size`, `change_spend_size` and `tx_noinputs_size` to `int` (ismaelsadeeq) baab0d2d43049a71dc90176bc4d72062f7b2ce19 [doc]: update reason for deducting change output weight (ismaelsadeeq) 7f61d31a5cec8fc61328bee43f90d3f1dcb0a035 [refactor]: update coin selection algorithms input parameter `max_weight` name (ismaelsadeeq) Pull request description: This PR taken over from #29264 The PR added an option `max_tx_weight` to transaction funding RPC's that ensures the resulting transaction weight does not exceed the specified `max_tx_weight` limit. If `max_tx_weight` is not given `MAX_STANDARD_TX_WEIGHT` is used as the max threshold. This PR addressed outstanding review comments in #29264 For more context and rationale behind this PR see https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418/11?u=instagibbs ACKs for top commit: achow101: ACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6 furszy: utACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6 rkrux: reACK [734076c](https://github.com/bitcoin/bitcoin/pull/29523/commits/734076c6de1781f957c8bc3bf7ed6951920cfcf6) Tree-SHA512: 013501aa443d239ee2ac01bccfc5296490c27b4edebe5cfca6b96c842375e895e5cfeb5424e82e359be581460f8be92095855763a62779a18ccd5bdfdd7ddce7
2024-07-17refactor: add coinbase constraints to BlockCreateOptionsSjors Provoost
When generating a block template through e.g. getblocktemplate RPC, we reserve 4000 weight units and 400 sigops. Pools use this space for their coinbase outputs. At least one pool patched their Bitcoin Core node to adjust these hardcoded values. They eventually produced an invalid block which exceeded the sigops limit. https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426 The existince of such patches suggests it may be useful to make this value configurable. This commit would make such a change easier. The main motivation however is that the Stratum v2 spec requires the pool to communicate the maximum bytes they intend to add to the coinbase outputs. A proposed change to the spec would also require them to communicate the maximum number of sigops. This commit also documents what happens when -blockmaxweight is lower than the coinbase reserved value. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-17Merge bitcoin/bitcoin#28893: Fix SSE4.1-related issuesmerge-script
d440f13db02c82c842000abe4fe4d0c721a4ad3b crypto: Guard code with `ENABLE_SSE41` macro (Hennadii Stepanov) 6ec1ca7c85a4009b77e149a798a331592b96ea42 build: Fix test for SSE4.1 intrinsics (Hennadii Stepanov) Pull request description: 1. Fix the test for SSE4.1 intrinsics during build system configuration, which currently can be false positive, for example, when `CXXFLAGS="-mno-sse4.1"` provided. This PR fixes the test by adding the `_mm_blend_epi16` SSE4.1 function used in our codebase. 2. Guard `sha_x86_shani.cpp` code with `ENABLE_SSE41` macro as it uses the `_mm_blend_epi16` function from the SSE4.1 instruction set. It is possible that SHA-NI is enabled even when SSE4.1 is disabled, which causes compile errors in the master branch. Closes https://github.com/bitcoin/bitcoin/issues/28864. ACKs for top commit: sipa: utACK d440f13db02c82c842000abe4fe4d0c721a4ad3b willcl-ark: tACK d440f13db02c82c842000abe4fe4d0c721a4ad3b theuni: utACK d440f13db02c82c842000abe4fe4d0c721a4ad3b Tree-SHA512: a6e1e8c94e1b94874ff51846815ef445e6135cbdb01b08eb695b3548115f2340dd835ebe53673ae46a553fe6be4815e68d8642c34235dd7af5106c4b7c9ea6f3
2024-07-17Merge bitcoin/bitcoin#30466: refactor: Make m_last_notified_header privatemerge-script
fa927055dd43dda945396574273a210186beec9f refactor: Make m_last_notified_header private (MarcoFalke) Pull request description: Seems brittle to expose mutable fields public. Fix it by making it private. Fixes https://github.com/bitcoin/bitcoin/pull/30425#discussion_r1677633601 ACKs for top commit: dergoegge: utACK fa927055dd43dda945396574273a210186beec9f Tree-SHA512: d9841c42571144ced0edeaa4bb1d96a177a011dca37c8342c66513477c37278602a1b88beb93068b94fc4443b1552c8fc9f98bcf0bda7d0fc101e61e90c33944
2024-07-17Merge bitcoin/bitcoin#30457: doc: getaddressinfo[isscript] is optionalmerge-script
fa6390df205513319f28e35e3e17c40ecaa7d731 doc: getaddressinfo[isscript] is optional (MarcoFalke) Pull request description: `isscript` is unknown for unknown witness versions, so it should be marked optional in the docs Fixes https://github.com/bitcoin/bitcoin/issues/30456 ACKs for top commit: stickies-v: ACK fa6390df205513319f28e35e3e17c40ecaa7d731 tdb3: ACK fa6390df205513319f28e35e3e17c40ecaa7d731 Tree-SHA512: f728f18e0871923225e0bf29594f8095997456cf55409f42087b5f70f95bef10f984323b48d2b484b6705f23b04e9e8a3fe42446830638fdd70453c18fd7f189
2024-07-17refactor: Use util::Split to avoid a harmless unsigned-integer-overflowMarcoFalke
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
2024-07-17refactor: Make m_last_notified_header privateMarcoFalke
2024-07-17doc: getaddressinfo[isscript] is optionalMarcoFalke
2024-07-16test: Fix MSVC warning C4101 "unreferenced local variable"Hennadii Stepanov
2024-07-16univalue, refactor: Convert indentation tabs to spacesHennadii Stepanov
2024-07-16Merge bitcoin/bitcoin#30357: Fix cases of calls to `FillPSBT` errantly ↵Ava Chow
returning `complete=true` 7e36dca657c66bc70b04d5b850e5a335aecfb902 test: add test for modififed walletprocesspsbt calls (willcl-ark) 39cea21ec51b9838669c38fefa14f25c36ae7096 wallet: fix FillPSBT errantly showing as complete (willcl-ark) Pull request description: Fixes: #30077 Fix cases of calls to `FillPSBT` returning `complete=true` when it's not the case. This can happen when some inputs have been signed but the transaction is subsequently modified, e.g. in the context of PayJoins. Also fixes a related bug where a finalized hex string is attempted to be added during `walletprocesspsbt` but a CHECK_NONFATAL causes an abort. ACKs for top commit: achow101: ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902 ismaelsadeeq: Tested ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902 pinheadmz: re-ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902 Tree-SHA512: e35d19789899c543866d86d513506494d672e4bed9aa36a995dbec4e72f0a8ec5536b57c4a940a18002ae4a8efd0b007c77ba64e57cd52af98e4ac0e7bf650d6
2024-07-16Merge bitcoin/bitcoin#22729: Make it possible to disable Tor binds and abort ↵Ava Chow
startup on bind failure bca346a97056748f1e3fb899f336d56d9fd45a64 net: require P2P binds to succeed (Vasil Dimov) af552534ab83c572d3bc3f93ccaee5c1961ccab5 net: report an error if unable to bind on the Tor listening addr:port (Vasil Dimov) 9a7e5f4d68dadc64a675f32d1e91199d6b1aa095 net: don't extra bind for Tor if binds are restricted (Vasil Dimov) Pull request description: Make it possible to disable the Tor binding on `127.0.0.1:8334` and stop startup if any P2P bind fails instead of "if all P2P binds fail". Fixes https://github.com/bitcoin/bitcoin/issues/22726 Fixes https://github.com/bitcoin/bitcoin/issues/22727 ACKs for top commit: achow101: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 cbergqvist: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 pinheadmz: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 Tree-SHA512: fabef89a957191eea4f3e3b6109d2b8389f27ecc74440a920b0c10f31fff00a85bcfd1eb3c91826c7169c618f4de8a8d0a260e2caf40fd854f07ea9a980d8603
2024-07-16Merge bitcoin/bitcoin#28923: script/sign: avoid duplicated signature ↵Ava Chow
verification after signing (+introduce signing benchmarks) fe92c15f0c44d1405b9048306736bd0eae868506 script/sign: avoid duplicated signature verification after signing (Sebastian Falbesoner) 080089567ca766d4c1cde8ec0e5c7df48f566e07 bench: add benchmark for `SignTransaction` (Sebastian Falbesoner) Pull request description: This PR is a small performance improvement on the `SignTransaction` function, which is used mostly by the wallet (obviously) and the `signrawtransactionwithkey` RPC. The lower-level function `ProduceSignature` already calls `VerifyScript` internally as last step in order to check whether the signature data is complete: https://github.com/bitcoin/bitcoin/blob/daa56f7f665183bcce3df146f143be37f33c123e/src/script/sign.cpp#L568-L570 If and only if that is the case, the `complete` field of the `SignatureData` is set to `true` accordingly and there is no need then to verify the script after again, as we already know that it would succeed. This leads to a rough ~20% speed-up for `SignTransaction` for single-input ECDSA or Taproot transactions, according to the newly introduced `SignTransaction{ECDSA,Taproot}` benchmarks: ``` $ ./src/bench/bench_bitcoin --filter=SignTransaction.* ``` without commit 18185f4f578b8795fdaa75926630a691e9c8d0d4: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 185,597.79 | 5,388.00 | 1.6% | 0.22 | `SignTransactionECDSA` | 141,323.95 | 7,075.94 | 2.1% | 0.17 | `SignTransactionSchnorr` with commit 18185f4f578b8795fdaa75926630a691e9c8d0d4: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 149,757.86 | 6,677.45 | 1.4% | 0.18 | `SignTransactionECDSA` | 108,284.40 | 9,234.94 | 2.0% | 0.13 | `SignTransactionSchnorr` Note that there are already signing benchmarks in the secp256k1 library, but `SignTransaction` does much more than just the cryptographical parts, i.e.: * calculate the unsigned tx's `PrecomputedTransactionData` if necessary * apply Solver on the prevout scriptPubKey, fetch the relevant keys from the signing provider * perform the actual signing operation (for ECDSA signatures, that could be more than once due to low-R grinding) * verify if the signatures are correct by calling `VerifyScript` (more than once currently, which is fixed by this PR) so it probably makes sense to also have benchmarks from that higher-level application perspective. ACKs for top commit: achow101: ACK fe92c15f0c44d1405b9048306736bd0eae868506 furszy: utACK fe92c15f0c44 glozow: light review ACK fe92c15f0c44d1405b9048306736bd0eae868506 Tree-SHA512: b7225ff9e8a640ca5222dea5b2a463a0f9b9de704e4330b5b9a7bce2d63a1f4620575c474a8186f4708d7d9534eab55d000393d99db79c0cfc046b35f0a4a778
2024-07-16Merge bitcoin/bitcoin#30429: rpc: Use CHECK_NONFATAL over AssertAva Chow
fa6270737eb9655bfb4e29b7070ecb6cd2087b7f rpc: Use CHECK_NONFATAL over Assert (MarcoFalke) Pull request description: Any RPC method should not abort the whole node when an internal logic error happens. Fix it by just aborting this single RPC method call when an error happens. Also, fix the linter to find the fixed cases. ACKs for top commit: achow101: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f stickies-v: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f tdb3: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f hodlinator: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f Tree-SHA512: dad2f31b01a66578949009499e4385fb4d72f0f897419f2a6e0ea02e799b9a31e6ecb5a67fa5d27fcbc7939fe8acd62dc04e877b35831493b7f2c604dec7dc64
2024-07-16Merge bitcoin/bitcoin#30435: init: change shutdown order of load block ↵merge-script
thread and scheduler 5fd48360198d2ac49e43b24cc1469557b03567b8 init: change shutdown order of load block thread and scheduler (Martin Zumsande) Pull request description: This avoids situations during a reindex, in which the shutdown doesn't finish since `LimitValidationInterfaceQueue()` is called by the load block thread when the scheduler is already stopped, in which case it would block indefinitely. This can lead to intermittent failures in `feature_reindex.py` (#30424), which I could locally reproduce with ```diff diff --git a/src/validation.cpp b/src/validation.cpp index 74f0e4975c..be1706fdaf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3446,6 +3446,7 @@ static void LimitValidationInterfaceQueue(ValidationSignals& signals) LOCKS_EXCL AssertLockNotHeld(cs_main); if (signals.CallbacksPending() > 10) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); signals.SyncWithValidationInterfaceQueue(); } } ``` It has also been reported by users running `reindex-chainstate` (#23234). I thought for a bit about potential downsides of changing this order, but couldn't find any. Fixes #30424 Fixes #23234 ACKs for top commit: maflcko: review ACK 5fd48360198d2ac49e43b24cc1469557b03567b8 hebasto: re-ACK 5fd48360198d2ac49e43b24cc1469557b03567b8. tdb3: ACK 5fd48360198d2ac49e43b24cc1469557b03567b8 BrandonOdiwuor: Code Review ACK 5fd48360198d2ac49e43b24cc1469557b03567b8 Tree-SHA512: 3b8894e99551c5d4392b55eaa718eee05841a7287aeef2978699e1d633d5234399fa2f5a3e71eac1508d97845906bd33e0e63e5351855139e7be04c421359b36
2024-07-16Merge bitcoin/bitcoin#30425: kernel: De-globalize static validation variablesRyan Ofsky
51fa26239af9bbfd44029aaf595cb4c6a8d4a75d refactor: Mark some static global vars as const (TheCharlatan) 39f9b80fba85d9818222c4d76e99ea1a804f5dda refactor: De-globalize last notified header index (TheCharlatan) 3443943f86678eb27d9895f3871fcf3945eb1c4f refactor: De-globalize validation benchmark timekeeping (TheCharlatan) Pull request description: In future, users of the kernel library might run multiple chainstates in parallel, or create and destroy multiple chainstates over the lifetime of a process. Having static, mutable variables could lead to state inconsistencies in these scenarios. --- This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: dergoegge: Code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d maflcko: ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d 🍚 tdb3: code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d Tree-SHA512: da91aa7ffa343325cabb8764ef03c8358845662cf0ba8a6cc1dd38e40e5462d88734f2b459c2de8e7a041551eda9143d92487842609f7f30636f61a0cd3c57ee
2024-07-16Merge bitcoin/bitcoin#28263: Add fuzz test for FSChaCha20Poly1305, ↵merge-script
AEADChacha20Poly1305 8607773750e60931e51a33e48cd077a1dedf9db3 Add fuzz test for FSChaCha20Poly1305 (stratospher) c807f3322897ca8c0da114556e5936e389da5059 Add fuzz test for AEADChacha20Poly1305 (stratospher) Pull request description: This PR adds fuzz tests for `AEADChaCha20Poly1305` and `FSChaCha20Poly1305` introduced in #28008. Run using: ``` $ FUZZ=crypto_aeadchacha20poly1305 src/test/fuzz/fuzz $ FUZZ=crypto_fschacha20poly1305 src/test/fuzz/fuzz ``` ACKs for top commit: dergoegge: tACK 8607773750e60931e51a33e48cd077a1dedf9db3 marcofleon: Tested ACK 8607773750e60931e51a33e48cd077a1dedf9db3. Ran both targets for ~200 CPU hours. Coverage of intended targets looks good to me. The simulation of damaged keys and checks that follow seem useful as well. Tree-SHA512: b6b85661d896e653caeed330f941fde665fc2bbd97ecd340808a3f365c469fe9134aa77316569a771dc36d1158cac1a5f76700bcfc45fff12aef07562e48feb9
2024-07-16[refactor] delete EraseTxNoLock, just use EraseTxglozow
2024-07-16remove obsoleted TxOrphanage::m_mutexglozow
The TxOrphanage is now guarded externally by m_tx_download_mutex.
2024-07-16lock m_recent_confirmed_transactions using m_tx_download_mutexglozow
2024-07-16remove obsoleted hashRecentRejectsChainTipglozow
This also means AlreadyHaveTx no longer needs cs_main held.
2024-07-16update recent_rejects filters on ActiveTipChangeglozow
Resetting m_recent_rejects once per block is more efficient than comparing hashRecentRejectsChainTip with the chain tip every time we call AlreadyHaveTx. We keep hashRecentRejectsChainTip for now to assert that updates happen correctly; it is removed in the next commit.
2024-07-16add ValidationInterface::ActiveTipChangeglozow
This is a synchronous callback notifying clients of all tip changes. It allows clients to respond to a new block immediately after it is connected. The synchronicity is important for things like m_recent_rejects, in which a transaction's validity can change (rejected vs accepted) when this event is processed. For example, the transaction might have a timelock condition that has just been met. This is distinct from something like m_recent_confirmed_transactions, in which the validation outcome is the same (valid vs already-have), so it does not need to be reset immediately.
2024-07-16guard TxRequest and rejection caches with new mutexglozow
We need to synchronize between various tx download structures. TxRequest does not inherently need cs_main for synchronization, and it's not appropriate to lock all of the tx download logic under cs_main.
2024-07-16Merge bitcoin/bitcoin#30394: net: fix race condition in self-connect detectionglozow
16bd283b3ad05daa41259a062aee0fc05b463fa6 Reapply "test: p2p: check that connecting to ourself leads to disconnect" (Sebastian Falbesoner) 0dbcd4c14855fe2cba15a32245572b693dc18c4e net: prevent sending messages in `NetEventsInterface::InitializeNode` (Sebastian Falbesoner) 66673f1c1302c986e344c7f44bb0b352213d5dc8 net: fix race condition in self-connect detection (Sebastian Falbesoner) Pull request description: This PR fixes a recently discovered race condition in the self-connect detection (see #30362 and #30368). Initiating an outbound network connection currently involves the following steps after the socket connection is established (see [`CConnman::OpenNetworkConnection`](https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/net.cpp#L2923-L2930) method): 1. set up node state 2. queue VERSION message (both steps 1 and 2 happen in [`InitializeNode`](https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/net_processing.cpp#L1662-L1683)) 3. add new node to vector `m_nodes` If we connect to ourself, it can happen that the sent VERSION message (step 2) is received and processed locally *before* the node object is added to the connection manager's `m_nodes` vector (step 3). In this case, the self-connect remains undiscovered, as the detection doesn't find the outbound peer in `m_nodes` yet (see `CConnman::CheckIncomingNonce`). Fix this by swapping the order of 2. and 3., by taking the `PushNodeVersion` call out of `InitializeNode` and doing that in the `SendMessages` method instead, which is only called for `CNode` instances in `m_nodes`. The temporarily reverted test introduced in #30362 is readded. Fixes #30368. Thanks go to vasild, mzumsande and dergoegge for suggestions on how to fix this (see https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200625017 ff. and https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1668290789). ACKs for top commit: naiyoma: tested ACK [https://github.com/bitcoin/bitcoin/pull/30394/commits/16bd283b3ad05daa41259a062aee0fc05b463fa6](https://github.com/bitcoin/bitcoin/pull/30394/commits/16bd283b3ad05daa41259a062aee0fc05b463fa6), built and tested locally, test passes successfully. mzumsande: ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6 tdb3: ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6 glozow: ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6 dergoegge: ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6 Tree-SHA512: 5b8aced6cda8deb38d4cd3fe4980b8af505d37ffa0925afaa734c5d81efe9d490dc48a42e1d0d45dd2961c0e1172a3d5b6582ae9a2d642f2592a17fbdc184445
2024-07-16refactor: pass BlockCreateOptions to createNewBlockSjors Provoost
Rather than pass options individually to createNewBlock and then combining them into BlockAssembler::Options, this commit introduces BlockCreateOptions and passes that instead. Currently there's only one option (use_mempool) but the next commit adds more. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-16refactor: use CHECK_NONFATAL to avoid single-use symbolSjors Provoost
2024-07-15Merge bitcoin/bitcoin#30428: log: LogError with FlatFilePos in UndoReadFromDiskRyan Ofsky
fa14e1d9d5c5dc44396a01583ae94480b7bc29ee log: Fix __func__ in LogError in blockstorage module (MarcoFalke) fad59a2f0f37f5b7f6076fd91be43448e35f4b7e log: LogError with FlatFilePos in UndoReadFromDisk (MarcoFalke) aaaa3323f37526862ebf2a2a4bf522c661e6976e refactor: Mark IsBlockPruned const (MarcoFalke) Pull request description: These errors should never happen in normal operation. If they do, knowing the `FlatFilePos` may be useful to determine if data corruption happened. Also, handle the error `pos.IsNull()` as part of `OpenUndoFile`, because it may as well have happened due to data corruption. This mirrors the `LogError` behavior from `ReadBlockFromDisk`. Also, two other fixup commits in this module. ACKs for top commit: kevkevinpal: ACK [fa14e1d](https://github.com/bitcoin/bitcoin/pull/30428/commits/fa14e1d9d5c5dc44396a01583ae94480b7bc29ee) tdb3: cr and light test ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee ryanofsky: Code review ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee. This should make logging clearer and more consistent Tree-SHA512: abb492a919b4796698d1de0a7874c8eae355422b992aa80dcd6b59c2de1ee0d2949f62b3cf649cd62892976fee640358f7522867ed9d48a595d6f8f4e619df50
2024-07-15Merge bitcoin/bitcoin#30407: test: [refactor] Pass TestOptsmerge-script
fa690c8e532672f7ab53be6f7a0bb3070858745e test: [refactor] Pass TestOpts (MarcoFalke) Pull request description: Currently optional test context setup settings are passed by adding a new optional argument to the constructors. For example `extra_args`. This is problematic, because: * Adding more optional settings in the future requires touching all affected constructors, increasing their verbosity. * Setting only a later option requires setting the earlier ones. * Clang-tidy named args passed to `std::make_unique` are not checked. Fix all issues by adding a new struct `TestOpts`, which holds all options. Notes: * The chain type is not an option in the struct for now, because the default values vary. * The struct holds all possible test options globally. Not all fields may be used by all constructors. Albeit harmless, it is up to the test author to not set a field that is unused. ACKs for top commit: kevkevinpal: utACK [fa690c8](https://github.com/bitcoin/bitcoin/pull/30407/commits/fa690c8e532672f7ab53be6f7a0bb3070858745e) dergoegge: utACK fa690c8e532672f7ab53be6f7a0bb3070858745e TheCharlatan: Nice, ACK fa690c8e532672f7ab53be6f7a0bb3070858745e Tree-SHA512: 8db8efa5dff854a73757d3f454f8f902e41bb4358f5f9bae29dbb3e251e20ee93489605de51d0822ba31d97835cd15526a29c075278dd6a8bbde26134feb4f49
2024-07-15gui: correct replacement of amp character in the wallet name for QMenuKonstantin Akimov
The comment in the code regarding the use of an "&" on a menu item is misleading. If a wallet name has an "&" in it, it is not supposed to be interpreted as a hot-key, but it should be shown as it is without replacing it to an underscore.
2024-07-15Merge bitcoin/bitcoin#30197: fuzz: bound some miniscript operations to avoid ↵merge-script
fuzz timeouts bc34bc288824978ef4b98e8802b47cb863c8a8c2 fuzz: limit the number of nested wrappers in descriptors (Antoine Poinsot) 8d7340105f5299e9a45e84f1704b8b4545cb85f0 fuzz: limit the number of sub-fragments per fragment for descriptors (Antoine Poinsot) Pull request description: Some of the logic in the miniscript module is quadratic. It only becomes an issue for very large uninteresting descriptors (like a `thresh` with 130k sub-fragments or a fragment with more than 60k nested `j:` wrappers). This PR fixes the two types of fuzz timeouts reported by Marco in https://github.com/bitcoin/bitcoin/issues/28812 by trying to pinpoint the problematic descriptors through a simple analysis of the string, without limiting the size of the string itself. This is the same approach as was adopted for limiting the depth of derivation paths. ACKs for top commit: dergoegge: utACK bc34bc288824978ef4b98e8802b47cb863c8a8c2 stickies-v: Light ACK bc34bc288824978ef4b98e8802b47cb863c8a8c2 marcofleon: Code review ACK bc34bc288824978ef4b98e8802b47cb863c8a8c2. The added comments are useful, thanks for those. Tested on the three inputs in https://github.com/bitcoin/bitcoin/issues/28812 that caused the timeouts. Tree-SHA512: 8811c7b225684c5ecc1eb1256cf39dfa60d4518161e70210086c8a01b38927481ebe747af86aa5f4803187672d43fadabcfdfbf4e3b049738d629a25143f0e77
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-15 10:40:20 +0000