👍 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.
(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
...
(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.
(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
...
(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.
(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
...
(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.
(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
...
(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"
(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.
(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
(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.
(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.
(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.
(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
...
(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.
(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.
📝 furszy opened a pull request: "wallet: fix crash on double block disconnection"
(https://github.com/bitcoin/bitcoin/pull/31757)
Before detailing the issue:
This crash should not occur in production, as block events are queued and processed in
order by the validation signal workers. This PR is just fixing brittle code.
Issue:
The wallet crashes if it processes the same block disconnection event twice in a row due
to an incompatible coinbase transaction state.
This happens because `disconnectBlock` provides `TxStateInactive` without the "abandoned"
flag for coinbase transactions to `SyncTransaction`, while `AddToW
...
(https://github.com/bitcoin/bitcoin/pull/31757)
Before detailing the issue:
This crash should not occur in production, as block events are queued and processed in
order by the validation signal workers. This PR is just fixing brittle code.
Issue:
The wallet crashes if it processes the same block disconnection event twice in a row due
to an incompatible coinbase transaction state.
This happens because `disconnectBlock` provides `TxStateInactive` without the "abandoned"
flag for coinbase transactions to `SyncTransaction`, while `AddToW
...
📝 sr-gi opened a pull request: "test: deduplicates p2p_tx_download constants"
(https://github.com/bitcoin/bitcoin/pull/31758)
Some of the networking constants defined in p2p_tx_download.py are more generally defined in p2p.py
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience o
...
(https://github.com/bitcoin/bitcoin/pull/31758)
Some of the networking constants defined in p2p_tx_download.py are more generally defined in p2p.py
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience o
...
💬 darosior commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2622618673)
I'm not sure i'm the most legitimate to give my opinion here, but since you asked me explicitly i will share.
First of all why is the default position that we should ship a new binary on short notice right before feature freeze, and people who are not comfortable with this should argue against it? It seems backward to me. The default should be business as usual and if people want to introduce new binaries for the project to release, they should be arguing for it.
If you start from this default
...
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2622618673)
I'm not sure i'm the most legitimate to give my opinion here, but since you asked me explicitly i will share.
First of all why is the default position that we should ship a new binary on short notice right before feature freeze, and people who are not comfortable with this should argue against it? It seems backward to me. The default should be business as usual and if people want to introduce new binaries for the project to release, they should be arguing for it.
If you start from this default
...
📝 sr-gi opened a pull request: "test: fixes p2p_ibd_txrelay wait time"
(https://github.com/bitcoin/bitcoin/pull/31759)
`p2p_ibd_txrelay` expects no GETDATA to have been received by a peer after announcing a transaction. The reason is that the node is doing IBD, so transaction requests are not replied to. However, the way this is checked is wrong, and the check will pass even if the node **was not** in IBD.
This is due to the mocktime not being properly initialized, so the check is always performed earlier than it should, making it impossible for the request to be there.
This can be checked by modifying the
...
(https://github.com/bitcoin/bitcoin/pull/31759)
`p2p_ibd_txrelay` expects no GETDATA to have been received by a peer after announcing a transaction. The reason is that the node is doing IBD, so transaction requests are not replied to. However, the way this is checked is wrong, and the check will pass even if the node **was not** in IBD.
This is due to the mocktime not being properly initialized, so the check is always performed earlier than it should, making it impossible for the request to be there.
This can be checked by modifying the
...