💬 maflcko commented on pull request "build, bench, msvc: Add missing benchmarks":
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095609119)
An alternative would be to update the generation script?

📝 hebasto opened a pull request: "build, test, doc: Temporarily remove Android-related stuff"
(https://github.com/bitcoin/bitcoin/pull/30049)
Previously, our Android builds were geared towards generating APKs, which relied on Qt. However, after migrating to C++20, compiling for Android became unfeasible due to Qt 5.15's compatibility limitations with NDK only up to r25, which includes an outdated embedded libc++.

All removed stuff will be reinstated after migrating the build system to CMake and upgrading Qt to version 6.x."

This PR makes possible a clean migration to the CMake-based build system as it removes code, which is not
...

📝 hebasto converted_to_draft a pull request: "build, test, doc: Temporarily remove Android-related stuff"
(https://github.com/bitcoin/bitcoin/pull/30049)
Previously, our Android builds were geared towards generating APKs, which relied on Qt. However, after migrating to C++20, compiling for Android became unfeasible due to Qt 5.15's compatibility limitations with NDK only up to r25, which includes an outdated embedded libc++.

All removed stuff will be reinstated after migrating the build system to CMake and upgrading Qt to version 6.x."

This PR makes possible a clean migration to the CMake-based build system as it removes code, which is not
...

👋 hebasto's pull request is ready for review: "build, test, doc: Temporarily remove Android-related stuff"
(https://github.com/bitcoin/bitcoin/pull/30049)

💬 josibake commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#discussion_r1590805470)
Although, to your point, `a.hash < b.hash` is doing a lexicographic comparison:

https://github.com/bitcoin/bitcoin/blob/e623d8a05ad4e2b81e19c4f10b22e2ce4db35b8d/src/uint256.h#L54

So maybe it would be more efficient to just serialize `a.vout, b.vout` as little-endian here, rather than serializing the whole `COutPoint`?

💬 hebasto commented on pull request "build, bench, msvc: Add missing benchmarks":
(https://github.com/bitcoin/bitcoin/pull/29773#issuecomment-2095647236)
> An alternative would be to update the generation script?

Yes. However, in the light of coming migration to CMake, it makes no substantial difference, right?

👋 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