💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201997480)
> This would give us more confidence to include the new binary in the next official release.
marcofleon, I think your understanding is all correct but note that -ipcbind feature and mining interface are available in v29 and can be easily toggled on in cmake. If you check discussion starting https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3189623427 not having the feature enabled in binary releases and depends builds has seemed to be a barrier to adoption. It do think it might not b
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201997480)
> This would give us more confidence to include the new binary in the next official release.
marcofleon, I think your understanding is all correct but note that -ipcbind feature and mining interface are available in v29 and can be easily toggled on in cmake. If you check discussion starting https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3189623427 not having the feature enabled in binary releases and depends builds has seemed to be a barrier to adoption. It do think it might not b
...
⚠️ CryptoYahan opened an issue: "Request for Review: Somali Translation and Explanation of Bitcoin Whitepaper"
(https://github.com/bitcoin/bitcoin/issues/33221)
We have translated the Bitcoin whitepaper into Somali and have provided a detailed explanation for sections that may be difficult to understand. This translation aims to make Bitcoin more accessible to the Somali-speaking community.
We kindly request that you publish both the Somali translation and its accompanying explanation on your official website to help reach a wider audience. Both documents are fully accurate, and their purpose is purely educational, without ads or marketing.
Here are t
...
(https://github.com/bitcoin/bitcoin/issues/33221)
We have translated the Bitcoin whitepaper into Somali and have provided a detailed explanation for sections that may be difficult to understand. This translation aims to make Bitcoin more accessible to the Somali-speaking community.
We kindly request that you publish both the Somali translation and its accompanying explanation on your official website to help reach a wider audience. Both documents are fully accurate, and their purpose is purely educational, without ads or marketing.
Here are t
...
💬 maflcko commented on pull request "cmake: Fix `-pthread` flags presentation in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-3202050752)
> The best alternative to printing a summary at configure time, is to generate it into a file at generate time with [`file(GENERATE)`](https://cmake.org/cmake/help/latest/command/file.html#generate). This supports generator expressions.
I think there is value in printing a configure summary to stdout, so writing it only to a file could make it harder to quickly double-check if an option or compile flag has been applied.
(https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-3202050752)
> The best alternative to printing a summary at configure time, is to generate it into a file at generate time with [`file(GENERATE)`](https://cmake.org/cmake/help/latest/command/file.html#generate). This supports generator expressions.
I think there is value in printing a configure summary to stdout, so writing it only to a file could make it harder to quickly double-check if an option or compile flag has been applied.
✅ achow101 closed an issue: "Request for Review: Somali Translation and Explanation of Bitcoin Whitepaper"
(https://github.com/bitcoin/bitcoin/issues/33221)
(https://github.com/bitcoin/bitcoin/issues/33221)
💬 achow101 commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202115220)
> What is being enabled here is nominally an optional feature (in the sense that _right now_ all the multiprocess binaries do is add the mining IPC service), but it's really the start of moving towards a multiprocess world, where the new binaries at some point become the "normal" way of using the Bitcoin Core daemon. Certainly at that point, I think ENABLE_IPC should be the default, and we should see capnp as a normal expected dependency, rather than an optional extra feature.
Right, but my t
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202115220)
> What is being enabled here is nominally an optional feature (in the sense that _right now_ all the multiprocess binaries do is add the mining IPC service), but it's really the start of moving towards a multiprocess world, where the new binaries at some point become the "normal" way of using the Bitcoin Core daemon. Certainly at that point, I think ENABLE_IPC should be the default, and we should see capnp as a normal expected dependency, rather than an optional extra feature.
Right, but my t
...
💬 fjahr commented on pull request "wallet: Remove isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3202116489)
re-ACK 620abe985e5150c3151192d08746b7845a69dbbf
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3202116489)
re-ACK 620abe985e5150c3151192d08746b7845a69dbbf
💬 naiyoma commented on pull request "test: rpc: add last block announcement time to getpeerinfo result":
(https://github.com/bitcoin/bitcoin/pull/27052#discussion_r2286253898)
I'm not sure I understand why this is optional. Originally, it was set to a sentinel value.
I don't see a situation where it wouldn't have a value.
My suggestion → `NodeSeconds oldest_block_announcement = NodeSeconds::max();`
That way `.has_value() `checks and `*oldest_block_announcement` can be avoided.
(https://github.com/bitcoin/bitcoin/pull/27052#discussion_r2286253898)
I'm not sure I understand why this is optional. Originally, it was set to a sentinel value.
I don't see a situation where it wouldn't have a value.
My suggestion → `NodeSeconds oldest_block_announcement = NodeSeconds::max();`
That way `.has_value() `checks and `*oldest_block_announcement` can be avoided.
💬 theuni commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202119931)
(Sorry in advance for the meandering comment, I was reading the others coming in as I was typing this up)
> @achow101 @ryanofsky In my [comment](https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3196869655) in the other PR regarding seeing this PR as a successor to that one, I failed the consider the possibility of enabling it in release builds without making it default in from-source builds, like Qt, ZMQ, USDT, so I saw it as two separate sequential decisions to be made, rather than
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202119931)
(Sorry in advance for the meandering comment, I was reading the others coming in as I was typing this up)
> @achow101 @ryanofsky In my [comment](https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3196869655) in the other PR regarding seeing this PR as a successor to that one, I failed the consider the possibility of enabling it in release builds without making it default in from-source builds, like Qt, ZMQ, USDT, so I saw it as two separate sequential decisions to be made, rather than
...
💬 davidgumberg commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2286268690)
The sneaky redownload and insufficient work cases should be differentiated by making the second chain have enough work, e.g.: https://github.com/davidgumberg/bitcoin/commit/1b1b7f2d06e3b11eac0f844002ff4c1bcf497b0d
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2286268690)
The sneaky redownload and insufficient work cases should be differentiated by making the second chain have enough work, e.g.: https://github.com/davidgumberg/bitcoin/commit/1b1b7f2d06e3b11eac0f844002ff4c1bcf497b0d
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202139955)
> Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
No, they will absolutely not see that and it I agree would be a huge problem if that is what were were shipping.
They will see a just one new binary called `bitcoin` which is a wrapper around internal binaries. `bitcoind` is unchanged.
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202139955)
> Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
No, they will absolutely not see that and it I agree would be a huge problem if that is what were were shipping.
They will see a just one new binary called `bitcoin` which is a wrapper around internal binaries. `bitcoind` is unchanged.
💬 sipa commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202140235)
@ryanofsky I wouldn't be opposed to this idea (having `ENABLE_IPC` that enables `-ipcbind` in `bitcoind` itself, separate from `ENABLE_MULTIPROCESS` which enables `bitcoin-node`, and later the broken-out features as separate binaries).
Multiprocess is more than just IPC, and the mining interface isn't really (or doesn't need to be) part of the multiprocess world, it just happens to use the same interface. Furthermore, at this point there really *isn't* any user-visible multiprocess functional
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202140235)
@ryanofsky I wouldn't be opposed to this idea (having `ENABLE_IPC` that enables `-ipcbind` in `bitcoind` itself, separate from `ENABLE_MULTIPROCESS` which enables `bitcoin-node`, and later the broken-out features as separate binaries).
Multiprocess is more than just IPC, and the mining interface isn't really (or doesn't need to be) part of the multiprocess world, it just happens to use the same interface. Furthermore, at this point there really *isn't* any user-visible multiprocess functional
...
💬 theuni commented on pull request "test: rpc: add last block announcement time to getpeerinfo result":
(https://github.com/bitcoin/bitcoin/pull/27052#discussion_r2286284964)
`optional` + `has_value()` communicates intent much more clearly than the sentinel imo. Though admittedly it's awkward as long as worst_peer is a sentinel.
I'd expect to see `std::optional<std::pair<NodeId, NodeSeconds>> worst_peer`, personally.
(https://github.com/bitcoin/bitcoin/pull/27052#discussion_r2286284964)
`optional` + `has_value()` communicates intent much more clearly than the sentinel imo. Though admittedly it's awkward as long as worst_peer is a sentinel.
I'd expect to see `std::optional<std::pair<NodeId, NodeSeconds>> worst_peer`, personally.
💬 theuni commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202198415)
> > Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
>
> No, they will absolutely not see that and it I agree would be a huge problem if that is what were were shipping.
>
> They will see a just one new binary called `bitcoin` which is a wrapper around internal binaries. `bitcoind` is unchanged.
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202198415)
> > Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
>
> No, they will absolutely not see that and it I agree would be a huge problem if that is what were were shipping.
>
> They will see a just one new binary called `bitcoin` which is a wrapper around internal binaries. `bitcoind` is unchanged.
...
📝 sipa opened a pull request: "miner: clamp options instead of asserting"
(https://github.com/bitcoin/bitcoin/pull/33222)
The `BlockAssembler::ClampOptions` function currently doesn't actually clamp most of the provided settings, but asserts that some are in range. This made sense while it was a purely internal interface.
However, with the mining IPC interface exposed in #30510, these options are now externally accessible, and it is not entirely intuitive how to set them. In particular, calling `Mining::createNewBlock` with a default-constructed `BlockCreateOptions` will right now instantly crash the bitcoin nod
...
(https://github.com/bitcoin/bitcoin/pull/33222)
The `BlockAssembler::ClampOptions` function currently doesn't actually clamp most of the provided settings, but asserts that some are in range. This made sense while it was a purely internal interface.
However, with the mining IPC interface exposed in #30510, these options are now externally accessible, and it is not entirely intuitive how to set them. In particular, calling `Mining::createNewBlock` with a default-constructed `BlockCreateOptions` will right now instantly crash the bitcoin nod
...
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202202526)
> I don't think it's helpful to be so dismissive. Shipping new binaries (but not on all platforms), recommending new ways of running Core, and deprecating old ones shouldn't be taken lightly.
I don't think I am being dismissive, but sorry if anything came across that way. I understand there have been a lot of PRs to keep track of and the interactions between them may be confusing, but I feel if anything this approach is more conservative than what sipa is suggesting, and is compatible with hi
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202202526)
> I don't think it's helpful to be so dismissive. Shipping new binaries (but not on all platforms), recommending new ways of running Core, and deprecating old ones shouldn't be taken lightly.
I don't think I am being dismissive, but sorry if anything came across that way. I understand there have been a lot of PRs to keep track of and the interactions between them may be confusing, but I feel if anything this approach is more conservative than what sipa is suggesting, and is compatible with hi
...
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202213363)
> You and I know that, yes, but figuring out what's changed may be non-trivial for a user.
Is it possible to state more concretely what may be confusing here? The new `bitcoin` binary is already enabled in #31375. The only thing this PR affects is the behavior of the -m option, if specified, and the contents of the libexec directory.
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202213363)
> You and I know that, yes, but figuring out what's changed may be non-trivial for a user.
Is it possible to state more concretely what may be confusing here? The new `bitcoin` binary is already enabled in #31375. The only thing this PR affects is the behavior of the -m option, if specified, and the contents of the libexec directory.
💬 theuni commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202236270)
> > I don't think it's helpful to be so dismissive. Shipping new binaries (but not on all platforms), recommending new ways of running Core, and deprecating old ones shouldn't be taken lightly.
>
> I don't think I am being dismissive, but sorry if anything came across that way. I understand there have been a lot of PRs to keep track of and the interactions between them may be confusing, but I feel if anything this approach is more conservative than what sipa is suggesting, and is compatible w
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3202236270)
> > I don't think it's helpful to be so dismissive. Shipping new binaries (but not on all platforms), recommending new ways of running Core, and deprecating old ones shouldn't be taken lightly.
>
> I don't think I am being dismissive, but sorry if anything came across that way. I understand there have been a lot of PRs to keep track of and the interactions between them may be confusing, but I feel if anything this approach is more conservative than what sipa is suggesting, and is compatible w
...
💬 sipa commented on pull request "miner: clamp options instead of asserting":
(https://github.com/bitcoin/bitcoin/pull/33222#issuecomment-3202248904)
This makes sense for v30.0, I think, if we're going to be shipping mining interface support (#31802 or variation thereof).
(https://github.com/bitcoin/bitcoin/pull/33222#issuecomment-3202248904)
This makes sense for v30.0, I think, if we're going to be shipping mining interface support (#31802 or variation thereof).
💬 polespinasa commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3202259011)
> If latency is not very important, there may be a simpler solution: compute the template, and schedule it for sending, but only send it as soon as all transactions in it have been announced.
Maybe a stupid question and I'm not understanding something, but what is the point of this if we have to wait for all transactions to be announced? Isn't the whole idea to make sure our peers know about what we think will be the next block?
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3202259011)
> If latency is not very important, there may be a simpler solution: compute the template, and schedule it for sending, but only send it as soon as all transactions in it have been announced.
Maybe a stupid question and I'm not understanding something, but what is the point of this if we have to wait for all transactions to be announced? Isn't the whole idea to make sure our peers know about what we think will be the next block?
💬 davidgumberg commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2286361853)
It really feels like the default constructor for this should be deleted, but that's not possible because `CChainParams()` default constructs it: https://github.com/bitcoin/bitcoin/blob/53341ea10dc2f7df371b416060863bbc094b8773/src/kernel/chainparams.h#L165
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2286361853)
It really feels like the default constructor for this should be deleted, but that's not possible because `CChainParams()` default constructs it: https://github.com/bitcoin/bitcoin/blob/53341ea10dc2f7df371b416060863bbc094b8773/src/kernel/chainparams.h#L165