💬 RandyMcMillan commented on pull request "util: Use steady clock instead of system clock to measure durations":
(https://github.com/bitcoin/bitcoin/pull/27405#issuecomment-1536244850)
Concept ACK

💬 fanquake commented on issue "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test":
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536245439)
> Is this reproducible?

Has happened 2/2 runs so far.

🚀 fanquake merged a pull request: "test, init: perturb file to ensure failure instead of only deleting them"
(https://github.com/bitcoin/bitcoin/pull/26653)

💬 dergoegge commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186081315)
I think this belongs in `net.cpp` and shouldn't use the block height.

💬 dergoegge commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186082093)
(ignore if you were going to change this once out of draft anyway)

👍 stickies-v approved a pull request: "util: improve FindByte() performance"
(https://github.com/bitcoin/bitcoin/pull/19690#pullrequestreview-1414773712)
re-ACK 72efc26439da9a1344a19569fb0cab01f82ae7d1

Verified that the only difference is to include `<util/fs.h>` instead of `<fs.h>` (from https://github.com/bitcoin/bitcoin/pull/27254/commits/00e9b97f37e0bdf4c647236838c10b68b7ad5be3)

```range-diff
% git range-diff HEAD~2 0fe832c4a4b2049fdf967bca375468d5ac285563 HEAD
1: 5842d92c8 ! 1: 604df63f6 [bench] add streams findbyte
@@ src/bench/streams_findbyte.cpp (new)
+
+#include <bench/bench.h>
+
-+#include <fs.h>

...

⚠️ Sjors opened an issue: "psbt: set global_xpubs (at least for multisig descriptors)"
(https://github.com/bitcoin/bitcoin/issues/27583)
### Please describe the feature you'd like to see added.

The `walletcreatefundedpsbt`, `walletprocesspsbt` and `send`* RPC, as well as the send dialog* in the GUI should populate the `PSBT_GLOBAL_XPUB` field (defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki)).

At least when used in a multisig context, e.g. when spending from a `multi()` descriptor.

The Ledger Bitcoin app enforces this as of version 2.1.1., see https://github.com/bitcoin-core/HWI/issues/6
...

💬 MarcoFalke commented on issue "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test":
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1536249866)
Ok, then it would be nice to try outside the CI env, starting with the same configure flags

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186087445)
I wasn't happy with the location myself but I needed more time to think about where it would fit in best, so happy to take suggestions. I just tried to put it somewhere where it requires the least amount of new dependencies and shipped it to get conceptual feedback.

> and shouldn't use the block height.

What do you suggest to use instead of block height?

💬 Sjors commented on issue "psbt: set global_xpubs (at least for multisig descriptors)":
(https://github.com/bitcoin/bitcoin/issues/27583#issuecomment-1536254243)
See also this discussion: https://github.com/cryptoadvance/specter-desktop/issues/1886#issuecomment-1246963927

💬 ryanofsky commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1186086311)
re: https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1185099728

In general it's ok to pass pointers and references. These are both really useful for output parameters so the IPC framework does support them. But the type needs to be serializable, and it isn't really possible to write serialization code for the `CBlockIndex` type because of the pprev pointers it contains. It compile error in #10102 to try to pass types that aren't serializable.

For this PR would suggest not adding a
...

💬 MarcoFalke commented on issue "[Linux] Add wayland support":
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-1536259204)
I don't really buy the concern. Even if this is adding additional depends in a gui/qt-only context, what is the downside? If this allows people to get a feature they want and there are people who maintain it? If someone doesn't use gui/qt, they should be unaffected by this.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1186093566)
Thanks for taking a look. It's also not used by an actual client of the interface, so I think I'll just drop the commit then. No need to needlessly pollute the interface.

💬 hebasto commented on issue "[Linux] Add wayland support":
(https://github.com/bitcoin/bitcoin/issues/19950#issuecomment-1536263017)
OK. Reopened this issue first :)

⚠️ hebasto reopened an issue: "[Linux] Add wayland support"
(https://github.com/bitcoin/bitcoin/issues/19950)
**Is your feature request related to a problem? Please describe.**
- Wayland provides extra security as apps can't have access to windows from other apps
- X apps looks blurry using fractional scaling on GNOME.
- X is mostly unmaintained, in favour of Wayland

**Describe the solution you'd like**
Add Wayland support to precompiled binaries.

**Additional context**
As far as I understand wayland is supported on the KDE 5.15 runtime. Currently running with `QT_QPA_PLATFORM="wayland"` res
...

💬 dergoegge commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1186101443)
> happy to take suggestions.

I think it would make sense for the `opencon` thread (`ThreadOpenConnections`) to handle this.

> What do you suggest to use instead of block height?

```c++
if (m_last_asmap_health_check + 24h < NodeClock::now()) {
// Do health check
}
```

👍 dergoegge approved a pull request: "fuzz: BIP 42, BIP 30, CVE-2018-17144"
(https://github.com/bitcoin/bitcoin/pull/17860#pullrequestreview-1414856320)
Code review ACK fa2d8b61f9343d350b67357a12f39b613c8ee8ad

👍 pablomartin4btc approved a pull request: "Wallet : Allow user to navigate options while encrypting at creation"
(https://github.com/bitcoin-core/gui/pull/722#pullrequestreview-1414865992)
re-ACK 78660e72001a2561c7ad3026367a69f65414dbd9. cc @jarolrod.

I see[`wallet_create_tx.py --descriptors`](https://cirrus-ci.com/task/6164029874372608?logs=functional_tests#L2752)test is failing and you are re-running it, I don't think it's related with your change but let's see.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1536319288)
Thank you for the review @MarcoFalke and @ryanofsky

Updated 9033dc6827cc623f1f7176fde120229f36ff5e74 -> e056d6f758382d3418682095ab02f8d487aa641f ([removeBlockstorageArgs_19](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_19) -> [removeBlockstorageArgs_20](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_20), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_19..removeBlockstorageArgs_20))
* Dropped the commit changing t
...

💬 RandyMcMillan commented on issue "deadlock with wrong system date/time and non-empty wallet":
(https://github.com/bitcoin-core/gui/issues/643#issuecomment-1536322254)
I vaguely remember finding a solution to this... I will try to find it - buried in a comment somewhere in bitcoin/bitcoin issues.

If I recall - the fix had to do with (re)casting the time back to int when passing it to the gui. The gui wasnt handling the chronos? variable correctly - it was likely a bug in the QT gui framework itself. I wasnt able to "prove" it because the crash didnt produce a stack error (for the gui) - but it worked. :)

I will try repost the fix with some tests to prov
...

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin-gui -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/19461#issuecomment-1536369001)
Thanks Sjors, I'm trying to reproduce the shutdown error but didn't succeed yet. Will try again with a mainnet node. I didn't look into the external signer bug yet, but maybe that is more straightforward. In both cases running with -debug=ipc could make it clearer what's going on, especially in the shutdown case. Probably what is happening in that case is that some thread is trying make an IPC call after the socket is closed, so an ipc::Exception is raised which is uncaught. Fix might be synchro
...