💬 instagibbs commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3207511492)
@ismaelsadeeq
> How does this proposal prevent a scenario where an adversary stuff transactions that are harmful to peers, forcing them to validate and save them
There's no need to fully validate these transactions you are given. If they violate your own policy you just won't include them in your own mempool (or include them in a block template).
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3207511492)
@ismaelsadeeq
> How does this proposal prevent a scenario where an adversary stuff transactions that are harmful to peers, forcing them to validate and save them
There's no need to fully validate these transactions you are given. If they violate your own policy you just won't include them in your own mempool (or include them in a block template).
💬 TheBlueMatt commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207513300)
> @TheBlueMatt I agree that having the experience be seamless for miners is the goal. I more meant for the purposes of SRI contributors and maybe a couple of brave miners who are willing to help with testing of Core and SV2, so we can avoid buggy code in releases.
After a few years of work, we're now at a point where I can (with only modest confidence) say that we need the ability to do proper mining job building in Bitcoin Core in the next six months or so, implying it needs to ship in v30 o
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207513300)
> @TheBlueMatt I agree that having the experience be seamless for miners is the goal. I more meant for the purposes of SRI contributors and maybe a couple of brave miners who are willing to help with testing of Core and SV2, so we can avoid buggy code in releases.
After a few years of work, we're now at a point where I can (with only modest confidence) say that we need the ability to do proper mining job building in Bitcoin Core in the next six months or so, implying it needs to ship in v30 o
...
💬 darosior commented on pull request "[29.x] 33106 backport and final changes for rc2":
(https://github.com/bitcoin/bitcoin/pull/33226#issuecomment-3207534823)
Concept ACK. Hopefully this speeds up recovery of block propagation on the network.
-------- Original Message --------
On 8/20/25 3:53 PM, Gloria Zhao wrote:
> Backports [#33106](https://github.com/bitcoin/bitcoin/pull/33106) and includes final changes for 29.1rc2. Built on top of [#33225](https://github.com/bitcoin/bitcoin/pull/33225). I'll rebase shortly after that's in.
>
> I did not include [#32750](https://github.com/bitcoin/bitcoin/pull/32750) because it causes [#33177](https://github.c
...
(https://github.com/bitcoin/bitcoin/pull/33226#issuecomment-3207534823)
Concept ACK. Hopefully this speeds up recovery of block propagation on the network.
-------- Original Message --------
On 8/20/25 3:53 PM, Gloria Zhao wrote:
> Backports [#33106](https://github.com/bitcoin/bitcoin/pull/33106) and includes final changes for 29.1rc2. Built on top of [#33225](https://github.com/bitcoin/bitcoin/pull/33225). I'll rebase shortly after that's in.
>
> I did not include [#32750](https://github.com/bitcoin/bitcoin/pull/32750) because it causes [#33177](https://github.c
...
🤔 janb84 reviewed a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3137965774)
ACK ce7d94a492e61f2a43ea315e75be607d6aa71702
After reading all the comments, I fail to see the risk of the new binaries.
- If you want to play safe, just run `bitcoind` and `bitcoin-cli` as normal.
- if you want to test the more experimental options use the new binaries. eg. `bitcoin` (and you will accept the risks with that)
People who run bitcoin-core binaries are not novices, they will not get confused by some extra binaries (especially if contained in a separate dir). Novices will
...
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3137965774)
ACK ce7d94a492e61f2a43ea315e75be607d6aa71702
After reading all the comments, I fail to see the risk of the new binaries.
- If you want to play safe, just run `bitcoind` and `bitcoin-cli` as normal.
- if you want to test the more experimental options use the new binaries. eg. `bitcoin` (and you will accept the risks with that)
People who run bitcoin-core binaries are not novices, they will not get confused by some extra binaries (especially if contained in a separate dir). Novices will
...
💬 pablomartin4btc commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2289136538)
These were all removed in #32977.
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2289136538)
These were all removed in #32977.
📝 ryanofsky opened a pull request: "multiprocess: Don't require bitcoin -m argument when IPC options are used"
(https://github.com/bitcoin/bitcoin/pull/33229)
sipa and theuni in #31802 pointed out that users shouldn't be exposed to multiprocess implementation details just to use IPC features, so current need to specify the `bitcoin -m` option in conjunction with `-ipcbind` could be seen as a design mistake and not just a usage inconvenience. This is easy to fix by having the `bitcoin` wrapper respect IPC settings. Was planning to do this anyway, but this provides a good excuse to do it now.
(https://github.com/bitcoin/bitcoin/pull/33229)
sipa and theuni in #31802 pointed out that users shouldn't be exposed to multiprocess implementation details just to use IPC features, so current need to specify the `bitcoin -m` option in conjunction with `-ipcbind` could be seen as a design mistake and not just a usage inconvenience. This is easy to fix by having the `bitcoin` wrapper respect IPC settings. Was planning to do this anyway, but this provides a good excuse to do it now.
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208001779)
re: [#31802 (comment)](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206701996)
> > I'm in favor of multiprocess happening when it's ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as "this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, ..."
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208001779)
re: [#31802 (comment)](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206701996)
> > I'm in favor of multiprocess happening when it's ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as "this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, ..."
...
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208041679)
> ### RFM?
I've been keeping my [review summary](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3200849464) comment up to date through this discussion and I am more confident this is ready to merge now, and plan to do this shortly.
I've just added the following new sections since my last update:
>### Review
>- janb84: current ack, high level review with questions and considerations of tradeoffs
> ### Concerns
> #### conflation of different multiprocess features
>
>*
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208041679)
> ### RFM?
I've been keeping my [review summary](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3200849464) comment up to date through this discussion and I am more confident this is ready to merge now, and plan to do this shortly.
I've just added the following new sections since my last update:
>### Review
>- janb84: current ack, high level review with questions and considerations of tradeoffs
> ### Concerns
> #### conflation of different multiprocess features
>
>*
...
💬 achow101 commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208075455)
ACK ce7d94a492e61f2a43ea315e75be607d6aa71702
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208075455)
ACK ce7d94a492e61f2a43ea315e75be607d6aa71702
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3208120136)
Updated 3a34e006918490eb968c7b3312693e8a27fa3fde -> bce88ae28ab2cd12f32aead1fbf47153c50c3b05 ([kernelApi_59](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_59) -> [kernelApi_60](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_60), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_59..kernelApi_60))
* Fixed conflict with #33078 and got rid of the separate `BlockPointer` type.
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3208120136)
Updated 3a34e006918490eb968c7b3312693e8a27fa3fde -> bce88ae28ab2cd12f32aead1fbf47153c50c3b05 ([kernelApi_59](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_59) -> [kernelApi_60](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_60), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_59..kernelApi_60))
* Fixed conflict with #33078 and got rid of the separate `BlockPointer` type.
📝 achow101 opened a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230)
There are some RPCs where the argument can be either JSON that needs to be parsed, or a string that we can pass straight through. However, `bitcoin-cli` would always parse those arguments as JSON which makes for some cumbersome argument passing when using those RPCs. Notably, `hash_or_height` in `getblockstats` and `gettxoutsetinfo` do this, and results in a more cumbersome command of `bitcoin-cli getblockstats '"<hash>"'`. Otherwise, using a normal invocation of `bitcoin-cli getblockstats <hash
...
(https://github.com/bitcoin/bitcoin/pull/33230)
There are some RPCs where the argument can be either JSON that needs to be parsed, or a string that we can pass straight through. However, `bitcoin-cli` would always parse those arguments as JSON which makes for some cumbersome argument passing when using those RPCs. Notably, `hash_or_height` in `getblockstats` and `gettxoutsetinfo` do this, and results in a more cumbersome command of `bitcoin-cli getblockstats '"<hash>"'`. Otherwise, using a normal invocation of `bitcoin-cli getblockstats <hash
...
🤔 marcofleon reviewed a pull request: "[29.x] 33106 backport and final changes for rc2"
(https://github.com/bitcoin/bitcoin/pull/33226#pullrequestreview-3138288017)
ACK 0034dcfba9dc599449e7569ed1b30e58d4f4434f
Went through the commits to confirm they match. The only differences are the addition of the `datacarriersize` argument in `feature_rbf.py` and the additional changes in `mempool_limit.py`. Also ran unit and functional tests on `29.x` branch locally.
(https://github.com/bitcoin/bitcoin/pull/33226#pullrequestreview-3138288017)
ACK 0034dcfba9dc599449e7569ed1b30e58d4f4434f
Went through the commits to confirm they match. The only differences are the addition of the `datacarriersize` argument in `feature_rbf.py` and the additional changes in `mempool_limit.py`. Also ran unit and functional tests on `29.x` branch locally.
💬 davidgumberg commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2289338049)
Ah, I see, thanks.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2289338049)
Ah, I see, thanks.
💬 davidgumberg commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2289342465)
I don't think we should be that worried about a 1 in 2^37 chance of spurious test failure, but we could just double the number of headers if that's a concern.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2289342465)
I don't think we should be that worried about a 1 in 2^37 chance of spurious test failure, but we could just double the number of headers if that's a concern.
🚀 ryanofsky merged a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802)
(https://github.com/bitcoin/bitcoin/pull/31802)
💬 achow101 commented on pull request "rpc: followups for 33106":
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2289350681)
None of the settings have decimals though
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2289350681)
None of the settings have decimals though
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208172294)
Thanks to all who reviewed, tested, and commented on this. This has been very useful and should lead to a lot of good followups. I think all concerns can be addressed in future PRs even if they have not been addressed here and I think there are clear next steps to address all the issues listed in the [review summary](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3200849464)
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208172294)
Thanks to all who reviewed, tested, and commented on this. This has been very useful and should lead to a lot of good followups. I think all concerns can be addressed in future PRs even if they have not been addressed here and I think there are clear next steps to address all the issues listed in the [review summary](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3200849464)
💬 TheCharlatan commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208178263)
Post-merge ACK ce7d94a492e61f2a43ea315e75be607d6aa71702
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3208178263)
Post-merge ACK ce7d94a492e61f2a43ea315e75be607d6aa71702
💬 davidgumberg commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-3208186878)
crACK fa3d88ac35de75af
The test changes here look good to me. I left some nits that can be addressed if following-up or rebasing. I sanity checked that a fresh node doing headers syncing for mainnet, testnet, and signet work on my machine with this branch.
I think it would be good to have input from one of the authors/reviewers of #25717 on the non-test changes to headers sync in this PR, which are https://github.com/bitcoin/bitcoin/pull/32579/commits/4aea5fa3690310bfc2617e3bf31f555dae7d93
...
(https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-3208186878)
crACK fa3d88ac35de75af
The test changes here look good to me. I left some nits that can be addressed if following-up or rebasing. I sanity checked that a fresh node doing headers syncing for mainnet, testnet, and signet work on my machine with this branch.
I think it would be good to have input from one of the authors/reviewers of #25717 on the non-test changes to headers sync in this PR, which are https://github.com/bitcoin/bitcoin/pull/32579/commits/4aea5fa3690310bfc2617e3bf31f555dae7d93
...
💬 ryanofsky commented on pull request "cli: Handle arguments that can be either JSON or string":
(https://github.com/bitcoin/bitcoin/pull/33230#issuecomment-3208194243)
Seems like a reasonable change. I wonder if an alternate solution might have the client treat hash_or_height as a string parameter instead of a JSON parameter, and update the server to accept height strings? But I guess that might not be backwards compatible if there are old scripts calling bitcoin-cli with extra quotes around the hash string.
(https://github.com/bitcoin/bitcoin/pull/33230#issuecomment-3208194243)
Seems like a reasonable change. I wonder if an alternate solution might have the client treat hash_or_height as a string parameter instead of a JSON parameter, and update the server to accept height strings? But I guess that might not be backwards compatible if there are old scripts calling bitcoin-cli with extra quotes around the hash string.