Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 willcl-ark commented on pull request "test: re-bucket p2p_node_network_limited":
(https://github.com/bitcoin/bitcoin/pull/30879#issuecomment-2352281417)
> Nit: May be good to reword the pull request description to say that this better reflects the reality in CI runs, and may even speed them up. The mention of "CI timeouts" could be removed, because adding or removing 200 seconds from a run shouldn't be a fix (or cause) of a timeout. Any run that takes longer than half of the overall timeout value is a warning that should be investigated and fixed, because the CI timeout is really only there to catch cases where there is a true timeout (zombie pr
...
💬 maflcko commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#discussion_r1760728552)
not sure this is going in the right direction. The namespace just happens to be a single value for all raw files. However, in theory, one could imagine several raw files having a different namespace each. This would then again increase the verbosity and complexity.
💬 fanquake commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#issuecomment-2352316231)
> Resolves https://github.com/bitcoin/bitcoin/issues/30876.

Not sure it does, as that issue applies to all dependencies, not just ZMQ.
💬 maflcko commented on pull request "kernel: Move background load thread to node context":
(https://github.com/bitcoin/bitcoin/pull/30896#discussion_r1760748575)
I think the doxygen links in this doc are dead anyway, or at least stale. (At least I can't remember when they last worked and were up-to-date).
💬 fanquake commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2352325167)
Seems like you missed some review comments?
https://github.com/bitcoin/bitcoin/pull/30861/files#r1752126150
https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752127638

Seems like https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752135299 is now also more relevant given #30905, so it'd be good to know how this is going to be handed going forward.
fanquake closed an issue: "translations: `28.x` update pulled in random strings?"
(https://github.com/bitcoin/bitcoin/issues/30897)
🚀 fanquake merged a pull request: "qt: Translations update"
(https://github.com/bitcoin/bitcoin/pull/30899)
💬 hebasto commented on pull request "cmake: Add `FindZeroMQ` module":
(https://github.com/bitcoin/bitcoin/pull/30903#issuecomment-2352358227)
> > Resolves #30876.
>
> Not sure it does, as that issue applies to all dependencies, not just ZMQ.

Clarified.
💬 hebasto commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2352376552)
> I am not really familiar with GHA, nor with macOS, so it would be good if someone else checked:
>
> * Does the macOS 13 GHA task succeed at all with an empty cache?

https://github.com/hebasto/bitcoin/actions/runs/10880757045
👋 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.