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

🚀 hebasto merged a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550)

💬 fanquake commented on pull request "ci: add Valgrind fuzz":
(https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2450847591)
Dropped to `md` for a look.

🤔 mzumsande reviewed a pull request: "addrman, net: filter during address selection via AddrPolicy to avoid underfill"
(https://github.com/bitcoin/bitcoin/pull/33663#pullrequestreview-3360827654)
Concept ACK

The main function of this is p2p (answering GetAddr requests from peers), RPCs like `getnodeaddresses` are of secondary importance.
Since the p2p use is also affected by this, the PR description should be changed to reflect this.

That said, it seems good to always try to return 1000 addresses in a GetAddr answer, so that peers don't know whether we have a long list of banned addresses or not.

💬 mzumsande commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#discussion_r2448349379)
here and below: would be good to expand this over multiple lines (clang-format)