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

📝 mossein opened a pull request: "rpc: Reject distinct transactions in combinerawtransaction"
(https://github.com/bitcoin/bitcoin/pull/34024)
Previously, combinerawtransaction silently processed only the first transaction when given unrelated transactions as input, ignoring the rest. This could be confusing and lead to unexpected behavior.

This change adds validation to ensure all transactions passed to combinerawtransaction have the same base structure (same inputs and outputs), throwing RPC_INVALID_PARAMETER if they differ.

Fixes #25980

<!--
*** Please remove the following help text before submitting: ***

Pull requests
...

💬 mossein commented on issue "combinerawtransaction confusing with distinct transactions":
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-3621298976)
Closing in favor of #31298 which has been in review since November 2024 and addresses the same issue. I wasn't aware of the existing PRs when I opened this. I'll contribute by reviewing #31298 instead.

Apologies for the duplicate!