💬 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
...

💬 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_r1157485963)
The bigger break was actually in the wallet dump format. I've added handling to keep `hdkeypath` the same for legacy wallets.

💬 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_r1157488214)
Dropped.

💬 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_r1157488656)
> it's the maximum allowed transaction weight, not only for the inputs.
I don't think so? The coin selection algos don't have any knowledge about the additional data of the to-be-created tx (static header size + outputs), so they can only work with a weight limit on the inputs.

oops, I shouldn't answer comments at night. My bad.
it's ok to say that is the max inputs weight. Will rename the variable in the next push.

> If it’s supposed to be the allowance for inputs, it might additionally
...

💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1496278211)
Biggest change is that the `hdkeypath` field in `getaddressinfo` keep using `'` for legacy wallets. The legacy backup (import) dump format also uses `'`.

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1496283614)
Rebased!

💬 sdaftuar commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1496286578)
I think in the interests of moving the package relay and v3 project forward, it makes sense to adopt this approach and defer potential improvements like #27018 to the future. My thinking is that this PR is a conservative way forward that seems obviously safe; our eviction logic on the other hand has a number of shortcomings, which I think will be best to fix up first before making more changes to use the eviction logic in new scenarios. We can work on the mempool and eviction in parallel and (ho
...

💬 MarcoFalke commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1157536088)
Can you explain this change?

💬 MarcoFalke commented on pull request "ci: use clang-16 in tidy task":
(https://github.com/bitcoin/bitcoin/pull/27404#issuecomment-1496322016)
> This error can be fixed with the following diff:

I wonder if another workaround would be to use libc++, but I haven't checked if the iwyu output is better or worse with libc++.

💬 brunoerg commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1496323887)
> It looks like logasmap is still added in the last push, though.

Sorry it was a leftover, fixed it.

💬 MarcoFalke commented on pull request "test: Remove windows workaround in authproxy":
(https://github.com/bitcoin/bitcoin/pull/27418#issuecomment-1496324726)
re-ran the windows test 12 times with only one failure: https://cirrus-ci.com/task/6093556574584832 . Which looks like an unrelated issue.

📝 theuni opened a pull request: "build: remove ancient unused define"
(https://github.com/bitcoin/bitcoin/pull/27420)
The generic define was removed in [upstream miniupnpc in 2014](https://github.com/miniupnp/miniupnp/commit/f6774e33169b3101c3a242984510c9b6da033e26).

Noticed while reviewing @hebasto's new CMake buildsystem: https://github.com/hebasto/bitcoin/pull/12#discussion_r1156267350.