💬 willcl-ark commented on issue "make check errors on big endian OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-2196848131)
Thanks, it would be good to know if it's still a problem.

If it is, we can re-open here (if you comment in here) or open a new issue.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658724899)
That looks very interesting. Going to leave this for a follow-up, as it'll need some benchmarking.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658725627)
Going to leave this as a follow-up (I think making this work with CRTP may mean splitting it up into two concepts).

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658726118)
Done.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658726213)
Done.

💬 TheCharlatan commented on pull request "kernel: remove mempool_persist":
(https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2196906030)
Re-ACK f1478c05458562a9bef5c2ba43959d758e7b4745

> I'll ping you on these in the future to make sure you don't already have them teed up.

No worries, it's nice to see contributions from more developers, and if there is some redundant work every now and then, it just means it was reviewed already :)

🤔 marcofleon reviewed a pull request: "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`"
(https://github.com/bitcoin/bitcoin/pull/30237#pullrequestreview-2148020508)
Code review ACK 55eea003af24169c883e1761beb997e151845225. I ran the `blockencodings` unit test and no issues with the new test case.

💬 hodlinator commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1658606208)
`LogPrintf` is deprecated since f7ce5ac08c669ac763e275bb7c82dcfb2b1b6c33 / August 2023. Should use `LogInfo`.

💬 hodlinator commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1658532351)
*node 2

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1658781284)
I believe the idea behind these nonces is to introduce diversity so that everyone has a different cache. That way there's no possibility of poisoning the "global" cache.

👍 stickies-v approved a pull request: "kernel: remove mempool_persist"
(https://github.com/bitcoin/bitcoin/pull/30344#pullrequestreview-2148087355)
ACK f1478c05458562a9bef5c2ba43959d758e7b4745

I think kernel should have an interface that (indirectly) allows users to dump/load mempool to/from disk (or elsewhere), but this `mempool_perist` implementation seems too node opinionated to be included in kernel.

💬 darosior commented on pull request "Wallet: don't underestimate the fees when spending a Taproot output":
(https://github.com/bitcoin/bitcoin/pull/26573#issuecomment-2196984111)
CI is erroring on
```
2024-06-28T11:41:00.1099453Z 5/309 - [1mwallet_fundrawtransaction.py --descriptors [0m failed, Duration: 30 s
2024-06-28T11:41:00.1100206Z
2024-06-28T11:41:00.1100438Z [1mstdout:
2024-06-28T11:41:00.1101244Z [0m2024-06-28T11:40:29.220000Z TestFramework (INFO): PRNG seed is: 3960890322720202555
2024-06-28T11:41:00.1108001Z 2024-06-28T11:40:29.221000Z TestFramework (INFO): Initializing test directory /home/runner/work/_temp/ci/scratch/test_runner/test_runner_₿_🏃_20
...

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1658791383)
Nit: Commit message still says CSignatureCache :)

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1658797600)
Nit: I don't see the harm in keeping it movable even if we don't currently need to move it. As a reader of the code, if I needed to move it in the future, I'd be afraid to remove these because "they're probably there for a reason".

Unless there actually is a reason?

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1658806068)
Same comment here about deleted moves.

💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2197038945)
> It is not being funded, I will add it. Thanks.

Force-pushed adding it.

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2197042027)
ACK 42757ae119b8bee208c7c997cb77470c8e624522 🏓

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 42757ae119b8bee208c7c997cb
...

🤔 ryanofsky reviewed a pull request: "Improve new LogDebug/Trace/Info/Warning/Error Macros"
(https://github.com/bitcoin/bitcoin/pull/29256#pullrequestreview-2148139535)
re: https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2195631997

> The fact that there's something behind the immediate macro level preventing LogInfo/LogError/LogWarning from being filtered is a weaker argument to use in pushing against added logging outside of LogDebug/LogTrace statements, than them not even accepting a category to potentially filter on. Gotta respect an opinionated API. :)

Understood. If the current restrictions which prevent categories from being filtered out
...

💬 ryanofsky commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#discussion_r1658818412)
re: https://github.com/bitcoin/bitcoin/pull/29256#discussion_r1657145216

> I suggested `constexpr auto EXPECTED = std::to_array({` above but maybe it doesn't work for you? [Should support `constexpr` that way under C++20 as far as I can tell](https://en.cppreference.com/w/cpp/container/array/to_array). Are you using some fancy interface to GitHub that doesn't yet show suggestion-embeds?

No sorry, that works perfectly. I was just careless and only noticed `constexpr` without seeing the rest
...

💬 ryanofsky commented on pull request "wallet, logging: Replace WalletLogPrintf() with LogInfo()":
(https://github.com/bitcoin/bitcoin/pull/30343#issuecomment-2197070499)
Updated and rebased db8c6fc2ca5e1ad99aeda741e3b453f1f7bf7955 -> 29eaec25af32e543df3cff091938c3cb8f7cce17 ([`pr/gwlog.3`](https://github.com/ryanofsky/bitcoin/commits/pr/gwlog.3) -> [`pr/gwlog.4`](https://github.com/ryanofsky/bitcoin/commits/pr/gwlog.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/gwlog.3-rebase..pr/gwlog.4)) with suggestion to use `constexpr`