💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228720121)
In commit "build: add bitcoin-{node,gui} to Maintenance.cmake" (362e233da72b89723cdb9f4f56342716b60976eb)

Note for reviewers: Since this PR was opened, full windows support (for both connecting to unix sockets and spawning subprocesses) was implemented in #32387. #32387 is a draft just because I don't want it to be a review burden while there are other changes like #32345 that I think are more important.

💬 fanquake commented on pull request "Don't fix Python patch version":
(https://github.com/bitcoin/bitcoin/pull/33051#issuecomment-3113844771)
> E.g. if the user installed Python 3.10.14 and 3.10.16 through PyEnv, if HWI was locked to 3.10 it would default to 3.10.16. HWI then can't find any of its dependencies.

Then the user should reinstall the HWI dependencies, given they've changed Python version. They would have the same issue any time they update their Python, to a newer `3.10.x` version.

Regardless, can you solve your issue using something out of https://github.com/pyenv/pyenv?tab=readme-ov-file#understanding-python-versio
...

📝 raul-anton-2005 opened a pull request: "test: ensure change address is set only if it exists"
(https://github.com/bitcoin/bitcoin/pull/33054)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

🤔 glozow reviewed a pull request: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896#pullrequestreview-3051854806)
Looks pretty good! Generally, I prefer more detailed comments when the code is not 100% self-explanatory, particularly for the confusing areas like mempool conflicts

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228715272)
nit: Naming these `parent_it` and `parent_wtx` would make it easier to understand

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228645591)
This might be clearer:


```suggestion

// an unconfirmed TRUC must spend only TRUC, and non-TRUC must only spend non-TRUC
if ((wtx.tx->version == TRUC_VERSION) != (coinControl->m_version == TRUC_VERSION)) continue;
```

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228722754)
nit: I think "marks as unspendable" seems a little strong? Perhaps also explain why you are doing this

```suggestion
// Unconfirmed TRUC transactions are only allowed a 1-parent-1-child topology.
// For any unconfirmed v3 parents (there should be a maximum of 1 except in reorgs),
// record this child so the wallet doesn't try to spend any other outputs
```

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228739872)
I think you could `Assume(!wtx.v3_spend.has_value())` here

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228744739)
Naming everywhere should replace "v3" with "truc." We have so many versions and "legacy" types everywhere, so it's best to try to reduce ambiguity when possible. Also, "child" is more clear than "spend," and "in_mempool" would also help with clarity.

```suggestion
std::optional<Txid> truc_child_in_mempool;
```

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228762116)
Naming this `sibling_txid` would be clearer

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228759954)
Again would be helpful to add one or two sentences to explain why we need to do this and why marking as mempool conflict is the appropriate treatment for a sibling (I don't think it's immediately obvious to everyone).

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228751179)
Question: why is eecff95b6fe611c8ed40895108aacd6b44543234 necessary? It would be helpful to put the motivation in the commit message.

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2228783155)
I think perhaps the wallet's default tx version should be specified in a separate constexpr that lives in the wallet code?

💬 Sjors commented on pull request "Don't fix Python patch version":
(https://github.com/bitcoin/bitcoin/pull/33051#issuecomment-3113870419)
It's not really a problem for me, but I noticed HWI stopped using patch versions in https://github.com/bitcoin-core/HWI/pull/695 and I don't think there's a good reason for us to insist on one.

Everywhere else in the project we only specify the Python minor version. And if there's ever a security issue with the patch version we picked, we probably should go and update it (which we've never done).

✅ raul-anton-2005 closed a pull request: "test: ensure change address is set only if it exists"
(https://github.com/bitcoin/bitcoin/pull/33054)

💬 fanquake commented on pull request "Don't fix Python patch version":
(https://github.com/bitcoin/bitcoin/pull/33051#issuecomment-3113880043)
> and I don't think there's a good reason for us to insist on one.

Well at least one is that we don't need to add more code/"workarounds", for a problem we don't have. (https://github.com/bitcoin/bitcoin/pull/33051#issuecomment-3113814862)

💬 Sjors commented on pull request "Don't fix Python patch version":
(https://github.com/bitcoin/bitcoin/pull/33051#issuecomment-3113885802)
But why are we cloning the PyEnv repo, but then not just using `pyenv` install? Then the workaround wouldn't be needed.

📝 raul-anton-2005 opened a pull request: "test: handle potential None value for change address in setlabel"
(https://github.com/bitcoin/bitcoin/pull/33055)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3113898522)
> so maybe if we had a separate PR enabling IPC in CI jobs first, and then this PR could just flip the depends

That's what I did before and people didn't like that: https://github.com/bitcoin/bitcoin/pull/30975#pullrequestreview-2761133797

💬 martinatime commented on issue "bitcoind 29.0 much slower than 28.0 on my system: cause found":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-3113904145)
> I'm running a Raspberry Pi 5 with 8GB of RAM and a 4TB SSD on RPi's Debian Lite Bookworm. This is a fresh install of v29 and after more than two weeks I'm only at blockheight of 829564 of 901531.

Just to update....this setup is still running and is at blockheight of 877015 of 906866. This means that I have been doing the initial state verification for close to two months and my estimation is that I have about 20 more days.