BIP 148 support #10417

pull achow101 wants to merge 2 commits into bitcoin:master from achow101:master-BIP148 changing 2 files +23 −0
  1. achow101 commented at 5:27 PM on May 17, 2017: member

    This PR adds support for BIP 148 as it is currently specified.

  2. add uasf logic e030506d3e
  3. jmcorgan commented at 5:34 PM on May 17, 2017: contributor

    Concept ACK :+1:

  4. jonasschnelli added the label Consensus on May 17, 2017
  5. luke-jr commented at 6:16 PM on May 17, 2017: member

    The whole subversion thing is for the forked client. We shouldn't merge it to Core IMO.

    Concept ACK on merging BIP 148 itself, though.

  6. jonasschnelli commented at 6:17 PM on May 17, 2017: contributor

    Shouldn't this at least have a startup configuration wether you want to run/enforce UASF/BIP148?

  7. luke-jr commented at 6:20 PM on May 17, 2017: member

    @jonasschnelli I don't see a need for that. We've never had one for any previous softfork; not even the controversial BIP16.

  8. weedcoder commented at 6:33 PM on May 17, 2017: none

    ACK

  9. ghost commented at 6:36 PM on May 17, 2017: none

    Concept ACK. @jonasschnelli I thought about that whilst talking to achow, but I only see it as an unnecessary complication.

  10. Seccour commented at 6:42 PM on May 17, 2017: none

    Concept ACK.

  11. Christewart commented at 6:43 PM on May 17, 2017: member

    Still undecided about the viability of BIP148/BIP149. I think BIP148 would be a lot safer if we could somehow convince BU to rebase on 0.14.1 but I doubt that will happen anytime soon as they are currently on 0.12.x AFAICT.

    On May 17, 2017 1:36 PM, "Lauda" notifications@github.com wrote:

    Concept ACK. @jonasschnelli https://github.com/jonasschnelli I thought about that whilst talking to achow, but I only see it as an unnecessary complication.

    — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/10417#issuecomment-302189109, or mute the thread https://github.com/notifications/unsubscribe-auth/ADWiTZzAEWN_l6SPpVJA5_Rmi8Kjlsx_ks5r6z5JgaJpZM4NeM2Z .

  12. Remove uasf comment string from user agent 081487d5ff
  13. achow101 force-pushed on May 17, 2017
  14. achow101 commented at 6:45 PM on May 17, 2017: member

    I removed the subversion string.

  15. in src/validation.cpp:1821 in 081487d5ff
    1816 | +          !IsWitnessEnabled(pindex->pprev, chainparams.GetConsensus())) )   // and is not active.
    1817 | +    {
    1818 | +        bool fVersionBits = (pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS;
    1819 | +        bool fSegbit = (pindex->nVersion & VersionBitsMask(chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) != 0;
    1820 | +        if (!(fVersionBits && fSegbit)) {
    1821 | +            return state.DoS(0, error("ConnectBlock(): relayed block must signal for segwit, please upgrade"), REJECT_INVALID, "bad-no-segwit");
    


    paveljanik commented at 6:55 PM on May 17, 2017:

    __func__ please


    achow101 commented at 7:03 PM on May 17, 2017:

    Isn't __func__ only for logprintf()? This code here follows the pattern of the other error() calls in this function.

  16. jameshilliard commented at 6:56 PM on May 17, 2017: contributor

    Concept ACK

  17. morcos commented at 7:07 PM on May 17, 2017: member

    This is not appropriate for a pull request. There clearly is not widespread consensus for BIP 148. NACK

  18. TheBlueMatt commented at 7:07 PM on May 17, 2017: member

    I find it ironic that this is tagged "Consensus", but I dont really see much in the way of "consensus" from the broad community on this. Maybe on reddit, but the community is much larger than just that.

  19. paveljanik commented at 7:10 PM on May 17, 2017: contributor

    Hmm, I somehow missed the whole BIP148 discussion, but from reading its implementation, I have got the feeling that it can do a lot of harm to the whole Bitcoin ecosystem between Aug 01 and Nov 15. I do not like such method of developing consensus in the community.

    NACK, sorry

  20. sipa commented at 7:11 PM on May 17, 2017: member

    Running BIP148 code implies the user is taking the risk that no majority will start signalling, and be forked off. I support people who want to make that choice, but it would be irresponsible to ship software that enables it by default. NACK.

  21. Vaesper commented at 7:14 PM on May 17, 2017: none

    @morcos isn't there a bit of a chicken-and-egg problem there? @sipa would you change your NACK if there was a toggle (perhaps off by default)?

  22. paveljanik commented at 7:25 PM on May 17, 2017: contributor

    What will happen e.g. in 2018 with this code when the UASF won't be successful? Is it able to IBD Bitcoin blockchain then? Or it will be on an UASF-altcoin chain running on the same port/network as Bitcoin causing a lot of disruption?

  23. sipa commented at 7:30 PM on May 17, 2017: member

    If there is clear community support for UASF (which I think will need time to assess), a new segwit deployment can always be defined - perhaps using BIP8.

  24. instagibbs commented at 7:30 PM on May 17, 2017: member

    @paveljanik speaking more broadly, if the BIP148 chain is lighter than the Bitcoin chain, any clients running BIP148 software will be on the lighter chain. Similar to extended false signalling during BIP9.

  25. ghost commented at 7:33 PM on May 17, 2017: none

    @sipa How exactly do you assess community support for said proposal?

  26. wewillfindyou commented at 7:47 PM on May 17, 2017: none

    @sipa Why there are this people saying there is no support?

    https://coin.dance/poli 88% agreement from community for segWit.

    Several other altcoins implemented SegWit. Its not a big deal for them, but here it is.

    Whats the deal of stopping it on bitcoin?

    Is it just about stopping? then go to hell or name your arguments from technical point of view.

    its a softfork, it cant split the chain as both versions are compatible.

    why having here the discussions from r/btc reddit ?

  27. sipa commented at 7:50 PM on May 17, 2017: member

    @wewillfindyou Please don't put words in my mouth. I believe there is widespread support for SegWit, and I sure as hell hope it activates at some point in time. Unsure if you're aware, but I'm the primary author of the SegWit proposal and patches.

    However, I don't think there is a rush, and BIP148 seems an unnecessarily risky way to bring that about.

  28. TheBlueMatt commented at 7:51 PM on May 17, 2017: member

    @wewillfindyou its nota question of stopping, its a question of "consensus", not "majority" but really the vast, vast majority. I think you'd agree that, while some places on the internet have very strong support, there are many businesses, miners, and exchanges (all of whom are also users of the system) who do not support BIP 148 (though many more support SegWit), at varying levels of disagreement. You may want to peruse some of my thoughts on why consensus is both critical, and why we should all be requiring consensus before agreeing to anything at http://bluematt.bitcoin.ninja/2017/02/28/bitcoin-trustlessness/. Maybe this should be locked, given the discussion is no longer about code?

  29. luke-jr approved
  30. luke-jr commented at 8:19 PM on May 17, 2017: member

    utACK

  31. wewillfindyou commented at 8:24 PM on May 17, 2017: none

    @TheBlueMatt @sipa

    This is github.com. Its about the code, not about the marketing effect of a proposal or politician reaction. Please stick to that. So if you argument, that proposal is bad for you because your are lobbing the opposite group and are nack on that please do it on reddit or other social media channels, but dont spam here, where we talk about the code.

    Please stick to the thematic discussion (the code), otherwise its useless talking.

    Your voice is not that of the community. You are a hired or supporting as coder. You cant speak with the voice of the community.

    If you see any problems with the code, as a coder, say it, otherwise, you are out of context of this discussion. Sit down and hold your feet quiet.

    You are not Gods of Bitcoin, you are part of it. I understand your feeling, you may feel strange.

    I do hope you understand my feedback given to you. Its very difficult, if people cant do their parts of the jobs or want to do parts of others job.

  32. luke-jr commented at 8:29 PM on May 17, 2017: member

    I think BIP148 would be a lot safer if we could somehow convince BU to rebase on 0.14.1 but I doubt that will happen anytime soon as they are currently on 0.12.x AFAICT. @Christewart BU is so small it doesn't matter, and implementing BIP 148 itself would be trivial for them regardless of what version they're based on.

    I find it ironic that this is tagged "Consensus", but I dont really see much in the way of "consensus" from the broad community on this. @TheBlueMatt I thought you knew this already, but the tag "Consensus" means it touches consensus code. It isn't an evaluation of community support.

    Maybe on reddit, but the community is much larger than just that.

    What indication do you have that the community outside of reddit is opposed?

    I have got the feeling that it can do a lot of harm to the whole Bitcoin ecosystem between Aug 01 and Nov 15. @paveljanik BIP 148 is going forward with or without Core merging it. The risk of harm is strictly reduced if Core does merge it.

    If there is clear community support for UASF (which I think will need time to assess), a new segwit deployment can always be defined - perhaps using BIP8. @sipa The time to make a redeployment safe is likely at least a year (probably longer IMO), during which time altcoins will be getting ahead of Bitcoin with real scaling like Lightning.

    There is enough support for a UASF today - remember, this isn't a hardfork, and doesn't require consensus.

    Maybe this should be locked, given the discussion is no longer about code?

    Or those of you who insist on derailing it can just stop, and let discussion continue about the code... Ironic to make a problem and then complain because of that problem all progress here should halt.

  33. Vaesper commented at 8:40 PM on May 17, 2017: none

    @wewillfindyou it would be appreciated if you could keep your political rants outside of this discussion; the topic at hand is the code and concepts behind it, and whether it should be merged or not. @sipa there have been claims that BIP149 (or some other iteration of BIP8 deployment of Segwit) would be technically complex as it would need to ensure there is no conflict with the current generation of nodes with the BIP9 logic that would not recognize Segwit as being activated.

    I bring this up because the delay damages Bitcoin as altcoins speed ahead in terms of functionality and there is a looming threat of less prudent parties forcing poorly-tested alternative softforks such as extension blocks. In that sense, not merging BIP148 code is also a potentially dangerous choice.

  34. acvanzant commented at 8:41 PM on May 17, 2017: none

    BIP 148 is going forward with or without Core merging it. The risk of harm is strictly reduced if Core does merge it. @luke-jr This is absurd. The only way a UASF goes forward is if you idiots merge it into Core and promote it. What about the 50%+ support from miners for Emergent Consensus or at least some larger block size. This doesn't seem to get any weight around here but then some idiots on reddit are running nodes with BIP148 and all of the sudden we have to follow them because 'consensus'.

  35. sipa commented at 8:41 PM on May 17, 2017: member

    This is github.com. Its about the code, not about the marketing effect of a proposal or politician reaction. Please stick to that.

    My concern is technical. I fear that this will cause Bitcoin Core's users end up on a fork. Merging that would be irresponsible for us as maintainers of said software.

    That does not mean that:

    • I don't support SegWit. I do - I wrote most of it.
    • I don't support UASF. All early softforks in Bitcoin were user activated, and I think it's a perfectly fine way of getting new features.

    its a softfork, it cant split the chain as both versions are compatible.

    Sorry, that's nonsense. Soft forks are safe if a majority of the hashrate enforces the new rules. Segwit itself (BIP141) is designed to have very little risks of forking even if that's not the case. BIP 148 is not, and will fork users off who are not aware of the risks they are taking.

  36. wewillfindyou commented at 8:44 PM on May 17, 2017: none

    sorry to give you this bad feedback. but starting talking about politics, when your task is to review the code is untakeable. its not even amateurish or not professional, its just bad behaviour. Please think about this critics, so its not happening again.

    we really try to bring this project back on track after lets say, coders lost their focus on their roles. we all are maybe multitalents but we are hired for a specific task to do. That way we can work in a multi billion decentralised project and make it grow.

    It all clear to us, that it cannot continue, how it was the last 1,5 years. Everyone saying that, you can hear it on the every corner.

  37. sipa commented at 8:58 PM on May 17, 2017: member

    Note: I've blocked wewillfindyou.

  38. luke-jr commented at 9:00 PM on May 17, 2017: member

    Both non-BIP148 and BIP148 nodes have some common risks in the event that the chain splits. However, non-BIP148 nodes have the additional risk that the BIP148 chain can replace (with a reorg) their chain at any moment. Additionally, the stronger the BIP148 support, the more this risk to non-BIP148 nodes grows, and the less risk BIP148 nodes have. Finally, with sufficient support to compel 50+% of miners (via financial incentives), BIP148 can avoid any chain split whatsoever. If Core merges BIP148 as-is soon, BIP148 is realistically guaranteed to get that sufficient support. Therefore, I conclude that merging this PR into Core is the safest possible option for users.

  39. hsjoberg commented at 9:04 PM on May 17, 2017: contributor

    there have been claims that BIP149 (or some other iteration of BIP8 deployment of Segwit) would be technically complex as it would need to ensure there is no conflict with the current generation of nodes with the BIP9 logic that would not recognize Segwit as being activated.

    This can be mitigated in several ways. A new service bit for the P2P network can be set, and for blocks, allocating a new version bit for the BIP8/9 deployment should be sufficient.

    Correct me if I'm wrong here. I don't see why BIP149 or a new deployment schedule for SegWit would be complex.


    Regarding the PR itself, concept-ACK, but I agree with people that feels there's no need to rush. BIP149 is far less aggressive in that it doesn't require 100% of the miners to BIP9-signal for SegWit in the end of the period - the only way miners would be able to fork off once BIP149 goes live is if they intentionally include invalid segwit transactions, which they would not as they are non-standard for those clients.

  40. TheBlueMatt commented at 9:08 PM on May 17, 2017: member

    @luke-jr I figured it'd be obvious that that was a very strict NACK - there is no consensus for this, and we DO NOT merge anything which changes consensus rules but does not have clear consensus among a vast majority of the ecosystem, something SegWit likely does, but BIP 148 very clearly does not. Irrespective of how any of us individually feel about a change like this, Bitcoin is a consensus system, and while its certainly up to people in the community to determine what they feel to be the consensus of the vast majority of users is, we should not be shipping code that does not implement the consensus rules of what we feel that consensus to be.

  41. h0jeZvgoxFepBQ2C commented at 9:11 PM on May 17, 2017: none

    @TheBlueMatt I'm not sure if luke and you were talking about different things. The label "consensus" is not about that this PR has consensus - it only marks that this PR modifies the consensus code part in core?

  42. TheBlueMatt commented at 9:13 PM on May 17, 2017: member

    @lichtamberg yes, sorry, I was making a joke while also making a serious point, hence the confusion. I understand what the tag means, but was also pointing out that this very clearly does not have consensus, so NACK, incredibly strong NACK.

  43. h0jeZvgoxFepBQ2C commented at 9:20 PM on May 17, 2017: none

    @TheBlueMatt Would you accept a PR with a toggle (default off)? There are so many people out there which are asking for a UASF BIP148 version of core. Just look at reddit, 5 of the top 10 posts are about UASF. I know that reddit is not the center of the world, but by dismissing the current situation and community requests it also doesn't make it better (maybe even worse if we don't act).

    If it's off by default you can't do something wrong?


    BIP149 deployment is quite far away.. We would have to redeploy 0.13.1 + 0.14.1 completely, because they won't activate segwit anymore - independently of BIP8.

  44. TheBlueMatt commented at 9:22 PM on May 17, 2017: member

    @lichtamberg we do not ship code which makes it trivial for users to blow their face off, no.

  45. Vaesper commented at 9:23 PM on May 17, 2017: none

    @TheBlueMatt So then why do you ship code that is at risk of having its chain reorganized into the BIP148 chain?

  46. hsjoberg commented at 9:27 PM on May 17, 2017: contributor

    BIP149 deployment is quite far away.. We would have to redeploy 0.13.1 + 0.14.1 completely, because they won't activate segwit anymore - independent of BIP8.

    That is okay.

  47. StevenPine commented at 9:28 PM on May 17, 2017: none

    That's inappropriate, if a user especially sets a config option they are doing so of their own volition.

    Your view that this would 'blow their face off' is profoundly untechnical as well as highly politically charged language.

    On May 17, 2017 17:23, "Matt Corallo" notifications@github.com wrote: @lichtamberg https://github.com/lichtamberg we do not ship code which makes it trivial for users to blow their face off, no.

    — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/10417#issuecomment-302235609, or mute the thread https://github.com/notifications/unsubscribe-auth/ACUhCG9BXlou86KY74ldLGTW0hixO9Ttks5r62U5gaJpZM4NeM2Z .

  48. sipa commented at 9:29 PM on May 17, 2017: member

    @StevenPine I think you're missing that there is no config option here. The proposal is to run the BIP148 code by default. That is a massive risk for users.

  49. Vaesper commented at 9:31 PM on May 17, 2017: none

    @sipa I think you've missed that @TheBlueMatt was saying that a toggle would make it "trivial for users to blow their face off".

    Also, isn't this the correct avenue to discuss not only the proposal as is, but also to discuss what could/should be changed about it to make it acceptable?

  50. h0jeZvgoxFepBQ2C commented at 9:32 PM on May 17, 2017: none

    @sipa Matt was directly answering my question about a toggle I think? What do you think about a toggle (default off)? Would this settle some of your concerns?

  51. sipa commented at 9:45 PM on May 17, 2017: member

    Again, in case people missed it: it is strictly higher-risk for users if we do NOT merge this.

    I strongly disagree with that. Your statement is only the case if you're sure that after BIP 148's activation time, miners massively go from not supporting it to supporting it.

    I don't expect BIP148 to happen, and the few forked off nodes will quickly revert to non-BIP148. If we force users to run BIP 148, they'll switch over to other software (who the hell wants to run software that is unsafe to use unless everyone does?).

    Perhaps I am wrong on this, but I'll be very happy to be wrong.

  52. TheBlueMatt commented at 9:47 PM on May 17, 2017: member

    @luke-jr that is absurd.

    In any case, if the goal is to have some level of code review, I'm happy to merge this if its all hidden behind an ifdef for a flag that is never defined (and cannot be defined without modifying the source, and reading a long comment explaining that, while the code has been reviewed and looked at, it is not something which you should turn on unless you believe there to be strong and vast support across the community).

  53. Vaesper commented at 9:48 PM on May 17, 2017: none

    (who the hell wants to run software that is unsafe to use unless everyone does?).

    When comparing the different potential distributions of hashrate backing of the different chains, non-BIP148 is strictly more unsafe than BIP148, due to the reorganization risk. Your statement here applies more so to non-BIP148 code.

  54. sipa commented at 9:51 PM on May 17, 2017: member

    When comparing the different potential distributions of hashrate backing of the different chains, non-BIP148 is strictly more unsafe than BIP148, due to the reorganization risk.

    Only under some scenarios in which miners actually eventually switch over to BIP148 after its activation time a reorg can happen. I don't believe that will happen unless a very large portion of the ecosystem runs it.

    As long as miners don't switch over to BIP148, everyone running BIP148 code will be forked off.

    That doesn't even come close to "strictly more unsafe". I'd say it's the opposite. Under any scenario in which such a reorg is possible, BIP148 nodes have already (possibly temporarily) been forked off.

  55. jet0 commented at 10:04 PM on May 17, 2017: none

    My strictly non-technical comment: the brilliant people who invented & built SegWit, which 90% of users/nodes love are now fighting those same nodes who want to finally use the darn feature. Effectively helping the cartelized bastards that crap on us all every day to maintain they 95% veto power.

  56. sipa commented at 10:07 PM on May 17, 2017: member

    are now fighting those same nodes who want to finally use the darn feature.

    Nobody is fighting segwit. There is just disagreement about whether BIP148 is good way to make that happen.

    Have some patience. Fear of delay shouldn't be a reason to give up good practices.

  57. RHavar commented at 12:29 AM on May 18, 2017: contributor

    In any case, if the goal is to have some level of code review, I'm happy to merge this if its all hidden behind an ifdef for a flag that is never defined

    What if it was hidden behind a configuration option? If people wish to (risk) forking themselves off the network, it doesn't seem unreasonable to allow them that option without having to compile their own copy, if there isn't a significant maintenance burden.

  58. laanwj commented at 4:46 AM on May 18, 2017: member

    The whole idea of BIP148 was that it is an explicit user choice to activate segwit without miner agreement, and I support that choice, but shipping it by default in core would defeat the point.

    I'm happy to gitian-sign BIP148 builds (as I've done before) but in my opinion it's irresponsible to open a PR like this here.

    (a command line option would be a possibility, though I think also for the future it makes sense if people learn to work with customized builds instead of relying on the core team for everything - we do not determine the consensus)

  59. laanwj commented at 4:50 AM on May 18, 2017: member

    And with that, I'm going to lock this topic. It has already gone out of control. This is not a suitable place for reddit-like fights.

  60. laanwj closed this on May 18, 2017

  61. laanwj locked this on May 18, 2017

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: 2026-04-19 00:15 UTC

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