💬 Sjors commented on pull request "clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc`":
(https://github.com/bitcoin/bitcoin/pull/33312#issuecomment-3264846807)
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
Is it possible to move this into the capnp subdirectory to further narrow the scope of this exception?
(https://github.com/bitcoin/bitcoin/pull/33312#issuecomment-3264846807)
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
Is it possible to move this into the capnp subdirectory to further narrow the scope of this exception?
💬 Sjors commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-3264907575)
I'm a bit confused about the relation between this and https://github.com/bitcoin/bitcoin/pull/33117. I would suggest rebasing 60f676abf54a454361ae9f5dd17509ec929acbac on that branch for clarity.
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-3264907575)
I'm a bit confused about the relation between this and https://github.com/bitcoin/bitcoin/pull/33117. I would suggest rebasing 60f676abf54a454361ae9f5dd17509ec929acbac on that branch for clarity.
💬 deyvid61 commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3264962944)
6tt4r
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3264962944)
6tt4r
🤔 yuvicc reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3195377066)
Concept ACK.
The approach of switching to `std::optional<bool>` here avoids the overhead of atomic operations relying on `cs_main` for synchronization makes sense.
<details> <summary>Initial script verification state is always logged without unnecessary complexity.</summary>
```bash
2025-09-08T07:43:58Z Disabling signature validations at block #16893 (00000000113c355415c3417c734a45b62790b91b41ff08028580254c584f5ea8).
```
</details>
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3195377066)
Concept ACK.
The approach of switching to `std::optional<bool>` here avoids the overhead of atomic operations relying on `cs_main` for synchronization makes sense.
<details> <summary>Initial script verification state is always logged without unnecessary complexity.</summary>
```bash
2025-09-08T07:43:58Z Disabling signature validations at block #16893 (00000000113c355415c3417c734a45b62790b91b41ff08028580254c584f5ea8).
```
</details>
🤔 Sjors reviewed a pull request: "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications"
(https://github.com/bitcoin/bitcoin/pull/33117#pullrequestreview-3195338293)
At first glance this looks very reasonable. I also tested it in the (current) GUI with https://github.com/bitcoin-core/gui/pull/870
I had a similar concern as @ryanofsky about how to go about passing the snapshot file (path) over the interface. The current approach in 3c8b578836dd0d8438a8aac81a1166df84811562 is the same as how we handle wallet backups, so probably fine?
(https://github.com/bitcoin/bitcoin/pull/33117#pullrequestreview-3195338293)
At first glance this looks very reasonable. I also tested it in the (current) GUI with https://github.com/bitcoin-core/gui/pull/870
I had a similar concern as @ryanofsky about how to go about passing the snapshot file (path) over the interface. The current approach in 3c8b578836dd0d8438a8aac81a1166df84811562 is the same as how we handle wallet backups, so probably fine?
💬 Sjors commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2329435219)
In 3c8b578836dd0d8438a8aac81a1166df84811562 _interfaces: expose UTXO snapshot loading in node interface_: a good way to add test coverage to this new interface would be to use it in the `loadtxoutset` RPC.
For an example, see the various uses of `EnsureMining(node);`
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2329435219)
In 3c8b578836dd0d8438a8aac81a1166df84811562 _interfaces: expose UTXO snapshot loading in node interface_: a good way to add test coverage to this new interface would be to use it in the `loadtxoutset` RPC.
For an example, see the various uses of `EnsureMining(node);`
📝 Snezhkko opened a pull request: "chore: compare against keys.size() in writeObject comma check"
(https://github.com/bitcoin/bitcoin/pull/33337)
Replace values.size() with keys.size() in the comma condition inside UniValue::writeObject. The loop iterates over keys.size(), and other UniValue methods consistently treat keys and values as parallel arrays. Using keys.size() here ensures internal consistency, avoids relying on the implicit invariant that keys.size() == values.size(
(https://github.com/bitcoin/bitcoin/pull/33337)
Replace values.size() with keys.size() in the comma condition inside UniValue::writeObject. The loop iterates over keys.size(), and other UniValue methods consistently treat keys and values as parallel arrays. Using keys.size() here ensures internal consistency, avoids relying on the implicit invariant that keys.size() == values.size(
💬 TheCharlatan commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#issuecomment-3265136162)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33324#issuecomment-3265136162)
Concept ACK
💬 TheCharlatan commented on pull request "RFC: coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#issuecomment-3265143719)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33333#issuecomment-3265143719)
Concept ACK
🚀 fanquake merged a pull request: "Update libmultiprocess subtree to improve build and logs"
(https://github.com/bitcoin/bitcoin/pull/33322)
(https://github.com/bitcoin/bitcoin/pull/33322)
✅ fanquake closed an issue: "build: indefinite mpgen hang on Ubuntu 22.04"
(https://github.com/bitcoin/bitcoin/issues/33176)
(https://github.com/bitcoin/bitcoin/issues/33176)
💬 fanquake commented on issue "build: indefinite mpgen hang on Ubuntu 22.04":
(https://github.com/bitcoin/bitcoin/issues/33176#issuecomment-3265327251)
Closing this with #33322. If there are new / more issues, lets open new issues.
(https://github.com/bitcoin/bitcoin/issues/33176#issuecomment-3265327251)
Closing this with #33322. If there are new / more issues, lets open new issues.
💬 hebasto commented on pull request "clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc`":
(https://github.com/bitcoin/bitcoin/pull/33312#issuecomment-3265331404)
> Is it possible to move this into the capnp subdirectory to further narrow the scope of this exception?
The most recent CI log shows false-positive `clang-analyzer-core.UndefinedBinaryOperatorResult` warnings in the following files:
- `/home/admin/actions-runner/_work/_temp/build/src/ipc/capnp/init.capnp.proxy-server.c++`
- `/home/admin/actions-runner/_work/_temp/build/src/ipc/capnp/echo.capnp.proxy-server.c++`
- `/home/admin/actions-runner/_work/_temp/build/src/ipc/capnp/mining.capnp.pro
...
(https://github.com/bitcoin/bitcoin/pull/33312#issuecomment-3265331404)
> Is it possible to move this into the capnp subdirectory to further narrow the scope of this exception?
The most recent CI log shows false-positive `clang-analyzer-core.UndefinedBinaryOperatorResult` warnings in the following files:
- `/home/admin/actions-runner/_work/_temp/build/src/ipc/capnp/init.capnp.proxy-server.c++`
- `/home/admin/actions-runner/_work/_temp/build/src/ipc/capnp/echo.capnp.proxy-server.c++`
- `/home/admin/actions-runner/_work/_temp/build/src/ipc/capnp/mining.capnp.pro
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2329682120)
Changed locally, will be in the next push, thanks!
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2329682120)
Changed locally, will be in the next push, thanks!
🚀 fanquake merged a pull request: "contrib: update fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/33283)
(https://github.com/bitcoin/bitcoin/pull/33283)
✅ fanquake closed an issue: "build: clang build broken on Debian Bookworm"
(https://github.com/bitcoin/bitcoin/issues/33279)
(https://github.com/bitcoin/bitcoin/issues/33279)
💬 fanquake commented on issue "build: clang build broken on Debian Bookworm":
(https://github.com/bitcoin/bitcoin/issues/33279#issuecomment-3265439112)
Going to close this for now, based on https://github.com/bitcoin-core/libmultiprocess/pull/205 (which was pulled in via #33322). We can followup further if any similar issues surface while testing `30.x`.
(https://github.com/bitcoin/bitcoin/issues/33279#issuecomment-3265439112)
Going to close this for now, based on https://github.com/bitcoin-core/libmultiprocess/pull/205 (which was pulled in via #33322). We can followup further if any similar issues surface while testing `30.x`.
💬 ryanofsky commented on issue "build: indefinite mpgen hang on Ubuntu 22.04":
(https://github.com/bitcoin/bitcoin/issues/33176#issuecomment-3265458641)
Makes sense to close. Since #33241 which includes https://github.com/bitcoin-core/libmultiprocess/pull/194, the build hang using old versions of GCC with v0.8.0 and v0.8.1 was fixed, but a cmake error was added warning about a CVE in v0.8.0, which means that Ubuntu 22.04 triggers this error. Ubuntu security team evaluated the CVE and opted not to fix it based on the severity and fact that is fix is in a library, so could cause other packages to need to be recompiled. #33322 improves the situatio
...
(https://github.com/bitcoin/bitcoin/issues/33176#issuecomment-3265458641)
Makes sense to close. Since #33241 which includes https://github.com/bitcoin-core/libmultiprocess/pull/194, the build hang using old versions of GCC with v0.8.0 and v0.8.1 was fixed, but a cmake error was added warning about a CVE in v0.8.0, which means that Ubuntu 22.04 triggers this error. Ubuntu security team evaluated the CVE and opted not to fix it based on the severity and fact that is fix is in a library, so could cause other packages to need to be recompiled. #33322 improves the situatio
...
💬 fanquake commented on pull request "ci: reduce runner sizes on various jobs":
(https://github.com/bitcoin/bitcoin/pull/33319#issuecomment-3265467783)
cc @maflcko
(https://github.com/bitcoin/bitcoin/pull/33319#issuecomment-3265467783)
cc @maflcko
💬 deyvid61 commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#discussion_r2329727735)
__
(https://github.com/bitcoin/bitcoin/pull/33275#discussion_r2329727735)
__
💬 TheCharlatan commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#issuecomment-3265507664)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/33117#issuecomment-3265507664)
Approach ACK