💬 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

💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2477594004)
> Or keep on being careful about log levels just as before when the rate limiting didn't exist?

Imo rate limiting changes nothing about how we should think about carefully using log levels. Logs are meant to be informative to the user, and any potential for uninformative entries to flood the log, potentially burying important information, should be avoided. Rate limiting is just a belt-and-suspenders, in my view.

💬 stringintech commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#issuecomment-3467431638)
986eb77 to 3e7d272 - reversed the order of `-regtest` flag and input `DATADIR` as suggested by @TheCharlatan and added simple validation for the flag as suggested by @frankomosh.

👍 stickies-v approved a pull request: "validation: Improve warnings in case of chain corruption"
(https://github.com/bitcoin/bitcoin/pull/33553#pullrequestreview-3398970603)
ACK 6d2c8ea9dbd77c71051935b5ab59224487509559

💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2477605536)
I think logging the block hash would be helpful here?

📝 hebasto converted_to_draft a pull request: "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`"
(https://github.com/bitcoin/bitcoin/pull/33725)
This PR continues the work from bitcoin/bitcoin#31308 by extending the treatment of IWYU warnings as errors to the `src/init` and `src/policy` directories.

💬 maflcko commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#issuecomment-3467501417)
lgtm ACK 0b3b8a3be1a0db0dfc634acca1d9305dc0fbfae6

This is now selecting the correct provider.


Some follow-up ideas:

I ran `codex -c model=gpt-5-mini --dangerously-bypass-approvals-and-sandbox exec 'Review the top commit'` on ff18b6bbaf322739fe98fd51b0d89d65a5775ab5 for fun, and it printed:

```
**Potential issue I found**
- In `action.yml` the `cache-provider` `options` list contains `gh`:
- `.github/actions/configure-docker/action.yml:4`
- But the workflow sets `provider=gh
...

💬 fanquake commented on pull request "depends: sqlite 3.50.4; switch to autosetup":
(https://github.com/bitcoin/bitcoin/pull/32655#issuecomment-3467654930)
Guix Build (aarch64)
```bash
3e2443f9828f1541c02f5c33d29c78945bf218411bc35cf13894ad048495aad2 guix-build-1db74914706f/output/aarch64-linux-gnu/SHA256SUMS.part
4a0f92a547fb3f94f930cbdb0575aaab16b1f7d95cd9172cce44068e1ec7ff62 guix-build-1db74914706f/output/aarch64-linux-gnu/bitcoin-1db74914706f-aarch64-linux-gnu-debug.tar.gz
c98f76eae0da3bf25774f5e8b25edac8d315bc428b2e21bf1d78cd8ebe9719b1 guix-build-1db74914706f/output/aarch64-linux-gnu/bitcoin-1db74914706f-aarch64-linux-gnu.tar.gz
7d210ac
...