📝 maflcko opened a pull request: "doc: Document valgrind aarch64 clang workaround"
(https://github.com/bitcoin/bitcoin/pull/33742)
Fixes https://github.com/bitcoin/bitcoin/issues/29635

The upstream bug will likely not be fixed any time soon, so it seems better to document the workaround and refer to the upstream bug.

This also allows to use any clang version (instead of requiring 16), going forward.

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`":
(https://github.com/bitcoin/bitcoin/pull/33725#issuecomment-3466897433)
Other candidates could be: `common/ compat/ primitives/ script/ support/ util/ zmq/`

💬 ismaelsadeeq commented on pull request "node: add `BlockTemplate{Cache, Manager}`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3466996399)
> I do think while the code changes seem good, it would be good to have more clarity about the cases where cache hits are expected and this cache is most likely to be useful. This seems like something that could be mentioned in the BlockTemplateCache documentation. I know the issue #33389 mentioned a number of cases but it's unclear if these would be helped by the caching implemented here or if more changes like sharing templates with different options would be required.
>
> If you have more
...

💬 fanquake commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#issuecomment-3467100580)
It seems a bit premature to be doing code review or merging anything here, until some discussion has played out in #33684 in regards to approach, and implementation details (can you link to it from the PR description). There still seem to be high-level questions that need answering/agreement on.

💬 fanquake commented on issue "Async Payjoin":
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3467103998)
Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc @hebasto

📝 maflcko opened a pull request: "fuzz: refactor memcpy to std::ranges::copy to work around ubsan warn"
(https://github.com/bitcoin/bitcoin/pull/33743)
Using std::ranges::copy from the C++ standard library has a few benefits here:

* It has the additional benefit of being a bit more type safe and document the byte cast explicitly.
* The compiler will likely optimize it to the same asm, but performance doesn't really matter here anyway.
* It works around an UB-Sanitizer bug, when the source range is empty.

Fixes https://github.com/bitcoin/bitcoin/issues/33643

💬 maflcko commented on pull request "fuzz: don't pass (maybe) nullptr to memcpy()":
(https://github.com/bitcoin/bitcoin/pull/33644#discussion_r2477298479)
Looks like this fell off the radar. It is a fuzz blocker for 31.0, so I went ahead and pushed my alternative fix in https://github.com/bitcoin/bitcoin/pull/33743

💬 fanquake commented on pull request "fuzz: don't pass (maybe) nullptr to memcpy()":
(https://github.com/bitcoin/bitcoin/pull/33644#discussion_r2477301763)
Ok, closing this for now.

✅ fanquake closed a pull request: "fuzz: don't pass (maybe) nullptr to memcpy()"
(https://github.com/bitcoin/bitcoin/pull/33644)

💬 fanquake commented on pull request "fuzz: refactor memcpy to std::ranges::copy to work around ubsan warn":
(https://github.com/bitcoin/bitcoin/pull/33743#issuecomment-3467141145)
cc @dergoegge @marcofleon

💬 maflcko commented on issue "fuzz: connman fuzz target: runtime error: null pointer passed as argument 2, which is declared to never be null":
(https://github.com/bitcoin/bitcoin/issues/33643#issuecomment-3467153921)
Just to copy the background details here, mentioned earlier:

Apart from https://www.open-std.org/JTC1/SC22/WG14/www/docs/n3466.pdf , see also https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3322.pdf , which says:

> Modify 7.26.1p3:
Where an argument declared as size_t n specifies the length of the array for a function, n can
have the value zero on a call to that function. Unless explicitly stated otherwise in the
description of a particular function in this subclause, pointer arguments on su
...

💬 fanquake commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2477333320)
Thanks, looks better now.

💬 fanquake commented on pull request "test: add valid tx test with minimum-sized ECDSA signature (8 bytes DER-encoded)":
(https://github.com/bitcoin/bitcoin/pull/32924#issuecomment-3467188503)
cc @real-or-random @jonasnick

💬 fanquake commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3467196366)
@151henry151 this still has conflicts.

🤔 fanquake requested changes to a pull request: "guix: Use UCRT runtime for Windows release binaries"
(https://github.com/bitcoin/bitcoin/pull/33593#pullrequestreview-3398698978)
What is the plan for adding CI, as that blocks everything here?

📝 willcl-ark opened a pull request: "Fix lint runner selection (and docker cache)"
(https://github.com/bitcoin/bitcoin/pull/33744)
Fixes: 33735

Correct runner type selection for the lint job.

This was erroneously left-out during refactor of the runner selection mechanism in #33302 causing the lint job to run on GH hosts (and therefore not be able to acces local cirrus caches).

💬 fanquake commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3467299965)
> we can't expect them to run them on a different machine and not even try them locally.

They always use Podman or Docker, and run a Linux VM, on their macOS machine.

💬 ismaelsadeeq commented on pull request "node: add `BlockTemplate{Cache, Manager}`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3467320432)
> I tend to think even if this PR is only used to improve getblocktemplate and remove global state [[1]](https://github.com/bitcoin/bitcoin/blob/292ea0eb8982faef460c210bd4215d603f487463/src/rpc/mining.cpp#L776), [[2]](https://github.com/bitcoin/bitcoin/blob/292ea0eb8982faef460c210bd4215d603f487463/src/rpc/mining.cpp#L859-L861), [[3]](https://github.com/bitcoin/bitcoin/blob/292ea0eb8982faef460c210bd4215d603f487463/src/node/miner.h#L186-L189), the changes might be worth it but I'm not sure if you
...

💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2477570759)
> As for `invalidateblock`:... because they should probably do _something_ to get compatible outbound peers...

I don't think that's necessarily true. In the "happy" path use case of `invalidateblock`, a large number of nodes will have done the same, so compatible nodes should be found automatically. Other approaches to prevent connecting with incompatible nodes would of course be better, but we're talking about an emergency here.

> ... and eventually rate-limiting will kick in.

Yes, but
...

👍 real-or-random approved a pull request: "test: add valid tx test with minimum-sized ECDSA signature (8 bytes DER-encoded)"
(https://github.com/bitcoin/bitcoin/pull/32924#pullrequestreview-3398953286)
utACK https://github.com/bitcoin/bitcoin/pull/32924/commits/5fa81e239a39d161a6d5aba7bcc7e1f22a5be777 interesting case