📝 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
```

💬 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-1496245186)
> I never said the opposite. Not sure why the "rush" term is being used here really. I haven't said anything like "this is great, we must have it now".. (nor here nor anywhere in the repository).

I'm sorry didn't mean to put words in your mouth. It seemed rushed to me because you are going with an "easier" solution here rather than the more complex but (imo) worth while long term design.

> it will need blockstorage dependency to use the thread analysis annotation

I was thinking of not a
...