backport: trivial 2024 10 23 pr4 #6348
Merged
Conversation
This file contains 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
|
This pull request has conflicts, please rebase. |
8c38509 contrib: move user32.dll from bitcoind.exe libs (fanquake) Pull request description: The user interface library is no-longer needed by `bitcoind.exe`, or utils, only `bitcoin-qt.exe`. Add missing doc. ACKs for top commit: hebasto: ACK 8c38509, I've verified imported libraries on a Windows machine with the `dumpbin /imports` command. Tree-SHA512: f752a5b807341c87320523f4e7c564c8acdbfc1313054a684844035102a7c4695d34cfefb0c6904f3151b2dfdcb54d6ea243c570deceeda30345944251e4c513
faa8c1b fuzz: Re-enable symbolize=1 in ASAN_OPTIONS (MarcoFalke) Pull request description: Looks like this fixed itself somehow and is no longer reproducible? ACKs for top commit: fanquake: ACK faa8c1b Tree-SHA512: 67d2d6349cc7485f32bebabc18869ab101ae66a778a40ff9ddb037980997e600d7c6d1e0a17a011fa2a4ba07c73594b087dd781248cb8351f2688bc4cf6e587d
50f7214 valgrind: add suppression for bug 472219 (fanquake) Pull request description: Now that https://bugs.kde.org/show_bug.cgi?id=472219 has been fixed upstream in: https://sourceware.org/git/?p=valgrind.git;a=commit;h=6ce0979884a8f246c80a098333ceef1a7b7f694d Add a supression to ignore the bug until we are using a fixed version of Valgrind. Related to bitcoin#28072. ACKs for top commit: MarcoFalke: lgtm ACK 50f7214 Tree-SHA512: 1030f3709195250350fd9c558420a9b1773fb54fdb323e0452a46eeb69ec6d60b5df50bde617c12d917e16dde07db64dee1b0101ddd4eda6161261fc7f6d4474
ab498d9 qa, doc: Fix comment (Hennadii Stepanov) Pull request description: This PR is a follow-up for: - bitcoin#9956 - bitcoin#10096 ACKs for top commit: RandyMcMillan: ACK ab498d9 Tree-SHA512: 267ae52c961ee79e172f27cb1587282ac5cf7ec929a136db6feed3de4f319a74b462befd9b99711081db8a5c30a513f72366364068700418b273a31958b31b1d
f054bd0 refactor: use "if constexpr" in std::vector's Unserialize() (Martin Leitner-Ankerl) 088caa6 refactor: use "if constexpr" in std::vector's Serialize() (Martin Leitner-Ankerl) 0fafaca refactor: use "if constexpr" in prevector's Unserialize() (Martin Leitner-Ankerl) c8839ec refactor: use "if constexpr" in prevector's Serialize() (Martin Leitner-Ankerl) 1403d18 refactor: use fold expressions instead of recursive calls in UnserializeMany() (Martin Leitner-Ankerl) bd08a00 refactor: use fold expressions instead of recursive calls in SerializeMany() (Martin Leitner-Ankerl) Pull request description: This simplifies the serialization code a bit and should also make it a bit faster. * use fold expressions instead of recursive calls. This simplifies the code, makes it most likely faster because it reduces the number of function calls, and compiles faster because there are fewer template instantiations. * use `if constexpr` instead of unnecessarily creating a temporary object only to call the right overload. This is used for `std::vector` and `prevector` serialization. ACKs for top commit: MarcoFalke: only change is to add a missing `&`. lgtm, re-ACK f054bd0 📦 jonatack: ACK f054bd0 sipa: utACK f054bd0 john-moffett: ACK f054bd0 Tree-SHA512: 0417bf2d6be486c581732297945449211fc3481bac82964e27628b38ef55a47dfa58d730148aeaf1b19fa8eb1076489cc646ceebb178162a9afa59034601501d
…-arguments` to compile without warnings 5197660 tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings (Martin Leitner-Ankerl) Pull request description: Fixes bitcoin#26916 by disabling the warning `-Wgnu-zero-variadic-macro-arguments` when clang is used as the compiler. Also see the comments * Proposed changes in the bug bitcoin#26916 (comment) * Proposed changes when moving to a variadic maro: bitcoin#26593 (comment) ACKs for top commit: hebasto: ACK 5197660, I've reconsidered my [comment](bitcoin#27401 (comment)) and I think the current localized approach is optimal. fanquake: ACK 5197660 - checked that this fixes the warnings under Clang. Tree-SHA512: c3dda3bcbb2540af6283ffff65885a9937bfdaaef3b00dc7d60b9f9740031d5c36ac9cb3d3d8756dbadce4812201a9754f5b8770df0d5e0d5ee690ba8a7135d2
…coins_view` target e417c98 fuzz: coins_view: remove an incorrect assertion (Antoine Poinsot) c5f6b1d fuzz: coins_view: correct an incorrect assertion (Antoine Poinsot) Pull request description: The `coins_view` fuzz target would assert in two places that the cache is consistent with the backend. But it's never the case (that's the whole point of using a cache). The only reason this didn't result in a crash was that we would never actually hit these assertions. I ran into this while introducing a new target with an in-memory `CCoinsViewDB` as the backend view (see bitcoin#28216) which made the code paths with those assertions actually reachable. ACKs for top commit: dergoegge: Code review ACK e417c98 Tree-SHA512: 5847bb2744a2f2831dace62d32b79cc491bf54e2af4ce425411d245d566622d9aff816d9be5ec8e830d10851c13f2500bf4f0c004d88b4d7cca1d483ef8960a6
…let_basic` 452c094 test: fix 'unknown named parameter' test in `wallet_basic` (brunoerg) Pull request description: This PR removes loop when testing an unknown named parameter. They don't have any effect. ACKs for top commit: jonatack: ACK 452c094 theStack: re-ACK 452c094 Tree-SHA512: cf1a37d738bb6fdf9817e7b1d33bc69643dae61e3dbfae5c1e9f26220c55db6f134018dd9a1c65c13869ee58bcb6f3337c5999aabf2614d3126fbc01270705e8
…es with shallow clone 360ac64 test: previous releases: speed up fetching sources with shallow clone (Sebastian Falbesoner) Pull request description: For the sake of building previous releases, fetching the whole history of the repository for each version seems to be overkill as it takes much more time, bandwidth and disk space than necessary. Create a shallow clone instead with history truncated to the one commit of the version tag, which is directly checked out in the same command. This has the nice side-effect that we can remove the extra `git checkout` step after as it's not needed anymore. Note that it might look confusing to pass a _tag_ to a parameter named `--branch`, but the git-clone manpage explicitly states that this is supported. ACKs for top commit: MarcoFalke: lgtm ACK 360ac64 Tree-SHA512: c885a695c1ea90895cf7a785540c24e8ef8d1d9ea78db28143837240586beb6dfb985b8b0b542d2f64e2f0ffdca7c65fc3d55f44b5e1b22cc5535bc044566f86
…args fbcacd4 test: remove fixed timeouts from feature_config_args (Martin Zumsande) Pull request description: Fixes bitcoin#28290 These fixed timeouts aren't affected by the `timeout_factor` option and can therefore cause timeouts in slow environments. They are also unnecessary for the test because they measure the wrong thing: While there is an internal waiting time of 60s within `ThreadOpenConnections` (beginning only when that thread is started) for fixed seeds querying, the timeouts here don't measure that but the time from startup until a debug log message is encountered, during which many other things happen in init, so they don't make much sense to me in the first place. ACKs for top commit: MarcoFalke: lgtm ACK fbcacd4 Tree-SHA512: 7bb3b7db2f9666b1929ffb7773c838ee98b0845569428e5d00ecf5234973d534c4f474e213896c71baabd6096a79347bd21b41a17b130053049714eb8a447c79
…INSTALL 8f54102 doc: s/--no-substitute/--no-substitutes in guix/INSTALL (fanquake) Pull request description: https://guix.gnu.org/manual/en/html_node/Common-Build-Options.html ACKs for top commit: hebasto: ACK 8f54102. TheCharlatan: ACK 8f54102 Tree-SHA512: 4d79791a8fa772d35723d35e85aaf9ab86304f2a1238e883bbb85051d0cc5f9841dab99ef0fe1e9d67f6259798e98942be00ae7c4320328987234293d9bf3012
…e reversed c0bf667 index: add [nodiscard] attribute to functions writing to the db (furszy) eef5955 index: coinstats reorg, fail when block cannot be reversed (furszy) Pull request description: Found it while reviewing bitcoin#24230 (comment). During a reorg, continuing execution when a block cannot be reversed leaves the coinstats index in an inconsistent state. This was surely overlooked when 'CustomRewind' was implemented. ACKs for top commit: ryanofsky: Code review ACK c0bf667. Only change since last review is new commit adding [[nodiscard]] Tree-SHA512: f4fc8522508d23e4fff09a29c935971819b1bd3b2a260e08e2e2b72f9340980d74fbec742a58fe216baf61d27de057c7c8300e8fa075f8507cd1227f128af909
97e2e1d [fuzz] Use afl++ shared-memory fuzzing (dergoegge) Pull request description: Using shared-memory is faster than reading from stdin, see https://github.com/AFLplusplus/AFLplusplus/blob/7d2122e0596132f9344a5d0896020ebc79cd33db/instrumentation/README.persistent_mode.md ACKs for top commit: MarcoFalke: review ACK 97e2e1d Tree-SHA512: 7e71b5f84835e41531c19ee959be2426da245869757de8e5dd1c730ae83ead650e2ef75f4d594d7965f661821a4ffbd27be84d3ce623702991501b34a8d02fc3
508d05f [fuzz] Don't use afl++ deferred forkserver mode (dergoegge) Pull request description: Fixes bitcoin#28469 This makes our afl++ harness essentially behave like libFuzzer, with the exception that the whole program does fully reset every 100000 iterations. 100000 is somewhat arbitrary and we could also go with `std::numeric_limits<unsigned in>::max()` but a smaller limit does allow for the occasional reset to counter act some amount of instability in the fuzzing loop (e.g. non-determinism, statefulness). It's a bit of a shame to do this just for the targets whose initial state can't be forked (e.g. threads) because other targets do benefit from not having to redo the state setup. An alternative would be bitcoin#28469 (comment): ``` If the goal is to be maximally performant, the fork would need to happen for each fuzz target specifically. I guess it can be achieved by wrapping __AFL_INIT(); into a helper function and then require all fuzz target initialize() to call it? ``` ACKs for top commit: MarcoFalke: lgtm ACK 508d05f Tree-SHA512: d9fe94e2e3198795f8fb58f67eb383531a534bcd4ec75a1f0ae6ccb5531863dbc09800bb7d77536417745c4c8bc49a4f84dcc959918b27d4997a270eeacb0e7e
…rd to __AFL_FUZZ_INIT fa33b2c fuzz: Add missing PROVIDE_FUZZ_MAIN_FUNCTION guard to __AFL_FUZZ_INIT (MarcoFalke) Pull request description: Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62455 ACKs for top commit: dergoegge: utACK fa33b2c Tree-SHA512: 735926f7f94ad1c3c5dc0fc62a2ef3a85abae25f4fe1e7654c2857ce3e867667ed28da58ab36281d730d3e206a0728cb429671ea5d3ccd11519e637eb191f70d
d05be12 test: added coverage to estimatefee (kevkevin) Pull request description: Added a assert for an rpc error when we try to estimate fee for the max conf_target Line I am adding coverage to https://github.com/bitcoin/bitcoin/blob/master/src/rpc/fees.cpp#LL71C52-L71C52 ACKs for top commit: MarcoFalke: lgtm ACK d05be12 Tree-SHA512: dfab075989446e33d1a5ff1a308f1ba1b9f80cce3848fbe4231f69212ceef456a3f2b19365a42123e0397c31893fd9f1fd9973cc00cfbb324386e12ed0e6bccc
befb42f qt: Silence `-Wcast-function-type` warning (Hennadii Stepanov) Pull request description: On Fedora 38 @ 8f7b9eb: ``` $ x86_64-w64-mingw32-g++ --version | head -1 x86_64-w64-mingw32-g++ (GCC) 12.2.1 20221121 (Fedora MinGW 12.2.1-8.fc38) $ ./configure CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site CXXFLAGS="-Wno-return-type -Wcast-function-type" $ make > /dev/null qt/winshutdownmonitor.cpp: In static member function 'static void WinShutdownMonitor::registerShutdownBlockReason(const QString&, HWND__* const&)': qt/winshutdownmonitor.cpp:46:42: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'PSHUTDOWNBRCREATE' {aka 'int (*)(HWND__*, const wchar_t*)'} [-Wcast-function-type] 46 | PSHUTDOWNBRCREATE shutdownBRCreate = (PSHUTDOWNBRCREATE)GetProcAddress(GetModuleHandleA("User32.dll"), "ShutdownBlockReasonCreate"); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` [Required](bitcoin#25972 (comment)) for bitcoin#25972. Picked from https://trac.nginx.org/nginx/ticket/1865. ACKs for top commit: MarcoFalke: review ACK befb42f Tree-SHA512: b37b2c5dd8702caf84d1833c3511bc46ee14f23b84678b8de0fd04e24e5ecc5fd4d27ba38be0d0b08de91299369f70d4924c895a71fd8e0b6feffcfb7407574a
…ns, add test coverage 2ab7952 test: add bip157 coverage for (start height > stop height) disconnect (Sebastian Falbesoner) 63e90e1 test: check for specific disconnect reasons in p2p_blockfilters.py (Sebastian Falbesoner) Pull request description: This PR checks for specific disconnect reasons using `assert_debug_log` in the functional test `p2p_blockfilters.py`. With that we ensure that the disconnect happens for the expected reason and also makes it easier to navigate between implementation and test code, i.e. both the questions "do we have test coverage for this disconnect cause?" (from an implementation reader's perspective) and "where is the code handling this disconnect cause?" (from a test reader's perspective) can be answered simply by grep-ping the corresponding debug message. Also, based on that, missing coverage for the (start height > stop height) disconnect case is added: https://github.com/bitcoin/bitcoin/blob/b7138252ace6d21476964774e094ed1143cd7a1c/src/net_processing.cpp#L3050-L3056 ACKs for top commit: MarcoFalke: lgtm ACK 2ab7952 furszy: Looks good, code ACK 2ab7952 Tree-SHA512: 0581cb569d5935aaa004a95a6f16eeafe628b9d816ebb89232f2832e377049df878a1e74c369fb46931b94e1a3a5e3f4aaa21a007c0a488f4ad2cda0919c605d
PastaPastaPasta
force-pushed
the
develop-trivial-2024-10-23-pr4
branch
from
cec192a
to
9a79217
Compare
knst
approved these changes
Oct 24, 2024
UdjinM6
approved these changes
Oct 24, 2024
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue being fixed or feature implemented
Batch of trivial backports
What was done?
See commits
How Has This Been Tested?
built locally; large combined merge passed tests locally
Breaking Changes
Should be none
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.