Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 willcl-ark commented on issue "ci: Replace wine tests with real tests on Windows?":
(https://github.com/bitcoin/bitcoin/issues/31071#issuecomment-2406956133)
I'm presuming that it may need WSL2 for the build, not the test stage?

If so, we could consider using two GH jobs to split/run this test in two stages; one which does the cross-compile on ubuntu and [saves the build artefacts](https://github.com/actions/upload-artifact), which, if successful, are then [restored](https://github.com/actions/download-artifact) by the second (native windows) job to be executed natively under Windows.

This could prove more tricky to actually execute in practice
...
🚀 fanquake merged a pull request: "contrib: fix typos in check-deps.sh"
(https://github.com/bitcoin/bitcoin/pull/31070)
💬 hebasto commented on issue "ci: Replace wine tests with real tests on Windows?":
(https://github.com/bitcoin/bitcoin/issues/31071#issuecomment-2406960015)
> Ugh, this may require WSL2, which isn't yet available on GHA?

The recent discussion is [here](https://github.com/actions/runner-images/issues/5920).
💬 maflcko commented on pull request "fuzz: fuzz connman with non-empty addrman + ASMap":
(https://github.com/bitcoin/bitcoin/pull/29536#issuecomment-2406968222)
cc @vasild Looks like this conflicts with one of your pulls, so you may be qualified to ack/nack this?
💬 willcl-ark commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407002453)
> RHEL 8 and the RHELatives that base off of this, such as AlmaLinux 8 and Rocky Linux 8.

> I would not be surprised if more than half of Bitcoin Core installations that run on dedicated Linux servers will not be able to update to 28.x as a result of this, without first updating to a new OS.

Do you have any further information on how you conclude that more than half of Bitcoin Core installations run on these OSes (and their derivatives)? This number doesn't feel right to me, even as a (hig
...
💬 l0rinc commented on pull request "Don't zero-after-free `DataStream`: Faster IBD on some configurations":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2407036182)
I have compared the full IBD speed of
* 0449a22bc0: test: avoid BOOST_CHECK_EQUAL for complex types
* 5cf2fefd33: refactor: Drop unused `zero_after_free_allocator`

The added test served as a baseline, dropping the zero fee allocator as the purpose of this PR (I know you've added new commits since, let me know if you think that changes the landscape).

I've used a low, but reasonable 2GB dbcache for the first 800k blocks to measure the underlying database instead of a single final dump wit
...
📝 hebasto converted_to_draft a pull request: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042)
The use of `PACKAGE_NAME` for the project's variable name is problematic, as this name is commonly used in CMake's [interface variables](https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-version-selection). If third-party CMake code handles with scopes improperly, our `PACKAGE_NAME` variable could end up with an unexpected value.

This PR avoids such conflicts by renaming all `PACKAGE_*` variables to `CLIENT_*`.

The code in the master branch works correctly only inci
...
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2407144282)
> developers may still prefer to write the tests in Python going forward, because they are used to it, and because it is more flexible

I agree. This PR does not intend to replace the functional testing framework, but complement it. For example, from C++ one has greater control and observability - the test can read any global or member variable and can call arbitrary functions (repeating https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1068176623).

Also, when used from within C++ o
...
👋 hebasto's pull request is ready for review: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042)
💬 vasild commented on pull request "fuzz: fuzz connman with non-empty addrman + ASMap":
(https://github.com/bitcoin/bitcoin/pull/29536#issuecomment-2407171509)
Concept ACK

The conflict with https://github.com/bitcoin/bitcoin/pull/28584 is only mechanical.

This PR changes "the contents" of the `CConnman` and does not add fuzzing to more connman methods (except `connman.ASMapHealthCheck()`). The main point of this PR is that methods that are fuzzed even before this PR would behave differently if addrman is not empty. Nice.

https://github.com/bitcoin/bitcoin/pull/28584 adds fuzzing to methods not fuzzed before: `OpenNetworkConnection()`, `Create
...
💬 fanquake commented on pull request "build: Add missing USDT header dependency to kernel":
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2407187265)
> Currently depends builds pick up usdt and boost from the same path, and because boost always exists, the usdt path is implicitly included. So without boost, USDT isn't found.

To clarify for myself, the build currently picks up all depends dependencies from the same path (i.e something like `-isystem /root/ci_scratch/depends/x86_64-pc-linux-gnu/include`, (which is what is introduced with the change here)), so using any dependency will implictly include all others. It's just the case that the
...
💬 maflcko commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407190445)
If this really was an issue, adding back the build patches would probably be easier than backporting all bugfixes for years: https://github.com/bitcoin/bitcoin/commit/0c57a798b50bf03a0b69f632c691932d608254ef#diff-42e88f6b2dc3f513ccde184c74d1853a01b2bfc3ee64debf98c52efe316b04c1L456

However, I still don't understand why this is an issue.

Maybe I am missing something obvious, but at least locally I can call `./configure` on the `28.x` branch just fine on Alma8:

```
dnf install gcc-toolset
...
📝 ryanofsky opened a pull request: "scripted-diff: Replace strprintf(Untranslated(...)) with Untranslated(strprintf(...))"
(https://github.com/bitcoin/bitcoin/pull/31072)
This PR is just a scripted-diff replacing strprintf calls like:

```c++
strprintf(Untranslated("Error parsing command line arguments: %s"), error))
```

with:

```c++
Untranslated(strprintf("Error parsing command line arguments: %s", error))
```

Currently code is inconsistent and uses a mix of both styles, but second style is better because:

1. It scans more cleanly. It is easier to read the strprintf call, and easier to think about because the whole string, not just one part, is
...
💬 hebasto commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1796886224)
Thanks! Reworked per feedback.
💬 hebasto commented on pull request "build: Add missing USDT header dependency to kernel":
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2407322051)
> Are you saying you think we shouldn't be able to include system headers when building using depends?

Yes, for headers-only packages built in depends, such as Boost and USDT. t's for the same reason, why we got rid of the `ALLOW_HOST_PACKAGES` variable.
💬 fanquake commented on pull request "build: Add missing USDT header dependency to kernel":
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2407334296)
> Yes, for headers-only packages built in depends, such as Boost and USDT.

Headers-only shouldn't make any difference here, and this should already be the case for Boost. If it's not, that should be fixed. See my point above re USDT, I don't understand how you propose to do this, given that the USDT headers exist amongst the libc headers.
👍 fanquake approved a pull request: "build: Add missing USDT header dependency to kernel"
(https://github.com/bitcoin/bitcoin/pull/30970#pullrequestreview-2362721686)
ACK ccd10fdb97f9b8268a5cd60d7461967cfe536f16
💬 ArmchairCryptologist commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407340807)
> Do you have any further information on how you conclude that more than half of Bitcoin Core installations run on these OSes (and their derivatives)? This number doesn't feel right to me, even as a (high) estimate.
> This report: https://repology.org/project/glibc/badges shows that glibc >=2.31 is supported by:

It is mostly a hunch based on my own experience with the server hosting industry. Most of the distros mentioned here are not generally considered stable for server usage, and are onl
...
🚀 fanquake merged a pull request: "build: Add missing USDT header dependency to kernel"
(https://github.com/bitcoin/bitcoin/pull/30970)
💬 sipa commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407347013)
*If* there is significant demand for running Bitcoin Core on RH8, I think it would make sense for us to consider adding the glibc workarounds necessary so that guix-built binaries support glibc 2.28. I suspect that is easier than backporting fixes to 27.x for longer, and doesn't preclude RH8 users from getting the features added in 28.x and later.

I don't think the possibility of self-compiling on RH8 is a full replacement for project binaries that just work. Presumably people who use RH8 want
...