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

💬 michaelfolkson commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2876824169)
Concept ACK for this and Concept ACK for #32359 unless @luke-jr/Knots argues convincingly that the removal of these config options in Core makes maintaining them in Knots an excessive amount of work.

The objectives of Core's mempool policy and Knots mempool policy are totally different. Knots is trying to filter what it defines as spam and Core[ isn't](https://btctranscripts.com/adopting-bitcoin/2021/2021-11-16-gloria-zhao-transaction-relay-policy). I don't know if it makes sense for Core to
...

💬 fanquake commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2087016856)
You can drop the changes in here, this file will be updated in a translations update. @hebasto might be worth doing one shortly just to cleanup these strings, since they keep being removed in various PRs.

💬 hebasto commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2876906638)
My Guix build:
```
aarch64
24986a84cc15356c2761a8d05898d33d60ff10431eecdf68d9fb758432a87f79 guix-build-915c1fa72c07/output/aarch64-linux-gnu/SHA256SUMS.part
be7bba66e8e84ba8428aed395a296e397d07f5d961bb8ab80bdd19d2c48589ba guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu-debug.tar.gz
a77553ea1b9eca35f6b9bc00f0be22f3016b8fdf1425b71ec93a6fdbcd8e154c guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu.tar.gz
4f1a82c8
...

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087061842)
Dropped and added `OP_2` as an example for inspiration.

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087066202)
I switched to using a range, where it makes more sense to use a code comment.

💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2087066691)
Any accounting inconsistency could've caused it to become negative and fail, so instead of removing these suppressions in the last commit that fixed the problem (giving the impression that it was just one change that enabled us removing it), I did it separately instead.
Is this important, is this holding back the ACK from your part?

💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927264)
I agree with you, but I also prefer the current code; seems easier to understand.

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927696)
I added `OP_2` through `OP_16`, with a new test case for the latter. It's not practical to catch _all_ possible trivial scripts though.