💬 Sjors commented on issue "Support validating a PoW-free block over over RPC":
(https://github.com/bitcoin/bitcoin/issues/31136#issuecomment-2642228270)
#31564 adds this to the Mining interface. It's not needed for the Stratum v2 Template Provider, so lower priority for me.

Additionally it introduces a `checkblock` RPC, which seems easier to use and easier to expand later.

Finally it can verify weak block proof-of-work.

💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#discussion_r1946163054)
Actually it's not needed. This code is for the proposal mode, where we verify a block given to us. It's not _our_ node error if this block is invalid.

When we generate a block template ourselves through `getblocktemplate` or `createNewBlock()`, it goes through `BlockAssembler::CreateNewBlock()` which by default runs `TestBlockValidity` at the end, and throws if this return an error. No need to log if we're already crashing with `std::runtime_error`.

💬 l0rinc commented on pull request "doc: update translation generation cmake example":
(https://github.com/bitcoin/bitcoin/pull/31731#discussion_r1946169077)
This PR was meant to fix the example only, I don't have enough context to update the docs as well.

💬 pythcoiner commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#discussion_r1946211743)
done

💬 pythcoiner commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#discussion_r1946213790)
my bad, my clangd add it automatically and I don't notice.
I drop it as it seems redundant

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946227366)
Changed to `LogError()` and removed the newline. I did not know the trailing newlines are now unnecessary (since bbbb2e43ee95c9a8866aa1f65e3f001f752dfed2).

💬 dergoegge commented on pull request "ci: Bump fuzz task timeout":
(https://github.com/bitcoin/bitcoin/pull/31814#issuecomment-2642410797)
ACK faca7ac13215fd88b420feea8f06d7404f8fd067

💬 dergoegge commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2642418075)
> corecheck is back up and integrated into the DrahtBot comment, so is anything left to be done here?

I think the only thing missing is an alert system or even just a better overview over all benchmarks? Seems difficult to spot regressions in here manually: https://corecheck.dev/benchmarks

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946240012)
Done.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946244171)
Done.

💬 pythcoiner commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#issuecomment-2642428740)
tahnks for your time @hodlinator!

🚀 fanquake merged a pull request: "ci: Bump fuzz task timeout"
(https://github.com/bitcoin/bitcoin/pull/31814)

💬 hodlinator commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946265398)
:facepalm: makes total sense, I was thinking about the remote sockets.

💬 hodlinator commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946267787)
Yeah, sorry, me confusing sockets again. An `Assume()` would be nice though.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946271517)
Done.

💬 maflcko commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2642488461)
Good point. I guess there could be a comment (or the summary comment could be edited) to encourage people to check the result link of a pull request if the difference is larger than some percent.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946284407)
Removed `struct` from `BindAndStartListening()` in commit `style: modernize the style of SockMan::BindListenPort()`.

`GetBindAddress()` was just moved verbatim without any mods from `net.cpp` to `sockman.cpp` in another commit. This makes it easier to review with
```
[diff]
colorMoved = dimmed-zebra
colorMovedWS = allow-indentation-change
```
in `~/.gitconfig`. Will leave it as it is. Don't want to bloat this PR further with one more "style changing" commit.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946299108)
In general, less indentation makes the code more readable, so I prefer to have `return;` and then the subsequent code to be with less indentation.

Anyway this code was changed and now uses `switch` (see `CConnman::EventI2PStatus()` after I push).

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1946303871)
```
error: delete called on non-final 'CConnman' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non-abstract-non-virtual-dtor]
```

💬 fanquake commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2642520881)
> Submitted patch in https://github.com/capnproto/capnproto/pull/2235

Thanks. Pulled that into #29796.