💬 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.

💬 ryanofsky commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210452958)
> Is anyone working on that? I guess @Sjors?

I could put together something very basic like a fuzz test calling the `echoipc` RPC method. But there are a number of other loose ends since #31802 which should take priority so will probably not get to this very soon. @darosior has mentioned writing fuzz tests for IPC before which could be more meaningful, like fuzzing the IPC socket.

💬 janb84 commented on pull request "doc: Remove wrong and redundant doxygen tag":
(https://github.com/bitcoin/bitcoin/pull/33236#issuecomment-3210470881)
Is there a reason why this change is limited to only `feerate.h` ? The wrongful use of the tag is also in `coinselection.cpp` and `spend.h`. To much churn ?

💬 maflcko commented on pull request "doc: Remove wrong and redundant doxygen tag":
(https://github.com/bitcoin/bitcoin/pull/33236#issuecomment-3210545416)
Thx, done. Good catch actually calling `git grep 'param@'` to find all of them :sweat_smile:

💬 Sjors commented on pull request "build: set ENABLE_IPC to OFF when fuzzing":
(https://github.com/bitcoin/bitcoin/pull/33235#issuecomment-3210571615)
Made a note to add a fuzz test if @darosior or @ryanofsky don't get around to it.

Would prefer to leave this ON unless such a PR doesn't land before branch-off. Maybe add a v30 milestone?