🚀 glozow merged a pull request: "scripted-diff: Unify error and warning log formatting"
(https://github.com/bitcoin/bitcoin/pull/34033)

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611367256)
>
> @sedited, are you sure still we need the `assert(false)` hack, it seems `-Wswitch` already warns:

Yes, the `assert(false)` is needed, even if the compiler can warn about missing enum class cases. this is, because even with an enum class, there could be unhandled cases. This is not forbidden by the language. So the switch may fall through and lead to UB or unexpected behavior. Generally, we prefer assert(false) over UB

💬 hebasto commented on pull request "cmake: Add fail pattern to `try_append_cxx_flags`":
(https://github.com/bitcoin/bitcoin/pull/34047#discussion_r2611383555)
Thanks! An inline explanatory comment has been added.

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611394282)
> what would be the advantage of zero size?

idk. It is generally valid to read zero bytes:

```
with open("example.txt", "rb") as f:
data = f.read(0)
print(data) # prints: b''
```

i don't mind either way. I just wanted to mention it, because some non-ideal hacky script may read blocks in ranges and read the last range, even if it is of zero size. yes, the script should be adjusted, but I don't see the harm in allowing it. In any case, just a nit and not important at all.

💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611396410)
How can you move out of `data` if it is `const` though? The `const` needs to be removed. I like the approach suggested by @romanz to inline it in the previous discussion.

💬 l0rinc commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611412135)
So the -Wswitch wouldn't catch them? I have checked multiple and they all failed the build if I left out any enum value

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611448233)




> Good point about the copy. If I keep the `const` and add `return std::move(*data)` at the end my LSP complains about it having no effect. (My build setup does not). Not sure if removing the `const` but adding the explicit move would help.

If the `const` is kept, the `std::move` will hopefully fail clang-tidy in the CI.

The const needs to be removed and an `std::move` will need to be added. This is what I was trying to show with the godbolt, but maybe it wasn't clear.

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611463509)
> So the -Wswitch wouldn't catch them? I have checked multiple and they all failed the build if I left out any enum value

No. See https://godbolt.org/z/6zGqrcnax

```cpp
#include <iostream>

enum class Color : uint8_t { Red, Green, Blue };

int to_int(Color c) {
switch (c) {
case Color::Red:
return 1;
case Color::Green:
return 2;
case Color::Blue:
return 3;
}
return -1; // this should never happen?
}

i
...

💬 laanwj commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3643004350)
Also have a partial RISC-V build. Sadly the device disk filled up before the MacOS build could start. i did a `guix gc` and i'm afraid it nuked some important packages so the rebuild may take a while 😅
The good thing is that the rest matches.
```
riscv64
02de261cf6c90f15a69d42b43e63485a6c0cdb3d745b5916b6834278a1d63dbc guix-build-a72ff31b879f/output/aarch64-linux-gnu/SHA256SUMS.part
cf2d9615338be34f5082940d7d2b1a4c085226a410a75dceec764e42cd5df5ee guix-build-a72ff31b879f/output/aarch64-lin
...

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611552247)
Added in e4f460bbe44ba11e3997da80daf39cbb2515fefc

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611552484)
Dropped in e4f460bbe44ba11e3997da80daf39cbb2515fefc

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611553805)
Deduplicated tests in 4e2af1c06547230b9245d94e7bcb1129f2c49714

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611555519)
Changed in f2fd1aa21c7694cef393b4a13e472ae9d3fc54fc

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2611558798)
Added in e4f460bbe44ba11e3997da80daf39cbb2515fefc

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3643160579)
Removed a trailing space in https://github.com/bitcoin/bitcoin/compare/e4f460bbe44ba11e3997da80daf39cbb2515fefc..07135290c1720a14c9d2f18a5700bb6565ae7a10.

🤔 danielabrozzoni reviewed a pull request: "rpc: Add optional peer_ids param to filter getpeerinfo"
(https://github.com/bitcoin/bitcoin/pull/32741#pullrequestreview-3568688179)
tACK 9b6090d8d985f3ce61eaa587511bbeeeedf28cb9

I left a micro-nit, but I'd say avoid fixing it unless you already need to update, so you don't invalidate the ack :)

💬 danielabrozzoni commented on pull request "rpc: Add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2611574944)
nit if retouched: "peer IDs" (so it stays consistent with the rest of the help text)

✅ l0rinc closed a pull request: "bench: run `FindByte` across block-sized buffer"
(https://github.com/bitcoin/bitcoin/pull/34046)

📝 l0rinc reopened a pull request: "bench: run `FindByte` across block-sized buffer"
(https://github.com/bitcoin/bitcoin/pull/34046)
The current benchmark doesn't represent a realistic scenario:
* it only checks 200 bytes, while in reality we have 8 MB: https://github.com/bitcoin/bitcoin/blob/2c44c41984e0a170cf38d595635a9e861854573c/src/validation.cpp#L5009
* it only checks the very last position (which is basically the missing case), while in reality it's a circular buffer, so the magic bytes can be anywhere
* plain array was needlessly serialized instead of written directly

The benchmark was updated to start from diff
...

💬 l0rinc commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#issuecomment-3643249468)
(close/open dance to fix boost download issue)

✅ l0rinc closed a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497)