Bury bip9 deployments (
consensus) Jun 5, 2019
Softforks have used a variety of deployment methods in the past:
After a soft fork is activated and some time has passed, the exact activation
method is no longer interesting. For example, we don’t care whether segwit
was activated by BIP9, BIP91 or UASF. What we know is that the majority of
economic nodes started enforcing the segwit rules from block 481824.
The ISM soft fork heights were hard-coded in
Bitcoin Core PR
#8391, and documented in BIP
on your definition, this could be considered a ‘hard fork’ since a previously
invalid chain becomes valid. However, for such a chain to be the most-work
chain would involve a multi-year reorg, so such a definition is not very
useful. P2SH and segwit script validation was enforced back to genesis (with one
Bitcoin Core PR
#11739. This PR attempts to hard-code the heights of CSV and segwit activation.
It also changes the format of soft fork reporting in the return object of
Review/test of this PR should include verifying that the hard-coded
activation heights are correct.
8 13:00 <sipa> comcept nack
10 13:00 <jnewbery> welcome all (except sipa, who's trolling)
13 13:00 <pinheadmz> comcept!
14 13:01 <jnewbery> did everyone have a chance to read the notes?
16 13:01 <michaelfolkson> Haha. Is he nacking this channel? Nack overruled
18 13:02 <jnewbery> hi folks. We're here at a Bitcoin Core meeting in Amsterdam
19 13:02 <sipa> i'm on my phone, sitting next to john, mildly intoxicated
20 13:02 <jnewbery> We talked about the review process today. kanzure's transcript may be of interest to people here
21 13:03 <kanzure> sipa is intoxicated by code review
22 13:03 <kanzure> hours and hours of code review today.
24 13:04 <jnewbery> This is the first PR that we've looked at that touches consensus code. It's a pretty minor change, but we always want to be especially careful when touching anything that goes near consensus
25 13:04 <michaelfolkson> That transcript was really interesting. Hopefully people experiment with some of the ideas in it in future
26 13:04 <merehap> I think this channel has to be shutdown if sipa nacks it. Not really sure we have a choice here.
27 13:05 <michaelfolkson> Sorry John, please continue
28 13:05 <jnewbery> No, it's possible to NACK a sipa NACK to neutralize it
29 13:05 <jnewbery> it's a rarely used technique
30 13:05 <moneyball> sipa NACK
32 13:05 <jnewbery> did anyone have any questions about the PR?
33 13:06 <michaelfolkson> Yup
34 13:06 <jb55> what does it mean to bury a deployment
35 13:06 <jb55> start with something simple
36 13:06 <jnewbery> thanks jb55
37 13:07 <jnewbery> I think the terminology was introduced in BIP 90
38 13:07 <jnewbery> the idea is that the soft fork activation has been buried under so much work, that there's no longer any doubt about when it activated
39 13:07 <michaelfolkson> So it is effectively introducing a checkpoint right?
40 13:08 <jnewbery> the logic in the client that tests for activation (whether that's ISM or BIP9) is effectively obsolete
41 13:08 <merehap> Can LOCK(cs_main) be removed from IsNullDummyEnabled? It looks like it. It was already removed from the similar IsWitnessEnabled.
42 13:08 <jnewbery> so instead of relying on difficult-to-understand logic, we just hard-code the activation height
43 13:09 <pinheadmz> Does this make the bip9 signal bit reuseable? Or is that already true
44 13:09 <jnewbery> michaelfolkson: it's not a checkpoint, but there are similarities in the concept. We're asserting a fact about the valid chain, and hardcoding that into the client.
45 13:10 <jnewbery> but it's not a checkpoint. It doesn't limit the valid chain at that height to a certain block.
46 13:10 <michaelfolkson> Ah ok so a re-org is still possible before that block height but deactivating it at that block height becomes impossible
47 13:10 <peevsie> it's like a retroactive hardfork
48 13:10 <jonatack__> The "what" of any consensus change is interesting. Nevertheless, I found the pros and cons of "why" even more so.
50 13:11 <jnewbery> peevsie: in a narrow technical sense, you could claim this is a hardfork, but I don't think this is a useful definition
51 13:12 <jnewbery> jonatack__: yes, I thought the discussion around whether such a change is desirable was more interesting than the code changes this week
52 13:12 <michaelfolkson> So the reasons why previous attempts failed was mainly lack of engagement? The only criticism seemed to be from Marco on splitting RPC and consensus changes
53 13:12 <jonatack__> yes... the discussion between you, marco and suhas
54 13:13 <jnewbery> merehap: I think you may be right. You should leave a review comment!
55 13:13 <lightlike> jnewbery: in the first PR #11398, you suggested that this might need might need a BIP itself and got some approval. In the subsequent PRs, this was not even a topic. What did change?
56 13:13 <merehap> jnewbery: Will do.
57 13:13 <jnewbery> pinheadmz: bip9 bits are reusable as long as the activation dates don't overlap. That was part of the design motivation for BIP9
58 13:15 <jnewbery> michaelfolkson: yes, I closed the previous PR because I couldn't convince other contributors to review
59 13:15 <jnewbery> I think this change is useful, especially with the upcoming schnorr/taproot softfork changes
60 13:15 <michaelfolkson> Is there a certain time period that we would want since activation before "burying"?
61 13:16 <michaelfolkson> Or does it not matter? 6 months, 12 months, 18 months whatever?
62 13:17 <jnewbery> I think if Bitcoin experienced a 6 month re-org then we might question the usefulness of the system
63 13:17 <jnewbery> so I'd consider a softfork that's buried by 6 months' work as a historic fact
64 13:18 <pinheadmz> Where do you think that line is?
65 13:18 <pinheadmz> Like how big a reorg is really “were done let’s go home”
66 13:18 <jnewbery> I don't know!
67 13:18 <michaelfolkson> Why do you say it is useful for upcoming Schnorr/taproot? Generally a good idea to address code complexity before attempting major change?
68 13:19 <michaelfolkson> The benefits of this are clearing up code complexity and testing right? Time of IBD isn't materially impacted
69 13:20 <jnewbery> Yes. This is just a tidy-up in the way softforks are reported. It also allows us to remove a bunch of tests for stuff that is no longer needed
70 13:21 <jnewbery> I think p2p_segwit.py and p2p_compactblocks.py
71 13:21 <jnewbery> lightlike: I don't know if a BIP is required. I think a post to the mailing list is probably sufficient
72 13:23 <michaelfolkson> Is there a good resource explaining the different activation methods? I didn't know what flag day, IsSuperMajority were.
73 13:23 <jnewbery> I don't know if there's a single resource that describes them all
75 13:25 <jnewbery> I've given talks on the history of consensus changes before. Perhaps I'll turn that into a video or blog post at some point
76 13:26 <michaelfolkson> Cool, thanks
77 13:26 <jonatack__> Kudos on being persistent. Taking note of the good idea to recap the ACKs from the previous PRs to rope in support.
78 13:27 <jnewbery> until then, the links to the BIPs in the notes describe all the activation methods
79 13:28 <michaelfolkson> Yeah... As a thought experiment from <kanzure> <moneyball> discussion is there anything to try to get this pushed through?
80 13:28 <jnewbery> thanks jonatack__. Nagging is an important skill to cultivate :)
81 13:29 <jnewbery> michaelfolkson: f2f nagging is my next tactic
82 13:29 <michaelfolkson> Assuming it is agreed that PR doesn't need splitting between RPC/consensus
83 13:30 <merehap> I think that a lot of the cases where different decisions were made in very similar situations tend to be disorganized in most peoples' minds. If you were to make a blog post that gets everything about historical activation methods in one place, I think a lot of people would benefit. I'd certainly be happy. I think blog posts are better than videos for that kind of thing since they are easily skim-able/reference-able.
84 13:31 <jonatack__> f2f ?
85 13:31 <jnewbery> face to face
87 13:32 <jnewbery> merehap: I'm planning to give a talk on this at our upcoming residency. I'll try to turn it into something more widely usable
88 13:32 <merehap> Awesome! Greatly appreciated.
89 13:32 <jonatack__> If we segue at some point to general questions, I have one about timestamping reviews.
90 13:33 <lightlike> i was a bit confused with the activation heights in test: in regtest, default heights are CSV:432, Segwit:0 (always active unless overridden). But in the tests, activation height for segwit seems to be hardcoded to 432 is as well. Why are CSV and segwit coupled like that?
91 13:33 <sipa> it's not hardcoded to 432; it just has bip9 start time in the past
92 13:33 <michaelfolkson> <merehap> <jnewbery> Cool. What they were and the pros and cons of each, followed by which is likely to be used in future
93 13:33 <sipa> so at block 144 (first period), it becomes active
94 13:33 <sipa> and blocm 288 it's locked in
95 13:34 <sipa> at block 432 it's active
96 13:35 <jnewbery> I can't remember exactly why the PR doesn't change CSV actication height to 0. It might have been too much effort to change the tests
97 13:36 <jnewbery> michaelfolkson: Discussion of the pros/cons of activation method is a bit too broad to be discussed here
98 13:36 <michaelfolkson> Oh no I mean in the blog post
100 13:37 <jnewbery> jonatack__: go ahead with general questions
103 13:39 <jonatack__> to ensure GitHub doesn't change your review
104 13:39 <jonatack__> idk if Marco is afk
105 13:40 <jnewbery> MarcoFalke isn't here
106 13:40 <jonatack__> if away at least he might see it
108 13:40 <jnewbery> but feel free to ask in #bitcoin-core-dev !
109 13:41 <sipa> he's about 2.5 metets behind you, jnewbery
111 13:41 <jonatack__> * suspense builds *
112 13:41 <jnewbery> That goes to everyone here. Please feel free to post in #bitcoin-core-dev if you have any questions. People are friendly there!
113 13:42 <jnewbery> he's just charged his wine glass. I wouldn't want to interupt a man with opentimestamps questions in such a condition.
114 13:44 <jnewbery> alright, any more questions?
115 13:44 <merehap> So there are a lot of magical int values used in these code sections for special block height conditions, potentially leading to missed error case handling cases. Would a drive to make some kind of int wrapper type for block heights that has helper methods exposing the special cases be well received? Or is that just the type of refactoring of consensus code that is considered too dangerous? Example magical block heights i
116 13:44 <merehap> nclude null, -1, and max int here: "consensusParams.SegwitHeight != std::numeric_limits<int>::max()".
117 13:45 <jnewbery> merehap: I wouldn't personally be interested in reviewing such a PR :)
118 13:46 <merehap> (In general my potential value-add to a project is figuring out compile time ways to eliminate or reduce missed edge case, which is why I'm asking)
119 13:46 <jnewbery> The payoff doesn't seem worth the codechurn/review burden
120 13:46 <merehap> Makes sense. Is that in general for Bitcoin Core, or more just for consensus critical stuff?
121 13:47 <jnewbery> Mostly consensus-critical stuff, but in general too
122 13:47 <michaelfolkson> I have some on setting up your review process from last week John but let's leave that for another week. Or maybe I could just post in #bitcoin-core-dev.
123 13:47 <merehap> Gotcha
124 13:48 <sipa> morcos is applauding jnewbery's dedication to the cause
125 13:48 <michaelfolkson> Let's wrap up. I feel we're delaying the drinking haha
126 13:49 <jonatack__> I began setting up a twitter/mastodon bot for live Bitcoin Core PR activity and code reviews. The goal is to encourage quality review and reviewers.
127 13:49 <jnewbery> My thoughts about opening PRs: no-one owes you a review. Anyone who reviews your code is doing you a favour. If you open a PR, you're competing with other PRs for review time.
128 13:49 <jonatack__> Anyone against that idea please let me know ;)
129 13:49 <michaelfolkson> Sounds good to me
131 13:49 <jnewbery> That shouldn't discourage you from opening PRs, but you should think about how to prioritize your PR against the other open PRs
132 13:51 <jnewbery> if in doubt about how useful other people think your PR will be, feel free to ask in #bitcoin-core-dev, or by directly asking other contributors
133 13:51 <michaelfolkson> Cool, thanks everyone, thanks John. Thanks <sipa> for your guest appearance
134 13:51 <jonatack__> The bot could hook into the PR reviews club website to broadcast the meetings if that is ok
135 13:52 <jnewbery> thanks jonatack__. I'm following!
136 13:52 <jonatack__> Thanks!
137 13:53 <jnewbery> Any other questions?
138 13:53 <pinheadmz> cheers!
139 13:53 <jnewbery> Did anyone build and test `getblockchaininfo` ?
140 13:54 <lightlike> i did.
141 13:55 <merehap> jnewbery: Yeah, I just want to figure out how we deal with potentially compile-time preventable bugs like the duplicate block validation bug ( https://nvd.nist.gov/vuln/detail/CVE-2018-17144). That kind of thing can't be statically prevented with the current code base, but there seems to be no feasible way to get the code base into a more compile-time safe state.
142 13:55 <jnewbery> lightlike: great!
143 13:56 <merehap> The tests didn't get that far for me, feature_dbcrash.py failed first unfortunately. I could manual run that as a one-off though.
144 13:57 <jnewbery> merehap: I think you're right thatCVE-2018-17144 can't be statically prevented.
145 13:58 <jnewbery> I don't know how you'd catch it with testing
146 13:58 <jnewbery> merehap: that's a shame. That test isn't run by travis, so I wouldn't be surprised if there are intermittent failures in master
147 13:58 <merehap> But could be, with a moderate amount of refactoring/better class design. But that introduces different refactor risks, and refactor risks are what caused the CVE in the first place.
148 13:59 <merehap> Basically by encapsulating all the validation into an object, and passing that validated object around, you can avoid cases like the CVE.
149 13:59 <merehap> But it's probably a bridge too far.
150 13:59 <jnewbery> refactors are ok if they're motivated. I think what people react agaionst is when someone opens a PR to refactor something without any obvious reason why it's an improvement
151 14:00 <jnewbery> merehap: seems very ambitious!
152 14:00 <jnewbery> ok, let's wrap it up there
153 14:00 <jnewbery> Thanks everyone! Same time again next week
154 14:00 <merehap> Thanks jnewbery! Informative as always.
155 14:01 <pinheadmz> have fun at 'Breaking!
156 14:01 <jonatack__> Thanks jnewbery, sipa, michael and everyone!
157 14:01 <lightlike> thanks!
158 14:01 <merehap> Also thanks to those who took the rare step of overruling a sipa NACK so we could have this discussion.
161 14:03 <michaelfolkson> Sure I would
162 14:03 <peevsie> thanks all!