consensus: make difficulty retarget windows contiguous (timewarp hardfork)

src/chainparams.cpp

@@ -49,6 +49,21 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option
         }
         options.pow_target_spacing = *signetblocktime;
     }
+    if (args.IsArgSet("-signettimewarpfixheight")) {
+        if (!args.IsArgSet("-signetchallenge")) {
+            throw std::runtime_error("-signettimewarpfixheight cannot be set without -signetchallenge");
+        }
+        // Parsed from the raw string rather than via GetIntArg, which funnels through
+        // LocaleIndependentAtoi and yields 0 for anything unparseable: a typo would then
+        // silently schedule the fork at genesis and split this signet from its peers.
+        // Same treatment -testactivationheight already gets.
+        int height{0};
+        const std::string value{args.GetArg("-signettimewarpfixheight", "")};
+        if (!ParseInt32(value, &height) || height < 0 || height >= std::numeric_limits<int>::max()) {
+            throw std::runtime_error(strprintf("Invalid height value (%s) for -signettimewarpfixheight.", value));
+        }
+        options.timewarpfix_height = height;
+    }
 }
 
 void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& options)

src/chainparamsbase.cpp

@@ -26,6 +26,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman)
     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_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-signetblocktime", "Difficulty adjustment will target a block time of the given amount in seconds (only for custom signet networks, must have -signetchallenge set; defaults to 10 minutes)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);
+    argsman.AddArg("-signettimewarpfixheight", "Activate contiguous difficulty retarget windows at the given height (only for custom signet networks, must have -signetchallenge set; unscheduled by default)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, 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_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);
 }
 

src/consensus/params.h

@@ -29,8 +29,9 @@ enum BuriedDeployment : int16_t {
     DEPLOYMENT_CSV,
     DEPLOYMENT_SEGWIT,
     DEPLOYMENT_BLAKE2B,
+    DEPLOYMENT_TIMEWARPFIX,
 };
-constexpr bool ValidDeployment(BuriedDeployment dep) { return dep <= DEPLOYMENT_BLAKE2B; }
+constexpr bool ValidDeployment(BuriedDeployment dep) { return dep <= DEPLOYMENT_TIMEWARPFIX; }
 
 enum DeploymentPos : uint16_t {
     DEPLOYMENT_TESTDUMMY,
@@ -139,6 +140,11 @@ struct Params {
     int Blake2bHeight{std::numeric_limits<int>::max()};
     std::vector<unsigned char> Blake2bHeadline;
     uint8_t Blake2bTargetShift{20};
+    /** Height from which contiguous retarget windows apply. Both rules key on the block
+     *  closing a window, so the first one actually affected is the first closing block at
+     *  or above this height, not the block at this height. Unscheduled by default: it is
+     *  a hardfork in its own right, and the BLAKE2b fork is already past. */
+    int TimewarpFixHeight{std::numeric_limits<int>::max()};
     /**
      * RDTS (BIP110 ReducedData Temporary Softfork) deployment expiry.
      *
@@ -216,6 +222,8 @@ struct Params {
             return SegwitHeight;
         case DEPLOYMENT_BLAKE2B:
             return Blake2bHeight;
+        case DEPLOYMENT_TIMEWARPFIX:
+            return TimewarpFixHeight;
         } // no default case, so the compiler can warn about missing cases
         return std::numeric_limits<int>::max();
     }
@@ -225,6 +233,11 @@ struct Params {
         return height >= Blake2bHeight;
     }
 
+    bool IsTimewarpFixHeight(int height) const
+    {
+        return height >= TimewarpFixHeight;
+    }
+
     /** RDTS activates at the BLAKE2b hardfork; there is no separate schedule. */
     int RdtsActivationHeight() const { return Blake2bHeight; }
 

src/deploymentinfo.cpp

@@ -35,6 +35,8 @@ std::string DeploymentName(Consensus::BuriedDeployment dep)
         return "segwit";
     case Consensus::DEPLOYMENT_BLAKE2B:
         return "blake2b";
+    case Consensus::DEPLOYMENT_TIMEWARPFIX:
+        return "timewarpfix";
     } // no default case, so the compiler can warn about missing cases
     return "";
 }
@@ -53,6 +55,8 @@ std::optional<Consensus::BuriedDeployment> GetBuriedDeployment(const std::string
         return Consensus::BuriedDeployment::DEPLOYMENT_CSV;
     } else if (name == "blake2b") {
         return Consensus::BuriedDeployment::DEPLOYMENT_BLAKE2B;
+    } else if (name == "timewarpfix") {
+        return Consensus::BuriedDeployment::DEPLOYMENT_TIMEWARPFIX;
     }
     return std::nullopt;
 }

src/init.cpp

@@ -2038,23 +2038,21 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // the reindex prompt below offers recovery, rather than running on (or
     // partially rewinding) an invalid chain.
     //
-    // Skip this on any reindex: both -reindex and -reindex-chainstate reconnect
-    // blocks through ConnectBlock, which re-runs the bad-version-blake2b header
-    // check (it is contextual/volatile) and rejects the offending blocks during
-    // the rebuild, so the correction is redundant. The same holds whenever the
-    // active chain has not been built yet (a coins database with no best block,
-    // reindex or not); the correction itself is a no-op in that state.
+    // Skipped only on a full -reindex, which re-accepts every header and so needs no
+    // correction. -reindex-chainstate IS corrected: the rebuild alone misses inherited
+    // retarget nBits (see ContextualCheckBlockHeaderVolatile), and with the coins
+    // database wiped there is no tip, so the call below marks without reorganizing.
     //
     // The block index is shared, so the invalid marks apply to any background
     // (assumeutxo) chainstate too; only the active chainstate is reorganized here.
-    // Safe while every snapshot base stays below the BLAKE2b fork height (as
-    // today); a future snapshot base above it would need re-review.
-    if (status == ChainstateLoadStatus::SUCCESS && !ShutdownRequested(node) &&
-            !do_reindex && !do_reindex_chainstate) {
-        bilingual_str rdts_error;
-        if (!node.chainman->ActiveChainstate().CorrectRdtsInvalidBlocks(rdts_error)) {
+    // Safe while every snapshot base stays below both the BLAKE2b fork height and
+    // TimewarpFixHeight (as today, the latter being unscheduled everywhere); a future
+    // snapshot base above either would need re-review.
+    if (status == ChainstateLoadStatus::SUCCESS && !ShutdownRequested(node) && !do_reindex) {
+        bilingual_str inherited_error;
+        if (!node.chainman->ActiveChainstate().CorrectInheritedInvalidBlocks(inherited_error)) {
             status = ChainstateLoadStatus::FAILURE;
-            error = rdts_error;
+            error = inherited_error;
         }
     }
 

src/kernel/chainparams.cpp

@@ -546,6 +546,9 @@ class SigNetParams : public CChainParams {
         consensus.SegwitHeight = 1;
         consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
         consensus.nPowTargetSpacing = options.pow_target_spacing;
+        if (options.timewarpfix_height) {
+            consensus.TimewarpFixHeight = *options.timewarpfix_height;
+        }
         consensus.fPowAllowMinDifficultyBlocks = false;
         consensus.enforce_BIP94 = false;
         consensus.fPowNoRetargeting = false;
@@ -656,6 +659,15 @@ class CRegTestParams : public CChainParams
             case Consensus::BuriedDeployment::DEPLOYMENT_BLAKE2B:
                 consensus.Blake2bHeight = int{height};
                 break;
+            case Consensus::BuriedDeployment::DEPLOYMENT_TIMEWARPFIX:
+                // CalculateNextWorkRequired takes the BIP94 base target without the
+                // contiguous-window shift, so the fix would be only half applied here.
+                // Refuse rather than run a half-applied consensus rule.
+                if (consensus.enforce_BIP94) {
+                    throw std::runtime_error("timewarpfix activation cannot be combined with BIP94 enforcement");
+                }
+                consensus.TimewarpFixHeight = int{height};
+                break;
             case Consensus::BuriedDeployment::DEPLOYMENT_SEGWIT:
                 consensus.SegwitHeight = int{height};
                 break;

src/kernel/chainparams.h

@@ -144,6 +144,9 @@ class CChainParams
         std::optional<std::vector<uint8_t>> challenge{};
         std::optional<std::vector<std::string>> seeds{};
         int64_t pow_target_spacing{10 * 60};
+        //! Height at which contiguous retarget windows activate (see
+        //! -signettimewarpfixheight). Left unscheduled by default.
+        std::optional<int> timewarpfix_height{};
     };
 
     /**

src/node/miner.cpp

@@ -31,8 +31,9 @@
 
 namespace node {
 
-int64_t GetMinimumTime(const CBlockIndex* pindexPrev, const int64_t difficulty_adjustment_interval)
+int64_t GetMinimumTime(const CBlockIndex* pindexPrev, const Consensus::Params& params)
 {
+    const int64_t difficulty_adjustment_interval{params.DifficultyAdjustmentInterval()};
     int64_t min_time{pindexPrev->GetMedianTimePast() + 1};
     // Height of block to be mined.
     const int height{pindexPrev->nHeight + 1};
@@ -41,13 +42,16 @@ int64_t GetMinimumTime(const CBlockIndex* pindexPrev, const int64_t difficulty_a
     if (height % difficulty_adjustment_interval == 0) {
         min_time = std::max<int64_t>(min_time, pindexPrev->GetBlockTime() - MAX_TIMEWARP);
     }
+    if (const std::optional<int64_t> closing_min{MinimumClosingBlockTime(pindexPrev, params)}) {
+        min_time = std::max<int64_t>(min_time, *closing_min);
+    }
     return min_time;
 }
 
 int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev)
 {
     int64_t nOldTime = pblock->nTime;
-    int64_t nNewTime{std::max<int64_t>(GetMinimumTime(pindexPrev, consensusParams.DifficultyAdjustmentInterval()),
+    int64_t nNewTime{std::max<int64_t>(GetMinimumTime(pindexPrev, consensusParams),
                                        TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()))};
 
     if (nOldTime < nNewTime) {

src/node/miner.h

@@ -232,9 +232,9 @@ class BlockAssembler
 /**
  * Get the minimum time a miner should use in the next block. This always
  * accounts for the BIP94 timewarp rule, so does not necessarily reflect the
- * consensus limit.
+ * consensus limit. It also applies MinimumClosingBlockTime, which does.
  */
-int64_t GetMinimumTime(const CBlockIndex* pindexPrev, const int64_t difficulty_adjustment_interval);
+int64_t GetMinimumTime(const CBlockIndex* pindexPrev, const Consensus::Params& params);
 
 int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev);
 

src/pow.cpp

@@ -60,6 +60,12 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     } else {
         // Go back by what we want to be 14 days worth of blocks
         int nHeightFirst = pindexLast->nHeight - (params.DifficultyAdjustmentInterval()-1);
+        if (params.IsTimewarpFixHeight(pindexLast->nHeight) && nHeightFirst > 0) {
+            // Measure from the previous window's last block, so windows share an
+            // endpoint and the seam between them is no longer unmeasured. Gated on
+            // pindexLast, the block MinimumClosingBlockTime floors: both must name it.
+            --nHeightFirst;
+        }
         assert(nHeightFirst >= 0);
         const CBlockIndex* pindexFirst = pindexLast->GetAncestor(nHeightFirst);
         assert(pindexFirst);
@@ -75,6 +81,18 @@ unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHead
     return nBits;
 }
 
+std::optional<int64_t> MinimumClosingBlockTime(const CBlockIndex* pindexPrev, const Consensus::Params& params)
+{
+    const int height{pindexPrev->nHeight + 1};
+    // Only the block closing a window, and only once windows are contiguous. Gate first:
+    // FindInheritedInvalidBlocks calls this for every entry in the block index.
+    if (!params.IsTimewarpFixHeight(height)) return std::nullopt;
+    const int64_t interval{params.DifficultyAdjustmentInterval()};
+    if (height < interval || height % interval != interval - 1) return std::nullopt;
+    // Non-null by the guard above: 0 <= height - interval <= pindexPrev->nHeight.
+    return Assert(pindexPrev->GetAncestor(static_cast<int>(height - interval)))->GetBlockTime();
+}
+
 unsigned int CalculateNextWorkRequired(const CBlockIndex* pindexLast, int64_t nFirstBlockTime, const Consensus::Params& params)
 {
     if (params.fPowNoRetargeting)
@@ -91,7 +109,8 @@ unsigned int CalculateNextWorkRequired(const CBlockIndex* pindexLast, int64_t nF
     const arith_uint256 bnPowLimit = UintToArith256(params.powLimit);
     arith_uint256 bnNew;
 
-    // Special difficulty rule for Testnet4
+    // Special difficulty rule for Testnet4. Note: no contiguous-window decrement here, so
+    // the fix would be half applied on a BIP94 chain; chainparams refuses that combination.
     if (params.enforce_BIP94) {
         // Here we use the first block of the difficulty period. This way
         // the real difficulty is always preserved in the first block as

src/pow.h

@@ -28,6 +28,19 @@ class arith_uint256;
 std::optional<arith_uint256> DeriveTarget(unsigned int nBits, const uint256 pow_limit);
 
 unsigned int GetNextWorkRequired(const CBlockIndex* pindexLast, const CBlockHeader *pblock, const Consensus::Params&) ARG_NONNULL(1) ARG_NONNULL(2);
+
+/** Earliest timestamp the block after pindexPrev may carry on account of the retarget
+ *  window it closes, or nullopt where nothing beyond the usual timestamp rules applies.
+ *
+ *  With contiguous windows the block closing one is where the next starts measuring, so
+ *  stamping it earlier lets the next window re-measure time the previous one already did.
+ *  The clamps do not prevent that: a negative span is credited as nPowTargetTimespan/4,
+ *  which RAISES difficulty, and an attacker spends that rise to buy two 4x drops.
+ *
+ *  Enforced from ContextualCheckBlockHeaderVolatile (see there for why); mirrored in
+ *  GetMinimumTime so a template cannot be built that validation would reject. */
+std::optional<int64_t> MinimumClosingBlockTime(const CBlockIndex* pindexPrev, const Consensus::Params& params) ARG_NONNULL(1);
+
 unsigned int CalculateNextWorkRequired(const CBlockIndex* pindexLast, int64_t nFirstBlockTime, const Consensus::Params&);
 
 /** Check whether a block hash satisfies the proof-of-work requirement specified by nBits */

src/rpc/blockchain.cpp

@@ -2010,6 +2010,10 @@ RPCHelpMan getdeploymentinfo()
                     {RPCResult::Type::NUM, "height", "the height the hardfork activates at"},
                     {RPCResult::Type::BOOL, "active", "whether the hardfork rules apply to the block after this one"},
                 }},
+                {RPCResult::Type::OBJ, "timewarpfix", /*optional=*/true, "contiguous retarget window schedule, present only when one is configured", {
+                    {RPCResult::Type::NUM, "height", "the height the rules activate at"},
+                    {RPCResult::Type::BOOL, "active", "whether the rules apply to the block after this one"},
+                }},
             }
         },
         RPCExamples{ HelpExampleCli("getdeploymentinfo", "") + HelpExampleRpc("getdeploymentinfo", "") },
@@ -2051,6 +2055,13 @@ RPCHelpMan getdeploymentinfo()
                                                           Consensus::DEPLOYMENT_BLAKE2B));
                 deploymentinfo.pushKV("blake2b", std::move(hf));
             }
+            if (consensus.TimewarpFixHeight != std::numeric_limits<int>::max()) {
+                UniValue tw(UniValue::VOBJ);
+                tw.pushKV("height", consensus.TimewarpFixHeight);
+                tw.pushKV("active", DeploymentActiveAfter(blockindex, chainman,
+                                                          Consensus::DEPLOYMENT_TIMEWARPFIX));
+                deploymentinfo.pushKV("timewarpfix", std::move(tw));
+            }
             return deploymentinfo;
         },
     };

src/rpc/mining.cpp

@@ -698,7 +698,7 @@ static RPCHelpMan getblocktemplate()
                 {RPCResult::Type::NUM, "coinbasevalue", "maximum allowable input to coinbase transaction, including the generation award and transaction fees (in satoshis)"},
                 {RPCResult::Type::STR, "longpollid", "an id to include with a request to longpoll on an update to this template"},
                 {RPCResult::Type::STR, "target", "The hash target"},
-                {RPCResult::Type::NUM_TIME, "mintime", "The minimum timestamp appropriate for the next block time, expressed in " + UNIX_EPOCH_TIME + ". Adjusted for the proposed BIP94 timewarp rule."},
+                {RPCResult::Type::NUM_TIME, "mintime", "The minimum timestamp appropriate for the next block time, expressed in " + UNIX_EPOCH_TIME + ". Adjusted for the proposed BIP94 timewarp rule, and for the floor on a block closing a retarget period."},
                 {RPCResult::Type::ARR, "mutable", "list of ways the block template may be changed",
                 {
                     {RPCResult::Type::STR, "value", "A way the block template may be changed, e.g. 'time', 'transactions', 'prevblock'"},
@@ -707,7 +707,7 @@ static RPCHelpMan getblocktemplate()
                 {RPCResult::Type::NUM, "sigoplimit", "limit of sigops in blocks"},
                 {RPCResult::Type::NUM, "sizelimit", "limit of block size"},
                 {RPCResult::Type::NUM, "weightlimit", /*optional=*/true, "limit of block weight"},
-                {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME + ". Adjusted for the proposed BIP94 timewarp rule."},
+                {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME + ". Adjusted for the proposed BIP94 timewarp rule, and for the floor on a block closing a retarget period."},
                 {RPCResult::Type::STR, "bits", "compressed target of next block"},
                 {RPCResult::Type::NUM, "height", "The height of the next block"},
                 {RPCResult::Type::STR_HEX, "signet_challenge", /*optional=*/true, "Only on signet"},
@@ -1099,7 +1099,7 @@ static UniValue TemplateToJSON(const Consensus::Params& consensusParams, const C
     result.pushKV("coinbasevalue", (int64_t)block.vtx[0]->vout[0].nValue);
     result.pushKV("longpollid", pindexPrev->GetBlockHash().GetHex() + ToString(nTransactionsUpdatedLast));
     result.pushKV("target", hashTarget.GetHex());
-    result.pushKV("mintime", GetMinimumTime(pindexPrev, consensusParams.DifficultyAdjustmentInterval()));
+    result.pushKV("mintime", GetMinimumTime(pindexPrev, consensusParams));
     result.pushKV("mutable", std::move(aMutable));
     result.pushKV("noncerange", "00000000ffffffff");
     int64_t nSigOpLimit = MAX_BLOCK_SIGOPS_COST;

src/test/CMakeLists.txt

@@ -107,6 +107,7 @@ add_executable(test_bitcoin
   system_ram_tests.cpp
   system_tests.cpp
   timeoffsets_tests.cpp
+  timewarp_tests.cpp
   torcontrol_tests.cpp
   transaction_tests.cpp
   translation_tests.cpp

src/test/fuzz/CMakeLists.txt

@@ -118,6 +118,7 @@ add_executable(fuzz
   strprintf.cpp
   system.cpp
   timeoffsets.cpp
+  timewarp.cpp
   torcontrol.cpp
   transaction.cpp
   txdownloadman.cpp