💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1267035663)
> Also why is /* inBlock = */true ?

That's so it's not counted as a failure.

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1267073906)
Added in push to 26dcb359acf421dc3f448b49e0b564e714c54792. Also had to tweak `authproxy.py` to accommodate code 204

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267078962)
Done

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267079059)
Done

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267080536)
Nice! Took the liberty of adding a commit credited to you. LMK if that's ok and if you'd like any changes.

💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267080665)
Done

💬 MarcoFalke commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1640695209)
> What do you think?

I'd say we should pick what reviewers like the best. However, as you say the clang-tidy plugin has many downsides:

* Likely incomplete (May be missing a Decl matcher, ignores comments, ...)
* Harder to review, and easier to break
* More code
* Requires a full build system
* Slower

I think it should be decided on a case-by-case basis whether a simple `git grep`-based linter is preferred, or a fully engineered clang-tidy plugin. Here, I think a `git grep` seems a
...

💬 MarcoFalke commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1640702774)
Are you still working on this?

💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1640704021)
> Are you still working on this?

Absolutely, thanks. Had a few other things to catch up on. This will be updated before the end of the week

💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1640710663)
We already use credits for some Linux tasks, so nothing will change for them. The others (or all) can be moved to persistent workers easily, I suspect.

Edited OP to mention GitHub Actions CI as alternative. Last time we looked it required write access, but GitHub already has full write access, so I am not sure if this is still a concern or if it even applies?

💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1640745025)
Rebased.

📝 sipa opened a pull request: "crypto: ChaCha20 `Span<std::byte>` modernization & follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28100)
This modernizes the ChaCha20 and ChaCha20Aligned interfaces to be `Span<std::byte>` based, and other improvements.

* Removes our OpenSSH-inspired variant of the ChaCha20Poly1305 AEAD (which was added in anticipation of BIP324 using it, but the spec was later changed to using the RFC8439 one unmodified). #28008 replaces the code anyway, so it is pointless to apply the refectoring done in this PR to that old code.
* Modifies all functions and constructors of `ChaCha20` and `ChaCha20Aligned` to
...

💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164225)
Done in #28100.

💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164605)
Done in #28100.

💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164850)
Done in #28100.

💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267165531)
I had forgotten about this PR comment, but this was effectively done in #27985.

💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267165897)
Done in #28100.

💬 jonatack commented on pull request "crypto: ChaCha20 `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267196895)
In commit 9e1199c7ce5

```
random.cpp:668:44: error: non-type template argument is not a constant expression
static constexpr std::array<std::byte, rng.KEYLEN> ZERO{};
^~~~~~~~~~
random.cpp:668:44: note: implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
1 error generated.
```

```
$ clang --version
Homebrew clang version 16.0.6
Target: arm64-apple-darwin22.5.0
Thread model:
...

💬 sipa commented on pull request "crypto: ChaCha20 `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267203493)
Strange that GCC doesn't complain about this. Fixed.

👍 ryanofsky approved a pull request: "Rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1535703245)
Code review ACK 137762f34a845e491b80f9cea07efc4427cb38bf. Left some more suggestions, but this looks good in its current form.

Just so I don't forget, I want to note that that it might make sense to move `IsInitialBlockDownload` and `NotifyHeaderTip` functions from the `Chainstate` class to `ChainstateManager` as a followups to this PR (https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1246868905, https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1237552792)