💬 hebasto commented on pull request "Make `cs_db` mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32384#issuecomment-2841170159)
It might be better to focus on reviewing https://github.com/bitcoin/bitcoin/pull/28710 instead.
(https://github.com/bitcoin/bitcoin/pull/32384#issuecomment-2841170159)
It might be better to focus on reviewing https://github.com/bitcoin/bitcoin/pull/28710 instead.
📝 stratospher opened a pull request: "test: add test for malleated transaction with valid witness"
(https://github.com/bitcoin/bitcoin/pull/32385)
- [`create_malleated_version()`](https://github.com/bitcoin/bitcoin/blob/d62c2d82e14d27307d8790fd9d921b740b784668/test/functional/p2p_orphan_handling.py#L141) function in the functional tests currently creates transactions with same txid, different wtxid but the witness is invalid.
- it is useful to have transactions with same txid, different wtxid and valid witness for better coverage
and to test how these transactions are relayed over different kinds of P2P connections
(ex: see https://
...
(https://github.com/bitcoin/bitcoin/pull/32385)
- [`create_malleated_version()`](https://github.com/bitcoin/bitcoin/blob/d62c2d82e14d27307d8790fd9d921b740b784668/test/functional/p2p_orphan_handling.py#L141) function in the functional tests currently creates transactions with same txid, different wtxid but the witness is invalid.
- it is useful to have transactions with same txid, different wtxid and valid witness for better coverage
and to test how these transactions are relayed over different kinds of P2P connections
(ex: see https://
...
💬 stratospher commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2068135636)
Here's something we can play with in this PR - https://github.com/stratospher/bitcoin/commit/dace16b34f31f6ad97ab21c66f3e65274cdace8c
It's not a pretty and compact method of generating like `malleate_tx()` but it works.
I've also opened #32385 to discuss how this could be done better!
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2068135636)
Here's something we can play with in this PR - https://github.com/stratospher/bitcoin/commit/dace16b34f31f6ad97ab21c66f3e65274cdace8c
It's not a pretty and compact method of generating like `malleate_tx()` but it works.
I've also opened #32385 to discuss how this could be done better!
💬 francisco-alonso commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841183765)
Concept NACK !
This affects node costs, increases pressure on fees, and slowly centralizes the network. These are not side effects, for me they are fundamental design tradeoffs. Bitcoin's value comes from being simple, predictable, and hard to change (and should be like this forever). Core fundamentals like minimizing blockchain bloat and protecting decentralization should not be addressed without consensus.
At the end of the day, this is not just about `OP_RETURN`. The real issue is wheth
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841183765)
Concept NACK !
This affects node costs, increases pressure on fees, and slowly centralizes the network. These are not side effects, for me they are fundamental design tradeoffs. Bitcoin's value comes from being simple, predictable, and hard to change (and should be like this forever). Core fundamentals like minimizing blockchain bloat and protecting decentralization should not be addressed without consensus.
At the end of the day, this is not just about `OP_RETURN`. The real issue is wheth
...
💬 stratospher commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2068145404)
> stale_tx came from the private broadcast storage, is the original transaction, not the malleated one.
ohh missed that, will check it again.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2068145404)
> stale_tx came from the private broadcast storage, is the original transaction, not the malleated one.
ohh missed that, will check it again.
💬 mrberlinorg commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2068154281)
Removing the check for nDataOut > 1 would allow multiple OP_RETURN outputs in a single transaction, which goes against the standard behavior of the Bitcoin protocol. This could introduce several issues:
Non-standard transactions: Multiple OP_RETURN outputs in a transaction are considered non-standard. Allowing them could lead to network inconsistencies, as some nodes might not be able to properly process or relay these transactions.
Network congestion: More OP_RETURN outputs would increase
...
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2068154281)
Removing the check for nDataOut > 1 would allow multiple OP_RETURN outputs in a single transaction, which goes against the standard behavior of the Bitcoin protocol. This could introduce several issues:
Non-standard transactions: Multiple OP_RETURN outputs in a transaction are considered non-standard. Allowing them could lead to network inconsistencies, as some nodes might not be able to properly process or relay these transactions.
Network congestion: More OP_RETURN outputs would increase
...
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841239327)
> You could consider leaving the options in and just increasing the default to max block size. I don't feel strongly about doing this - the config option would just be a footgun long term - but we did do `-mempoolfullrbf` removal in separate stages. It might make this less controversial.
At this point I think the controversy is sunk cost, and staging the removal just means another round of all this.
---
Things that should be discussed on the mailinglist, but I'll respond to them here an
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841239327)
> You could consider leaving the options in and just increasing the default to max block size. I don't feel strongly about doing this - the config option would just be a footgun long term - but we did do `-mempoolfullrbf` removal in separate stages. It might make this less controversial.
At this point I think the controversy is sunk cost, and staging the removal just means another round of all this.
---
Things that should be discussed on the mailinglist, but I'll respond to them here an
...
💬 Haaroon commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841301806)
I'm on the fence with this proposal as a user of core.
It's wholly unfair that those who have the power to use a private mempool or use custom node implementations can already do such things. Those around us are using bypasses, then this clearly levels the playing field.
After reading the discussion...
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841301806)
I'm on the fence with this proposal as a user of core.
It's wholly unfair that those who have the power to use a private mempool or use custom node implementations can already do such things. Those around us are using bypasses, then this clearly levels the playing field.
After reading the discussion...
Concept ACK.
💬 naiyoma commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#discussion_r2068234486)
nit: I think the message can be a bit clearer, e.g., "Timeout downloading block %s from manual peer (addnode/connect). Not disconnecting
(https://github.com/bitcoin/bitcoin/pull/32051#discussion_r2068234486)
nit: I think the message can be a bit clearer, e.g., "Timeout downloading block %s from manual peer (addnode/connect). Not disconnecting
💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2841336941)
Rebased after #29039.
@instagibbs how long do you think we should wait for this? https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2694932831
My [mailinglist post](https://gnusha.org/pi/bitcoindev/4B21A2CA-2224-4BD1-AA5D-C5B2F8859E4B@sprovoost.nl/T/#t) didn't result in very passionate pleas for keeping it. And although some projects do still use it, they haven't demonstrated a serious intention to upgrade anytime soon.
Should we perhaps wait for testnet5 to see if that fixes th
...
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2841336941)
Rebased after #29039.
@instagibbs how long do you think we should wait for this? https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2694932831
My [mailinglist post](https://gnusha.org/pi/bitcoindev/4B21A2CA-2224-4BD1-AA5D-C5B2F8859E4B@sprovoost.nl/T/#t) didn't result in very passionate pleas for keeping it. And although some projects do still use it, they haven't demonstrated a serious intention to upgrade anytime soon.
Should we perhaps wait for testnet5 to see if that fixes th
...
💬 cezar1 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841351673)
as a node runner, concept NACK. the parameters and functionalities to be removed act ad minimum as tutorial for spam self defense.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841351673)
as a node runner, concept NACK. the parameters and functionalities to be removed act ad minimum as tutorial for spam self defense.
💬 hebasto commented on pull request "util: Remove `fsbridge::get_filesystem_error_message()`":
(https://github.com/bitcoin/bitcoin/pull/32383#issuecomment-2841353847)
Friendly ping @ryanofsky @davidgumberg @hodlinator @sipsorcery :)
(https://github.com/bitcoin/bitcoin/pull/32383#issuecomment-2841353847)
Friendly ping @ryanofsky @davidgumberg @hodlinator @sipsorcery :)
👍 maflcko approved a pull request: "util: Remove `fsbridge::get_filesystem_error_message()`"
(https://github.com/bitcoin/bitcoin/pull/32383#pullrequestreview-2806410576)
lgtm ACK 671a9a258705b22b26cb8066fc0e53988c960fc6
The new error messages also read nicer. Left some further nits, while touching those lines.
(https://github.com/bitcoin/bitcoin/pull/32383#pullrequestreview-2806410576)
lgtm ACK 671a9a258705b22b26cb8066fc0e53988c960fc6
The new error messages also read nicer. Left some further nits, while touching those lines.
💬 maflcko commented on pull request "util: Remove `fsbridge::get_filesystem_error_message()`":
(https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068272988)
```suggestion
LogInfo/Warning("Unable to remove random auth cookie file %s: %s\n", fs::PathToString(e.path1()), e.code().message());
```
Since you are changing the error message anyway, seems fine to remove the redundant `__func__`. Also, could clarify to info or warning level?
(https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068272988)
```suggestion
LogInfo/Warning("Unable to remove random auth cookie file %s: %s\n", fs::PathToString(e.path1()), e.code().message());
```
Since you are changing the error message anyway, seems fine to remove the redundant `__func__`. Also, could clarify to info or warning level?
💬 maflcko commented on pull request "util: Remove `fsbridge::get_filesystem_error_message()`":
(https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068278097)
(the other messages look like warnings to me, too)
(https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068278097)
(the other messages look like warnings to me, too)
💬 hebasto commented on pull request "util: Remove `fsbridge::get_filesystem_error_message()`":
(https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068323394)
Thanks! Updated.
(https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068323394)
Thanks! Updated.
💬 hebasto commented on pull request "util: Remove `fsbridge::get_filesystem_error_message()`":
(https://github.com/bitcoin/bitcoin/pull/32383#issuecomment-2841412596)
The recent [feedback](https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068272988) from @maflcko has been addressed.
(https://github.com/bitcoin/bitcoin/pull/32383#issuecomment-2841412596)
The recent [feedback](https://github.com/bitcoin/bitcoin/pull/32383#discussion_r2068272988) from @maflcko has been addressed.
💬 hebasto commented on pull request "[PoC] Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-2841418207)
Rebased on https://github.com/bitcoin/bitcoin/pull/32383.
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-2841418207)
Rebased on https://github.com/bitcoin/bitcoin/pull/32383.
💬 boring877 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841433194)
Hello, I dont support this bip. Bitcoin is P2P and not financial institutions. The core use case for bitcoin is we can send bitcoin to each other W/O middle man. I dont need more then thats.
Bitcoin being simple what make it good.
Please Do NOT play with BITCOIN and LEAVE IT AS It is.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841433194)
Hello, I dont support this bip. Bitcoin is P2P and not financial institutions. The core use case for bitcoin is we can send bitcoin to each other W/O middle man. I dont need more then thats.
Bitcoin being simple what make it good.
Please Do NOT play with BITCOIN and LEAVE IT AS It is.
💬 boring877 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841440267)
I have No IDEA how commenting on Bitcoin change make it off-topic. when Changing core thesis of Bitcoin. Bitcoin CORE ARE NOT BITCOIN FRIENDS. YOU are guys are the enemy.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2841440267)
I have No IDEA how commenting on Bitcoin change make it off-topic. when Changing core thesis of Bitcoin. Bitcoin CORE ARE NOT BITCOIN FRIENDS. YOU are guys are the enemy.