💬 maflcko commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086887015)
Not sure. This will make compile time and runtime slower. I don't see the point?

💬 maflcko commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086887675)
thx, done

👋 fanquake's pull request is ready for review: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32292)

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086911846)
that's not the same behavior IIUC

```
diff --git a/test/functional/mempool_datacarrier.py b/test/functional/mempool_datacarrier.py
index 6347215b86..ce1d65a9ed 100755
--- a/test/functional/mempool_datacarrier.py
+++ b/test/functional/mempool_datacarrier.py
@@ -20,16 +20,17 @@ from random import randbytes
# The historical maximum, now used to test coverage
CUSTOM_DATACARRIER_ARG = 83

class DataCarrierTest(BitcoinTestFramework):
def set_test_params(self):
- self.num_
...

💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876659639)
> > I was wondering whether OP_2...OP_16 should also be treated as trivial,
>
> It may be useful to have multiple public signets out there and avoid them accidentally reorging each other.

It would be useful indeed.

[BIP325](https://github.com/bitcoin/bips/blob/master/bip-0325.mediawiki) specifies a behavior that's impossible to detect in the general case, though:

> There is one other acceptable special case: if a block's challenge is e.g. `OP_TRUE` (`0x51`), where an empty solution w
...

💬 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 :)