💬 darosior commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#discussion_r1590747954)
I fail to see how it's not functionally equivalent?

👍 maflcko approved a pull request: "rpc: return warnings as an array instead of just a single one"
(https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-2040332641)
ACK 42fb5311b19582361409d65c6fddeadbee14bb97 🔺

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 42fb5311b19582361409d65c6f
...

💬 maflcko commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1590743718)
```suggestion
can temporarily be restored by running with configuration
```

nit (only if you re-touch): Could remove the executable name, as it can be `bitcoin-qt` as well, or a different name.

💬 maflcko commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1590753252)
> kernel

I guess it is a bit odd to have `g_timeoffset_warning` in the kernel, but happy to review either approach.

📝 josibake opened a pull request: "refactor: Model the bech32 charlimit as an Enum"
(https://github.com/bitcoin/bitcoin/pull/30047)
Broken out from #28122

---

Bech32(m) was defined with a 90 character limit so that certain guarantees for error detection could be made for segwit addresses (see https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#checksum-design).

However, there is nothing about the encoding scheme itself that requires a limit of 90 and in practice bech32(m) is being used without the 90 char limit (e.g. lightning invoices, silent payments). Further, increasing the character limit doesn't d
...

💬 josibake commented on pull request "Compare `COutPoints` lexicographically":
(https://github.com/bitcoin/bitcoin/pull/30046#discussion_r1590763287)
See https://github.com/bitcoin/bitcoin/blob/246e2a24c5460504c799fbc6312ef075f0ca4e72/src/test/data/bip352_send_and_receive_vectors.json#L383 for an example of how the old method (a.hash < b.hash, a.vout < b.vout) will yield a different ordering than comparing outpoints lexicographically.

📝 josibake opened a pull request: "crypto: add `NUMS_H` const"
(https://github.com/bitcoin/bitcoin/pull/30048)
Broken out from #28122

---

[BIP341](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs) defines a NUMS point `H` as *H = lift_x(0x50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0)* which is [constructed](https://github.com/ElementsProject/secp256k1-zkp/blob/11af7015de624b010424273be3d91f117f172c82/src/modules/rangeproof/main_impl.h#L16) by taking the hash of the standard uncompressed encoding of the [secp256k1](https:/
...

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1590788176)
opened in https://github.com/bitcoin/bitcoin/pull/30046

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