💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596422778)
We can at least change the declaration to not be an optional:
```
std::pair<size_t, size_t> block_part;
```

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596452456)
WDYT about https://github.com/bitcoin/bitcoin/compare/9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3..262f4dfe691b12171a31c9219ba33f1fe55f67a7?

💬 sedited commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596516751)
See https://github.com/sedited/bitcoin/tree/rmHeaderLowWork :)

The reason we are using it in net_processing is the following: We might receive an unrequested block whose block header we previously have not processed and which forks off at a low-work point. A peer sending as such a block is not punished, but the message is essentially just ignored. We could just check for that condition in net_processing and return early, but then we are also skipping a bunch of validation work, that might cat
...

👍 sedited approved a pull request: "Add util::Expected (std::expected)"
(https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3549497745)
Re-ACK faa23738fc2576e412edb04a4004fab537a3098e

📝 codeabysss opened a pull request: "[p2p] Saturate LocalServiceInfo::nScore to prevent overflow"
(https://github.com/bitcoin/bitcoin/pull/34028)
This prevents signed integer overflow in `LocalServiceInfo::nScore` by saturating at `std::numeric_limits<int>::max()` in `SeenLocal()`.

The `nScore` field could overflow from `INT_MAX` to `INT_MIN` when incremented during version handshakes, causing undefined behavior.

I implemented saturation in `SeenLocal()` to cap `nScore` at `std::numeric_limits<int>::max()` instead of allowing overflow. This prevents undefined behavior while maintaining memory efficiency - after 2 billion connections
...

💬 ajtowns commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3623426022)
> Yeah, I also wonder what the use-case here is or was.

Looks like it was introduced in v0.2.9 where it was used as a heuristic for determining if we're in IBD (due to being more than 2k blocks behind where our peers were), The check was removed in #20624, which also has some more context.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3623680270)
Now a proper repository for the Python code: https://github.com/sipa/pyclusterlin

💬 ryanofsky commented on pull request "mining: add getTransactions(ByWitnessID) IPC methods":
(https://github.com/bitcoin/bitcoin/pull/34020#issuecomment-3624001654)
>figure out [Treat std::shared_ptr nullptr as empty Data bitcoin-core/libmultiprocess#235](https://github.com/bitcoin-core/libmultiprocess/issues/235) (this PR is draft pending that, subtree linter failure is expected)

This turned out to be an interesting problem, because in general it's not safe to treat empty `Data` fields the same as unset values. For example with `std::optional<std::string>` or `std::string*` we would want to interpret empty `Data` fields as empty strings not null values.
...

💬 andrewtoth commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2596842637)
Do we really need to pass the dirty count here? We always iterate through the entire cursor, so we can just set `m_dirty_count = 0;` after a `Sync` or `Flush` (we already do for `Flush`).

📝 Osptium-Invincible opened a pull request: "added waring lmao"
(https://github.com/bitcoin/bitcoin/pull/34029)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2597340445)
Hi @rkrux there is an issue [CI / Windows, ucrt, test cross-built (pull_request)](https://github.com/bitcoin/bitcoin/actions/runs/19963803010/job/57251640476?pr=31668)Failing after 18m but in Linux it passed do you have any idea how to fix this.

💬 Chand-ra commented on pull request "refactor: C++20 operators":
(https://github.com/bitcoin/bitcoin/pull/33771#issuecomment-3625675526)
tACK [`48840bf`](https://github.com/bitcoin/bitcoin/commit/48840bfc2d7beeac0ddf56a3c26b243156ec8936). Built the PR and ran unit tests; everything passes.

Verified that all instances of `operator!=` have been removed and `operator<=>` replaces combined implementations of `<`, `<=`, `>`, and `>=`. (Note: PR #33772 and not this one gets rid of `operator<` in [`src/prevector.h`](https://github.com/bitcoin/bitcoin/blob/89dc82295ebd8a959fb5f6e7a116b6ce9e44e340/src/prevector.h#L458).)

💬 Sjors commented on pull request "mining: add getTransactions(ByWitnessID) IPC methods":
(https://github.com/bitcoin/bitcoin/pull/34020#issuecomment-3625705058)
Thanks, I swapped in your commit (subtree linter is still expected to fail).

> So your workaround of interpreting empty Data values as null CTransactionRef is probably the best approach for now.

One reason I have some confidence in this approach is that the Python functional tests are able to read the result. Although it interprets the empty fields as `b''` rather than `None`, that shouldn't be an issue for client developers.

👍 hodlinator approved a pull request: "Add util::Expected (std::expected)"
(https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3550883332)
ACK faa23738fc2576e412edb04a4004fab537a3098e

💬 hodlinator commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2597671961)
(Tried old-school `(void)connection_client.release();` and `[[maybe_unused]] (void)connection_client.release();` to avoid the noisy scope but they don't pass clang-tidy).

💬 hodlinator commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2597679959)
The above patch + `Assume()` would be nice but I won't insist if you prefer not to, in this case.

💬 hodlinator commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2597689237)
Great suggestion about `bugprone-unused-return-value`!

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2597786362)
There is an `AllowCastToVoid` option in https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unused-return-value.html. `(void)` would be shorter than `[[maybe_unused]] auto _`. No strong opinion, but maybe it can be changed in a follow-up?

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2597791146)
Will leave for a follow-up :)

⚠️ plebhash opened an issue: "consider adding a new `interface RawTxFeed` on Mining IPC"
(https://github.com/bitcoin/bitcoin/issues/34030)
Sv2 Job Declarator Server (a.k.a. JDS) needs to keep an in-memory representation of the mempool so it can validate the custom jobs it receives via `DeclareMiningJob` message

currently, SRI JDS polls some RPCs against Bitcoin Core, but polling is rather suboptimal

on https://github.com/stratum-mining/sv2-apps/issues/26 we considered switching to an approach where we subscribe for the `rawtx` ZMQ feed... JDS would have some task that's solely dedicated to consuming this feed

however, [I hear ZM
...