💬 achow101 commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#issuecomment-2289158872)
ACK 6ed424f2db609f9f39ec1d1da2077c7616f3a0c2

💬 TheCharlatan commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#issuecomment-2289173555)
Concept ACK

🚀 achow101 merged a pull request: "doc: Deduplicate list of possible chain strings in RPC help texts"
(https://github.com/bitcoin/bitcoin/pull/30648)

💬 TheCharlatan commented on pull request "coins: Simplify std::move to ternary in `coins.cpp`":
(https://github.com/bitcoin/bitcoin/pull/30656#discussion_r1717208536)
I don't understand the reasoning to be honest. This seems to be just a cosmetic refactor, and I am not sure if it is really clearer, or if this was desired by other reviewers. The commit message also makes no mention of why the move constructors are now defaulted, or why that is required.

🚀 achow101 merged a pull request: "validation: assumeutxo params mainnet"
(https://github.com/bitcoin/bitcoin/pull/28553)

💬 MarnixCroes commented on pull request "Migrate legacy wallets that are not loaded":
(https://github.com/bitcoin-core/gui/pull/824#discussion_r1717223832)
nit: maybe a more descriptive text would be good?
something like `No wallets that need to be migrated`

for people who don't know about the migration and why/what it is.
seeing _No wallets available_ while they have wallet(s) might seem wrong/confusing.

sry for commenting post-merge

🤔 pablomartin4btc reviewed a pull request: "validation: assumeutxo params mainnet"
(https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-2238694990)
tACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410

Checksum matches downloaded file and performed `loadtxoutset` on a new node.

I'm still performing some other tests.

💬 ryanofsky commented on pull request "test: [refactor] Use g_rng/m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#issuecomment-2289335140)
Code review ACK facc85b766dec2481384c878ccbf1696a8386e10. But I think we can eliminate the global now, and I think would be good to do it in this PR to changing affected tests more than once. I posted a branch https://github.com/ryanofsky/bitcoin/commits/pr/rng which I hope you can steal. It has the following commits:

- a334a9b9bd0d8ee520f9e2769534625a5d147705 test: Add m_rng alias for the global random context
- c38390fb43e40913afa2072c7986ea80da1d4d38 test: refactor: Make unsigned promotio
...

💬 achow101 commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#issuecomment-2289394756)
Updated for testnet4.

Also refreshed the seeds for pre-28.0.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717020191)
Responding to https://github.com/bitcoin/bitcoin/pull/30377/files#r1716616576:
Enough casing changes for one PR. :)

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716886175)
Responding to https://github.com/bitcoin/bitcoin/pull/30377/files#r1716693711:
@maflcko: Might introduce `VecFromHex` for tests but I like that this constant is fully compile time.

@paplorinc: I have been trying out exactly that approach quite a bit, but was [warned by maflcko](https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1692989490) that there may be lying dragons among the different serializations of `vector`/`array`/`span`/`Span`. Also had some annoying MSVC compiler errors w
...

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716895696)
Responding to https://github.com/bitcoin/bitcoin/pull/30377/files#r1716610850:
I think it's more rigorous/strict to not use `ParseHex` on both left & right sides of the comparison.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716890935)
Clang and GCC don't, but MSVC does. :(

Removed in this function in latest push.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716887365)
Responding to https://github.com/bitcoin/bitcoin/pull/30377/files#r1716685019:
I'll meet you half-way with `constexpr auto`. :)

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716903532)
Responding to https://github.com/bitcoin/bitcoin/pull/30377/files#r1716703571:
Tried to start switching to `std::byte` after your comment, but it quickly propagated to untouched files.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717015082)
Attempted to minimize the diff in latest push, but went with `VecFromHex` for string literals and introduced a new `ScriptFromHexStr` for the 2 runtime validated call sites.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716875704)
Responding to https://github.com/bitcoin/bitcoin/pull/30377/files#r1716678159:

Thanks, better get this part right. :)

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716900164)
Responding to https://github.com/bitcoin/bitcoin/pull/30377/files#r1716752262:

Weird, actually works for me on same clang version as @stickies-v, but under Linux:
```
$ clang --version
clang version 17.0.6
Target: x86_64-unknown-linux-gnu
```
Maybe it's the standard library that is somehow different (missing some `constexpr`)?

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717012368)
> `ScriptFromHex` failed for empty inputs.

How come? `ParseHex("")` returns `0` as verified by **util_tests.cpp**. Besides that, I think it's not really what we are testing in this file.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717314517)
Taken in latest push.