🤔 janb84 reviewed a pull request: "doc: reference fuzz coverage steps in quick-start"
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3442038438)
ACK dee7eec64389aa48daff6f7f3ecbd931af72050a

The author removed the duplicate and redundant code coverage instructions which made me disagree with this PR. The author now only references that there are instructions on generating code coverage for fuzzing elsewhere in the documentation, which seems to me as a reasonable thing and good addition.

Thank you for changing this.

✅ fanquake closed an issue: "RFC: Cancelling waitNext calls in the IPC mining interface"
(https://github.com/bitcoin/bitcoin/issues/33575)

🚀 fanquake merged a pull request: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676)

💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3510573430)
`84b2ad0334...9989853447`: rebase due to conflicts

> I don't think it makes much sense to split this out; it doesn't reduce the size of the parent PR meaningfully,

I agree it does not help much. It helps _a little_. And that is the point - to get some smaller changes out of the main PR.

> and renaming a function doesn't really seem like it justifies a PR on its own.

Having this in its own PR helped have a [fresh discussion](https://github.com/bitcoin/bitcoin/pull/33565#discussion_r24
...

👍 rkrux approved a pull request: "scripted-diff: Remove obsolete comment"
(https://github.com/bitcoin/bitcoin/pull/33826#pullrequestreview-3442161682)
crACK 36724205fc1f226d7b5493ed0212c336e7f2ae84

✅ fanquake closed a pull request: "Check required interfaces before generating manpages"
(https://github.com/bitcoin/bitcoin/pull/33828)

💬 fanquake commented on pull request "Check required interfaces before generating manpages":
(https://github.com/bitcoin/bitcoin/pull/33828#issuecomment-3510609561)
This duplicates #33085. If the author doesn't follow up there shortly, we could reopen here.

💬 dergoegge commented on issue "RFC: Add multiprocess fuzz target":
(https://github.com/bitcoin/bitcoin/issues/23015#issuecomment-3510614348)
I'm working on making it easier to deploy custom testing workloads with fuzzamoto, so that something as simple as a different binary name isn't a struggle (i.e. doesn't require patches). See https://github.com/dergoegge/fuzzamoto/issues/48, which will hopefully also enable the use of python with our testing framework. I'll circle back here once that is done.

🚀 fanquake merged a pull request: "ci: Use cmake --preset=dev-mode in test-each-commit task"
(https://github.com/bitcoin/bitcoin/pull/33823)

👍 dergoegge approved a pull request: "doc: reference fuzz coverage steps in quick-start"
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3442187499)
ACK dee7eec64389aa48daff6f7f3ecbd931af72050a

🚀 fanquake merged a pull request: "doc: reference fuzz coverage steps in quick-start"
(https://github.com/bitcoin/bitcoin/pull/33536)

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2509736434)
I don't like this, why not just remove the const?
```patch
-const CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))};
+CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))};
```

💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3510658120)
> The test case producing the ubsan error should just be deleted in my opinion. It does not add anything in terms of coverage. How about:

I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?

I understand the nonnull check can in theory be useful in low-level environments where a nullptr-deref would not be detected as access violation, but there are probably more users using modern systems that may wa
...

💬 l0rinc commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509751663)
Bumped into this again - if we remove the `const` from https://godbolt.org/z/fjc6be65M (`int& ref{Assert(a)};`) that seems to also fix the problem - why is that worse than introducing global suppression?

💬 l0rinc commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3510675433)
I also prefer extracting these smaller changes from big PR to smaller, dedicated ones to have *some* progress and to have focused discussions

💬 maflcko commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509780870)
Removing `const` over a bunch of places, including consensus code, seems like the wrong fix for a short-term temporary workaround for a long-fixed single compiler bug that affects a single version.

The suppression is not global, all other CI or local runs will still have the warning enabled.

Also, the temporary workaround can trivially and quickly removed in https://github.com/bitcoin/bitcoin/pull/33775, which is needed anyway for other reasons.

💬 fanquake commented on pull request "doc: Correct `pkgin` command usage on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/33827#issuecomment-3510723650)
ACK 0698c6b494de0e28c9b909585905aab5b187286e

🚀 fanquake merged a pull request: "doc: Correct `pkgin` command usage on NetBSD"
(https://github.com/bitcoin/bitcoin/pull/33827)

💬 stickies-v commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509810054)
I hadn't thought of the reduced-height reorg scenario, thanks for that. Reorg-safety in general is still an unsolved problem for the public interface, and this is another case of that.

Note: the problem you report already exists for objects based on the `Range` class whenever `size()` depends on chain length, such as for `Chain::Entries` with `Chain::Entries::back()` being able to throw a `std::runtime_error`.

💬 l0rinc commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509811677)
> Removing const over a bunch of places

k, I had the impression this was the only one 👍