I hard coded ISM SF in regtest and adapted the tests. Will try a sync on mainnet and testnet soon.
Note that even if the activation threshold (75% which enforce a rule for block version above a number) and enforcement threshold (95% reject old versions) are different, only the enforcement one really matter post fork.
2375 }
2376
2377 // Start enforcing CHECKLOCKTIMEVERIFY, (BIP65) for block.nVersion=4
2378 // blocks, when 75% of the network has upgraded:
2379- if (block.nVersion >= 4 && IsSuperMajority(4, pindex->pprev, chainparams.GetConsensus().nMajorityEnforceBlockUpgrade, chainparams.GetConsensus())) {
2380+ if (block.nVersion >= 4 && pindex->nHeight >= chainparams.GetConsensus().BIP65Height) {
829 {
830- if (pstart->nVersion >= minVersion)
831- ++nFound;
832- pstart = pstart->pprev;
833+ case 2:
834+ activated = consensusParams.BIP34Height >= pindex->nHeight;
816@@ -817,22 +817,23 @@ UniValue verifychain(const UniValue& params, bool fHelp)
817 }
818
819 /** Implementation of IsSuperMajority with better feedback */
820-static UniValue SoftForkMajorityDesc(int minVersion, CBlockIndex* pindex, int nRequired, const Consensus::Params& consensusParams)
821+static UniValue SoftForkMajorityDesc(int minVersion, CBlockIndex* pindex, const Consensus::Params& consensusParams)
249- consensus.nMajorityRejectBlockOutdated = 950;
250- consensus.nMajorityWindow = 1000;
251- consensus.BIP34Height = -1; // BIP34 has not necessarily activated on regtest
252+ consensus.BIP34Height = 100000000; // BIP34 has not necessarily activated on regtest
253 consensus.BIP34Hash = uint256();
254+ consensus.BIP65Height = 1351; // BIP65 activated on regtest
2366@@ -2372,15 +2367,13 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
2367
2368 unsigned int flags = fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE;
2369
2370- // Start enforcing the DERSIG (BIP66) rules, for block.nVersion=3 blocks,
2371- // when 75% of the network has upgraded:
2372- if (block.nVersion >= 3 && IsSuperMajority(3, pindex->pprev, chainparams.GetConsensus().nMajorityEnforceBlockUpgrade, chainparams.GetConsensus())) {
2373+ // Start enforcing the DERSIG (BIP66) rule
2374+ if (pindex->nHeight >= chainparams.GetConsensus().BIP66Height) {
block.nVersion to be below 3 at this point. I’m using the 95% threshold for this test (compared to the 75% of before), so if the block.nVersion were below 3 it would have been rejected before by https://github.com/bitcoin/bitcoin/pull/8391/files#diff-7ec3c68a81efff79b6ca22ac1f1eabbaR3516
@sipa? Hm? I was just asking about nVersion check.
That said, future refactoring purposes is good enough for me.
170- consensus.nMajorityRejectBlockOutdated = 75;
171- consensus.nMajorityWindow = 100;
172 consensus.BIP34Height = 21111;
173 consensus.BIP34Hash = uint256S("0x0000000023b3a96d3484e5abb3755c413e7d41500f8e2a5c3f0dd01299cd8ef8");
174+ consensus.BIP65Height = 581885; // 00000000007f6655f22f98e72ed80d8b06dc761d5da09df0fa1dc4be4f861eb6
175+ consensus.BIP66Height = 330776; // 000000002104c8c45e99a8853285a3b592602a3ccde2b832481da85e9e4ba182
896@@ -897,13 +897,9 @@ UniValue getblockchaininfo(const UniValue& params, bool fHelp)
897 " {\n"
898 " \"id\": \"xxxx\", (string) name of softfork\n"
899 " \"version\": xx, (numeric) block version\n"
900- " \"enforce\": { (object) progress toward enforcing the softfork rules for new-version blocks\n"
901+ " \"reject\": { (object) progress toward rejecting pre-softfork blocks (same fields as \"enforce\")\n"
244@@ -247,11 +245,10 @@ class CRegTestParams : public CChainParams {
245 CRegTestParams() {
246 strNetworkID = "regtest";
247 consensus.nSubsidyHalvingInterval = 150;
248- consensus.nMajorityEnforceBlockUpgrade = 750;
249- consensus.nMajorityRejectBlockOutdated = 950;
250- consensus.nMajorityWindow = 1000;
251- consensus.BIP34Height = -1; // BIP34 has not necessarily activated on regtest
252+ consensus.BIP34Height = 100000000; // BIP34 has not necessarily activated on regtest
@MarcoFalke I just re added it because without, tests with block v1 on regnet get rejected by the “obsolete version” (https://travis-ci.org/bitcoin/bitcoin/jobs/146661777).
It makes sense as if BIP34Height is -1 then https://github.com/bitcoin/bitcoin/pull/8391/files#diff-7ec3c68a81efff79b6ca22ac1f1eabbaR3515 will kick out.
44- int nMajorityRejectBlockOutdated;
45- int nMajorityWindow;
46 /** Block height and hash at which BIP34 becomes active */
47 int BIP34Height;
48 uint256 BIP34Hash;
49+ /** Block height at which BIP65 becomes active */
Here are my proposed changes in code: https://github.com/jtimon/bitcoin/compare/remove-ism...consensus-post-remove-ism
In any case, utACK 122786d
I’ll do a separate PR on top of this one for using the approach of #8418 alongs with @jtimon suggestion. I prefer keeping this PR as simple as I can.
EDIT: something like https://github.com/NicolasDorier/bitcoin/commit/e4844f776a5048cf7f10c0ec326e9a1ad73cb39f
@morcos and I were discussing this, and I think this change is subtle enough that it deserves some kind of BIP write-up.
Technically this change is a hardfork: old ISM-based code, when presented with an alternate chain history in which one of these softforks activated earlier, would then reject a subsequent block that violated the softfork before the hardcoded heights here, whereas this code would not reject such a block (since the softfork would not yet be enforced). This seems like a fundamental consequence of removing the ISM code (excluding doing something like checkpointing the whole headers chain to detect this kind of thing, which seems like obvious overkill).
In my judgement this is fine, as by the time we deploy this code in 0.14, it’ll have been over a year since the most recent one of these softforks deployed. But to further everyone’s understanding of how these kinds of consensus changes are made (especially for future developers, as well as authors of alternate implementations) I think it’d be beneficial to document this change along with the considerations and rationale that went into it.