f842ed9a40c0db656b86f85e84dd4978865cc0a0 test: refactor: replace unnecessary `BytesIO` uses (Sebastian Falbesoner)
Pull request description:
Rather than needing to create intermediate stream variables, we can use helper functions like `tx_from_hex` instead or access the result directly, leading both to increased readability and less code.
ACKs for top commit:
stickies-v:
ACK f842ed9a40c0db656b86f85e84dd4978865cc0a0
brunoerg:
crACK f842ed9a40c0db656b86f85e84dd4978865cc0a0
aureleoules:
ACK f842ed9a40c0db656b86f85e84dd4978865cc0a0 - It seems that these are the only instances that can be changed and it simplifies test code.
Tree-SHA512: 7f4fd7a26720d1988bf27f66c817ff6cd7060350a3be62d28e7848c768fd43578719ca475193d4057ccf4f2458af18564fd513fe3a1d458a11c799927c12bd65
00e9b97f37e0bdf4c647236838c10b68b7ad5be3 refactor: Move fs.* to util/fs.* (TheCharlatan)
106b46d9d25b5228ef009fbbe6f9a7ae35090d15 Add missing fs.h includes (TheCharlatan)
b202b3dd6393b415fa68e18dc49c9431dc6b58b2 Add missing cstddef include in assumptions.h (TheCharlatan)
18fb36367a28819bd5ab402344802796a1248979 refactor: Extract util/fs_helpers from util/system (Ben Woosley)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303https://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 #25152.
#### Context
There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in https://github.com/bitcoin/bitcoin/pull/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 #25152 and will be submitted in separate PRs once this PR has been processed.
ACKs for top commit:
hebasto:
ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3
Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666
c9aace2bfd180906d528cc2a80bc856685fbc6ca depends, doc: Document `NO_USDT` option (Hennadii Stepanov)
Pull request description:
A follow-up for https://github.com/bitcoin/bitcoin/pull/23724.
This also removes a stray `</dd>` from the `NO_NATPMP` docs.
ACKs for top commit:
0xB10C:
ACK c9aace2bfd180906d528cc2a80bc856685fbc6ca
Tree-SHA512: 44426f5b2bf9eaa3b8a6c54020d23c97ab04f3d75bfa93469c2c5d3bd512fc214d82652e4558d9ad33e2b2e3605991cdbf7779ba47b10a50385d4c962a598b60
fae66fceb3147385320593d1e15faf290b0f4caf test: Remove python3.5 workaround in authproxy (MarcoFalke)
Pull request description:
Remove workaround for a bug that is long fixed in a EOL python version, that isn't used by us.
If the workaround is still needed, it should at least log the exception before silently discarding it, so that debugging is possible/easier.
ACKs for top commit:
fanquake:
ACK fae66fceb3147385320593d1e15faf290b0f4caf
Tree-SHA512: 9da28e495d530b9f9c5c75eff4982ef23b3775309e1f8d509722a9e7fd8b3535942c9a9cbd2d5e43e6487d46fdec4a63114aaa104e258c261cb98cb58560872a
3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3 [net processing] Pass TxRelay to FindTxForGetData instead of Peer (dergoegge)
c85ee76a3647a42d3a59c28444c5ca52e7db7c5e [net processin] Don't take cs_main in FindTxForGetData (dergoegge)
Pull request description:
Addresses left over feedback from #26140.
* https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153498543
* https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153499627
`mapRelay` is only accessed from the message processing thread and does not need to be kept in sync with anything validation specific, it is therfore perfectly fine to have it guarded by `g_msgproc_mutex`.
ACKs for top commit:
jnewbery:
utACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3
hebasto:
ACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 3ef84bfe4abfa8d991a7e65d9184221294d80e0df0bbb47f0270ab6ca1593266c98abf83c610f9f86b4d16c7a4b62bcf83f8856c68d3c2e10894bff6ed3e88cd
e414edd8fc88b55a91f446dcc4d55cc4cd6d32e7 qt: Update translation source file (Hennadii Stepanov)
b7800950913c47bc1a939bd7f99c6b0629aea530 qt: Adjust plural forms for translations (Hennadii Stepanov)
6ae8a24009cb006476b3be612a34e4deda30433a GUI: Send: Make feerates translatable (Luke Dashjr)
bd42f5e1cdb0cbd829c8fe1700d036b681f3930d Bugfix: GUI: Send/PSBT: Correct virtual size unit and make translatable (Luke Dashjr)
1b0407f5f1f5902f67b58c7ada9c4b0c45457244 Bugfix: GUI: transactiondesc: Translate outlier "own address" and "watch-only" (Luke Dashjr)
170f3126f220f9ffe3db471e0024a23dede3f90b GUI: Use translated external signer errors for messagebox text (Luke Dashjr)
96989599d6c7264022c188babdcd9cacac9ab69f GUI: Make messages for copying unsigned PSBTs translatable (Luke Dashjr)
08b8b287d3762dd6d4be09fad0b71c4cacfe6658 Bugfix: GUI: Debug info: Use correct "kB" case for small mempool sizes, and make translation-friendly (Luke Dashjr)
dacc322be10c6709338d2f0d7f5c4366609ad1a0 GUI: PSBTOperationsDialog: Support translating window title (Luke Dashjr)
5a4fe5527045ad3d7f1c834c3c6ea1d531c87879 GUI: Intro: Support translating caption of data directory chooser (Luke Dashjr)
3868ba3a27d594dd2968548fff3db457cbeb0080 GUI: Support translating peer network names (Luke Dashjr)
f1f981119884389f1f355bc11847a73812a2836e GUI: Support translating address type dropdown entries (Luke Dashjr)
Pull request description:
This PR updates the `src/qt/locale/bitcoin_en.xlf` translation source file according to [Release schedule for 25.0](https://github.com/bitcoin/bitcoin/issues/26549).
Some translation-related fixes have been picked from https://github.com/bitcoin-core/gui/pull/599 and https://github.com/bitcoin-core/gui/pull/716.
Note for reviewers: it is expected to get a zero diff after running `make -C src translate` locally.
ACKs for top commit:
jarolrod:
ACK e414edd8fc88b55a91f446dcc4d55cc4cd6d32e7
Tree-SHA512: 5b0c70db1e2f5871067e84f43ebea4ee4f0027fc5f2be49bbcb1d04e162ae76607b2b038e9d0622bcb5b3658d0ede8c10c4421ddaa3343f0e0be54315ca7a4f5
Rather than needing to create intermediate stream variables, we can use
helper functions like `tx_from_hex` instead or access the result
directly, leading both to increased readability and less code.
1869310f3cfa4ab26b5090d8a4002eefdc84870e refactor: remove unused param from legacy pubkey (Bushstar)
Pull request description:
Unused param present in legacy pubkey manager interface. This param will not be used and should be removed to prevent unintended usage.
ACKs for top commit:
Sjors:
ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e
furszy:
ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e
Tree-SHA512: 0fb41fc8f481f859262f2e8e9a93c990c1b4637e74fd9191ccc0b3c523d0e7d94217a3074bb357276e1941a10d29326f850f9b27eccc1eca57cf6b549353400c
Also, move the burden of checking for a timeout to the client and
disable the timeout on the server. This should avoid intermittent issues
in slow tests (for example mining_getblocktemplate_longpoll.py, or
feature_dbcrash.py), or possibly when the server is running slow (for
example in valgrind). There shouldn't be any downside in tests caused
by a high rpcservertimeout.
fafe3a8e38bdb6d12b2654b066b8036ba3e9455d ci: Remove second user account (MarcoFalke)
Pull request description:
The rationale for the second (nonroot) account no longer applies. See also https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148898438
ACKs for top commit:
josibake:
utACK fafe3a8e38
Tree-SHA512: 4464e50bc6499e012ff33c591f20c5989b99e223c476cefea669aedb79a0e731734db9469d4c7d800642191b43b57ea32cda187093395f100dec719fea17724d
Taking cs_main is no longer necessary since we moved
`m_recently_announced_invs` to `Peer` and `mapRelay` is actually only
accessed from the message processing thread.
71b3e9b0ade9680f6847e93785225c5927929336 sanitizers: remove GetRNGState lsan suppression (fanquake)
Pull request description:
I am no-longer seeing this, testing with the native_asan job over `x86_64` (Ubuntu 22.04) and `aarch64` (Fedora 37).
Can anyone recreate the false-positive?
ACKs for top commit:
MarcoFalke:
lgtm ACK 71b3e9b0ade9680f6847e93785225c5927929336
hebasto:
ACK 71b3e9b0ade9680f6847e93785225c5927929336, tested on Ubuntu 22.04 x86_64.
Tree-SHA512: 63020327d61acd6c94c6c278c9c4d72aedc10253fa172bcf9353bcad4c28d068bee824969eb3ce92152244831df8fe92cffae536453c8073a4fda74dfdfbcefa
d0e571ebb187d7c4c2821f1334cb2dd4222dd8ce guix: use python-minimal (3.9) (fanquake)
Pull request description:
This further minifies the Guix release build environment.
ACKs for top commit:
TheCharlatan:
ACK d0e571ebb187
hebasto:
ACK d0e571ebb187d7c4c2821f1334cb2dd4222dd8ce
Tree-SHA512: 0a8aa9ae861107f106c3b9c41f78ffbaf0e71e3c61f6d96e5c82415b4570b8ac85d6578d37cd0df0ec315c1c9f35fc90b281f139271ccfd15a1495ba76166789
b5ef1419ece77db77cc196eb541f99a72360a607 ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321) (Vasil Stoyanov)
Pull request description:
Basically it removes the above-mentioned env-vars as per MarcoFalke's instructions. The only deviation from the plan laid out there was that I double-quoted the last instance of $ANDROID_HOME for the sake of consistency and future-proofing and the rest of the non-quoted vars due to lint failing the build.
Fixes#27321.
ACKs for top commit:
josibake:
ACK b5ef1419ec
hernanmarino:
untested ACK b5ef1419ece77db77cc196eb541f99a72360a607. LGTM
Tree-SHA512: a79776bf64a2fa8b38195cc84445e171fd689f156aac5a1e5d39040300567eb9f4c2ebd00fbf3fa0e55b68793f8f752d94f7d817f6097ed9dd3a8ea57651b981
e669833943bda13b2840a174dc8e45194187fc8e test: dedup package limit checks via decorator in mempool_package_limits.py (Sebastian Falbesoner)
72f25e238c1f791f9fd3018152d76f9127b745e2 test: refactor: use Satoshis for fees in mempool_package_limits.py (Sebastian Falbesoner)
Pull request description:
The subtests in the functional test mempool_package_limits.py all follow the same pattern:
1. first, check that the mempool is currently empty
2. create and submit certain single txs to the mempool, prepare list of hex transactions
3. check that `testmempoolaccept` on the package hex fails with a "package-mempool-limits" error on each tx result
4. after mining a block, check that submitting the package succeeds
Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so this might be a nice opportunity to deduplicate code by using a newly introduced decorator which executes the necessary before and after the essential part of the subtest. This also makes it easier to add new subtests without having to copy-paste those parts once again.
In addition, the first commit switches the fee unit from BTC to Satoshis, which allows to get rid of some imports (`COIN` and `Decimal`) and a comment for the `test_desc_size_limits` subtest is fixed (s/25KvB/21KvB/).
ACKs for top commit:
ismaelsadeeq:
ACK e669833943bda13b2840a174dc8e45194187fc8e
glozow:
utACK e669833943bda13b2840a174dc8e45194187fc8e
Tree-SHA512: 84a85e739de7387391c13bd46aeb015a74302ea7c6f0ca3d4e2b1b487d38df390dc118eb5b1c11d3e4206bff316a4dab60ef6b25d8feced672345d4e36ffd205
3a060ae7b67cc28fc60cf28cbc518fa1df24f545 scripted-diff: Rename nUnconnectingHeaders and fPreferHeaders (dergoegge)
279c53d7e4e920d3d09da74689761a2961b1a636 [net processing] Move m_recently_announced_invs from CNodeState to Peer (dergoegge)
938a8e2566bf2f819adeb75a08c0c8ba9a85fe1c [net processing] Annotate m_recently_announced_invs as guarded by g_msgproc_mutex (dergoegge)
8a2cb1f749ade31bb0655567cd0062db8a7164e5 [net processing] Move fPreferHeaders from CNodeState to Peer (dergoegge)
3605011e79d0d43ff532a2c3cc9cad4cc48eb06c [net processing] Annotate fPreferHeaders as guarded by g_msgproc_mutex (dergoegge)
4b84e502f5ba69d650191ce8f2e20b86c1d427ed [net processing] Move m_headers_sync_timeout from CNodeState to Peer (dergoegge)
689b747fc34a5670a24bb5c4b1986dd8670b55ef [net processing] Annotate m_headers_sync_timeout as guarded by g_msgproc_mutex (dergoegge)
d8c0d1c345546c6066274edbf3bf4fb5d6f75419 [net processing] Move nUnconnectingHeaders from CNodeState to Peer (dergoegge)
5f80d8d1eec14253407ceed1a813619d3e5a0fe4 [net processing] Annotate nUnconnectingHeaders as guarded by g_msgproc_mutex (dergoegge)
1d87137227ee125018006935d55164a72265c935 [validation] Annotate ChainstateManager::m_best_header as guarded by cs_main (dergoegge)
Pull request description:
`nUnconnectingHeaders`, `m_headers_sync_timeout`, `fPreferHeaders` and `m_recently_announced_headers` are currently all `CNodeState` members even though they are only ever accessed from the message processing thread (therefore sufficiently guarded exclusively by `g_msgproc_mutex`). `CNodeState` exists purely to hold validation-specific state guarded by `cs_main` that is accessed by multiple threads.
This PR adds thread-safety annotations for the above mentioned `CNodeState` members and moves them to `Peer`.
ACKs for top commit:
glozow:
code review ACK 3a060ae7b67cc28fc60cf28cbc518fa1df24f545, as in I am convinced these members shouldn't be guarded by cs_main and belong in Peer/TxRelay. clang checked the annotations for me.
hebasto:
ACK 3a060ae7b67cc28fc60cf28cbc518fa1df24f545
Tree-SHA512: 2db27c03f2c6ed36ad7dfbb4f862eeed3c3e57f845cf8abb9e7cada36f976257311892020bbcff513fbe662a881c93270e3a126946ceb0c3f94213b546bcaa81
a634c288c34627eb4ea6f919c99339256f490ede ci: use LLVM/clang-16 in native_fuzz (ASAN) job (fanquake)
Pull request description:
Similar to #27298.
ACKs for top commit:
dergoegge:
utACK a634c288c34627eb4ea6f919c99339256f490ede
Tree-SHA512: 7a2625a3ac83710063d941dcbca42431b3b79a1380872fd2c566c0ab3041d8123d7dcddeb8a4972efd0ef6496b15bbe0b39b6d2de84df81fcdd8d68e1248fbc5
ea7ec7808745805c0a18513d7da271dedb2de3f1 refactor: Drop no longer used `CNetMsgMaker` instances (Hennadii Stepanov)
Pull request description:
The removed lines have been unused since the abf5d16c24 commit from https://github.com/bitcoin/bitcoin/pull/25454.
ACKs for top commit:
dergoegge:
utACK ea7ec7808745805c0a18513d7da271dedb2de3f1
Sjors:
ACK ea7ec7808745805c0a18513d7da271dedb2de3f1
TheCharlatan:
ACK ea7ec7808745805c0a18513d7da271dedb2de3f1
Tree-SHA512: 9a2a9ff3f124b68a8cd20a637e90885096996c3aa354a4d8adbec98f5761e9e826c1c064ccd90aaf6d72beac61dd9e22c8b76d089e18bba6e0ad51e59a9c7df8
