💬 instagibbs commented on pull request "p2p: Add witness mutation check inside FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2132037362)
did that in a7cbaca0af38c183f009a7c8e2cfb54c17a1dc5a

💬 instagibbs commented on pull request "p2p: Add witness mutation check inside FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2132037528)
will do the next time I touch it

💬 maflcko commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2132074962)
Hmm, is there a reason why blockundo takes only 1.6ms in total end-to-end when spenttxouts takes 4.6ms in total end-to-end, where about 2.6ms happen on the server (https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2109843001). So with spenttxouts, the client spends about 2ms with 18% CPU and with blockundo the client spends about .8ms with 58% CPU.

Just wondering out of curiosity.

Personally I'd just go with `spenttxouts`, but no strong opinion.

💬 willcl-ark commented on pull request "[28.x] 28.2rc2":
(https://github.com/bitcoin/bitcoin/pull/32684#issuecomment-2949119684)
```
❯ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
be30f094366c71bd7251247319dbb0cf1b5674d13e210685328daff1bf855766 guix-build-3b05c498a08e/output/aarch64-linux-gnu/SHA256SUMS.part
1823e9b3d7c97a533ab77540095223bc48759216b9e9fe757f24575f2323680f guix-build-3b05c498a08e/output/aarch64-linux-gnu/bitcoin-3b05c498a08e-aarch64-linux-gnu-debug.tar.gz
8fd4d3a844e845ae13a95d165ae2cd1fea735295c24bc68b3fd28cb91f59c397 guix-bui
...

👍 stickies-v approved a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2904751108)
re-ACK fdcdc2cdeadb5d36076f1b94b54261e22e031354

Mostly documentation changes addressing review feedback. Non-trivial code changes are:
- [deduplicating](https://github.com/bitcoin/bitcoin/compare/ed172d3002da68a3ddbd5d13e7d3f0618c1378d4..1f09e3754363382806cc1e3d6bed25cd235ac5f4#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R4523-R4525) `InvalidChainFound()` logic, addressing https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2090791378
- [Choosing](https://gith
...

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132101738)
nit for greppability (and line lenght)
```suggestion
// Do not remove candidate from highpow_outofchain_headers, because it might be a
// descendant of the block being invalidated which needs to be marked failed later.
```

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132118238)
inline nit
```suggestion
if (Assume(state.IsInvalid())) {
```

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132120486)
nit
```suggestion
In case of multiple best headers with the same work, it could point to any.
```

💬 fanquake commented on pull request "guix: warn and abort when SOURCE_DATE_EPOCH is set":
(https://github.com/bitcoin/bitcoin/pull/32678#issuecomment-2949198561)
Concept ACK

💬 marcofleon commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2132182057)
This should be before `merkleblock.cpp`.

💬 Sjors commented on issue "depends: OpenBSD needs gcc (instruction) for libevent":
(https://github.com/bitcoin/bitcoin/issues/32691#issuecomment-2949258981)
I tried again using a Hetzner VM running OpenBSD 7.7 amd64 (to install, just pick any distro, mount OpenBSD from their list of ISO images and reboot into the installer - get an IPv4 address unless you enjoy pain).

gcc/g++ are not present by default, nor after `pkg_add bash gmake gtar`

Initially the libevent installer fails because `cmake` isn't installed (also missing from the instruction, but the error is clear enough so it shouldn't confuse anyone).

Once cmake is install the depends build s
...

🤔 yuvicc reviewed a pull request: "rpc: combinerawtransaction now rejects unmergeable transactions"
(https://github.com/bitcoin/bitcoin/pull/31298#pullrequestreview-2904915800)
re-ACK 9d31e94bf584e12269683b74b1c96c7ac883a760

💬 Sjors commented on issue "depends: OpenBSD needs gcc (instruction) for libevent":
(https://github.com/bitcoin/bitcoin/issues/32691#issuecomment-2949301882)
I spun up an equivalent aarch64 VM. I installed `bash gmake gtar` and `cmake` (and `git` to clone the repo). Ran into the same error as with UTM.

So it seems to be an arm thing?

📝 HowHsu opened a pull request: "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1"
(https://github.com/bitcoin/bitcoin/pull/32692)
A fixed MAX_SCRIPTCHECK_THREADS value is not flexible for users to leverage their cpu resources, and a value large than nCores - 1 doesn't make sense since it only adds some context switch overhead. Set it to nCores - 1. Assumption: A user who sets the number of script verification workers is aware of how this affects the system performance, otherwise he/she leaves it as default (which is 0)

💬 sipa commented on pull request "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1":
(https://github.com/bitcoin/bitcoin/pull/32692#issuecomment-2949356920)
I think up to `nCores` makes sense, and we need to be sure that that value is an actual reflection of hardware concurrency on all platforms. It may be worth benchmarking to see if there is actually a gain above certain values still (because thread contention will start to become more and more noticeable, which may at some point overtake the gains from more parallellism).

💬 Sjors commented on pull request "depends: fix multiprocess build on OpenBSD (apply capnp patch, correct SHA256SUM command)":
(https://github.com/bitcoin/bitcoin/pull/32690#discussion_r2132276270)
I'll spin up a FreeBSD 14.1 VM to try that.

👍 willcl-ark approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32479#pullrequestreview-2905089748)
ACK 1d8856dddfcd6978ca8104d7c3fc96f522ad2e9b

Backports and attributions in release note appear correct.

Is my understanding correct that commit "ci: remove --enable-external-signer" e0af7bffff4b33d1de9d95bb7a4f4424f2e6c2dd is not a backport?

💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/32479#issuecomment-2949426665)
That is correct.

💬 GregTonoski commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2949427156)
Concept NACK.

After careful review, I must conclude that the PR doesn't accomplish the objectives.

💬 Sjors commented on pull request "depends: fix multiprocess build on OpenBSD (apply capnp patch, correct SHA256SUM command)":
(https://github.com/bitcoin/bitcoin/pull/32690#discussion_r2132308827)
FreeBSD depends instruction is also missing `gmake` and `cmake` in the list of things to install.

`build_freebsd_SHA256SUM = sha256sum` already outputs the format we need.