Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 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
💬 maflcko 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-2598785767)
Ok, put into draft for now. I'll rebase after centos 10.
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920481732)
Done
📝 pablomartin4btc converted_to_draft a pull request: "Add new "address type" column to the "receiving tab" address book page"
(https://github.com/bitcoin-core/gui/pull/753)
This PR fixes #646 and introduces some enhancements to the Address Book functionality.

A new "Address Type" column has been added to the address book table page, only visible for the "Receiving address" tab. Users can employ an also new added combo box at the bottom of the page to filter address by their type, this filtering can be combined with the current search line text box.
When the export feature is used, this new field will be included.

![Peek 2023-09-10 19-25](https://github.com/b
...
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2598790040)
I applied the simplification suggested here: https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920454143
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1920485524)
(it snuck back in, but it's now also dropped from `00_setup_env_arm.sh`)
📝 ryanofsky opened a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679)
Currently when "make install" or "cmake --install" are run, various internal binaries that are confusing and not typically useful and installed to `${CMAKE_INSTALL_PREFIX}/bin` and can wind up on the system PATH. This PR moves internal binaries out of `bin/` into `libexec/` where they will still be accessible but will not be automatically placed on the PATH or be confused with more useful binaries. The PR also adds an install rule for the bitcoin-chainstate binary. After this PR binaries install
...
💬 pablomartin4btc commented on pull request "Add new "address type" column to the "receiving tab" address book page":
(https://github.com/bitcoin-core/gui/pull/753#issuecomment-2598800192)
<details>
<summary><B>Drafted</B> while solving the nested filter logic that immediately after rebasing <code>clang-tidy-19</code> was complaining about.</summary>

```
[14:05:17.184] [623/666][21.4s] clang-tidy-19 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/src/qt/addressbookpage.cpp
[14:05:17.184] /ci_container_base/src/qt/addressbookpage.cpp:30:5: error: function 'AddressBookSortFilterProxyModel' is within a re
...
💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2598801543)
So now the macOS native job, without depends, as well as the CentOS job, with depends, run the functional tests using `bitcoin-node`.
💬 ryanofsky commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2598808316)
re: https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2556956641

> It seems better to either not install these binaries or to put them out of the way in libexec/ to avoid confusion. [...] Would like to see more discussion about this and find out where the disagreement is so binaries don't have to move twice, but this doesn't need to block the PR.

I opened #31679 to move internal binaries to libexec, since using libexec isn't directly related to what this PR is trying to do. W
...
💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1920217085)
It's `extern const` rather than `constexpr` because its defined in the cpp file, rather than the header -- for constexpr stuff you need to include its value in the header.
💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1920262478)
This period is probably too low for testnet3 which would have ~20k periods analysed and cached rather than the ~1400 that it should have with the BIP9 recommended period size of 2016.