Skip to content

Commit

Permalink
Less cs_main locks in quorums (#2702)
Browse files Browse the repository at this point in the history 
* Drop cs_main from CQuorumManager::UpdatedBlockTip()

* CLLMQUtils::IsQuorumActive() shouldn't require cs_main to be held

* Revert comment deletion
  • Loading branch information
@UdjinM6 @codablock
UdjinM6 authored and codablock committed Feb 15, 2019
1 parent 3bbc75f commit 3e4286a
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 113 deletions.
64 changes: 36 additions & 28 deletions src/llmq/quorums.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,27 +166,23 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockI
return;
}

LOCK(cs_main);

for (auto& p : Params().GetConsensus().llmqs) {
EnsureQuorumConnections(p.first, pindexNew);
}
}

void CQuorumManager::EnsureQuorumConnections(Consensus::LLMQType llmqType, const CBlockIndex* pindexNew)
{
AssertLockHeld(cs_main);

const auto& params = Params().GetConsensus().llmqs.at(llmqType);

auto myProTxHash = activeMasternodeInfo.proTxHash;
auto lastQuorums = ScanQuorums(llmqType, (size_t)params.keepOldConnections);
auto lastQuorums = ScanQuorums(llmqType, pindexNew, (size_t)params.keepOldConnections);

auto connmanQuorumsToDelete = g_connman->GetMasternodeQuorums(llmqType);

// don't remove connections for the currently in-progress DKG round
int curDkgHeight = pindexNew->nHeight - (pindexNew->nHeight % params.dkgInterval);
auto curDkgBlock = chainActive[curDkgHeight]->GetBlockHash();
auto curDkgBlock = pindexNew->GetAncestor(curDkgHeight)->GetBlockHash();
connmanQuorumsToDelete.erase(curDkgBlock);

for (auto& quorum : lastQuorums) {
Expand Down Expand Up @@ -222,19 +218,14 @@ void CQuorumManager::EnsureQuorumConnections(Consensus::LLMQType llmqType, const
}
}

bool CQuorumManager::BuildQuorumFromCommitment(const CFinalCommitment& qc, std::shared_ptr<CQuorum>& quorum) const
bool CQuorumManager::BuildQuorumFromCommitment(const CFinalCommitment& qc, const CBlockIndex* pindexQuorum, std::shared_ptr<CQuorum>& quorum) const
{
AssertLockHeld(cs_main);
assert(pindexQuorum);
assert(qc.quorumHash == pindexQuorum->GetBlockHash());

if (!mapBlockIndex.count(qc.quorumHash)) {
LogPrintf("CQuorumManager::%s -- block %s not found", __func__, qc.quorumHash.ToString());
return false;
}
auto& params = Params().GetConsensus().llmqs.at((Consensus::LLMQType)qc.llmqType);
auto quorumIndex = mapBlockIndex[qc.quorumHash];
auto members = CLLMQUtils::GetAllQuorumMembers((Consensus::LLMQType)qc.llmqType, qc.quorumHash);

quorum->Init(qc.quorumHash, quorumIndex->nHeight, members, qc.validMembers, qc.quorumPublicKey);
quorum->Init(qc.quorumHash, pindexQuorum->nHeight, members, qc.validMembers, qc.quorumPublicKey);

bool hasValidVvec = false;
if (quorum->ReadContributions(evoDb)) {
Expand Down Expand Up @@ -301,11 +292,15 @@ bool CQuorumManager::HasQuorum(Consensus::LLMQType llmqType, const uint256& quor

std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, size_t maxCount)
{
LOCK(cs_main);
return ScanQuorums(llmqType, chainActive.Tip()->GetBlockHash(), maxCount);
const CBlockIndex* pindex;
{
LOCK(cs_main);
pindex = chainActive.Tip();
}
return ScanQuorums(llmqType, pindex, maxCount);
}

std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const uint256& startBlock, size_t maxCount)
std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t maxCount)
{
std::vector<CQuorumCPtr> result;
result.reserve(maxCount);
Expand All @@ -318,19 +313,13 @@ std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp
return result;
}

LOCK(cs_main);
if (!mapBlockIndex.count(startBlock)) {
return result;
}

CBlockIndex* pindex = mapBlockIndex[startBlock];
pindex = chainActive[pindex->nHeight - (pindex->nHeight % params.dkgInterval)];
auto pindex = pindexStart->GetAncestor(pindexStart->nHeight - (pindexStart->nHeight % params.dkgInterval));

while (pindex != nullptr
&& pindex->nHeight >= params.dkgInterval
&& result.size() < maxCount
&& deterministicMNManager->IsDIP3Enforced(pindex->nHeight)) {
auto quorum = GetQuorum(llmqType, pindex->GetBlockHash());
auto quorum = GetQuorum(llmqType, pindex);
if (quorum) {
result.emplace_back(quorum);
}
Expand All @@ -348,7 +337,25 @@ std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp

CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash)
{
AssertLockHeld(cs_main);
CBlockIndex* pindexQuorum;
{
LOCK(cs_main);
auto quorumIt = mapBlockIndex.find(quorumHash);

if (quorumIt == mapBlockIndex.end()) {
LogPrintf("CQuorumManager::%s -- block %s not found", __func__, quorumHash.ToString());
return nullptr;
}
pindexQuorum = quorumIt->second;
}
return GetQuorum(llmqType, pindexQuorum);
}

CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindexQuorum)
{
assert(pindexQuorum);

auto quorumHash = pindexQuorum->GetBlockHash();

// we must check this before we look into the cache. Reorgs might have happened which would mean we might have
// cached quorums which are not in the active chain anymore
Expand All @@ -371,7 +378,8 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint25
auto& params = Params().GetConsensus().llmqs.at(llmqType);

auto quorum = std::make_shared<CQuorum>(params, blsWorker);
if (!BuildQuorumFromCommitment(qc, quorum)) {

if (!BuildQuorumFromCommitment(qc, pindexQuorum, quorum)) {
return nullptr;
}

Expand Down
14 changes: 9 additions & 5 deletions src/llmq/quorums.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,20 +91,24 @@ class CQuorumManager

void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload);

public:
bool HasQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash);

// all these methods will lock cs_main
CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType,const uint256& quorumHash);
// all these methods will lock cs_main for a short period of time
CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash);
CQuorumCPtr GetNewestQuorum(Consensus::LLMQType llmqType);
std::vector<CQuorumCPtr> ScanQuorums(Consensus::LLMQType llmqType, size_t maxCount);
std::vector<CQuorumCPtr> ScanQuorums(Consensus::LLMQType llmqType, const uint256& startBlock, size_t maxCount);

// this one is cs_main-free
std::vector<CQuorumCPtr> ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t maxCount);

private:
// all private methods here are cs_main-free
void EnsureQuorumConnections(Consensus::LLMQType llmqType, const CBlockIndex *pindexNew);

bool BuildQuorumFromCommitment(const CFinalCommitment& qc, std::shared_ptr<CQuorum>& quorum) const;
bool BuildQuorumFromCommitment(const CFinalCommitment& qc, const CBlockIndex* pindexQuorum, std::shared_ptr<CQuorum>& quorum) const;
bool BuildQuorumContributions(const CFinalCommitment& fqc, std::shared_ptr<CQuorum>& quorum) const;

CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindex);
};

extern CQuorumManager* quorumManager;
Expand Down
79 changes: 36 additions & 43 deletions src/llmq/quorums_signing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,19 +259,15 @@ bool CSigningManager::PreVerifyRecoveredSig(NodeId nodeId, const CRecoveredSig&
return false;
}

CQuorumCPtr quorum;
{
LOCK(cs_main);
CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, recoveredSig.quorumHash);

quorum = quorumManager->GetQuorum(llmqType, recoveredSig.quorumHash);
if (!quorum) {
LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__,
recoveredSig.quorumHash.ToString(), nodeId);
return false;
}
if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) {
return false;
}
if (!quorum) {
LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__,
recoveredSig.quorumHash.ToString(), nodeId);
return false;
}
if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) {
return false;
}

if (!recoveredSig.sig.IsValid()) {
Expand Down Expand Up @@ -316,37 +312,34 @@ void CSigningManager::CollectPendingRecoveredSigsToVerify(
}
}

{
LOCK(cs_main);
for (auto& p : retSigShares) {
NodeId nodeId = p.first;
auto& v = p.second;

for (auto it = v.begin(); it != v.end();) {
auto& recSig = *it;

Consensus::LLMQType llmqType = (Consensus::LLMQType) recSig.llmqType;
auto quorumKey = std::make_pair((Consensus::LLMQType)recSig.llmqType, recSig.quorumHash);
if (!retQuorums.count(quorumKey)) {
CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, recSig.quorumHash);
if (!quorum) {
LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__,
recSig.quorumHash.ToString(), nodeId);
it = v.erase(it);
continue;
}
if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) {
LogPrintf("CSigningManager::%s -- quorum %s not active anymore, node=%d\n", __func__,
recSig.quorumHash.ToString(), nodeId);
it = v.erase(it);
continue;
}

retQuorums.emplace(quorumKey, quorum);
for (auto& p : retSigShares) {
NodeId nodeId = p.first;
auto& v = p.second;

for (auto it = v.begin(); it != v.end();) {
auto& recSig = *it;

Consensus::LLMQType llmqType = (Consensus::LLMQType) recSig.llmqType;
auto quorumKey = std::make_pair((Consensus::LLMQType)recSig.llmqType, recSig.quorumHash);
if (!retQuorums.count(quorumKey)) {
CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, recSig.quorumHash);
if (!quorum) {
LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__,
recSig.quorumHash.ToString(), nodeId);
it = v.erase(it);
continue;
}
if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) {
LogPrintf("CSigningManager::%s -- quorum %s not active anymore, node=%d\n", __func__,
recSig.quorumHash.ToString(), nodeId);
it = v.erase(it);
continue;
}

++it;
retQuorums.emplace(quorumKey, quorum);
}

++it;
}
}
}
Expand Down Expand Up @@ -561,17 +554,17 @@ CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType
auto& llmqParams = Params().GetConsensus().llmqs.at(llmqType);
size_t poolSize = (size_t)llmqParams.signingActiveQuorumCount;

uint256 startBlock;
CBlockIndex* pindexStart;
{
LOCK(cs_main);
int startBlockHeight = signHeight - SIGN_HEIGHT_OFFSET;
if (startBlockHeight > chainActive.Height()) {
return nullptr;
}
startBlock = chainActive[startBlockHeight]->GetBlockHash();
pindexStart = chainActive[startBlockHeight];
}

auto quorums = quorumManager->ScanQuorums(llmqType, startBlock, poolSize);
auto quorums = quorumManager->ScanQuorums(llmqType, pindexStart, poolSize);
if (quorums.empty()) {
return nullptr;
}
Expand Down
61 changes: 27 additions & 34 deletions src/llmq/quorums_signing_shares.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,31 +347,26 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(NodeId nodeId, const CBatchedS
return false;
}

CQuorumCPtr quorum;
{
LOCK(cs_main);

quorum = quorumManager->GetQuorum(llmqType, batchedSigShares.quorumHash);
if (!quorum) {
// TODO should we ban here?
LogPrintf("CSigSharesManager::%s -- quorum %s not found, node=%d\n", __func__,
batchedSigShares.quorumHash.ToString(), nodeId);
return false;
}
if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) {
// quorum is too old
return false;
}
if (!quorum->IsMember(activeMasternodeInfo.proTxHash)) {
// we're not a member so we can't verify it (we actually shouldn't have received it)
return false;
}
if (quorum->quorumVvec == nullptr) {
// TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG
LogPrintf("CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible. node=%d\n", __func__,
batchedSigShares.quorumHash.ToString(), nodeId);
return false;
}
CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, batchedSigShares.quorumHash);
if (!quorum) {
// TODO should we ban here?
LogPrintf("CSigSharesManager::%s -- quorum %s not found, node=%d\n", __func__,
batchedSigShares.quorumHash.ToString(), nodeId);
return false;
}
if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) {
// quorum is too old
return false;
}
if (!quorum->IsMember(activeMasternodeInfo.proTxHash)) {
// we're not a member so we can't verify it (we actually shouldn't have received it)
return false;
}
if (quorum->quorumVvec == nullptr) {
// TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG
LogPrintf("CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible. node=%d\n", __func__,
batchedSigShares.quorumHash.ToString(), nodeId);
return false;
}

std::set<uint16_t> dupMembers;
Expand Down Expand Up @@ -994,17 +989,15 @@ void CSigSharesManager::Cleanup()
}
}

{
// Find quorums which became inactive
LOCK(cs_main);
for (auto it = quorumsToCheck.begin(); it != quorumsToCheck.end(); ) {
if (CLLMQUtils::IsQuorumActive(it->first, it->second)) {
it = quorumsToCheck.erase(it);
} else {
++it;
}
// Find quorums which became inactive
for (auto it = quorumsToCheck.begin(); it != quorumsToCheck.end(); ) {
if (CLLMQUtils::IsQuorumActive(it->first, it->second)) {
it = quorumsToCheck.erase(it);
} else {
++it;
}
}

{
// Now delete sessions which are for inactive quorums
LOCK(cs);
Expand Down
2 changes: 0 additions & 2 deletions src/llmq/quorums_utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ std::set<size_t> CLLMQUtils::CalcDeterministicWatchConnections(Consensus::LLMQTy

bool CLLMQUtils::IsQuorumActive(Consensus::LLMQType llmqType, const uint256& quorumHash)
{
AssertLockHeld(cs_main);

auto& params = Params().GetConsensus().llmqs.at(llmqType);

// sig shares and recovered sigs are only accepted from recent/active quorums
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/rpcquorums.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ UniValue quorum_list(const JSONRPCRequest& request)
for (auto& p : Params().GetConsensus().llmqs) {
UniValue v(UniValue::VARR);

auto quorums = llmq::quorumManager->ScanQuorums(p.first, chainActive.Tip()->GetBlockHash(), count);
auto quorums = llmq::quorumManager->ScanQuorums(p.first, chainActive.Tip(), count);
for (auto& q : quorums) {
v.push_back(q->quorumHash.ToString());
}
Expand Down

0 comments on commit 3e4286a

Please sign in to comment.