📝 darosior opened a pull request: "validation: detect witness stripping early on"
(https://github.com/bitcoin/bitcoin/pull/33105)
Since it was introduced in 4eb515574e1012bc8ea5dafc3042dcdf4c766f26 (#18044), the detection of a stripped witness relies on running the Script checks 3 times. In the worst case, this consists in running Script validation for every single input.

Detection of a stripped witness is necessary because in this case wtxid==txid, and the transaction's wtxid must not be added to the reject filter or it could allow a malicious peer to interfere with txid-based orphan resolution as used in 1p1c package
...

💬 darosior commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3137704209)
I opened https://github.com/bitcoin/bitcoin/pull/33105, which ended up implementing a variant of @ajtowns' suggestion, because as pointed out the resource consumption gains can be achieved independently of this work (which i still think is desirable) as a much smaller patch which i think would be nice to get in before the feature freeze.

💬 achow101 commented on pull request "depends: hard-code necessary c(xx)flags rather than setting them per-host":
(https://github.com/bitcoin/bitcoin/pull/32584#issuecomment-3137786763)
ACK 9954d6c833381a44e1ea34d182ffe4d61b65d2ba

🚀 achow101 merged a pull request: "init: [gui] Avoid UB/crash in InitAndLoadChainstate"
(https://github.com/bitcoin/bitcoin/pull/32987)

🚀 achow101 merged a pull request: "depends: hard-code necessary c(xx)flags rather than setting them per-host"
(https://github.com/bitcoin/bitcoin/pull/32584)

🤔 jlest01 reviewed a pull request: "refactor: remove unused `ser_writedata16be` and `ser_readdata16be`"
(https://github.com/bitcoin/bitcoin/pull/33093#pullrequestreview-3073357661)
ACK https://github.com/bitcoin/bitcoin/pull/33093/commits/0431a690c3a498a1e728c9df34a132ac16177a04

💬 gmaxwell commented on pull request "net, validation: don't punish peers for consensus-invalid txs":
(https://github.com/bitcoin/bitcoin/pull/33050#issuecomment-3137886715)
I don't see why all standardness rules can't be validated prior to doing any costly hashing or signature validation (sadly, they do require looking up inputs which is costly).

If indeed standardness validation happens before costly operations then the fact that they drop txn without disconnecting the peer doesn't really tell us anything about what should be done *after* performing expensive operations.

But most critically the primary reason to disconnect (and potentially keep disconnected)
...

💬 achow101 commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2243912114)
I also think it would be better to drop the `m_block_*` class members since they're basically only used by `CustomAppend`.

💬 achow101 commented on pull request "test: Slay BnB Mutants":
(https://github.com/bitcoin/bitcoin/pull/33060#issuecomment-3137945940)
ACK a3cf623364e84819bc16fd407b80d8dba46bbcb5

💬 achow101 commented on pull request "refactor: Update `XOnlyPubKey::GetKeyIDs()` to return a pair of pubkeys":
(https://github.com/bitcoin/bitcoin/pull/32332#issuecomment-3138020102)
-0

I don't think such refactorings are meaningfully useful. While it may improve readability a bit, I don't think the difference is enough to justify making this change. Such refactorings both can cause merge conflicts with other higher priority work, and also make git blame a little bit harder.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#issuecomment-3138094678)
> I would suggest a followup PR to improve various code comments and such.

I can pull them into #29675

💬 achow101 commented on pull request "wallet: Set descriptor cache upgraded flag for migrated wallets":
(https://github.com/bitcoin/bitcoin/pull/33031#discussion_r2244047141)
I've changed this to use the existing `ser_string` utility which is more accurate.

💬 achow101 commented on pull request "wallet: Set descriptor cache upgraded flag for migrated wallets":
(https://github.com/bitcoin/bitcoin/pull/33031#discussion_r2244047413)
Done as suggested.

🤔 jlest01 reviewed a pull request: "test: Slay BnB Mutants"
(https://github.com/bitcoin/bitcoin/pull/33060#pullrequestreview-3073614732)
ACK https://github.com/bitcoin/bitcoin/pull/33060/commits/a3cf623364e84819bc16fd407b80d8dba46bbcb5

It improves the BnB coin selection tests by covering previously untested weight-limit edge cases and verifying the proper error flagging.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2244054257)
> it only ratchets hardened from false to true

No? It resets it to false first.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3138129596)
Added a few commits to address followups from #31244

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-3138169095)
Rebased and added a new commit on top for deduplicating the integer rounding. Ready for review again.

👍 brunoerg approved a pull request: "test: Slay BnB Mutants"
(https://github.com/bitcoin/bitcoin/pull/33060#pullrequestreview-3073687261)
code review ACK a3cf623364e84819bc16fd407b80d8dba46bbcb5

💬 ajtowns commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3138260890)
> However it seemed cleaner to me to get rid of the WITNESS_STRIPPED edge case detection in the first place, which was always meant to go?

I don't think it makes sense to get rid of WITNESS_STRIPPED while we still care about resolving orphans via their missing parents' txids. If/when we have protocol support for receiver-initiated package relay by wtxid, then I think it could make sense to treat every request as being by wtxid -- so a witness-stripped response from a non-wtxid-relay peer woul
...

💬 luke-jr commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3138265471)
Concept NACK. Everything below 1s/vB is spam. There's no reason to change the default.

>Over that period the USD price of BTC has risen by roughly 2-3 orders of magnitude,

It's the USD that has fallen, Bitcoin has remained more or less the same.

>This maintains DoS resistance without discouraging low-value UTXO consolidation.

DoS resistance is clearly insufficient even at 1s/vB.