💬 sedited commented on pull request "refactor: Add util::Result failure types and ability to merge result values":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2595150404)
If it is important to you, it is enough to link the respective lines in the iwyu report.

💬 sedited commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2595152765)
Removed the arg in https://github.com/bitcoin/bitcoin/pull/34022.

💬 arejula27 commented on pull request "refactor: Add util::Result failure types and ability to merge result values":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2595158431)
Thx🫡, I guess this conversation can be marked as resolved then

💬 arejula27 commented on pull request "refactor: Add util::Result failure types and ability to merge result values":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2595158488)
Thx🫡, I guess this conversation can be marked as resolved then

💬 Ataraxia009 commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3620731752)
Wouldn't this cause all logs for all categories to be not rate limited mid i do put just -debug with no specific category?

💬 Ataraxia009 commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3620735787)
> Wouldn't this cause all logs for all categories to be not rate limited if i do put just -debug with no specific category?

If this is true, it seems like we wouldn't cause more friction for users to submit in depth logs

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3620885363)
re-ACK f391296b17a755153960e0afc736df37d1d5fb1e

Minor changes since last review.

💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2595369704)
We can make this a little easier to parse with less nesting if we did:
```C++
std::pair<size_t, size_t> block_part;
if (const auto opt_offset{ToIntegral<size_t>(req->GetQueryParameter("offset").value_or(""))}) {
block_part.first = *opt_offset;
} else {
return RESTERR(req, HTTP_BAD_REQUEST, "Block part offset missing or invalid");
}
if (const auto opt_size{ToIntegral<size_t>(req->GetQueryParameter("size").value_or(""))}) {

...

💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2595371448)
nit
```suggestion
const auto block_data{ReadRawBlock(pos)};
```

💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2595371025)
nit
```suggestion
if (const auto block_data{m_chainman.m_blockman.ReadRawBlock(block_pos)}) {
```

💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2595372132)
nit
```suggestion
const auto block_data{chainman.m_blockman.ReadRawBlock(pos, block_part)};
```

💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2595372728)
nit
```suggestion
if (const auto data{blockman.ReadRawBlock(pos)}) return *data;
```

✅ b-l-u-e closed a pull request: "[p2p] Fix signed integer overflow in LocalServiceInfo::nScore"
(https://github.com/bitcoin/bitcoin/pull/33072)

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2595408520)
Done.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2595409347)
Nice catch, done both.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3621078014)
I have reorganized this PR, dropping most optimizations in favor of a future follow-up.

Addressed comments, added more explanations, and made a few code simplifications.

A python version of the algorithm, lacking most optimizations and using simpler data structures than the C++ version, can be found in https://gist.github.com/sipa/822f60db6608a26bb4aae75fd31bcb12 (in sfl.py).

📝 sipa opened a pull request: "Optimized SFL cluster linearization"
(https://github.com/bitcoin/bitcoin/pull/34023)
Follow-up to #32545, part of #33629.

This contains many of the optimizations that were originally part of #32545 itself.

👍 sedited approved a pull request: "guix: reduce allowed exported symbols"
(https://github.com/bitcoin/bitcoin/pull/33950#pullrequestreview-3548327547)
Nice, ACK 7b90b4f5bb10e2156709b07e3996f867e2421232

Also did a guix build.

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3621171277)
Verified via `git range-diff 2d398050ee31db05e9222149b5e60572ac31d803...f391296b17a755153960e0afc736df37d1d5fb1e` since my last ack.
It contains the suggested comment adjustments, const brace inits, include without comment, fake test ADDRMAN_ADDRESSES, missing coverage for `nonexistent_recipient` confirmation.

code review ACK f391296b17a755153960e0afc736df37d1d5fb1e

💬 l0rinc commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2595516221)
In this case I think the nesting's fine, it signals clearly that `opt_size` should only be checked if `opt_offset` was already successful - in this version we have to check the return values. Both are fine, I'm just explaining why we may not want to avoid the nesting here.