💬 trevarj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3429494296)
https://codeberg.org/guix/guix/pulls/3727

🤔 cedwies reviewed a pull request: "refactor: optimize: reuse containers in transaction policy verification"
(https://github.com/bitcoin/bitcoin/pull/33645#pullrequestreview-3362906716)
Concept ACK

- reusing vector capacity across loop iterations is makes sense
- no functional or policy logic changes introduced
- Code refactoring (range-based loops, variable renames) improves readability

Here is my bench on MacOS M2 MAX:

**Before:**
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 134,805.18 | 7,418.11 | 0.7% | 3.30 | `AssembleBlock`
|
...

💬 cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2449841418)
Just double checking: `vSolutions.clear();` is intentional despite `Solver(...)` already clearing `vSolutions`? For me, I think it's good to keep.

⚠️ carlyluminattinetprise-code opened an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/33673)
### Please describe the feature you'd like to see added.

Bitcoin Core

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No response_

✅ pinheadmz closed an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/33673)

💬 instagibbs commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3430679733)
concept ACK

🤔 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`...