💬 fjahr commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2714343635)
I guess that the revalidation is expected could be added in the design doc here as well: https://github.com/bitcoin/bitcoin/blob/45719390a1434ad7377a5ed05dcd73028130cf2d/doc/design/assumeutxo.md#bitcoind-restarts-sometime-after-snapshot-validation-has-completed

💬 hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#issuecomment-2714345545)
> Based on `git show --stat`, looking for files with massive deletions:
>
> * `src/qt/locale/bitcoin_cs.ts`
>
> * `src/qt/locale/bitcoin_da.ts`
>
> * `src/qt/locale/bitcoin_nl.ts`
>
>
> When I undo the changes to those files, at least Dutch looks fine to me.

Thanks! Updated.

📝 fjahr opened a pull request: "test: Check datadir cleanup after assumeutxo was successful"
(https://github.com/bitcoin/bitcoin/pull/32033)
I noticed that the proper datadir cleanup after a successful restart of an assumutxo node does not seem to be covered in our tests. This is added here.

💬 ryanofsky commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2714361350)
> but the wine tests seem to have been fundamentally broken anyways, both brittle and not catching real [bugs](https://github.com/bitcoin/bitcoin/commit/7b16ae23fba38eaf58c0efec4d1a47214e4b1930), so not a real solution.

It's true that wine tests can't catch every bug that could happen on windows since Wine is another platform and "Is Not an Emulator." But calling the wine tests "fundamentally broken" would seem to imply that there were bugs in wine or in the wine test setup, and I haven't see
...

💬 laanwj commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2714382203)
It almost looks like ld flag `--no-export-dynamic`, which is the default, doesn't work on RISC-V. Not even if added explicitly. Without `--exclude-libs ALL`, all weak symbols imported from libraries end up in the exported binary. With `--exclude-libs ALL` only the ones that are exported by the `.o` file (the behavior we're seeing).

Passing `--export-dynamic` to the x86_64 build approximates what we're seeing on RISC-V. But not entirely, we get more symbols than that. So *some* filtering is happ
...

🚀 glozow merged a pull request: "seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/31960)

👍 pinheadmz approved a pull request: "wallet: fix crash on double block disconnection"
(https://github.com/bitcoin/bitcoin/pull/31757#pullrequestreview-2674634683)
ACK 11f8ab140fe63857f6a93b81021efda8f90ceeda

Built on arm/macos. Confirmed the functional tests passes on the branch, on master bitcoind fails the test with a crash:

> stderr Assertion failed: (TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state)), function AddToWallet, file wallet.cpp, line 1114.

Also confirmed that if the assertion is commented out, the test still passes on both master and branch, indicating that the new state (abandoned) is already correct.

The cod
...

💬 pinheadmz commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1989334604)
nanonit: funny, the only occurrences of `assert(...` (with parenthesis instead of just a space) are in this test ;-)

👍 brunoerg approved a pull request: "Require sqlite when building the wallet"
(https://github.com/bitcoin/bitcoin/pull/31961#pullrequestreview-2674676295)
code review ACK ac6cde731314d981391bc313c98d671c68211d33

🤔 rkrux reviewed a pull request: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2674241033)
Partial review.

Some of the suggested comments I have not categorised as nits so that the newer changes are consistent with the rest of the wallet codebase. However, I'm open to them not being implemented if the PR author and other reviewers believe so.

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989154933)
Here and in other similar occurrences of 33.

```diff
-while (s_val.size() >= 33) {
+while (s_val.size() >= CPubKey::COMPRESSED_SIZE) {
```

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989175187)
Using `CPubKey::COMPRESSED_SIZE + 1` here instead of `34` would make it more explicit and reader-friendly (similarly in the other key unserialization flows below) like done for `PSBT_IN_PARTIAL_SIG` and `PSBT_IN_RIPEMD160` cases previously.

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989129187)
I think these 2 can be benefited with some documentation something like below. It was only after I read the below serialisation code that I understood this type.

```cpp
/** key is a pair of aggregate public key and leaf hash, value is a map of participant pubkey to pubnonce */
std::map<std::pair<CPubKey, uint256>, std::map<CPubKey, std::vector<uint8_t>>> m_musig2_pubnonces;
/** key is a pair of aggregate public key and leaf hash, value is a map of participant pubkey to partial sig */
std:
...

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989151765)
Nit for verbosity here and in the partial sigs loop below.

```diff
-for (const auto& [pubkey, pubnonce] : pubnonces) {
+for (const auto& [participant_pubkey, pubnonce] : pubnonces) {
```

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989218447)
Since it is being reused now, maybe a constant or an enum can also be an option.

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989223541)
I would be in favour of using `compressed` terminology here over `plain` like proposed in the BIP change here: https://github.com/bitcoin/bips/pull/1705

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989243401)
`agg_pubkey`?

Using only `key` alludes to the private key though in this case there is no aggregate private key but still to be consistent with the rest of the codebase and to get rid of any doubt in the mind of the reader.

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989209332)
Maybe?

`(2 * CPubKey::COMPRESSED_SIZE + 1)`
`(2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1)`

💬 rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989306381)
`_pubkeys` instead of `_keys`

In order to be consistent with the codebase

💬 l0rinc commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2714449973)
Thanks for the context @mzumsande, @Sjors, @fjahr.

> so that a user that Ctrl+C's out of the first check

The first check finished fully (took me several nights), but now if I want to use that node, I still can't because it's validating again.

I didn't cancel it this time, the new validation after restart took *23.5 more minutes* (it's frustrating to wait here, maybe https://github.com/bitcoin/bitcoin/pull/31645 helps), telling me yet again that `has been fully validated`:
```bash
[...]
2025-0
...