👋 maflcko's pull request is ready for review: "build: Require libc++-16 or later"
(https://github.com/bitcoin/bitcoin/pull/29077)

💬 maflcko commented on pull request "build: Require libc++-16 or later":
(https://github.com/bitcoin/bitcoin/pull/29077#discussion_r1590811175)
This is now at v11

💬 fanquake commented on pull request "build, test, doc: Temporarily remove Android-related stuff":
(https://github.com/bitcoin/bitcoin/pull/30049#discussion_r1590816532)
Need to drop applying this as well

💬 hebasto commented on pull request "build, test, doc: Temporarily remove Android-related stuff":
(https://github.com/bitcoin/bitcoin/pull/30049#discussion_r1590818277)
Thanks! Dropped.

💬 maflcko commented on pull request "build: Require libc++-16 or later":
(https://github.com/bitcoin/bitcoin/pull/29077#discussion_r1590821935)
Thanks, clarified that "libstdc++ (any version) requires at least clang-16"

📝 josibake opened a pull request: "refactor, wallet: get serialized size of `CRecipient`s directly"
(https://github.com/bitcoin/bitcoin/pull/30050)
Broken out from #28201

---

In order to estimate fees properly, we need to know what the final serialized transaction size will be. This PR refactors `CreateTransactionInternal` to:

* Get the serialized size directly from the `CRecipient`: this sets us up in a future PR to calculate the serialized size of silent payment `CTxDestinations` (see https://github.com/bitcoin/bitcoin/pull/28201/commits/797e21c8c1bf393e668eeef8bb7ee9cae1e5e41e)
* Use the new `GetSerializeSizeForRecipient` to m
...

📝 maflcko converted_to_draft a pull request: "build: Require libc++-16 or later"
(https://github.com/bitcoin/bitcoin/pull/29077)
I am not familiar with macOS, but given that https://github.com/bitcoin/bitcoin/pull/28622 prescribes a minimum of libc++-16 for macOS, doing the same on Linux could make sense?

This is needed for stuff:

* `std::ranges::equal`, see https://github.com/bitcoin/bitcoin/pull/29071
* `std::source_location`, see https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1851949647

I don't think anyone is using `-stdlib=libc++` on Linux, except for the CI and OSS-Fuzz?

The minimum required
...

💬 maflcko commented on pull request "build, bench, msvc: Add missing benchmarks":
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095738135)
lgtm ACK 31a15f0aff79d2b34a9640909b9e6fb39a647b60

📝 josibake opened a pull request: "crypto, refactor: add method for applying the taptweak"
(https://github.com/bitcoin/bitcoin/pull/30051)
The wallet returns an untweaked internal key for taproot outputs. If the output commits to a tree of scripts, this key needs to be tweaked with the merkle root. Even if the output does not commit to a tree of scripts, BIP341/342 recommend commiting to a hash of the public key.

Previously, this logic for applying the taptweak was implemented in the `CKey::SignSchnorr` method.

This PR moves this logic to its own method, `CKey::ApplyTapTweak` and refactors CKey::SignSchnorr` to use this new m
...

💬 Sjors commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#issuecomment-2095752757)
Don't forget to add a source comment that BIP352 depends on this exact ordering.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2095761769)
Concept ACK

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2095786951)
I'm fine with either fixing the timewarp or not, but in the latter case we might needs testnet5 soon - if someone decides to exploit it.

> I think at least we need another variation with the latest block hash in the message.

I don't think that's necessary. However, you could use a recent block hash as a provably unspendable public key (instead of all zeros).

💬 darosior commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#discussion_r1590889878)
But 1 sorts before 256 lexicographically? So it's lexicographical *serialized* order, i see.

💬 instagibbs commented on pull request "p2p: Allow 1P1C to fetch parent from compact block extra_txn":
(https://github.com/bitcoin/bitcoin/pull/30032#issuecomment-2095820937)
from my non-listening node, looks like ~3% of the time of package evaluation is being considered it triggers

💬 Sjors commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2095830881)
Briefly retested 83c6332dc7b8145b19d725830b26dc4ef21e4b61 which still works.

👍 1440000bytes approved a pull request: "chainparams: Add achow101 DNS seeder"
(https://github.com/bitcoin/bitcoin/pull/30007#pullrequestreview-2040610276)
ACK https://github.com/bitcoin/bitcoin/pull/30007/commits/ee218aa9a9eaac53030c31b099b4afe354197ba7

💬 hebasto commented on pull request "build, bench, msvc: Add missing benchmarks":
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095835910)
cc @sipsorcery

🤔 theStack reviewed a pull request: "crypto: add `NUMS_H` const"
(https://github.com/bitcoin/bitcoin/pull/30048#pullrequestreview-2040638390)
Concept ACK

Found via `$ git grep 5092929b74` that we already define this byte-string constant twice for the miniscript fuzz and unit tests (`src/test/fuzz/miniscript.cpp` and `src/test/miniscript_tests.cpp`, called `NUMS_PK` there), that's a good opportunity to deduplicate.

💬 josibake commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#discussion_r1590976847)
Yep, good point. I added a comment to clarify that this is a lexicographic sort on the serialized outpoint. I think this is preferable to what we were doing before since its based on the protocol definition of an outpoint: `<32-byte txid, little-endian>:<4-byte vout, little-endian>`.

💬 josibake commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#issuecomment-2095964573)
> Don't forget to add a source comment that BIP352 depends on this exact ordering. Either in this PR or as a commit in #28122.

Hm, I don't think we need a BIP352 specific comment here? I'd argue if we are going to sort outpoints, we should be sorting them based on their protocol definition, regardless of BIP352. In fact, when writing BIP352 we already assumed this was the case which is why we defined the outpoint sorting in BIP352 to be done lexicographically on the outpoint serialization. It
...