💬 l0rinc commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2056768817)
We're continually updating this doc and for now we're adding more and more to the mac section - it's not getting any simpler for now, seems we still need the mac section for now. And when that changes, we'll update the doc.

💬 l0rinc commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#issuecomment-2825349060)
> it would be good to mention [Wiki: AddressSanitizerContainerOverflow (false positives) (google/sanitizers)](https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow#false-positives)

Added the links to the doc and added another workaround of setting the target architecture an Apple Silicon for errors like:
```bash
usr/include/malloc/_malloc_type.h:66:126: error: unknown type name 'size_t'; did you mean 'std::size_t'?
```

🤔 janb84 reviewed a pull request: "guix: Remove unused `file` package"
(https://github.com/bitcoin/bitcoin/pull/32242#pullrequestreview-2788550078)
Code review and Tested ACK [513e202](https://github.com/bitcoin/bitcoin/commit/513e2020a9acdd366d4933780b331f97bac85597)

- Did a code review
- Full GUIX build under nixos on macos via UTM, works !

💬 maflcko commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2825373348)
I've pushed an update to check more broadly. The new issues (for just Polish) include some false positives around (change/reszta). Though, the others largely apply as far as I can see:

#### Erroneous translation:
<source>Warning: If you encrypt your wallet and lose your passphrase, you will &lt;b&gt;LOSE ALL OF YOUR BITCOINS&lt;/b&gt;!</source>
<translation>hasłoOstrzeżenie: Jeśli zaszyfrujesz swój portfel i zgubisz hasło - &lt;b&gt;STRACISZ WSZYSTKIE SWOJE BITCONY&lt;/b&gt;!</t
...

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056790435)
Rather than explaining the line below, I added this comment to add context that I think isn't immediately available from the test, namely what the (obscure in both use and name) `listwalletdir` rpc does, I think your suggested comment would repeat what the test is asserting.

🤔 achow101 reviewed a pull request: "Added rescan option for import descriptors"
(https://github.com/bitcoin/bitcoin/pull/31668#pullrequestreview-2788566889)
There should also be a test where multiple things are imported, one with a timestamp, and one with `never`. There should be a rescan in this case.

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2056798497)
While this verifies that the address was imported, what we care about here is whether a rescan occurred. It would be better to send to the address for the descriptor being imported, then check to see that after importing, the transaction is not detected.

🤔 mzumsande reviewed a pull request: "wallet: Ensure best block matches wallet scan state"
(https://github.com/bitcoin/bitcoin/pull/30221#pullrequestreview-2788360332)
Looks good, except for an issue with commit 00fe040eca4b77d05ec5652b4d494b0ded183653

💬 mzumsande commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2056670803)
This function is declared here, but not implemented or used anywhere. Is this meant to be called somewhere in `UnloadWallets`? Or does UnloadWallets already close the db somewhere (where?) and this can be deleted?

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056813556)
Unlike, [`assert_not_equal`](https://github.com/bitcoin/bitcoin/blob/6b4b7ac2113ed969bd31ce42190b96bd2e1b9c97/test/functional/test_framework/util.py#L79-L81), `assert_equal` [does not support](https://github.com/bitcoin/bitcoin/blob/6b4b7ac2113ed969bd31ce42190b96bd2e1b9c97/test/functional/test_framework/util.py#L72-L77) passing an error message. Re: the conceptual change; while it might have some advantages over the style currently used, as far as I can tell it is almost never the case that we l
...

💬 achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2056818503)
Just forgot to remove it, done now.

The db is closed when a `CWallet` is destroyed.

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056837989)
We are already asserting the transaction id exists in the migrated wallet, and that the wallet has the correct balance, I'm not totally opposed to checking the tx amount as opposed to the wallet balance, but I'm not sure that this change really does add any guarantees, or help with debugging when the test fails, so I will err on the side of keeping this as-is so it [parallels](https://github.com/bitcoin/bitcoin/blob/9a4c92eb9ac29204df3d826f5ae526ab16b8ad65/test/functional/wallet_migration.py#L53
...

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056839286)
I'll leave this as-is for now.

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056840721)
See https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056813556.

💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2056844763)
If `open` fails an exception will be thrown and the block below it will not be executed.

https://docs.python.org/3/reference/compound_stmts.html#with

🚀 achow101 merged a pull request: "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan"
(https://github.com/bitcoin/bitcoin/pull/32023)

💬 achow101 commented on pull request "validation: set BLOCK_FAILED_CHILD correctly":
(https://github.com/bitcoin/bitcoin/pull/31835#issuecomment-2825477728)
ACK 3c3548a70eedb8dcf6a4a8d605a4a12e814c7cac

🚀 achow101 merged a pull request: "validation: set BLOCK_FAILED_CHILD correctly"
(https://github.com/bitcoin/bitcoin/pull/31835)

💬 mzumsande commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2825517598)
Looks like `WalletMigration` bench fails on this branch (which is now run in the CI).

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2056922934)
Reworked and upstreamed the fix in https://github.com/arun11299/cpp-subprocess/pull/112.