The PR branch HEAD was e72e062 at the time of this review club meeting.
Notes
A soft fork is a forwards-compatible change to the Bitcoin consensus rules.
Any block or transaction that was invalid for nodes that donât implement the
new soft fork rules will be invalid for nodes that do implement the soft
fork rules.
There have been many consensus changes in Bitcoinâs 12 year history. A BitMEX
research blog post lists
those consensus changes.
Changing Bitcoinâs consensus rules requires coordination across a decentralized
network. We want as close as possible to 100% of economic nodes to
start enforcing the new consensus rules and as close as possible to 100% of
miners to be mining valid blocks according to the new consensus rules.
There have been several different methods to coordinate these changes to the
consensus rules:
Flag Day upgrades were used to activate early consensus rules changes.
Developers simply chose a time after which the new rules would be enforced.
IsSuperMajority was used for three changes to the consensus rules between
2012 and 2015. This activation method is described in
BIP34.
Version Bits was used to activate two changes to the consensus rules between
2015 and 2017. The method is described in
BIP9.
Once a change to the consensus rules has been activated and buried under many
months or years of work, the soft fork activation height is established fact.
We assume that a competing fork of the chain will not re-org back to before the
activation and change the activation height.
At that point, the exact method of activation is no longer interesting, and
any logic in the node that enforces that activation method is technical debt.
We can hard code the activation height into the nodeâs logic. This is called
âburyingâ the deployment method, and is documented in
BIP90.
Bitcoin Core PR8391 buried
the IsSuperMajority soft fork deployments (BIP34, BIP65, and BIP66).
PR16060 later buried the
version bits soft fork deployments (CLTV and segwit).
Both of those PRs required changing multiple code paths in the validation
logic. In a review
comment,
Pieter Wuille suggested abstracting out the difference between a soft fork
activation and a buried deployment, so that burying deployments in future wouldnât
require so many changes to different code paths.
This PR does exactly that, by introducing
Deployment{ActiveAfter|ActiveAt|Enabled} functions that are defined for both
version bits deployments and buried deployments.
<amiti> approach ACK ! I think its a clever way to make it simple to bury deployments & the way the enums / functions are defined provide some safety around accidentally mixing them up
<jnewbery> glozow: right, two soft forks have been activated using version bits. What I meant to ask was "how many version bits deployments are defined in the code after this PR?"
<amiti> with the version bits deployments there are more logical forks.. check the state to decide what validation rules to apply to a txn / block. with buried deployment you just assume the new rules apply
<glozow> if the code is still in there, IBD nodes will use it to calculate when to activate stuff when they're catching up. could it be possible to accidentally introduce a bug while we're touching it, and affect IBD nodes?
<jnewbery> b10c: that's not what I was thinking, but it is an interesting point. A very deep reorg could result in a consensus failure. We're talking a reorg of many years worth of blocks, and if that happens we have other problems
<maqusat> generally easier burying, less code changes. does it also allow for earlier testability (regtest)? seen that mentioned in one of original comments with motivations
<jnewbery> BIP 90 included performance as a motivation. The activation method before BIP 9 was ISM, and that was inefficient. Burying those ISM deployments is a performance win.
<jnewbery> So the original motivation for BIP 90 was both performance and code simplicity. The performance part isn't such a consideration for burying BIP 9 deployments, but the code simplicity motivation is still there
<jnewbery> prayank: there have been buried deployments in Bitcoin Core since BIP 90 was implemented. This PR simply refactors the code to make it simpler to bury future deployments
<jnewbery> let's keep moving! Since C++11, we generally prefer to use scoped enumerations. What are the advantages of using scoped enums? Are they used in this PR? Why?
<maqusat> determine if a deployment has a block height assigned. probably could return false for deployments that have got buried without ever being activated (not having block height assigned), though probably it's cleaner to remove such from the code altogether?
<glozow> All of the buried deployments have assigned DeploymentHeights that are not `std::numeric_limits<int>::max()`, so the condition should always return true.
<jnewbery> Next question. If the taproot soft fork is activated, and sometime later we want to bury that activation method, what changes need to be made to the code?
<ccdle12> apologies if this is a silly question but.. with burying taproot, I think theres still some taproot_active bools floating around, would those have to be removed as well to constitute a bury?
<jnewbery> sugarjig amiti maqusat: exactly! There are a couple more small changes, but that's essentially it. Crucially, I don't think you'd need to change any logic in validation.