💬 l0rinc commented on pull request "fuzz: Add tests for `CCoinControl` methods":
(https://github.com/bitcoin/bitcoin/pull/34026#discussion_r2610297526)
Please resolve the comment

💬 l0rinc commented on pull request "fuzz: Add tests for `CCoinControl` methods":
(https://github.com/bitcoin/bitcoin/pull/34026#discussion_r2610306247)
`SetScriptWitness` is already void:

```suggestion
coin_control.Select(out_point).SetScriptWitness(script_wit);
```

💬 l0rinc commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#discussion_r2610361286)
Deleted the manual removal, thanks!

💬 maflcko commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#discussion_r2610422861)
I don't understand this either. There is no obfuscation, so the two are the same either way?

I'd prefer to just keep the `<<` in high level code, unless there is a reason not to.

💬 l0rinc commented on pull request "streams: replace `std::find` with `memchr` (5x improvement)":
(https://github.com/bitcoin/bitcoin/pull/34044#issuecomment-3641734759)
it doesn't help that you rebase on a Knots base - the first few commits are already merged on Core, please fix that.

🤔 maflcko reviewed a pull request: "bench: run `FindByte` across block-sized buffer"
(https://github.com/bitcoin/bitcoin/pull/34046#pullrequestreview-3567263151)
lgtm 464ee9e26f7851ff27c2d18f807ae64c8dd0fc62, but I'd prefer to not use the low-level buffer writing questionable "optimization"

💬 l0rinc commented on pull request "streams: replace `std::find` with `memchr` (5x improvement)":
(https://github.com/bitcoin/bitcoin/pull/34044#discussion_r2610442547)
you could inline hit to the if condition

💬 l0rinc commented on pull request "streams: replace `std::find` with `memchr` (5x improvement)":
(https://github.com/bitcoin/bitcoin/pull/34044#discussion_r2610445121)
why extract `end` if we're getting rid of it in the next commit?

💬 l0rinc commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#discussion_r2610448127)
the vector serializes differently, we don't need the size prefix, it would make the assertion at the end a bit less intuitive

💬 Raimo33 commented on pull request "streams: replace `std::find` with `memchr` (5x improvement)":
(https://github.com/bitcoin/bitcoin/pull/34044#discussion_r2610450968)
messier imo

💬 Raimo33 commented on pull request "streams: replace `std::find` with `memchr` (5x improvement)":
(https://github.com/bitcoin/bitcoin/pull/34044#discussion_r2610453046)
I've thought about it as well. but it makes it easier to review. smaller diff

💬 l0rinc commented on pull request "streams: replace `std::find` with `memchr` (5x improvement)":
(https://github.com/bitcoin/bitcoin/pull/34044#discussion_r2610458005)
how so, how is it easier to review multiple conflicting changes? Same for the `it_start` change - it's completely removed in a next commit.

💬 l0rinc commented on pull request "refactor: inline constant `f_obfuscate = false` parameter":
(https://github.com/bitcoin/bitcoin/pull/34048#discussion_r2610463571)
Yes, it's an unused argument - it's clearer to remove the fake single-instance-abstraction

💬 maflcko commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#discussion_r2610467129)
To use span-serialization, you can just type `std::span{bla}`, see also `git grep '<< std::span{'`.

I don't think it makes sense to use write_buffer for places where span-serialization is wanted. Otherwise, this leads to inconsistent code, such as:

```cpp
file << int_val;
file.write_buffer(vec_val);
file << int_other_val;
```

Also, this mutating the buffer (possibly) makes it doubly confusing.

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2610471488)
Yes, I copied that from the source to simplify greeping for code (since I couldn't adjust the source instead). If you insist, I can decouple the two if I need to touch again

💬 l0rinc commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#discussion_r2610503721)
> this mutating the buffer

There was no obfuscation enabled, so no mutation was happening.

> don't think it makes sense to use write_buffer for places where span-serialization is wanted

I dislike magical operators and needless serialization when we want to directly write content, but I'm also fine with `file << std::span{data}` - pushed.

💬 maflcko commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#issuecomment-3641837244)
review ACK 93941dc74be8817463e024e5aa11ed0638fdc9f3 🐪

<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: review ACK 93941dc74be8
...

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3641843791)
Clean rebase, reviews are welcome again.

💬 hebasto commented on pull request "cmake: Add fail pattern to `try_append_cxx_flags`":
(https://github.com/bitcoin/bitcoin/pull/34047#discussion_r2610521348)
The PR description has been updated with the steps to reproduce the issue.

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2610543103)
This might do it - an earlier version of it passed Windows CI (https://github.com/hodlinator/bitcoin/actions/runs/20131062861):
```diff
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -7,9 +7,11 @@
from decimal import Decimal
from enum import Enum
from io import BytesIO
+from pathlib import Path
import http.client
import json
-import platform
+import os
+import re
import typing
import urllib.parse

@@ -496,15 +498,14 @@ class RESTTest (
...

💬 Raimo33 commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#issuecomment-3641908735)
re ACK 93941dc74be8817463e024e5aa11ed0638fdc9f3