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
90. Depending
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
exception) in 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
getblockchaininfo.
Review/test of this PR should include verifying that the hard-coded
activation heights are correct.
<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
<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.
<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
<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?
<michaelfolkson> Why do you say it is useful for upcoming Schnorr/taproot? Generally a good idea to address code complexity before attempting major change?
<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
<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.
<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?
<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
<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)
<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.
<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.
<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.
<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
<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.
<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.
<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