💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804511824)
This is a great point. Instead of the type-pun, we'd likely want to make this safer by assigning the result to a `uint64_t` then memcpy it in instead. That likely isn't going to be significantly slower.

⚠️ maflcko opened an issue: "How to compile the GUI on opensuse tumbleweed with cmake?"
(https://github.com/bitcoin-core/gui/issues/842)
Looks like this broke after the cmake migration.

Steps to reproduce on a fresh `podman run -it --rm 'registry.opensuse.org/opensuse/tumbleweed:latest'`:

`zypper in -y awk qrencode libevent-devel boost-devel sqlite3-devel libqt5-qttools-devel libqt5-qtbase-devel libdb-4_8-devel find bison gcc-c++ libtool make autoconf automake python3 clang llvm lbzip2 patch xz curl wget htop git vim ccache && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && cmake -
...

💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804519365)
> Edit: Wait, this code is being removed. Sorry.

Yeah, sorry, I was just trying to come up with another reason to change the code here :sweat_smile:

⚠️ maflcko opened an issue: "CI failure: `Error: Directory '/tmp/ccache_dir' must be created in advance.`"
(https://github.com/bitcoin/bitcoin/issues/31108)
This may happen when re-running the CI (which happens every week on every pull request).

Unfortunately there is no fix for this, other than a new push (normal push, force-push, rebase, ...), or to ignore the failure.

This happens after commit fa252da0b9cc6c7e795366ce4a1ddc4c198dff15, which modified the cirrus config.

💬 l0rinc commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804545063)
Do you have any concrete recommendation that I could apply here?

💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804599437)
Sorry again, I am just trying to add useful context that hasn't been mentioned before. I just have an allergy against undefined behavior, so I wanted to mention that the previous code may have had one.

As a fun fact, I tried it myself and in theory the UB should look like:

```
src/bench/crypto_hash.cpp:198:9: runtime error: store to misaligned address 0x7fffdb31cb81 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
0x7fffdb31cb81: note: pointer points here
00 0
...

💬 l0rinc commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804609063)
Thank you for checking!

💬 dergoegge commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2419332156)
> -par=1 (or whatever to make the thread inline) should work, no?

That worked, thanks!

💬 dergoegge commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#discussion_r1804632137)
Will do if i push again

👍 0xB10C approved a pull request: "rpc: net: follow-ups for #30062"
(https://github.com/bitcoin/bitcoin/pull/30183#pullrequestreview-2375071825)
Code Review ACK b33eb137e39c434a7be69e1453a708b0c52553c4

💬 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