Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 hodlinator approved a pull request: "test: Pin and document TEST_DIR_PATH_ELEMENT, SeedRand::FIXED_SEED"
(https://github.com/bitcoin/bitcoin/pull/30748#pullrequestreview-2277469893)
ACK fa84f9decd224ea1c25dc7095bad70a48fa1a534

`git range-diff master fa9c3aec fa84f9de`

Only one minor change (that I suggested) since [prior review](https://github.com/bitcoin/bitcoin/pull/30748#pullrequestreview-2268619255).

Passed `ctest -j <cores> --test-dir build`.
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2326565169)
rebased, addresses some of @maflcko's nits and https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1741287188
vostrnad closed an issue: "IBD performance regression in 27.0rc1 on Windows"
(https://github.com/bitcoin/bitcoin/issues/29785)
💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2326596592)
After seeing the same high variance in 27.0 and 27.1, it seems to have gone away in 28.0rc1. I'll close the issue for now and will look out for future regressions.

Last set of graphs for your enjoyment:

27.0:
![27.0](https://github.com/user-attachments/assets/01eb27b3-768d-4a18-b356-8a292316dc56)

27.1:
![27.1](https://github.com/user-attachments/assets/52ab5547-56c5-48d3-b4de-22af43a84529)

28.0rc1:
![28.0rc1](https://github.com/user-attachments/assets/080ad441-ad5b-4ca8-af42-dca34
...
💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326621951)
Also see https://cirrus-ci.com/task/6221640676147200.
💬 hodlinator 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-2326628987)
Regarding process start issues, would there be any support for me attempting to integrate [Microsoft SysInternals ProcDump](https://learn.microsoft.com/en-us/sysinternals/downloads/procdump) into CI & test framework to be able to call `procdump -ma <PID>` on the `bitcoind.exe` process and hopefully figure out where the process is stalling?
👍 danielabrozzoni approved a pull request: "kernel: Use spans instead of vectors for passing block headers to validation functions"
(https://github.com/bitcoin/bitcoin/pull/30742#pullrequestreview-2277579415)
ACK a2955f09792b6232f3a45aa44a498b466279a8b7
💬 l0rinc commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#issuecomment-2326639869)
reACK 9ef049db5ea30f60d0b72714c42c74e2d816c820
📝 maflcko opened a pull request: "test: Use std::span and std::string_view for raw data"
(https://github.com/bitcoin/bitcoin/pull/30796)
The build system converts raw data into a C++ header file for tests.

This change modernizes the code to use the convenience wrappers `std::span` and `std::string_view`, so that redundant copies can be avoided.
💬 hebasto commented on pull request "test: Use std::span and std::string_view for raw data":
(https://github.com/bitcoin/bitcoin/pull/30796#issuecomment-2326652638)
Concept ACK.
⚠️ maflcko opened an issue: "intermittent issue in wallet_keypool.py: assert_equal(nodes[0].getwalletinfo()["unlocked_until"], 0) AssertionError: not(1725366101 == 0)"
(https://github.com/bitcoin/bitcoin/issues/30797)
https://cirrus-ci.com/task/5492198311985152?logs=ci#L4100


```
node0 2024-09-03T12:21:40.411672Z [httpworker.0] [src/rpc/server.cpp:586] [RPCRunLater] [rpc] queue run of timer lockwallet(default_wallet) in 1 seconds (using HTTP)
node0 2024-09-03T12:21:40.416379Z [http] [src/httpserver.cpp:304] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:34622
node0 2024-09-03T12:21:40.416429Z [httpworker.1] [src/rpc/request.cpp:232] [parse] [rpc] ThreadRPCServer method=keypo
...
💬 maflcko commented on pull request "kernel: Use spans instead of vectors for passing block headers to validation functions":
(https://github.com/bitcoin/bitcoin/pull/30742#issuecomment-2326683029)
CI failure looks unrelated and can be ignored: https://github.com/bitcoin/bitcoin/issues/30797
💬 maflcko 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-2326696273)
> call `procdump -ma <PID>` on the `bitcoind.exe` process and hopefully figure out where the process is stalling?

Is it clear that the issue is Bitcoin Core? IIRC it may also be an issue in the Python subprocess module, no?
💬 maflcko 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-2326700782)
Seems fine to integrate in a pull request and then keep the pull request unmerged, and only merge the pull request in the GHA CI, as that is the only place where it reproduces. Adding temporary test test-only code for everyone else may be a bit too much.
💬 maflcko commented on pull request "test: Use std::span and std::string_view for raw data":
(https://github.com/bitcoin/bitcoin/pull/30796#issuecomment-2326707221)
On my system the test_bitcoin and bench_bitcoin binaries are smaller by 30kB, and 4kB respectively. But that isn't the goal of this pull, just a nice side-effect.
💬 fjahr commented on pull request "test: Use std::span and std::string_view for raw data":
(https://github.com/bitcoin/bitcoin/pull/30796#issuecomment-2326717439)
Concept ACK
💬 hodlinator 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-2326719252)
Alright, I can live with having it a pending PR until it has proven itself useful on CI. :)

Will see if I can get a Windows/Wine environment going locally and study how to integrate it into GHA.
🤔 glozow reviewed a pull request: "test: add check that too large txs aren't put into orphanage"
(https://github.com/bitcoin/bitcoin/pull/30784#pullrequestreview-2277680444)
Concept ACK, but this should be a unit test, not a functional test looking for logs. We could accidentally delete the `return false;` line but keep the `LogPrint` line, and this test would pass.

> so this is only relevant if tx standardness rules are disabled via -acceptnonstdtxns=1.

Similarly, it's a bit icky to have to disable standardness checks to test this.

A unit test in orphanage_tests.cpp could directly create a large tx and call `AddTx`. With #30110, we can also test by callin
...
tdb3 closed a pull request: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793)
💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2326728575)
Thought more about this and other than increased visibility and monitoring of the orphanage over time (e.g. for statistical purposes), I'm not seeing a ton of compelling reasons to add. Will close for now.