From 8639aa480b0fd65cd8b9d7c6a56f995175108c13 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 6 Nov 2026 01:05:16 -0600 Subject: [PATCH 1/2] feat(llmq): report whether a mined-commitment scan covered its window GetMinedCommitmentsUntilBlock stops silently when an index entry cannot be read, so a short result cannot tell a young chain from a damaged index. A caller that must not mistake a damaged index for a short one needs that difference. ScanMinedCommitmentsUntilBlock and its indexed variant return the same quorum bases plus `complete`: true when the requested number of entries was read or the walk left the type's index range, false when an entry inside the range could not be read or cannot be trusted: a value that does not deserialize, an impossible base height, quorum bases that do not strictly decrease along an index (a commitment is mined within its own quorum's DKG interval) or repeat across quorum indexes, a LevelDB read error, or a table block that the iterator stepped over because it failed its checksum. The bases read before that point are kept, except those a contradiction implicates: either entry of a pair that does not decrease, and both entries naming a repeated base, may be the wrong one, so neither is returned. Once the window is full no further entry or iterator status decides coverage. CDBIterator::HasError() exposes LevelDB's iterator status for this, since Valid() alone reads a skipped corrupt block as the end of the data. The range end is detected by comparing the raw key prefix before deserializing, and malformed values are reported instead of asserted on. Existing GetMinedCommitments* callers are unchanged. Co-Authored-By: Claude Opus 5.5 --- src/dbwrapper.cpp | 1 + src/dbwrapper.h | 6 + src/llmq/blockprocessor.cpp | 107 +++++++++++++++ src/llmq/blockprocessor.h | 22 +++ src/test/llmq_blockprocessor_tests.cpp | 180 +++++++++++++++++++++++++ src/test/util/llmq_tests.h | 45 +++++++ 6 files changed, 361 insertions(+) diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index ecc406699b32..1e6541a50664 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -427,6 +427,7 @@ Span CDBIterator::GetValueImpl() const CDBIterator::~CDBIterator() = default; bool CDBIterator::Valid() const { return m_impl_iter->iter->Valid(); } +bool CDBIterator::HasError() const { return !m_impl_iter->iter->status().ok(); } void CDBIterator::SeekToFirst() { m_impl_iter->iter->SeekToFirst(); } void CDBIterator::Next() { m_impl_iter->iter->Next(); } diff --git a/src/dbwrapper.h b/src/dbwrapper.h index affdb583438a..f997592fff94 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -163,6 +163,8 @@ class CDBIterator ~CDBIterator(); bool Valid() const; + //! True once a read failed, e.g. on a corrupt block. Entries may then have been skipped. + bool HasError() const; void SeekToFirst(); @@ -423,6 +425,10 @@ class CDBTransactionIterator return transactionIt != transaction.writes.end() || parentIt->Valid(); } + bool HasError() const { + return parentIt->HasError(); + } + void Next() { if (transactionIt == transaction.writes.end() && !parentIt->Valid()) { return; diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index f8442e093ab4..a97513130101 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -17,6 +17,8 @@ #include #include #include +#include +#include #include #include #include @@ -833,6 +835,111 @@ std::vector CQuorumBlockProcessor::GetMinedCommitmentsIndexe return ret; } +// Appends quorum bases from one inversed-height index, newest first, until `bases` holds +// `max_count` entries. Returns false if an entry inside `prefix` could not be read or contradicts +// the index, keeping the entries before it that the contradiction does not implicate; a key outside +// `prefix` ends the walk because no older entry exists. +static bool ReadMinedCommitmentIndex(CEvoDB& evo_db, const DataStream& prefix, const CBlockIndex& pindex, + size_t max_count, std::vector& bases) +{ + AssertLockNotHeld(evo_db.cs); + LOCK(evo_db.cs); + + const auto unreadable{[&](const char* reason) { + LogPrint(BCLog::LLMQ, "Mined commitment index below height %d unreadable after %d entries: %s\n", + pindex.nHeight, bases.size(), reason); + return false; + }}; + + DataStream first_key{prefix}; + first_key << htobe32_internal(std::numeric_limits::max() - pindex.nHeight); + + auto it{evo_db.GetCurTransaction().NewIteratorUniquePtr()}; + try { + for (it->Seek(first_key); bases.size() < max_count; it->Next()) { + // The iterator steps over a block it cannot read, so later entries are not the next newest. + if (it->HasError()) return unreadable("read error"); + if (!it->Valid()) break; + const DataStream key{it->GetKey()}; + if (key.size() < prefix.size() || !std::equal(prefix.begin(), prefix.end(), key.begin())) { + break; + } + int base_height{-1}; + if (key.size() != prefix.size() + sizeof(uint32_t) || !it->GetValue(base_height)) { + return unreadable("malformed entry"); + } + const int64_t mined_height{std::numeric_limits::max() - + be32toh_internal(ReadLE32(UCharCast(key.data() + prefix.size())))}; + const CBlockIndex* base{pindex.GetAncestor(base_height)}; + if (!base || base_height >= mined_height) return unreadable("inconsistent entry"); + // A commitment is mined within its own quorum's DKG interval, so along one index the + // quorum bases strictly decrease. Either of the two entries may be the wrong one. + if (!bases.empty() && base_height >= bases.back()->nHeight) { + bases.pop_back(); + return unreadable("out-of-order entry"); + } + bases.push_back(base); + } + } catch (const dbwrapper_error& e) { + return unreadable(e.what()); + } + return true; +} + +MinedCommitmentScan CQuorumBlockProcessor::ScanMinedCommitmentsUntilBlock(Consensus::LLMQType llmqType, + gsl::not_null pindex, + size_t max_count) const +{ + DataStream prefix{}; + prefix << DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT << llmqType; + MinedCommitmentScan scan; + scan.complete = ReadMinedCommitmentIndex(m_evoDb, prefix, *pindex, max_count, scan.bases); + return scan; +} + +MinedCommitmentScan CQuorumBlockProcessor::ScanMinedCommitmentsIndexedUntilBlock(Consensus::LLMQType llmqType, + gsl::not_null pindex, + size_t max_count) const +{ + MinedCommitmentScan scan; + const auto llmq_params{Params().GetLLMQ(llmqType)}; + if (!Assume(llmq_params.has_value())) { + return scan; + } + // Cycle by cycle, the cycle-th newest commitment of every quorum index in index order. + std::vector> taken(llmq_params->signingActiveQuorumCount); + for (size_t cycle{0}; scan.bases.size() < max_count; ++cycle) { + const size_t found_before{scan.bases.size()}; + for (const int quorum_index : util::irange(llmq_params->signingActiveQuorumCount)) { + if (scan.bases.size() == max_count) break; + DataStream prefix{}; + prefix << DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT_Q_INDEXED << llmqType << quorum_index; + std::vector index_bases; + if (!ReadMinedCommitmentIndex(m_evoDb, prefix, *pindex, cycle + 1, index_bases)) { + // Earlier cycles took this index's newer entries; drop any the failed read no longer keeps. + for (size_t rank{index_bases.size()}; rank < taken[quorum_index].size(); ++rank) { + std::erase(scan.bases, taken[quorum_index][rank]); + } + return scan; + } + if (index_bases.size() > cycle) { + // Quorums of different quorum indexes have different base blocks, so either entry may + // be the wrong one. + if (std::erase(scan.bases, index_bases[cycle]) > 0) { + LogPrint(BCLog::LLMQ, "Mined commitment index of quorum index %d repeats base height %d\n", + quorum_index, index_bases[cycle]->nHeight); + return scan; + } + scan.bases.push_back(index_bases[cycle]); + } + taken[quorum_index] = std::move(index_bases); + } + if (scan.bases.size() == found_before) break; + } + scan.complete = true; + return scan; +} + // The returned quorums are in reversed order, so the most recent one is at index 0 std::map> CQuorumBlockProcessor::GetMinedAndActiveCommitmentsUntilBlock(gsl::not_null pindex) const { diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index f506a3c99deb..790247390387 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -22,6 +22,7 @@ #include #include #include +#include class BlockValidationState; class CBlock; @@ -47,6 +48,17 @@ using QcHashMap = std::map>; //! As above, but keyed by quorumIndex, for rotation-enabled types. using QcIndexedHashMap = std::map>; +struct MinedCommitmentScan { + //! Quorum base blocks of mined commitments. Non-indexed scans return them newest first. Indexed + //! scans return them by cycle rank, newest rank first, and by quorum index within each rank. + std::vector bases; + //! True if `bases` holds the requested number of entries or every older entry was read; false + //! if an index entry could not be read or contradicts another, so an absent quorum may still + //! have been mined. Entries a contradiction implicates are left out. A wrong entry that fits + //! the index's order is not detected. + bool complete{false}; +}; + /** Erase a mined commitment unless another chainstate still contains its block. */ bool EraseMinedCommitmentIfUnreferenced(CEvoDB& evo_db, const Chainstate& chainstate, gsl::not_null pindex, @@ -155,6 +167,16 @@ class CQuorumBlockProcessor const CBlockIndex* pindex, size_t cycle) const; std::optional GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const; + + //! GetMinedCommitmentsUntilBlock, stopping at an entry it cannot read or trust and reporting + //! whether the walk read every entry it needed. + MinedCommitmentScan ScanMinedCommitmentsUntilBlock(Consensus::LLMQType llmqType, + gsl::not_null pindex, size_t max_count) const; + //! GetMinedCommitmentsIndexedUntilBlock for rotation-enabled types, reporting the same way. + MinedCommitmentScan ScanMinedCommitmentsIndexedUntilBlock(Consensus::LLMQType llmqType, + gsl::not_null pindex, + size_t max_count) const; + private: //! Called from every site that writes or erases mined commitment state. void DropQcHashesCache() EXCLUSIVE_LOCKS_REQUIRED(!m_qc_hashes_cache_mutex); diff --git a/src/test/llmq_blockprocessor_tests.cpp b/src/test/llmq_blockprocessor_tests.cpp index 46ccfc1f1f16..4ba30396f3e7 100644 --- a/src/test/llmq_blockprocessor_tests.cpp +++ b/src/test/llmq_blockprocessor_tests.cpp @@ -6,13 +6,17 @@ #include #include +#include #include #include +#include #include #include #include #include #include +#include +#include #include #include #include @@ -25,6 +29,8 @@ #include +#include + using namespace llmq; using namespace llmq::testutils; @@ -135,4 +141,178 @@ BOOST_FIXTURE_TEST_CASE(unrequested_qfcommit_is_dropped_and_scored, TestChain100 SetMockTime(0s); } +namespace { +std::vector BaseHeights(const MinedCommitmentScan& scan) +{ + std::vector heights; + heights.reserve(scan.bases.size()); + for (const auto* base : scan.bases) + heights.push_back(base->nHeight); + return heights; +} +} // namespace + +// A scan of mined commitments may only be read as "nothing older exists" when it filled the requested +// window or walked off the end of the type's index. Entries are installed directly in EvoDB. +BOOST_FIXTURE_TEST_CASE(scan_mined_commitments_reports_coverage, TestChain100Setup) +{ + const auto& qbp = *m_node.llmq_ctx->quorum_block_processor; + constexpr auto type = Consensus::LLMQType::LLMQ_TEST; + LOCK(::cs_main); + const CChain& active = m_node.chainman->ActiveChain(); + { + auto db_tx = m_node.evodb->BeginTransaction(); + for (const int base : {10, 20, 30, 40, 50}) { + WriteMinedCommitment(*m_node.evodb, CreateValidCommitment(GetLLMQParams(type), active[base]->GetBlockHash()), + active[base + 1]->GetBlockHash(), /*mined_height=*/base + 1, /*quorum_height=*/base); + } + db_tx->Commit(); + } + // The key right after this type's range is no index key at all and holds no height. The walk must + // end at the prefix without parsing it. + const auto next_type = static_cast(std23::to_underlying(type) + 1); + BOOST_REQUIRE( + m_node.evodb->GetRawDB().Write(std::make_pair(DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT, next_type), uint8_t{0})); + + auto scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({50, 40, 30, 20})); + BOOST_CHECK(scan.complete); + BOOST_CHECK(scan.bases == qbp.GetMinedCommitmentsUntilBlock(type, active.Tip(), 4)); + + // Young chain: fewer entries than requested, and complete because the walk left the type's range. + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active[25], 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({20, 10})); + BOOST_CHECK(scan.complete); + + // An index value that does not deserialize, at mined height 35. + BOOST_REQUIRE(m_node.evodb->GetRawDB().Write(BuildInversedHeightKey(type, 35), uint8_t{0})); + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({50, 40})); + BOOST_CHECK(!scan.complete); + // A window filled before the corrupt entry, or a walk starting below it, never reads it. + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 2); + BOOST_CHECK(BaseHeights(scan) == std::vector({50, 40})); + BOOST_CHECK(scan.complete); + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active[34], 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({30, 20, 10})); + BOOST_CHECK(scan.complete); + + // An entry whose quorum base is not below the block that mined it. + BOOST_REQUIRE(m_node.evodb->GetRawDB().Write(BuildInversedHeightKey(type, 15), 15)); + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active[34], 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({30, 20})); + BOOST_CHECK(!scan.complete); + + // A commitment is mined within its own quorum's DKG interval, so the quorum bases strictly + // decrease along the index. A second entry for base 50, or an entry for an older base above + // base 50's, contradicts the next one, and either of the two may be the wrong one. + BOOST_REQUIRE(m_node.evodb->GetRawDB().Write(BuildInversedHeightKey(type, 55), 50)); + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 2); + BOOST_CHECK(BaseHeights(scan).empty()); + BOOST_CHECK(!scan.complete); + BOOST_REQUIRE(m_node.evodb->GetRawDB().Write(BuildInversedHeightKey(type, 55), 45)); + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 2); + BOOST_CHECK(BaseHeights(scan).empty()); + BOOST_CHECK(!scan.complete); + // An entry before the contradicting pair is kept. + BOOST_REQUIRE(m_node.evodb->GetRawDB().Write(BuildInversedHeightKey(type, 58), 52)); + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 3); + BOOST_CHECK(BaseHeights(scan) == std::vector({52})); + BOOST_CHECK(!scan.complete); +} + +// LevelDB steps over a table block that fails its checksum and keeps iterating. The entries after +// such a block are not the next newest ones, and the window is not complete. The EvoDB here is on +// disk, closed, corrupted and reopened. +BOOST_FIXTURE_TEST_CASE(scan_mined_commitments_stops_at_corrupt_block, TestChain100Setup) +{ + constexpr auto type = Consensus::LLMQType::LLMQ_TEST; + const fs::path path{m_args.GetDataDirNet() / "corrupt_index"}; + { + CEvoDB db{util::DbWrapperParams{.path = path, .memory = false, .wipe = true}}; + for (const int base : {20, 30, 40, 50}) { + BOOST_REQUIRE(db.GetRawDB().Write(BuildInversedHeightKey(type, base + 1), PaddedIndexValue(base))); + } + db.GetRawDB().CompactFull(); + } + // Each padded entry ends its table block, so base 40's entry has a block of its own. + BOOST_REQUIRE(CorruptTableBlock(path / "evodb", std::vector(64, 40))); + + CEvoDB db{util::DbWrapperParams{.path = path, .memory = false, .wipe = false}}; + const CQuorumBlockProcessor qbp{*m_node.chainman, *m_node.dmnman, db, *m_node.llmq_ctx->qsnapman, + /*bls_threads=*/0}; + LOCK(::cs_main); + const CChain& active = m_node.chainman->ActiveChain(); + auto scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({50})); + BOOST_CHECK(!scan.complete); + // A window filled before the corrupt block does not depend on it, and a walk starting below it + // never reaches it. + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active.Tip(), 1); + BOOST_CHECK(BaseHeights(scan) == std::vector({50})); + BOOST_CHECK(scan.complete); + scan = qbp.ScanMinedCommitmentsUntilBlock(type, active[35], 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({30, 20})); + BOOST_CHECK(scan.complete); +} + +// Rotation-enabled types are read cycle by cycle, quorum index by quorum index, as +// GetMinedCommitmentsIndexedUntilBlock does. +BOOST_FIXTURE_TEST_CASE(scan_mined_commitments_indexed_reports_coverage, TestChain100Setup) +{ + const auto& qbp = *m_node.llmq_ctx->quorum_block_processor; + constexpr auto type = Consensus::LLMQType::LLMQ_TEST_DIP0024; + BOOST_REQUIRE(Params().GetLLMQ(type).has_value()); + BOOST_REQUIRE_EQUAL(GetLLMQParams(type).signingActiveQuorumCount, 2); + LOCK(::cs_main); + const CChain& active = m_node.chainman->ActiveChain(); + { + // Cycles at 24, 48 and 72: quorum index i has its base at cycle + i and is mined at cycle + 12 + i. + auto db_tx = m_node.evodb->BeginTransaction(); + for (const int cycle : {24, 48, 72}) { + for (const int quorum_index : {0, 1}) { + m_node.evodb->Write(BuildInversedHeightKeyIndexed(type, cycle + 12 + quorum_index, quorum_index), + cycle + quorum_index); + } + } + db_tx->Commit(); + } + + auto scan = qbp.ScanMinedCommitmentsIndexedUntilBlock(type, active.Tip(), 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({72, 73, 48, 49})); + BOOST_CHECK(scan.complete); + BOOST_CHECK(scan.bases == qbp.GetMinedCommitmentsIndexedUntilBlock(type, active.Tip(), 4)); + + scan = qbp.ScanMinedCommitmentsIndexedUntilBlock(type, active.Tip(), 10); + BOOST_CHECK(BaseHeights(scan) == std::vector({72, 73, 48, 49, 24, 25})); + BOOST_CHECK(scan.complete); + scan = qbp.ScanMinedCommitmentsIndexedUntilBlock(type, active[40], 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({24, 25})); + BOOST_CHECK(scan.complete); + + // Quorum index 0's newest entry names base 40, older than its next entry's base 48. The first + // cycle takes base 40; the second meets the contradiction and drops it again. + m_node.evodb->Write(BuildInversedHeightKeyIndexed(type, 84, 0), 40); + scan = qbp.ScanMinedCommitmentsIndexedUntilBlock(type, active.Tip(), 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({73})); + BOOST_CHECK(!scan.complete); + m_node.evodb->Write(BuildInversedHeightKeyIndexed(type, 84, 0), 72); + + // Quorum index 1 has an unreadable entry at mined height 62, met in the second cycle. + BOOST_REQUIRE(m_node.evodb->GetRawDB().Write(BuildInversedHeightKeyIndexed(type, 62, 1), uint8_t{0})); + scan = qbp.ScanMinedCommitmentsIndexedUntilBlock(type, active.Tip(), 4); + BOOST_CHECK(BaseHeights(scan) == std::vector({72, 73, 48})); + BOOST_CHECK(!scan.complete); + scan = qbp.ScanMinedCommitmentsIndexedUntilBlock(type, active.Tip(), 2); + BOOST_CHECK(BaseHeights(scan) == std::vector({72, 73})); + BOOST_CHECK(scan.complete); + + // Quorum indexes never share a base: quorum index 1's newest entry names index 0's base 72, and + // neither entry is kept. + BOOST_REQUIRE(m_node.evodb->GetRawDB().Write(BuildInversedHeightKeyIndexed(type, 86, 1), 72)); + scan = qbp.ScanMinedCommitmentsIndexedUntilBlock(type, active.Tip(), 2); + BOOST_CHECK(BaseHeights(scan).empty()); + BOOST_CHECK(!scan.complete); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/llmq_tests.h b/src/test/util/llmq_tests.h index c8e3718c8861..fe3d328122d8 100644 --- a/src/test/util/llmq_tests.h +++ b/src/test/util/llmq_tests.h @@ -23,6 +23,11 @@ #include #include +#include + +#include +#include +#include #include #include #include @@ -119,6 +124,7 @@ inline uint256 GetTestBlockHash(uint32_t n) { return ArithToUint256(arith_uint25 // mined-commitment state without a full DKG/mining path. inline const std::string DB_MINED_COMMITMENT = "q_mc"; inline const std::string DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT = "q_mcih"; +inline const std::string DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT_Q_INDEXED = "q_mcihi"; inline std::tuple BuildInversedHeightKey(Consensus::LLMQType llmqType, int nMinedHeight) @@ -127,6 +133,45 @@ inline std::tuple BuildInversedHeigh htobe32_internal(std::numeric_limits::max() - nMinedHeight)); } +inline std::tuple BuildInversedHeightKeyIndexed(Consensus::LLMQType llmqType, + int nMinedHeight, + int quorumIndex) +{ + return std::make_tuple(DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT_Q_INDEXED, llmqType, quorumIndex, + htobe32_internal(std::numeric_limits::max() - nMinedHeight)); +} + +// An inversed-height index value padded past LevelDB's 4 KiB table block size, so that the entry +// ends the table block it is written to. The padding repeats the quorum height's low byte for +// CorruptTableBlock to find; readers deserialize only the leading quorum height. +inline std::pair> PaddedIndexValue(int quorum_height) +{ + return {quorum_height, std::vector(8192, static_cast(quorum_height))}; +} + +// Flips a byte of the first occurrence of `needle` in a table file of the closed LevelDB at +// `db_dir`, so that the table block holding it fails its checksum. +inline bool CorruptTableBlock(const fs::path& db_dir, const std::vector& needle) +{ + for (const auto& entry : fs::directory_iterator(db_dir)) { + if (entry.path().extension() != ".ldb") continue; + std::vector bytes; + { + std::ifstream in{entry.path(), std::ios::binary}; + bytes.assign(std::istreambuf_iterator(in), std::istreambuf_iterator()); + } + const auto pos{std::search(bytes.begin(), bytes.end(), needle.begin(), needle.end(), + [](char a, uint8_t b) { return static_cast(a) == b; })}; + if (pos == bytes.end()) continue; + *pos ^= 1; + std::ofstream out{entry.path(), std::ios::binary | std::ios::trunc}; + out.write(bytes.data(), bytes.size()); + out.close(); + return !out.fail(); + } + return false; +} + // Store a mined commitment as if it was mined at `mined_height` for the quorum whose base block // is the active-chain ancestor at `quorum_height`. GetMinedCommitmentsUntilBlock iterates // inverted-height keys in [pindex->nHeight, 0), so scan height must be >= mined_height and From 36057c81ceaa9a637671c69a0cd9cab777b539d5 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 6 Nov 2026 01:31:29 -0600 Subject: [PATCH 2/2] feat(interfaces): look up a Platform quorum at its signed Core height getPlatformQuorums exported every retained Platform quorum key (1440 on mainnet) with no notion of when each quorum could sign. A verifier that accepts any of them accepts state signed by a quorum that was never in the signing set at the state's Core height, and an empty or short list reads the same as "this quorum does not exist", which would ban an honest endpoint on a node that is merely behind or has a damaged index. It has no production caller. getPlatformQuorum(llmq_type, quorum_hash, core_chain_locked_height) replaces it and answers FOUND (with the 48-byte basic-scheme key), INVALID or UNAVAILABLE: - FOUND: the quorum is in the window at that height, the newest commitments mined at or below it: signingActiveQuorumCount + 2 for a non-rotating type, and 3 * signingActiveQuorumCount for a rotating type, read cycle by cycle across its quorum indexes. Each is a quorum's signing-active period plus two DKG intervals of slack. - INVALID: not the Platform type; a height above INT32_MAX; a height more than those DKG intervals ((signingActiveQuorumCount + 2), or 3 for a rotating type) below a ChainLock this node knows, clamped to the tip; or a quorum hash that is no block of the active chain below that height, when the window was read in full and the block at that height is covered by the best ChainLock whose block this node knows. - UNAVAILABLE: everything else. That includes missing node services, a height above the tip, a height that only the tip shows stale because no ChainLock is known (after a restart during a ChainLock stall, Platform may still carry the stalled one), and a selected commitment record that cannot be read, is missing, is inconsistent with its index entry or carries an invalid key. Mined commitments passed consensus validation, so those record states are local faults, never evidence against the quorum, and a database read error is answered rather than thrown. A quorum outside the window is UNAVAILABLE, not INVALID, when it was mined at or below the height. Platform verifies a block against its last committed validator set, and Tenderdash installs a new set only when it commits, so after a Core-height jump the first Platform block carrying the new height is signed by the outgoing quorum however far it has fallen behind the window. The lookup counts a quorum as never mined by then only when its hash is no block of the active chain below the height. That proof covers this node's active chain only. Above a ChainLock that chain may be a competing branch, and a ChainLock's height alone does not rule that out: a CLSIG can name a block this node does not know or a branch it has not activated yet. A quorum found in the window is answered regardless of ChainLock coverage. Both ChainLock reads happen before taking cs_main: the best lock whose block is known, which alone can lock part of the active chain, and the best height, which bounds staleness only and is clamped to the tip, since a ChainLock can arrive before its block. The staleness bound is the pure llmq::PlatformQuorumMinSignedHeight, the window size is llmq::PlatformEligibleQuorumCount, and both follow llmq::PlatformEligibleIntervals. For non-rotating types a lookup reads at most signingActiveQuorumCount + 2 index entries and one commitment record. Co-Authored-By: Claude Opus 5.5 --- src/interfaces/node.h | 23 +- src/llmq/params.h | 30 +++ src/node/interfaces.cpp | 100 +++++--- src/test/interfaces_tests.cpp | 413 ++++++++++++++++++++++++++++----- src/test/llmq_params_tests.cpp | 38 +++ 5 files changed, 512 insertions(+), 92 deletions(-) diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 559746221f13..f7ce427638cb 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -273,16 +273,21 @@ class LLMQ int32_t m_expiry_height{0}; }; virtual std::vector getQuorumStats() = 0; - struct PlatformQuorum { - uint256 m_quorum_hash{}; - std::vector m_pubkey{}; //!< serialized BLS public key (basic scheme) - int32_t m_height{0}; + enum class PlatformQuorumStatus { FOUND, INVALID, UNAVAILABLE }; + struct PlatformQuorumResult { + PlatformQuorumStatus status{PlatformQuorumStatus::UNAVAILABLE}; + std::vector public_key; //!< 48-byte basic-scheme key for FOUND; empty otherwise }; - //! Locally retained quorums of the given LLMQ type with their public - //! keys. Used by the GUI Platform client to verify Platform state-root - //! quorum signatures against locally synced quorum data. This includes - //! retained signing quorums that are no longer in the active signing set. - virtual std::vector getPlatformQuorums(uint8_t llmq_type) = 0; + //! Whether `quorum_hash` (Core internal byte order) could sign Platform state whose signed metadata + //! carries `core_chain_locked_height`, judged from this node's active chain. FOUND means it is among + //! the quorums eligible at that height. INVALID means the local chain proves it could not: the + //! height is stale against a ChainLock this node knows, or the hash is no block of the active chain + //! below that height, so no quorum with it was mined by then, which counts only if the active + //! chain's block at that height is, or is an ancestor of, the block of the best ChainLock whose + //! block this node knows. UNAVAILABLE means this node proves neither, as for an older quorum, which + //! may still sign the one Platform block that first carries a height after a jump. + virtual PlatformQuorumResult getPlatformQuorum(uint8_t llmq_type, const uint256& quorum_hash, + uint32_t core_chain_locked_height) = 0; //! Serialized InstantSend lock for the given txid, or empty if the //! transaction has no islock (used to build asset lock proofs). virtual std::vector getInstantSendLock(const uint256& txid) = 0; diff --git a/src/llmq/params.h b/src/llmq/params.h index 2c975d828cb8..b9701e7f46f7 100644 --- a/src/llmq/params.h +++ b/src/llmq/params.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_LLMQ_PARAMS_H #define BITCOIN_LLMQ_PARAMS_H +#include #include #include #include @@ -512,6 +513,35 @@ static constexpr std::array available_llmqs = { } // namespace Consensus +namespace llmq { +//! DKG intervals beyond its signing-active period during which a Platform quorum is still accepted +//! as the signer of Platform state. +static constexpr int PLATFORM_QUORUM_SLACK{2}; + +//! DKG intervals during which a Platform quorum may sign Platform state. A quorum of a non-rotating +//! type is signing-active for signingActiveQuorumCount intervals, one of a rotating type for one. +[[nodiscard]] constexpr int PlatformEligibleIntervals(const Consensus::LLMQParams& params) +{ + return (params.useRotation ? 1 : params.signingActiveQuorumCount) + PLATFORM_QUORUM_SLACK; +} + +//! How many of the newest mined Platform quorums may sign Platform state. A non-rotating type mines +//! one quorum per DKG interval, a rotating type one per quorum index. +[[nodiscard]] constexpr int PlatformEligibleQuorumCount(const Consensus::LLMQParams& params) +{ + return PlatformEligibleIntervals(params) * (params.useRotation ? params.signingActiveQuorumCount : 1); +} + +//! Lowest Core chain-locked height that Platform state may carry for this node to verify its +//! quorum signature, given the node's ChainLock height. Bounding by the ChainLock rather than the +//! tip keeps state signed before a ChainLock stall verifiable while the tip keeps moving. +[[nodiscard]] constexpr int PlatformQuorumMinSignedHeight(const Consensus::LLMQParams& params, int cl_height) +{ + const int64_t window{int64_t{PlatformEligibleIntervals(params)} * params.dkgInterval}; + return static_cast(std::max(0, int64_t{cl_height} - window)); +} +} // namespace llmq + // This must be outside of all namespaces. We must also duplicate the forward declaration of is_serializable_enum to // avoid inclusion of serialize.h here. template struct is_serializable_enum; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index a12196d561e9..170c9502967b 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -37,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -86,6 +88,7 @@ #include #include +#include #include #include #include @@ -684,44 +687,79 @@ class LLMQImpl : public LLMQ } return stats; } - std::vector getPlatformQuorums(uint8_t llmq_type) override + PlatformQuorumResult getPlatformQuorum(uint8_t llmq_type, const uint256& quorum_hash, + uint32_t core_chain_locked_height) override { - std::vector ret; + PlatformQuorumResult invalid{PlatformQuorumStatus::INVALID, {}}; + const auto type{static_cast(llmq_type)}; + const auto llmq_params{Params().GetLLMQ(type)}; + if (type != Params().GetConsensus().llmqTypePlatform || !llmq_params.has_value()) { + return invalid; + } + if (core_chain_locked_height > static_cast(std::numeric_limits::max())) { + return invalid; + } + const int signed_height{static_cast(core_chain_locked_height)}; if (!context().llmq_ctx || !context().llmq_ctx->quorum_block_processor || !context().chainman) { - return ret; + return {}; } - const auto* pindex{WITH_LOCK(::cs_main, return context().chainman->ActiveChain().Tip())}; - if (!pindex) { - return ret; + // Only a ChainLock whose block is known can lock part of the active chain; the best height may + // belong to a block this node has not seen. + const CBlockIndex* chainlocked_block{nullptr}; + int32_t best_cl_height{-1}; + if (const auto& chainlocks{context().chainlocks}) { + chainlocked_block = chainlocks->GetBestChainlockWithPindex().second; + best_cl_height = chainlocks->GetBestChainLockHeight(); } - const auto type{static_cast(llmq_type)}; - const auto llmq_params{Params().GetLLMQ(type)}; - if (!llmq_params.has_value()) { - return ret; + + LOCK(::cs_main); + const CChain& active{context().chainman->ActiveChain()}; + const CBlockIndex* tip{active.Tip()}; + if (!tip || signed_height > tip->nHeight) { + return {}; } - // Drive proofs may be signed by an older Platform quorum while they - // are still consensus-valid and retained locally. Export the full - // retained-key window, not only the current signing-active set. - const auto quorum_count{static_cast(std::max(llmq_params->signingActiveQuorumCount, - llmq_params->keepOldKeys))}; - // Read mined final commitments directly: they already carry the quorum - // hash and public key, so there is no need to materialize full CQuorum - // objects (member lists, vvec/contribution reads, quorum cache inserts) - // via ScanQuorums. Newest-first, matching ScanQuorums' ordering. + // A ChainLock can arrive before its block. Without one the tip still proves a height fresh, + // but not stale: Platform may carry a ChainLock this node has not learned yet. + const int cl_height{best_cl_height >= 0 ? std::min(best_cl_height, tip->nHeight) : tip->nHeight}; + if (signed_height < llmq::PlatformQuorumMinSignedHeight(*llmq_params, cl_height)) { + return best_cl_height >= 0 ? invalid : PlatformQuorumResult{}; + } + const auto& qbp{*context().llmq_ctx->quorum_block_processor}; - const auto quorum_base_block_indexes{llmq_params->useRotation - ? qbp.GetMinedCommitmentsIndexedUntilBlock(type, pindex, quorum_count) - : qbp.GetMinedCommitmentsUntilBlock(type, pindex, quorum_count)}; - for (const auto* pQuorumBaseBlockIndex : quorum_base_block_indexes) { - const auto qc{qbp.GetMinedCommitment(type, pQuorumBaseBlockIndex->GetBlockHash()).first}; - if (!qc.quorumPublicKey.IsValid()) continue; - ret.emplace_back(PlatformQuorum{ - .m_quorum_hash = qc.quorumHash, - .m_pubkey = qc.quorumPublicKey.ToByteVector(/*specificLegacyScheme=*/false), - .m_height = pQuorumBaseBlockIndex->nHeight, - }); + const CBlockIndex* signed_block{active[signed_height]}; + const auto window{static_cast(llmq::PlatformEligibleQuorumCount(*llmq_params))}; + const auto scan{llmq_params->useRotation ? qbp.ScanMinedCommitmentsIndexedUntilBlock(type, signed_block, window) + : qbp.ScanMinedCommitmentsUntilBlock(type, signed_block, window)}; + if (std::ranges::none_of(scan.bases, [&](const CBlockIndex* base) { return base->GetBlockHash() == quorum_hash; })) { + // Platform verifies a block against its last committed validator set, so the block that + // first carries a height may be signed by a quorum no longer in the window at that height. + // A quorum mined at or below the height has its base block on the active chain below it; + // any other hash could never have signed. + const CBlockIndex* base{context().chainman->m_blockman.LookupBlockIndex(quorum_hash)}; + const bool never_mined{!base || base->nHeight >= signed_height || !active.Contains(base)}; + // That is final only on the part of the active chain a ChainLock locks; beyond it the + // active chain may be a competing branch. Unlike Chainlocks::HasChainLock, this keeps the + // lock while spork 19 is off. A node whose quorum index reads as corrupt may have bad storage + // elsewhere too, so it rejects nothing until the window reads cleanly. + const bool chainlocked{chainlocked_block && chainlocked_block->GetAncestor(signed_height) == signed_block}; + return never_mined && chainlocked && scan.complete ? invalid : PlatformQuorumResult{}; } - return ret; + + // Mined commitments passed consensus validation, so an unreadable or inconsistent record is + // local corruption, not evidence against the quorum. + std::pair record; + try { + record = qbp.GetMinedCommitment(type, quorum_hash); + } catch (const dbwrapper_error&) { + return {}; + } + const auto& [qc, mined_block_hash]{record}; + const CBlockIndex* mined_block{context().chainman->m_blockman.LookupBlockIndex(mined_block_hash)}; + if (qc.llmqType != type || qc.quorumHash != quorum_hash || !mined_block || !active.Contains(mined_block) || + mined_block->nHeight > signed_height || !qc.quorumPublicKey.IsValid()) { + return {}; + } + return {PlatformQuorumStatus::FOUND, qc.quorumPublicKey.ToByteVector(/*specificLegacyScheme=*/false)}; } std::vector getInstantSendLock(const uint256& txid) override { diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp index 2bdca0b6f459..2a9d9fbf06f1 100644 --- a/src/test/interfaces_tests.cpp +++ b/src/test/interfaces_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -13,21 +14,30 @@ #include #include #include +#include +#include #include