Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ hebasto commented on pull request "cmake: Fix `-pthread` flags in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#discussion_r1934186962)
> if an upstream change in CMake ever sets different `COMPILE_OPTIONS` for `Threads::Threads` (probably unlikely)...

I would argue the opposite. It’s possible that more cases could be added where the `-pthread` flag should be filtered out using additional generator expressions. The last [modification](https://gitlab.kitware.com/cmake/cmake/-/commit/d7963aa9ee38bf26ab31433f2e7bfaff7ddf6c57) was made two years ago.
πŸ’¬ hebasto commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#issuecomment-2622101449)
> Should be RFM with two acks?
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2622111651)
> However when using the result cmake complains about OpenSSL missing:

Openssh issue you are seeing is puzzling because I don't know what change here could cause it, and I wonder if you see the same error without this PR? When you are building with depends capnproto is built without ssl support should should not be looking for ssl at all.

If you are not seeing this issue happen before this PR, but are seeing this issue happen after this PR, I think it would probably most likely be the chan
...
πŸ’¬ vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2622122439)
> Something else I see using Sockman is in the `mempool_limit` functional test, we call `getrawtransaction` and expect around 540,000 bytes back. The call to `send()` returns the total amount of bytes immediately ...

Hmm, what I observed is that the send call sends less bytes than requested.

_(Lets move the discussion away from the main thread of this PR into the below link)_

Some a more elaborate explanation plus patch that fixes the problem in https://github.com/pinheadmz/bitcoin/comm
...
πŸ‘ darosior approved a pull request: "rpc: have getblocktemplate mintime account for timewarp"
(https://github.com/bitcoin/bitcoin/pull/31600#pullrequestreview-2581619397)
utACK e1676b08f7b0b9a6c8ed76e31f24faa03a3facc9 on the code changes

Could bikeshed the doc but i don't think it's necessary.
πŸ’¬ hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2622149035)
Latest push adds 2347ce8f018b1b9b8d6ef1933a778023fd228b7f which fixes a Windows Python issue [found by CI](https://github.com/bitcoin/bitcoin/actions/runs/13021269281/job/36322263072). It's still a mystery to me why the old test from a previous version of this PR in ee4c9d7eba6ba6fb6dcb7abd539c23f5b62d5991 didn't provoke the same issue on Windows CI ([successful run including that commit](https://github.com/bitcoin/bitcoin/actions/runs/12696631983/job/35391102229#step:12:388)). CI output states
...
πŸ’¬ Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2622173341)
> wonder if you see the same error without this PR?

I tried with master @ ad2f9324c619ae608c434ee85ad9ae1b2812877d and indeed got the same error. So it seems unrelated.
πŸ’¬ ryanofsky commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2622182835)
re: https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2620155964

> I don't think the cons of option 3 are necessary to get its pros. How about simply keeping `bitcoind` and `bitcoin-qt` which would start `-node`, `-wallet` and `-gui` binaries from `libexec`?

Nice idea! This is an interesting hybrid of approaches 2 & 3 and I'll add it as a 4th option in the description.

4. **Minimally combined binaries**: add limited multiprocess functionality to existing `bitcoind` and `bitcoin-qt
...
πŸ’¬ Sjors commented on pull request "Add multiprocess binaries to release build (except Windows, OpenBSD)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2622194575)
#31740 landed, now trying on top #31741.
⚠️ ryanofsky opened an issue: "RFC: multiprocess binaries in 29.0"
(https://github.com/bitcoin/bitcoin/issues/31756)
I want to reopen topic of including multiprocess binaries in [29.0](https://github.com/bitcoin/bitcoin/issues/31029). I'm fine with any decision on this issue, but I was rereading the previous [irc discussion](https://www.erisian.com.au/bitcoin-core-dev/log-2025-01-23.html#l-303) and it struck me as abstract and process-oriented, detached from reality, and likely based on misperceptions of the actual change being discussed.

I think it would be helpful if people who had concerns about including
...
πŸ’¬ Sjors commented on pull request "Add multiprocess binaries to release build (except Windows, OpenBSD)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2622362658)
I dropped eb41f92c0889708045ce0a30c6d0f8d43cecd6d5 if favor of setting `MULTIPROCESS=1` for most depends jobs (9a20dc4f2ebbb4179dfabd6426aa442069649907).

Once https://github.com/bitcoin/bitcoin/pull/31741 lands (and this build succeeds) I'll probably open a fresh PR to avoid confusion.
πŸ’¬ ryanofsky commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2622374479)
Responding to fanquake's [post](https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2610191420) a few minutes minutes before the meeting which was very useful and constructive, but also one sided and I think did not result in a balanced discussion at the meeting.

> Given that merging this PR for `29.x` is going to be discussed further, I wanted to leave a few general thoughts here.
>
> #### The mining interface isn't finished yet?
> Looking at #31098, there are still un-merged changes in
...
πŸ‘ tdb3 approved a pull request: "rpc: have getblocktemplate mintime account for timewarp"
(https://github.com/bitcoin/bitcoin/pull/31600#pullrequestreview-2581759004)
brief code review re ACK e1676b08f7b0b9a6c8ed76e31f24faa03a3facc9

Most significant change observed was to "always apply"
πŸ€” marcofleon reviewed a pull request: "test: fix intermittent timeout in p2p_1p1c_network.py"
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2581769938)
ACK 152a2dcdefa6ec744db5b106d5c0a8c5b8aca416

iiuc, when we disconnect, this clears any 60s requests for parents that node1 has with the initial setup peers. So now there's no chance the `sync_mempools()` timeout is hit before the parent request timeout.
πŸ€” glozow reviewed a pull request: "test: fix intermittent timeout in p2p_1p1c_network.py"
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2581783125)
reACK 152a2dcdefa6ec744db5b106d5c0a8c5b8aca416
πŸ’¬ Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2622421647)
Just saw #31756, but even if multiprocess does make it into the v29 release, it seems better at this point to switch the default in a separate PR. And also to not have the changes here be blocked by ongoing discussion.
πŸ’¬ Sjors commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2622442953)
Sorry about changing the title and scope of #30975 from under you right before you opened this. It now only changes the CI to run multiprocess everywhere and avoids enabling it by default (including it in the release, since this follows what's in depends). But the latter of course is what the main discussion is about.
πŸ’¬ darosior commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2622444828)
> Maybe you can be more concrete and say what negative outcome(s) you would like to avoid?

Sure, i meant i don't see why we should release non-IPC binaries once we have decided to enable multiprocess. It seems it would make us be in this halfway situation of keeping around the bigger binaries with no clear reason nor timeline about when we'd get rid of them.
πŸ’¬ ryanofsky commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2622479743)
> Sure, i meant i don't see why we should release non-IPC binaries once we have decided to enable multiprocess.

Got it. I honestly don't have an opinion about that. I think existing non-ipc binaries should be kept as long as features like adding a Chain IPC interface (#29409) or removing wallet code from `bitcoin-node` process (#10102) are planned. But after that point you could easily drop them, especially if #31375 gained adoption. You could also remove them in phases, like (1) move them to l
...
πŸ’¬ darosior commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2622488749)
I agree the multiprocess project should be finished before we consider enabling it and shipping binaries. I guess that ties into the discussion in #31756 so i'll expand there.