From e34105f0a3d6eb1960e32e62fae62630c805822b Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Mon, 8 Jun 2026 18:55:06 -0500 Subject: [PATCH 1/2] fix: deduplicate QRINFO base blocks --- src/llmq/snapshot.cpp | 9 ++++--- src/test/llmq_snapshot_tests.cpp | 34 ++++++++++++++++++++++++ test/functional/feature_llmq_rotation.py | 3 +++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/llmq/snapshot.cpp b/src/llmq/snapshot.cpp index 81043f98729d..2d27a667536e 100644 --- a/src/llmq/snapshot.cpp +++ b/src/llmq/snapshot.cpp @@ -14,6 +14,8 @@ #include +#include + namespace { constexpr std::string_view DB_QUORUM_SNAPSHOT{"llmq_S"}; @@ -74,10 +76,9 @@ 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; }); - } + std::sort(baseBlockIndexes.begin(), baseBlockIndexes.end(), + [](const CBlockIndex* a, const CBlockIndex* b) { return a->nHeight < b->nHeight; }); + baseBlockIndexes.erase(std::unique(baseBlockIndexes.begin(), baseBlockIndexes.end()), baseBlockIndexes.end()); } const CBlockIndex* tipBlockIndex = chainman.ActiveChain().Tip(); diff --git a/src/test/llmq_snapshot_tests.cpp b/src/test/llmq_snapshot_tests.cpp index 7e3b1d191ece..a353a2d64638 100644 --- a/src/test/llmq_snapshot_tests.cpp +++ b/src/test/llmq_snapshot_tests.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -176,6 +177,39 @@ 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 blocks(4); + std::vector hashes{ + GetTestBlockHash(10), + GetTestBlockHash(20), + GetTestBlockHash(30), + GetTestBlockHash(40), + }; + for (size_t i{0}; i < blocks.size(); ++i) { + blocks[i].nHeight = static_cast((i + 1) * 10); + blocks[i].phashBlock = &hashes[i]; + } + + std::vector 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]); + + std::vector sorted_repeated_base_blocks{ + &blocks[0], + &blocks[1], + &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]); +} + BOOST_AUTO_TEST_CASE(get_quorum_rotation_info_serialization_test) { CGetQuorumRotationInfo getInfo; diff --git a/test/functional/feature_llmq_rotation.py b/test/functional/feature_llmq_rotation.py index f0e3651ddbf5..813016c5b9ef 100755 --- a/test/functional/feature_llmq_rotation.py +++ b/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"]) From 9f3086755d355332ed7816ec69504092d8585482 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Mon, 15 Jun 2026 08:14:47 -0500 Subject: [PATCH 2/2] fix: scope QRINFO base-block dedupe to non-legacy construction Address review feedback: the prior commit unconditionally sorted and deduplicated baseBlockIndexes, which touched the legacy construction path used to serve peers < EFFICIENT_QRINFO_VERSION. Restrict deduplication to the non-legacy path so the wire response to older peers stays bit-for-bit identical to the pre-fix behavior. The legacy path keeps the sort it always relied on for baseBlockIndexes.back() and GetLastBaseBlockHash(). Strengthen the unit test to explicitly prove the legacy no-op invariant: GetLastBaseBlockHash returns the same hash whether the sorted input contains duplicates or not, so legacy multiplicity is harmless. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/llmq/snapshot.cpp | 15 ++++++++++++--- src/test/llmq_snapshot_tests.cpp | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/llmq/snapshot.cpp b/src/llmq/snapshot.cpp index 2d27a667536e..00ea1f0b7cb2 100644 --- a/src/llmq/snapshot.cpp +++ b/src/llmq/snapshot.cpp @@ -76,9 +76,18 @@ bool BuildQuorumRotationInfo(CDeterministicMNManager& dmnman, CQuorumSnapshotMan } baseBlockIndexes.push_back(blockIndex); } - std::sort(baseBlockIndexes.begin(), baseBlockIndexes.end(), - [](const CBlockIndex* a, const CBlockIndex* b) { return a->nHeight < b->nHeight; }); - baseBlockIndexes.erase(std::unique(baseBlockIndexes.begin(), baseBlockIndexes.end()), baseBlockIndexes.end()); + if (use_legacy_construction) { + // Legacy construction (served to peers < EFFICIENT_QRINFO_VERSION) only needs the + // input sorted; do not deduplicate so the wire response stays bit-for-bit identical + // to the pre-fix behavior for older peers. + std::sort(baseBlockIndexes.begin(), baseBlockIndexes.end(), + [](const CBlockIndex* a, const CBlockIndex* b) { return a->nHeight < b->nHeight; }); + } else { + std::sort(baseBlockIndexes.begin(), baseBlockIndexes.end(), + [](const CBlockIndex* a, const CBlockIndex* b) { return a->nHeight < b->nHeight; }); + baseBlockIndexes.erase(std::unique(baseBlockIndexes.begin(), baseBlockIndexes.end()), + baseBlockIndexes.end()); + } } const CBlockIndex* tipBlockIndex = chainman.ActiveChain().Tip(); diff --git a/src/test/llmq_snapshot_tests.cpp b/src/test/llmq_snapshot_tests.cpp index a353a2d64638..8ed6c16da5d2 100644 --- a/src/test/llmq_snapshot_tests.cpp +++ b/src/test/llmq_snapshot_tests.cpp @@ -191,6 +191,7 @@ BOOST_AUTO_TEST_CASE(get_last_base_block_hash_repeated_base_blocks_test) blocks[i].phashBlock = &hashes[i]; } + // Non-legacy: sorts internally, so unsorted input with duplicates is fine. std::vector unsorted_repeated_base_blocks{ &blocks[2], &blocks[0], @@ -200,14 +201,28 @@ BOOST_AUTO_TEST_CASE(get_last_base_block_hash_repeated_base_blocks_test) 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 sorted_repeated_base_blocks{ &blocks[0], &blocks[1], &blocks[1], &blocks[2], }; + std::vector 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)