✅ davidgumberg closed a pull request: "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1"
(https://github.com/bitcoin/bitcoin/pull/33187)

💬 davidgumberg commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3197770885)
I disagree with the reasons given for not enabling this, in particular I think what is underestimated is the number of people that use LSP tooling, and the range of development environments that support `compile_commands.json` metadata, but I'll close this for now as it lacks conceptual support.

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282987892)
re: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282752457

On nixos, I needed to change this as follows:

```diff
- cpp_capnp_dir = Path(capnp.__path__[0]).parent
+ capnp_dir = Path(shutil.which("capnp")).parent.parent / "include"
src_dir = Path(self.config['environment']['SRCDIR']) / "src"
mp_dir = src_dir / "ipc" / "libmultiprocess" / "include"
- imports = ["/usr/include", str(cpp_capnp_dir), str(src_dir), str(mp_dir)]
+ im
...

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282984358)
re: https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282782619

> AFAICT, this only works when capnproto is bundled inside `pycapnp`.

Can confirm at least on nixos there are no .capnp files in this directory. Seems harmless to keep this if it helps other platforms, but would be good to document where it's necessary.

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282980169)
In commit "ci: add functional test for IPC interface" (3f7ff4849e6165a6bcc4d5977c35a467fd7fc232)

This seems like it leaves behind an empty directory after the test runs. In any case it should be possible drop this code and simplify this commit by cherrypicking three commits from #30437:

- c3d82ef8fa947f108502ec80c522b4bd0084dd4c test: add is_ipc_enabled() and skip_if_no_ipc() functions
- 3af4d9a50ab008aa3974c580a831095e2ba17042 test: Provide path to `bitcoin` binary
- 6d0103472f1d691e8e1
...

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994052)
Good idea, done.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994684)
I added a comment to explain that Python 3.13 doesn't currently work.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282994842)
Done.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995076)
Thanks, added!

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995525)
Cool, used this in the instructions.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2282995874)
Fixed.

💬 jaredchapman4216-create commented on pull request "coins,refactor: Reduce `getblockstats` RPC UTXO overhead estimation":
(https://github.com/bitcoin/bitcoin/pull/31449#issuecomment-3197797741)
packages/components-shared/src/components/LoaderStakeEngine.svelteVerify your account

💬 ryanofsky commented on pull request "build: Enable ENABLE_IPC option by default":
(https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3197824813)
Updated 732c134bfc1208377f449e5956e01dd8b78d73d9 -> 5a34156ab65ac7460c26141bb29651477e000d19 ([`pr/ipc-default.3`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-default.3) -> [`pr/ipc-default.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-default.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-default.3..pr/ipc-default.4) applying fanquake's suggestion https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3197036114 to update valgrind jobs, and sjors suggest
...

💬 janb84 commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283046276)
This branch / patch works for me on Nix on macOS. Hint, do not forget to `import shutil` when applying the patch below.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283047618)
@ryanofsky Thanks! Will address later today.

💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2283103326)
> Is the 256-byte value used at all? If not, seems easier to just hardcode a value (like 0 or 1 or 2), or use a deterministic random comtext?

Good catch. I don't think it's used at all, we can simply use a hardcoded value instead of spending part of the buffer with it.

💬 fanquake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3197934560)
https://github.com/bitcoin/bitcoin/actions/runs/17047602183/job/48327342413?pr=33201#step:8:3379:
```bash
stderr:
Traceback (most recent call last):
File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/interface_ipc.py", line 161, in <module>
IPCInterfaceTest(__file__).main()
~~~~~~~~~~~~~~~~^^^^^^^^^^
File "/Users/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 186, in __init__
self.set_t
...

💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2283133265)
Done.

💬 josibake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283137376)
Sorry, didn’t mean to imply this breaks things if present, rather that adding this line works because it allows accessing the bundled version. This is why I recommended in a separate comment that our docs prompt the user to always install from source using the bundled version when installing pycapnp.

Longer term, I am confident we can drop all of this from the functional test in favour of having CMake pipe through the correct info to the config file.

📝 Crypt-iQ opened a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211)
Change `time_window` from 20s to 1h so `Reset` is not accidentally called if the test takes a while.

Change `num_lines` from 1024 to 10 since `LogRateLimiter` is parameterized and does not require logging 1MiB of data.