💬 mercie-ux commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#issuecomment-3619834301)
Concept ACK #34003

I ran the interface_ipc.py test locally and it passes. The PR cleans up the test by fixing broken checks, missing await calls, and variable naming.

I’m new to PR reviews, but from what I can tell, the changes seem correct and improve readability. I don’t see any issues with the changes.

💬 maflcko commented on pull request "scripted-diff: Use LogInfo over LogPrintf":
(https://github.com/bitcoin/bitcoin/pull/29641#issuecomment-3619835819)
> nit: would be nice to update the scripted-diff to also remove trailing newlines if that's not a huge pita

Thx, I can take a look, if I have to re-touch.

💬 arejula27 commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2594702214)
Agree, I started with your review, and now I am doing #25665, the explanation I suggested feels more suitable in the other one; however, I would like you to consider adding **directly** as I mentioned, as you already did some small text corrections on `results.h`

⚠️ maflcko opened an issue: "ci: The bitcoincore.org depends fallback is missing sqlite-autoconf-3500400.tar.gz"
(https://github.com/bitcoin/bitcoin/issues/34021)
The depends fallback is down again?

```
$ curl --show-headers 'https://bitcoincore.org/depends-sources/sqlite-autoconf-3500400.tar.gz'
HTTP/2 404
server: nginx
date: Fri, 28 Nov 2025 09:21:07 GMT
content-type: text/html
content-length: 146
strict-transport-security: max-age=63072000; includeSubDomains; preload
```

_Originally posted by @maflcko in https://github.com/bitcoin/bitcoin/issues/32655#issuecomment-3588516475_

💬 arejula27 commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3619893884)
@maflcko So you think it will be interesting to follow the `std::expected` class methods and add:
- [value_or](https://en.cppreference.com/w/cpp/utility/expected/value_or.html)
- [error_or](https://en.cppreference.com/w/cpp/utility/expected/error_or.html)

I think it will be valuable and will give more flexibility in error handling cases

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3620016424)
> @maflcko Do you think it will be interesting to follow the `std::expected` class methods and add:
>
> * [value_or](https://en.cppreference.com/w/cpp/utility/expected/value_or.html)
>
> * [error_or](https://en.cppreference.com/w/cpp/utility/expected/error_or.html)
>
>
> I think it will be valuable and will give more flexibility in error handling cases

I don't see a valid use-case for `error_or`, but i guess value_or may come in handy possibly in the future. Other than that
...

💬 stickies-v commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2594785458)
I haven't used `std::{un}expected` yet in practice, so I'm not really familiar with real life use cases yet. From research, one use case that seems plausible is to write a handler for failure cases (e.g. to log and then abort). Templating that function on `std::unexpected<E>` seems sensible? There are of course plenty of other ways to write that handler, but my point is that because it's public people _might_ do it, and then that'll break drop-in replacement. Regardless, it probably will be a ra
...

📝 sedited opened a pull request: "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders"
(https://github.com/bitcoin/bitcoin/pull/34022)
It is always set to true, so there is no point in keeping it.

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2594802701)
> Regardless, it probably will be a rare occurence, and it should be an easy fix when we switch to `std::expected`, so I'm not worried either way.

Same. Will leave as-is for now, but happy to push any diff, or review a follow-up, if there is need.

💬 arejula27 commented on pull request "refactor: Add util::Result failure types and ability to merge result values":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-3620230189)
This PR was started in 2022, before the C++23 standard introduced `std::expected`. I agree with @holdinator that this PR attempts to solve two different issues:
1. The error-handling workflow
2. The ability to merge results, which is useful for high-level functions (e.g., loading a wallet, connecting a block) that perform many internal operations and may want to return a cumulative status instead of a single one-shot value

I agree that these are interesting problems to address. However, I b
...

💬 sedited commented on pull request "p2p: avoid traversing blocks (twice) during IBD":
(https://github.com/bitcoin/bitcoin/pull/32730#issuecomment-3620304717)
> That part was merged with https://github.com/bitcoin/bitcoin/pull/32827

Ah right, forgot about that. Should have mentioned this directly, but the change I was looking for is in the txdownloadman. Was thinking that if this is rebased, we could add a patch for avoiding the additional iteration and filter calculation there in this pull request.

🚀 fanquake merged a pull request: "fuzz: Add a test case for `ParseByteUnits()`"
(https://github.com/bitcoin/bitcoin/pull/34017)

🚀 fanquake merged a pull request: "scripted-diff: Use LogInfo over LogPrintf"
(https://github.com/bitcoin/bitcoin/pull/29641)

✅ fanquake closed a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565)

💬 fanquake commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3620419916)
This is going to get merged via #29415, which at this point, is getting ACKs.

💬 fanquake commented on pull request "net_processing: reorder the code that handles the VERSION message":
(https://github.com/bitcoin/bitcoin/pull/33792#issuecomment-3620420757)
This is going to get merged via #29415, which at this point, is getting ACKs.

✅ fanquake closed a pull request: "net_processing: reorder the code that handles the VERSION message"
(https://github.com/bitcoin/bitcoin/pull/33792)

💬 fanquake commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3620426553)
@ajtowns @mzumsande conceptual thoughts?

🤔 romanz reviewed a pull request: "Add util::Expected (std::expected)"
(https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3547700709)
tACK faa23738fc

Works with #33657 - see rebased commits [here](https://github.com/maflcko/bitcoin-core/compare/2512-exp...romanz:bitcoin:rest-blockpart-expected).

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3620474096)
Many thanks @l0rinc!

I have rebased this PR over #34006 together with https://github.com/l0rinc/bitcoin/pull/61/commits/536f93df79975f50781072301947cafa4640b606 - please take a look :)