💬 josibake commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2110464755)
> Yes, but those are manual constraints that you are writing by hand rather than automatic constraints expressed implicitly in the data definition

Fair point, in that bugs could be introduced by someone not writing these pre-checks correctly / efficiently. I'll admit I'm not fully convinced that the struct per state approach isn't going to be harder to maintain / extend in the future, but given that I haven't convinced you guys on that point and you have convinced me there is an advantage per
...

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1600207532)
Thanks so much for the patch. I had to move this line back up to where it was, otherwise the test fails because after pushing the error into the batch reply, we would also then push a weird extra junk reply with the current id but the result from the last successful query

```diff
diff --git a/src/httprpc.cpp b/src/httprpc.cpp
index 0e3e66c515..777ad32bbe 100644
--- a/src/httprpc.cpp
+++ b/src/httprpc.cpp
@@ -238,12 +238,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPReque
...

🤔 hebasto reviewed a pull request: "util: avoid using thread_local variable that has a destructor"
(https://github.com/bitcoin/bitcoin/pull/30095#pullrequestreview-2055708177)
Approach ACK c5f9afd946efb4eb6b27b2d0316f2f787a9608c7.

💬 hebasto commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1600218912)
Maybe add a comment to prevent future attempts to reintroduce `std::string`?

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1600219420)
Took a bit longer than I expected. I've pushed to the [dev branch](https://github.com/setavenger/blindbit-oracle/tree/dev). It seems to be working as intended. No cut-through and you can choose any value you like for a dustLimit. The request looks like this `"localhost:8000/tweak-index/840000?dustLimit=546"` (`tweak-index` then only returns tweaks where the highest value is `>= 546`)

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1600225849)
I'm trying to figure out how to write a test to catch one of these `RPC_MISC_ERROR` since they obviously weren't catching a `RPC_PARSE_ERROR` before adding your patch to the commit. If I understand correctly, that "misc" error would only throw if there was a bug in an RPC command because most of our try blocks try to catch a UniValue error first with a specific code.

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1600233020)
(since @sr-gi mentioned this offline) Yes, the last push turned this 1 line into 2 identical lines. The purpose is to illustrate to current reviewers and future readers more clearly... if wtxid, we do this. if txid, we also do this, but notice that we are actually doing a "cast" and that's ok because we are *guessing* what the wtxid is.

Also, perhaps these will diverge again in the future if `GenTxid` becomes a `std::variant<Txid, Wtxid>`.

🚀 ryanofsky merged a pull request: "kernel: Remove batchpriority from kernel library"
(https://github.com/bitcoin/bitcoin/pull/30083)

🤔 edilmedeiros reviewed a pull request: "Extend signetchallenge to set target block spacing"
(https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-2055713496)
Concept ACK.

I prefer the approach in #27446 but I can't say this would not be the bitcoin style of using the existing script machinery to implement so a logic.

I would love to see an accompanying BIP to upgrade BIP 325, since the signet behavior is standardized there. In practice, this change in core will potentially make this implementation an "undocumented standard".

I personally would prefer the BIP discussion to happen before merging, specially to define which params would be incl
...

💬 edilmedeiros commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1600222312)
I don't feel like changing this test is a good idea since the proposed change is supposed to be backward-compatible. Better would be to add an additional functional test just for the purpose of testing the new behavior while keeping the old functional test as a guard rail.

👍 hebasto approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2055846816)
re-ACK b58156701ac132f87b8ef8da1c7d22158c804a81, only suggested changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2055510319).

🤔 ryanofsky reviewed a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2055809906)
Thanks for the review! Made suggested changes

Updated 850c73e250fe0b248cae80d26561da7047696e3d -> b58156701ac132f87b8ef8da1c7d22158c804a81 ([`pr/rmutil.16`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.16) -> [`pr/rmutil.17`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.17), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.16..pr/rmutil.17))
Updated 850c73e250fe0b248cae80d26561da7047696e3d -> b58156701ac132f87b8ef8da1c7d22158c804a81 ([`pr/rmutil.16`](https
...

💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600281482)
re: https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600120289

> nit: Sort these lines?

Fixed, thanks. (I actually did sort them, but it seems sort command treats underscore and space characters the same on my system so the wallet and wallet_util dependencies were mixed up)

💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600281049)
re: https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600117347

> Duplicated lines?

Thanks, cleaned up

💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600281239)
re: https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600118750

Good catch, added

💬 AngusP commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2110638042)
ACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22

💬 AngusP commented on pull request "test: Refactor fee/feerate calculations in feature_fee_estimation.py to use Decimal instead of float":
(https://github.com/bitcoin/bitcoin/pull/29566#issuecomment-2110643263)
ACK aa46f0ec82c65fc8bb67c544c6b16296940d96dd

👍 hebasto approved a pull request: "ci: Roll clang in test-each-commit task"
(https://github.com/bitcoin/bitcoin/pull/30060#pullrequestreview-2055885000)
ACK 2094da680fe580ba1bc62e9bab1e196f5e1c5341.

> This fails, because the image does not exist yet.

I've re-run the CI job.

💬 TomBriar commented on pull request "Compressed Bitcoin Transactions":
(https://github.com/bitcoin/bitcoin/pull/29134#issuecomment-2110656305)
Hi,

This is a reference implementation for the compression scheme described in this WIP BIP:
https://github.com/bitcoin/bips/pull/1556

It may be a bit heavy to be PR'd into bitcoin core at this time, although it has a great application for both the P2P and Storage in bitcoin core. Another project that implements the same schema is underway as an external tool and the RPC commands were found to be sufficient.

Thanks-
Tom.

📝 hebasto opened a pull request: "build: Enable `thread_local` for MinGW-w64 builds"
(https://github.com/bitcoin/bitcoin/pull/30099)
The previously mentioned erroneous behavior is not an issue anymore (I used [this](https://gist.github.com/jamesob/fe9a872051a88b2025b1aa37bfa98605) test case on platforms that support C++20).

Guix build:
```
a6bb81780d46656c825f1c25fc4d49e7f567ac07f77c0ca1d86682e0d5e3ddda guix-build-df879e5a9113/output/dist-archive/bitcoin-df879e5a9113.tar.gz
d2c5b11be468b6da9ce4590ed04bcd862e05148a35eb116488f728bce170eeff guix-build-df879e5a9113/output/x86_64-w64-mingw32/SHA256SUMS.part
394ef4e2a064ff
...