💬 hebasto commented on pull request "doc: Fix typo":
(https://github.com/bitcoin/bitcoin/pull/32472#issuecomment-2873177104)
> A translator on Transifex noticed:
>
> > This is the only label which has two dots: ..
> > Usually we see the elipsis (…)
Reported by @jesterhodl.
(https://github.com/bitcoin/bitcoin/pull/32472#issuecomment-2873177104)
> A translator on Transifex noticed:
>
> > This is the only label which has two dots: ..
> > Usually we see the elipsis (…)
Reported by @jesterhodl.
💬 supertestnet commented on pull request "rpc: generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2873215557)
Here is my motivation:
When mining a block on regtest, the command "generatetoaddress" is available if you want to send the entire coinbase to 1 address. Let's add generatetomany in case you want to split up the coinbase among multiple addresses, similar to how the "sendtoaddress" and "sendmany" commands both exist and let you send money to (1) one address or (2) multiple addresses. The generatetomany command would be particularly useful for simulating protocols that send money to multiple re
...
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2873215557)
Here is my motivation:
When mining a block on regtest, the command "generatetoaddress" is available if you want to send the entire coinbase to 1 address. Let's add generatetomany in case you want to split up the coinbase among multiple addresses, similar to how the "sendtoaddress" and "sendmany" commands both exist and let you send money to (1) one address or (2) multiple addresses. The generatetomany command would be particularly useful for simulating protocols that send money to multiple re
...
💬 andrewtoth commented on pull request "rpc: generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2873256932)
> Still not sure about this, would like to know more opinions, for other things like `send` we have multiple RPCs
Since I don't seem to have convinced you, here are some more arguments for why this feature belongs in `generateblock` instead of a new RPC:
- New RPCs require adding more code to test, and once added must go through a deprecation cycle to be removed (if they can be removed at all without disrupting too many users).
- Consumers of the RPCs must write additional code for the ne
...
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2873256932)
> Still not sure about this, would like to know more opinions, for other things like `send` we have multiple RPCs
Since I don't seem to have convinced you, here are some more arguments for why this feature belongs in `generateblock` instead of a new RPC:
- New RPCs require adding more code to test, and once added must go through a deprecation cycle to be removed (if they can be removed at all without disrupting too many users).
- Consumers of the RPCs must write additional code for the ne
...
💬 aviv57 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873261060)
Concept ACK, if this won't be merged, actors will mine their non-standard transactions by contacting the miners directly and not via the p2p network.
in this way anyone willing to change their own mempool policy is still able to do it
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873261060)
Concept ACK, if this won't be merged, actors will mine their non-standard transactions by contacting the miners directly and not via the p2p network.
in this way anyone willing to change their own mempool policy is still able to do it
💬 chrisguida commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873262276)
Concept NACK, same rationale as https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2833932824
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873262276)
Concept NACK, same rationale as https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2833932824
💬 brunoerg commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2085079203)
Couldn't you use `ConsumeDeserializable` to create a `CBlock`? I think it might simplify the way you're creating the block and the transactions.
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2085079203)
Couldn't you use `ConsumeDeserializable` to create a `CBlock`? I think it might simplify the way you're creating the block and the transactions.
💬 donaldevinev1 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873306021)
Concept NACK, by removing the per-output cap and switching to a cumulative budget model, you're increasing the degrees of freedom in how data can be embedded in transactions. Nodes will have to sum all OP_RETURN output sizes to determine policy compliance, rather than applying a simple size check per output. This adds a non-trivial computational cost during mempool admission. Even if the cumulative size is bounded multiple small OP_RETURN outputs cause higher indexing and management overhead in
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873306021)
Concept NACK, by removing the per-output cap and switching to a cumulative budget model, you're increasing the degrees of freedom in how data can be embedded in transactions. Nodes will have to sum all OP_RETURN output sizes to determine policy compliance, rather than applying a simple size check per output. This adds a non-trivial computational cost during mempool admission. Even if the cumulative size is bounded multiple small OP_RETURN outputs cause higher indexing and management overhead in
...
🤔 mzumsande reviewed a pull request: "wallet: Ensure best block matches wallet scan state"
(https://github.com/bitcoin/bitcoin/pull/30221#pullrequestreview-2833838561)
Code Review ACK f1f254f6c9d3cead617300367442c1bbb449af7c
(nits are not important)
(https://github.com/bitcoin/bitcoin/pull/30221#pullrequestreview-2833838561)
Code Review ACK f1f254f6c9d3cead617300367442c1bbb449af7c
(nits are not important)
💬 mzumsande commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2085051375)
nit: could save a little bit of duplicated code by having `SetLastBlockProcessed()` call `WriteBestBlock()` (which means that the locking would have to happen in `RemoveWallet()`.
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2085051375)
nit: could save a little bit of duplicated code by having `SetLastBlockProcessed()` call `WriteBestBlock()` (which means that the locking would have to happen in `RemoveWallet()`.
💬 mzumsande commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2085053587)
nit: could also call just introduce and use `WriteBestBlock()` here since `SetLastBlockProcessedInMem` is already called above.
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2085053587)
nit: could also call just introduce and use `WriteBestBlock()` here since `SetLastBlockProcessedInMem` is already called above.
🤔 l0rinc reviewed a pull request: "doc: Fix typo"
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833955701)
If we're already tackling this, we could fix some of these as well (some are less important than others, but at least we'd be consistent):
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_miniscript_decaying_multisig_descriptor_psbt.py#L128
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_reindex.py#L86
* https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L238
* https://github.com/bitcoin/bitcoin/blob/master/src/test/bip32_test
...
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833955701)
If we're already tackling this, we could fix some of these as well (some are less important than others, but at least we'd be consistent):
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_miniscript_decaying_multisig_descriptor_psbt.py#L128
* https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_reindex.py#L86
* https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L238
* https://github.com/bitcoin/bitcoin/blob/master/src/test/bip32_test
...
🤔 janb84 reviewed a pull request: "doc: Fix typo"
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833964876)
good find, ACK https://github.com/bitcoin/bitcoin/pull/32472/commits/d847e17c9656020de9f378ae82ec89e0bb39ecbd
- "code" review ✅
- quick search for more, could not find others ✅
(https://github.com/bitcoin/bitcoin/pull/32472#pullrequestreview-2833964876)
good find, ACK https://github.com/bitcoin/bitcoin/pull/32472/commits/d847e17c9656020de9f378ae82ec89e0bb39ecbd
- "code" review ✅
- quick search for more, could not find others ✅
💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873422307)
@donaldevinev1 (just in case this isn't an LLM answer) there are no quadratics introduced here, downstream parsers doing bizarre things is not in scope for this PR.
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873422307)
@donaldevinev1 (just in case this isn't an LLM answer) there are no quadratics introduced here, downstream parsers doing bizarre things is not in scope for this PR.
📝 sipa opened a pull request: "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts"
(https://github.com/bitcoin/bitcoin/pull/32473)
This introduces a per-txin cache for sighash midstate computation to the script interpreter for legacy (bare), P2SH, P2WSH, and (as collateral effect, but not actually useful) P2WPKH. This reduces the impact of certain types of quadratic hashing attacks that use standard transactions. It is not known to improve the situation for attacks involving non-standard transaction attacks.
The cache works by remembering for each of the 6 sighash modes a `(scriptCode, midstate)` tuple, which gives a mid
...
(https://github.com/bitcoin/bitcoin/pull/32473)
This introduces a per-txin cache for sighash midstate computation to the script interpreter for legacy (bare), P2SH, P2WSH, and (as collateral effect, but not actually useful) P2WPKH. This reduces the impact of certain types of quadratic hashing attacks that use standard transactions. It is not known to improve the situation for attacks involving non-standard transaction attacks.
The cache works by remembering for each of the 6 sighash modes a `(scriptCode, midstate)` tuple, which gives a mid
...
💬 stonecutter-group commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2873462743)
At first glance, the proposal to remove the 80-byte `OP_RETURN` limit may seem like a harmless concession for convenience. But in protocol design, as in sovereign systems, limits are not flaws—they are the foundation of order. This change, therefore, warrants careful consideration as it directly impacts the foundational design principles and long-term integrity of the Bitcoin protocol.
The arguments against such a modification are rooted in these core tenets:
1. **Bitcoin's Defined Purpos
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2873462743)
At first glance, the proposal to remove the 80-byte `OP_RETURN` limit may seem like a harmless concession for convenience. But in protocol design, as in sovereign systems, limits are not flaws—they are the foundation of order. This change, therefore, warrants careful consideration as it directly impacts the foundational design principles and long-term integrity of the Bitcoin protocol.
The arguments against such a modification are rooted in these core tenets:
1. **Bitcoin's Defined Purpos
...
💬 darosior commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2085182810)
Should it be named `default_scriptcode_wsh` to be explicit that it implements the Segwit v0 (and not legacy) logic with regard to `OP_CODESEPARATOR` serialization?
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2085182810)
Should it be named `default_scriptcode_wsh` to be explicit that it implements the Segwit v0 (and not legacy) logic with regard to `OP_CODESEPARATOR` serialization?
💬 copy2018 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873505859)
Concept NACK
Relaxing the datacarriersize limit may seem appealing for certain niche use cases, but it comes with significant trade-offs that contradict Bitcoin’s fundamental goals.
First, there is no **pressing** indication that this relaxation is necessary at the current time. The current limit strikes a balance between enabling modest data-carrier use cases and protecting the network's efficiency, decentralization, and sustainability. Introducing unnecessary changes risks opening the do
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873505859)
Concept NACK
Relaxing the datacarriersize limit may seem appealing for certain niche use cases, but it comes with significant trade-offs that contradict Bitcoin’s fundamental goals.
First, there is no **pressing** indication that this relaxation is necessary at the current time. The current limit strikes a balance between enabling modest data-carrier use cases and protecting the network's efficiency, decentralization, and sustainability. Introducing unnecessary changes risks opening the do
...
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2873513948)
re: Sjors https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2858241771
> @ryanofsky CI complains about `std::move` in `libmultiprocess`: https://cirrus-ci.com/task/6187773452877824, e.g.:
Thanks for catching this. I implemented a fix for that error and the other errors in https://github.com/bitcoin-core/libmultiprocess/pull/172
If you wanted to test the fix here you could do:
```shell
git fetch https://github.com/bitcoin-core/libmultiprocess 8d8e9f65f1d9faaafe91ca90910c1f96
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2873513948)
re: Sjors https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2858241771
> @ryanofsky CI complains about `std::move` in `libmultiprocess`: https://cirrus-ci.com/task/6187773452877824, e.g.:
Thanks for catching this. I implemented a fix for that error and the other errors in https://github.com/bitcoin-core/libmultiprocess/pull/172
If you wanted to test the fix here you could do:
```shell
git fetch https://github.com/bitcoin-core/libmultiprocess 8d8e9f65f1d9faaafe91ca90910c1f96
...
💬 xstoicunicornx commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873550444)
>Bitcoin’s success hinges on its decentralization and accessibility.
>Moreover, out-of-band services that cater to data-carrier transactions are not inherently harmful to the network. They exist because Bitcoin was intentionally not designed as a general-purpose data storage layer, but rather as a secure payments system. Private markets offering these services reflect a willingness to pay for non-standard transactions, and this market demand does not justify altering Bitcoin’s core design to
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873550444)
>Bitcoin’s success hinges on its decentralization and accessibility.
>Moreover, out-of-band services that cater to data-carrier transactions are not inherently harmful to the network. They exist because Bitcoin was intentionally not designed as a general-purpose data storage layer, but rather as a secure payments system. Private markets offering these services reflect a willingness to pay for non-standard transactions, and this market demand does not justify altering Bitcoin’s core design to
...
💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873582873)
@xstoicunicornx It's LLM spam
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2873582873)
@xstoicunicornx It's LLM spam