Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/dbwrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ Span<const std::byte> 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(); }

Expand Down
6 changes: 6 additions & 0 deletions src/dbwrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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;
Expand Down
23 changes: 14 additions & 9 deletions src/interfaces/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,16 +273,21 @@ class LLMQ
int32_t m_expiry_height{0};
};
virtual std::vector<QuorumInfo> getQuorumStats() = 0;
struct PlatformQuorum {
uint256 m_quorum_hash{};
std::vector<uint8_t> 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<uint8_t> 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<PlatformQuorum> 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<uint8_t> getInstantSendLock(const uint256& txid) = 0;
Expand Down
107 changes: 107 additions & 0 deletions src/llmq/blockprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include <checkqueue.h>
#include <consensus/params.h>
#include <consensus/validation.h>
#include <crypto/common.h>
#include <dbwrapper.h>
#include <deploymentstatus.h>
#include <net.h>
#include <primitives/block.h>
Expand Down Expand Up @@ -833,6 +835,111 @@ std::vector<const CBlockIndex*> 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<const CBlockIndex*>& 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<uint32_t>::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<uint32_t>::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);
Comment on lines +875 to +881

@thepastaclaw thepastaclaw Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved at c7d30c71; see the replies below.

🔴 Blocking: Do not return FOUND for a base implicated by an ordering contradiction

The ordering check leaves the previously retained base in bases, even though the contradiction can show that this earlier entry is wrong. getPlatformQuorum ignores scan.complete when it finds a matching hash, and the selected-record checks do not verify that record's mining height against the index entry. In the four-entry interface fixture, overwriting the index value at mined height 51 from base 50 to base 10 produces bases [10], followed by an ordering failure when base 40 is read. A lookup for base 10 then returns FOUND using its valid record mined at height 11, although the healthy eligibility window is [50,40,30,20]. This is detected index inconsistency, not the documented limitation concerning wrong entries that fit the index's order. Exclude matches implicated by ordering contradictions or propagate enough fault information to return UNAVAILABLE for them. Preserve FOUND ahead of an unrelated unreadable entry, and add a lookup regression that requests the implicated retained base.

source: muse-spark-1.3-contributor (phase1-reviewer: general)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and reproduced. With the mined-41 entry rewritten from base 40 to base 10, the scan at h = 44 kept [10] before base 30 contradicted it, and the lookup for base 10 returned FOUND.

Fixed in cdbbb2b. A contradiction now leaves both entries out, because either may be the wrong one:

  • a non-decreasing pair along an index drops the earlier entry;
  • in the indexed scan, it also drops the copy an earlier cycle had already taken from that index;
  • a base repeated across quorum indexes drops the earlier copy too.

Other faults still keep the verified prefix, so FOUND ahead of an unrelated unreadable entry is unchanged and still tested.

Tests:

  • getPlatformQuorum_local_faults adds your case: base 10 at h = 44 is UNAVAILABLE (it was FOUND), and FOUND again once the entry is repaired.
  • The scan tests now expect {} for the duplicate and out-of-order pairs.
  • A new row checks that an entry before the pair is kept.
  • An indexed row has a contradiction met in the second cycle drop the base the first cycle took.

🤖 Posted autonomously by Claude on behalf of pasta.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved (re-reviewed at c7d30c71): Your scanners now exclude both members of detected ordering contradictions, retract implicated entries retained in earlier indexed cycles, and remove earlier cross-index duplicates. The lookup regression requests the implicated base itself, while separate unreadable-entry cases still preserve FOUND for an unaffected prefix.

}
} catch (const dbwrapper_error& e) {
return unreadable(e.what());
}
return true;
}

MinedCommitmentScan CQuorumBlockProcessor::ScanMinedCommitmentsUntilBlock(Consensus::LLMQType llmqType,
gsl::not_null<const CBlockIndex*> 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<const CBlockIndex*> 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<std::vector<const CBlockIndex*>> 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<const CBlockIndex*> 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<Consensus::LLMQType, std::vector<const CBlockIndex*>> CQuorumBlockProcessor::GetMinedAndActiveCommitmentsUntilBlock(gsl::not_null<const CBlockIndex*> pindex) const
{
Expand Down
22 changes: 22 additions & 0 deletions src/llmq/blockprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <functional>
#include <memory>
#include <optional>
#include <vector>

class BlockValidationState;
class CBlock;
Expand All @@ -47,6 +48,17 @@ using QcHashMap = std::map<Consensus::LLMQType, std::vector<uint256>>;
//! As above, but keyed by quorumIndex, for rotation-enabled types.
using QcIndexedHashMap = std::map<Consensus::LLMQType, std::map<int16_t, uint256>>;

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<const CBlockIndex*> 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<const CBlockIndex*> pindex,
Expand Down Expand Up @@ -155,6 +167,16 @@ class CQuorumBlockProcessor
const CBlockIndex* pindex,
size_t cycle) const;
std::optional<const CBlockIndex*> 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<const CBlockIndex*> pindex, size_t max_count) const;
//! GetMinedCommitmentsIndexedUntilBlock for rotation-enabled types, reporting the same way.
MinedCommitmentScan ScanMinedCommitmentsIndexedUntilBlock(Consensus::LLMQType llmqType,
gsl::not_null<const CBlockIndex*> 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);
Expand Down
30 changes: 30 additions & 0 deletions src/llmq/params.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef BITCOIN_LLMQ_PARAMS_H
#define BITCOIN_LLMQ_PARAMS_H

#include <algorithm>
#include <array>
#include <cstdint>
#include <string_view>
Expand Down Expand Up @@ -512,6 +513,35 @@ static constexpr std::array<LLMQParams, 14> 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<int>(std::max<int64_t>(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<typename T> struct is_serializable_enum;
Expand Down
Loading
Loading