Merge dashpay#6724: test: functional tests for invalid QuorumCommitment

c9ef70a tests: add is_mature for quorum generation logs (Konstantin Akimov)
59060b5 fmt: order imports and fix gap in feature_llmq_dkgerrors.py (Konstantin Akimov)
58377f8 test: added functional tests for invalid CQuorumCommitment (Konstantin Akimov)
bb0b8b0 test: add serialization/deserialization of CFinalCommitmentPayload (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  As I noticed implementing dashpay#6692 if BlsChecker works incorrectly it won't be caught by unit or functional tests. See also dashpay#6692 (comment) how 6692 has been tested without this PR.

  ## What was done?
  This PR introduces new functional tests to validated that `llmqType`, `membersSig`, `quorumSig` and `quorumPublicKey` are indeed validated by Dash Core as part of consensus.

  ## How Has This Been Tested?
  See changes in `feature_llmq_dkgerrors.py`

  ## Breaking Changes
  N/A

  ## 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
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  kwvg:
    utACK c9ef70a
  UdjinM6:
    utACK c9ef70a

Tree-SHA512: ad61f8c845f6681765105224b2a84e0b206791e2c9a786433b9aa91018ab44c1fa764528196fd079f42f08a55794756ba8c9249c6eb10af6fe97c33fa4757f44

Author: PastaPastaPasta

test/functional/feature_llmq_dkgerrors.py

@@ -3,8 +3,17 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-from test_framework.test_framework import DashTestFramework
+import copy
+from io import BytesIO
+
+from test_framework.messages import (
+    CBlock,
+    CFinalCommitmentPayload,
+    from_hex,
+)
 
+from test_framework.test_framework import DashTestFramework
+from test_framework.util import assert_equal
 '''
 feature_llmq_dkgerrors.py
 
@@ -20,8 +29,9 @@ def run_test(self):
         self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
-        self.log.info("Mine one quorum without simulating any errors")
-        qh = self.mine_quorum()
+        qh = self.test_qc()
+
+        self.log.info("Mine one regular quorum with no invalid members is mined at this point")
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)
 
         self.log.info("Lets omit the contribution")
@@ -68,6 +78,59 @@ def run_test(self):
         qh = self.mine_quorum(expected_contributions=3, expected_complaints=0, expected_justifications=0, expected_commitments=2)
         self.assert_member_valid(qh, self.mninfo[0].proTxHash, True)
 
+    def test_qc(self):
+        quorumHash = self.mine_quorum(skip_maturity=True)
+        best = self.nodes[0].getbestblockhash()
+        block_hex = self.nodes[0].getblock(best, 0)
+
+        block = from_hex(CBlock(), block_hex)
+
+        self.nodes[0].invalidateblock(best)
+
+        self.test_invalid(block, 'bad-qc-commitment-type', lambda qc : (setattr(qc, 'llmqType', 77), qc)[1])
+        self.test_invalid(block, 'bad-qc-invalid', lambda qc : (setattr(qc, 'llmqType', 106), qc)[1])
+        # TODO: test quorumIndex for rotation quorums
+        # self.test_invalid(block, 'bad-qc-invalid', lambda qc : (setattr(qc, 'quorumIndex', 2), qc)[1])
+        self.test_invalid(block, 'bad-qc-invalid', lambda qc : (setattr(qc, 'quorumSig', getattr(qc, 'membersSig')), qc)[1])
+        self.test_invalid(block, 'bad-qc-invalid', lambda qc : (setattr(qc, 'membersSig', getattr(qc, 'quorumSig')), qc)[1])
+        self.test_invalid(block, 'bad-qc-invalid', lambda qc : (setattr(qc, 'quorumPublicKey', b'\x00' * 48), qc)[1])
+        # TODO: test quorumVvecHash
+        # TODO: test signers
+        # TODO: test validMembers
+
+        self.nodes[0].reconsiderblock(best)
+        # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions
+        self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(self.nodes))
+
+        return quorumHash
+
+    def test_invalid(self, original_block, error, transform):
+        node = self.nodes[0]
+
+        block = copy.deepcopy(original_block)
+
+        for tx in block.vtx:
+            if tx.nType == 6:
+                qc_payload = CFinalCommitmentPayload()
+                qc_payload.deserialize(BytesIO(tx.vExtraPayload))
+                # test not only quorum 100, rotation quorums also and single-node quorum
+                if qc_payload.commitment.llmqType == 100:
+                    qc_payload.commitment = transform(qc_payload.commitment)
+
+                tx.vExtraPayload = qc_payload.serialize()
+                tx.rehash()
+
+        def assert_submitblock(block, result_str_1, result_str_2=None):
+            block.hashMerkleRoot = block.calc_merkle_root()
+            block.solve()
+            result_str_2 = result_str_2 or 'duplicate-invalid'
+            assert_equal(result_str_1, node.submitblock(hexdata=block.serialize().hex()))
+            assert_equal(result_str_2, node.submitblock(hexdata=block.serialize().hex()))
+
+        # TODO: implement similar validation not only for submitblock but for p2p message
+        assert_submitblock(block, error)
+
+
     def assert_member_valid(self, quorumHash, proTxHash, expectedValid):
         q = self.nodes[0].quorum('info', 100, quorumHash, True)
         for m in q['members']:

test/functional/test_framework/messages.py

@@ -1407,6 +1407,34 @@ def __repr__(self):
             .format(self.nVersion, self.llmqType, self.quorumHash, self.quorumIndex, repr(self.signers),
                     repr(self.validMembers), self.quorumPublicKey.hex(), self.quorumVvecHash, self.quorumSig.hex(), self.membersSig.hex())
 
+
+class CFinalCommitmentPayload:
+    __slots__ = ("nVersion", "nHeight", "commitment")
+
+    def __init__(self):
+        self.set_null()
+
+    def set_null(self):
+        self.nVersion = 0
+        self.nHeight = 0
+        self.commitment = CFinalCommitment()
+
+    def deserialize(self, f):
+        self.nVersion = struct.unpack("<H", f.read(2))[0]
+        self.nHeight = struct.unpack("<I", f.read(4))[0]
+        self.commitment = CFinalCommitment()
+        self.commitment.deserialize(f)
+
+    def serialize(self):
+        r = b""
+        r += struct.pack("<H", self.nVersion)
+        r += struct.pack("<I", self.nHeight)
+        r += self.commitment.serialize()
+        return r
+
+    def __repr__(self):
+        return f"CFinalCommitmentPayload(nVersion={self.nVersion} nHeight={self.nHeight} commitment={self.commitment})"
+
 class CGovernanceObject:
     __slots__ = ("nHashParent", "nRevision", "nTime", "nCollateralHash", "vchData", "nObjectType",
                  "masternodeOutpoint", "vchSig")

test/functional/test_framework/test_framework.py

@@ -1815,7 +1815,7 @@ def move_blocks(self, nodes, num_blocks):
         self.bump_mocktime(1, nodes=nodes)
         self.generate(self.nodes[0], num_blocks, sync_fun=lambda: self.sync_blocks(nodes))
 
-    def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connections=None, expected_members=None, expected_contributions=None, expected_complaints=0, expected_justifications=0, expected_commitments=None, mninfos_online=None, mninfos_valid=None):
+    def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connections=None, expected_members=None, expected_contributions=None, expected_complaints=0, expected_justifications=0, expected_commitments=None, mninfos_online=None, mninfos_valid=None, skip_maturity=False):
         spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
         spork23_active = self.nodes[0].spork('show')['SPORK_23_QUORUM_POSE'] <= 1
 
@@ -1893,10 +1893,11 @@ def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connec
         assert_equal(q, new_quorum)
         quorum_info = self.nodes[0].quorum("info", llmq_type, new_quorum)
 
-        # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions
-        self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(nodes))
+        if not skip_maturity:
+            # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions
+            self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(nodes))
 
-        self.log.info("New quorum: height=%d, quorumHash=%s, quorumIndex=%d, minedBlock=%s" % (quorum_info["height"], new_quorum, quorum_info["quorumIndex"], quorum_info["minedBlock"]))
+        self.log.info(f"New quorum: height={quorum_info['height']}, quorumHash={new_quorum}, is_mature={not skip_maturity} quorumIndex={quorum_info['quorumIndex']}, minedBlock={quorum_info['minedBlock']}")
 
         return new_quorum