Reserve 16 bits of nversion in blockheader #12633

pull btcdrak wants to merge 1 commits into bitcoin:master from btcdrak:reservedbits changing 4 files +18 −6
  1. btcdrak commented at 3:39 pm on March 7, 2018: contributor
    This will prevent versionbits false positive warnings for blocks mined with version-rolling or any other known or unknown uses of those nversion bits 0x1fffe000
  2. Remove 16 bits from versionbits signalling
    This removes bits 13-28 inclusive (0x1fffe000) from the versionbits
    signalling system, preventing warnings from being generated for any
    miners using these bits.
    
    This effectively reserves 16 bit for miscellaneous use by miners
    for things like version-rolling and nonce-rolling and establishes
    a zone miners can use without causing unexpected disruption.
    d12516e136
  3. luke-jr commented at 3:43 pm on March 7, 2018: member
    Concept ACK, assuming community support. It seems like having the 16 bits removed be on byte boundaries would be useful (but this may be more complicated).
  4. TheBlueMatt commented at 3:45 pm on March 7, 2018: member

    Without some kind of review in the form of several mining hardware manufacturers weighing in that they feel comfortable building in asicboost into their hardware, I don’t think we can give this kind of implied approval of it’s use (until then, we could maybe consider changing the warning message to something that implies someone is attacking the network, instead of “unknown rules”).

    On March 7, 2018 3:40:11 PM UTC, “฿tcDrak” notifications@github.com wrote:

    This will prevent versionbits false positive warnings for blocks mined with version-rolling or any other known or unknown uses of those nversion bits 0x1fffe000 You can view, comment on, or merge this pull request online at:

    #12633

    – Commit Summary –

    • Remove 16 bits from versionbits signalling

    – File Changes –

    M src/chainparams.cpp (6) M src/validation.cpp (2) M src/versionbits.h (4) M test/functional/feature_versionbits_warning.py (12)

    – Patch Links –

    https://github.com/bitcoin/bitcoin/pull/12633.patch https://github.com/bitcoin/bitcoin/pull/12633.diff

    – You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/bitcoin/bitcoin/pull/12633

  5. laanwj added the label TX fees and policy on Mar 7, 2018
  6. MarcoFalke commented at 7:57 pm on March 22, 2018: member
    This needs a bip and mailing list discussion, I guess.
  7. jameshilliard commented at 8:10 pm on March 22, 2018: contributor
    Mailing list discussion is here with BIP PR here.
  8. Cobra-Bitcoin commented at 6:58 pm on March 24, 2018: none

    NACK. Very strongly against this.

    This change is designed to make it easier for a business to use patented technology on the network. This change in Bitcoin Core substantially reduces the number of concurrent soft fork deployments. Rather than improve the Bitcoin network and its functionality, this pull request is designed to aid outside interests and give competitive advantage to a mining hardware business owned by the OP, thereby allowing him to make more money as a result.

    Merging this change would be an ultimate betrayal against the Bitcoin users, who depend on Bitcoin Core as trusted software that is supposed to be outside the influence of business interests.

  9. fivepiece commented at 7:43 pm on March 24, 2018: contributor

    This change is designed to make it easier for a business to use patented technology on the network.

    This change doesn’t make it easier to use asicboost on the network. It only standardizes the overt type.

    This change in Bitcoin Core substantially reduces the number of concurrent soft fork deployments.

    Soft fork deployment by miners’ signaling is bunk, as we’ve already seen with recent segwit deployment. Moving on to time\block height based deployment is much safer and doesn’t require nversion setting. Even if it would be needed to use nversion setting, there are still many of these bits for use.

    Rather than improve the Bitcoin network and its functionality, this pull request is designed to aid outside interests and give competitive advantage to a mining hardware business owned by the OP, thereby allowing him to make more money as a result.

    Again, nothing stops anybody from using asicboost already. This proposal only standardizes overt asicboost using nversion rolling, which is the cleanest type of asicboost. It doesn’t require small\empty blocks and is not dangerous in requiring transaction index permutation.

    Merging this change would be an ultimate betrayal against the Bitcoin users, who depend on Bitcoin Core as trusted software that is supposed to be outside the influence of business interests.

    Let’s wait for the rest of the community to react to this proposal before putting words in their mouths.

  10. Cobra-Bitcoin commented at 8:18 pm on March 24, 2018: none

    @fivepiece: Getting into a discussion about whether nversion bits signalling is useful is beyond the scope of what’s being discussed.

    We have to ask, who is this change benefiting? On a technical level, this change reduces the number of possible soft fork deployments from 29 to 13, so functionality is actually being diminished. The only beneficiary is the company owned by OP, which sells hardware implementing the patented version rolling ASICBOOST being standardized by this pull request and associated BIP.

    Without this, the company is likely to face a public relations nightmare when their use of the patented version roll triggers warnings for full node operators, as it rightly should. Bitcoin Core should not exist to help standardize patented tech, or aid in it’s use. At the very least this should not be the default behavior. It’s hard to not see how this pull request being merged would not significantly benefit the OP and his business. I ask you to consider if it where BITMAIN that had this optimization in their hardware, and they created a similar pull request.

  11. fivepiece commented at 8:40 pm on March 24, 2018: contributor

    @Cobra-Bitcoin , are you taking this into account ? https://www.asicboost.com/single-post/2018/03/01/opening-asicboost-for-defensive-use/

    Bitmain already has asicboost in their hardware as we all know, but their implementation of covert asicboost only hurts the network (small blocks, incompatible with witness commitment). If they were to propose this same proposal of overt boost then I would be very much for it, taking into account the DPL. I don’t believe the reduction from 29 to 13 concurrent soft forks is an issue, and even more so - I believe giving any miner a say in soft fork deployment is inherently broken, so nversion’s use in soft fork deployment is even less of a concern for me.

  12. MarcoFalke commented at 8:45 pm on March 24, 2018: member

    The review comments on pull requests are not meant for extended off-topic discussions. Please refer to the mailing list if you want to provide feedback on the BIP and keep this discussion for review comments on the code.

    #12633 (comment)

  13. MarcoFalke closed this on Mar 24, 2018

  14. MarcoFalke locked this on Mar 24, 2018
  15. MarcoFalke deleted a comment on Mar 25, 2018
  16. MarcoFalke deleted a comment on Mar 25, 2018
  17. MarcoFalke unlocked this on Mar 28, 2018
  18. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 18:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me