Merge pull request #7 from luke-jr/bip8_lottrue-0.21

Bip8 lottrue 0.21

doc/bips.md

@@ -1,6 +1,6 @@
 BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.21.0**):
 
-* [`BIP 8`](https://github.com/bitcoin/bips/blob/master/bip-0008.mediawiki): The changes for parallel, rapid deployment based on block height miner activation have been implemented since **v0.21.1** ([PR #21392](https://github.com/bitcoin/bitcoin/pull/21392)). The UASF fallback with forced signaling (`LOT=true`) has not yet been implemented. The current implementation is the equivalent of `LOT=false`.
+* [`BIP 8`](https://github.com/bitcoin/bips/blob/master/bip-0008.mediawiki): The changes for parallel, rapid deployment based on block height miner activation with UASF failover have been implemented since **v0.21.1**  ([PR #19573](https://github.com/bitcoin/bitcoin/pull/19573)).
 * [`BIP 9`](https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki): The changes allowing multiple soft-forks to be deployed in parallel have been implemented since **v0.12.1**  ([PR #7575](https://github.com/bitcoin/bitcoin/pull/7575)) Support was removed in **v0.21.1** ([PR #21392](https://github.com/bitcoin/bitcoin/pull/21392)).
 * [`BIP 11`](https://github.com/bitcoin/bips/blob/master/bip-0011.mediawiki): Multisig outputs are standard since **v0.6.0** ([PR #669](https://github.com/bitcoin/bitcoin/pull/669)).
 * [`BIP 13`](https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki): The address format for P2SH addresses has been implemented since **v0.6.0** ([PR #669](https://github.com/bitcoin/bitcoin/pull/669)).

doc/release-notes-21392.md

@@ -4,4 +4,4 @@ Low-level changes
 RPC
 ---
 
-* BIP 9 has been replaced with a partial implementation of BIP 8. This change is reflected in `getblockchaininfo` where references to BIP 9 have been replaced with references to BIP 8.
+* BIP 9 has been replaced with BIP 8. This change is reflected in `getblockchaininfo` where references to BIP 9 have been replaced with references to BIP 8.

src/chainparams.cpp

@@ -457,11 +457,12 @@ class CRegTestParams : public CChainParams {
     /**
      * Allows modifying the Version Bits regtest parameters.
      */
-    void UpdateVersionBitsParameters(Consensus::DeploymentPos d, int startheight, int timeoutheight, int min_activation_height)
+    void UpdateVersionBitsParameters(Consensus::DeploymentPos d, int startheight, int timeoutheight, int min_activation_height, bool lockinontimeout)
     {
         consensus.vDeployments[d].startheight = startheight;
         consensus.vDeployments[d].timeoutheight = timeoutheight;
         consensus.vDeployments[d].m_min_activation_height = min_activation_height;
+        consensus.vDeployments[d].lockinontimeout = lockinontimeout;
     }
     void UpdateActivationParametersFromArgs(const ArgsManager& args);
 };
@@ -529,8 +530,12 @@ void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args)
     for (const std::string& strDeployment : args.GetArgs("-vbparams")) {
         std::vector<std::string> vDeploymentParams;
         boost::split(vDeploymentParams, strDeployment, boost::is_any_of(":"));
-        if (vDeploymentParams.size() < 3 || vDeploymentParams.size() > 4) {
-            throw std::runtime_error("Version bits parameters malformed, expecting deployment:@startheight:@timeoutheight[:@min_activation_height]");
+        if (vDeploymentParams.size() == 3 && vDeploymentParams[1].compare(0, 2, "@-") == 0) {
+            // Don't require lockinontimeout for always/never-active special cases
+            vDeploymentParams.emplace_back("0");
+        }
+        if (vDeploymentParams.size() < 4 || vDeploymentParams.size() > 5) {
+            throw std::runtime_error("Version bits parameters malformed, expecting deployment:@startheight:@timeoutheight[:@min_activation_height]:lockinontimeout");
         }
         int32_t startheight = 0, timeoutheight = 0, min_activation_height = 0;
         if (vDeploymentParams[1].empty() || vDeploymentParams[1].front() != '@' || !ParseInt32(vDeploymentParams[1].substr(1), &startheight)) {
@@ -539,19 +544,25 @@ void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args)
         if (vDeploymentParams[2].empty() || vDeploymentParams[2].front() != '@' || !ParseInt32(vDeploymentParams[2].substr(1), &timeoutheight)) {
             throw std::runtime_error(strprintf("Invalid timeoutheight (%s)", vDeploymentParams[2]));
         }
-        if (vDeploymentParams.size() == 4 && (vDeploymentParams[3].front() != '@' || !ParseInt32(vDeploymentParams[3].substr(1), &min_activation_height))) {
+        if (vDeploymentParams.size() == 5 && (vDeploymentParams[3].front() != '@' || !ParseInt32(vDeploymentParams[3].substr(1), &min_activation_height))) {
             throw std::runtime_error(strprintf("Invalid min_activation_height (%s)", vDeploymentParams[3]));
         }
+        bool lockinontimeout = false;
+        if (vDeploymentParams.back().size() != 1 || (vDeploymentParams.back().front() != '0' && vDeploymentParams.back().front() != '1')) {
+            throw std::runtime_error(strprintf("Invalid lockinontimeout (%s)", vDeploymentParams.back()));
+        } else {
+            lockinontimeout = (vDeploymentParams.back().front() == '1');
+        }
         std::string error;
         if (!CheckVBitsHeights(error, consensus, startheight, timeoutheight, min_activation_height)) {
             throw std::runtime_error(error);
         }
         bool found = false;
         for (int j=0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) {
             if (vDeploymentParams[0] == VersionBitsDeploymentInfo[j].name) {
-                UpdateVersionBitsParameters(Consensus::DeploymentPos(j), startheight, timeoutheight, min_activation_height);
+                UpdateVersionBitsParameters(Consensus::DeploymentPos(j), startheight, timeoutheight, min_activation_height, lockinontimeout);
                 found = true;
-                LogPrintf("Setting version bits activation parameters for %s to startheight=%ld, timeoutheight=%ld, min_activation_height=%ld\n", vDeploymentParams[0], startheight, timeoutheight, min_activation_height);
+                LogPrintf("Setting version bits activation parameters for %s to startheight=%ld, timeoutheight=%ld, min_activation_height=%ld, lockinontimeout=%d\n", vDeploymentParams[0], startheight, timeoutheight, min_activation_height, lockinontimeout);
                 break;
             }
         }

src/chainparamsbase.cpp

@@ -23,7 +23,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman)
                  "This is intended for regression testing tools and app development. Equivalent to -chain=regtest.", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-segwitheight=<n>", "Set the activation height of segwit. -1 to disable. (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-testnet", "Use the test chain. Equivalent to -chain=test.", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
-    argsman.AddArg("-vbparams=deployment:@startheight:@timeoutheight[:@min_activation_height]", "Use given start, timeout, and minimum activation heights for specified version bits deployment (regtest-only). For an always active deployment, use @-1:@-1. For a never active deployment, use @-2:@-2.", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    argsman.AddArg("-vbparams=deployment:@startheight:@timeoutheight[:@min_activation_height]:lockinontimeout", "Use given start, timeout, and minimum activation heights and lockinontimeout (0/1) for specified version bits deployment (regtest-only). For an always active deployment, use @-1:@-1. For a never active deployment, use @-2:@-2.", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-signet", "Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-signetchallenge", "Blocks must satisfy the given script to be considered valid (only for signet networks; defaults to the global default signet test network challenge)", ArgsManager::ALLOW_STRING, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-signetseednode", "Specify a seed node for the signet network, in the hostname[:port] format, e.g. sig.net:1234 (may be used multiple times to specify multiple seed nodes; defaults to the global default signet test network seed node(s))", ArgsManager::ALLOW_STRING, OptionsCategory::CHAINPARAMS);

src/consensus/params.h

@@ -35,6 +35,8 @@ struct BIP9Deployment {
      * If lock in occurs, delay activation until at least this block height. Activations only occur on retargets.
      */
     int m_min_activation_height{0};
+    /** If true, final period before timeout will transition to MUST_SIGNAL. */
+    bool lockinontimeout{false};
 
     /** Constant for timeoutheight very far in the future. */
     static constexpr int NO_TIMEOUT = std::numeric_limits<int>::max();

src/rpc/blockchain.cpp

@@ -1223,25 +1223,27 @@ static void BIP9SoftForkDescPushBack(UniValue& softforks, const std::string &nam
     switch (thresholdState) {
     case ThresholdState::DEFINED: bip9.pushKV("status", "defined"); break;
     case ThresholdState::STARTED: bip9.pushKV("status", "started"); break;
+    case ThresholdState::MUST_SIGNAL: bip9.pushKV("status", "must_signal"); break;
     case ThresholdState::LOCKED_IN: bip9.pushKV("status", "locked_in"); break;
     case ThresholdState::ACTIVE: bip9.pushKV("status", "active"); break;
     case ThresholdState::FAILED: bip9.pushKV("status", "failed"); break;
     }
-    if (ThresholdState::STARTED == thresholdState)
-    {
+    if (ThresholdState::STARTED == thresholdState || ThresholdState::MUST_SIGNAL == thresholdState || ThresholdState::LOCKED_IN == thresholdState) {
         bip9.pushKV("bit", consensusParams.vDeployments[id].bit);
     }
     bip9.pushKV("startheight", consensusParams.vDeployments[id].startheight);
     bip9.pushKV("timeoutheight", consensusParams.vDeployments[id].timeoutheight);
     bip9.pushKV("minimum_activation_height", consensusParams.vDeployments[id].m_min_activation_height);
+    bip9.pushKV("lockinontimeout", consensusParams.vDeployments[id].lockinontimeout);
     int64_t since_height = VersionBitsTipStateSinceHeight(consensusParams, id);
     bip9.pushKV("since", since_height);
-    if (ThresholdState::STARTED == thresholdState)
-    {
+    if (ThresholdState::STARTED == thresholdState || ThresholdState::MUST_SIGNAL == thresholdState || ThresholdState::LOCKED_IN == thresholdState) {
         UniValue statsUV(UniValue::VOBJ);
         BIP9Stats statsStruct = VersionBitsTipStatistics(consensusParams, id);
         statsUV.pushKV("period", statsStruct.period);
-        statsUV.pushKV("threshold", statsStruct.threshold);
+        if (thresholdState != ThresholdState::LOCKED_IN) {
+            statsUV.pushKV("threshold", statsStruct.threshold);
+        }
         statsUV.pushKV("elapsed", statsStruct.elapsed);
         statsUV.pushKV("count", statsStruct.count);
         statsUV.pushKV("possible", statsStruct.possible);
@@ -1288,11 +1290,12 @@ RPCHelpMan getblockchaininfo()
                                 {RPCResult::Type::STR, "type", "one of \"buried\", \"bip8\""},
                                 {RPCResult::Type::OBJ, "bip8", "status of BIP 8 softforks (only for \"bip8\" type)",
                                 {
-                                    {RPCResult::Type::STR, "status", "one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\""},
+                                    {RPCResult::Type::STR, "status", "one of \"defined\", \"started\", \"must_signal\", \"locked_in\", \"active\", \"failed\""},
                                     {RPCResult::Type::NUM, "bit", "the bit (0-28) in the block version field used to signal this softfork (only for \"started\" status)"},
                                     {RPCResult::Type::NUM, "startheight", "the minimum height of a block at which the bit gains its meaning"},
                                     {RPCResult::Type::NUM, "timeoutheight", "the height of a block at which the deployment is considered failed if not yet locked in"},
                                     {RPCResult::Type::NUM, "minimum_activation_height", "the minimum block height at which activation is allowed to occur"},
+                                    {RPCResult::Type::BOOL, "lockinontimeout", "true if the period before timeoutheight transitions to must_signal"},
                                     {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"},
                                     {RPCResult::Type::OBJ, "statistics", "numeric statistics about BIP8 signalling for a softfork (only for \"started\" status)",
                                     {

src/rpc/mining.cpp

@@ -815,6 +815,7 @@ static RPCHelpMan getblocktemplate()
     aRules.push_back("csv");
     if (!fPreSegWit) aRules.push_back("!segwit");
     UniValue vbavailable(UniValue::VOBJ);
+    int32_t vbrequired{0};
     for (int j = 0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) {
         Consensus::DeploymentPos pos = Consensus::DeploymentPos(j);
         ThresholdState state = VersionBitsState(pindexPrev, consensusParams, pos, versionbitscache);
@@ -823,6 +824,10 @@ static RPCHelpMan getblocktemplate()
             case ThresholdState::FAILED:
                 // Not exposed to GBT at all
                 break;
+            case ThresholdState::MUST_SIGNAL:
+                // Bit must be set in block version
+                vbrequired |= VersionBitsMask(consensusParams, pos);
+                // FALL THROUGH to set nVersion and get vbavailable set...
             case ThresholdState::LOCKED_IN:
                 // Ensure bit is set in block version
                 pblock->nVersion |= VersionBitsMask(consensusParams, pos);
@@ -858,7 +863,7 @@ static RPCHelpMan getblocktemplate()
     result.pushKV("version", pblock->nVersion);
     result.pushKV("rules", aRules);
     result.pushKV("vbavailable", vbavailable);
-    result.pushKV("vbrequired", int(0));
+    result.pushKV("vbrequired", vbrequired);
 
     if (nMaxVersionPreVB >= 2) {
         // If VB is supported by the client, nMaxVersionPreVB is -1, so we won't get here

src/test/fuzz/versionbits.cpp

@@ -31,9 +31,10 @@ class TestConditionChecker : public AbstractThresholdConditionChecker
     const int m_period;
     const int m_threshold;
     const int m_bit;
+    const bool m_lockinontimeout;
 
-    TestConditionChecker(int begin, int end, int min_act, int period, int threshold, int bit)
-        : m_begin{begin}, m_end{end}, m_min_activation(min_act), m_period{period}, m_threshold{threshold}, m_bit{bit}
+    TestConditionChecker(int begin, int end, int min_act, int period, int threshold, int bit, bool lockinontimeout)
+        : m_begin{begin}, m_end{end}, m_min_activation(min_act), m_period{period}, m_threshold{threshold}, m_bit{bit}, m_lockinontimeout{lockinontimeout}
     {
         assert(m_period > 0);
         assert(0 <= m_threshold && m_threshold <= m_period);
@@ -43,6 +44,7 @@ class TestConditionChecker : public AbstractThresholdConditionChecker
     bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override { return Condition(pindex->nVersion); }
     int StartHeight(const Consensus::Params& params) const override { return m_begin; }
     int TimeoutHeight(const Consensus::Params& params) const override { return m_end; }
+    bool LockinOnTimeout(const Consensus::Params& params) const override { return m_lockinontimeout; }
     int MinActivationHeight(const Consensus::Params& params) const override { return m_min_activation; }
     int Period(const Consensus::Params& params) const override { return m_period; }
     int Threshold(const Consensus::Params& params) const override { return m_threshold; }
@@ -133,11 +135,17 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     int startheight;
     int timeoutheight;
     int min_activation = 0;
+    bool lockinontimeout = false;
     if (fuzzed_data_provider.ConsumeBool()) {
         // pick the timestamp to switch based on a block
         startheight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 2));
         timeoutheight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 2));
         min_activation = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 1));
+        if (startheight < int(period * (max_periods - 3)) && threshold < period) {
+            // LOT=True requires 3 periods (STARTED->MUST_SIGNAL->LOCKED_IN), pushing it past the deadline
+            // Furthermore, this fuzzer doesn't let us easily guarantee the signal of the first block in a period, so skip LOT=True when threshold is 100%
+            lockinontimeout = fuzzed_data_provider.ConsumeBool();
+        }
     } else {
         if (fuzzed_data_provider.ConsumeBool()) {
             startheight = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
@@ -150,7 +158,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         }
     }
 
-    TestConditionChecker checker(startheight, timeoutheight, min_activation, period, threshold, bit);
+    TestConditionChecker checker(startheight, timeoutheight, min_activation, period, threshold, bit, lockinontimeout);
 
     // Early exit if the versions don't signal sensibly for the deployment
     if (!checker.Condition(ver_signal)) return;
@@ -209,7 +217,11 @@ void test_one_input(const std::vector<uint8_t>& buffer)
 
     // mine (period-1) blocks and check state
     for (int b = 1; b < period; ++b) {
-        const bool signal = (signalling_mask >> (b % 32)) & 1;
+        bool signal = (signalling_mask >> (b % 32)) & 1;
+        if (exp_state == ThresholdState::MUST_SIGNAL && threshold - blocks_sig >= period - b) {
+            // Further blocks need to signal to be valid
+            signal = true;
+        }
         if (signal) ++blocks_sig;
 
         CBlockIndex* current_block = blocks.mine_block(signal);
@@ -223,8 +235,8 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         assert(state == exp_state);
         assert(since == exp_since);
 
-        // GetStateStatistics may crash when state is not STARTED
-        if (state != ThresholdState::STARTED) continue;
+        // GetStateStatistics may crash when state is not STARTED or MUST_SIGNAL
+        if (state != ThresholdState::STARTED && state != ThresholdState::MUST_SIGNAL) continue;
 
         // check that after mining this block stats change as expected
         const BIP9Stats stats = checker.GetStateStatisticsFor(current_block);
@@ -236,7 +248,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         last_stats = stats;
     }
 
-    if (exp_state == ThresholdState::STARTED) {
+    if (exp_state == ThresholdState::STARTED || exp_state == ThresholdState::MUST_SIGNAL) {
         // double check that stats.possible is sane
         if (blocks_sig >= threshold - 1) assert(last_stats.possible);
     }
@@ -289,11 +301,15 @@ void test_one_input(const std::vector<uint8_t>& buffer)
             assert(exp_state == ThresholdState::DEFINED);
         }
         break;
+    case ThresholdState::MUST_SIGNAL:
+        assert(height >= checker.m_end - period);
+        assert(exp_state == ThresholdState::STARTED);
+        break;
     case ThresholdState::LOCKED_IN:
         if (exp_state == ThresholdState::LOCKED_IN) {
             assert(height < checker.m_min_activation);
         } else {
-            assert(exp_state == ThresholdState::STARTED);
+            assert(exp_state == ThresholdState::STARTED || exp_state == ThresholdState::MUST_SIGNAL);
             assert(blocks_sig >= threshold);
         }
         break;