Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 fanquake commented on pull request "[29.x] 33106 backport and final changes for rc2":
(https://github.com/bitcoin/bitcoin/pull/33226#issuecomment-3210023448)
This needs `gen-bitcoin-conf.sh` run, to accomodate the changed options, before tagging an rc2. I'll open a followup, and we can get rc2 tagged after that.
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3210097534)
For some reason on the macOS native job (`00_setup_env_mac_native.sh`) the interface test is skipped despite `pycapnp` being installed and `bitcoin-node` being built: https://github.com/bitcoin/bitcoin/actions/runs/17120162263/job/48559027457?pr=33201#step:8:3718

I checked that this test did run for native_asan.

I wrote a commit that enables it for additional jobs, which I'm testing in:
- CentOS
- native man
- kernel
- native_tsan
- previous releases
- native valgrind
🚀 fanquake merged a pull request: "[29.x] 33106 backport and final changes for rc2"
(https://github.com/bitcoin/bitcoin/pull/33226)
📝 fanquake opened a pull request: "doc: update example bitcoin conf for 29.1rc2"
(https://github.com/bitcoin/bitcoin/pull/33234)
Followup to #33226.
💬 musaHaruna commented on pull request "test: p2p block malleability":
(https://github.com/bitcoin/bitcoin/pull/33172#discussion_r2290729841)
Added in the latest push. Thank you!
💬 musaHaruna commented on pull request "test: p2p block malleability":
(https://github.com/bitcoin/bitcoin/pull/33172#discussion_r2290730358)
Fixed as suggested in the latest push
📝 fanquake opened a pull request: "build: set ENABLE_IPC to OFF when fuzzing"
(https://github.com/bitcoin/bitcoin/pull/33235)
A `BUILD_FOR_FUZZING` build will currently failure to configure, with missing `capnp`.
💬 maflcko commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210147334)
> A `BUILD_FOR_FUZZING` build will currently failure to configure, with missing `capnp`.

IPC is by default on, and any build will fail with missing `capnp`? Whatever the user/dev is doing to fix those should be trivial to apply to a fuzz build to not have to special-case it?
💬 fanquake commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210150249)
> Whatever the user/dev is doing to fix those should be trivial to apply to a fuzz build to not have to special-case it?

It's already special cased though: "BUILD_FOR_FUZZING=ON will disable all other targets and force BUILD_FUZZ_BINARY=ON.", and that will no-longer we true if we leave IPC on.
💬 maflcko commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210245926)
Yes, the multiprocess binary targets are correctly disabled already on current master https://cirrus-ci.com/task/4847132122808320?logs=ci#L2057:

```
[17:44:32.757] Configure summary
[17:44:32.757] =================
[17:44:32.757] Executables:
[17:44:32.757] bitcoin ............................. OFF
[17:44:32.757] bitcoind ............................ OFF
[17:44:32.757] bitcoin-node (multiprocess) ......... OFF
[17:44:32.757] bitcoin-qt (GUI) .................... OFF
[17:44:32.
...
💬 fanquake commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210257560)
Yea. So failing to configure because a dependency you don't even need insn't installed, doesn't make any sense.
📝 maflcko opened a pull request: "doc: Remove wrong and redundant doxygen tag"
(https://github.com/bitcoin/bitcoin/pull/33236)
`param@[in]` is not a valid doxygen tag. Also, no other function in this file uses the annotations, and they are redundant with the line above, so just remove them to fix all issues.
💬 ryanofsky commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-3210296557)
sipa, theuni, TheBlueMatt, and others raised a question in #31802 that wasn't directly considered here and is still relevant: **Should `bitcoind` accept an `-ipcbind` option?**

### Background

The multiprocess code and libmultiprocess/cap'nproto dependencies allow for two distinct features:

1. An *IPC listening* feature (`-ipcbind`), which lets the bitcoin node listen on a Unix socket and accept IPC connections from utilities and third-party applications.
2. A *process separation* feature, whi
...
💬 hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-3210338335)
@Sjors or @sdaftuar: would you mind doing some light review of 1771e59f1a1fcaa6a1d79c93bc0c0c3b1003cf82? It represents the meat of this PR, making the headers sync state params that affect memory usage able to be configured for tests/other chains instead of hard-coded to mainnet conditions.

(davidgumberg also suggested (https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-3208186878) reviewing 4aea5fa3690310bfc2617e3bf31f555dae7d9337 which also touches the `HeadersSyncState` implementat
...
💬 fanquake commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3210347297)
> Feature freeze (bug fixes only until release)

We are now in feature freeze.
💬 ryanofsky commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210390527)
IIUC, this change which toggles off the ENABLE_IPC option automatically when BUILD_FOR_FUZZING is set is safe for now because the fuzz test binary doesn't currently call any IPC code.

The change makes it more convenient to run the fuzz test without capnp installed since you don't have to turn off ENABLE_IPC manually.

But I think we would want to revert this change as soon as any fuzz test is written that does call IPC code, so it does seem like there may not much long term benefit in makin
...
💬 fanquake commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210404418)
> It would be good to have some fuzz tests exercising IPC code.

Is anyone working on that? I guess @Sjors?
💬 ryanofsky commented on pull request "IPC followups for PR 31802":
(https://github.com/bitcoin/bitcoin/pull/33233#issuecomment-3210422658)
Code review ACK 2a815d126bc90f787f7adbe8cade45cb7307429b

Maybe we want to keep this PR open a little longer in case there are other small documentation updates like this? Seems fine to merge now though if preferred.

Another little thing I noticed is that italics should be removed from these files in files.md since they are now included in binary releases:

https://github.com/bitcoin/bitcoin/blob/7d9789401be4c91a9eb3c1112564a6524bdc4f70/doc/files.md?plain=1#L153-L154
💬 fjahr commented on pull request "index: Don't commit state in BaseIndex::Rewind":
(https://github.com/bitcoin/bitcoin/pull/33212#issuecomment-3210430071)
Concept ACK

Re [my concerns here](https://github.com/bitcoin/bitcoin/issues/33208#issuecomment-3194652080) that this might not address the full issue I am now more confident that this may be all there is to it. This is anecdotal evidence and not based on statistics but vague memories but it seems that it happens more often on testnets than on mainnet and they couldn't remember seeing it on signet ever. So this would make a lot of sense if the issues are always connected to reorgs.
💬 fanquake commented on issue "build: indefinite mpgen hang on Ubuntu 22.04":
(https://github.com/bitcoin/bitcoin/issues/33176#issuecomment-3210436475)
Put on the v30.0 milestone.