Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 fanquake commented on pull request "Update leveldb subtree to latest upstream":
(https://github.com/bitcoin/bitcoin/pull/31671#issuecomment-2598610152)
Guix Build (aarch64):
```bash
139048cf62397ec98d6863fc1706a3c7c2afd02307873ec9a9bd707c531283a6 guix-build-910a11fa6630/output/aarch64-linux-gnu/SHA256SUMS.part
7d2de717c990cbc3f7da29291e8de257e610a5a79b6a351f00b9c9cad4e8ed65 guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu-debug.tar.gz
c9b45a7c734d6cba6f736cbc53cea1dd207600133eaa2e82f58fa4dc96bd21c9 guix-build-910a11fa6630/output/aarch64-linux-gnu/bitcoin-910a11fa6630-aarch64-linux-gnu.tar.gz
8c11bb
...
💬 fanquake commented on pull request "ci: Enable DEBUG=1 for one GCC-12+ build to catch 117966 regressions":
(https://github.com/bitcoin/bitcoin/pull/31522#issuecomment-2598614934)
https://cirrus-ci.com/task/5812783272427520?logs=ci#L4621:
```bash
[15:22:49.280] [ 86%] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/float.cpp.o
[15:22:49.281] cd /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz && /usr/bin/ccache /opt/rh/gcc-toolset-12/root/usr/bin/g++ -m64 -DABORT_ON_FAILED_ASSUME -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DDEBUG -DDEBUG_LOCKCONTENTION -DDEBUG_LOCKORDER -DRPC_DOC_CHECK -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_HARDE
...
💬 furszy commented on pull request "descriptor: check whitespace in pubkeys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#discussion_r1920362724)
nit: Because there is a `return` statement in the if block that is above, you could remove the `else` and use `str.front()` instead of `str[0]`.
💬 TheCharlatan commented on pull request "depends: Qt 5.15.16":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2598620293)
Guix build (aarch64):
```
e80422975315244b3365bf9eddb9e6e5d9da08f4b1a5fba34430c6f3021c6b8a guix-build-31a0e5f0905b/output/aarch64-linux-gnu/SHA256SUMS.part
db53cd0057bf0048534e3daea2e39130cdf64a140becaa5c6ff4f98fdfa5936a guix-build-31a0e5f0905b/output/aarch64-linux-gnu/bitcoin-31a0e5f0905b-aarch64-linux-gnu-debug.tar.gz
fa061afbd1dac25c739a63a1644f2af1ad5dab36ea5e4245a7b0bb9008ca4f79 guix-build-31a0e5f0905b/output/aarch64-linux-gnu/bitcoin-31a0e5f0905b-aarch64-linux-gnu.tar.gz
f286a7442a
...
🤔 hebasto reviewed a pull request: "depends: Qt 5.15.16"
(https://github.com/bitcoin/bitcoin/pull/30774#pullrequestreview-2559349957)
Approach ACK 31a0e5f0905bfc6b22ceaaeca53466dfd74967ab.

I've verified changes in `patches/qt`, and they look correct.

Building...
🤔 ryanofsky requested changes to a pull request: "Add multiprocess binaries to release build (except Windows)"
(https://github.com/bitcoin/bitcoin/pull/30975#pullrequestreview-2559268555)
Code review 9a90f964a3c4fe75477926d34b7f1813b144449d.

It looks like latest depends changes are not actually enabling multiprocess binaries anymore due to a referencing a bad `$(multiprocess_$(host_os)_packages)` variable that should no longer include $(host_os). Also it is concerning that there doesn't seem to be any test coverage for this because in current version of this multiprocess binaries are only used in functional tests in a non-depends build. There's no depends build using multiproc
...
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920338053)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)

OpenBSB typo
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920354544)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)

Would suggest reverting this change to simplify. Requires also reverting the `ifeq ($(multiprocess_packages_),)` change below.
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920355534)
In commit "build: depends makes libmultiprocess by default" (https://github.com/bitcoin/bitcoin/commit/48268facc9bb4f3e4f98cff5c21e1fe490b232e2)

Would suggest reverting this change to simplify the PR.
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920385465)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)

This isn't actually doing anything because RUN_FUNCTIONAL_TESTS is false above
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920342619)
In commit "build: depends makes libmultiprocess by default" (48268facc9bb4f3e4f98cff5c21e1fe490b232e2)

It doesn't seem right to include host_os in these variables anymore. I think this is is causing multiprocess packages not to be built right now. If I run `make print-multiprocess_packages_` the variable is empty and the toolchain file does not seem to have multiprocess enabled. But if I drop host_os these things are fixed:

```diff
-multiprocess_packages_$(NO_MULTIPROCESS) = $(multiproces
...
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920426923)
Indeed, I overlooked that.
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920431102)
I'm confused. What should depends/Makefile look like?

I'll push a new version with just the above _host_ bugfix for now.
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2598732399)
> There's no depends build using multiprocess binaries in functional tests.

I added `BITCOIND=bitcoin-node` to "CentOS, dash, gui". So after the _host_ bug fix (https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920342619), the it should use depends, should include multiprocess and run the functional tests.
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920450203)
Moved to "CentOS, dash, gui".
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920454143)
> I'm confused. What should depends/Makefile look like?

I think the only changes that need to be made to this file in this PR are:

```diff
--- a/depends/Makefile
+++ b/depends/Makefile
@@ -41,7 +41,10 @@ NO_SQLITE ?=
NO_WALLET ?=
NO_ZMQ ?=
NO_USDT ?=
-MULTIPROCESS ?=
+# Default NO_MULTIPROCESS value is 1 on unsupported host platforms:
+# - Windows
+# - OpenBSB: https://github.com/capnproto/capnproto/pull/1907
+NO_MULTIPROCESS ?= $(if $(findstring mingw32,$(HOST))$(findstring
...
💬 mzumsande commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1920457435)
Right, with`ProcessOrphanTx()` doing at most one actual validation, it would probably not be good to call it in a loop until all outstanding work is done. Maybe calling it just once (as if it was this peer's next turn) would already help in most cases, but to find out it'd probably be best to get some empirical data from mainnet.
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2598764171)
@fanquake IMO, monitoring CPU usage is useful on its own, even if we don't start treating peers differently based on that metric (https://github.com/bitcoin/bitcoin/issues/31033). In other words, this metric is useful at least as much as a bunch of other metrics in the `getpeerinfo` output.

Yes, I have been running this locally - there is like 65x difference between the least and most demanding peers. I am curious to correlate this to the messages being sent/received to/from those peers and t
...
📝 maflcko converted_to_draft a pull request: "ci: Enable DEBUG=1 for one GCC-12+ build to catch 117966 regressions"
(https://github.com/bitcoin/bitcoin/pull/31522)
It is possible that someone accidentally removes the workaround in fa9e0489f57968945d54ef56b275f51540f3e5e4, or more likely that someone accidentally adds new code without the workaround.

Avoid this by adding a temporary CI check.

This can be tested by reverting the workaround and observing a failure.
👍 TheCharlatan approved a pull request: "ci: optionally use local docker build cache"
(https://github.com/bitcoin/bitcoin/pull/31545#pullrequestreview-2559492845)
tACK e87429a2d0f23eb59526d335844fa5ff5b50b21f