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

💬 1440000bytes commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2419090597)
> One thing to consider is that this PR changes the REST interface from being read-only to also being able to "modify" node state (mempool in this case). This PR might introduce new considerations for someone exposing the REST interface to a local or even a public network: Someone who previously could only query information can now also publish transactions via the node.

- It improves privacy if a node broadcasts some transactions on behalf of others
- It is possible to modify the node state
...

👍 maflcko approved a pull request: "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues"
(https://github.com/bitcoin/bitcoin/pull/30349#pullrequestreview-2374747440)
lgtm ACK c49cc30b8185a50b4aa00cf705d313c8aa9482a3

💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804480124)
Maybe I am wrong, but in theory I think that a byte array has weaker memory alignment requirements than `uint64_t`. So this is (in a strict sense) undefined behavior anyway. I presume it could be tested by adding a padding byte to uint256 and compile with ubsan, and then run this bench to observe the failure.

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