💬 pinheadmz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2063795724)
Could this be done in a followup? I actually just checked https://github.com/bitcoin/bitcoin/pull/30321 to see why we didn't do this back then -- I remember when that was authored and merged but I didn't realize you were the author!
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2063795724)
Could this be done in a followup? I actually just checked https://github.com/bitcoin/bitcoin/pull/30321 to see why we didn't do this back then -- I remember when that was authored and merged but I didn't realize you were the author!
💬 andrewtoth commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2838942988)
I think @wizkid057 raises a valid point. Merging this would significantly reduce the technical barriers to getting arbitrary large(ish) files stored plaintext on users' disks. I don't think appealing to what other software systems have gotten away with so far is relevant to this project.
The mempool is trivial to obfuscate, simply by restarting the node. The chainstate has been obfuscated for a decade, so there are not likely many users at risk. Nor are any standardness rules for adding plain
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2838942988)
I think @wizkid057 raises a valid point. Merging this would significantly reduce the technical barriers to getting arbitrary large(ish) files stored plaintext on users' disks. I don't think appealing to what other software systems have gotten away with so far is relevant to this project.
The mempool is trivial to obfuscate, simply by restarting the node. The chainstate has been obfuscated for a decade, so there are not likely many users at risk. Nor are any standardness rules for adding plain
...
⚠️ l0rinc opened an issue: "Verify AssumeUTXO snapshot hashes during full validation as well"
(https://github.com/bitcoin/bitcoin/issues/32377)
### Summary
We already hard‑code the AssumeUTXO height/UTXO-snapshot-hash/tx-count pairs (e.g. for mainnet we have heights **840,000** and **880,000**) in https://github.com/bitcoin/bitcoin/blob/master/src/kernel/chainparams.cpp#L170-L183.
Currently, this commitment is enforced only when a node explicitly activates a snapshot via `loadtxoutset`.
During a regular initial block download, `-reindex`, or `-reindex-chainstate`, the node never cross‑checks the live chainstate against these values.
...
(https://github.com/bitcoin/bitcoin/issues/32377)
### Summary
We already hard‑code the AssumeUTXO height/UTXO-snapshot-hash/tx-count pairs (e.g. for mainnet we have heights **840,000** and **880,000**) in https://github.com/bitcoin/bitcoin/blob/master/src/kernel/chainparams.cpp#L170-L183.
Currently, this commitment is enforced only when a node explicitly activates a snapshot via `loadtxoutset`.
During a regular initial block download, `-reindex`, or `-reindex-chainstate`, the node never cross‑checks the live chainstate against these values.
...
✅ 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.