✅ fanquake closed a pull request: "ci: Use previous releases in tests on Windows"
(https://github.com/bitcoin/bitcoin/pull/32363)
(https://github.com/bitcoin/bitcoin/pull/32363)
💬 fanquake commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#issuecomment-2838956614)
This is complete AI slop. Please stop opening these kinds of PRs against this repo.
(https://github.com/bitcoin/bitcoin/pull/32363#issuecomment-2838956614)
This is complete AI slop. Please stop opening these kinds of PRs against this repo.
💬 laanwj commented on issue "windows: usage of deprecated `std:wstring_convert`":
(https://github.com/bitcoin/bitcoin/issues/32361#issuecomment-2838964712)
Great, good to know that's possible. i guess by the time we switch to C++26 and this becomes a blocking issue, it will be fine to require a 2019 windows version, and we can simply things by using the "ASCII" Windows APIs everywhere and provide strings as-is. This would also allow dropping the leveldb patch.
(https://github.com/bitcoin/bitcoin/issues/32361#issuecomment-2838964712)
Great, good to know that's possible. i guess by the time we switch to C++26 and this becomes a blocking issue, it will be fine to require a 2019 windows version, and we can simply things by using the "ASCII" Windows APIs everywhere and provide strings as-is. This would also allow dropping the leveldb patch.
📝 ismaelsadeeq opened a pull request: "interfaces: move `Mining` and `BlockTemplate` implementation to miner"
(https://github.com/bitcoin/bitcoin/pull/32378)
#### Motivation
In [Internal interface guidelines](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#internal-interface-guidelines)
It's stated that
> Interface method definitions should wrap existing functionality instead of implementing new functionality. Any substantial new node or wallet functionality should be implemented in [src/node/](https://github.com/bitcoin/bitcoin/blob/master/src/node) or [src/wallet/](https://github.com/bitcoin/bitcoin/blob/master/src/
...
(https://github.com/bitcoin/bitcoin/pull/32378)
#### Motivation
In [Internal interface guidelines](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#internal-interface-guidelines)
It's stated that
> Interface method definitions should wrap existing functionality instead of implementing new functionality. Any substantial new node or wallet functionality should be implemented in [src/node/](https://github.com/bitcoin/bitcoin/blob/master/src/node) or [src/wallet/](https://github.com/bitcoin/bitcoin/blob/master/src/
...
💬 glozow commented on pull request "test: Use the correct node for doubled keypath test":
(https://github.com/bitcoin/bitcoin/pull/32369#issuecomment-2839006892)
My bad for not running this test, thanks @achow101!
(https://github.com/bitcoin/bitcoin/pull/32369#issuecomment-2839006892)
My bad for not running this test, thanks @achow101!
💬 instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839046515)
> I am also concerned that in the future we might see a large scale bypassing of the Core standarness rules, but it’s unclear whether we’ve reached that point yet
I see no evidence that it's not being regularly bypassed already: https://opreturnbot.com/
If it was any standardness issue that had systemic risks being turned off I'd consider a bit harder, but it does not. Things are simply worse for decentralization with the restrictions in place. There are explicit plans to either use centra
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839046515)
> I am also concerned that in the future we might see a large scale bypassing of the Core standarness rules, but it’s unclear whether we’ve reached that point yet
I see no evidence that it's not being regularly bypassed already: https://opreturnbot.com/
If it was any standardness issue that had systemic risks being turned off I'd consider a bit harder, but it does not. Things are simply worse for decentralization with the restrictions in place. There are explicit plans to either use centra
...
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839060523)
> The mempool is trivial to obfuscate, simply by restarting the node.
Anyone upgrading their node to v30, or any other version, has to restart their node.
> block obfuscation is relatively new, and will only be enabled if the blockchain is downloaded from scratch
There's no new attack here, getting arbitrary data confirmed on chain has been trivial for years. See https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2838068278
> node policies are subjective by definition and, as
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839060523)
> The mempool is trivial to obfuscate, simply by restarting the node.
Anyone upgrading their node to v30, or any other version, has to restart their node.
> block obfuscation is relatively new, and will only be enabled if the blockchain is downloaded from scratch
There's no new attack here, getting arbitrary data confirmed on chain has been trivial for years. See https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2838068278
> node policies are subjective by definition and, as
...
💬 furszy commented on issue "Cannot import descriptors with label and internal:false":
(https://github.com/bitcoin/bitcoin/issues/32376#issuecomment-2839108651)
The fix looks correct (could also use `value_or(false)` to shorten it). Remember to add a test case when you push it.
(https://github.com/bitcoin/bitcoin/issues/32376#issuecomment-2839108651)
The fix looks correct (could also use `value_or(false)` to shorten it). Remember to add a test case when you push it.
👍 instagibbs approved a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2803748588)
ACK 47e713ea5a96d3bb2ddd64c8a87607e5ccea8c72
Commits could be squashed
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2803748588)
ACK 47e713ea5a96d3bb2ddd64c8a87607e5ccea8c72
Commits could be squashed
💬 instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066599744)
micro-nit: `0.05000026` is pretty magical, would be better to be computed if possible
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066599744)
micro-nit: `0.05000026` is pretty magical, would be better to be computed if possible
💬 instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066594778)
what's this doubling doing?
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066594778)
what's this doubling doing?
💬 instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066638321)
nit sanity check
```Suggestion
tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len + 1)])
assert_greater_than(tx.get_vsize(), int(MAX_STANDARD_TX_WEIGHT / 4))
```
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066638321)
nit sanity check
```Suggestion
tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len + 1)])
assert_greater_than(tx.get_vsize(), int(MAX_STANDARD_TX_WEIGHT / 4))
```
💬 instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066626406)
sanity check
```Suggestion
tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len)])
assert_equal(tx.get_vsize(), int(MAX_STANDARD_TX_WEIGHT / 4))
```
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066626406)
sanity check
```Suggestion
tx.vout[0].scriptPubKey = CScript([OP_RETURN, b"\xff" * (data_len)])
assert_equal(tx.get_vsize(), int(MAX_STANDARD_TX_WEIGHT / 4))
```
🤔 jamesob reviewed a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2803827620)
Concept ACK
Relaxing Core's standardness rules makes it less likely that some users will eschew its P2P relay network in favor of "private" mempool submission systems that are more permissive but (debatably) less censorship resistant.
While I think this outcome is probably inevitable, any change that brings Core's policy closer to the consensus rules is likely to slow this eventuality, and so is probably worth doing.
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2803827620)
Concept ACK
Relaxing Core's standardness rules makes it less likely that some users will eschew its P2P relay network in favor of "private" mempool submission systems that are more permissive but (debatably) less censorship resistant.
While I think this outcome is probably inevitable, any change that brings Core's policy closer to the consensus rules is likely to slow this eventuality, and so is probably worth doing.
💬 jamesob commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066646065)
Can't remember how this has been handled in the past, but simply removing these options will cause existing invocations/configurations that use these options to error on start. Might be better to show a deprecation message and set new defaults, but might also be unnecessary if similar provisions haven't been made in the past.
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2066646065)
Can't remember how this has been handled in the past, but simply removing these options will cause existing invocations/configurations that use these options to error on start. Might be better to show a deprecation message and set new defaults, but might also be unnecessary if similar provisions haven't been made in the past.
📝 darosior opened a pull request: "p2p: stop special-casing witness-stripped error for unconfirmed transactions"
(https://github.com/bitcoin/bitcoin/pull/32379)
Opening as draft to facilitate discussions, this has some issues.
This special-case was introduced in #18044 to avoid hindering relay of witness in adversarial situations where most connections on the network were still using txid-based relay. This is not a concern anymore. Since it is expensive to detect, get rid of the special case.
This may however cause issues for orphan resolution in adversarial situations, since fetching the parent is done using its txid. Greg Sanders pointed out to
...
(https://github.com/bitcoin/bitcoin/pull/32379)
Opening as draft to facilitate discussions, this has some issues.
This special-case was introduced in #18044 to avoid hindering relay of witness in adversarial situations where most connections on the network were still using txid-based relay. This is not a concern anymore. Since it is expensive to detect, get rid of the special case.
This may however cause issues for orphan resolution in adversarial situations, since fetching the parent is done using its txid. Greg Sanders pointed out to
...
💬 darosior commented on pull request "p2p: stop special-casing witness-stripped error for unconfirmed transactions":
(https://github.com/bitcoin/bitcoin/pull/32379#discussion_r2066694246)
It seems this comment was wrong in the first place. The parent is requested by txid, so the unstripped wtxid doesn't matter. The only reason that this was working is because WITNESS_STRIPPED was special cased as a failure and we didn't add the wtxid of the witness-stripped transaction (i.e. its txid) to the reject filters. Not special-casing this issue prevents requesting the parent since now its txid would be in the reject filter.
(https://github.com/bitcoin/bitcoin/pull/32379#discussion_r2066694246)
It seems this comment was wrong in the first place. The parent is requested by txid, so the unstripped wtxid doesn't matter. The only reason that this was working is because WITNESS_STRIPPED was special cased as a failure and we didn't add the wtxid of the witness-stripped transaction (i.e. its txid) to the reject filters. Not special-casing this issue prevents requesting the parent since now its txid would be in the reject filter.
💬 glozow commented on pull request "p2p: stop special-casing witness-stripped error for unconfirmed transactions":
(https://github.com/bitcoin/bitcoin/pull/32379#issuecomment-2839156365)
> This may however cause issues for orphan resolution in adversarial situations, since fetching the parent is done using its txid. Greg Sanders pointed out to me on IRC this could be an issue for package relay.
Just want to point out this is one of the motivations for (real) package relay, so we can get rid of txid-based fetching. https://github.com/bitcoin/bips/blob/master/bip-0331.mediawiki#handle-orphans-better
Today however, you'd be able to block (opportunistic) "package relay" by sen
...
(https://github.com/bitcoin/bitcoin/pull/32379#issuecomment-2839156365)
> This may however cause issues for orphan resolution in adversarial situations, since fetching the parent is done using its txid. Greg Sanders pointed out to me on IRC this could be an issue for package relay.
Just want to point out this is one of the motivations for (real) package relay, so we can get rid of txid-based fetching. https://github.com/bitcoin/bips/blob/master/bip-0331.mediawiki#handle-orphans-better
Today however, you'd be able to block (opportunistic) "package relay" by sen
...
💬 fjahr commented on pull request "RFC: Accept non-std transactions in Testnet4 by default again":
(https://github.com/bitcoin/bitcoin/pull/32133#issuecomment-2839189354)
> In that case, the same setup that worked for testnet (getting a direct connection to a miner that is willing to support your consensus-valid but non-standard transactions) also works for mainnet, and they haven't shot themselves in the foot.
I didn't mean that they setup this connection to a miner on purpose, I meant that they just happened to be connect to a miner by chance (which I think is much more likely to happend on testnet than on mainnet).
> If you want to do your testing in sit
...
(https://github.com/bitcoin/bitcoin/pull/32133#issuecomment-2839189354)
> In that case, the same setup that worked for testnet (getting a direct connection to a miner that is willing to support your consensus-valid but non-standard transactions) also works for mainnet, and they haven't shot themselves in the foot.
I didn't mean that they setup this connection to a miner on purpose, I meant that they just happened to be connect to a miner by chance (which I think is much more likely to happend on testnet than on mainnet).
> If you want to do your testing in sit
...
💬 mzumsande commented on issue "rfc: only relay transactions to v2 encrypted peers":
(https://github.com/bitcoin/bitcoin/issues/32373#issuecomment-2839194234)
> Related, but more radical: https://github.com/bitcoin/bitcoin/issues/29618
This proposal sounds more radical to me, assuming it's meant to be the default and not optional like #29618. Once >90% of all nodes would implement it, v1 nodes wouldn't percolate the network graph anymore, and the remaining v1 nodes (bitcoin core and alternative implementations) would become second-class citizens that wouldn't see most transactions anymore before they are included in a block.
Another aspect is that t
...
(https://github.com/bitcoin/bitcoin/issues/32373#issuecomment-2839194234)
> Related, but more radical: https://github.com/bitcoin/bitcoin/issues/29618
This proposal sounds more radical to me, assuming it's meant to be the default and not optional like #29618. Once >90% of all nodes would implement it, v1 nodes wouldn't percolate the network graph anymore, and the remaining v1 nodes (bitcoin core and alternative implementations) would become second-class citizens that wouldn't see most transactions anymore before they are included in a block.
Another aspect is that t
...