👋 marcofleon's pull request is ready for review: "fuzz: Test headers pre-sync through p2p"
(https://github.com/bitcoin/bitcoin/pull/30661)

👍 TheCharlatan approved a pull request: "[RFC] Switch and/or align debugging flags (back) to `-Og`"
(https://github.com/bitcoin/bitcoin/pull/29796#pullrequestreview-2306128813)
ACK 296def1f6fd936d72d49ce98f9473b4a5d2f9c4b

💬 TheCharlatan commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2352408633)
> I think the use of FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION is a good approach.

Concept ACK

💬 fanquake commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2352412673)
Seen in https://github.com/bitcoin/bitcoin/actions/runs/10851531017/job/30115514734?pr=24123#step:12:242:
```bash
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\socket.py", line 850, in create_connection

sock.connect(sa)

ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it
```

💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2352420906)
@brunoerg @glozow When you get the chance, try this input: [presync_crash.txt](https://github.com/user-attachments/files/17010767/presync_crash.txt)

The assert in the harness should fail with that input after reverting 7ad15d1

💬 maflcko commented on pull request "test: re-bucket p2p_node_network_limited":
(https://github.com/bitcoin/bitcoin/pull/30879#issuecomment-2352422298)
Should feature_assumeutxo also be moved up a bit? According to https://corecheck.dev/tests it is one of the top-20 slowest tests. Also, according to the CI runs in this pull (Asan + macOS) it is in the tail.

💬 hebasto commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2352434497)
> I am not really familiar with GHA, nor with macOS, so it would be good if someone else checked:
>
> * Does the M1 (macOS 14) GHA runner give a better performance?

https://github.com/hebasto/bitcoin/actions/runs/10881199478/job/30189639531

👍 TheCharlatan approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2306175166)
Re-ACK a93c171faa7b4426823466e972c8f24260918bbf

📝 h0z3yn opened a pull request: "Refactor TxStateString Function for Improved Performance and Readability"
(https://github.com/bitcoin/bitcoin/pull/30907)
This pull request refactors the `TxStateString` function in `wallet/transaction.`h to utilize `std::ostringstream` for more efficient string concatenation. This change enhances performance and readability by avoiding multiple string concatenations. The new implementation leverages `std::visit` to handle the `TxState` variant and calls the `toString` method of each state class, while preserving the existing behavior.

This update does not affect other parts of the codebase or its functionality.
...

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2352534736)
Thanks. So it looks like even with an empty cache, the macOS 13 GHA can complete in less than 50 minutes, way less than the 2h timeout. Also, the macOS 14 GHA may complete even faster in less than 40 minutes.

Not sure if this helps to debug this issue. Maybe https://github.com/bitcoin/bitcoin/pull/30866 and https://github.com/bitcoin/bitcoin/pull/30856 are based on an old(er) commit, which may be used by GitHub to run the CI. So I guess, rerunning them via the web interface won't help and a r
...

💬 maflcko commented on pull request "Refactor TxStateString Function for Improved Performance and Readability":
(https://github.com/bitcoin/bitcoin/pull/30907#issuecomment-2352558763)
Closing, as a low-quality (and obviously wrong) LLM generated bot submission.

✅ maflcko closed a pull request: "Refactor TxStateString Function for Improved Performance and Readability"
(https://github.com/bitcoin/bitcoin/pull/30907)

📝 fanquake locked a pull request: "Refactor TxStateString Function for Improved Performance and Readability"
(https://github.com/bitcoin/bitcoin/pull/30907)
This pull request refactors the `TxStateString` function in `wallet/transaction.`h to utilize `std::ostringstream` for more efficient string concatenation. This change enhances performance and readability by avoiding multiple string concatenations. The new implementation leverages `std::visit` to handle the `TxState` variant and calls the `toString` method of each state class, while preserving the existing behavior.

This update does not affect other parts of the codebase or its functionality.
...

💬 maflcko commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2352578910)
Are you still working on this?

💬 maflcko commented on pull request "refactor: policy: Pass kernel::MemPoolOptions to IsStandard[Tx] rather than long list of individual options":
(https://github.com/bitcoin/bitcoin/pull/30232#issuecomment-2352580180)
Are you still working on this?

💬 fanquake commented on pull request "Remove Autotools packages from depends and CI":
(https://github.com/bitcoin/bitcoin/pull/30902#issuecomment-2352582657)
> About to close it in favour of https://github.com/bitcoin/bitcoin/pull/30875.

Mark as draft for now, or close?

💬 maflcko commented on pull request "Remove Autotools packages from depends and CI":
(https://github.com/bitcoin/bitcoin/pull/30902#issuecomment-2352585712)
I think it is fine to do the CI/libtool changes separate. Looks like the other pull is still a draft anyway.

🤔 pablomartin4btc reviewed a pull request: "doc: cmake: prepend "build" to functional/test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/30859#pullrequestreview-2306330141)
ACK 15cc65649b78ffcbd1e1b302c2e4866623cee294

> (Bonus points if the message can be defined in one place, didn't check.)

I agree. `ConfigParser` should not raise an exception if the config file path is invalid?

💬 fanquake commented on pull request "cmake: Switch to crc32c upstream build system":
(https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2352607141)
> A few things still need to be addressed:

This PR is also changing the flags used for code generation. Is that intentional? If yes, would be good to mention in the PR description (and why).

💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2352609290)
Overall, I think when adding a new feature (even if it is "just" CI code), there should be a reason and motivation, along with some evidence that it will be used correctly and meaningfully.

Looking at the comments so far, it looks like the motivation is weak at best, there doesn't seem to be any evidence of a (useful) use-case in any past commit, and it could even promote incorrect usage, leading to issues down the line.