💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1729052874)
@sipa I psyop'd myself into thinking they were connected... somehow.. Well, I believe your example!

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1729049664)
Might do if I re-touch.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1729053334)
Yeah, my goal was to contrast it against `ParseHex` returning a vector. Will do if I re-touch.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1729064946)
I want to explicitly enforce compile time where possible, hence the `consteval`.

In the `""_hex_v_u8` case, `constexpr` would be a lie, as `UCharCast` being called is `inline`, not `constexpr`, and calls `reinterpret_cast` which is not supported at compile time. (This means that `UCharSpanCast` and `MakeUCharSpan` are `constexpr` in name only, except for if called on a collection that is already `unsigned char` and doesn't require `reinterpret_cast`).

💬 martinsaposnic commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#issuecomment-2307233648)
Out of curiosity I ran a little benchmark

Master:

<img width="607" alt="image" src="https://github.com/user-attachments/assets/875f96fd-5ef2-4730-a3df-f7b3ff40f039">


With MiniWallet:

<img width="903" alt="image" src="https://github.com/user-attachments/assets/40bdcb95-1b08-4054-9223-44566bdca548">

Miniwallet is cool

💬 martinsaposnic commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#issuecomment-2307235414)
@theStack I implemented the 3 suggestions. Let me know if you have further comments 🙂

💬 virtu commented on issue "DNS seed "seed.bitcoinstats.com" doesn't support filtering while the comments says it does":
(https://github.com/bitcoin/bitcoin/issues/29911#issuecomment-2307236913)
Only bumping this because there's a good chance this seed will serve zero addresses by release time.

Seeder been stuck [since January](https://21.ninja/dns-seeds/advertised-count/) and has deteriorated from 37 down to currently 5 addresses:
```bash
» for addr in seed.bitcoinstats.com x9.seed.bitcoinstats.com; do echo -n "unique addresses from $addr: " && host $addr | grep address | awk '{print $NF}' | sort -u | wc -l ; done

unique addresses from seed.bitcoinstats.com: 5
unique addresses
...

🚀 fanquake merged a pull request: "[27.x] Even more backports"
(https://github.com/bitcoin/bitcoin/pull/30558)

💬 furszy commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#discussion_r1729102053)
> In commit "init: fix fatal error on '-wallet' negated option value" ([e56e2f5](https://github.com/bitcoin/bitcoin/commit/e56e2f51803b276945c74a479d30d884256d7ffc))
>
> If desired, you could include the value in the error message like `strprintf(_("Invalid -wallet value %s detected"), wallet.write())`

Sadly, that wouldn't help much and could end up confusing users. The error output for `-nowallet=not_a_boolean` would be `-wallet=true`, which does not correspond to what the user inputted.

💬 furszy commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#issuecomment-2307260467)
> Could update the description and note that this bug was caused by https://github.com/bitcoin/bitcoin/pull/22217.

Sure. Done!.

> I think this error could also be triggered by editing settings.json and adding a non-string wallet name.

Yes, nice. Added coverage for it too.

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1729027931)
nit: could be made a bit more readable by using `auto` and implicitly constructing `Hex`:

<details>
<summary>git diff on df92661444</summary>

```diff
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 644c8ffc72..050aa2a5a9 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -154,15 +154,14 @@ BOOST_AUTO_TEST_CASE(parse_hex)

// Basic test vector
std::vector<unsigned char> expected(std::begin(HEX_PARSE_OUTPUT), std::end(HEX_PARSE_OUTP
...

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1728787408)
Is there a specific reason to put `Hex` in the `detail` namespace? I think it could be useful to have `consteval` functions take a `Hex` parameter. For example, the `base_blob(string_view)` ctor can be quite elegantly deduplicated:

<details>
<summary>git diff on df92661444</summary>

```diff
diff --git a/src/uint256.h b/src/uint256.h
index 4124a34719..3616b0f9cb 100644
--- a/src/uint256.h
+++ b/src/uint256.h
@@ -16,6 +16,7 @@
#include <cstdint>
#include <cstring>
#include <optio
...

👍 stickies-v approved a pull request: "refactor: Replace ParseHex with consteval ""_hex literals"
(https://github.com/bitcoin/bitcoin/pull/30377#pullrequestreview-2256959862)
ACK df92661444f46790b12d5061344d72106ef820d9 .

The new ""_hex literals approach makes for a much more ergonomic interface that is concise, clear and easy to use. Since the ""_hex operators are templated on the string literal, It does generate a lot of template instantiations, but since these are all consteval, I believe these should not end up in the binary, so that seems like a reasonable trade-off.

Left a few nits and a possible approach to further clean up the `base_blob::base_blob(str
...

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1729071473)
nit in 6954b1f1143a73d4024f5564e287d8eeb34ff577: no more need for `MakeByteSpan`:

```suggestion
hw.write(G_uncompressed);
```

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1729082336)
Since we're templating these on `Hex` instances, this string literal approach will produce quite a rather large number of template instantiations. I wonder if keeping these `consteval` would also better allow the compiler to make sure these instantiations don't end up in the binary?

👍 fanquake approved a pull request: "test: Avoid duplicate curl call in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/30703#pullrequestreview-2257475032)
ACK fa5aeab3cb18405ecf8a1401d89539b924a618f6

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2307287556)
> Find1P1CPackage doesn't actually check the normal reject filter at all with respect to the child, only reconsiderable, so that assertion gets hit.

I think it might be more that `MempoolRejectedTx` doesn't remember if a tx failed for a different reason before considering an orphan?

💬 l0rinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#issuecomment-2307287794)
While I'm not a fan of mixed casing, I agree that it's probably outside the scope of current change.

ACK 18d65d27726bf9fc7629b8e794047a10c9cf6156

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1729130184)
done

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1729130331)
done