💬 hebasto commented on pull request "Prevent weird focus rect on inital sync":
(https://github.com/bitcoin-core/gui/pull/795#discussion_r1490825810)
Please [use](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c) braces.

💬 maflcko commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1945874746)
I re-checked b20d882fd53c21098eb7858b2dbca84eafd2b344 and re-confirmed `time loadwallet` (https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1943469180)

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#discussion_r1490849107)
@maflcko shouldn't TSan on CI output something useful about why it crashed? I currently only says "error 2": https://cirrus-ci.com/task/5124733717446656?logs=ci#L3531

When running this locally on Ubuntu with clang 16.0.6 I get a `WARNING: ThreadSanitizer: data race` and significantly more details (still a bit cryptic, but hopefully enough to figure out what's happening).

💬 hebasto commented on issue "Bitcoin Core GUI getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/issues/645#issuecomment-1945884609)
@zndtoshi @BrandonOdiwuor

Are there any compelling reasons not using RPC in the "Console" window?

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#discussion_r1490855751)
@vasild any thoughts on how to make mock `Sock`s that can be used to play messages in two directions?

💬 vasild commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490857252)
With this PR all production code passes both arguments. No need anymore for defaults. This compiles:

```diff
- std::pair<CAddress, NodeSeconds> Select(bool new_only = false, std::optional<std::vector<Network>> networks = std::nullopt) const;
+ std::pair<CAddress, NodeSeconds> Select(bool new_only, std::vector<Network> networks) const;
```

Some tests use `Select()` and I adjusted them to do `Select(false, {})` just to check that the compilation will succeed. They can be adjusted to
...

⚠️ maflcko opened an issue: "wallet: pre-HD HDD migratewallet "
(https://github.com/bitcoin/bitcoin/issues/29438)
Opening a brainstorming issue, if someone wants to test `migratewallet` on a pre-HD wallet on a slow storage device (HDD).

Steps to reproduce:

* Create a wallet in a folder. For example: `./releases/bitcoin-0.11.2/bin/bitcoin-qt -datadir=/tmp/rgw -regtest`. Then stop bitcoin-qt.
* Copy the wallet file to a folder residing on the intended storage device and call `migratewallet` with the latest version of Bitcoin Core.

The second step can be done by a functional test (I modified an exist
...

💬 maflcko commented on pull request "rpc: Drop migratewallet experimental warning":
(https://github.com/bitcoin/bitcoin/pull/28037#issuecomment-1945902492)
Done in https://github.com/bitcoin/bitcoin/issues/29438 . Maybe move the discussion there, so that this can move forward and be merged?

💬 jadijadi commented on pull request "Prevent weird focus rect on inital sync":
(https://github.com/bitcoin-core/gui/pull/795#issuecomment-1945903374)
> Why does the issue happen only on certain platforms?

Not sure about other platforms. I had a mac and a KDE machine and happened on both of them. Have not checked on other platforms.

💬 Sjors commented on issue "RFC: Migration from NSUserNotificationCenter to UNUserNotificationCenter on macOS":
(https://github.com/bitcoin-core/gui/issues/112#issuecomment-1945907679)
I think (2) is fine. We already sign the release build, and anyone who can compile from source can run the extra `codesign` command (maybe as part of `make deploy`?).

If (1) just means that they have to use `make deploy` in order to get notifications working, that seems fine.

🤔 hebasto reviewed a pull request: "RFC: build: remove confusing and inconsistent disable-asm option"
(https://github.com/bitcoin/bitcoin/pull/29407#pullrequestreview-1882535266)
Concept ACK.

💬 maflcko commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#discussion_r1490880303)
Ah, I guess the unit tests don't capture the tsan output?

💬 maflcko commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#discussion_r1490884343)
But they should. At least back when I tested https://github.com/bitcoin/bitcoin/pull/27667

💬 maflcko commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#discussion_r1490885359)
Maybe good to re-check this when/after the cmake migration is done?

💬 maflcko commented on issue "wallet: pre-HD HDD migratewallet ":
(https://github.com/bitcoin/bitcoin/issues/29438#issuecomment-1945940628)
Putting a perf context around the RPC call (`with n.profile_with_perf("migrate-wallet-perf"):`) allows to generate flame graphs. However, I am not sure how useful they are, as they may not capture IO?

SSD:



HDD:


💬 hebasto commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#discussion_r1490894282)
It seems there are no reasons to keep `libbitcoin_crypto_sse4` library anymore. The `crypto/sha256_sse4.cpp` source might be a part of the `libbitcoin_crypto_base`, no?

✅ maflcko closed an issue: "wallet: pre-HD HDD migratewallet "
(https://github.com/bitcoin/bitcoin/issues/29438)

💬 maflcko commented on issue "wallet: pre-HD HDD migratewallet ":
(https://github.com/bitcoin/bitcoin/issues/29438#issuecomment-1945949263)
Closing for now, but feel free to leave a comment if you think this should be re-opened. Happy to provide more details, if there are questions.

💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490912711)
Yes, nice catch. I'll address it.

💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490938595)
It doesn't work with vector I guess. But since I'll change it to `unordered_set`, it will work.