Skip to content

refactor/feat: Refactor and add safety belts in llmq utils #5378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UdjinM6 merged 2 commits into from
May 20, 2023
Merged

refactor/feat: Refactor and add safety belts in llmq utils #5378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2225fa9
refactor: pQuorumBaseBlockIndex -> pCycleQuorumBaseBlockIndex/pindex …
UdjinM6 May 16, 2023
30e0330
feat: make sure llmq util functions are called for correct types and …
UdjinM6 May 17, 2023
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
84 changes: 54 additions & 30 deletions src/llmq/utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,29 @@ namespace utils
static std::vector<CDeterministicMNCPtr> ComputeQuorumMembers(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex);
static std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRotation(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pCycleQuorumBaseBlockIndex);

static std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pQuorumBaseBlockIndex, const PreviousQuorumQuarters& quarters);
static std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pCycleQuorumBaseBlockIndex, const PreviousQuorumQuarters& quarters);

static PreviousQuorumQuarters GetPreviousQuorumQuarterMembers(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pBlockHMinusCIndex, const CBlockIndex* pBlockHMinus2CIndex, const CBlockIndex* pBlockHMinus3CIndex, int nHeight);
static std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pQuorumBaseBlockIndex, const llmq::CQuorumSnapshot& snapshot, int nHeights);
static std::pair<CDeterministicMNList, CDeterministicMNList> GetMNUsageBySnapshot(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pQuorumBaseBlockIndex, const llmq::CQuorumSnapshot& snapshot, int nHeight);
static std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pCycleQuorumBaseBlockIndex, const llmq::CQuorumSnapshot& snapshot, int nHeights);
static std::pair<CDeterministicMNList, CDeterministicMNList> GetMNUsageBySnapshot(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pCycleQuorumBaseBlockIndex, const llmq::CQuorumSnapshot& snapshot, int nHeight);

static void BuildQuorumSnapshot(const Consensus::LLMQParams& llmqParams, const CDeterministicMNList& allMns, const CDeterministicMNList& mnUsedAtH, std::vector<CDeterministicMNCPtr>& sortedCombinedMns, CQuorumSnapshot& quorumSnapshot, int nHeight, std::vector<int>& skipList, const CBlockIndex* pQuorumBaseBlockIndex);
static void BuildQuorumSnapshot(const Consensus::LLMQParams& llmqParams, const CDeterministicMNList& allMns, const CDeterministicMNList& mnUsedAtH, std::vector<CDeterministicMNCPtr>& sortedCombinedMns, CQuorumSnapshot& quorumSnapshot, int nHeight, std::vector<int>& skipList, const CBlockIndex* pCycleQuorumBaseBlockIndex);

static bool IsInstantSendLLMQTypeShared();

void PreComputeQuorumMembers(const CBlockIndex* pQuorumBaseBlockIndex, bool reset_cache)
void PreComputeQuorumMembers(const CBlockIndex* pindex, bool reset_cache)
{
for (const Consensus::LLMQParams& params : GetEnabledQuorumParams(pQuorumBaseBlockIndex->pprev)) {
if (IsQuorumRotationEnabled(params, pQuorumBaseBlockIndex) && (pQuorumBaseBlockIndex->nHeight % params.dkgInterval == 0)) {
GetAllQuorumMembers(params.type, pQuorumBaseBlockIndex, reset_cache);
for (const Consensus::LLMQParams& params : GetEnabledQuorumParams(pindex->pprev)) {
if (IsQuorumRotationEnabled(params, pindex) && (pindex->nHeight % params.dkgInterval == 0)) {
GetAllQuorumMembers(params.type, pindex, reset_cache);
}
}
}

uint256 GetHashModifier(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pQuorumBaseBlockIndex)
uint256 GetHashModifier(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pCycleQuorumBaseBlockIndex)
{
const CBlockIndex* pWorkBlockIndex = pQuorumBaseBlockIndex->GetAncestor(pQuorumBaseBlockIndex->nHeight - 8);
ASSERT_IF_DEBUG(pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval == 0);
Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta May 18, 2023

Choose a reason for hiding this comment

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

I kinda wonder if we should create an "ASSUME" macro or something similar which if debug is enabled with act as an assert, but if not debug would print to log if the assumption was false? Idk, probably not the most useful

const CBlockIndex* pWorkBlockIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 8);

if (IsV20Active(pWorkBlockIndex)) {
// v20 is active: calculate modifier using the new way.
Expand All @@ -83,7 +84,7 @@ uint256 GetHashModifier(const Consensus::LLMQParams& llmqParams, const CBlockInd
if (llmqParams.useRotation) {
return ::SerializeHash(std::make_pair(llmqParams.type, pWorkBlockIndex->GetBlockHash()));
}
return ::SerializeHash(std::make_pair(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash()));
return ::SerializeHash(std::make_pair(llmqParams.type, pCycleQuorumBaseBlockIndex->GetBlockHash()));
}

std::vector<CDeterministicMNCPtr> GetAllQuorumMembers(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, bool reset_cache)
Expand Down Expand Up @@ -166,7 +167,7 @@ std::vector<CDeterministicMNCPtr> ComputeQuorumMembers(Consensus::LLMQType llmqT
bool HPMNOnly = (Params().GetConsensus().llmqTypePlatform == llmqType) && IsV19Active(pQuorumBaseBlockIndex);
const auto& llmq_params_opt = GetLLMQParams(llmqType);
assert(llmq_params_opt.has_value());
if (llmq_params_opt->useRotation) {
if (llmq_params_opt->useRotation || pQuorumBaseBlockIndex->nHeight % llmq_params_opt->dkgInterval != 0) {
ASSERT_IF_DEBUG(false);
return {};
}
Expand All @@ -184,7 +185,10 @@ std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRota
const Consensus::LLMQType llmqType = llmqParams.type;

const int cycleLength = llmqParams.dkgInterval;
assert(pCycleQuorumBaseBlockIndex->nHeight % cycleLength == 0);
if (!llmqParams.useRotation || pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval != 0) {
ASSERT_IF_DEBUG(false);
return {};
}

const CBlockIndex* pBlockHMinusCIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - cycleLength);
const CBlockIndex* pBlockHMinus2CIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 2 * cycleLength);
Expand Down Expand Up @@ -291,16 +295,21 @@ PreviousQuorumQuarters GetPreviousQuorumQuarterMembers(const Consensus::LLMQPara
}

std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(const Consensus::LLMQParams& llmqParams,
const CBlockIndex* pQuorumBaseBlockIndex,
const CBlockIndex* pCycleQuorumBaseBlockIndex,
const PreviousQuorumQuarters& previousQuarters)
{
if (!llmqParams.useRotation || pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval != 0) {
ASSERT_IF_DEBUG(false);
return {};
}

size_t nQuorums = static_cast<size_t>(llmqParams.signingActiveQuorumCount);
std::vector<std::vector<CDeterministicMNCPtr>> quarterQuorumMembers{nQuorums};

size_t quorumSize = static_cast<size_t>(llmqParams.size);
auto quarterSize{quorumSize / 4};
const CBlockIndex* pWorkBlockIndex = pQuorumBaseBlockIndex->GetAncestor(pQuorumBaseBlockIndex->nHeight - 8);
const auto modifier = GetHashModifier(llmqParams, pQuorumBaseBlockIndex);
const CBlockIndex* pWorkBlockIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 8);
const auto modifier = GetHashModifier(llmqParams, pCycleQuorumBaseBlockIndex);

auto allMns = deterministicMNManager->GetListForBlock(pWorkBlockIndex);

Expand All @@ -312,7 +321,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(cons
auto MnsNotUsedAtH = CDeterministicMNList();
std::vector<CDeterministicMNList> MnsUsedAtHIndexed{nQuorums};

bool skipRemovedMNs = IsV19Active(pQuorumBaseBlockIndex) || (Params().NetworkIDString() == CBaseChainParams::TESTNET);
bool skipRemovedMNs = IsV19Active(pCycleQuorumBaseBlockIndex) || (Params().NetworkIDString() == CBaseChainParams::TESTNET);

for (const size_t i : irange::range(nQuorums)) {
for (const auto& mn : previousQuarters.quarterHMinusC[i]) {
Expand Down Expand Up @@ -391,7 +400,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(cons
}
ss << "]";
LogPrint(BCLog::LLMQ, "BuildNewQuorumQuarterMembers h[%d] sortedCombinedMns[%s]\n",
pQuorumBaseBlockIndex->nHeight, ss.str());
pCycleQuorumBaseBlockIndex->nHeight, ss.str());
}

std::vector<int> skipList;
Expand Down Expand Up @@ -438,22 +447,27 @@ std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(cons

CQuorumSnapshot quorumSnapshot = {};

BuildQuorumSnapshot(llmqParams, allMns, MnsUsedAtH, sortedCombinedMnsList, quorumSnapshot, pQuorumBaseBlockIndex->nHeight, skipList, pQuorumBaseBlockIndex);
BuildQuorumSnapshot(llmqParams, allMns, MnsUsedAtH, sortedCombinedMnsList, quorumSnapshot, pCycleQuorumBaseBlockIndex->nHeight, skipList, pCycleQuorumBaseBlockIndex);

quorumSnapshotManager->StoreSnapshotForBlock(llmqParams.type, pQuorumBaseBlockIndex, quorumSnapshot);
quorumSnapshotManager->StoreSnapshotForBlock(llmqParams.type, pCycleQuorumBaseBlockIndex, quorumSnapshot);

return quarterQuorumMembers;
}

void BuildQuorumSnapshot(const Consensus::LLMQParams& llmqParams, const CDeterministicMNList& allMns,
const CDeterministicMNList& mnUsedAtH, std::vector<CDeterministicMNCPtr>& sortedCombinedMns,
CQuorumSnapshot& quorumSnapshot, int nHeight, std::vector<int>& skipList, const CBlockIndex* pQuorumBaseBlockIndex)
CQuorumSnapshot& quorumSnapshot, int nHeight, std::vector<int>& skipList, const CBlockIndex* pCycleQuorumBaseBlockIndex)
{
if (!llmqParams.useRotation || pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval != 0) {
ASSERT_IF_DEBUG(false);
return;
}

quorumSnapshot.activeQuorumMembers.resize(allMns.GetAllMNsCount());
const auto modifier = GetHashModifier(llmqParams, pQuorumBaseBlockIndex);
const auto modifier = GetHashModifier(llmqParams, pCycleQuorumBaseBlockIndex);
auto sortedAllMns = allMns.CalculateQuorum(allMns.GetAllMNsCount(), modifier);

LogPrint(BCLog::LLMQ, "BuildQuorumSnapshot h[%d] numMns[%d]\n", pQuorumBaseBlockIndex->nHeight, allMns.GetAllMNsCount());
LogPrint(BCLog::LLMQ, "BuildQuorumSnapshot h[%d] numMns[%d]\n", pCycleQuorumBaseBlockIndex->nHeight, allMns.GetAllMNsCount());

std::fill(quorumSnapshot.activeQuorumMembers.begin(),
quorumSnapshot.activeQuorumMembers.end(),
Expand All @@ -476,14 +490,19 @@ void BuildQuorumSnapshot(const Consensus::LLMQParams& llmqParams, const CDetermi
}

std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot(const Consensus::LLMQParams& llmqParams,
const CBlockIndex* pQuorumBaseBlockIndex,
const CBlockIndex* pCycleQuorumBaseBlockIndex,
const llmq::CQuorumSnapshot& snapshot,
int nHeight)
{
if (!llmqParams.useRotation || pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval != 0) {
ASSERT_IF_DEBUG(false);
return {};
}

std::vector<CDeterministicMNCPtr> sortedCombinedMns;
{
const auto modifier = GetHashModifier(llmqParams, pQuorumBaseBlockIndex);
const auto [MnsUsedAtH, MnsNotUsedAtH] = GetMNUsageBySnapshot(llmqParams, pQuorumBaseBlockIndex, snapshot, nHeight);
const auto modifier = GetHashModifier(llmqParams, pCycleQuorumBaseBlockIndex);
const auto [MnsUsedAtH, MnsNotUsedAtH] = GetMNUsageBySnapshot(llmqParams, pCycleQuorumBaseBlockIndex, snapshot, nHeight);
// the list begins with all the unused MNs
auto sortedMnsNotUsedAtH = MnsNotUsedAtH.CalculateQuorum(MnsNotUsedAtH.GetAllMNsCount(), modifier);
sortedCombinedMns = std::move(sortedMnsNotUsedAtH);
Expand All @@ -500,7 +519,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot
}
ss << "]";
LogPrint(BCLog::LLMQ, "GetQuorumQuarterMembersBySnapshot h[%d] from[%d] sortedCombinedMns[%s]\n",
pQuorumBaseBlockIndex->nHeight, nHeight, ss.str());
pCycleQuorumBaseBlockIndex->nHeight, nHeight, ss.str());
}

size_t numQuorums = static_cast<size_t>(llmqParams.signingActiveQuorumCount);
Expand Down Expand Up @@ -566,15 +585,20 @@ std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot
}

std::pair<CDeterministicMNList, CDeterministicMNList> GetMNUsageBySnapshot(const Consensus::LLMQParams& llmqParams,
const CBlockIndex* pQuorumBaseBlockIndex,
const CBlockIndex* pCycleQuorumBaseBlockIndex,
const llmq::CQuorumSnapshot& snapshot,
int nHeight)
{
if (!llmqParams.useRotation || pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval != 0) {
ASSERT_IF_DEBUG(false);
return {};
}

CDeterministicMNList usedMNs;
CDeterministicMNList nonUsedMNs;

const CBlockIndex* pWorkBlockIndex = pQuorumBaseBlockIndex->GetAncestor(pQuorumBaseBlockIndex->nHeight - 8);
const auto modifier = GetHashModifier(llmqParams, pQuorumBaseBlockIndex);
const CBlockIndex* pWorkBlockIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 8);
const auto modifier = GetHashModifier(llmqParams, pCycleQuorumBaseBlockIndex);

auto allMns = deterministicMNManager->GetListForBlock(pWorkBlockIndex);
auto sortedAllMns = allMns.CalculateQuorum(allMns.GetAllMNsCount(), modifier);
Expand Down
4 changes: 2 additions & 2 deletions src/llmq/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ namespace utils
// includes members which failed DKG
std::vector<CDeterministicMNCPtr> GetAllQuorumMembers(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, bool reset_cache = false);

void PreComputeQuorumMembers(const CBlockIndex* pQuorumBaseBlockIndex, bool reset_cache = false);
uint256 GetHashModifier(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pQuorumBaseBlockIndex);
void PreComputeQuorumMembers(const CBlockIndex* pindex, bool reset_cache = false);
uint256 GetHashModifier(const Consensus::LLMQParams& llmqParams, const CBlockIndex* pCycleQuorumBaseBlockIndex);
Copy link
Copy Markdown

@ogabrielides ogabrielides May 18, 2023

Choose a reason for hiding this comment

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

My only "objection" is here. GetHashModifier is called for both non-rotated and rotated quorums, hence "cycle" is more adapted to rotation.

Copy link
Copy Markdown
Author

@UdjinM6 UdjinM6 May 18, 2023

Choose a reason for hiding this comment

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

for non-rotation quorums pCycleQuorumBaseBlockIndex == pQuorumBaseBlockIndex and since using pQuorumBaseBlockIndex for rotation ones is wrong using pCycleQuorumBaseBlockIndex for both is a good compromise imo

uint256 BuildCommitmentHash(Consensus::LLMQType llmqType, const uint256& blockHash, const std::vector<bool>& validMembers, const CBLSPublicKey& pubKey, const uint256& vvecHash);
uint256 BuildSignHash(Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, const uint256& msgHash);

Expand Down