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

✅ fanquake closed a pull request: "guix: build glibc with `--enable-static-pie`"
(https://github.com/bitcoin/bitcoin/pull/33821)

💬 fanquake commented on pull request "guix: build glibc with `--enable-static-pie`":
(https://github.com/bitcoin/bitcoin/pull/33821#issuecomment-3510739654)
Given more patching needed, wont bother with this.

💬 fanquake commented on pull request "doc: Correct `pkgin` command usage on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/33827#issuecomment-3510748600)
Backported to `30.x` in #33609.

✅ fanquake closed an issue: "fuzz: AddressSanitizer: odr-violation typeinfo name for CCoinsViewBacked"
(https://github.com/bitcoin/bitcoin/issues/32995)

💬 fanquake commented on issue "fuzz: AddressSanitizer: odr-violation typeinfo name for CCoinsViewBacked":
(https://github.com/bitcoin/bitcoin/issues/32995#issuecomment-3510749704)
This was either solved @ 21.1.x or user error.

💬 TheCharlatan commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509842466)
Yes, I was merely mentioning it, because the `Tip` and `Genesis` methods in current master at least provided a bit better integrity. I don't think this is important at all though. External callers should provide synchronization for this. So the two calls should just be removed imo.

🚀 fanquake merged a pull request: "scripted-diff: Remove obsolete comment"
(https://github.com/bitcoin/bitcoin/pull/33826)

💬 fanquake commented on pull request "scripted-diff: Remove obsolete comment":
(https://github.com/bitcoin/bitcoin/pull/33826#issuecomment-3510884754)
Backported to `30.x` in #33609.

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2510056357)
From what I can infer, we use `MinimumChainWork` to set an "expected chain length". This protects the node from any chain that is not at least as good as the set expected chain length, especially when it is eclipsed. Since the assumevalid block is typically set at the `MinimumChainWork` height, it would seem that we can remove the `MinimumChainWork` check from here by checking that the assumevalid block is an ancestor of (or is) the best_header, but in the case we are using a malicious assumeva
...

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2510065160)
I wouldn't mind adding a comment stating this. Reviewers can confirm if it correctly reflects the intent of the code.

💬 willcl-ark commented on issue "Inconsistent CJDNS address handling in Local addresses and AddLocal logs":
(https://github.com/bitcoin/bitcoin/issues/33471#issuecomment-3511034888)
@big14way you don't need to be assigned an issue to work on it in this project. Opening a pull request (please read CONTRIBUTING.md frist) and linking it to this issue in the PR description is all that's needed.

💬 TheCharlatan commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065)
> 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?

Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?

💬 l0rinc commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511133487)
reACK 2e713c22c67b3551f04273428db266503178e7fa

<details>
<summary>change since last ack</summary>

```bash
git fetch origin 2e713c22c67b3551f04273428db266503178e7fa && git range-diff 854269a...2e713c2
```
commit messages include scripted diffs and the suggested change by AJ:
```
+%define TBL rbp
```
</details>

💬 hebasto commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2510211223)
This change has just been merged [upstream](https://github.com/arun11299/cpp-subprocess/pull/121).