💬 fanquake commented on pull request "test: Remove windows workaround in authproxy":
(https://github.com/bitcoin/bitcoin/pull/27418#issuecomment-1495927906)
Concept ACK

📝 TheCharlatan opened a pull request: "refactor: Extract common/args from util/system"
(https://github.com/bitcoin/bitcoin/pull/27419)
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the `util/system` files. Its preceding pull request is https://github.com/bitcoin/bitcoin/pull/27254.

The pull request contains an extraction of ArgsManager related functions from util/system into their
...

👋 TheCharlatan's pull request is ready for review: "refactor: Extract common/args from util/system"
(https://github.com/bitcoin/bitcoin/pull/27419)

💬 hebasto commented on pull request "ci: use clang-16 in tidy task":
(https://github.com/bitcoin/bitcoin/pull/27404#issuecomment-1495953487)
FWIW, on a local Ubuntu 23.04 installation, I have no issues with running the IWYU tool manually.

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157298121)
> I haven't written a test for this, so all the following is not verified and just my guess based on the code understanding.
>
> This is not a regression in this particular PR, but this could lead to misleading errors in a situation when a proper solution exists, but our coin selection failed to find it and only found solution that exceeded weight. Let's imagine following scenario:
>
> * UTXO pool: two big coins 50BTC, a ton of 0.001BTC.
> * Target amount: 90BTC
> * Proper solution: just
...

💬 dergoegge commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#issuecomment-1496041020)
> It isn’t the main one actually, there are other areas that shouldn’t be locking the net messages reception / validation thread because of a disk read: e.g. “getblock”, “getblocktxn” and compact block net messages, the current indexes sync process (all of them), getblock/gettxoutproof RPC commands. They all are locking the node at every single block read.

This has been the case for 10+ years, if we're gonna change it then we should take the time to do it without adding to our technical debt.
...

✅ MarcoFalke closed an issue: "Investiage RPC connection lifetime issues"
(https://github.com/bitcoin/bitcoin/issues/18623)

✅ MarcoFalke closed an issue: "test: use-of-uninitialized-value in sqlite3Strlen30"
(https://github.com/bitcoin/bitcoin/issues/27222)

💬 brunoerg commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#discussion_r1157342831)
Yeah, I think it would be simpler to remove the new flag I created and log it when using `-asmap`.

💬 brunoerg commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1496076469)
Just updated the description and changed the approach to leave it simpler. Before I had created a flag `-logasmap` (similar to `-logips`), now I removed it and when using `-asmap` it will includes the peers' ASN in debug output.

💬 amitiuttarwar commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1496088231)
concept ACK. good catch!

💬 furszy commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#issuecomment-1496122583)
> > It isn’t the main one actually, there are other areas that shouldn’t be locking the net messages reception / validation thread because of a disk read: e.g. “getblock”, “getblocktxn” and compact block net messages, the current indexes sync process (all of them), getblock/gettxoutproof RPC commands. They all are locking the node at every single block read.
>
> This has been the case for 10+ years, if we're gonna change it then we should take the time to do it without adding to our technical
...

💬 MarcoFalke commented on issue "Building `--with-experimental-kernel-lib` fails on OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/27242#issuecomment-1496152826)
Did you try with a different compiler?

👋 jonatack's pull request is ready for review: "net, refactor: extract Network and BIP155Network logic to node/network"
(https://github.com/bitcoin/bitcoin/pull/27385)

💬 brunoerg commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1157420672)
Why not deal with `MaybeFlipIPv6toCJDNS` into `LookupHost`?

💬 jonatack commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1496190775)
> Before I had created a flag `-logasmap` (similar to `-logips`), now I removed it

Thanks! It looks like `logasmap` is still added in the last push, though.

💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1157435639)
I modified the text a bit, and also updated the PR description.

💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1157439737)
We don't normalize the private key version of descriptors in general. I'm not sure if it matters a lot, but would prefer to keep such a change to followup.

💬 Xekyo commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157441154)
I’m confused, @aureleoules’s suggestion makes sense to me. If it’s the maximum allowed transaction weight, why is it reduced by the `tx_noinputs_size`? If it’s supposed to be the allowance for inputs, it might additionally need to be reduced by an allowance for the change output?

💬 jonatack commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#discussion_r1157437268)
```suggestion
steps, for signing transactions, or for dumping wallet descriptors
```