backport: bitcoin#27254(partial),29236(partial),29479,29433,30257,30063,30358,30397,29633,22729#17
Draft
Shailendra53 wants to merge 10 commits into
Draft
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
Shailendra53
force-pushed
the
backport-btc-skg-30-04-26
branch
from
d09ffea to
48c414f
Compare
Shailendra53
force-pushed
the
backport-btc-skg-30-04-26
branch
3 times, most recently
from
472cd0c to
a142c61
Compare
00e9b97 refactor: Move fs.* to util/fs.* (TheCharlatan) 106b46d Add missing fs.h includes (TheCharlatan) b202b3d Add missing cstddef include in assumptions.h (TheCharlatan) 18fb363 refactor: Extract util/fs_helpers from util/system (Ben Woosley) Pull request description: This pull request is part of the `libbitcoinkernel` project bitcoin#24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by empact and is taken from its parent PR bitcoin#25152. #### Context There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (bitcoin#25290, bitcoin#25487, bitcoin#25527, bitcoin#25862, bitcoin#26177, and bitcoin#27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in bitcoin#27238. #### Changes Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving filesystem related functions out of the `system.*` files. There is already a pair of `fs.h` / `fs.cpp` in the top-level `src/` directory. They were not combined with the files introduced here, to keep the patch cleaner and more importantly because they are often included without the utility functions. The new files are therefore named `fs_helpers` and the existing `fs` files are moved into the util directory. Further commits splitting more functionality out of `system.h` are still in bitcoin#25152 and will be submitted in separate PRs once this PR has been processed. ACKs for top commit: hebasto: ACK 00e9b97 Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666 Removing dependency on util/system.h file for logging
fa39151 refactor: Remove unused error() (MarcoFalke) fad0335 scripted-diff: Replace error() with LogError() (MarcoFalke) fa808fb refactor: Make error() return type void (MarcoFalke) fa1d624 scripted-diff: return error(...); ==> error(...); return false; (MarcoFalke) fa9a5e8 refactor: Add missing {} around error() calls (MarcoFalke) Pull request description: `error(...)` has many issues: * It is often used in the context of `return error(...)`, implying that it has a "fancy" type, creating confusion with `util::Result/Error` * `-logsourcelocations` does not work with it, because it will pretend the error happened inside of `logging.h` * The log line contains `ERROR: `, as opposed to `[error]`, like for other errors logged with `LogError`. Fix all issues by removing it. ACKs for top commit: fjahr: re-utACK fa39151 stickies-v: re-ACK fa39151, no changes since 4a90374 ryanofsky: Code review ACK fa39151. Just rebase since last review Tree-SHA512: ec5bb502ab0d3733fdb14a8a00762638fce0417afd8dd6294ae0d485ce2b7ca5b1efeb50fc2cd7467f6c652e4ed3e99b0f283b08aeca04bbfb7ea4f2c95d283a Rebase and fixing blockstorage.cpp file src/validation.cpp - Fixing linting error
80fa7da test: Refactor subtree exclusion in lint tests (Brandon Odiwuor) Pull request description: Fixes bitcoin#17413 Refactor subtree exclusion in lint tests to one place Second attempt after PR: bitcoin#24435 ACKs for top commit: fjahr: re-ACK 80fa7da maflcko: lgtm ACK 80fa7da davidgumberg: ACK bitcoin@80fa7da Tree-SHA512: deff7457dd19ca5ea440d3d53feae047e8863b9ddeb6494a3c94605a5d16edc91db8f99a435b4fab2ef89aedee42439562be006da647fb85bbf3def903a3ce50
…output text in json format 9adf949 contrib: rpcauth.py - Add new option (-j/--json) to output text in json format (bstin) Pull request description: This is a simple change to rpcauth.py utility in order to output as json instead raw text. This is beneficial because integrating json output is simpler with multiple different forms of automation and tooling ACKs for top commit: maflcko: ACK 9adf949 achow101: ACK 9adf949 willcl-ark: tACK 9adf949 tdb3: ACK for 9adf949 Tree-SHA512: 2cdc3b2071fbe4fb32a84ce42ee8ad216cff96ed82aaef58daeb3991953ac137ae42d6898a7fdb6cbd1800e1f61ff8d292f0b150eaebdd2a3fd9d37ed7450787
fa780e1 build: Remove --enable-gprof (MarcoFalke) Pull request description: It is unclear what benefit this option has, given that: * `gprof` requires re-compilation (`perf` and other tools can instead be used on existing executables) * `gprof` requires hardening to be disabled * `gprof` doesn't work with `clang` * `perf` is documented in the dev-notes, and test notes, and embedded into the functional test framework; `gprof` isn't * Anyone really wanting to use it could pass the required flags to `./configure` * I couldn't find any mention of the use of `gprof` in the discussions in this repo, apart from the initial pull request adding it (cfaac2a) * Keeping it means that it needs to be maintained and ported to CMake Fix all issues by removing it. ACKs for top commit: TheCharlatan: ACK fa780e1 hebasto: ACK fa780e1, I have reviewed the code and it looks OK. willcl-ark: crACK fa780e1 Tree-SHA512: 0a9ff363ac2bec8b743878a4e3147f18bc16823d00c5007568432c36320bd0199b13b6d0ce828a9a83c2cc434c058afaa64eb2eccfbd93ed85b81ce10c41760c
… variable 189d0da build, test: Remove unused `TIMEOUT` environment variable (Hennadii Stepanov) Pull request description: Setting the `TIMEOUT` environment variable has been a noop in both cases since its introduction. It seems to have been inadvertently copy-pasted from existed code. For example, in commit d80e3cb, it was needlessly copied from a valid case a few lines above for the `qa/pull-tester/run-bitcoind-for-test.sh` script. ACKs for top commit: maflcko: utACK 189d0da edilmedeiros: ACK 189d0da Tree-SHA512: 61111eba30e0c82a0220bea48eba451cd9caa68785b48ec8a91059ca5aadfaff2f6d2ccdc5aa737c5cefa33579cb735431bb9e94bda8fa047825d7bd28d542fb
fa1bc7c scripted-diff: Log parameter interaction not thrice (MarcoFalke) fafb787 doc: Fix outdated dev comment about logging (MarcoFalke) Pull request description: Seems a bit overkill to log the words "parameter interaction" thrice, when at least once is enough. So do that. Before: ``` 2024-06-28T15:30:57Z [init.cpp:745] [InitParameterInteraction] InitParameterInteraction: parameter interaction: -connect or -maxconnections=0 set -> setting -dnsseed=0 ``` After: ``` 2024-06-28T15:47:27Z [init.cpp:745] [InitParameterInteraction] parameter interaction: -connect or -maxconnections=0 set -> setting -dnsseed=0 ACKs for top commit: paplorinc: ACK fa1bc7c fjahr: utACK fa1bc7c TheCharlatan: Nice, ACK fa1bc7c hodlinator: utACK fa1bc7c Tree-SHA512: 83cd92e20dffa38737d4fd31764481284383e12671d9e4b33cfa496743c95c10921a113b1da2caafeb44fca3759a28a8e230df5e30c29fb55d5854ff1531382c
…l/net.cpp e233ec0 refactor: Use designated initializer (Hodlinator) Pull request description: Block was recently touched (e2d1f84) and the codebase recently switched to C++20 which allows this to improve robustness. Follow-up suggested in bitcoin#29625 (comment) ACKs for top commit: maflcko: ACK e233ec0 Tree-SHA512: ce3a18f513421e923710a43c8f97db1badb7ff5c6bdbfd62d9543312d2225731db5c14bef16feb47c43b84fad4dc24485086634b680feba422d2b7b363e13fa6
d0e6564 log: Remove error() reference (Fabian Jahr) Pull request description: Mini-followup to bitcoin#29236 that was just merged. Removes a reference to `error()` that was missed in a comment. ACKs for top commit: ryanofsky: Code review ACK d0e6564. Just dropped LogPrintf reference since last review stickies-v: ACK d0e6564 Empact: ACK bitcoin@d0e6564 Tree-SHA512: 8abe4895951013c2ceca9a57743aacabaf8af831d07eee9ae8372c121c16e88b7226f0e537200c3464792e19ac7e03b57ba0be31f43add8802753972b0aefc48
…startup on bind failure bca346a net: require P2P binds to succeed (Vasil Dimov) af55253 net: report an error if unable to bind on the Tor listening addr:port (Vasil Dimov) 9a7e5f4 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 bitcoin#22726 Fixes bitcoin#22727 ACKs for top commit: achow101: ACK bca346a cbergqvist: ACK bca346a pinheadmz: ACK bca346a Tree-SHA512: fabef89a957191eea4f3e3b6109d2b8389f27ecc74440a920b0c10f31fff00a85bcfd1eb3c91826c7169c618f4de8a8d0a260e2caf40fd854f07ea9a980d8603
Shailendra53
force-pushed
the
backport-btc-skg-30-04-26
branch
from
dff1f34 to
5ac9f09
Compare
Shailendra53
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Bitcoin backport changes for below list of commits:
TIMEOUTenvironment variable bitcoin/bitcoin#30063