Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 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.
💬 ryanofsky commented on pull request "build: Enable ENABLE_IPC option by default":
(https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3198025822)
re: https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3196869655

> I think https://github.com/bitcoin/bitcoin/pull/31802 should be seen as a successor to this PR, regardless of which release it goes into.

@sipa if you are saying you see this PR as a prerequisite to #31802, that seems ok, but I would be interested to know why.

As far as I can tell, there haven't been any other features in bitcoin core enabled by default in non-depends builds but disabled by default by default in
...
💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3198034390)
Note that when this is merged, `oss-fuzz` will need fixing, to either opt-out of multiprocess, or configure depends to use the pre-built Clang for native package compilation (as Ubuntu 20.04 ships with GCC 9). This isn't an issue currently, as we don't build native packages (NO_QT=1), but with multiprocess on by default, we'll build `native_capnp`:
```bash
/src/bitcoin-core/depends/work/download/native_capnp-1.2.0/capnproto-cxx-1.2.0.tar.gz.temp: OK
Extracting native_capnp...
/src/bitcoin-co
...
🚀 fanquake merged a pull request: "ipc: Handle unclean shutdowns better"
(https://github.com/bitcoin/bitcoin/pull/32345)
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3198178581)
Updated 7bcb122e6e55339f25238a44433cc5aadc4526f1 -> 3a34e006918490eb968c7b3312693e8a27fa3fde ([kernelApi_58](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_58) -> [kernelApi_59](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_59), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_58..kernelApi_59))

* Implemented owned delegates as lined out in @purpleKarrot's [blog post](https://purplekarrot.github.io/btck/design/delegates.html) - though for now without error
...