💬 monlovesmango commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2070755965)
Ok I see! I understand now. Sorry was running with `--log_level=all` and was misinterpreting these as success messages. Please disregard..

🤔 1440000bytes reviewed a pull request: "Extend signetchallenge to set target block spacing"
(https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-2810654144)
utACK https://github.com/bitcoin/bitcoin/pull/29365/commits/41728b516ec28ba2b6bbffd0b5e4cec8120f61d1

Isn't the commit message too long? You could even have multiple commits in this pull request IMO.

💬 hebasto commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2845818802)
> > During my quick (and possibly not thorough) tests, I was unable to reproduce the issues mentioned in those PRs on our codebase.
>
> Seems odd to not pull in all changes, given no real rationale (i.e the code being incorrect/broken) for excluding them. This also means that next time this header is updated, they may or may not get pulled in anyways, depending on the author of the change, given there's nothing documented to say they should be excluded.

https://github.com/arun11299/cpp-sub
...

💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2845902580)
> utACK [41728b5](https://github.com/bitcoin/bitcoin/commit/41728b516ec28ba2b6bbffd0b5e4cec8120f61d1)
>
> Isn't the commit message too long? You could even have multiple commits in this pull request IMO.

It is possible to split the commit (and the commit message) into multiple parts:

- add function `ParseWrappedSignetChallenge` and its unit test (files `src/chainparams.{h,cpp} src/test/chainparams_tests.cpp src/test/CMakeLists.txt`)
- add function `ParseSignetParams` and its unit t
...

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2070888721)
Thanks for taking such a thorough look!

💬 davidgumberg commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r2070888806)
In commit ef83e9569752b4273e6aed3124642dc057cab540 (shutdown: Tear down mempool after chainman)

Feel free to disregard as out of scope, but while addressing this, it might be nice to make reintroducing this dangling pointer impossible by making the various mempool pointers `shared_ptr`, e.g. https://github.com/davidgumberg/bitcoin/commit/75c839b8d77c207a0946294672b703d9f8eecbe1

💬 oxqnd commented on issue "combinerawtransaction confusing with distinct transactions":
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-2845930776)
I’ll take a look at #31298, but it might take me a little while to get to it. Hope that’s okay!

📝 mzumsande opened a pull request: "cli: add -usefile option"
(https://github.com/bitcoin/bitcoin/pull/32399)
Running large commands via `bitcoin-cli` (such as submitting a large tx or block) runs into the `MAX_ARG_STRLEN` limit, which is 128KB on many systems. (see e.g. https://trofi.github.io/posts/299-maximum-argument-count-on-linux-and-in-gcc.html for more info).

This PR suggests to make it possible to bypass this limit by allowing to put the command and all of its args into a file. The functional test framework is then adjusted to automatically use this option (using a temporary file) if `--usec
...

💬 oxqnd commented on issue "combinerawtransaction confusing with distinct transactions":
(https://github.com/bitcoin/bitcoin/issues/25980#issuecomment-2845932631)
> > Could I try this issue?
>
> Would you mind reviewing https://github.com/bitcoin/bitcoin/pull/31298 instead?
>
> This solution is still up to date, it just hasn't gotten the right reviewers.

I’ll take a look at #31298, but it might take me a little while to get to it. Hope that’s okay!

💬 rebroad commented on pull request "Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence":
(https://github.com/bitcoin-core/gui/pull/866#issuecomment-2845967863)
I've removed the code that changed the totals on the node - the GUI just keeps track now without writing values to the node. Also, added some checks for corrupt save files so that it doesn't load bad data.

💬 achow101 commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#issuecomment-2846003391)
ACK 85368aafa0d1772626d5f615e272b1c1a580b42f

💬 monlovesmango commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#issuecomment-2846047880)
ACK 85368aafa0d1772626d5f615e272b1c1a580b42f

📝 davidgumberg opened a pull request: "random: Use modern Windows randomness functions"
(https://github.com/bitcoin/bitcoin/pull/32400)
This resolves #32391 and is a follow-up to #14089.

The old randomness API has been deprecated and will be removed at some point.[^1]

For reference on `BCryptGenRandom`, see: https://learn.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom.

[`STATUS_SUCCESS`](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/596a1078-e883-4972-9bbc-49e60bebca55) gets defined here since including `ntstatus.h` is [more trouble](https://github.com/bitcoin-core/secp25
...

💬 davidgumberg commented on issue "Use of deprecated CryptAcquireContext in random.cpp":
(https://github.com/bitcoin/bitcoin/issues/32391#issuecomment-2846087303)
Opened #32400 to resolve.

💬 davidgumberg commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070981899)
Thanks, fixed.

💬 davidgumberg commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070981955)
Thanks, fixed.

💬 davidgumberg commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#issuecomment-2846099273)
Rebase to address style feedback.

🤔 shahsb reviewed a pull request: "random: Use modern Windows randomness functions"
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2811079433)
Concept ACK

🤔 shahsb reviewed a pull request: "random: Use modern Windows randomness functions"
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2811079804)
LGTM..!!

ACK https://github.com/bitcoin/bitcoin/pull/32400/commits/9dcf2105a9fa60b600145e3fe032a296d47b28e3

💬 monlovesmango commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#issuecomment-2846328501)
Concept ACK, but I think approach NACK.

Instead of creating a whole new set of output, why not just take the summary and print it as results arrive rather than holding them all until the end? I put together a quick POC which produces the output below in real time (rather than waiting until end):
```bash
(1/221) addition_overflow #60 DONE cov: 265 ft: 320 corp: 47/1674b lim: 62 exec/s: 0 rss: 115Mb
(2/221) addr_info_deserialize #110 DO
...