🤔 maflcko reviewed a pull request: "ci: add Valgrind fuzz"
(https://github.com/bitcoin/bitcoin/pull/33461#pullrequestreview-3363885023)
lgtm

Conceptually, the same feedback on valgrind applies here (https://github.com/bitcoin/bitcoin/pull/33411#issuecomment-3410475429), but if an issue arises, it should be fine to quickly drop this task again to unblock the CI caused by a false-positive.

💬 maflcko commented on pull request "ci: add Valgrind fuzz":
(https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2450576229)
```suggestion
cirrus-runner: 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-md'
```

Looks like the runtime with lg was 50 minutes. I wonder if it will be the same speed, using md, as there are only three trailing, slow fuzz tests.

💬 laanwj commented on pull request "test: Use unassigned p2p_port instead of hardcoded 60000 in p2p_i2p_ports.py":
(https://github.com/bitcoin/bitcoin/pull/33670#issuecomment-3430717688)
Code review ACK fa20275db32c5b9b0fe35effe2d1cf3d958e7310

📝 maflcko opened a pull request: "ci: Doc ASLR workaround for sanitizer tasks"
(https://github.com/bitcoin/bitcoin/pull/33674)
Fixes https://github.com/bitcoin/bitcoin/issues/30674

✅ maflcko closed an issue: "wallet: CPU use proportional to wallet transaction count when idle"
(https://github.com/bitcoin/bitcoin/issues/16815)

💬 maflcko commented on issue "wallet: CPU use proportional to wallet transaction count when idle":
(https://github.com/bitcoin/bitcoin/issues/16815#issuecomment-3430799802)
Closing for now, but a comment can be left below to have it reopened. Also, a new issue can be filed.

✅ maflcko closed an issue: "Node stuck with repeated "Cache size exceeds total space" log message"
(https://github.com/bitcoin/bitcoin/issues/27599)

💬 maflcko commented on issue "Node stuck with repeated "Cache size exceeds total space" log message":
(https://github.com/bitcoin/bitcoin/issues/27599#issuecomment-3430802736)
Yeah, closing for now, but this can be re-opened when it happens again, or a new issue can be filed.

💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3430815886)
Opened a PR for removal of redundant sighash calculation here: #33665.

💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3430818448)
> Not so sure about the `Assemble Block` bench though. Doesn't it measure block assembly **_after_** admission, therefore not timing the policy checks you changed?

Thanks for the review & benchmarks. I'm positive about the fact that the `AssembleBlock` bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.

💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2450692465)
yes, it is deliberate to prevent future programming errors.

🤔 mzumsande reviewed a pull request: "test: Use unassigned p2p_port instead of hardcoded 60000 in p2p_i2p_ports.py"
(https://github.com/bitcoin/bitcoin/pull/33670#pullrequestreview-3364100215)
ACK fa20275db32c5b9b0fe35effe2d1cf3d958e7310

LGTM, I also took a look recently trying to find the root cause but didn't come up with a better idea than just changing the port and hoping for the best.

💬 maflcko commented on pull request "fuzz: don't pass (maybe) nullptr to memcpy()":
(https://github.com/bitcoin/bitcoin/pull/33644#issuecomment-3430890579)
> > Given that the len argument is 0, memcpy() should not try to dereference the maybenull argument
>
> According to the C standard (§7.26.1), the behaviour of `memcpy`is undefined if either `src`or `dst`are null. So it is not a bug.

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 func
...

💬 pablomartin4btc commented on pull request "wallettool, test: Remove BDB/ legacy wallets dump feature":
(https://github.com/bitcoin/bitcoin/pull/33161#discussion_r2450750637)
Yeah, it would be nice... Perhaps the framework can be extended somehow and we can just evaluate the execution of the test... kind of calling `if self.has_previous_releases():`, but at the moment the releases paths are being validated during the test setup where you call `add_nodes`...

🤔 w0xlt reviewed a pull request: "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key"
(https://github.com/bitcoin/bitcoin/pull/32471#pullrequestreview-3364117332)
ACK https://github.com/bitcoin/bitcoin/pull/32471/commits/2b16014dfbe7e249e4929beafb4ce26415206e7a

💬 Sjors commented on issue "v27.2 guix build fails with hash mismatch":
(https://github.com/bitcoin/bitcoin/issues/31266#issuecomment-3430904842)
I think it was a cryptic reference to the fact that v27 is end-of-maintenance, but these type of errors still happen, at least until the next time-machine bump.

💬 maflcko commented on pull request "fuzz: don't pass (maybe) nullptr to memcpy()":
(https://github.com/bitcoin/bitcoin/pull/33644#discussion_r2450783853)
Instead of using C functions in a c++ codebase and then working around C quirks temporarily, it seems better to just use the c++ algorithms. The have a few benefits:

* The compiler will optimize them to the same asm, but performance doesn't really matter here anyway
* They work around the uban bug
* they have the additional benefit of being a bit more type safe and document the byte cast explicitly
* The resulting code and logic is shorter

```
std::ranges::copy(sin_addr_bytes, reinterp
...

💬 ryanofsky commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#discussion_r2450789739)
It was intentional just to minimize verbosity of the code and size of the diff, but it's true this goes in direction of using std::filesystem::path instead of fs::path, and maybe we don't want that.

To me, it seems good to prefer using fs::path instead of std::filesystem::path, but using std::filesystem::path is not always avoidable anyway, and it is at least safe to use as long as not doing conversions to or from arbitrary `char` strings, so it's ok to tolerate in some places

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-3430964240)
Just to give another data point: 60000 falls into the ephemeral port range, which normally is avoided by the functional tests, as they start using ports starting from around 11000. However, I can't explain how and if the ephemeral port range itself was related to the issue here.

To check it, you can run:

```
cat /proc/sys/net/ipv4/ip_local_port_range

32768 60999
```


So a more trivial fix could be to change 60000 to 61000, but https://github.com/bitcoin/bitcoin/pull/33670 seems more natural.
...

💬 Sjors commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3430978163)
Two arguments against this change that I find worth pondering a bit about:

@fanquake wrote:

> Would conflict with anyone using a .gitignored `agents.md` locally?

I haven't checked if there's an existing solution for that.

@kanzure wrote:

> One other concern I have is that this is essentially a change that is meant
to be subversive to a developer's tools or development environment.

I could take that argument even further: let's say an anonymous developer uses the LLM to slight
...