Bitcoin Core Github
45 subscribers
118K links
Download Telegram
πŸ’¬ Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284683975)
In cd86ddaa1dac104415ab16cc8068fd50d068c12f _ci: add functional test for IPC interface_: is it somehow possible for `import` to succeed but capnp to remain undefined?

https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3197934560
πŸ’¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284714840)
Was concerned about increasing the runtime of the tests due to repeatedly re-generating `first_chain`/`second_chain` as documented in the commit message:
> The approach to use one BOOST_AUTO_TEST_CASE for each scenario was attempted but the common initialization is repeated for every test case using the same fixture. This made the local runtime go from 0.39s to 0.65s. Would rather avoid global fixtures, so continued with one boost test case for now.

However, I figured out an okay way to swit
...
πŸ€” BrandonOdiwuor reviewed a pull request: "build: Enable ENABLE_IPC option by default"
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131561164)
Verified that `ENABLE_IPC` is enabled by default and that both `bitcoin-node` and `bitcoin-gui` (when built with -DBUILD_GUI=ON) are included in `build/bin`.

```bash
$ cmake -B build -DENABLE_GUI=ON
```

(system: macOS 15.6)
<img width="1512" height="912" alt="Screenshot 2025-08-19 at 11 32 05" src="https://github.com/user-attachments/assets/2bd88841-6e0b-4054-bff0-f6b224a1cc9e" />
πŸ€” BrandonOdiwuor reviewed a pull request: "build: Enable ENABLE_IPC option by default"
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131564457)
I still don't understand the value of enabling these binaries in the build tree without also including them in release artifact compared to https://github.com/bitcoin/bitcoin/pull/31802
βœ… glozow closed an issue: "Add support for creating v3 raw transactions in `createrawtransaction` RPC"
(https://github.com/bitcoin/bitcoin/issues/31348)
πŸš€ glozow merged a pull request: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)
πŸ’¬ BrandonOdiwuor commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#issuecomment-3200113920)
@janb84 thanks for having a look

I think introducing a main function would require a huge refactor to the file for such small check.

I would also love to hear some advantages of adding a main function here
πŸ’¬ ismaelsadeeq commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3200160757)
Nice idea.

A couple of questions:

1. How do nodes build the block template they share with peers? When they use only their mempool to build those templates, I don’t think that the transactions with diverging mempool policy will relay through the network and improve the compact block reconstruction process via this mechanism. Only peers whose outbound connections are miners will benefit.

2. We don’t accept transactions in the mempool that have divergent policy because we deem them non-st
...
πŸ“ hebasto opened a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin-core/gui/pull/884)
This change avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
πŸ’¬ sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284831733)
This was with an earlier version of the PR which called `load_capnp_modules` from `set_test_params` (which tried to do so before deciding whether pycapnp was even installed).
πŸ€” hodlinator reviewed a pull request: "headerssync: Preempt unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3131691825)
Latest push:
* Breaks up test into multiple boost test cases instead of regular functions (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2277678459). In doing so, I changed back to using a custom fixture as on master. Diffing with dimmed-zebra helps show what was just moved and what actually changed `git diff 342359f 53341ea`.
* Removed redundant `assert()` after bringing that block closer to constants (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2271294627).
* Minor
...
πŸ’¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284849835)
Pushed version with multiple test cases.
πŸ’¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284838171)
Moved up the macro above the constants in the latest push, and brought the assert block up closer to the constants - in the process removing the final assert.
πŸ‘ stickies-v approved a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3131772733)
ACK ab5c3be4b384d48d514b06dc8a391f1efa51ea38

> then we could set set the reset window to 5 hours

That would require more changes to `CScheduler` because `MockForward` is currently [capped at 1h](https://github.com/bitcoin/bitcoin/blob/be356fc49be4a32f790c38ca72c76e9d6fa2de3d/src/scheduler.cpp#L82).

---

I considered previously to also add `log_lines` to the boost test context, but decided not to for brevity, but it would've been helpful in debugging this. I think the below diff would
...
πŸ“ hebasto opened a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin/bitcoin/pull/33215)
This PR avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
βœ… hebasto closed a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin-core/gui/pull/884)
πŸ’¬ hebasto commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3200273773)
Incorporated into https://github.com/bitcoin/bitcoin/pull/33215.

Closing
πŸ€” stickies-v reviewed a pull request: "rpc: require integer verbosity; remove boolean 'verbose'"
(https://github.com/bitcoin/bitcoin/pull/33214#pullrequestreview-3131799623)
Directionally, this is the right way to go, but I don't think we should be breaking the API for just this change. Let's do these cleanups when we can bundle them with other, necessary breaking changes, on a case-by-case basis. Concept NACK.
πŸ€” janb84 reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131829788)
re ACK 3c4a109aa821cbf1e46a67275b4f456673ed13d8

changes since last ACK:
- multilines are now joint
- changes in sorting

Manually checked diff of `bitcoinstrings.cpp` because of change of sorting, all the lines are included. This PR introduces 2 new lines in `bitcoinstrings.cpp`:

<details>

````C++
QT_TRANSLATE_NOOP("bitcoin-core", "The %s path uses exFAT, which is known to have intermittent corruption problems on macOS. Move this directory to a different filesystem to avoid data l
...
πŸ’¬ hebasto commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3200309116)
> Manually checked diff of `bitcoinstrings.cpp` because of change of sorting, all the lines are included. This PR introduces 2 new lines in `bitcoinstrings.cpp`:
>
> ```c++
> QT_TRANSLATE_NOOP("bitcoin-core", "The %s path uses exFAT, which is known to have intermittent corruption problems on macOS. Move this directory to a different filesystem to avoid data loss."),
> QT_TRANSLATE_NOOP("bitcoin-core", "default wallet"),
> ```

This is expected. See: https://github.com/bitcoin/bitcoin/p
...