Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 purpleKarrot commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3249833121)
> Further, a lot of the YAML is actually setting things up like caching which is not only essential for reasonable performance but also is vendor specific and so couldn't really exist in reproducible CI scripts either written as they are or in cmake.

I don't see how setting up a cache could require YAML in a way that is not possible to reproduce in the cmake language. I explicitly covered caching in my presentation.
💬 purpleKarrot commented on issue "CI: Cmake warnings should be errors":
(https://github.com/bitcoin/bitcoin/issues/31476#issuecomment-3249855202)
> It is not possible for errors to exist, because pull requests will not be merged when errors exist in the CI.

This is just a matter of configuration. The same mechanism that prevents merging when errors exist could be used to prevent merging when warnings exist.
💬 stickies-v commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3249932464)
> A better approach would be to not override user preferences at all

Thanks for the review. I think I agree. I already tried to improve that behaviour by not overriding `CMAKE_POSITION_INDEPENDENT_CODE` when it was set by the user, but it seems better to not change it at all, as you say.

I've force-pushed to use our existing `core_interface` to propagate `INTERFACE_POSITION_INDEPENDENT_CODE` to all our targets, since I think the current behaviour of using PIE for all targets is sensible. W
...
💬 purpleKarrot commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3249972507)
> I think the current behaviour of using PIE for all targets is sensible

I am not convinced and I really would like to know the rationale for that. Maybe @hebasto or @theuni knows?
💬 mzumsande commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-3249980322)
I see unconditional log messages `[net:warning] pcp: Could not receive response: Connection refused (111)` every 5 minutes (in an environment where PCP is expected to not work), and it's a bit spammy. How about a exponential backoff, similar to how it's done in `torcontrol.cpp`?
💬 darosior commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-3250010277)
Good idea. I got annoyed by it as well, and @instagibbs also mentions it above.
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319596500)
It looks like the maintainer wants to be replaced. Since people have been opening pull requests, hopefully they'll find a replacement.

So far `pycapnp` seems to work fine up to Python 3.12 and we support a minimum of 3.10, so we have some time.
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319606567)
Right, we use _enable_ in cmake to decide if we want to _compile_. The test suite care about whether it was compiled.
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319612545)
I don't think we can remove this, because the `import` has to be at the top of the file and the `skip_if_no_py_capnp` happens later.
💬 Sjors commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3250085705)
I'm currently trying a time-machine at `f3e74ef25d479ed77f7cc029d312425b7776c3b0` (`gnu: clisp: Actually fix failing test`, see https://lists.gnu.org/archive/html/guix-patches/2025-05/msg00088.html - page currently doesn't load for me). I'll check if that gets me past the test failure and produces working binaries.

I still don't know how to install an individual stubborn package like this from a substitute server, maybe @dongcarl does?
💬 ajtowns commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3250125812)
Backport at https://github.com/ajtowns/bitcoin/tree/202509-29x-backport-compactblock ; don't think it's urgent enough to try pushing for 29.2 though?
⚠️ sixtuselemi73-dev opened an issue: "It may be some router setting to enable it, the more secure routers have these kind of protocols disabled by default. Or maybe it doesn't support it. There isn't any reply, not even 2 NOT_AUTHORIZED (but nice to see the retry code is working 😄 )."
(https://github.com/bitcoin/bitcoin/issues/33295)
It may be some router setting to enable it, the more secure routers have these kind of protocols disabled by default. Or maybe it doesn't support it. There isn't any reply, not even 2 NOT_AUTHORIZED (but nice to see the retry code is working 😄 ).

_Originally posted by @laanwj in https://github.com/bitcoin/bitcoin/issues/30005#issuecomment-2085190180_
💬 w0xlt commented on issue "GUI (?): Copying output from console causes large mem usage/OOM":
(https://github.com/bitcoin/bitcoin/issues/33285#issuecomment-3250166108)
On debug build, it seems to use all available memory.
Is it something related to Qt?
🤔 glozow reviewed a pull request: "Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3181193400)
In the "Still to do" section of the PR description, it seems like most can be checked off other than

> Updating wallet code to be cluster-aware (including mini_miner and coin selection)
> Rework many of our functional tests to be cluster-aware
> Figure out what package validation and package RBF rules should be in this design
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319357837)
5bc5edf8743408979add1b15d23cc8ab7b948db Checking limits using changeset is so nice. It could be helpful to add a comment that `changeset` is automatically destroyed at the end of this function and thus doesn't need to be handled/aborted.
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319334033)
e72b0b1a7eb31078c998d95b4cfffe9cd1f9113d re-adds `-addrmantest` which was removed in #29007
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319516863)
nit in aa8eb8c3ab1 Fix miniminer_tests to work with cluster limits, I think it's txp0 to txp31 and txc0 to txc30?
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319578206)
Perhaps also add `(maximum: %u), MAX_CLUSTER_COUNT_LIMIT`
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319395594)
For f172ac5e6c37f446bc3c38419cf40f3adac3ddd8 Use cluster linearization for transaction relay sort order

In addition to changing the inv order, should we also use the transaction's chunk feerate for fee filtering?