Merge #7349: fix: deduplicate QRINFO base blocks

470ec0e refactor: clarify QRINFO base-block sorting (PastaClaw)
9f30867 fix: scope QRINFO base-block dedupe to non-legacy construction (PastaClaw)
e34105f fix: deduplicate QRINFO base blocks (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  QRINFO request handling accepts caller-provided base block hashes and carries the
  validated block indexes through quorum rotation response construction. Repeated
  base hashes do not add useful information, so this change canonicalizes those
  base indexes before the builder phases use them.

  Provenance: thepastaclaw/tracker#1450

  ## What was done?

  - Sort validated QRINFO base block indexes by height for both construction
    paths.
  - Remove duplicate base block index pointers before response construction.
  - Add focused unit coverage for repeated base block selection behavior.
  - Add RPC functional coverage showing repeated base hashes produce the same
    result as a single base hash.

  ## How Has This Been Tested?

  Environment: macOS arm64, local Dash Core worktree based on `upstream/develop`.

  Commands run:

  - `git diff --check upstream/develop..HEAD` — passed
  - `make -C src -j$(sysctl -n hw.ncpu) test/test_dash` — passed earlier in this
    worktree before the final Python-only test addition
  - `src/test/test_dash --run_test=llmq_snapshot_tests/get_last_base_block_hash_repeated_base_blocks_test`
    — passed
  - `test/functional/feature_llmq_rotation.py` — skipped locally because this
    worktree was configured with `--disable-wallet`
  - `code-review dashpay/dash upstream/develop fix/qrinfo-dedupe-base-hashes ...`
    — passed with recommendation: ship

  Also checked:

  - `gh api repos/dashpay/dash --jq '.default_branch'` — `develop`
  - Open PR search for duplicate QRINFO/baseBlockHashes dedupe work — no duplicate
    open PR found

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [ ] 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
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 470ec0e
  PastaPastaPasta:
    utACK 470ec0e

Tree-SHA512: e160f2d5ee0019854a9721d357ecd7d59b5b2294a264668862ab52686fc530c468cdd0dd5cba56958d640cb631f15356b28e5b5cd8db24e00c496278657de60e

Author: PastaPastaPasta

src/llmq/snapshot.cpp

@@ -14,6 +14,8 @@
 
 #include <univalue.h>
 
+#include <algorithm>
+
 namespace {
 constexpr std::string_view DB_QUORUM_SNAPSHOT{"llmq_S"};
 
@@ -74,9 +76,15 @@ bool BuildQuorumRotationInfo(CDeterministicMNManager& dmnman, CQuorumSnapshotMan
             }
             baseBlockIndexes.push_back(blockIndex);
         }
-        if (use_legacy_construction) {
-            std::sort(baseBlockIndexes.begin(), baseBlockIndexes.end(),
-                      [](const CBlockIndex* a, const CBlockIndex* b) { return a->nHeight < b->nHeight; });
+        // Sort in all cases: the legacy path (served to peers < EFFICIENT_QRINFO_VERSION)
+        // relies on the order for baseBlockIndexes.back() and GetLastBaseBlockHash().
+        std::sort(baseBlockIndexes.begin(), baseBlockIndexes.end(),
+                  [](const CBlockIndex* a, const CBlockIndex* b) { return a->nHeight < b->nHeight; });
+        if (!use_legacy_construction) {
+            // Only deduplicate on the non-legacy path; leave the legacy path untouched so the
+            // wire response to older peers stays bit-for-bit identical to the pre-fix behavior.
+            baseBlockIndexes.erase(std::unique(baseBlockIndexes.begin(), baseBlockIndexes.end()),
+                                   baseBlockIndexes.end());
         }
     }
 

src/test/llmq_snapshot_tests.cpp

@@ -5,6 +5,7 @@
 #include <test/util/llmq_tests.h>
 #include <test/util/setup_common.h>
 
+#include <chain.h>
 #include <streams.h>
 #include <univalue.h>
 
@@ -176,6 +177,54 @@ BOOST_AUTO_TEST_CASE(quorum_rotation_info_construction_test)
 // Note: CQuorumRotationInfo serialization requires complex setup
 // This is better tested in functional tests
 
+BOOST_AUTO_TEST_CASE(get_last_base_block_hash_repeated_base_blocks_test)
+{
+    std::vector<CBlockIndex> blocks(4);
+    std::vector<uint256> hashes{
+        GetTestBlockHash(10),
+        GetTestBlockHash(20),
+        GetTestBlockHash(30),
+        GetTestBlockHash(40),
+    };
+    for (size_t i{0}; i < blocks.size(); ++i) {
+        blocks[i].nHeight = static_cast<int>((i + 1) * 10);
+        blocks[i].phashBlock = &hashes[i];
+    }
+
+    // Non-legacy: sorts internally, so unsorted input with duplicates is fine.
+    std::vector<const CBlockIndex*> unsorted_repeated_base_blocks{
+        &blocks[2],
+        &blocks[0],
+        &blocks[1],
+        &blocks[1],
+    };
+    BOOST_CHECK(GetLastBaseBlockHash(unsorted_repeated_base_blocks, &blocks[3], false) == hashes[2]);
+    BOOST_CHECK(GetLastBaseBlockHash(unsorted_repeated_base_blocks, &blocks[1], false) == hashes[1]);
+
+    // Legacy: relies on caller-supplied sort and tolerates duplicates as a no-op.
+    // BuildQuorumRotationInfo deliberately does NOT deduplicate in the legacy path so
+    // the wire response to older peers stays bit-for-bit identical; these checks
+    // demonstrate that the duplicate is harmless to GetLastBaseBlockHash's output.
+    std::vector<const CBlockIndex*> sorted_repeated_base_blocks{
+        &blocks[0],
+        &blocks[1],
+        &blocks[1],
+        &blocks[2],
+    };
+    std::vector<const CBlockIndex*> sorted_unique_base_blocks{
+        &blocks[0],
+        &blocks[1],
+        &blocks[2],
+    };
+    BOOST_CHECK(GetLastBaseBlockHash(sorted_repeated_base_blocks, &blocks[3], true) == hashes[2]);
+    BOOST_CHECK(GetLastBaseBlockHash(sorted_repeated_base_blocks, &blocks[1], true) == hashes[1]);
+    // Legacy no-op proof: duplicate vs unique input produces the same hash.
+    BOOST_CHECK(GetLastBaseBlockHash(sorted_repeated_base_blocks, &blocks[3], true) ==
+                GetLastBaseBlockHash(sorted_unique_base_blocks, &blocks[3], true));
+    BOOST_CHECK(GetLastBaseBlockHash(sorted_repeated_base_blocks, &blocks[1], true) ==
+                GetLastBaseBlockHash(sorted_unique_base_blocks, &blocks[1], true));
+}
+
 BOOST_AUTO_TEST_CASE(get_quorum_rotation_info_serialization_test)
 {
     CGetQuorumRotationInfo getInfo;

test/functional/feature_llmq_rotation.py

@@ -231,6 +231,9 @@ def run_test(self):
         hmc_base_blockhash = self.nodes[0].getblockhash(block_count - (block_count % 24) - 24 - 8)
         best_block_hash = self.nodes[0].getbestblockhash()
         rpc_qr_info = self.nodes[0].quorum("rotationinfo", best_block_hash, False, [hmc_base_blockhash])
+        rpc_qr_info_repeated_base = self.nodes[0].quorum("rotationinfo", best_block_hash, False,
+                                                          [hmc_base_blockhash, hmc_base_blockhash])
+        assert_equal(rpc_qr_info_repeated_base, rpc_qr_info)
         assert_equal(rpc_qr_info["mnListDiffTip"]["blockHash"], best_block_hash)
         assert_equal(rpc_qr_info["mnListDiffTip"]["baseBlockHash"], rpc_qr_info["mnListDiffH"]["blockHash"])
         assert_equal(rpc_qr_info["mnListDiffH"]["baseBlockHash"], rpc_qr_info["mnListDiffAtHMinusC"]["blockHash"])