💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723168808)
Started changing it based on your comment, but as there is already another if-return false above in the untouched code, I think it's more consistent to keep as-is.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723169146)
Thanks, taking that!

💬 paplorinc commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2298662545)
reACK c3fa29db1b05aa51f74cc8be5bdae59be4f3b7c0

https://github.com/bitcoin/bitcoin/compare/80a596ecca5df9f471d9fbcd9fcd15ddd296cdca..c3fa29db1b05aa51f74cc8be5bdae59be4f3b7c0:
* Removed many `<uint8_t>` inside `ToScript`
* out_exp -> string_view
* braces in wallet_crypto_tests.cpp and crypter.cpp
* remove_reference -> remove_reference_t

📝 brunoerg opened a pull request: "test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py`"
(https://github.com/bitcoin/bitcoin/pull/30683)
Sending a `verack` without `wtxidrelay`, regardless of `sendtxrcncl`, will trigger `Forget txreconciliation state of peer`. To ensure that we are testing that `SENDTXRCNCL` without `WTXIDRELAY` is ignored, we can first verify that the peer was registered.

💬 MummaStacks commented on issue "Running Bitcoin Bitcore on new Apple M3":
(https://github.com/bitcoin/bitcoin/issues/30680#issuecomment-2298710905)
thank you it does appear to be :) sorry for doubling up

💬 fanquake commented on issue "Control-flow application capabilities for `x86_64-linux-gnu` release binaries":
(https://github.com/bitcoin/bitcoin/issues/30677#issuecomment-2298711926)
> Should we adjust glibc [Hardware Capability Tunables](https://www.gnu.org/software/libc/manual/html_node/Hardware-Capability-Tunables.html)?

Why?

💬 brunoerg commented on pull request "test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py`":
(https://github.com/bitcoin/bitcoin/pull/30683#issuecomment-2298714161)
Can be tested with:
```diff
diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py
index 2c7216b5ca..1e0e82f076 100755
--- a/test/functional/p2p_sendtxrcncl.py
+++ b/test/functional/p2p_sendtxrcncl.py
@@ -218,7 +218,6 @@ class SendTxRcnclTest(BitcoinTestFramework):
self.log.info('SENDTXRCNCL without WTXIDRELAY is ignored (recon state is erased after VERACK)')
peer = self.nodes[0].add_p2p_connection(PeerNoVerack(wtxidrelay=False), send_versi
...

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2298715703)
re-ACK c3fa29db1b05aa51f74cc8be5bdae59be4f3b7c0

✅ MummaStacks closed an issue: "Running Bitcoin Bitcore on new Apple M3"
(https://github.com/bitcoin/bitcoin/issues/30680)

✅ fanquake closed an issue: "Unable to build Bitcoin using './contrib/guix/guix-build'"
(https://github.com/bitcoin/bitcoin/issues/30676)

💬 brunoerg commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1723269955)
In 9d84717c92ed052dea6f78c715833d328835ba79 "fuzz: Add harness for p2p headers sync": Is there any reason to use `resize` instead of simply doing:
```diff
diff --git a/src/test/fuzz/p2p_headers_sync.cpp b/src/test/fuzz/p2p_headers_sync.cpp
index 8de39431b8..f474263620 100644
--- a/src/test/fuzz/p2p_headers_sync.cpp
+++ b/src/test/fuzz/p2p_headers_sync.cpp
@@ -48,10 +48,11 @@ void HeadersSyncSetup::ResetAndInitialize()
connman.StopNodes();

NodeId id{0};
- std::vector<Conn
...

💬 hodlinator commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1723257479)

nit in cb9d4712aec195f1fc5b3bc0a46bc13b46477b85:
```suggestion
CCoinsViewTest(FastRandomContext& rng LIFETIMEBOUND) : m_rng{rng} {}
```

🤔 hodlinator reviewed a pull request: "test: [refactor] Use m_rng directly"
(https://github.com/bitcoin/bitcoin/pull/30571#pullrequestreview-2243672559)
Concept ACK 164732369fc5ecf4c7705136a2de884419023b5a

💬 hodlinator commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1720433250)
It would leave the code in a better state after this PR if the constructor wasn't made into this odd sidecar lambda. We already have `m_rng` available from prior commit so just pass it down.

Applies on top of and reverts part of ac4e757bc3633e01db973201e1633331c0815e3a "Give unit test functions access to test state":
```diff
diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp
index d2b28b9b18..ac4fe4404e 100644
--- a/src/test/cuckoocache_tests.cpp
+++ b/src/test/
...

👍 theStack approved a pull request: "test: check xor.dat creation when missing"
(https://github.com/bitcoin/bitcoin/pull/30669#pullrequestreview-2248062503)
ACK 7b7c287a33dc0c6f09bf32f31a8f05941b0cc9a2

Thanks for following up! If you want, you could also tackle the suggestion to move the `read_xor_key` helper to the `TestNode` class, see https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1717985952

💬 theStack commented on pull request "test: check xor.dat creation when missing":
(https://github.com/bitcoin/bitcoin/pull/30669#discussion_r1723350082)
nit, personally I prefer to be explicit about the zeroes initialization, but no big deal
```suggestion
NULL_BLK_XOR_KEY = bytes([0] * NUM_XOR_BYTES)
```

💬 theStack commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1723358327)
> nit: Any reason to make this a global function and pass a node to it? Seems clearer to just make it a member function on the node, like `blocks_path()`, no?

No strong reason. Personally I first look into util.py for helpers and tend to forget that there are also some implemented as `TestNode` methods, but I agree that the latter makes sense. Could be picked up in a follow-up, e.g. #30669.

💬 glozow commented on pull request "test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py`":
(https://github.com/bitcoin/bitcoin/pull/30683#discussion_r1723364426)
Approach nack on using logs to test behavior. Is this covered in unit tests?

🤔 instagibbs reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2245996944)
reviewed all refactorings through 388c2f5d7ee5f38cbefaff0c85cf298083127299

Pretty straight forward the whole way through.

going to spend time reviewing/testing the fuzz portions next

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1722086417)
3824d1f81ad9e4ff44ec46d07f4e8f86f27ce643 decent time to rename the function since that's been a previous discussion point?