905dfdee86d679f8ea31d841bceb77a5724a6b1b test: use ModuleNotFoundError in interface_ipc.py (fanquake)
Pull request description:
Change this so we catch the case where the capnp shared libs have been updated, and can no-longer be loaded by the Python module, resulting in a skipped test, even though pycapnp is installed. i.e:
```bash
stderr:
Traceback (most recent call last):
File "/root/ci_scratch/build/test/functional/interface_ipc.py", line 20, in <module>
import capnp # type: ignore[import] # noqa: F401
^^^^^^^^^^^^
File "/usr/local/lib64/python3.14/site-packages/capnp/__init__.py", line 36, in <module>
from .version import version as __version__
File "/usr/local/lib64/python3.14/site-packages/capnp/version.py", line 1, in <module>
from .lib.capnp import _CAPNP_VERSION_MAJOR as LIBCAPNP_VERSION_MAJOR # noqa: F401
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ImportError: libcapnpc.so.1.0.1: cannot open shared object file: No such file or directory
```
Failing in this way should make it clear that `pycapnp` needs to be reinstalled/rebuilt.
If `pycapnp` is not installed, the test still skips as expected:
```bash
Remaining jobs: [interface_ipc.py]
1/1 - interface_ipc.py skipped (capnp module not available.)
TEST | STATUS | DURATION
interface_ipc.py | ○ Skipped | 0 s
```
Fixes: #34016.
ACKs for top commit:
maflcko:
lgtm ACK 905dfdee86d679f8ea31d841bceb77a5724a6b1b
hebasto:
ACK 905dfdee86d679f8ea31d841bceb77a5724a6b1b, I have reviewed the code and it looks OK. However, I'm [not able](https://github.com/bitcoin/bitcoin/issues/34016#issuecomment-3799532047) to reproduce https://github.com/bitcoin/bitcoin/issues/34016.
sedited:
ACK 905dfdee86d679f8ea31d841bceb77a5724a6b1b
Tree-SHA512: 3cedbe8fc51cc18f1c993f7747d20905f3bf94c736db99a9c4090f5823bf8c09dfbc19ef03c573d504dcdfba6ea0f7d088a7f4563b220742c9a441167c04cfd6
fa9c92d7b6394194696e014e48346a8b8562e16a log: Print warning about privacy-sensitive log info unconditionally (MarcoFalke)
Pull request description:
There is a warning about logs containing privacy-sensitive information. However, it is only printed when at least one debug log category is enabled.
This is confusing, because:
* Setting (let's say) `-debug=reindex` enables this warning, but it is hard to see what sensitive logs could be contained in reindex debug logs.
* Dropping `-debug=reindex` again disabled this warning, but the wallet continues to log txids (and other sensitive stuff) at info level.
So instead of implying the wrong thing, it would be better to remove this log line (because it should be common sense), or log it unconditionally.
ACKs for top commit:
l0rinc:
ACK fa9c92d7b6394194696e014e48346a8b8562e16a
sedited:
ACK fa9c92d7b6394194696e014e48346a8b8562e16a
Tree-SHA512: 42f71b030e7722203f225f04e979143e829dae3556f64e322a791361a3b9c16150d53bb7bb9a99839c975d9052115770b9473138acc58baeee457253526fd892
ab649ce459450533ffdf068b00472259b72cf07a guix: documented shasum gathering command (janb84)
Pull request description:
When a PR requires proof of Guix builds (sha256sums), the PR author or reviewer uses a not well documented command to collect the sha256sums of build outputs or manually gathers them from files.
This pull request introduces a new section in the documentation, providing some documentation on the command's functionality and usage.
ACKs for top commit:
willcl-ark:
ACK ab649ce459450533ffdf068b00472259b72cf07a
sedited:
ACK ab649ce459450533ffdf068b00472259b72cf07a
Tree-SHA512: 0188663ad117b636c7d32a1b655db97610f558cfcffe4abd6f0fb097b3990db0dc6d23ab972926fefd2531b21f429742dcbea6b0fa579d22d5da7a7d6a4c753e
fa578d9434fdb090d27c7b5598dcd7f0ff0965cc lint: [move-only] Move python related lints to lint_py.rs (MarcoFalke)
fa392c31e7b9a487b61c881b06bb1e3787dcf4d9 lint: [move-only] Move repo related lints to lint_repo_hygiene.rs (MarcoFalke)
fab0cfa987c916a17c9cdd57636798866a56d74d lint: [move-only] Move cpp related lints to lint_cpp.rs (MarcoFalke)
fa3e48e3fd4d01dd8453994d214c24297aa83f62 lint: [move-only] Move docs related lints to lint_docs.rs (MarcoFalke)
fad09e77dbe521eb35206cd72536619225b18550 lint: [move-only] Move text related lints to text_format.rs (MarcoFalke)
faf40c2f848d7b5542862a4fd10b41cc6d418a90 lint: [move-only] Move util functions to util.rs (MarcoFalke)
Pull request description:
The single, large `main.rs` file is fine, but at some point it becomes harder to read.
So reduce the size by pulling functions out into modules.
This can be reviewed with the git option: `--color-moved=dimmed-zebra`
ACKs for top commit:
l0rinc:
Lightly tested code review ACK fa578d9434fdb090d27c7b5598dcd7f0ff0965cc
sedited:
ACK fa578d9434fdb090d27c7b5598dcd7f0ff0965cc
Tree-SHA512: f1e29fd3cf695fb6634d0b9f9e55508992b4b9885afee9dbe4d5d9e99cad3061e7141f39acbfe69d698422888169128cd7658a6dc991fd904b8520328b51586d
fdc9fe2da6a8640b11a2871f8b653764652f8c1f ci, iwyu: Fix warnings in `src/primitives` and treat them as errors (Hennadii Stepanov)
Pull request description:
This PR [continues](https://github.com/bitcoin/bitcoin/pull/33725#issuecomment-3466897433) the ongoing effort to enforce IWYU warnings.
See [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-iwyu).
ACKs for top commit:
maflcko:
review ACK fdc9fe2da6a8640b11a2871f8b653764652f8c1f 📀
janb84:
ACK fdc9fe2da6a8640b11a2871f8b653764652f8c1f
sedited:
ACK fdc9fe2da6a8640b11a2871f8b653764652f8c1f
Tree-SHA512: d290545c7aab477b4a5bf121b694899a78e0526be72efa31fa4205b0fd840e6e8240d32f9134a18c9dc58c5f91e7847d7f20ca34f8d2edc4d541ac858ec0dccc
c8abac994122b67de8145579489576db5df2b3d3 ci: mount .git dir rw (ci)
Pull request description:
On merges to master we set LINT_CI_SANITY_CHECK_COMMIT_SIG (when "GITHUB_REPOSITORY == bitcoin/bitcoin") which runs verify-commits.py.
This requires write access to the .git directory.
Make the mounted .git directory writable.
This is currently not run on PR branches or locally which caused a miss during review.
Ideally we can have the same checks running in PRs as on merges to master to avoid future discrepancies like this.
ACKs for top commit:
maflcko:
lgtm ACK c8abac994122b67de8145579489576db5df2b3d3
l0rinc:
untested code review ACK c8abac994122b67de8145579489576db5df2b3d3
Tree-SHA512: 7ae4f63227ecffe1dc9003454a7473d6d592550af2e1c899457f34a947e5604b04c13319fb8979f36789ae7787bed62066be60697d163ad5ebedde3fbe8ce45f
On merges to master we set LINT_CI_SANITY_CHECK_COMMIT_SIG (when
"GITHUB_REPOSITORY == bitcoin/bitcoin") which runs verify-commits.py.
This requires write access to the .git directory.
Make the mounted .git directory writable.
This is currently not run on PR branches or locally which caused a miss
during review.
3400db80401d65ba16b52e5055486c75cd1412ff doc: add missing param description to SRD (yancy)
Pull request description:
The params documentation is missing `change_fee` and the description is lacking recent changes.
ACKs for top commit:
murchandamus:
ACK 3400db80401d65ba16b52e5055486c75cd1412ff
brunoerg:
code review ACK 3400db80401d65ba16b52e5055486c75cd1412ff
Tree-SHA512: 8f6fac0d92873c5c9f77b19fbc0c6ecfb425b2a6b3d5f5ad69c82ed706b21cf4627e68c71acbc43661000e6063e8f8dbcd3b8ff60e3c727bdcba497d13ee1383
ddae1b4efa568b6099d5bece941f8f2733faf29e ci: remove gnu-getopt usage (fanquake)
Pull request description:
This is used for argument parsing in the `retry` script, however we don't use the script with any arguments. So remove the unused code, and the dependency on `gnu-getopt`.
This came up in the context of adding new CI jobs, where gnu-getopt might not be available, or working properly. It seemed easier to just remove the unused code, than look for more workarounds.
ACKs for top commit:
maflcko:
review ACK ddae1b4efa568b6099d5bece941f8f2733faf29e 🔀
sedited:
ACK ddae1b4efa568b6099d5bece941f8f2733faf29e
Tree-SHA512: a73cf61fe0965127f87f1725b3a25a305ebfd354c318f5f44ecfa20da02ba72fef42dca656dae07f6e1ece956b9d7c58e99edb124d968a4bffb2ce6ac8fc018b
fad7bd9ba3eef03fcdd7cb17011ea0c6e483c767 noui: Remove always empty caption while formatting (MarcoFalke)
fa8ebeb332325604e8ca6080262543e10de4e46c refactor: [gui] Document that the title is always empty for node message (MarcoFalke)
fafe71b743a0637d16812d26430d99464cab0cee refactor: Remove empty caption from ThreadSafeMessageBox (MarcoFalke)
fa8d0088e76d4def59dff92bfb2ebbfc6cd4c195 refactor: Remove empty caption from ThreadSafeQuestion (MarcoFalke)
fa0195499ca611b513d9d1986d79c5e3a58cd0f2 refactor: [gui] Use lambdas over std::bind (MarcoFalke)
eeee1e341fa59b5b0b05f974105104fb2a0df9c3 refactor: Remove trailing semicolon after ADD_SIGNALS_DECL_WRAPPER (MarcoFalke)
Pull request description:
Currently, the user interface (noui, gui) has a caption for each message. However, the caption has many issues:
* It is always hard-coded to the empty string.
* This is confusing and tedious when reading or maintaining the code.
* It is redundant, because `noui` will ignore the caption and set the logging prefix (error, warning, info) based on the `style`.
* The gui does prefer to set the title based on the caption, but since it the caption is always empty, the fallback will always be used.
Fix all issues by removing it.
ACKs for top commit:
hebasto:
ACK fad7bd9ba3eef03fcdd7cb17011ea0c6e483c767, I have reviewed the code and it looks OK. Tested on Ubuntu 25.10.
sedited:
ACK fad7bd9ba3eef03fcdd7cb17011ea0c6e483c767
Tree-SHA512: 58ef538b9b3e1cfdcf2955f6de9b8cee335edbf6339723cb693cb4d584817904c962dac5199ee44d7e2860a5332dec1a6abf47e621eb5cf919aa1cdae271b55f
477c5504e05f9031449cdbf62bf329eac427cb0c coins: replace `std::distance` with unambiguous pointer subtraction (Lőrinc)
Pull request description:
### Problem
Calling `std::distance(nullptr, nullptr)` has ambiguous status in the C++ standard [iterator.requirements.general](https://eel.is/c++draft/iterator.requirements.general#7):
> Iterators can also have singular values that are not associated with any sequence. Results of most expressions are undefined for singular values.
It seems to work correctly in every implementation we use, but [LWG 1213](https://cplusplus.github.io/LWG/issue1213) ("Meaning of valid and singular iterator underspecified") has been Open since 2009, acknowledging that the standard's wording on this topic is unclear.
<details>
<summary>Details</summary>
The [iterator.requirements.general](https://eel.is/c++draft/iterator.requirements.general#7) states:
> Iterators can also have singular values that are not associated with any sequence. Results of most expressions are undefined for singular values.
And [LWG 208](https://cplusplus.github.io/LWG/issue208)'s rationale explicitly confirms:
> Null pointers are singular.
Therefore they cannot form a valid range required by [std::distance](https://eel.is/c++draft/iterator.operations#4):
> Preconditions: last is reachable from first, or InputIterator meets the Cpp17RandomAccessIterator requirements and first is reachable from last.
</details>
### Fix
A previous version of this PR checked both values for `nullptr`, the current one uses unambiguously well-defined pointer subtraction instead, which is per [expr.add](https://eel.is/c++draft/expr.add#5):
> If P and Q both evaluate to null pointer values, the value is 0.
This applies on the first call before any memory is allocated, when both pointers are `nullptr`.
Using `operator-` directly is simpler and avoids the ambiguity entirely.
ACKs for top commit:
maflcko:
review ACK 477c5504e05f9031449cdbf62bf329eac427cb0c 🍶
optout21:
ACK 477c5504e05f9031449cdbf62bf329eac427cb0c
sedited:
ACK 477c5504e05f9031449cdbf62bf329eac427cb0c
Tree-SHA512: 5edfb19ab4820e2003928f60f20d4a5893bcd3c316afdfe91c9c06e9b465352769b2cddb0d0e2419ea083a906d35f4aada74149e81f4ea0315f8173ac538789f
5aeaa71c77ac31a1b05f8361356d2810cfc5bc28 lint: pass args from lint.py to cargo run in container (will)
c17a2adb8dc0e9706319ddecc42737be4268ac6a lint: upgrade lint scripts for worktrees (will)
Pull request description:
Fixes#29972
Use a single script to run the linter locally or in CI.
Works from inside a worktree.
ACKs for top commit:
maflcko:
review ACK 5aeaa71c77ac31a1b05f8361356d2810cfc5bc28 🔒
davidgumberg:
code review and lightly tested reACK 5aeaa71
l0rinc:
Tested (+ lightly reviewed) ACK 5aeaa71c77ac31a1b05f8361356d2810cfc5bc28
Tree-SHA512: 7c11f649b4752739d31c4f9e6306a98bd2e615b27a0819bbb5e7d9284b9e28bd9f424e145f16361f672f1a63441a1ae2f901c4f99759e997b72a4bf2d56d8d39
fa2e1b85dd6b263a77294566bd361d7873731045 build: Remove outdated comment about -ffile-prefix-map (MarcoFalke)
fa06cd4ba7303bd3036467cf00d12f36c94bcaff doc: Remove outdated -fdebug-prefix-map section in dev notes (MarcoFalke)
Pull request description:
This removes some docs. See the commit messages for an explanation.
ACKs for top commit:
l0rinc:
ACK fa2e1b85dd6b263a77294566bd361d7873731045
sedited:
ACK fa2e1b85dd6b263a77294566bd361d7873731045
Tree-SHA512: 6be33bdf9365be5fb75d39a48fd1295b193649775a00e8344123dc0f588da22f7efe80b1490dde2c74aea3d7fec6a3fa75785791296f3fb248ddf45e40b95eb7
e71c4df1685131f5ab48aac6ccb07ac944e91e9f refactor: replace manual promise with SyncWithValidationInterfaceQueue (ANtutov)
Pull request description:
`BroadcastTransaction()` now waits for validation callbacks using the built-in `validation_signals>SyncWithValidationInterfaceQueue()` instead of creating a local `std::promise` and scheduling a lambda. This removes an unnecessary allocation and uses the canonical API.
ACKs for top commit:
maflcko:
review ACK e71c4df1685131f5ab48aac6ccb07ac944e91e9f 🌃
rkrux:
lgtm ACK e71c4df1685131f5ab48aac6ccb07ac944e91e9f
sedited:
ACK e71c4df1685131f5ab48aac6ccb07ac944e91e9f
Tree-SHA512: 602994ba3c2ac91996068aee6eac7e788c3832d7ab949519a9420d2b59e2a67d2d4e67c3c9191ba60e9caa75f1524a95b0851fcd40b6732f6a9956a011b4a120
This is used for argument parsing in the retry script, however we don't
use the script with any arguments. So remove the unused code, and the
dependency on gnu-getopt.
This came up in the context of adding new CI jobs, where gnu-getopt
might not be available, or working properly. It seemed easier to just
remove the unused code, than look for more workarounds.
14f99cfe53f07280b6f047844fc4fba0da8cd328 rpc: make `uptime` monotonic across NTP jumps (Lőrinc)
a9440b1595be7053b17895f7ee36652bac24be6e util: add `TicksSeconds` (Lőrinc)
Pull request description:
### Problem
`bitcoin-cli uptime` was derived from wall-clock time, so it could jump by large amounts when the system clock is corrected after `bitcoind` starts (e.g. on RTC-less systems syncing NTP).
This breaks the expectation that uptime reflects process runtime.
### Fix
Compute uptime from a [monotonic clock](https://en.cppreference.com/w/cpp/chrono/steady_clock.html) so it is immune to wall-clock jumps, and use that monotonic uptime for the RPC.
GUI startup time is derived from wall clock time minus monotonic uptime so it remains sensible after clock corrections.
### Reproducer
Revert the fix commit and run the `rpc_uptime` functional test (it should fail with `AssertionError: uptime should not jump with wall clock`):
Or alternatively:
```bash
cmake -B build && cmake --build build --target bitcoind bitcoin-cli -j$(nproc)
DATA_DIR=$(mktemp -d)
./build/bin/bitcoind -regtest -datadir="$DATA_DIR" -connect=0 -daemon
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" -rpcwait uptime
sleep 1
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" setmocktime $(( $(date +%s) + 20000000 ))
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" uptime
./build/bin/bitcoin-cli -regtest -datadir="$DATA_DIR" stop
```
<details>
<summary>Before (uptime jumps with wall clock)</summary>
```bash
Bitcoin Core starting
0
20000001
Bitcoin Core stopping
```
</details>
<details>
<summary>After (uptime stays monotonic)</summary>
```bash
Bitcoin Core starting
0
1
Bitcoin Core stopping
```
</details>
----------
Issue: https://github.com/bitcoin/bitcoin/issues/34326
ACKs for top commit:
maflcko:
review ACK 14f99cfe53f07280b6f047844fc4fba0da8cd328 🎦
willcl-ark:
tACK 14f99cfe53f07280b6f047844fc4fba0da8cd328
w0xlt:
ACK 14f99cfe53f07280b6f047844fc4fba0da8cd328
sedited:
ACK 14f99cfe53f07280b6f047844fc4fba0da8cd328
Tree-SHA512: 3909973f58666ffa0b784a6df087031b9e34d2022d354900a4dbb6cbe1d36285cd92770ee71350ebf64d6e8ab212d8ff0cd851f7dca1ec46ee2f19b417f53984
Add a ci/lint.py script to run the linter both locally or inside the CI
(replacing .github/ci-lint-exec.py) which supports running from a
worktree.
Determines whether we are in a worktree, and mounts the real `.git`
directory as a read-only volume if we are.
b39291f4cde03d5aa7936bf5aa7cc4fa18f65cad doc: fix `-logips` description to clarify that non-debug logs can also contain IP addresses (Lőrinc)
c7028d3368e90fef2dd2a7ae68877767d602eff0 init: log that additional logs may contain privacy-sensitive information (Lőrinc)
31b771a9425dace38582e0de0fb468f388df170c net: move `privatebroadcast` logs to debug category (Lőrinc)
Pull request description:
### Motivation
The recently merged [private broadcast](https://github.com/bitcoin/bitcoin/pull/29415) is a privacy feature, and users may share `debug.log` with support.
Unconditional `LogInfo()` messages that mention private broadcast and/or include (w)txids can leak sensitive context (e.g. which transactions a user originated).
Since it's meant to be a private broadcast, we should minimize leaks.
It's a best effort, it's not invalidated by other logs possibly leaking identifiable information, those can be addressed separately.
We're not promising that the logs won't ever contain data that could be used against the user, but we should still try to minimize that data, especially for a feature that's advertised as privacy-focused.
Follow up to [#29415 (comment)](https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2637012294)
### Changes
* Move private-broadcast event logs from `LogInfo()` to `LogDebug(BCLog::PRIVBROADCAST, ...)`, so they are only emitted when `-debug=privatebroadcast` was explicitly provided.
* Remove hardcoded `"[privatebroadcast]"` log-string prefixes (category logging already adds the prefix).
* Keep warning at the default log level for startup failures.
* Add an init log (not a warning since that would require excessive test framework updates) when any `-debug` categories are enabled that additional logs may contain privacy-sensitive information and should not be shared publicly.
* Update a related startup arg (`-logips`) to clarify that clarify that non-debug logs can also contain IP addresses.
### Reproducer
The new warning can be checked with:
```bash
./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 | grep 'Debug logging is enabled' | wc -l
0
./build/bin/bitcoind -printtoconsole=1 -stopatheight=1 -listen=0 -connect=0 -debug | grep 'Debug logging is enabled' | wc -l
1
```
ACKs for top commit:
janb84:
re ACK b39291f4cde03d5aa7936bf5aa7cc4fa18f65cad
vasild:
ACK b39291f4cde03d5aa7936bf5aa7cc4fa18f65cad
andrewtoth:
ACK b39291f4cde03d5aa7936bf5aa7cc4fa18f65cad
frankomosh:
crACK b39291f4cde03d5aa7936bf5aa7cc4fa18f65cad .The approach and implementation look good. Moving private broadcast logs to debug only would effectively reduce privacy leaks for users sharing logs.
sedited:
ACK b39291f4cde03d5aa7936bf5aa7cc4fa18f65cad
Tree-SHA512: feca25ebe72a03948ba436e25f9a682947966c4c09627e8f20201ef3872ddbce1c636cd82f06be1afdc09cb80da305058667c0c2eaeadeb351311155325ea06f
3f5211cba8e73e8eb03781e6ec32ba9c4a263782 test: remove child_one/child_two (w)txid variables (naiyoma)
7cfe790820cf247e8a27bb8091defc54c74d6aec test: replace ValidWitnessMalleatedTx class with function (naiyoma)
81675a781f3ab62a0576a9739d13b4997b63230d test: use pre-generated chain (naiyoma)
Pull request description:
This PR refactors ` ValidWitnessMalleatedTx` class into a `build_malleated_tx_package` function. As a result, two tests are updated: `mempool_accept_wtxid` and `p2p_p2p_private_broadcast`. Also included are a few small refactors in mempool_accept_wtxid , (switching to MiniWallet, using a pre-mined chain, using txid directly.)
Together, these changes reduce complexity and improve test runtime.
ACKs for top commit:
stratospher:
reACK 3f5211c.
cedwies:
reACK 3f5211c
maflcko:
review ACK 3f5211cba8e73e8eb03781e6ec32ba9c4a263782 👥
rkrux:
ACK 3f5211cba8e73e8eb03781e6ec32ba9c4a263782
Tree-SHA512: 1fd02be3432fef6b68e54fbe8b15ed56d2699580bb13d0777b21f9cbe4c6d33bbb710541e3ca2fc93eab771d17bf1c427e4b08fa216d561bdb320cc6b36ac8fc
14e56970cb Merge bitcoin-core/secp256k1#1794: ecmult: Use size_t for array indices
c7a52400d6 Merge bitcoin-core/secp256k1#1809: release cleanup: bump version after 0.7.1
ae7eb729c0 release cleanup: bump version after 0.7.1
1a53f4961f Merge bitcoin-core/secp256k1#1808: Prepare for 0.7.1
20a209f11c release: prepare for 0.7.1
c4b6a81a60 changelog: update in preparation for the v0.7.1 release
ebb35882da Merge bitcoin-core/secp256k1#1796: bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS
c09215f7af bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS
471e3a130d Merge bitcoin-core/secp256k1#1800: sage: verify Eisenstein integer connection for GLV constants
29ac4d8491 sage: verify Eisenstein integer connection for GLV constants
4721e077b4 Merge bitcoin-core/secp256k1#1793: doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult
bd5ced1fe1 doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult
47eb70959a ecmult: Use size_t for array indices in _odd_multiplies_table
bb1d199de5 ecmult: Use size_t for array indices into tables
2d9137ce9d Merge bitcoin-core/secp256k1#1764: group: Avoid using infinity field directly in other modules
f9a944ff2d Merge bitcoin-core/secp256k1#1790: doc: include arg -DSECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS=ON for cmake
0406cfc4d1 doc: include arg -DUSE_EXTERNAL_DEFAULT_CALLBACKS=1 for cmake
8d445730ec Merge bitcoin-core/secp256k1#1783: Add VERIFY_CHECKs and documentation that flags must be 0 or 1
aa2a39c1a7 Merge bitcoin-core/secp256k1#1778: doc/bench: Added cmake build options to bench error messages
540fec8ae9 Merge bitcoin-core/secp256k1#1788: test: split monolithic ellswift test into independent cases
d822b29021 test: split monolithic ellswift test into independent cases
ae00c552df Add VERIFY_CHECKs that flags are 0 or 1
5c75183344 Merge bitcoin-core/secp256k1#1784: refactor: remove ret from secp256k1_ec_pubkey_serialize
be5e4f02fd Merge bitcoin-core/secp256k1#1779: Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL
3daab83a60 refactor: remove ret from secp256k1_ec_pubkey_serialize
8bcda186d2 test: Add non-NULL checks for "pointer of array" API functions
5a08c1bcdc Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL
3b5b03f301 doc/bench: Added cmake build options to bench error messages
e7f7083b53 Merge bitcoin-core/secp256k1#1774: refactor: split up internal pubkey serialization function into compressed/uncompressed variants
b6c2a3cd77 Merge bitcoin-core/secp256k1#1761: ecmult_multi: reduce strauss memory usage by 30%
f5e815f430 remove secp256k1_eckey_pubkey_serialize function
0d3659c547 use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig)
adb76f82ea use new `_eckey_pubkey_serialize{33,65}` functions in public API
fc7458ca3e introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions
c8206b1ce6 Merge bitcoin-core/secp256k1#1771: ci: Use Python virtual environment in "x86_64-macos-native" job
f252da7e6e ci: Use Python virtual environment in "x86_64-macos-native" job
115b135fe8 Merge bitcoin-core/secp256k1#1763: bench: Use `ALIGNMENT` macro instead of hardcoded value
2f73e5281d group: Avoid using infinity field directly in other modules
153eea20c2 bench: Use `ALIGNMENT` macro instead of hardcoded value
26166c4f5f ecmult_multi: reduce strauss memory usage by 30%
7a2fff85e8 Merge bitcoin-core/secp256k1#1758: ci: Drop workaround for Valgrind older than 3.20.0
43e7b115f7 Merge bitcoin-core/secp256k1#1759: ci: Switch to macOS 15 Sequoia Intel-based image
8bc50b72ff ci: Switch to macOS 15 Sequoia Intel-based image
c09519f0e3 ci: Drop workaround for Valgrind older than 3.20.0
git-subtree-dir: src/secp256k1
git-subtree-split: 14e56970cba37ffe4ee992c1e08707a16e22e345
a73a3ec5532ddc05c1b013d868d9994f2889c9cf doc: fix invalid arg name hints for bugprone validation (Lőrinc)
Pull request description:
The extra leading `=` or missing trailing `=` prevented clang-tidy's `bugprone-argument-comment` check from validating the parameter name, as it only matches comments formatted strictly as `/*parameter_name=*/` (see https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html).
I have considered doing a scripted diff, but the values I found aren't so numerous and can easily be reviewed manually.
ACKs for top commit:
b-l-u-e:
ACK a73a3ec tested and saw that argument comments now use the strict "/*param=*/" format required by bugprone-argument-comment
Sjors:
utACK a73a3ec5532ddc05c1b013d868d9994f2889c9cf
maflcko:
review ACK a73a3ec5532ddc05c1b013d868d9994f2889c9cf 🍦
Tree-SHA512: 31177934d645116f381668a0f945028d7e04fab1fc6185dd0e3b7451aab71f89f1e4dd07246db667d1c4734eea3e5d73433c8b0e09181b3ece47dacc8677401e
The -ffile-prefix-map option is no longer used and it seems fine to
remove the warning about it possibly breaking coverage builds.
If this needs documentation, the dev notes seem like a better place,
because it also affects other places, such as depends. C.f. commit
407062f2ac93624f350e9e8a4f641c882a2aaf2f
The section claims to be for ccache builds, however those are already
fixed after commit 1cc58d3a0c653ac30df04d1010a3cf84c6bc307a.
If there are still any build or debug problems after that commit,
dedicated instructions can be added back, along with exact steps to
reproduce and test.
fa37928536e0048a262260baf998ead026b14bb9 build: Temporarily remove confusing and brittle -fdebug-prefix-map (MarcoFalke)
Pull request description:
The compiler option `-fdebug-prefix-map` is unconditionally set by the build system. This is problematic for many reasons:
* Users and devs have no easy way to disable it without modifying the build system source code
* The mapping is broken since the cmake migration, and requires manual fixups such as https://github.com/bitcoin/bitcoin/issues/31204 or https://github.com/bitcoin/bitcoin/issues/31957
Fix all issues by temporarily removing it.
Though, the option is kept for the guix build, so that no change in behavior is observed for the release binaries.
Fixes https://github.com/bitcoin/bitcoin/issues/31957
Fixes https://github.com/bitcoin/bitcoin/issues/31204
The option can be added back in the future, if there is any need to. Though, adding it back should ideally work out of the box, or at least provide easy workarounds for all commonly used tooling.
ACKs for top commit:
pinheadmz:
ACK fa37928536e0048a262260baf998ead026b14bb9
l0rinc:
ACK fa37928536e0048a262260baf998ead026b14bb9
hebasto:
ACK fa37928536e0048a262260baf998ead026b14bb9.
Tree-SHA512: 5c76faab36ec516b286c2b5b2404e1488c0c4fbc678904593b0acb9c8da9b1db1b41436a22e6aa2f2671650288ccf635554773ef3144dc1df6ea838afce07ecb
Change this so we catch the case where the capnp shared libs have been
updated, and can no-longer be loaded by the Python module, resulting in
a skipped test, even though pycapnp is installed. i.e:
```bash
stderr:
Traceback (most recent call last):
File "/root/ci_scratch/build/test/functional/interface_ipc.py", line 20, in <module>
import capnp # type: ignore[import] # noqa: F401
^^^^^^^^^^^^
File "/usr/local/lib64/python3.14/site-packages/capnp/__init__.py", line 36, in <module>
from .version import version as __version__
File "/usr/local/lib64/python3.14/site-packages/capnp/version.py", line 1, in <module>
from .lib.capnp import _CAPNP_VERSION_MAJOR as LIBCAPNP_VERSION_MAJOR # noqa: F401
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ImportError: libcapnpc.so.1.0.1: cannot open shared object file: No such file or directory
```
Failing in this way should make it clear that `pycapnp` needs to be
reinstalled/rebuilt.
If `pycapnp` is not installed, the test still skips as expected:
```bash
Remaining jobs: [interface_ipc.py]
1/1 - interface_ipc.py skipped (capnp module not available.)
TEST | STATUS | DURATION
interface_ipc.py | ○ Skipped | 0 s
```
Fixes: #34016.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
eeee3755f8c415b227820479b5492261f3a8aa08 fuzz: Return chrono point from ConsumeTime(), Add ConsumeDuration() (MarcoFalke)
faa5a9ebad15fe41e8ddf45f11ad72bdc5aabf99 fuzz: Use min option in ConsumeTime (MarcoFalke)
Pull request description:
Returning a raw i64 is a bit confusing when it comes to chrono types. For example, in the addrman fuzz tests, the `time_penalty` is not a time point, but a duration.
Also, all call-sites assume second resolution right now, so document that better by returning `NodeSeconds` from `ConsumeTime(...)` and `std::chrono::seconds` from `ConsumeDuration(...)`.
ACKs for top commit:
l0rinc:
ACK eeee3755f8c415b227820479b5492261f3a8aa08
Crypt-iQ:
crACK eeee3755f8c415b227820479b5492261f3a8aa08
Tree-SHA512: 25dd779a1bf79fa42c6e69db0f0593ad4daa4c0d746e8e82a26bdd65391a27c38e484431056d4e2207b542c511a71cb536c259809728a7166b8d304c0490e321
ccf9172ab3bbd6d6979acb9b02bc36ca55ab031f util: Remove `FilterHeaderHasher` (rustaceanrob)
Pull request description:
With respect to `std::unordered_map` documentation, the `Hash` type
defined in the template is over the `Key` and not `T`, the value. This
hasher is incorrectly named as the `FilterHeader` is the value within this map.
I consider this a bug as opposed to a refactor as the key and value
relationship is implied to be `filter header -> block hash` when it is
the opposite.
Further, the hasher for the key already exists via `BlockHasher`.
ref: https://en.cppreference.com/w/cpp/container/unordered_map.html
ACKs for top commit:
andrewtoth:
ACK ccf9172ab3bbd6d6979acb9b02bc36ca55ab031f
maflcko:
lgtm ACK ccf9172ab3bbd6d6979acb9b02bc36ca55ab031f
ismaelsadeeq:
ACK ccf9172ab3bbd6d6979acb9b02bc36ca55ab031f 👍🏾
Tree-SHA512: 607602391bf337d4e25b04a6a643fa32c3ab4599009b181b46ecdb0705e8ff2af89a6192042453c9e8e44abcb2150589019f02c5c944ecdff41322c3e0ad45ac
fa15a8d2d03b0099b97262e47a8cbf685c29dd49 doc: Explain that low-effort pull requests may be closed (MarcoFalke)
Pull request description:
Lately, there seems to be a rise in low-effort pull requests. For example, where a contributor does not seem to understand the changes they are submitting, or it becomes clear that they have not tested the changes at all.
I don't think such pull requests are helpful, as they extract precious review time, which could be better spent on reviewing pull requests by reviewers who care about understanding the changes they are submitting, and who ensure their changes are sound and tested.
So document that such low-effort pull request may be closed.
ACKs for top commit:
l0rinc:
ACK fa15a8d2d03b0099b97262e47a8cbf685c29dd49
willcl-ark:
ACK fa15a8d2d03b0099b97262e47a8cbf685c29dd49
dergoegge:
ACK fa15a8d2d03b0099b97262e47a8cbf685c29dd49
pinheadmz:
ACK fa15a8d2d0
Tree-SHA512: ba880f61c90c95e1e9007e337bad1a612a53ca85448f0ebfe97b34139489f22e5f709b8a0e302b11f71213e3b7863ab36ebd89b5c11cd550022d96493f917dd7
b261100e71697dd4859cdeb5a69f5a1d6c557099 [qt] Set peer version and subversion to N/A when not available or detecting (WakeTrainDev)
Pull request description:
In the debug console peer detail window, display "N/A" for the User Agent and Version when the peer is still detecting or the information is unavailable, instead of retaining the previous values.
ACKs for top commit:
maflcko:
lgtm ACK b261100e71697dd4859cdeb5a69f5a1d6c557099
luke-jr:
utACK b261100e71697dd4859cdeb5a69f5a1d6c557099
Tree-SHA512: ffcba716fe6173062fe00e2d428d41bbdcaebfe8c76c804519e46a448ade2785ae32efb1a30322adc19cf29e07ea8ab4d7593ef5a17b6c418c8dd77b381e4f77
be2b48b9f3e56b672e391e18a09f2d5f4e78d74e test: allow overriding tar in get_previous_releases (fanquake)
Pull request description:
Facilitate use on distros that might have `tar` as something else, such as `gtar`, i.e Chimera.
ACKs for top commit:
maflcko:
lgtm ACK be2b48b9f3e56b672e391e18a09f2d5f4e78d74e
hebasto:
ACK be2b48b9f3e56b672e391e18a09f2d5f4e78d74e, I have reviewed the code and it looks OK.
sedited:
ACK be2b48b9f3e56b672e391e18a09f2d5f4e78d74e
Tree-SHA512: e5da14ef17f37dc52e438f191efbe9041477246bab04b9e166cf2104389e97bfc6b41005c9d79a99f193e8d7b24ecd32dbd8344c65878e92fd94c77d3efbbe8c
40735450c00b10baa03e3a7f1e2bee439077e356 Remove unused epochguard.h (Suhas Daftuar)
1a8494d16c7b1c21dec384438c18ac08a469bb61 Rework CTxMemPool::GetChildren() to not use epochs (Suhas Daftuar)
Pull request description:
Since #33591, the epoch-based graph traversal optimization logic is only used for `CTxMempool::GetChildren()`, a function that is only used in RPC code and tests. Rewrite it without epochs, and remove `util/epochguard.h` itself, as that was its last use.
This allows us to reduce per-transaction memory usage by 8 bytes, for no material loss. With the new TxGraph-based mempool implementation, I also don't foresee future uses for it, as TxGraph can do even better by using BitSet-based traversal tracking.
ACKs for top commit:
ajtowns:
ACK 40735450c00b10baa03e3a7f1e2bee439077e356
instagibbs:
ACK 40735450c00b10baa03e3a7f1e2bee439077e356
l0rinc:
code review ACK 40735450c00b10baa03e3a7f1e2bee439077e356
Tree-SHA512: 7ce7c04835cd2425a71c4fd47f316b6fb7381caa27383de7ecc4aa81100fcf7bc5e062699b307c08e0b853b35f06710d9ac761d6e660af9f9331e708d36f2fe0
9a9d797ef6ed8e1b3e876fc93cf1a6395ab270e9 kernel: Add support for block headers (yuvicc)
b851ff6cae71934bf2389d109908339d60ec6e5b kernel: Add Handle/View pattern for BlockValidationState (yuvicc)
Pull request description:
Adds a new `btck_BlockHeader` type and associated functions to create, access, and validate block headers. Block headers will have their own type (`btck_BlockHeader`) that can be created from raw data, copied, and queried for all the standard header fields (hash, prev hash, timestamp, bits, version, nonce). We can also extract headers from full blocks or block tree entries.
The first commit here refactors `BlockValidationState` to use Handle/View pattern so external code can own them, which is required for the header processing in the API.
#### New Block Header API
- **`btck_BlockHeader` type**: Opaque handle for block headers
- **Header methods**:
- `btck_block_header_create()`: Create header from 80-byte serialized data
- `btck_block_header_copy()`: Copy block headers
- `btck_block_header_destroy()`: Destroy header object
- `btck_block_header_get_hash()`: Calculate block hash
- `btck_block_header_get_prev_hash()`: Get previous block hash
- `btck_block_header_get_timestamp()`: Get block timestamp
- `btck_block_header_get_bits()`: Get difficulty target (compact format)
- `btck_block_header_get_version()`: Get block version
- `btck_block_header_get_nonce()`: Get proof-of-work nonce
- `btck_block_get_header()`: Extract header from a full block
- `btck_block_tree_entry_get_block_header()`: Get header associated with a block tree entry
- **Header Processing Methods:**
- **`btck_chainstate_manager_process_block_header()`**: Validates and processes a block header without requiring the full block. This performs proof-of-work verification, timestamp validation, and updates the internal chain state.
- **`btck_chainstate_manager_get_best_entry()`**: Returns the block tree entry with the most cumulative proof-of-work.
Why `btck_chainstate_manager_get_best_entry()` is included alongside header validation? Just as we have logic to get the tip for block validation (so you can request more blocks extending your best from your peers), we need the equivalent for header validation. To make header validation worthwhile, knowing what the best current header is seems useful—it tells you what headers to request next from peers.
### Testing
Added tests in `test_kernel.cpp` that cover creating headers from raw data, extracting all header fields, and processing headers through the chainstate manager.
CC sedited
ACKs for top commit:
stringintech:
re-ACK 9a9d797e
sedited:
Re-ACK 9a9d797ef6ed8e1b3e876fc93cf1a6395ab270e9
janb84:
ACK 9a9d797ef6ed8e1b3e876fc93cf1a6395ab270e9
Tree-SHA512: 1dde9ef860543c906d1bb5e604f0d2956e7382fcbb55090686261b2277270a1fd3826f02ecf1749b2774da66e88f686c7845172b4c68b62259e7a7aee0825fa2
1911db8c6dc6b32c8971b14b2b271ec39d9f3ab9 string: add LineReader (Matthew Zipkin)
ee62405cce2bf3d14117bdb327832f12584968d6 time: implement and test RFC1123 timestamp string (Matthew Zipkin)
eea38787b9be99c3f192cb83fc18358397e4ab52 string: add AsciiCaseInsensitive{KeyEqual, Hash} for unordered map (Matthew Zipkin)
4e300df7123a402aef472aaaac30907b18a10c27 string: add `base` argument for ToIntegral to operate on hexadecimal (Matthew Zipkin)
0b0d9125c19c04c1fc19fb127d7639ed9ea39bec Modernize GetBindAddress() (Matthew Zipkin)
a0ca851d26f8a9d819708db06fec2465e9f6228c Make GetBindAddress() callable from outside net.cpp (Matthew Zipkin)
Pull request description:
This is a component of [removing libevent as a dependency of the project](https://github.com/bitcoin/bitcoin/issues/31194). It is the first six commits of #32061 and provides a string-parsing utility (`LineReader`) that is also consumed by #34158.
These are the functions that are added / updated for HTTP and Torcontrol:
- `GetBindAddress()`: Given a socket, provides the bound address as a CService. Currently used by p2p but moved from `net` to `netbase` so other modules can call it.
- `ToIntegral()`: Already used to parse numbers from strings, added new argument `base = 10` so it can also be used to parse hexadecimal integers. HTTP chunked transfer-encoding uses hex-encoded integers to specify payload size: https://datatracker.ietf.org/doc/html/rfc7230.html#section-4.1
- `AsciiCaseInsensitive` comparators: Needed to store HTTP headers in an `unordered_map`. Headers are key-value pairs that are parsed with case-insensitive keys: https://httpwg.org/specs/rfc9110.html#rfc.section.5.1
- `FormatRFC1123DateTime()`: The required datetime format for HTTP headers (e.g. `Fri, 31 May 2024 19:18:04 GMT`)
- `LineReader`: Fields in HTTP requests are newline-terminated. This struct is given an input buffer and provides methods to read lines as strings.
ACKs for top commit:
maflcko:
review ACK 1911db8c6dc6b32c8971b14b2b271ec39d9f3ab9 👲
furszy:
utACK 1911db8c6dc6b32c8971b14b2b271ec39d9f3ab9
sedited:
ACK 1911db8c6dc6b32c8971b14b2b271ec39d9f3ab9
Tree-SHA512: bb8d3b7b18f158386fd391df6d377c9f5b181051dc258efbf2a896c42e20417a1b0b0d4637671ebd2829f6bc371daa15775625af989c19ef8aee76118660deff
fab2f3df4beb230eef63bdcf5042b6417c0012dc fuzz: Exclude too expensive inputs in descriptor_parse targets (MarcoFalke)
Pull request description:
Accepting "expensive" fuzz inputs which have no real use-case is problematic, because it prevents the fuzz engine from spending time on the next useful fuzz input.
For example, those will take several seconds (!) and the flamegraph shows that base58 encoding is the cause:
```
curl -fLO 'f5abf41608'
curl -fLO '78cb317546'
FUZZ=mocked_descriptor_parse ./bld-cmake/bin/fuzz ./f5abf41608addcef3538da61d8096c2050235032
FUZZ=descriptor_parse ./bld-cmake/bin/fuzz ./78cb3175467f53b467b949883ee6072e92dbb267
```
This will also break 32-bit fuzzing, see https://github.com/bitcoin/bitcoin/issues/34110#issuecomment-3759461248.
Fix all issues by checking for `HasTooLargeLeafSize`.
Sorry for creating several pull requests to fix this class of issue, but I think this one should be the last one. 😅
ACKs for top commit:
brunoerg:
reACK fab2f3df4beb230eef63bdcf5042b6417c0012dc
frankomosh:
re-ACK fab2f3df4beb230eef63bdcf5042b6417c0012dc
Tree-SHA512: 4ecf98ec4adc39f6e014370945fb1598cdd3ceba60f7209b00789ac1164b6d20e82a69d71f8419d9a40d57ee3fea36ef593c47fe48b584b6e8344c44f20a15c1
c9ce1c7c4a12b54ada7d48f100ec3a141ae99f86 test: Fix P2PK script test (billymcbip)
Pull request description:
I found another script_tests case that isn't behaving the way it was meant to. It's a P2PK spend where we add an `OP_NOP8` to the scriptSig to make it non-push-only. The test should check that [`scriptSig.IsPushOnly()`](691dc830c6/src/script/interpreter.cpp (L2055)) is only enforced in P2SH mode when the scriptPubKey actually matches the P2SH pattern. To test this, we need to **turn on the P2SH flag**.
ACKs for top commit:
sipa:
ACK c9ce1c7c4a12b54ada7d48f100ec3a141ae99f86
darosior:
utACK c9ce1c7c4a12b54ada7d48f100ec3a141ae99f86
Tree-SHA512: 0af1d7b4651478349abc97cf0c009488cf5af5f97135382f7dd37cef0ef9b563192244330899a54ee7e0296bf03ba702e37a7aa15248c5c0ab4745095efc2402
This is likely slightly slower, but this was the last place we were using
epochs instead of sets to deduplicate, and this is only used by the RPC code
and in tests, and should not be CPU-performance critical. Eliminating this
allows us to save 8 bytes in CTxMemPoolEntry.
Co-Authored-By: Pieter Wuille <bitcoin-dev@wuille.net>
8b9d30e3facff0cd132dc3faf6282d75b1f9b532 bench/test: clarify merkle bench and witness test intent (Lőrinc)
Pull request description:
Follow-up to #32497.
Clarify why the witness merkle test uses an odd leaf count (it exercises leaf duplication in `ComputeMerkleRoot()`), and make the coinbase witness hash initialization explicit.
Also simplify the leaf-copy loop in the `MerkleRoot` benchmark for readability.
No production code is changed in this follow-up, for simplicity and safety.
ACKs for top commit:
optout21:
ACK 8b9d30e3facff0cd132dc3faf6282d75b1f9b532
maflcko:
lgtm ACK 8b9d30e3facff0cd132dc3faf6282d75b1f9b532
achow101:
ACK 8b9d30e3facff0cd132dc3faf6282d75b1f9b532
w0xlt:
ACK 8b9d30e3fa
danielabrozzoni:
tACK 8b9d30e3facff0cd132dc3faf6282d75b1f9b532
Tree-SHA512: 6efca7c19ebf96bb8d0def4217ed30d3b74b58a7be15566967e98aba9b03aaddd0e0ebb3b8f43130b5f397a7d9eed0470a48a55438f440e0bceefb87edd16b27
This is a helper struct to parse HTTP messages from data in buffers
from sockets. HTTP messages begin with headers which are
CRLF-terminated lines (\n or \r\n) followed by an arbitrary amount of
body data. Whitespace is trimmed from the field lines but not the body.
https://httpwg.org/specs/rfc9110.html#rfc.section.5
