fix: keep short rotation quarters as selected

doc/release-notes-8074.md

@@ -0,0 +1,47 @@
+Consensus changes
+-----------------
+
+- A rotating LLMQ (DIP0024) cycle that can't fill a quarter, because too few eligible masternodes are left, now
+  keeps the members it actually selected: the quarter stays short or empty when it is rebuilt from the cycle's
+  quorum snapshot. Previously, rebuilding a stored short quarter padded it by wrapping around the candidate list,
+  adding members that were never selected and possibly repeating members of the quorum's other quarters. With too
+  few enabled masternodes, or when the selection walk made no progress, the builder instead returned empty
+  quarters without storing a snapshot, and later cycles stopped loading their older quarters at the missing
+  snapshot. Given the same earlier quarters, cycles whose quarters all fill are selected, stored and rebuilt
+  exactly as before. Quorum sizes, minimum sizes and thresholds are unchanged: the existing `minSize`, DKG and
+  commitment rules still decide whether a quorum with short quarters forms.
+- There is no activation: the new rules apply to every cycle. A variant of this build replayed mainnet through
+  h=2554746 and testnet through h=1579827 without rejecting a block; that doesn't establish identical quorum
+  members for every historical cycle. The testnet cycles whose selection differs (base blocks at h=838368,
+  h=901152-903168 and h=1366560-1367136) keep the old rules. Short or empty quarters only occur when very few
+  masternodes are eligible. Older and upgraded nodes can then derive different quorum members, either in that
+  cycle or when its quarters are reused in later cycles, and reject each other's commitments, so nodes have to
+  upgrade before such a cycle occurs.
+- Devnets and regtest chains that already had a cycle with a short or empty quarter are not supported across
+  this upgrade and have to be started over. Their stored snapshots, or missing snapshots, can give upgraded nodes
+  different members than nodes that reindex or sync from scratch, and those can reject historical commitments
+  whose members change, so reindexing alone is not a guaranteed recovery.
+
+Notes for SPV and other clients that rebuild rotating quorums from `qrinfo`
+-------------------------------------------------------------------------------
+
+The serialized `CQuorumSnapshot` and `qrinfo` formats don't change, but `mnSkipListMode` gains a value,
+`MODE_SHORT_QUARTERS` (4). Snapshots in `MODE_NO_SKIPPING` (0) and `MODE_SKIPPING_ENTRIES` (1) keep their
+meaning and are decoded as before. Nodes store `MODE_SHORT_QUARTERS` only for cycles with a short or empty
+quarter, so clients must decode it before such a cycle occurs:
+
+- Candidate list: as before, `activeQuorumMembers` refers to the work block's valid masternodes with a
+  confirmed registration (non-null `confirmedHash`), sorted by score. The candidates are the unmarked ones
+  followed by the marked ones, each part in that order.
+- Walk: a step counter starts at 0 and counts every candidate visited in the cycle, across all quorum indices
+  in order. The candidate at step `t` is `list[t % list.size()]`. For each quorum index, a visited candidate is
+  skipped if `t` is a skip step and added otherwise. An index's quarter ends when it has `quarterSize` members
+  or after `list.size()` consecutive skips; the next index continues at the next step. An empty candidate list
+  gives empty quarters and an empty skip list.
+- Skip steps: `mnSkipList` holds the steps at which a candidate was skipped. The first entry is the absolute
+  step of the first skip; each later entry is the positive difference from the previous skip step. A snapshot
+  is malformed if an entry is negative (or a later entry is zero), if a skip step is never reached by the walk,
+  or if a quarter would contain the same position twice.
+
+For cycles they compute successfully, outside predictions, nodes now store a snapshot even when no masternodes
+are eligible. The exempt testnet cycles keep the old storage behaviour.

src/chainparams.cpp

@@ -158,7 +158,7 @@ static void ReadRegTestBudgetArgs(const ArgsManager& args, CChainParams::RegTest
 
 static void ReadRegTestLLMQTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& options, const Consensus::LLMQType llmqType)
 {
-    assert(llmqType == Consensus::LLMQType::LLMQ_TEST || llmqType == Consensus::LLMQType::LLMQ_TEST_INSTANTSEND || llmqType == Consensus::LLMQType::LLMQ_TEST_PLATFORM);
+    assert(llmqType == Consensus::LLMQType::LLMQ_TEST || llmqType == Consensus::LLMQType::LLMQ_TEST_INSTANTSEND || llmqType == Consensus::LLMQType::LLMQ_TEST_PLATFORM || llmqType == Consensus::LLMQType::LLMQ_TEST_DIP0024);
 
     std::string cmd_param{"-llmqtestparams"}, llmq_name{"LLMQ_TEST"};
     if (llmqType == Consensus::LLMQType::LLMQ_TEST_INSTANTSEND) {
@@ -169,6 +169,10 @@ static void ReadRegTestLLMQTestArgs(const ArgsManager& args, CChainParams::RegTe
         cmd_param = "-llmqtestplatformparams";
         llmq_name = "LLMQ_TEST_PLATFORM";
     }
+    if (llmqType == Consensus::LLMQType::LLMQ_TEST_DIP0024) {
+        cmd_param = "-llmqtestdip0024params";
+        llmq_name = "LLMQ_TEST_DIP0024";
+    }
 
     if (!args.IsArgSet(cmd_param)) return;
 
@@ -188,6 +192,13 @@ static void ReadRegTestLLMQTestArgs(const ArgsManager& args, CChainParams::RegTe
         throw std::runtime_error(strprintf("Invalid %s size (%d), must not exceed the maximum supported quorum size (%d)",
                                            llmq_name, size, Consensus::MAX_LLMQ_SIZE));
     }
+    if (llmqType == Consensus::LLMQType::LLMQ_TEST_DIP0024 && (size <= 0 || size % 4 != 0)) {
+        throw std::runtime_error(strprintf("Invalid %s size (%d), must be a positive multiple of 4", llmq_name, size));
+    }
+    if (llmqType == Consensus::LLMQType::LLMQ_TEST_DIP0024 && (threshold < 2 || threshold > size)) {
+        throw std::runtime_error(strprintf("Invalid %s threshold (%d), must be at least 2 and not exceed the size (%d)",
+                                           llmq_name, threshold, size));
+    }
     LogPrintf("Setting %s parameters to size=%ld, threshold=%ld\n", llmq_name, size, threshold);
     options.llmq_test_parameters[llmqType] = CChainParams::LLMQParameters{size, threshold};
 }
@@ -209,6 +220,7 @@ void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& opti
     ReadRegTestLLMQTestArgs(args, options, Consensus::LLMQType::LLMQ_TEST);
     ReadRegTestLLMQTestArgs(args, options, Consensus::LLMQType::LLMQ_TEST_INSTANTSEND);
     ReadRegTestLLMQTestArgs(args, options, Consensus::LLMQType::LLMQ_TEST_PLATFORM);
+    ReadRegTestLLMQTestArgs(args, options, Consensus::LLMQType::LLMQ_TEST_DIP0024);
     ReadRegTestLLMQInstantSendDIP0024Args(args, options);
 }
 
@@ -342,6 +354,7 @@ void SetupChainParamsOptions(ArgsManager& argsman)
     argsman.AddArg("-llmqtestinstantsenddip0024=<quorum name>", "Override the default LLMQ type used for InstantSendDIP0024. Used mainly to test Platform. (default: llmq_test_dip0024, regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-llmqtestinstantsendparams=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_TEST_INSTANTSEND quorums (default: 3:2, regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-llmqtestparams=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_TEST quorum (default: 3:2, regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    argsman.AddArg("-llmqtestdip0024params=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_TEST_DIP0024 quorum and set its minSize to <threshold> (default: size 4, threshold 3, minSize 4, regtest-only; size must be a positive multiple of 4 and threshold between 2 and size)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-llmqtestplatformparams=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_TEST_PLATFORM quorum (default: 3:2, regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-minimumdifficultyblocks=<n>", "The number of blocks that can be mined with the minimum difficulty at the start of a chain (default: 0, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
     argsman.AddArg("-powtargetspacing=<n>", "Override the default PowTargetSpacing value in seconds (default: 2.5 minutes, devnet-only)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS);

src/consensus/params.h

@@ -205,6 +205,9 @@ struct Params {
     LLMQType llmqTypeDIP0024InstantSend{LLMQType::LLMQ_NONE};
     LLMQType llmqTypePlatform{LLMQType::LLMQ_NONE};
     LLMQType llmqTypeMnhf{LLMQType::LLMQ_NONE};
+    //! Base blocks of the rotation cycles that keep the legacy quarter walk: the ones whose quarters it
+    //! built differently from the step walk
+    std::vector<uint256> llmqLegacyRotationCycles;
 
     int DeploymentHeight(BuriedDeployment dep) const
     {

src/evo/snapshot.cpp

@@ -305,7 +305,7 @@ void EvoSnapshot::Validate(bool require_canonical_order) const
                 !cycle_hashes.insert(entry.cycle_base_block_hash).second ||
                 !historical_hashes.contains(entry.work_block_hash) ||
                 entry.snapshot.mnSkipListMode < SnapshotSkipMode::MODE_NO_SKIPPING ||
-                entry.snapshot.mnSkipListMode > SnapshotSkipMode::MODE_ALL_SKIPPED ||
+                entry.snapshot.mnSkipListMode > SnapshotSkipMode::MODE_SHORT_QUARTERS ||
                 entry.snapshot.activeQuorumMembers.size() > EVO_SNAPSHOT_MAX_MNS ||
                 entry.snapshot.mnSkipList.size() > EVO_SNAPSHOT_MAX_SKIPLIST_ENTRIES ||
                 // Only the first entry is an absolute index; later entries are

src/kernel/chainparams.cpp

@@ -505,6 +505,20 @@ class CTestNetParams : public CChainParams {
         consensus.llmqTypeDIP0024InstantSend = Consensus::LLMQType::LLMQ_60_75;
         consensus.llmqTypePlatform = Consensus::LLMQType::LLMQ_25_67;
         consensus.llmqTypeMnhf = Consensus::LLMQType::LLMQ_50_60;
+        consensus.llmqLegacyRotationCycles = {
+            uint256S("0x000000c2b743330194753dd04612d315c6d6fa82bb2dd8133390d322665652a8"), // 838368
+            uint256S("0x0000019b49d8472cf7a61eebcde5b3ae76899fda0eb6977b4c4b89e83a61c5d6"), // 901152
+            uint256S("0x0000002ebb49b371fc4048530d4a86120ad7df05494f4f6796486794e074cbb0"), // 901440
+            uint256S("0x0000006bf3f118e89b51caabdf52ffbf25307fa3de29dbaaf4fd8885767b4a9b"), // 901728
+            uint256S("0x000000c62c4ec85e413a688ab555063cdb20f286b93f2ea30baa57a3ead62c32"), // 902016
+            uint256S("0x00000032576582b5908013c96bbccbeecb29266aaa65db56ef2a7a3f52d2e7f1"), // 902304
+            uint256S("0x0000008725d64d01965d95ae510dfcc74f00cfec09ac152e0b2e0eb2899b4e41"), // 902592
+            uint256S("0x00000130a955271036c1661e1892549d22b0089c03619446066cda443dd44dc8"), // 902880
+            uint256S("0x00000273417c2fdb13dcf3c35fcab3ace02605c67bd7b6c37df4f75038c357fc"), // 903168
+            uint256S("0x0000007dba91b5abb6003250d435f1e84e28999fa46486f67fb47e5a91eebca3"), // 1366560
+            uint256S("0x000000bbf499d2303e41488c13296197a01647d7e1712e64390c8bbbf7e70906"), // 1366848
+            uint256S("0x0000017466c3129af87638058f3abc27096061a8b06d0894095ad7b640f46c79"), // 1367136
+        };
 
         fDefaultConsistencyChecks = false;
         fRequireRoutableExternalIP = true;
@@ -1027,6 +1041,7 @@ class CRegTestParams : public CChainParams {
         UpdateLLMQTestParameters(opts, Consensus::LLMQType::LLMQ_TEST);
         UpdateLLMQTestParameters(opts, Consensus::LLMQType::LLMQ_TEST_INSTANTSEND);
         UpdateLLMQTestParameters(opts, Consensus::LLMQType::LLMQ_TEST_PLATFORM);
+        UpdateLLMQTestParameters(opts, Consensus::LLMQType::LLMQ_TEST_DIP0024);
         UpdateLLMQInstantSendDIP0024(opts);
         // V20 features for CbTx (credit pool, CL) have no meaning without masternodes
         assert(consensus.V20Height >= consensus.DIP0003Height);
@@ -1147,7 +1162,7 @@ class CRegTestParams : public CChainParams {
      */
     void UpdateLLMQTestParameters(const RegTestOptions& opts, const Consensus::LLMQType llmqType)
     {
-        assert(llmqType == Consensus::LLMQType::LLMQ_TEST || llmqType == Consensus::LLMQType::LLMQ_TEST_INSTANTSEND || llmqType == Consensus::LLMQType::LLMQ_TEST_PLATFORM);
+        assert(llmqType == Consensus::LLMQType::LLMQ_TEST || llmqType == Consensus::LLMQType::LLMQ_TEST_INSTANTSEND || llmqType == Consensus::LLMQType::LLMQ_TEST_PLATFORM || llmqType == Consensus::LLMQType::LLMQ_TEST_DIP0024);
 
         const auto it = opts.llmq_test_parameters.find(llmqType);
         if (it == opts.llmq_test_parameters.end()) return;

src/llmq/dkgsession.cpp

@@ -122,7 +122,7 @@ bool CDKGSession::Init(const uint256& _myProTxHash, int _quorumIndex)
         const CBlockIndex* pCycleQuorumBaseBlockIndex = m_quorum_base_block_index->GetAncestor(cycleQuorumBaseHeight);
         std::stringstream ss;
         for (const auto& mn : members) {
-            ss << mn->dmn->proTxHash.ToString().substr(0, 4) << " | ";
+            ss << mn->dmn->proTxHash.ToString().substr(0, 16) << " | ";
         }
         logger.Batch("DKGComposition h[%d] i[%d] DKG:[%s]", pCycleQuorumBaseBlockIndex->nHeight, quorumIndex, ss.str());
     }

src/llmq/snapshot.h

@@ -32,7 +32,8 @@ enum class SnapshotSkipMode : int {
     MODE_NO_SKIPPING = 0,
     MODE_SKIPPING_ENTRIES = 1,
     MODE_NO_SKIPPING_ENTRIES = 2,
-    MODE_ALL_SKIPPED = 3
+    MODE_ALL_SKIPPED = 3,
+    MODE_SHORT_QUARTERS = 4, // Some quarter is short or empty; the list holds the walk steps at which a candidate was skipped
 };
 template<> struct is_serializable_enum<SnapshotSkipMode> : std::true_type {};