📝 maflcko reopened a pull request: "test: Add DERSIG unit tests to script_tests.json"
(https://github.com/bitcoin/bitcoin/pull/33973)
I'd like to add a few `DERSIG` variants of existing `STRICTENC` tests, since `DERSIG` is a consensus flag.

I ran:
```
cmake -B build -DENABLE_WALLET=OFF
cmake --build build --parallel 8
ctest --test-dir build --parallel 8
```
```
6/133 Test #93: script_tests ......................... Passed 1.57 sec
```

✅ diegoviola closed an issue: "Consider enabling plugin=wayland for bitcoincore.org builds"
(https://github.com/bitcoin-core/gui/issues/916)

💬 diegoviola commented on issue "Consider enabling plugin=wayland for bitcoincore.org builds":
(https://github.com/bitcoin-core/gui/issues/916#issuecomment-3616188091)
> The main concern raised was the introduction of additional dependencies: [bitcoin/bitcoin#22708 (comment)](https://github.com/bitcoin/bitcoin/pull/22708#issuecomment-1100861599).

Ah, I understand. As someone who doesn't use `depends` and compiles their own builds this doesn't affect me. I guess not a lot has changed when it comes to "additional dependencies", so I'll just close this.

💬 maflcko commented on issue "ci: failure in Windows native job":
(https://github.com/bitcoin/bitcoin/issues/34012#issuecomment-3616198218)
I haven't looked here, but generally CI does not work well with re-running tasks, when the code is merged with current master, but the ci config is stale.

The solution is to minimize the ci config (yaml) and pull out all ci settings and ci scripts into stand-alone files (similar to the files `.github/ci-lint-exec.py .github/ci-test-each-commit-exec.py`)

As the error is zmq related, it could be due to 49c672853503e561cd1e7fed19a32f21ad345370, but again, I haven't checked this closely.

💬 maflcko commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-3616207470)
Could turn into draft, while CI is red?

💬 fanquake commented on pull request "qa: Remove no longer needed `feature_dirsymlinks.py`":
(https://github.com/bitcoin/bitcoin/pull/33924#issuecomment-3616226134)
~0. Seems fine to leave this for now.

🚀 fanquake merged a pull request: "net: fix use-after-free with v2->v1 reconnection logic"
(https://github.com/bitcoin/bitcoin/pull/33956)

💬 maflcko commented on pull request "util: generalize `util::Result` to support custom errors":
(https://github.com/bitcoin/bitcoin/pull/34005#issuecomment-3616239972)
> I don't see that as a concern for this PR, since the current `Result` class could just be renamed to `Expected` in #25665 to avoid that if desired. So #34005 and #34006 both seem like good solutions to me, and I'd be happy with either.

Either is fine by me as well. It just seems more churn to (1) introduce the Expected alias based on Result, (2) move it a different file, (3) likely rework it to remove the implicit bilingual_string error stuff (because it is not needed and confusing). It see
...

💬 maflcko commented on pull request "util: generalize `util::Result` to support custom errors":
(https://github.com/bitcoin/bitcoin/pull/34005#discussion_r2592174675)
if this alias is added, one should also be added for `std::unexpected`.

✅ maflcko closed an issue: "ASAN use-after-free in `m_reconnections`"
(https://github.com/bitcoin/bitcoin/issues/33615)

⚠️ Sjors opened an issue: "rpc: getrawtransaction lookup by witness txid"
(https://github.com/bitcoin/bitcoin/issues/34013)
For Stratum v2 custom job declaration to be bandwidth efficient, the pool can request only the transactions that it doesn't know about.

The spec doesn't specify how this is achieved, but one method is to call `getrawtransaction` on each of the transaction ids listed in [DeclareMiningJob](https://stratumprotocol.org/specification/06-Job-Declaration-Protocol?query=DeclareMiningJob#644-declareminingjob-client-server).

Unfortunately this RPC only supports `txid`, not `wtxid`.

I think the easiest
...

🤔 hodlinator reviewed a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3543967284)
> Could you provide a patch for me which would go through every single transaction in mainchain history and query it both ways (full block + new RPC) and assert that they're equal? I expect it to be heavy, I can run it on my benchmark servers to make sure it works for every real scenario we have.

What value would that provide? The new API allows fetching arbitrary byte-ranges from within blocks. The current unit tests checking the bytes match seem sufficient for validating the logic (testing wi
...

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2592108737)
It was part of my suggestion (https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2584393520), inspired by https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2582578921. A somewhat common alternative to C++23's `std::unreachable()`.

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2592121707)
Unit test seems sufficient to me.

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2592146736)
Agree it probably makes sense to have an `optional` for the range. It's interesting to explore what could be done with `util::Result`, but I feel that could be done as a follow-up to this PR.

Agree that the ergonomics of `std::expected` are better than `std::variant`, but not that they are terrible. If you are strongly against `std::variant` then maybe we could step back to using out-parameters for now in this PR.

💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2592196089)
> > * Ideally, low level util functions should not be logging at all, but rather pass up a `Result` (possibly with an error string or error value)
>
> Agreed.

Probably don't want translated strings of those low-level errors here (Result is based on that), so I ported `std::expected` in https://github.com/bitcoin/bitcoin/pull/34006. I haven't tried, but it could be useful here.

💬 sedited commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2592204362)
See the dev notes: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures

⚠️ maflcko opened an issue: "mptest hangs, when run in a loop"
(https://github.com/bitcoin/bitcoin/issues/34014)
Initially ci ran into a segfault in https://github.com/maflcko/bitcoin-core-nightly/actions/runs/19882806964/job/56984144564#step:6:4040:

```
3/152 Test #3: mptest ...............................***Failed 0.05 sec
[ TEST ] test.cpp:117: Call FooInterface methods
[ PASS ] test.cpp:117: Call FooInterface methods (18106 μs)
[ TEST ] test.cpp:209: Call IPC method after client connection is closed
[ PASS ] test.cpp:209: Call IPC method after client connection is closed (744 μs)
[ TEST ] test
...

💬 maflcko commented on issue "mptest hangs, when run in a loop":
(https://github.com/bitcoin/bitcoin/issues/34014#issuecomment-3616337285)
I can also reproduce the i686 segfault from the CI run locally, but I suspect it is similar to issue https://github.com/bitcoin/bitcoin/issues/31772

💬 fanquake commented on issue "mptest hangs, when run in a loop":
(https://github.com/bitcoin/bitcoin/issues/34014#issuecomment-3616352068)
cc @Sjors @ryanofsky