backport: Merge bitcoin#30230#19
Draft
Shailendra53 wants to merge 22 commits intovijaydasmp:integration_mayfrom
Draft
backport: Merge bitcoin#30230#19Shailendra53 wants to merge 22 commits intovijaydasmp:integration_mayfrom
Shailendra53 wants to merge 22 commits intovijaydasmp:integration_mayfrom
Conversation
feature_governance_cl.py was opening debug.log in text mode but seeking with a binary offset. The fully safe fix is to read the tail in binary mode, seek using a binary offset, then decode the resulting bytes afterward for the string match. This preserves the existing 100 KiB tail scan while avoiding invalid mixed-mode seek behavior in Python text I/O. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0293c6d fix: read governance debug log tail in binary mode (PastaClaw) Pull request description: # Summary - fix `feature_governance_cl.py` log tail polling to seek `debug.log` in binary mode - decode the tailed bytes after reading so the seek offset stays valid for the stream mode - preserve the existing behavior of scanning only the last 100 KiB for the expected governance tip message ## Context This is a follow-up for a valid review finding from merged PR dashpay#7258: <dashpay#7258 (comment)> Upstream `develop` currently opens `debug.log` in text mode but computes the seek position with a binary offset. The final fix here uses a fully binary tail read and decodes afterward, which avoids both mixed-mode seek problems and text-stream offset arithmetic. ## Validation - `python3 -m py_compile test/functional/feature_governance_cl.py` - pre-PR code review gate verdict: `ship` I did not run `feature_governance_cl.py` end-to-end because this worktree does not have built `dashd`/test binaries available, and building Dash Core would be disproportionate for this narrowly scoped Python functional-test fix. Top commit has no ACKs. Tree-SHA512: 0f5f4837da1657e35a370e8591a4438c610084ce8f9b277be9e20c7318d48d86ada3c4a78b3e8182a5b61797bd72344addc9e07503581c7095ecb55e17aa8c5b
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
ad06e68 test: write functional test results to csv (tdb3) Pull request description: Adds argument `--resultsfile` to test_runner.py. Enables functional test results to be written to a (csv) file for processing by other applications (or for historical archiving). Test name, status, and duration are written to the file provided with the argument. Since `test_runner.py` is being touched, also fixes a misspelling (linter warning). Can split into its own commit if desired. #### Notes - Total runtime of functional tests has seemed to have increased on my development machines over the past few months (more tests added, individual test runtime increase, etc.). Was interested in recording test runtime data over time to detect trends. Initially searched `doc/benchmarking.md`, existing PRs, and Issues, but didn't immediately see this type of capability or alternate solutions (please chime in if you know of one!). Thought it would be beneficial to add this capability to `test_runner` to facilitate this type of data analysis (and potentially other use cases) - Saw https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#benchmarking-with-perf, and this PR's higher level data seems complimentary. - Was on the fence as to whether to expand `print_results()` (i.e. take advantage of the same loop over `test_results`) or implement in a separate `write_results()` function. Decided on the latter for now, but interested in reviewers' thoughts. #### Example 1: all tests pass ``` $ test/functional/test_runner.py --resultsfile functional_test_results.csv --cachedir=/mnt/tmp/cache --tmpdir=/mnt/tmp feature_blocksdir wallet_startup feature_config_args mempool_accept Temporary test directory at /mnt/tmp/test_runner_₿_🏃_20240614_201625 Test results will be written to functional_test_results.csv ... $ cat functional_test_results.csv test,status,duration(seconds) feature_blocksdir.py,Passed,1 feature_config_args.py,Passed,29 mempool_accept.py,Passed,9 wallet_startup.py,Passed,2 ALL,Passed,29 ``` #### Example 2: one test failure ``` $ cat functional_test_results.csv test,status,duration(seconds) feature_blocksdir.py,Passed,1 feature_config_args.py,Passed,28 wallet_startup.py,Passed,2 mempool_accept.py,Failed,1 ALL,Failed,28 ``` ACKs for top commit: maflcko: re-ACK ad06e68 kevkevinpal: tACK [ad06e68](bitcoin@ad06e68) achow101: ACK ad06e68 rkrux: tACK [ad06e68](bitcoin@ad06e68) brunoerg: ACK ad06e68 marcofleon: Good idea, tested ACK ad06e68 Tree-SHA512: 561194406cc744905518aa5ac6850c07c4aaecdaf5d4d8b250671b6e90093d4fc458f050e8a85374e66359cc0e0eaceba5eb24092c55f0d8f349d744a32ef76c
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
Shailendra53
force-pushed
the
backport-btc-skg-04-05-2026
branch
from
b82e96d to
63e9ca7
Compare
…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
… time fa601ab util: Catch translation string errors at compile time (MarcoFalke) Pull request description: The translation helper function `_()` has many problems. For example, the following compiles: ```cpp auto ptr{"wrong"}; _(ptr); _(nullptr); _(0); _(NULL); ``` However, it is wrong, because none of the arguments passed to the function can be picked up by the translation tooling for transifex. Fix all issues by enforcing only real string literals can be passed to the function. ACKs for top commit: ryanofsky: Code review ACK fa601ab hebasto: ACK fa601ab. Tree-SHA512: 33aed02d7e8fc9bfb8f90746f5c8072a8c0910fa900ec3516af2e732780b0fee8b07b6596c0fc210b018c0869111d6c34bf8d083de0e88ecdb4dee88e809186d
…20Poly1305 8607773 Add fuzz test for FSChaCha20Poly1305 (stratospher) c807f33 Add fuzz test for AEADChacha20Poly1305 (stratospher) Pull request description: This PR adds fuzz tests for `AEADChaCha20Poly1305` and `FSChaCha20Poly1305` introduced in bitcoin#28008. Run using: ``` $ FUZZ=crypto_aeadchacha20poly1305 src/test/fuzz/fuzz $ FUZZ=crypto_fschacha20poly1305 src/test/fuzz/fuzz ``` ACKs for top commit: dergoegge: tACK 8607773 marcofleon: Tested ACK 8607773. 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
…tion after signing (+introduce signing benchmarks) fe92c15 script/sign: avoid duplicated signature verification after signing (Sebastian Falbesoner) 0800895 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 fe92c15 furszy: utACK fe92c15 glozow: light review ACK fe92c15 Tree-SHA512: b7225ff9e8a640ca5222dea5b2a463a0f9b9de704e4330b5b9a7bce2d63a1f4620575c474a8186f4708d7d9534eab55d000393d99db79c0cfc046b35f0a4a778
Shailendra53
force-pushed
the
backport-btc-skg-04-05-2026
branch
3 times, most recently
from
8f7bfd6 to
ce4346f
Compare
…verification after signing (+introduce signing benchmarks)" This reverts commit 234878a.
Shailendra53
force-pushed
the
backport-btc-skg-04-05-2026
branch
from
ce4346f to
b98fa43
Compare
…ADChacha20Poly1305" This reverts commit 67047cc.
… compile time" This reverts commit 82c0bac.
…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
…ptions area rather than waste space b71bfd9 GUI/OptionsDialog: Prefer to stretch actual options area rather than waste space (Luke Dashjr) Pull request description: ACKs for top commit: hebasto: ACK b71bfd9 Tree-SHA512: b706a07292fe81379e303f9069fca6efd5ceb15ee5bb77c6aeddbf63f736494ce877b76767ff17d7becf98d07209e51c74bdb99365596b7b9f4904a30438d72d
…actual options area rather than waste space" This reverts commit c5118b3.
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
bf264e0 test: check_mempool_result negative feerate (kevkevin) Pull request description: Adds test coverage in `mempool_accept.py` to check if a negative `maxfeerate` is input into `check_mempool_result` Asserts "Amount out of range" error message and `-3` error code Motivated by this [comment](https://github.com/bitcoin/bitcoin/pull/29434/files#r1491112250) ACKs for top commit: maflcko: lgtm ACK bf264e0 brunoerg: nice, utACK bf264e0 davidgumberg: Looks great, ACK bitcoin@bf264e0 Tree-SHA512: 58931b774cc887c616f2fd91af3ee65cc5db55acd8e2875c76de448c80bd4e020b057c5f4f85556431377f0d0e7553771fb285d1ec20cf64f64ec92a47776b78
…void intermittent test failure 4444de1 test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure (MarcoFalke) fa6aa40 test: Fix typos and use names args (MarcoFalke) Pull request description: Otherwise, the test may fail on slow hardware when running in valgrind. Also, use named args for the absolute timepoint, while touching this file. ACKs for top commit: tdb3: ACK for 4444de1 AngusP: re-ACK 4444de1 Tree-SHA512: 660269c8dd18887d69b284f38656899caf028159ce83ddf921f3e9c080a5d0e663989f0e42b4baf4c4939f20f34da0e7e844dff9b7c91d0cab570c60958bd0e1
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
…nsaction` ab98e6f test: add coverage for errors for `combinerawtransaction` RPC (brunoerg) Pull request description: This PR adds test coverage for the following errors for the `combinerawtransaction` RPC: * Tx decode failed * Missing transactions * Input not found or already spent For reference: https://maflcko.github.io/b-c-cov/total.coverage/src/rpc/rawtransaction.cpp.gcov.html ACKs for top commit: maflcko: lgtm ACK ab98e6f tdb3: ACK ab98e6f Tree-SHA512: 8a133c25dad2e1b236e0278a88796f60f763e3fd6fbbc080f926bb23f9dcc55599aa242d6e0c4ec15a179d9ded10a1f17ee5b6063719107ea84e6099f10416b2
24bc46c cli: Add warning for duplicate port definition (tdb3) e208fb5 cli: Sanitize ports in rpcconnect and rpcport (tdb3) Pull request description: Adds invalid port detection to bitcoin-cli for -rpcconnect and -rpcport. In addition to detecting malformed/invalid ports (e.g. those outside of the 16-bit port range, not numbers, etc.), bitcoin-cli also now considers usage of port 0 to be invalid. bitcoin-cli previously considered port 0 to be valid and attempted to use it to reach bitcoind. Functional tests were added for invalid port detection as well as port prioritization. Additionally, a warning is provided when a port is specified in both -rpcconnect and -rpcport. This PR is an alternate approach to PR bitcoin#27820 (e.g. SplitHostPort is unmodified). Considered an alternative to 127.0.0.1 being specified in functional tests, but at first glance, this might need an update to test_framework/util.py (e.g. rpc_url), which might be left to a future PR. ACKs for top commit: S3RK: light code review ACK 24bc46c achow101: ACK 24bc46c cbergqvist: re ACK 24bc46c Tree-SHA512: c83ab6a30a08dd1ac8b368a7dcc2b4f23170f0b61dd67ffcad7bcda05096d333bcb9821fba11018151f55b2929c0a333bfec15b8bb863d83f41fc1974c6efca5
….py to avoid intermittent test failure" This reverts commit eb544b3.
…d rpcport" This reverts commit 542b303.
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
7 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