👍 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.

💬 ryanofsky commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2158381286)
re: https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2157745629

> No, and I'm not sure it should be given we don't support this. Maybe we can add a comment and assert that just wiping the block index db is not supported for now?

FWIW, my original draft of 804f09dfa116300914e2aeef05ed9710dd504e8c added this code to LoadChainstate:

```c++
// For now, don't allow wiping block tree db without also wiping chainstate
// db. There's no reason this could not work in theory, but in p
...

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2158381917)
Release note added in #30261.

💬 fanquake commented on pull request "build: Bump g++ minimum supported version to 11":
(https://github.com/bitcoin/bitcoin/pull/29091#issuecomment-2158382309)
Release note in #30261.

👍 maflcko approved a pull request: "doc: add release note for 29091 and 29165"
(https://github.com/bitcoin/bitcoin/pull/30261#pullrequestreview-2107809990)
lgtm

💬 instagibbs commented on pull request "Ephemeral Anchors, take 2":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2158409770)
@theStack the primary motivation is to cover cases where non-0 value is attached to handle cases where a smart contract may want to "throw away" a few sats to fees, but otherwise cannot because of the 0-fee requirement of this PR for transactions with ephemeral anchors. If the ephemeral anchor-having transaction had non-0-fee, that would allow endogenous incentives to get it mined on its own, leaving the dust in the utxo set. As an example from the LN spec, [trimmed outputs(https://github.com/li
...