feat: mnlistdiff v20 CL sig quorums (#5377)

## Issue being fixed or feature implemented
Implementation of Randomness Beacon Part 3.

Starting from v20 activation fork, members for quorums are sorted using
(if available) the best CL signature found in Coinbase.
If no CL signature is present yet, then the usual way is used (By using
Blockhash instead)

The actual new way to shuffle is already implemented in
https://github.com/dashpay/dash/pull/5366.

SPV clients also need to calculate members, but they only know block
headers.
Since Coinbase is in the actual block, then they lack the required
information to correctly calculate quorum members.

## What was done?
- Message `MNLISTIDFF` is enriched with a new field `quorumsCLSigs`.
This field holds the Chainlock Signature required for each set of
indexes corresponding to quorums in field `newQuorums`.
-  Protocol version has been bumped to `70230`.
- Clients with protocol version greater or equal to `70230` will receive
the new field `quorumsCLSigs`.
- The same field is returned in `protx diff` RPC.

Note:
- Field `quorumsCLSigs` will populated only after v20 activation
- If for one or more quorums, no non-null CL sig was found in CbTx then
a null signature is returned in `quorumsCLSigs`.

## How Has This Been Tested?
- Functional test mininode's protocol version was bumped to `70230`.
- `feature_llmq_rotation.py` checks that `quorumsCLSigs` match in both
P2P and RPC messages.

## Breaking Changes
No

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>

doc/release-notes-5377.md

@@ -0,0 +1,25 @@
+Updated RPCs
+--------
+
+- `protx diff` RPC returns a new field `quorumsCLSigs`.
+This field is a list containing: a ChainLock signature and the list of corresponding quorum indexes in `newQuorums`.
+
+`MNLISTDIFF` P2P message
+--------
+
+Starting with protocol version `70230`, the following fields are added to the `MNLISTDIFF` after `newQuorums`.
+
+| Field              | Type                  | Size     | Description                                                         |
+|--------------------|-----------------------|----------|---------------------------------------------------------------------|
+| quorumsCLSigsCount | compactSize uint      | 1-9      | Number of quorumsCLSigs elements                                    |
+| quorumsCLSigs      | quorumsCLSigsObject[] | variable | CL Sig used to calculate members per quorum indexes (in newQuorums) |
+
+The content of `quorumsCLSigsObject`:
+
+| Field         | Type             | Size     | Description                                                                                 |
+|---------------|------------------|----------|---------------------------------------------------------------------------------------------|
+| signature     | BLSSig           | 96       | ChainLock signature                                                                         |
+| indexSetCount | compactSize uint | 1-9      | Number of quorum indexes using the same `signature` for their member calculation            |
+| indexSet      | uint16_t[]       | variable | Quorum indexes corresponding in `newQuorums` using `signature` for their member calculation |
+
+Note: The `quorumsCLSigs` field in both RPC and P2P will only be populated after the v20 activation.

src/bls/bls.h

@@ -88,6 +88,10 @@ public:
     {
         return !((*this) == r);
     }
+    bool operator<(const C& r) const
+    {
+        return GetHash() < r.GetHash();
+    }
 
     bool IsValid() const
     {

src/evo/simplifiedmns.cpp

@@ -9,6 +9,7 @@
 #include <evo/deterministicmns.h>
 #include <llmq/blockprocessor.h>
 #include <llmq/commitment.h>
+#include <llmq/quorums.h>
 #include <llmq/utils.h>
 #include <evo/specialtx.h>
 
@@ -23,6 +24,7 @@
 #include <validation.h>
 #include <key_io.h>
 #include <util/underlying.h>
+#include <util/enumerate.h>
 
 CSimplifiedMNListEntry::CSimplifiedMNListEntry(const CDeterministicMN& dmn) :
     proRegTxHash(dmn.proTxHash),
@@ -178,6 +180,48 @@ bool CSimplifiedMNListDiff::BuildQuorumsDiff(const CBlockIndex* baseBlockIndex,
             newQuorums.emplace_back(*qc);
         }
     }
+
+    return true;
+}
+
+bool CSimplifiedMNListDiff::BuildQuorumChainlockInfo(const CBlockIndex* blockIndex)
+{
+    // Group quorums (indexes corresponding to entries of newQuorums) per CBlockIndex containing the expected CL signature in CbTx.
+    // We want to avoid to load CbTx now, as more than one quorum will target the same block: hence we want to load CbTxs once per block (heavy operation).
+    std::multimap<const CBlockIndex*, uint16_t>  workBaseBlockIndexMap;
+
+    for (const auto [idx, e] : enumerate(newQuorums)) {
+        auto quorum = llmq::quorumManager->GetQuorum(e.llmqType, e.quorumHash);
+        // In case of rotation, all rotated quorums rely on the CL sig expected in the cycleBlock (the block of the first DKG) - 8
+        // In case of non-rotation, quorums rely on the CL sig expected in the block of the DKG - 8
+        const CBlockIndex* pWorkBaseBlockIndex =
+                blockIndex->GetAncestor(quorum->m_quorum_base_block_index->nHeight - quorum->qc->quorumIndex - 8);
+
+        workBaseBlockIndexMap.insert(std::make_pair(pWorkBaseBlockIndex, idx));
+    }
+
+    for(auto it = workBaseBlockIndexMap.begin(); it != workBaseBlockIndexMap.end(); ) {
+        // Process each key (CBlockIndex containing the expected CL signature in CbTx) of the std::multimap once
+        const CBlockIndex* pWorkBaseBlockIndex = it->first;
+        const auto cbcl = GetNonNullCoinbaseChainlock(pWorkBaseBlockIndex);
+        CBLSSignature sig;
+        if (cbcl.has_value()) {
+            sig = cbcl.value().first;
+        }
+        // Get the range of indexes (values) for the current key and merge them into a single std::set
+        const auto [begin, end] = workBaseBlockIndexMap.equal_range(it->first);
+        std::set<uint16_t> idx_set;
+        std::transform(begin, end, std::inserter(idx_set, idx_set.end()), [](const auto& pair) { return pair.second; });
+        // Advance the iterator to the next key
+        it = end;
+
+        // Different CBlockIndex can contain the same CL sig in CbTx (both non-null or null during the first blocks after v20 activation)
+        // Hence, we need to merge the std::set if another std::set already exists for the same sig.
+        if (auto [it_sig, inserted] = quorumsCLSigs.insert({sig, idx_set}); !inserted) {
+            it_sig->second.insert(idx_set.begin(), idx_set.end());
+        }
+    }
+
     return true;
 }
 
@@ -233,6 +277,18 @@ void CSimplifiedMNListDiff::ToJson(UniValue& obj, bool extended) const
             obj.pushKV("merkleRootQuorums", cbTxPayload.merkleRootQuorums.ToString());
         }
     }
+
+    UniValue quorumsCLSigsArr(UniValue::VARR);
+    for (const auto& [signature, quorumsIndexes] : quorumsCLSigs) {
+        UniValue j(UniValue::VOBJ);
+        UniValue idxArr(UniValue::VARR);
+        for (const auto& idx : quorumsIndexes) {
+            idxArr.push_back(idx);
+        }
+        j.pushKV(signature.ToString(),idxArr);
+        quorumsCLSigsArr.push_back(j);
+    }
+    obj.pushKV("quorumsCLSigs", quorumsCLSigsArr);
 }
 
 CSimplifiedMNListDiff BuildSimplifiedDiff(const CDeterministicMNList& from, const CDeterministicMNList& to, bool extended)
@@ -310,6 +366,13 @@ bool BuildSimplifiedMNListDiff(const uint256& baseBlockHash, const uint256& bloc
         return false;
     }
 
+    if (llmq::utils::IsV20Active(blockIndex)) {
+        if (!mnListDiffRet.BuildQuorumChainlockInfo(blockIndex)) {
+            errorRet = strprintf("failed to build quorums chainlocks info");
+            return false;
+        }
+    }
+
     // TODO store coinbase TX in CBlockIndex
     CBlock block;
     if (!ReadBlockFromDisk(block, blockIndex, Params().GetConsensus())) {

src/evo/simplifiedmns.h

@@ -137,6 +137,10 @@ public:
     std::vector<std::pair<uint8_t, uint256>> deletedQuorums; // p<LLMQType, quorumHash>
     std::vector<llmq::CFinalCommitment> newQuorums;
 
+    // Map of Chainlock Signature used for shuffling per set of quorums
+    // The set of quorums is the set of indexes corresponding to entries in newQuorums
+    std::map<CBLSSignature, std::set<uint16_t>> quorumsCLSigs;
+
     SERIALIZE_METHODS(CSimplifiedMNListDiff, obj)
     {
         if ((s.GetType() & SER_NETWORK) && s.GetVersion() >= MNLISTDIFF_VERSION_ORDER) {
@@ -148,6 +152,9 @@ public:
         }
         READWRITE(obj.deletedMNs, obj.mnList);
         READWRITE(obj.deletedQuorums, obj.newQuorums);
+        if ((s.GetType() & SER_NETWORK) && s.GetVersion() >= MNLISTDIFF_CHAINLOCKS_PROTO_VERSION) {
+            READWRITE(obj.quorumsCLSigs);
+        }
     }
 
     CSimplifiedMNListDiff();
@@ -155,6 +162,7 @@ public:
 
     bool BuildQuorumsDiff(const CBlockIndex* baseBlockIndex, const CBlockIndex* blockIndex,
                           const llmq::CQuorumBlockProcessor& quorum_block_processor);
+    bool BuildQuorumChainlockInfo(const CBlockIndex* blockIndex);
 
     void ToJson(UniValue& obj, bool extended = false) const;
 };

src/version.h

@@ -11,7 +11,7 @@
  */
 
 
-static const int PROTOCOL_VERSION = 70229;
+static const int PROTOCOL_VERSION = 70230;
 
 //! initial proto version, to be increased after version/verack negotiation
 static const int INIT_PROTO_VERSION = 209;
@@ -20,7 +20,7 @@ static const int INIT_PROTO_VERSION = 209;
 static const int MIN_PEER_PROTO_VERSION = 70215;
 
 //! minimum proto version of masternode to accept in DKGs
-static const int MIN_MASTERNODE_PROTO_VERSION = 70227;
+static const int MIN_MASTERNODE_PROTO_VERSION = 70230;
 
 //! protocol version is included in MNAUTH starting with this version
 static const int MNAUTH_NODE_VER_VERSION = 70218;
@@ -55,6 +55,9 @@ static const int SMNLE_VERSIONED_PROTO_VERSION = 70228;
 //! Versioned Simplified Masternode List Entries were introduced in this version
 static const int MNLISTDIFF_VERSION_ORDER = 70229;
 
+//! Masternode type was introduced in this version
+static const int MNLISTDIFF_CHAINLOCKS_PROTO_VERSION = 70230;
+
 // Make sure that none of the values above collide with `ADDRV2_FORMAT`.
 
 #endif // BITCOIN_VERSION_H

test/functional/feature_llmq_rotation.py

@@ -9,10 +9,11 @@ feature_llmq_rotation.py
 Checks LLMQs Quorum Rotation
 
 '''
+import struct
 from io import BytesIO
 
 from test_framework.test_framework import DashTestFramework
-from test_framework.messages import CBlock, CBlockHeader, CCbTx, CMerkleBlock, FromHex, hash256, msg_getmnlistd, QuorumId
+from test_framework.messages import CBlock, CBlockHeader, CCbTx, CMerkleBlock, FromHex, hash256, msg_getmnlistd, QuorumId, ser_uint256, sha256
 from test_framework.mininode import P2PInterface
 from test_framework.util import (
     assert_equal,
@@ -94,7 +95,7 @@ class LLMQQuorumRotationTest(DashTestFramework):
 
         expectedDeleted = []
         expectedNew = [h_100_0, h_106_0, h_104_0, h_100_1, h_106_1, h_104_1]
-        quorumList = self.test_getmnlistdiff_quorums(b_h_0, b_h_1, {}, expectedDeleted, expectedNew)
+        quorumList = self.test_getmnlistdiff_quorums(b_h_0, b_h_1, {}, expectedDeleted, expectedNew, testQuorumsCLSigs=False)
 
         self.activate_v20(expected_activation_height=1440)
         self.log.info("Activated v20 at height:" + str(self.nodes[0].getblockcount()))
@@ -122,9 +123,21 @@ class LLMQQuorumRotationTest(DashTestFramework):
 
         b_0 = self.nodes[0].getbestblockhash()
 
-        self.log.info("Wait for chainlock")
+        # At this point, we want to wait for CLs just before the self.mine_cycle_quorum to diversify the CLs in CbTx.
+        # Although because here a new quorum cycle is starting, and we don't want to mine them now, mine 8 blocks (to skip all DKG phases)
+        nodes = [self.nodes[0]] + [mn.node for mn in self.mninfo.copy()]
+        self.nodes[0].generate(8)
+        self.sync_blocks(nodes)
         self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash())
 
+        # And for the remaining blocks, enforce new CL in CbTx
+        skip_count = 23 - (self.nodes[0].getblockcount() % 24)
+        for i in range(skip_count):
+            self.nodes[0].generate(1)
+            self.sync_blocks(nodes)
+            self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash())
+
+
         (quorum_info_0_0, quorum_info_0_1) = self.mine_cycle_quorum(llmq_type_name=llmq_type_name, llmq_type=llmq_type)
         assert(self.test_quorum_listextended(quorum_info_0_0, llmq_type_name))
         assert(self.test_quorum_listextended(quorum_info_0_1, llmq_type_name))
@@ -207,8 +220,8 @@ class LLMQQuorumRotationTest(DashTestFramework):
         wait_until(lambda: self.nodes[0].getbestblockhash() == new_quorum_blockhash, sleep=1)
         assert_equal(self.nodes[0].quorum("list", llmq_type), new_quorum_list)
 
-    def test_getmnlistdiff_quorums(self, baseBlockHash, blockHash, baseQuorumList, expectedDeleted, expectedNew):
+    def test_getmnlistdiff_quorums(self, baseBlockHash, blockHash, baseQuorumList, expectedDeleted, expectedNew, testQuorumsCLSigs = True):
-        d = self.test_getmnlistdiff_base(baseBlockHash, blockHash)
+        d = self.test_getmnlistdiff_base(baseBlockHash, blockHash, testQuorumsCLSigs)
 
         assert_equal(set(d.deletedQuorums), set(expectedDeleted))
         assert_equal(set([QuorumId(e.llmqType, e.quorumHash) for e in d.newQuorums]), set(expectedNew))
@@ -235,7 +248,7 @@ class LLMQQuorumRotationTest(DashTestFramework):
         return newQuorumList
 
 
-    def test_getmnlistdiff_base(self, baseBlockHash, blockHash):
+    def test_getmnlistdiff_base(self, baseBlockHash, blockHash, testQuorumsCLSigs):
         hexstr = self.nodes[0].getblockheader(blockHash, False)
         header = FromHex(CBlockHeader(), hexstr)
 
@@ -258,9 +271,87 @@ class LLMQQuorumRotationTest(DashTestFramework):
         assert_equal(set([int(e["proRegTxHash"], 16) for e in d2["mnList"]]), set([e.proRegTxHash for e in d.mnList]))
         assert_equal(set([QuorumId(e["llmqType"], int(e["quorumHash"], 16)) for e in d2["deletedQuorums"]]), set(d.deletedQuorums))
         assert_equal(set([QuorumId(e["llmqType"], int(e["quorumHash"], 16)) for e in d2["newQuorums"]]), set([QuorumId(e.llmqType, e.quorumHash) for e in d.newQuorums]))
-
+        # Check if P2P quorumsCLSigs matches with the corresponding in RPC
+        rpc_quorums_clsigs_dict = {k: v for d in d2["quorumsCLSigs"] for k, v in d.items()}
+        # p2p_quorums_clsigs_dict is constructed from the P2P message so it can be easily compared to rpc_quorums_clsigs_dict
+        p2p_quorums_clsigs_dict = dict()
+        for key, value in d.quorumsCLSigs.items():
+            idx_list = list(value)
+            p2p_quorums_clsigs_dict[key.hex()] = idx_list
+        assert_equal(rpc_quorums_clsigs_dict, p2p_quorums_clsigs_dict)
+        # The following test must be checked only after v20 activation
+        if testQuorumsCLSigs:
+            # Total number of corresponding quorum indexes in quorumsCLSigs must be equal to the total of quorums in newQuorums
+            assert_equal(len(d2["newQuorums"]), sum(len(value) for value in rpc_quorums_clsigs_dict.values()))
+            for cl_sig, value in rpc_quorums_clsigs_dict.items():
+                for q in value:
+                    self.test_verify_quorums(d2["newQuorums"][q], cl_sig)
         return d
 
+    def test_verify_quorums(self, quorum_info, quorum_cl_sig):
+        if int(quorum_cl_sig, 16) == 0:
+            # Skipping null-CLSig. No need to verify old way of shuffling (using BlockHash)
+            return
+        if quorum_info["version"] == 2 or quorum_info["version"] == 4:
+            # Skipping rotated quorums. Too complicated to implemented.
+            # TODO: Implement rotated quorum verification using CLSigs
+            return
+        quorum_height = self.nodes[0].getblock(quorum_info["quorumHash"])["height"]
+        work_height = quorum_height - 8
+        modifier = self.get_hash_modifier(quorum_info["llmqType"], work_height, quorum_cl_sig)
+        mn_list = self.nodes[0].protx('diff', 1, work_height)["mnList"]
+        scored_mns = []
+        # Compute each valid mn score and add them (mn, score) in scored_mns
+        for mn in mn_list:
+            if mn["isValid"] is False:
+                # Skip invalid mns
+                continue
+            score = self.compute_mn_score(mn, modifier)
+            scored_mns.append((mn, score))
+        # Sort the list based on the score in descending order
+        scored_mns.sort(key=lambda x: x[1], reverse=True)
+        llmq_size = self.get_llmq_size(int(quorum_info["llmqType"]))
+        # Keep the first llmq_size mns
+        scored_mns = scored_mns[:llmq_size]
+        quorum_info_members = self.nodes[0].quorum('info', quorum_info["llmqType"], quorum_info["quorumHash"])["members"]
+        # Make sure that each quorum member returned from quorum info RPC is matched in our scored_mns list
+        for m in quorum_info_members:
+            found = False
+            for e in scored_mns:
+                if m["proTxHash"] == e[0]["proRegTxHash"]:
+                    found = True
+                    break
+            assert found
+        return
+
+    def get_hash_modifier(self, llmq_type, height, cl_sig):
+        bytes = b""
+        bytes += struct.pack('<B', int(llmq_type))
+        bytes += struct.pack('<i', int(height))
+        bytes += bytes.fromhex(cl_sig)
+        return hash256(bytes)[::-1].hex()
+
+    def compute_mn_score(self, mn, modifier):
+        bytes = b""
+        bytes += ser_uint256(int(mn["proRegTxHash"], 16))
+        bytes += ser_uint256(int(mn["confirmedHash"], 16))
+        confirmed_hash_pro_regtx_hash = sha256(bytes)[::-1].hex()
+
+        bytes_2 = b""
+        bytes_2 += ser_uint256(int(confirmed_hash_pro_regtx_hash, 16))
+        bytes_2 += ser_uint256(int(modifier, 16))
+        score = sha256(bytes_2)[::-1].hex()
+        return int(score, 16)
+
+    def get_llmq_size(self, llmq_type):
+        return {
+            100: 4, # In this test size for llmqType 100 is overwritten to 4
+            102: 3,
+            103: 4,
+            104: 4, # In this test size for llmqType 104 is overwritten to 4
+            106: 3
+        }.get(llmq_type, -1)
+
     def test_quorum_listextended(self, quorum_info, llmq_type_name):
         extended_quorum_list = self.nodes[0].quorum("listextended")[llmq_type_name]
         quorum_dict = {}

test/functional/test_framework/messages.py

@@ -32,7 +32,7 @@ from test_framework.util import hex_str_to_bytes, assert_equal
 import dash_hash
 
 MIN_VERSION_SUPPORTED = 60001
-MY_VERSION = 70229  # MNLISTDIFF_VERSION_ORDER
+MY_VERSION = 70230  # MNLISTDIFF_CHAINLOCKS_PROTO_VERSION
 MY_SUBVERSION = b"/python-mininode-tester:0.0.3%s/"
 MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37)
 
@@ -2023,7 +2023,7 @@ class msg_getmnlistd:
 QuorumId = namedtuple('QuorumId', ['llmqType', 'quorumHash'])
 
 class msg_mnlistdiff:
-    __slots__ = ("baseBlockHash", "blockHash", "merkleProof", "cbTx", "nVersion", "deletedMNs", "mnList", "deletedQuorums", "newQuorums",)
+    __slots__ = ("baseBlockHash", "blockHash", "merkleProof", "cbTx", "nVersion", "deletedMNs", "mnList", "deletedQuorums", "newQuorums", "quorumsCLSigs")
     command = b"mnlistdiff"
 
     def __init__(self):
@@ -2036,6 +2036,8 @@ class msg_mnlistdiff:
         self.mnList = []
         self.deletedQuorums = []
         self.newQuorums = []
+        self.quorumsCLSigs = {}
+
 
     def deserialize(self, f):
         self.nVersion = struct.unpack("<H", f.read(2))[0]
@@ -2062,6 +2064,14 @@ class msg_mnlistdiff:
             qc = CFinalCommitment()
             qc.deserialize(f)
             self.newQuorums.append(qc)
+        self.quorumsCLSigs = {}
+        for i in range(deser_compact_size(f)):
+            signature = f.read(96)
+            idx_set = set()
+            for j in range(deser_compact_size(f)):
+                set_element = struct.unpack('H', f.read(2))[0]
+                idx_set.add(set_element)
+            self.quorumsCLSigs[signature] = idx_set
 
     def __repr__(self):
         return "msg_mnlistdiff(baseBlockHash=%064x, blockHash=%064x)" % (self.baseBlockHash, self.blockHash)

test/lint/lint-cppcheck-dash.sh

@@ -34,7 +34,7 @@ IGNORED_WARNINGS=(
     "src/test/dip0020opcodes_tests.cpp:.* warning: There is an unknown macro here somewhere. Configuration is required. If BOOST_FIXTURE_TEST_SUITE is a macro then please configure it."
     "src/ctpl_stl.h:.*22: warning: Dereferencing '_f' after it is deallocated / released"
     "src/cachemultimap.h:.*: warning: Variable 'mapIt' can be declared as reference to const"
-
+    "src/evo/simplifiedmns.cpp:.*:20: warning: Consider using std::copy algorithm instead of a raw loop."
 #    "src/llmq/snapshot.cpp:.*:17: warning: Consider using std::copy algorithm instead of a raw loop."
 #    "src/llmq/snapshot.cpp:.*:18: warning: Consider using std::copy algorithm instead of a raw loop."