BIP 341: Add Speedy Trial activation parameters #21393

pull achow101 wants to merge 10 commits into bitcoin:master from achow101:taproot-speedy-trial changing 17 files +504 −300
  1. achow101 commented at 10:34 pm on March 8, 2021: member

    Adds mainnet, testnet, and regtest parameters for the proposed Speedy Trial activation method.

    The mainnet and testnet parameters are specified in https://github.com/bitcoin/bips/pull/1081. The mainnet parameters were discussed on the mailing list, the testnet ones were made up in the process of writing this PR because no one discussed testnet activation parameters.

    For regtest, I chose to mirror the number of retarget periods that mainnet would experience, just on the regtest threshold and retarget period.

    Also adds a small test for activating on regtest.

    Requires #21392

  2. DrahtBot added the label Consensus on Mar 9, 2021
  3. DrahtBot added the label Docs on Mar 9, 2021
  4. DrahtBot added the label Mining on Mar 9, 2021
  5. DrahtBot added the label RPC/REST/ZMQ on Mar 9, 2021
  6. DrahtBot added the label Validation on Mar 9, 2021
  7. achow101 force-pushed on Mar 9, 2021
  8. DrahtBot commented at 4:20 am on March 9, 2021: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #21401 (Refactor versionbits deployments to avoid potential uninitialized variables by achow101)
    • #21391 ([Bundle 5/n] Prune g_chainman usage in RPC modules by dongcarl)
    • #21378 (Convert taproot to flag day activation by ajtowns)
    • #21377 (Speedy trial support for versionbits by ajtowns)
    • #20354 (test: Add feature_taproot.py –previous_release by MarcoFalke)
    • #19438 (Introduce deploymentstatus by ajtowns)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  9. achow101 force-pushed on Mar 9, 2021
  10. in src/chainparams.cpp:353 in bf5736c611 outdated
    349@@ -350,7 +350,7 @@ class SigNetParams : public CChainParams {
    350         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
    351         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].startheight = Consensus::VBitsDeployment::ALWAYS_ACTIVE;
    352         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].timeoutheight = Consensus::VBitsDeployment::NO_TIMEOUT;
    353-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1916; // 95% of 2016
    


    Empact commented at 7:15 am on March 9, 2021:
    Comment needs an update

    achow101 commented at 2:57 am on March 15, 2021:
    Change was dropped, no longer relevant.
  11. in src/chainparams.cpp:353 in bf5736c611 outdated
    349@@ -350,7 +350,7 @@ class SigNetParams : public CChainParams {
    350         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
    351         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].startheight = Consensus::VBitsDeployment::ALWAYS_ACTIVE;
    352         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].timeoutheight = Consensus::VBitsDeployment::NO_TIMEOUT;
    353-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1916; // 95% of 2016
    354+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1815; // 95% of 2016
    


    luke-jr commented at 8:18 pm on March 14, 2021:
    Why mess with SigNet? It’s already active…

    achow101 commented at 2:56 am on March 15, 2021:
    I think this was accidental. Removed.
  12. in src/chainparams.cpp:421 in 6c8469218c outdated
    420-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 108; // 75% of 144
    421-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].m_min_activation_height = 0; // No minimum activation height
    422+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].startheight = 576;
    423+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].timeoutheight = 1584;
    424+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 108; // 75% for testchains
    425+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].m_min_activation_height = 2592; // No minimum activation height
    


    luke-jr commented at 8:19 pm on March 14, 2021:
    Prefer just setting -vbparams in the new test.

    achow101 commented at 2:56 am on March 15, 2021:
    Since feature_bip8.py ostensibly tests the same code, I’ve dropped this commit.
  13. in test/functional/test_runner.py:293 in 6c8469218c outdated
    289@@ -290,6 +290,7 @@
    290     'feature_help.py',
    291     'feature_shutdown.py',
    292     'p2p_ibd_txrelay.py',
    293+    'feature_taproot_activation.py',
    


    luke-jr commented at 8:20 pm on March 14, 2021:
    0    # Don't append tests at the end to avoid merge conflicts
    1    # Put them in a random line within the section that fits their approximate run-time
    

    (This location does in fact conflict at backport)


    achow101 commented at 2:56 am on March 15, 2021:
    Since feature_bip8.py ostensibly tests the same code, I’ve dropped this commit.
  14. in src/chainparams.cpp:91 in bf5736c611 outdated
    87@@ -88,10 +88,10 @@ class CMainParams : public CChainParams {
    88 
    89         // Deployment of Taproot (BIPs 340-342)
    90         consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
    91-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].startheight = Consensus::VBitsDeployment::NEVER_ACTIVE;
    92-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].timeoutheight = Consensus::VBitsDeployment::NEVER_ACTIVE;
    93-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].threshold = 1916; // 95% of 2016
    94-        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].m_min_activation_height = 0; // No minimum activation height
    95+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].startheight = 681408;
    96+        consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].timeoutheight = 695520;
    


    luke-jr commented at 1:24 am on March 15, 2021:
    The last period before timeoutheight here overlaps with the current BIP8(True) deployment plan. So if this period specifically were to reach 90% signalling, nodes would activate Taproot at height 697536, but ST-only nodes would still wait until 709632 instead.

    michaelfolkson commented at 10:37 am on March 15, 2021:
    The data suggests there is considerable more consensus around ST than BIP8(True). Therefore I think the onus is on the UASF release to fit around the ST release rather than vice versa.
  15. luke-jr changes_requested
  16. achow101 force-pushed on Mar 15, 2021
  17. achow101 force-pushed on Mar 15, 2021
  18. achow101 force-pushed on Mar 15, 2021
  19. achow101 force-pushed on Mar 16, 2021
  20. achow101 force-pushed on Mar 17, 2021
  21. MarcoFalke removed the label Consensus on Mar 18, 2021
  22. MarcoFalke removed the label Docs on Mar 18, 2021
  23. MarcoFalke removed the label Mining on Mar 18, 2021
  24. MarcoFalke removed the label RPC/REST/ZMQ on Mar 18, 2021
  25. MarcoFalke removed the label Validation on Mar 18, 2021
  26. DrahtBot added the label Consensus on Mar 18, 2021
  27. DrahtBot added the label Mining on Mar 18, 2021
  28. DrahtBot added the label RPC/REST/ZMQ on Mar 18, 2021
  29. DrahtBot added the label Validation on Mar 18, 2021
  30. achow101 force-pushed on Mar 18, 2021
  31. achow101 force-pushed on Mar 20, 2021
  32. chainparams: make versionbits threshold per-deployment 2be730c31d
  33. Migrate versionbits to use height instead of MTP
    Co-authored-by: Anthony Towns <aj@erisian.com.au>
    c95287e378
  34. Rename user facing mentions of BIP 9 to versionbits and/or BIP 8
    Co-authored-by: Anthony Towns <aj@erisian.com.au>
    8f55573ed3
  35. Add minimum activation height to BIP9Deployments 1dac84b5de
  36. tests: test versionbits delayed activation 73d2756244
  37. Clarify and reduce nRuleChangeActivationThreshold
    As thresholds are now parameterized, nRuleChangeActivationThreshold is
    no longer the threshold used for activating new rule changes. Instead it
    is now only used to warn if there is an unkonwn versionbits deployment.
    To make this clear, rename to m_vbits_min_threshold and update the
    comment describing it.
    
    Additionally, because this is just a minimum used for a warning, reduce
    the threshold to 75% so that future soft forks which may have thresholds
    lower than 95% will still have warnings.
    50eb7f0b28
  38. test: add min_activation_height to -vbparams 91b2e4b4f9
  39. test: BIP 8 delayed activation functional test 2e55bcedb8
  40. Set taproot activation parameters 9a2fe56cce
  41. Add testnet taproot activation params 93c7225823
  42. achow101 force-pushed on Apr 2, 2021
  43. achow101 closed this on Apr 6, 2021

  44. maaku commented at 8:37 pm on April 7, 2021: contributor
    Why was this closed?
  45. achow101 commented at 9:22 pm on April 7, 2021: member

    Why was this closed?

    The current plan is to go with the MTP based version of ST, so this PR is no longer applicable as it uses the height based. However if significant issues end up being found there, then this can be reopened.

  46. maaku commented at 9:59 pm on April 7, 2021: contributor

    I can find any record of consensus being reached about that “current plan” in GitHub or mailing list discussions.

    On Apr 7, 2021, at 2:22 PM, Andrew Chow @.***> wrote:

     Why was this closed?

    The current plan is to go with the MTP based version of ST, so this PR is no longer applicable as it uses the height based. However if significant issues end up being found there, then this can be reopened.

    — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

  47. achow101 commented at 10:33 pm on April 7, 2021: member

    I can find any record of consensus being reached about that “current plan” in GitHub or mailing list discussions.

    https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-April/018746.html (Note that the coin flip was not what made the decision)

  48. maaku commented at 0:24 am on April 8, 2021: contributor

    One email from one contributor? That’s not how consensus decisions have been made in the past or ought to be made going forward.

    There were serious concerns raised by respected developers about MTP as recently as two days ago. There are devs who concept NACK’d MTP. Were these devs comprehensively polled for their opinions in light of this new change?

    If you no longer want to maintain/champion #21393 that’s understandable. But someone else should probably open a new PR with the changes.

    On Apr 7, 2021, at 3:34 PM, Andrew Chow @.***> wrote:  I can find any record of consensus being reached about that “current plan” in GitHub or mailing list discussions.

    https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-April/018746.html

    — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

  49. luke-jr commented at 0:45 am on April 8, 2021: member
    #19573 is reopened and rebased
  50. achow101 commented at 1:16 am on April 8, 2021: member

    One email from one contributor?

    It is a summary of a discussion that occurred in an IRC channel involving many people.

    There were serious concerns raised by respected developers about MTP as recently as two days ago. There are devs who concept NACK’d MTP. Were these devs comprehensively polled for their opinions in light of this new change?

    Many (including myself, note that I had NACK’d a previous iteration of #21377) were as they were part of the discussions that took place on the IRC channel. Changes to #21377 have been made that I personally find acceptable.

    But someone else should probably open a new PR with the changes.

    The purpose is to focus review and attention on a single proposal rather than competing proposals. Many contributors have stated that they will not provide in depth review to either proposal until a specific one is chosen. So this was closed in order to focus review onto the other proposal. From the IRC discussions, it appears that #21377 has rough consensus and has a reasonable chance of being the proposal that is actually used. However, if it is found to be deficient, then this PR will be reopened.

    The outcome of the discussions was that most were okay with either this PR or #21377, and the decision for which to move forward on fell to the PR authors. As such, the objections to this PR and #21377 were analyzed, and we determined that #21377 could be modified that satisfied all of my objections to it, but no solutions could be found for the objections brought for this PR. There are others who still object to #21377 but they have not provided any concrete/specific objections when asked. Therefore #21377 has rough consensus and this PR does not.

  51. JeremyRubin commented at 1:49 am on April 8, 2021: contributor
    @maaku i’d encourage you to review the IRC meeting logs http://gnusha.org/taproot-activation/2021-04-06.log
  52. luke-jr commented at 1:52 am on April 8, 2021: member
    There is no consensus on MTP nonsense. Claiming there is, is just a neo-NYA attempt. “Dev muscle” does not have any standing to override community consensus on BIP8, especially in favour of a clearly inferior “solution”.
  53. JeremyRubin commented at 1:53 am on April 8, 2021: contributor

    Also note, from my email:

    • If in the next day or so, AJ and Andrew Chow reach a compromise between approaches that is compatible with the timeline of getting to a RC1 with deployment, then that can be considered on its merits in preference of either of the existing approaches.

    everyone who signed on to the coinflip acknowledged a preference for going with something that @ajtowns and @achow101 agreed would work and was conceptually close to either PR. If the measured blockhash LSB coinflip had gone either way, we’d still have converged on the same.

    So @achow101 is correct that it is not the coin flip which made the decision.

  54. michaelfolkson commented at 1:56 am on April 8, 2021: contributor

    @maaku i’d encourage you to review the IRC meeting logs http://gnusha.org/taproot-activation/2021-04-06.log

    Maybe for the entertainment value, I can’t think of any other reason why anyone would want to do that. If you think MTP concerns aren’t bikeshedding @maaku and shouldn’t be subject to coin tosses I’d recommend you comment on #21377. @achow101 is doing his best in the circumstances (and shouldn’t be subject to this circus imo).

  55. JeremyRubin commented at 2:04 am on April 8, 2021: contributor

    There were serious concerns raised by respected developers about MTP as recently as two days ago. There are devs who concept NACK’d MTP. Were these devs comprehensively polled for their opinions in light of this new change?

    I don’t know which devs he is referring to nor which concerns, so if he wants to verify for himself his best bet is to review the meeting notes. if @maaku had an original concern he felt wasn’t addressed he’s capable of reading the logs and seeing if they were addressed sufficiently.

    encouraging someone to comment without reading relevant discussions is encouraging political grandstanding and theater, and I’ll kindly ask that you not engage in encouraging blindly brigading a PR

    cheers

  56. michaelfolkson commented at 2:13 am on April 8, 2021: contributor
    Most people put Approach ACKs on two competing PRs they are happy with and move on. Others create circuses around coin tosses and waste everyone’s time. There are plenty of resources to get informed on MTP other than reviewing discussions of coin tosses including @maaku’s own BIP.
  57. maaku commented at 7:13 am on April 8, 2021: contributor

    I read the logs fully before commenting, by the way. My original note in this issue was that I couldn’t find a record of consensus being reached in GitHub or the milling list archive. According to the contribution guidelines of this repo those are the places in which consensus is reached and decisions are made about consensus-affecting pull requests.

    It is against policy for consensus decisions to be reached in IRC meetings, even on the official #bitcoin-core-dev much less an undocumented, less-official offshoot channel.

    Jeremy, you’re not building consensus; you’re ignoring people who disagree with you, then accusing some of them of conducting “political grandstanding and theater” and “blindly brigading a PR”. Please try to be more respectful of your fellow developers and hold good-faith assumptions about those who disagree with your views.

  58. harding commented at 9:06 am on April 8, 2021: contributor

    @maaku

    It is against policy for consensus decisions to be reached in IRC meetings

    Andrew said “it [appeared] that #21377 has rough consensus”. That’s not a consensus decision, it’s an individual’s decision based on where he thinks consensus is headed. The result of that decision is Andrew closing his own PR—an action we (and GitHub) have always allowed PR openers to make unilaterally—and him putting his effort into helping 21377 get merged, at least for as long as he thinks that’s the best use of his energy.

    That seems to me like an entirely reasonable position. I think all of us frequently make decisions about what to work on next based on a huge variety of factors we expect to affect project consensus—and most of those factors are never mentioned on the public mailing list. It seems to me that this is just another case of that.

  59. JeremyRubin commented at 9:13 am on April 8, 2021: contributor

    I’m doing no such thing.

    Most communication about Bitcoin Core development happens on IRC, in the #bitcoin-core-dev channel on Freenode. The easiest way to participate on IRC is with the web client, webchat.freenode.net. Chat history logs can be found on http://www.erisian.com.au/bitcoin-core-dev/ and http://gnusha.org/bitcoin-core-dev/. Discussion about codebase improvements happens in GitHub issues and pull requests. The developer mailing list should be used to discuss complicated or controversial consensus or P2P protocol changes before working on a patch set.

    It is not against any policy for consensus decisions to be made in IRC, nor were any made beyond personal commitments to review one PR or another, so you must be mistaken. Further, bitcoin core’s CONTRIBUTING.md is not binding. Note that everything is a should. If folks choose to discuss in other venues that is more than fine, as long as the process is inclusive.

    What I did was organize and host 2 meetings and summarize meeting notes from each. The meetings were scheduled well in advance such that anyone could join, and were announced also on #bitcoin-core-dev IRC at the start to get any stragglers in. The ##taproot-activation channel was explicitly made to serve as a venue for anyone interested to discuss activation, and to allow the main core development channel to maintain focus on other work.

    When you don’t have a substantive critique of work being done, it’s natural to claim a process and procedure fault. But the process has been open for anyone to participate and I’ve put ample personal work into organizing and summarizing. If there is a process you think that is more conducive to consensus, feel free to take the lead on implementing it.

    I’m certainly not ignoring anyone who disagrees with me. My intention with hosting the meetings was to be able to cast a wide net for opinions and share back outcomes of that to the mailing list. I’ve responded in good faith to other developers whose positions I disagree with, and held organized space for others to voice their concerns. There was no censorship of content in the meetings, and all views were responded to there.

    I’m glad to hear that you did read the logs. I’m more curious in your comment for you to substantiate

    There were serious concerns raised by respected developers about MTP as recently as two days ago. There are devs who concept NACK’d MTP. Were these devs comprehensively polled for their opinions in light of this new change?

    The two NACKs I am aware of are from Luke and from Rusty. I cannot personally say that their NACKs are irrelevant, but it seems to be the sentiment among developers given that Luke has declined to substantiate his NACK with a technical argument e.g.:

    16:36 < achow101> luke-jr: michaelfolkson: Can you articulate technical reasons why you are opposed to MTP? I get that heights were agreed upon previously, however things are not static. There have been changes proposed to the MTP implementation that would allow for a UASF. 16:36 < luke-jr> jeremyrubin: miners don’t decide 16:37 < roasbeef> luke-jr: I think that’s been discussed ad nauseam 16:37 < jeremyrubin> we’ve used a coin flip to break a stalemate so people feel comfortable spending review energy, and then the proposal isn’t “decided” it’s then put forth for clients to upgrade to, and even then it’s not final, miners have to signal 16:37 < luke-jr> jeremyrubin: achow’s PR is already 100% reviewed 16:37 < luke-jr> there is also zero reason to even cosnider MTP 16:37 < achow101> luke-jr: why?

    https://twitter.com/JeremyRubin/status/1379591448425734148

    etc

    It seems not to matter much W.R.T. achieving consensus for luke, as whatever client is released by core, Luke seems dead set on preparing a UASF client with chain split risk (which has been NACK’d by many many contributors for years and years now, speaking of ignoring consensus).

    & WRT to rusty’s NACK, it’s been addressed here https://gist.github.com/michaelfolkson/92899f27f1ab30aa2ebee82314f8fe7f#gistcomment-3695024 as well as on the mailing list. Further, Rusty’s NACK is not technical in nature, it’s procedural, as he’s previously noted that the activation mechanisms themselves are probably sufficient for activating, he just dislikes the precedent.

    Are there other NACKs I am missing?

    In contrast to the NACKs, there are many ACKs on a speedy trial w.r.t. harding’s post https://gist.github.com/michaelfolkson/92899f27f1ab30aa2ebee82314f8fe7f for either MTP or Height, many ACKs for this approach during the meeting, and a myriad of developers who are working towards reviewing AJ’s pull request to ensure it is technically sound. Further, it’s not new quantum physics, it is minor tweaks to an existing activation mechanism. Activation mechanisms do not generally have the same bar for consensus as do changes to bitcoin like Taproot because we aren’t “stuck with them” beyond the single instance they are used. So whereas deep technical consensus on e.g. Taproot is much more critical, the activation code we’re working on need only work once (that doesn’t mean insecure code would pass review). Lastly, most users/developers/etc seem to be more concerned with when taproot will be available, as opposed to if it is MTP or height. As such the meeting’s have been making an effort to coordinate review for a release sometime this month, so Taproot can be active in November.

    Lastly I just want to note that I do appreciate your concern for wanting to make sure voices are heard and represented, hence I’ll repeat my earlier question to check and see if you have any technical concerns with the current proposal you feel have not been addressed as of yet.

  60. michaelfolkson commented at 10:10 am on April 8, 2021: contributor

    @jeremyrubin on block height vs MTP:

    March 8th “I have a preference for fully height based design, correct.”

    March 24th “There are two NACKs, one (luke-jr) against MTP, one (jeremyrubin) against height.”

    April 6th “The following folks in the meeting agreed to abide by the flip…. jeremyrubin"

    With you we have gone from preferring height to literally NACKing height to thinking height vs MTP is bikeshedding and worthy of a coin flip.

    AJ Towns has Approach NACKed height based on test network considerations. He wasn’t listed as one of those “abiding by the flip”

    There are multiple people who have expressed a preference for block height but don’t feel strongly enough about it to Approach NACK a PR using a mix of MTP and block height. There are two StackExchange answers here discussing block height vs MTP.

  61. maaku commented at 11:46 am on April 8, 2021: contributor

    The ACK/NACK system is broken so I stopped using that terminology after the segwit debacle. But it seems I have no voice here unless I state my position clearly in those terms, so here it is:

    I’m NACK taproot, for reasons I’ve published elsewhere. Counter-factually assuming that were not the case, then I’m NACK speedy trial for the same reasons Rusty has given on the mailing list. Now finally assuming that I was for taproot via speedy trial, then I’d be NACK MTP and approach-ACK a BIP8(LOT=false) with just a few technical concerns regarding thresholds and start/stop heights that I’m ultimately flexible on.

    Segwit activation exposed a number of problems with the BIP9 activation state machine which was based on MTP windows. These concerns include:

    • Difficulty of interfacing with in-parallel activation mechanisms like BIP148, BIP91, or the presumed future LOT=true UASF for taproot. (You may not like UASF. You may be against UASF and wish is never happened. Well reality doesn’t care. There will be a UASF client, that is simply a fact. That cat is out of the bag. Our job is to make sure that UASF risks are minimized for the sake of the network, which height-based activation mechanisms do better. I’m not saying MTP can’t be UASF compatible, I’m just saying it’s way easier to avoid foot-gunning a UASF deployment with height-based windows. And while your inner schadenfreude might laugh at a UASF foot-gun, such a chain-splitting failure affects everyone.)

    • Perverse timestamp-manipulation incentives for miners, who can use timestamp manipulation to slow or speed up MTP to delay or speed up the closing of an activation window. People rely on accurate MTP timestamps for things like lock times, and of course it affects difficulty adjustments, and so it is very bad for there to be incentives for miners to manipulate timestamps.

    • Two-week uncertainty over activation window start or end times based on alignment of 2016-block window with calendar dates. Makes it very difficult for people to schedule travel or events around a single likely activation date (+/- a day or two). This affects not just activation parties (which is a silly reason IMHO, even more so in the midst of a pandemic), but also mining and infrastructure organizations which will want staff at hand in case there are activation issues.

    • BIP8 was specifically created to codify our lessons learned from activating segwit into a new activation protocol to be used in the next soft-fork. It got significant review 4 years ago and in the time since, and was strengthened by that review. There is no way that some new state machine for MTP-based activation could receive the same amount of review in the speedy trial timeline. We should use the reviewed mechanisms that are already in place (or very nearly so) and which incorporate lessons learned. If you think there are flaws in BIP8(LOT=false), then start working now on an alternative activation mechanism for the next soft-fork.

    Every single one of the above points has been made on the mailing list or in relevant Github discussions. I don’t think that any of them have been adequately addressed by MTP advocates. For example, pointing out that “timewarp could cause more uncertainty than +/- 2 weeks” ignores a few facts: (1) timewarp is NOT a normal condition and would itself be seen as an attack on the network; (2) timewarp would require the coordination of more miners than it would take to block taproot anyway; and (3) having monotonically decreasing estimation error without a two-week floor is critical to scheduling needs.

    I think many of us who were involved in that prior work that went into segwit lessons learned and BIP8 review are sitting around dumbfounded and speechless at what has happened. What is the point of doing all that advanced planning if it’s immediately thrown away precisely when it is needed? I’m not surprised if Luke-Jr and Jorge seem obstinate on this. They’ve invested literally years on this problem so that we’d have a solution when it is needed, as it is now, and getting buy-in from the developers at the time… only to have all that casually thrown away. (Because of a freaking coin flip?)

    So instead of doing useful work, I am here getting nerd-sniped into a technical discussion about MTP-vs-height which I thought was settled four years ago. And you know what, I have better things to do so I won’t be commenting further and won’t bother reviewing #21377. It has a concept-NACK from me.


    But the matter of process there are some things here that are important to discuss. We have always, always held the mailing list to be the ground truth for consensus in theory, and in practice have used GitHub for this purpose as well. Consensus decisions have not been made in person or on IRC. Discussion happens, of course, but not polling of developer consensus. That happens on Github and via mailing list discussions.

    To have an IRC meeting (at an inconvenient time for many participants!) make a consensus-affecting decision about Bitcoin Core is ridiculous. To have done by coin flip is just plain insulting to those of us who have actually invested review effort into this and have strong technical arguments for one side or the other. If it was a joke it was in bad taste.

    Nothing is wrong with @achow101 and @ajtowns deciding what they want to focus their time and effort on based on an IRC discussion. That’s cool. But don’t assume it is more than that. There are a lot more stakeholders involved.

    Btw @JeremyRubin, should is a synonym for must in technical and procedural contexts.

  62. michaelfolkson commented at 12:03 pm on April 8, 2021: contributor
    Thanks for this @maaku. I will update my StackExchange answer based on some of the above. I apologize (to the extent I personally bear responsibility) for wasting your time.
  63. harding commented at 12:39 pm on April 8, 2021: contributor

    Difficulty of interfacing with in-parallel activation mechanisms like BIP148, BIP91, or the presumed future LOT=true UASF for taproot.

    AJ has a branch for using UASF with MTP: https://github.com/ajtowns/bitcoin/commit/f802805cae80df66dcc8b874ce3d2d801cb9627e

    Perverse timestamp-manipulation incentives for miners, who can use timestamp manipulation to slow or speed up MTP to delay or speed up the closing of an activation window. People rely on accurate MTP timestamps for things like lock times, and of course it affects difficulty adjustments, and so it is very bad for there to be incentives for miners to manipulate timestamps.

    I know you said you’re not planning on commenting further, but I’m unaware of these incentives in the absence of a UASF. If a UASF is done with AJ’s branch above, then all miners can do is delay activation—which they can also do with a height-based approach by not mining blocks. Arguably, it’s better to use the MTP mechanism that allows miners to continue producing blocks at a reasonable rate for a while (until difficulty changes kick in, assuming they don’t timewarp) so the economy keeps functioning than it is to lose that hash rate entirely.

    Two-week uncertainty over activation window start or end times based on alignment of 2016-block window with calendar dates.

    This is the first I’ve heard of anyone saying it was important to them to staff for start and end times. I’ve only previously heard (and expressed) this concern related to activation time, which in BIP9 and 21377 are based on heights (BIP9 a relative 2016 blocks; 21377 a static min_activation_height, if used). BIP8 also uses a relative 2016 blocks for voluntary miner activations and a static activation height for forced activations, so I’m not sure what your concern is here regarding activation.

    BIP8 was specifically created to codify our lessons learned from activating segwit into a new activation protocol to be used in the next soft-fork. It got significant review 4 years ago and in the time since, and was strengthened by that review.

    BIP8 has also been modified for speedy trial, so I don’t think the argument for conservatism is well founded.

  64. maaku commented at 6:49 am on April 12, 2021: contributor

    Someone asked me out of band if my concerns had been addressed here, apparently since it was referenced on #21377 and on IRC. No I don’t feel my concerns have been addressed.

    AJ has a branch for using UASF with MTP: ajtowns@f802805

    I know that UASF with MTP is possible, so there’s no new information here. What I said was that it’s not as simple. This is because, as you note, all considered activation mechanisms involve a relative height-based lockin period. This means that for mixed MTP-based activation schemes it is impossible to pick a single absolute height or time beyond which you can be certain that the activation mechanism has failed.

    This is unlike a fully height-based solution like BIP8 where adding UASF can be as simple as a literal one-liner, turning if (VVersionBitsState(...) == ACTIVE) into if (VVersionBitsState(...) == ACTIVE || height > UASF) where UASF is the height of activation after lockin following the last signaling period.

    That’s not how LOT=true works in BIP8, but it would be a totally safe UASF activation mechanism if Bitcoin Core were to release with BIP8(LOT=false). And my point is that we have a responsibility to either (a) offer LOT=true as a configuration parameter (Rusty’s argument, and my own); or (b) design activation mechanism which have these sorts of perfectly safe, trivial to review UASF mechanisms built-in.

    Also I think this point need making: there will be a UASF client. I don’t know if there will be a UASF movement like we saw last time, but there will be a UASF client. The lesson I learned from 2017 is that “devs propose, miners activate, users override” isn’t statement about fallback mechanisms but rather how things need to be structured from the beginning. I was not the only one to learn this lesson.

    The only reason I won’t be making a UASF client myself or having my miners run it is because I think taproot shouldn’t activate. But there are others from that 2017 time who I won’t name who feel the same as I do regarding lessons learned and who I have no doubt will make and run a UASF client.

    What we don’t know now is whether the UASF movement will be fridge, principled radicals or a real movement like it was in 2017. But to assume that there will be NO UASF client is a fantasy.

    I’m unaware of these incentives in the absence of a UASF.

    You can use timewarp to keep MTP in the past prior to the start point, and keep it there for the entire duration, and then zoom it forward to the present with a mere six blocks once the wall clock has exited the activation window. That would skip activation entirely.

    It’s an unlikely outcome I think because it takes less hash power to simply block activation than to execute a timewarp, and the timewarp is more destructive. But we should nevertheless choose designs that have defense in depth against possible attacks, on the justification that we can’t see all possible motivations for executing such an attack. E.g. maybe a miner wants to do a timewarp attack anyway (to play out subsidy) and then uses blocking taproot as more agreeable excuse for why they did it, to try and get away with it.

    It’s better to use height-based activation windows because it blocks off the possibility of an activation-related timewarp entirely, instead of a fragile argument about miner incentives.

    This is the first I’ve heard of anyone saying it was important to them to staff for start and end times.

    Any state machine transition should be watched in realtime, not just activation. Especially if it is a first-time deployment using a new state machine.

    BIP8 has also been modified for speedy trial, so I don’t think the argument for conservatism is well founded.

    The modification for speedy trial could be as simple as extending the lock-in period from one 2016 block period to twelve, or approximately six months. That would be, again, a literal one-liner change to BIP8 and does not modify the state machine structure itself. My conservatism is over the size of the code delta from BIP8 or BIP9.

  65. harding commented at 9:31 am on April 12, 2021: contributor

    I know that UASF with MTP is possible, so there’s no new information here. What I said was that it’s not as simple.

    Actually, you said height was “way easier” than MTP. I think that may have been the case with BIP148 where signaling had to complete before BIP141’s stop time, but AJ’s revised code eliminates stop time as an issue and so I think the review burden between MTP and height is much closer to parity now. That was a key part of the recent compromise and, given your responses, I suspect it is actually new information to you.

    It seems to me the slight difference in MTP vs height activation of a UASF is insignificant compared to all the other code, documentation, outreach, testing, etc that needs to be performed to ensure regular users don’t lose money because of a disagreement over activation between some users and some miners. Given that comparative insignificance of total effort, I think it’s reasonable for people to consider using MTP rather than height when MTP provides them other advantages they want.

    adding UASF can be as simple as a literal one-liner […] if (VVersionBitsState(...) == ACTIVE || height > UASF)

    That oneliner is also compatible with the original BIP9 and with #21377. To maintain compatibility with signets, the UASF parameter could be set to height UINT_MAX and the signet administrators could activate using versionbits.

    You can use timewarp to keep MTP in the past prior to the start point, and keep it there for the entire duration, and then zoom it forward to the present with a mere six blocks once the wall clock has exited the activation window. That would skip activation entirely.

    Not in AJ’s MTP UASF design, as I mentioned in my previous reply (“with AJ’s branch above, then all miners can do is delay activation”).

  66. maaku commented at 10:08 am on April 12, 2021: contributor

    Actually, you said height was “way easier” than MTP.

    Maybe I’m too much of an engineer, but “easier” and “simpler” are synonymous to me, when we’re talking about design space. Anyway sorry for not being consistent in my language.

    AJ’s revised code eliminates stop time as an issue and so I think the review burden between MTP and height is much closer to parity now. That was a key part of the recent compromise and, given your responses, I suspect it is actually new information to you.

    As short as his revised diff is (and it’s not a one-liner), it’s still mixing MTP and heights so it’s not safe under all edge cases, like extreme divergence of inter-block time from expected. You’re comparing apples to oranges.

    Yes you can do a similar one-liner with #21377. But with some bad choices of parameters and/or some serious chain manipulation by miners you end up in edge cases with undesired outcomes. These are edge cases that are supremely unlikely to happen by chance, but that doesn’t mean we can ignore them. There may be entities out there that want to timewarp the chain (for that sweet, sweet subsidy) and are looking for an excuse. Don’t give it to them.

    As I mentioned before, using heights for all threshold transitions cuts off this attack surface entirely. It’s a strictly safer construct.

    That oneliner is also compatible with the original BIP9 and with #21377.

    No, it’s not. No matter what value you pick for the absolute height or MTP of UASF activation, it’ll be something that could overlap with the final activation period of BIP9 or #21377 in weird edge cases (mostly due miner manipulation of block times).

    You have to go either all-MTP for all state transitions, or all-height to permanently fix this and remove the miner incentive to perform timewarps. Since we’re pretty fixed on 2016 block adjustment windows for very good engineering reasons, it’s easier to go all-heights than all-MTP. So that is what BIP8 did.

    To maintain compatibility with signets…

    This requirement never made sense to me. We’re talking about signets that don’t yet exist. Any future signet could simply activate taproot from genesis. Make that the default.

    (Also, listen to Greg Sanders. He and I have operated plenty of signets over the years, and every one of them had a truckload of configuration options that shipped along with them. There’s literally no point to an all-default signet. Just use regtest for that.)

    Not in AJ’s MTP UASF design, as I mentioned in my previous reply (“with AJ’s branch above, then all miners can do is delay activation”).

    Are you talking about the fact that AJ’s design forces at least one consideration period? We’re already conditioning this discussion on a timewarp attack being executed, which requires enough of a mining cartel to also block taproot activation. So I’m not sure why you think it would still activate?

    If a cartel large enough to cause a timewarp wants to block taproot, they will. So don’t give them a reason to.

  67. harding commented at 12:35 pm on April 12, 2021: contributor

    As I mentioned before, using heights for all threshold transitions cuts off this attack surface entirely. It’s a strictly safer construct.

    Again, I agree it’s easier to analyze its safety, but I think the relative difference between it and MTP in the context of all the other things that need to be done for safety’s sake is small, so it’s reasonable to consider the other impacts these parameters have on users and developers. If we always chose what was easier to analyze, we wouldn’t be adding new features to Bitcoin at all.

    That oneliner is also compatible with the original BIP9 and with #21377.

    No, it’s not. No matter what value you pick for the absolute height or MTP of UASF activation, it’ll be something that could overlap with the final activation period of BIP9 or #21377 in weird edge cases (mostly due miner manipulation of block times).

    Why is overlap an issue for a flag height? If a flag node already accepts the risk of falling out of consensus, it can start enforcing new rules before other nodes have finished waiting for a signal to also start enforcing those rules.

    Any future signet could simply activate taproot from genesis. Make that the default.

    Taproot is already activated by default on new signets from genesis, which was convenient to do because signet was released in the same version of Bitcoin Core that contained taproot’s code (minus activation code). The concern is that signets being created now will require every one of their users to run with custom parameters (or a custom client) if they want to use future height-activated soft forks on that network. That’s a coordination headache that would be nice to avoid.

    Are you talking about the fact that AJ’s design forces at least one consideration period? We’re already conditioning this discussion on a timewarp attack being executed, which requires enough of a mining cartel to also block taproot activation. So I’m not sure why you think it would still activate?

    We’re talking about UASF where activation is mandatory. In an MTP UASF, that guaranteed consideration period becomes the mandatory signaling period, so (As I said) all miners can do is delay. In my original response, I also noted: “[miners] can also [delay] with a height-based approach by not mining blocks. Arguably, it’s better to use the MTP mechanism that allows miners to continue producing blocks at a reasonable rate for a while (until difficulty changes kick in, assuming they don’t timewarp) so the economy keeps functioning than it is to lose that hash rate entirely.”

    (As an aside, I don’t think timewarp is a realistic concern. It’s easy to detect, it’s a problem for all Bitcoin users, it’s easy to block on an ad hoc basis, and it’s easy to deploy a tradeoff-free soft fork permanently eliminating it as a concern. At least it’s easy if we can agree on an activation mechanism :-)

  68. DrahtBot locked this on Aug 18, 2022

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-12-03 15:12 UTC

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