Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 hodlinator commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2326433232)
> > 2. but `BOOST_CHECK` is deprecated
>
> I don't [see the deprecation](https://live.boost.org/doc/libs/1_86_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level.html), I think it's still useful for truthy checks (e.g. empty optionals).

It seems to be poorly documented, but if you search [the header](https://www.boost.org/doc/libs/1_86_0/boost/test/tools/interface.hpp) for "DEPRECATED TOOLS" you'll find it.
💬 jadijadi commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2326443447)
> The test only wants to check the stderr and exit code, so instead of using `ls`, it may be easier to use a python command ([#29868 (comment)](https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1740457415)), or something similar.

Agree with your solution. Python one-liner is cleaner and more reliable. Used that one so we do not need the Locale anymore. tests pass both in PT and C locale.

by the way since you are working on this in a larger scale, I can close my PR and wait for a f
...
💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326448582)
> ... as a follow up it would be good to:
>
> * Update oss-fuzz to match what is being done here? Now that they've diverged.

The OSS-Fuzz script controls all flags explicitly, including https://github.com/google/oss-fuzz/blob/5403c34ff006baf410b94cad1112a6c385d1bade/projects/bitcoin-core/build.sh#L50:
```bash
export CPPFLAGS="-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE"
```
💬 maflcko commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2326450938)
Looks good. Make sure to adjust the pull request description, now that the approach has changed.
💬 fanquake commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326460252)
> The OSS-Fuzz script controls all flags explicitly

Sure, but you changed the CMake build type here, so why not change it in oss-fuzz? Otherwise, it'd be good to explain somewhere why they (should) differ.
💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326491197)
> > The OSS-Fuzz script controls all flags explicitly
>
> Sure, but you changed the CMake build type here, so why not change it in oss-fuzz? Otherwise, it'd be good to explain somewhere why they (should) differ.

Sure. Please consider https://github.com/google/oss-fuzz/pull/12443.
💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2326502478)
Rebased
💬 theStack commented on pull request "qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls":
(https://github.com/bitcoin-core/gui/pull/834#issuecomment-2326505635)
> The log shows "No static plugins."
>
> To verify that static plugins are still being imported, you should build with depends.

Tested that on Ubuntu 23.04 LTS (since for OpenBSD, depends doesn't build QT):
```
2024-09-03T11:47:55Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
2024-09-03T11:47:55Z Qt 5.15.14 (static), plugin=xcb
2024-09-03T11:47:55Z Static plugins:
2024-09-03T11:47:55Z QMinimalIntegrationPlugin, version 331520
2024-09-03T11:47:55Z QXcbIntegrationPlugin
...
👍 hodlinator approved a pull request: "Remove unsafe uint256S() and test-only uint160S()"
(https://github.com/bitcoin/bitcoin/pull/30773#pullrequestreview-2277432402)
re-ACK 9ef049db5ea30f60d0b72714c42c74e2d816c820

`git range-diff master a9d19ae 9ef049d` showed only small expected edits + new commit to move `conversion`-tests.

`ctest -j <cores> --test-dir build` passed.
👍 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
...