💬 fjahr commented on pull request "p2p: For assumeutxo, download snapshot chain before background chain":
(https://github.com/bitcoin/bitcoin/pull/29519#issuecomment-2141014859)
@ryanofsky would be really great to get your review here when you get the chance :)

✅ achow101 closed a pull request: "Fix typos in 36 files | Almost only documentation"
(https://github.com/bitcoin/bitcoin/pull/30188)

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1621719893)
I am somewhat ~0 on erroring in the case of 8manual/0in/0out, but my bigger anxiety with the latest commit is that it makes it confusing again (or too complicated in my judgement). I tried to code a simple way to account for the automatic outbounds but gave up. Also, most naturally, as the "minimum required" name suggests, the check should be `if (... < min_required)`. I find `if (... < min_required + something more)` confusing.

💬 vasild commented on pull request "net: log connections failures via SOCKS5 with less severity":
(https://github.com/bitcoin/bitcoin/pull/30064#issuecomment-2141242771)
`7524aaf58f...f3cfbd65f5`: drop the changes to `LogConnectFailure()`. They were somewhat related, but not directly to the SOCKS5 stuff.

> perhaps look if other ones there could be done here

The changes from #25203 look sound, but I want to keep this PR small, just to address the noisy and inconsistent logging due to connection failures via the SOCKS5 proxy.

💬 vasild commented on pull request "net: log connections failures via SOCKS5 with less severity":
(https://github.com/bitcoin/bitcoin/pull/30064#discussion_r1621727838)
Dropped this hunk altogether.

💬 maflcko commented on pull request "depends: qt 5.15.14 and fix macOS build with Clang 18":
(https://github.com/bitcoin/bitcoin/pull/30198#issuecomment-2141246562)
Tested on:

```
# clang --version && file src/qt/bitcoin-qt
Ubuntu clang version 18.1.3 (1)
Target: riscv64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
src/qt/bitcoin-qt: Mach-O 64-bit arm64 executable, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL|WEAK_DEFINES|BINDS_TO_WEAK|PIE|HAS_TLV_DESCRIPTORS>
```

```
# clang --version && file src/qt/bitcoin-qt
Ubuntu clang version 18.1.3 (1)
Target: powerpc64le-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
src/qt/bit
...

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621795298)
Thanks for the confirmation, appreciate it!

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621797908)
> To my knowledge, weight is always measured in weight units, so I don't think adding the _wu suffix here would add any new information.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621812773)
Agreed. However if it had not been for the absolute fee, we could come up with a more generic function `get_effective_unspent_value` that calculates based on the `fee_rate` and the `vsize` of the unspent.

Can't include the absolute fee in this because it is paid for the whole transaction and not just one unspent - still valid for transactions spending only one unspent.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621826033)
I see, nice!

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621826528)
I can pick it up in a follow-up.

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1621830415)
Actually, I can't reproduce it so far. So a corrupt CI machine is more likely.

💬 rkrux commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1621836458)
IMO, this 3 deserves a constant soon because it has started showing up in a comment in the functional test as well now besides being in few other places too in this diff. Otherwise, the mind of the code reader wanders off to figuring out where this 3 comes from.

👍 rkrux approved a pull request: "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)"
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2090023300)
tACK [39d135e](https://github.com/bitcoin/bitcoin/pull/30162/commits/39d135e79f3f0c40dfd8fad2c53723d533cd19b4)

@theStack Thanks for addressing the comments and answering my questions!

📝 zoupingshi opened a pull request: "chore: fix some comments"
(https://github.com/bitcoin/bitcoin/pull/30208)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 maflcko commented on pull request "chore: fix some comments":
(https://github.com/bitcoin/bitcoin/pull/30208#issuecomment-2141386493)
You'll have to submit them upstream, see the CI failure (lint)

✅ maflcko closed a pull request: "chore: fix some comments"
(https://github.com/bitcoin/bitcoin/pull/30208)

⚠️ Sjors opened an issue: "Make Transport independent of CNetMessage and CSerializedNetMsg"
(https://github.com/bitcoin/bitcoin/issues/30209)
Currently `class Transport` uses `CNetMessage` in one place:

https://github.com/bitcoin/bitcoin/blob/62f7f59ff495fbcbfc10c25e97bb0dc032647abf/src/net.h#L278-L285

And `CSerializedNetMsg` here:

https://github.com/bitcoin/bitcoin/blob/62f7f59ff495fbcbfc10c25e97bb0dc032647abf/src/net.h#L287-L295

This creates a problem in my Stratum v2 Template Provider implementation in #28983 which introduces `class Sv2Transport : Transport`. Because the sv2 noise protocol (#29346) is very similar to BI
...

💬 fanquake commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#discussion_r1621965700)
Pushed that now.

⚠️ fanquake opened an issue: "build: use UCRT runtime for Windows (release) binaries"
(https://github.com/bitcoin/bitcoin/issues/30210)
Switching to the modern runtime would be good, because the old runtime is missing features, which has meant writing workarounds for Windows in our code: i.e #29014/#29357.

[Mingw-w64 12.0.0 has been released](https://sourceforge.net/p/mingw-w64/mailman/message/58776404/), which now defaults to the `UCRT` runtime, over `MSVCRT`. See https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-doc/howto-build/ucrt-vs-msvcrt.txt for more details. This makes using it somewhat easier, because we c
...