👍 shaavan approved a pull request: "consensus: Store transaction nVersion as uint32_t"
(https://github.com/bitcoin/bitcoin/pull/29325#pullrequestreview-2098882561)
ACK 2431ecff2fb6f4b68d3de0ea36e3bcc4403e94ba 🚀

💬 fanquake commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149620807)
> Can you re-try on 27.x, because https://github.com/bitcoin/bitcoin/commit/c3530254c926a5ab9b710512bbbd29e5cd5b10f7 is not in 27.x.

27.x & fccd32efe6e2950b2c74fdec2ade54040ca32a2c compiles with `gcc-10 (Ubuntu 10.5.0-4ubuntu2) 10.5.0`.

💬 maflcko commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149663564)
Oh, 27.x remains on g++-10, so no backport needed. I was wrong. See https://github.com/bitcoin/bitcoin/blob/27.x/doc/dependencies.md

💬 maflcko commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149664984)
utACK 232928b58a82e3f15307deba1ae921ae2960ccc8

Seems fine to fail early here, instead of running into a `util/time` compile failure later on.

💬 maflcko commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1627639850)
```suggestion
for (const auto& ch: reverse_iterate(buff)) {
```

💬 maflcko commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1627643940)
Alternative, if it doesn't compile with `Span`:

```suggestion
for (const auto& ch: reverse_iterate(std::span{buff}) {
```

💬 brunoerg commented on pull request "test: add validation for gettxout RPC response":
(https://github.com/bitcoin/bitcoin/pull/30226#issuecomment-2149692078)
Why not just expanding the tests in `wallet_basic.py`?

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1627668193)
Sorry, i saw your comment earlier, just didn't have time to comment earlier. Thanks, will push soon.

📝 maflcko opened a pull request: "fuzz: Use std::span in FuzzBufferType"
(https://github.com/bitcoin/bitcoin/pull/30229)
The use of `Span` is problematic, because it lacks methods such as `rbegin`, leading to compile failures when used:

```
error: no member named 'rbegin' in 'Span<const unsigned char>'
```

One could fix `Span`, but it seems better to use `std::span`, given that `Span` will be removed anyway in the long term.

💬 maflcko commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1627691575)
Sure no rush. See also https://github.com/bitcoin/bitcoin/pull/30229

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1627725865)
We may still need to deallocate the old buffer in that case.

💬 sipa commented on pull request "refactor: Reduce memory copying operations in bech32 encoding":
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2149920766)
utACK 07f64177a49f1b6b4d486d10cf67fddfa3c995eb

Making the code more readable is a good reason to make this change, but this code was specifically written to prioritize simplicity over performance. If we'd care about performance, it's probably better to use an approach similar to that of the C reference implementation (as it works entirely on the stack, while the current code does several allocations: 1 in `ExpandHRP`, between 1 and 3 in `CreateChecksum`, and 1 inevitable one in `Encode`).

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627796687)
I've added an Assume that `bits` is in range.

🤔 maflcko reviewed a pull request: "Several randomness improvements"
(https://github.com/bitcoin/bitcoin/pull/29625#pullrequestreview-2098835816)
e7676b6c52672f557f0e2b036d5e877393f1ce46 🎩

<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: e7676b6c52672f557f0e2b036d5e877393
...

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627567196)
nit in 0b92cd2ae4a317be4fe0dfd208017a7d05d1eb2a: Not sure I understand this concept. Is there anything that is not already enforced normally in C++ that would be enforced by this concept?

For example, `static_cast` in the `Impl()` method, which enforces this concept should already check for inheritance:

```
<source>:17:40: error: static_cast from 'Mixin<FRC> *' to 'FRC *', which are not related by inheritance, is not allowed
```

Conversely, if the class wasn't derived from `RandomMixi
...

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627736240)
unrelated q in 6370a51aac06a430ec53f29fe03f5bbd08371b34: Can you explain where one finds this "copy()" method?

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627554476)
nit in 0b92cd2ae4a317be4fe0dfd208017a7d05d1eb2a:

Looks like this commit is doing several things at once. It may be easier to review if move-only stuff (like moving rand256 and randbytes function bodies to the header file, and removing the explicit template instantiation) was done in a separate prepare commit?

💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1627806235)
perfect thanks

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1627807244)
nit in c099cd2e3c20fc416e3f7fc1da4798bb8042af1c: Could increase the severity to `LogError()` while touching this?

📝 marcofleon opened a pull request: "fuzz: add I2P harness"
(https://github.com/bitcoin/bitcoin/pull/30230)
Addresses https://github.com/bitcoin/bitcoin/issues/28803.

The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness.

<details>
<summary>I2P dict</summary>

```
"HELLO VERSION"
"HELLO REPLY RESULT=OK VERSION="
"HELLO REPLY RESULT=NOVERSION"
"HELLO REPLY RESULT=I2P_ERROR"
"SESSION CREATE"
"SESSION STATUS RESULT=OK DESTINATION="
"SESSION STATUS RESULT=DUPLICATED_ID"
"SESSION STATUS RESULT=DUPLICATED_DEST"
"SE
...