💬 brunoerg commented on issue "fuzz: crypter: Abrt in __cxxabiv1::failed_throw":
(https://github.com/bitcoin/bitcoin/issues/30251#issuecomment-2158194986)
Maybe it's not good to use `ConsumeRandomLengthByteVector`, worthes limiting its size.
https://github.com/bitcoin/bitcoin/blob/cad127235e307d7de0e9cc04708dbd31aa6c24b0/src/wallet/test/fuzz/crypter.cpp#L59

💬 fanquake commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG`":
(https://github.com/bitcoin/bitcoin/pull/30201#issuecomment-2158204184)
Added a commit to rename some references to macho ld64 to lld.

💬 brunoerg commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#issuecomment-2158209801)
Concept ACK

💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633197400)
> We could make it so that a peek read only returns one byte and a subsequent normal read return N bytes where the first byte corresponds to the peek byte.

This was the case before this PR, right?

> But I'm not sure if that would change much for our use case.

Where can I see that use case? Is there some code that does not work without the changes to `FuzzedSock::Recv()` done in this PR?

The doc you cited, in my understanding, means that what I described in the first comment of this t
...

🤔 furszy reviewed a pull request: "test: Remove redundant verack check"
(https://github.com/bitcoin/bitcoin/pull/30252#pullrequestreview-2107711418)
utACK 0000276b31ce

👍 BrandonOdiwuor approved a pull request: "log: use error level for critical log messages"
(https://github.com/bitcoin/bitcoin/pull/30255#pullrequestreview-2107735380)
ACK 2222f2573c8de0a7f7171e6fc9e844ad448e1f65 - Replacing LogPrintf with Logerror to enhance critical error logging

👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2107726655)
Code review ACK 09ef322acc0a88a9e119f74923399598984c68f6. Just fuzz test error checking fix and updated comment since last review

💬 ryanofsky commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1633207579)
In commit "[[refactor]] Check CTxMemPool options in constructor" (09ef322acc0a88a9e119f74923399598984c68f6)

It's a shame this has to return a pointer now. It would have been nice to avoid the unreachable code with something like [scope_exit](https://en.cppreference.com/w/cpp/experimental/scope_exit), but I guess that is still experimental and hasn't been standardized.

```c++
bilingual_str error;
auto error_check{std::experimental::scope_exit([&] { Assert(error.empty()); })};

...

🤔 vasild reviewed a pull request: "fuzz: Make FuzzedSock fuzz friendlier"
(https://github.com/bitcoin/bitcoin/pull/30211#pullrequestreview-2107713174)
I think this was merged prematurely.

💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633216366)
Now it will not overflow `buf` but it will lose the trailing `N - M` bytes from the peek.

To fix this, when assigning `random_bytes` earlier, it should only take/pop the first `len` bytes from `m_peek_data`.

I think all this may be unnecessary if the app has to work with peek only returning 1 byte, as discussed in https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1623330207

💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633199919)
`std::vector<uint8_t>` can be used instead of `std::optional<std::vector<uint8_t>>` and if the vector is empty that is the same as the optional without a value.

💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633223401)
> This was the case before this PR, right?

No, a peek read before this PR would return 1 byte and a normal read following that would also only return that one byte (with the exception of sometimes padding with a bunch of zeros). This is what causes the fuzz input to never be interpreted as one continuous sequence of data, which is why mutations that insert pieces of data (e.g. from a dictionary) are ineffective.

> Where can I see that use case? Is there some code that does not work without
...

👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2107775468)
Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.

📝 glozow opened a pull request: "[26.x] backports and final changes for 26.2rc1"
(https://github.com/bitcoin/bitcoin/pull/30260)
Backports:
- #30151

And final changes for 26.2rc1:
- build changes
- manpages

👍 ryanofsky approved a pull request: "indexes: Don't wipe indexes again when continuing a prior reindex"
(https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2107778178)
Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.

💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633234440)
These changes were very much necessary for `FuzzedSock` to be fuzzer friendly. Feel free to open a follow up, if you have suggestion how further improve things.

💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633229348)
Feel free to open a follow up.

💬 glozow commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2158358461)
backported to 26.x in #30260

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#issuecomment-2158379951)
ACK https://github.com/bitcoin/bitcoin/pull/30160/commits/47f705b33fc1381d96c99038e2110e6fe2b2f883

swap coverage added, some of the nits taken, and a couple `Assume()`s added

reviewed via `git range-diff master c99063e 47f705b`

📝 fanquake opened a pull request: "doc: add release note for 29091 and 29165"
(https://github.com/bitcoin/bitcoin/pull/30261)
GCC 11.x or Clang 15.x are now required to compile Bitcoin Core.