🤔 mzumsande reviewed a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338#pullrequestreview-2804176049)
Code Review ACK f1b142856a4ecd0a0d90bc3d73ef5137997b14ff
(https://github.com/bitcoin/bitcoin/pull/32338#pullrequestreview-2804176049)
Code Review ACK f1b142856a4ecd0a0d90bc3d73ef5137997b14ff
💬 dergoegge commented on issue "build: x86 ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839393525)
Now that we've removed `ENABLE_HARDENING` this is happening again. The suggested workarounds of injecting flags manually via `APPEND_*` does not work.
This is an issue for me with https://github.com/dergoegge/fuzzamoto and it''ll also be a problem on oss-fuzz, when they bump their afl++ version.
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839393525)
Now that we've removed `ENABLE_HARDENING` this is happening again. The suggested workarounds of injecting flags manually via `APPEND_*` does not work.
This is an issue for me with https://github.com/dergoegge/fuzzamoto and it''ll also be a problem on oss-fuzz, when they bump their afl++ version.
💬 moth-oss commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839395732)
Concept NACK
removing this limit won't solve the UTXO bloat issue. There are efforts underway to solve that which is a more productive direction for those worried about the bloat.
I'm not opposed to changes in the future, but right now there is no hurry to do this. In fact, doing so now can be counterproductive, both from a scalability lens, as well as community trust and perception of core.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839395732)
Concept NACK
removing this limit won't solve the UTXO bloat issue. There are efforts underway to solve that which is a more productive direction for those worried about the bloat.
I'm not opposed to changes in the future, but right now there is no hurry to do this. In fact, doing so now can be counterproductive, both from a scalability lens, as well as community trust and perception of core.
💬 ctrlbreak- commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839408099)
Concept NACK.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839408099)
Concept NACK.
💬 hebasto commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839412263)
> Now that we've removed `ENABLE_HARDENING` this is happening again. The suggested workaround of injecting flags manually via `APPEND_*` does not work.
Which suggestion exactly?
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839412263)
> Now that we've removed `ENABLE_HARDENING` this is happening again. The suggested workaround of injecting flags manually via `APPEND_*` does not work.
Which suggestion exactly?
💬 dergoegge commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839416536)
Sorry forgot to link it, from the PR description here: https://github.com/bitcoin/bitcoin/pull/32071.
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839416536)
Sorry forgot to link it, from the PR description here: https://github.com/bitcoin/bitcoin/pull/32071.
💬 hsjoberg commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839479865)
Concept ACK for the reasons given in OP.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839479865)
Concept ACK for the reasons given in OP.
💬 PlayBounty commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839496799)
We can all at least agree that this is controversial, so the urgency to merge is confusing. Please stop and give folks more opportunity to reach a rough consensus.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839496799)
We can all at least agree that this is controversial, so the urgency to merge is confusing. Please stop and give folks more opportunity to reach a rough consensus.
💬 D33r-Gee commented on pull request "interface: Expose load utxo snapshot functionality":
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2839515522)
@Sjors,
Thank you for your response and for clarifying the path forward.
I apologize for the confusion on my part regarding the intended repository for the interface change. Following your comment in QML PR [#424](https://github.com/bitcoin-core/gui-qml/pull/424), I incorrectly assumed you meant `bitcoin-core/gui` when you referred to "Bitcoin Core" rather than `bitcoin/bitcoin`. Thank you for clearing that up.
It's encouraging to hear that there's interest in potentially exposing the A
...
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2839515522)
@Sjors,
Thank you for your response and for clarifying the path forward.
I apologize for the confusion on my part regarding the intended repository for the interface change. Following your comment in QML PR [#424](https://github.com/bitcoin-core/gui-qml/pull/424), I incorrectly assumed you meant `bitcoin-core/gui` when you referred to "Bitcoin Core" rather than `bitcoin/bitcoin`. Thank you for clearing that up.
It's encouraging to hear that there's interest in potentially exposing the A
...
💬 Jeremy-coding commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839517064)
ACK.
So many people in this thread have not read Hijacking Bitcoin & it shows. People here are actually commenting on Github expecting their opinion or "the amount of controversy" to have any impact on what happens at all.
Sorry guys, but that's how you like to imagine Bitcoin works but not how it actually does. Read some Bitcoin history & figure it out.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839517064)
ACK.
So many people in this thread have not read Hijacking Bitcoin & it shows. People here are actually commenting on Github expecting their opinion or "the amount of controversy" to have any impact on what happens at all.
Sorry guys, but that's how you like to imagine Bitcoin works but not how it actually does. Read some Bitcoin history & figure it out.
💬 glozow commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839518223)
Reminder that this is a pull request open for code review, not a per-username voting forum. Reviews are weighted by the merit of their arguments, not the number of times they are posted. Please rest assured that if your point has already been made here or on #28130, you don't need to post it again. There is no reason to leave unsubstantiated or repeat acks/nacks other than to spam the people subscribed to this repo.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839518223)
Reminder that this is a pull request open for code review, not a per-username voting forum. Reviews are weighted by the merit of their arguments, not the number of times they are posted. Please rest assured that if your point has already been made here or on #28130, you don't need to post it again. There is no reason to leave unsubstantiated or repeat acks/nacks other than to spam the people subscribed to this repo.
💬 donaldevine commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839531293)
Concept NACK. There are no valid reasons to remove the code to enforce datacarrier limits unless the objective was to provide a potential attack vector.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839531293)
Concept NACK. There are no valid reasons to remove the code to enforce datacarrier limits unless the objective was to provide a potential attack vector.
💬 glozow commented on pull request "p2p: stop special-casing witness-stripped error for unconfirmed transactions":
(https://github.com/bitcoin/bitcoin/pull/32379#discussion_r2066937238)
Yep. Should say 'parent should be requested since witness stripped-transaction rejections shouldn't be cached at all'
(https://github.com/bitcoin/bitcoin/pull/32379#discussion_r2066937238)
Yep. Should say 'parent should be requested since witness stripped-transaction rejections shouldn't be cached at all'
💬 glozow commented on pull request "p2p: stop special-casing witness-stripped error for unconfirmed transactions":
(https://github.com/bitcoin/bitcoin/pull/32379#discussion_r2066935115)
I don't think you want to delete the entire comment, just the part about witness stripping
(https://github.com/bitcoin/bitcoin/pull/32379#discussion_r2066935115)
I don't think you want to delete the entire comment, just the part about witness stripping
💬 wizkid057 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839558518)
> > P2P encryption must be default and no more clear relay of txn or block data can happen
>
> [It's been default for a few releases now](https://github.com/bitcoin/bitcoin/pull/29347)
>
> Mandating it for peers would split the network for any nodes that don't support v2 transport. It's not going to happen in the next 5-10 years, maybe ever.
Sad reality is that we'll directly cause people grief from this.
> > 30 of which used a miner-based service to exceed the 83 byte limit. Not 30,
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839558518)
> > P2P encryption must be default and no more clear relay of txn or block data can happen
>
> [It's been default for a few releases now](https://github.com/bitcoin/bitcoin/pull/29347)
>
> Mandating it for peers would split the network for any nodes that don't support v2 transport. It's not going to happen in the next 5-10 years, maybe ever.
Sad reality is that we'll directly cause people grief from this.
> > 30 of which used a miner-based service to exceed the 83 byte limit. Not 30,
...
💬 ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#discussion_r2066957921)
Thanks for taking a look
I've taken your suggestion with the addition of `CTxMemPool*` as a parameter to `WaitNext`.
I think it's best to pass it as well, to make the method self contained and allow for testing with a custom mempool.
Also, if I use the mempool pointer in the active chainstate not the node, a miner test fails.
This happens because in `TestPackageSelection`, we construct a new mempool and update the node interfaces' mempool pointer, but we don't update the raw pointer stored
...
(https://github.com/bitcoin/bitcoin/pull/32378#discussion_r2066957921)
Thanks for taking a look
I've taken your suggestion with the addition of `CTxMemPool*` as a parameter to `WaitNext`.
I think it's best to pass it as well, to make the method self contained and allow for testing with a custom mempool.
Also, if I use the mempool pointer in the active chainstate not the node, a miner test fails.
This happens because in `TestPackageSelection`, we construct a new mempool and update the node interfaces' mempool pointer, but we don't update the raw pointer stored
...
💬 ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2839569970)
- Updated to fix recent comment by @ryanofsky see [5fa54e2..c8a3fab](https://github.com/bitcoin/bitcoin/compare/77949ef5267e4de4e68a19560bab81e8d5fa54e2..c8a3fabe4090fa2b9a0e8cef73ed5365e8221ad6)
- Also fixed tidy issues
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2839569970)
- Updated to fix recent comment by @ryanofsky see [5fa54e2..c8a3fab](https://github.com/bitcoin/bitcoin/compare/77949ef5267e4de4e68a19560bab81e8d5fa54e2..c8a3fabe4090fa2b9a0e8cef73ed5365e8221ad6)
- Also fixed tidy issues
💬 TheGuySwann commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839584321)
NACK
Not only is this issue deeply controversial and the standard has been since the beginning to not force or push changes that are controversial and this is a horrible look and will create pointless distrust in Core, but continuing to spend time on this issue and making changes for the sake of making changes when nobody asked for this is such a profound waste of time and resources.
This is only showing people that core isn't interested in what the users or the network actually care about
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839584321)
NACK
Not only is this issue deeply controversial and the standard has been since the beginning to not force or push changes that are controversial and this is a horrible look and will create pointless distrust in Core, but continuing to spend time on this issue and making changes for the sake of making changes when nobody asked for this is such a profound waste of time and resources.
This is only showing people that core isn't interested in what the users or the network actually care about
...
💬 fanquake commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839586999)
I think the issue here is similar to #32323. The user provided flags (to override the hardening options) aren't checked during CMake compiler checks, which means the disabling doesn't actually happen when using afl. As far as I can tell this also isn't fixed by #32367. Will open a PR to further patch fcf-protection out.
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2839586999)
I think the issue here is similar to #32323. The user provided flags (to override the hardening options) aren't checked during CMake compiler checks, which means the disabling doesn't actually happen when using afl. As far as I can tell this also isn't fixed by #32367. Will open a PR to further patch fcf-protection out.
💬 rot13maxi commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839587470)
Here’s an opreturn from a year ago that contains a base64-encoded dickbutt: https://mempool.space/tx/8c9fe58186c199fa9f8d72c121a45270f70d3854e67238f3c7c319989a50921b
Not mined by MARA btw.
if someone wants to put something fun or malicious in an opreturn, its not hard.
Also, if inscriptions ever became a popular mechanism for content that was illegal or politically unpopular and people/companies/LEO wanted to scan for it, its trivial to add (the hashes of) inscription-encoded versions
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2839587470)
Here’s an opreturn from a year ago that contains a base64-encoded dickbutt: https://mempool.space/tx/8c9fe58186c199fa9f8d72c121a45270f70d3854e67238f3c7c319989a50921b
Not mined by MARA btw.
if someone wants to put something fun or malicious in an opreturn, its not hard.
Also, if inscriptions ever became a popular mechanism for content that was illegal or politically unpopular and people/companies/LEO wanted to scan for it, its trivial to add (the hashes of) inscription-encoded versions
...