💬 0xB10C commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1804687699)
I agree that the wording isn't perfect, but I still think this can be merged as is.

👍 laanwj approved a pull request: "tracing: explicitly cast block_connected duration to nanoseconds"
(https://github.com/bitcoin/bitcoin/pull/29877#pullrequestreview-2375109977)
re-ACK cd0edf26c07c8c615f3ae3ac040c4774dcc8e650

💬 brunoerg commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1804741197)
In 0c5bba5313f6eff05676d25c9dca1b73c5a0c95b: I think we could limit the size of `random_string` based on its max possible size.

💬 l0rinc commented on pull request "test: Fuzz the human-readable part of bech32 as well":
(https://github.com/bitcoin/bitcoin/pull/30623#discussion_r1804754220)
Aren't we introducing bias that way? Can you point me to a BIP or any doc that defines these max values (I don't know about any, that's why I'm asking)?

💬 TheBlueMatt commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2419520283)
> It's literally in https://github.com/Sjors/bitcoin/pull/68, split between https://github.com/Sjors/bitcoin/pull/66, https://github.com/Sjors/bitcoin/pull/67, https://github.com/Sjors/bitcoin/pull/50 and https://github.com/Sjors/bitcoin/pull/49 in that order. I've already refactored that code to use the interface, just not via IPC, so it's all additional. The first three PRs don't use the interface at all.

I guess I was more curious what you think the bulk of the issue is, rather than just t
...

⚠️ TheBlueMatt opened an issue: "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants"
(https://github.com/bitcoin/bitcoin/issues/31109)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Finally getting around to reviewing the mining interface, and sadly its missing some critical features that a new mining protocol should have. Specifically, one of the key goals of replacing `getblocktemplate` is that Bitcoin Core should be able to push work to clients at opportune times. This includes the ability to create a new block in between validating a new block and updating the mem
...

💬 TheBlueMatt commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1804784541)
Shouldn't this return a blocktemplate instead? Requiring a second round-trip after getting a notification is just unnecessary latency.

💬 TheBlueMatt commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1804793319)
It'd be nice if `bitcoin.conf` could specify some options to restrict the fee threshold/timeout to some window. That way (once we drop CNB from the interface, see #31109) the protocol is at least kinda robust against trivial DoS attacks, which would be nice for teeing it up to be served over TCP in the future (or the inevitable exposing it over netcat that'll happen).

🤔 darosior reviewed a pull request: "validation: Improve input script check error reporting"
(https://github.com/bitcoin/bitcoin/pull/31097#pullrequestreview-2375362753)
re-ACK 86e2a6b749c7fecbd086b361806ac9f6e9426d79

💬 jonatack commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1804904158)
"at the end of the BGP route to the peer" in all of them (including -netinfo as well) seems good

💬 danielabrozzoni commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2419741359)
> Worth adding a release note?

Thank you! Done in 6b32a886e351f0af3084ccab076a52e0eafc5aab

@stickies-v:

> It seems electrs already uses the authentication cookie by default (as per their docs), so for most setups, this shouldn't be a meaningful difference?

I agree that adding this endpoint won't make much difference in the short term, but I still believe that in the long term, if both existing and new indexers adopt the REST interface, it will improve UX and security.

> It seems t
...

🚀 fanquake merged a pull request: "tracing: explicitly cast block_connected duration to nanoseconds"
(https://github.com/bitcoin/bitcoin/pull/29877)

💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1805079799)
This fails ubsan:


```
src/bench/crypto_hash.cpp:202:9: runtime error: implicit conversion from type 'int' of value 256 (32-bit, signed) to type 'unsigned char' changed the value to 0 (8-bit, unsigned)
#0 0x55b763589f3f in SipHash_32b(ankerl::nanobench::Bench&)::$_0::operator()() const bld-cmake/src/bench/./src/bench/crypto_hash.cpp:202:9


...

SUMMARY: UndefinedBehaviorSanitizer: implicit-signed-integer-truncation /home/marco/workspace/b-c/src/bench/crypto_hash.cpp:202:9
```

...

🤔 BrandonOdiwuor reviewed a pull request: "init: Some small chainstate load improvements"
(https://github.com/bitcoin/bitcoin/pull/31046#pullrequestreview-2376140194)
Code Review ACK 31cc5006c3de4dd6a1f7a238684163956604df45.

Nice work on the chainstate loading improvements, especially the enhanced handling of database-specific errors.

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1805421857)
Updated to take @vasild's proposal. I believe it's the best path of the 3 options discussed above.

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1805423837)
Didn't take, as those braces are unneeded due to the low precedence of the ternary operator, clang-format doesn't prefer them, and they add clutter. Will add if people insist.

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#issuecomment-2420560896)
> I think you also should update line 93 (change "4" to "5")
>
> ```
> argsman.AddArg("-netinfo", "Get network peer connection information from the remote server. An optional integer argument from 0 to 4 can be passed for different peers listings (default: 0). Pass \"help\" for detailed help documentation.", ArgsManager::ALLOW_ANY, OptionsCategory::CLI_COMMANDS);
> ```

Good catch! added 153b898a77f4c892731c220ce85d29c1cf880355 to ensure we don't overlook updating this.

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#issuecomment-2420562022)
> I think you also should update line 93 (change "4" to "5")
>
> ```
> argsman.AddArg("-netinfo", "Get network peer connection information from the remote server. An optional integer argument from 0 to 4 can be passed for different peers listings (default: 0). Pass \"help\" for detailed help documentation.", ArgsManager::ALLOW_ANY, OptionsCategory::CLI_COMMANDS);
> ```

Good catch @LarryRuane -- added 153b898a77f4c892731c220ce85d29c1cf880355 to ensure we don't overlook updating this.

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#issuecomment-2420580314)
Updated per `git diff 683b558 1847b9` to take @vasild's [suggestion](https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1795748339) and a fix for @LarryRuane's [catch](https://github.com/bitcoin/bitcoin/pull/30930#pullrequestreview-2363290423) (thanks!)

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#issuecomment-2420629857)
Updated to take @vasild's [suggestion](https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1795748339) and a fix for @LarryRuane's [catch](https://github.com/bitcoin/bitcoin/pull/30930#pullrequestreview-2363290423) (thanks!) and doc updates I overlooked.

<details><summary><code>git diff 683b558 17f8f03</code></summary><p>

```diff
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 000f7695de6..6bda4cbe0c5 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -57
...