💬 achow101 commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715139938)
> Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?

Makes sense if you don't want the wallet?

💬 volkanural commented on pull request "BIP-322 basic support":
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-2715146758)
> > @kallewoofBIP-322'yi uygulayan başka bir cüzdan olup olmadığını biliyor musunuz, böylece uygulamayı karşılaştırabiliriz?
> > Bu, BIP'e (ve bu PR'ye) test vektörleri eklemek için gerçekten iyi bir zamandır.
>
> Burada bir uygulamam var: [bitcoin-s/bitcoin-s#3823](https://github.com/bitcoin-s/bitcoin-s/pull/3823)

💬 volkanural commented on pull request "BIP-322 basic support":
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-2715147604)
open

💬 GarmashAlex commented on pull request "tests: Improve stderr validation in test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/31966#issuecomment-2715155554)
@yancyribbens Sorry, my bad. I removed TODO

💬 hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1989825367)
There are too many of them. The entire Thai (th) translation update has been discarded.

📝 VolodymyrBg opened a pull request: "qt: Add addressList field to SendCoinsRecipient for multiple addresses"
(https://github.com/bitcoin/bitcoin/pull/32035)
This pull request adds a new field `addressList` to the SendCoinsRecipient class
to properly handle multiple addresses from unauthenticated payment requests.
Previously, the `address` field was being abused for this purpose, as noted
in a TODO comment.

The changes include:
- Adding a new `addressList` field to SendCoinsRecipient
- Updating the serialization methods to handle the new field
- Modifying PaymentServer to populate the new field when multiple addresses
are present
- Updat
...

💬 hebasto commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715187814)
> > Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?
>
> Makes sense if you don't want the wallet?

```
$ make -C depends NO_SQLITE=1
$ cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake
-- The CXX compiler identification is GNU 14.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-
...

💬 hebasto commented on pull request "qt: Add addressList field to SendCoinsRecipient for multiple addresses":
(https://github.com/bitcoin/bitcoin/pull/32035#issuecomment-2715197889)
Please re-open this PR in https://github.com/bitcoin-core/gui.

✅ hebasto closed a pull request: "qt: Add addressList field to SendCoinsRecipient for multiple addresses"
(https://github.com/bitcoin/bitcoin/pull/32035)

💬 mzumsande commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715268576)
> _23.5 more minutes_

That's way longer than I thought it would take but I guess the UTXO set has grown a lot over the last years.

My idea of a fix would be to set some kind of DB flag such as (`DB_ASSUMEUTXO_VALIDATED`) in the background chainstate after the assumeutxo hash was successfully validated the first time, and then skip the check at next startup if that flag was set. I think I'll try that out / open a PR if it works.

💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2715298456)
> > Perhaps @purpleKarrot could suggest a better approach?
>
> CPack?

https://github.com/bitcoin/bitcoin/blob/dbc89b604c4dae9702f1ff08abd4ed144a5fcb76/cmake/module/Maintenance.cmake#L47-L48 :)

💬 Sjors commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715317205)
I could change depends here, but maybe it's easier to do that in the final PR that drops BDB itself?

💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1989903555)
> This seems far too verbose...

Having an optional dependency, the absence of which is revealed during the configuration stage, is quite unusual and requires better communication to the user.

> This seems ... exposing implementation details

`MAKENSIS_EXECUTABLE` is a cache variable that the user is expected to set when necessary, like any other cache variable. It cannot be considered as "implementation details".

> we don't do this for any other dep

Other deps are handled during th
...

📝 VolodymyrBg opened a pull request: "qt: Add addressList field to SendCoinsRecipient for multiple addresses"
(https://github.com/bitcoin-core/gui/pull/857)
This pull request adds a new field addressList to the SendCoinsRecipient class
to properly handle multiple addresses from unauthenticated payment requests.
Previously, the address field was being abused for this purpose, as noted
in a TODO comment.

The changes include:

- Adding a new addressList field to SendCoinsRecipient
- Updating the serialization methods to handle the new field
- Modifying PaymentServer to populate the new field when multiple addresses
are present
- Updating Re
...

💬 l0rinc commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715508521)
23 minutes on my M4 Max - likely ~50 minutes on commodity hardware

🤔 darosior reviewed a pull request: "qt: 29.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2675997883)
Sanity checked the three French translation files (`bitcoin_fr.ts`, `bitcoin_fr_CM.ts` and `bitcoin_fr_LU.cs`) and i did not find anything malicious as of 9132824947005421057f6a5f035082c7b99f3853. There is a lot of unnecessary churn however.

💬 darosior commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1990054501)
Seems harmless but there is some spurious symbol inserted in some sentences, such as this period here.

🤔 glozow reviewed a pull request: "qt: 29.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2676023046)
ACK 9132824947005421057f6a5f035082c7b99f3853

Lightly sanity checked by pulling translations and diffing with this, grepping for URLs and addresses, and spot-checking a few strings.

💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1989248213)
Seems brittle on master that we just `delete` the node here without checking the refcount. From what I see we only call `StopNodes()` from tests and `CConnman::Stop()`, and we only create `NodesSnapshot`s inside threads we've already stopped earlier in `Stop()`.

💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1989217778)
Function body was removed.