💬 tdb3 commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1820702143)
nit: for awareness, may want to mention in OP that this changes directory names from 256-bit randoms to 32-bit randoms. I'm not seeing an issue with it (virtually non-existent likelihood of collision).

💬 hebasto commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2444103245)
> > > The cross-compiled on Ubuntu 24.10 bitcoind.exe fails to run on Windows 11 Pro 23H2.
> >
> >
> > Can you provide some actionable information? The CI and cross-compiled unit tests have run & passed.
>
> The only difference from CI is that I run `bitcoind.exe` on Windows, not under Wine.

> This seems like even more reason to do #31071.

This branch @ 7dd0ee89a092c6ec4e305fbdb0cf3afa9e41cab6 rebased on top of the https://github.com/bitcoin/bitcoin/pull/31176 fails to run `bitcoin
...

💬 l0rinc commented on pull request "doc: Extend developer-notes with file-name-only debugging fix":
(https://github.com/bitcoin/bitcoin/pull/30670#discussion_r1820749380)
Done, thanks

💬 ryanofsky commented on issue "ci: intermittent issue in rpc_misc.py node0 stderr terminate called after throwing an instance of 'kj::ExceptionImpl' [15:12:14.943] what(): mp/proxy.cpp:242: disconnected: write(m_post_fd, &buffer, 1): Broken pipe [15:12:14.943] stack: 657ed083 657ed86e f7ac7d20 f7713156 f77a7e07 ":
(https://github.com/bitcoin/bitcoin/issues/31151#issuecomment-2444170080)
Assuming this is a new error, it is possible this is caused by #31105 which was merged a week ago and changed some parts of IPC shutdown sequence.

It seems this failure here is happening because some code is trying to use the IPC event loop after it is closed. Specifically some code is calling the `mp::EventLoop::removeClient` which fails writing to the event loop's local pipe at https://github.com/chaincodelabs/libmultiprocess/blob/abe254b9734f2e2b220d1456de195532d6e6ac1e/src/mp/proxy.cpp#L2
...

👍 laanwj approved a pull request: "doc: Extend developer-notes with file-name-only debugging fix"
(https://github.com/bitcoin/bitcoin/pull/30670#pullrequestreview-2401819547)
ACK 1b0b9b4c7873ff0c6323de0c7f439466eed06049

👍 hebasto approved a pull request: "Fix unsigned integer overflows in interpreter"
(https://github.com/bitcoin/bitcoin/pull/24214#pullrequestreview-2401888182)
ACK bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9, I have reviewed the code and it looks OK.

A decent alternative to https://github.com/bitcoin/bitcoin/pull/29543.

💬 ryanofsky commented on issue "ci: intermittent issue in rpc_misc.py node0 stderr terminate called after throwing an instance of 'kj::ExceptionImpl' [15:12:14.943] what(): mp/proxy.cpp:242: disconnected: write(m_post_fd, &buffer, 1): Broken pipe [15:12:14.943] stack: 657ed083 657ed86e f7ac7d20 f7713156 f77a7e07 ":
(https://github.com/bitcoin/bitcoin/issues/31151#issuecomment-2444255035)
Looking more closely at changes in #31105, I don't see anything that directly relates to this code, so it seems possible this is an older bug or a different bug. Looking up the "Broken pipe" error, it also seems like that can only be triggered if something is closing the `EventLoop::m_wait_fd` file descriptor before the `write(m_post_fd, ...)` call happens. But I don't see a code path where that would happen. May need to reproduce locally to debug. Would be curious to know if this happens more.

👍 TheCharlatan approved a pull request: "ci: Use clang-19 from apt.llvm.org"
(https://github.com/bitcoin/bitcoin/pull/30634#pullrequestreview-2401970706)
ACK fabe90c8242aa45a8b9925347ca6fc11d5852ffd

💬 laanwj commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2444317433)
> I implemented your suggestion by adding the --skip-missing-binaries option to the command.

Yes, thanks!

> I wonder if a better approach here would be to further integrate the manpage generation into the build system; given that the two issues of "which binaries exist" and "where do they exist" are already solved in that case. This should also remove the dep on git.

That would be better. The thing is, the build system isn't allowed to execute the binaries it generated (and even if so t
...

💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1820864917)
I'll let the others weigh in a bit before touching docs more

💬 0xB10C commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1820876152)
Thanks for the ping, sorry for the late response. I agree that a "package hash" on it's own might not be useful for someone interested in "which transactions were replaced by which other transactions". On the other hand, the tracepoints are on purpose only semi-stable as they expose internals. If the internals change, the tracepoints need to change too.

> which (as of this PR) would now report the package hash along with the transaction ids that are in it?

That could be useful, yes. Howev
...

💬 sipa commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1820886255)
You can mimic what we did for BIP61 in the same document (which just lists the history of when it was introduced, and when it was removed).

⚠️ fanquake opened an issue: "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously"
(https://github.com/bitcoin/bitcoin/issues/31178)
According to https://corecheck.dev/benchmarks, the `linearizeoptimallyexample11` benchmark 4x'd it's execution time at some point in the last few days.

💬 Christewart commented on pull request "consensus: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2444403494)
> Hey @Christewart! Do you plan to update for cmake?

Done, i squashed everything into 1 commit as well.

👍 hebasto approved a pull request: "ci: Use clang-19 from apt.llvm.org"
(https://github.com/bitcoin/bitcoin/pull/30634#pullrequestreview-2402063455)
ACK fabe90c8242aa45a8b9925347ca6fc11d5852ffd, I have reviewed the code and it looks OK.

💬 sipa commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2444413270)
More like 2x (from 0.64 to 1.21)? Still bizarre, this code has not been touched the past month for as far as I can see?

💬 fanquake commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2444418197)
I was just looking at total execution time increasing from ~4.3s to ~16.3.

💬 sipa commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2444420462)
Oh nevermind, I was looking at the "outliers" panel, which doesn't give absolute time. Indeed, 4x.

🤔 jonatack reviewed a pull request: "rpc, cli: return "verificationprogress" of 1 when up to date"
(https://github.com/bitcoin/bitcoin/pull/31177#pullrequestreview-2402104620)
Concept ACK, some ideas to encourage review:

- make sure your proposal compiles in your local environment, with passing unit and functional tests, so updating all the callers of `GuessVerificationProgress`. This makes it easier for reviewers to build and test your code and see if any test coverage needs to be updated or possibly added. Some reviewers may also wait until your pull has a green CI as enough proof of work before reviewing it more closely.
- squash your commits

This [article](
...

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1820926787)
Ah, yeah, right, that was stupid. I'll stick to just renaming `use_pcp` to `enabled`.

💬 mzumsande commented on pull request "test: Don't enforce BIP94 on regtest unless specified by arg":
(https://github.com/bitcoin/bitcoin/pull/31156#discussion_r1820928668)
Just following the process described in https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#release-notes - a change to that process should be discussed in a separate issue / PR - whatever works best for the maintainers I guess, personally I don't care at all.