💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/32479#issuecomment-2876661608)
This is failing in the macOS CI. An issue with Boost Process:
```bash
configure:34786: checking whether Boost.Process can be used
configure:34817: g++ -std=gnu++11 -std=c++20 -o conftest -Wno-error=narrowing -isystem /usr/local/include -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -pthread conftest.cpp >&5
conftest.cpp:64:9: error: no type named 'opstream' in namespace 'boost::process'
bp::opstream stdin_stream;
~~~~^
conftest.cpp:65:18: error: use of undeclared identifier 'stdou
...

💬 theStack commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2086920895)
are these prepended paths (here and also for the `gui`, `bench` and `test` commands above) still relevant? They seem to reflect a folder structure that doesn't exist anymore since https://github.com/bitcoin/bitcoin/pull/31161

💬 theStack commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2086894554)
```suggestion
//! strace -e trace=execve -s 10000 build/bin/bitcoin ...
```

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933393)
No, these are different concepts entirely. @Sjors asked for a comment and thought it wouldn't hurt. Leaving as is or removing this if it adds to confusion. Thoughts?

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933574)
style difference I guess? we assert the invariant we expect and I like subtests cleaning up their state in general for mempool stuff. Keeping as is.

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933716)
taken

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933886)
taken with minor modifications

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086936293)
combined commits, still leaning towards a long deprecation cycle to not mislead longer term

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086937998)
The options is ostensibly to stop people from data packing, not vbytes usage per se. The smaller the payload, the more overhead they are incuring.

💬 laanwj commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-2876721714)
> you might be interested in circling back here to review?

Yes, will look into it.

> This is not a regression from migration from libnatpmp as I first thought.

Removed the backport label as this was confirmed not to be a regression (thanks!).

🤔 darosior reviewed a pull request: "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts"
(https://github.com/bitcoin/bitcoin/pull/32473#pullrequestreview-2836993312)
Neat. Concept ACK.

> I'm open to discussing alternatives, however.

I think enabling it by consensus is unnecessarily risky. I would rather take some slightly more complicate logic, for instance by having `CScriptCheck` own the sighash cache:
<details>

<summary>Click to see diff</summary>

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index ffac2acb948..fe24dae6c54 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1700,7 +
...

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2876729479)
Thanks to everyone who gave thoughtful review, I hopefully addressed all code-related questions, if not let me know.

💬 maflcko commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2086955469)
> std::hash::DefaultHasher::new().finish()

Pretty sure this returns a constant. Maybe you wanted to say `std::hash::RandomState::new().build_hasher().finish()`?

💬 darosior commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2876744585)
> I've been thinking about this approach over the approach @darosior suggested in #30983 again, and think this would indeed be cleaner.

Explicitly signaling here that despite my suggesting a different approach in #30983 i am not opposed to this one.

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2876746382)
Friendly ping @laanwj @sipsorcery :)

🤔 maflcko reviewed a pull request: "doc: update unix build doc with build flags"
(https://github.com/bitcoin/bitcoin/pull/32269#pullrequestreview-2837037436)
lgtm ACK be6e4c4db80030aa100cadb68706601eae13c010

💬 maflcko commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#discussion_r2086979578)
nit: Not sure we want to manually number the sections. The order should already be clear

💬 theStack commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876795632)
@edilmedeiros: I agree it's fine as-is for now and can easily extended later. Note though that including OP_2...OP_16 is trivial, as it's just an extended range to OP_1/OP_TRUE, i.e. a diff like this would likely be sufficient:
```diff
diff --git a/contrib/signet/miner b/contrib/signet/miner
index 27f7afba8d..b79fa5d4f8 100755
--- a/contrib/signet/miner
+++ b/contrib/signet/miner
@@ -245,8 +245,8 @@ def trivial_challenge(spkhex):
challenges such as OP_TRUE
"""
spk = bytes
...

💬 maflcko commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2087011545)
Seems fine, but it may be better to remove the suppression in the exact commit that removes the need for it. This would make it easier to revert the commit atomically in one go, if there is need for whatever reason.

📝 pablomartin4btc opened a pull request: "wallet, refactor: Remove Legacy wallet unused warnings and errors"
(https://github.com/bitcoin/bitcoin/pull/32481)
Remove dead code due to legacy wallet support removal.