refactor: mark stuff as [[nodiscard]] and use Mutex (#4996)

* refactor: mark some functions as [[nodiscard]] in CDeterministicMN/CDeterministicMNList

* refactor: use a Mutex instead of CCriticalSection
This commit is contained in:
PastaPastaPasta 2022-09-03 06:20:06 -04:00 committed by GitHub
parent b30005d246
commit f72ebeec8c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 45 additions and 43 deletions

View File

@ -171,7 +171,7 @@ auto CachedGetQcHashesQcIndexedHashes(const CBlockIndex* pindexPrev) ->
std::optional<std::pair<QcHashMap /*qcHashes*/, QcIndexedHashMap /*qcIndexedHashes*/>> { std::optional<std::pair<QcHashMap /*qcHashes*/, QcIndexedHashMap /*qcIndexedHashes*/>> {
auto quorums = llmq::quorumBlockProcessor->GetMinedAndActiveCommitmentsUntilBlock(pindexPrev); auto quorums = llmq::quorumBlockProcessor->GetMinedAndActiveCommitmentsUntilBlock(pindexPrev);
static CCriticalSection cs_cache; static Mutex cs_cache;
static std::map<Consensus::LLMQType, std::vector<const CBlockIndex*>> quorums_cached GUARDED_BY(cs_cache); static std::map<Consensus::LLMQType, std::vector<const CBlockIndex*>> quorums_cached GUARDED_BY(cs_cache);
static QcHashMap qcHashes_cached GUARDED_BY(cs_cache); static QcHashMap qcHashes_cached GUARDED_BY(cs_cache);
static QcIndexedHashMap qcIndexedHashes_cached GUARDED_BY(cs_cache); static QcIndexedHashMap qcIndexedHashes_cached GUARDED_BY(cs_cache);

View File

@ -88,9 +88,9 @@ public:
SerializationOp(s, CSerActionUnserialize(), oldFormat); SerializationOp(s, CSerActionUnserialize(), oldFormat);
} }
uint64_t GetInternalId() const; [[nodiscard]] uint64_t GetInternalId() const;
std::string ToString() const; [[nodiscard]] std::string ToString() const;
void ToJson(UniValue& obj) const; void ToJson(UniValue& obj) const;
}; };
using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>; using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
@ -198,12 +198,12 @@ public:
} }
} }
size_t GetAllMNsCount() const [[nodiscard]] size_t GetAllMNsCount() const
{ {
return mnMap.size(); return mnMap.size();
} }
size_t GetValidMNsCount() const [[nodiscard]] size_t GetValidMNsCount() const
{ {
return ranges::count_if(mnMap, [](const auto& p){ return IsMNValid(*p.second); }); return ranges::count_if(mnMap, [](const auto& p){ return IsMNValid(*p.second); });
} }
@ -241,7 +241,7 @@ public:
} }
} }
const uint256& GetBlockHash() const [[nodiscard]] const uint256& GetBlockHash() const
{ {
return blockHash; return blockHash;
} }
@ -249,7 +249,7 @@ public:
{ {
blockHash = _blockHash; blockHash = _blockHash;
} }
int GetHeight() const [[nodiscard]] int GetHeight() const
{ {
return nHeight; return nHeight;
} }
@ -257,36 +257,36 @@ public:
{ {
nHeight = _height; nHeight = _height;
} }
uint32_t GetTotalRegisteredCount() const [[nodiscard]] uint32_t GetTotalRegisteredCount() const
{ {
return nTotalRegisteredCount; return nTotalRegisteredCount;
} }
bool IsMNValid(const uint256& proTxHash) const; [[nodiscard]] bool IsMNValid(const uint256& proTxHash) const;
bool IsMNPoSeBanned(const uint256& proTxHash) const; [[nodiscard]] bool IsMNPoSeBanned(const uint256& proTxHash) const;
static bool IsMNValid(const CDeterministicMN& dmn); static bool IsMNValid(const CDeterministicMN& dmn);
static bool IsMNPoSeBanned(const CDeterministicMN& dmn); static bool IsMNPoSeBanned(const CDeterministicMN& dmn);
bool HasMN(const uint256& proTxHash) const [[nodiscard]] bool HasMN(const uint256& proTxHash) const
{ {
return GetMN(proTxHash) != nullptr; return GetMN(proTxHash) != nullptr;
} }
bool HasMNByCollateral(const COutPoint& collateralOutpoint) const [[nodiscard]] bool HasMNByCollateral(const COutPoint& collateralOutpoint) const
{ {
return GetMNByCollateral(collateralOutpoint) != nullptr; return GetMNByCollateral(collateralOutpoint) != nullptr;
} }
bool HasValidMNByCollateral(const COutPoint& collateralOutpoint) const [[nodiscard]] bool HasValidMNByCollateral(const COutPoint& collateralOutpoint) const
{ {
return GetValidMNByCollateral(collateralOutpoint) != nullptr; return GetValidMNByCollateral(collateralOutpoint) != nullptr;
} }
CDeterministicMNCPtr GetMN(const uint256& proTxHash) const; [[nodiscard]] CDeterministicMNCPtr GetMN(const uint256& proTxHash) const;
CDeterministicMNCPtr GetValidMN(const uint256& proTxHash) const; [[nodiscard]] CDeterministicMNCPtr GetValidMN(const uint256& proTxHash) const;
CDeterministicMNCPtr GetMNByOperatorKey(const CBLSPublicKey& pubKey) const; [[nodiscard]] CDeterministicMNCPtr GetMNByOperatorKey(const CBLSPublicKey& pubKey) const;
CDeterministicMNCPtr GetMNByCollateral(const COutPoint& collateralOutpoint) const; [[nodiscard]] CDeterministicMNCPtr GetMNByCollateral(const COutPoint& collateralOutpoint) const;
CDeterministicMNCPtr GetValidMNByCollateral(const COutPoint& collateralOutpoint) const; [[nodiscard]] CDeterministicMNCPtr GetValidMNByCollateral(const COutPoint& collateralOutpoint) const;
CDeterministicMNCPtr GetMNByService(const CService& service) const; [[nodiscard]] CDeterministicMNCPtr GetMNByService(const CService& service) const;
CDeterministicMNCPtr GetMNByInternalId(uint64_t internalId) const; [[nodiscard]] CDeterministicMNCPtr GetMNByInternalId(uint64_t internalId) const;
CDeterministicMNCPtr GetMNPayee() const; [[nodiscard]] CDeterministicMNCPtr GetMNPayee() const;
/** /**
* Calculates the projected MN payees for the next *count* blocks. The result is not guaranteed to be correct * Calculates the projected MN payees for the next *count* blocks. The result is not guaranteed to be correct
@ -294,7 +294,7 @@ public:
* @param count * @param count
* @return * @return
*/ */
std::vector<CDeterministicMNCPtr> GetProjectedMNPayees(int nCount) const; [[nodiscard]] std::vector<CDeterministicMNCPtr> GetProjectedMNPayees(int nCount) const;
/** /**
* Calculate a quorum based on the modifier. The resulting list is deterministically sorted by score * Calculate a quorum based on the modifier. The resulting list is deterministically sorted by score
@ -302,15 +302,15 @@ public:
* @param modifier * @param modifier
* @return * @return
*/ */
std::vector<CDeterministicMNCPtr> CalculateQuorum(size_t maxSize, const uint256& modifier) const; [[nodiscard]] std::vector<CDeterministicMNCPtr> CalculateQuorum(size_t maxSize, const uint256& modifier) const;
std::vector<std::pair<arith_uint256, CDeterministicMNCPtr>> CalculateScores(const uint256& modifier) const; [[nodiscard]] std::vector<std::pair<arith_uint256, CDeterministicMNCPtr>> CalculateScores(const uint256& modifier) const;
/** /**
* Calculates the maximum penalty which is allowed at the height of this MN list. It is dynamic and might change * Calculates the maximum penalty which is allowed at the height of this MN list. It is dynamic and might change
* for every block. * for every block.
* @return * @return
*/ */
int CalcMaxPoSePenalty() const; [[nodiscard]] int CalcMaxPoSePenalty() const;
/** /**
* Returns a the given percentage from the max penalty for this MN list. Always use this method to calculate the * Returns a the given percentage from the max penalty for this MN list. Always use this method to calculate the
@ -320,7 +320,7 @@ public:
* @param percent * @param percent
* @return * @return
*/ */
int CalcPenalty(int percent) const; [[nodiscard]] int CalcPenalty(int percent) const;
/** /**
* Punishes a MN for misbehavior. If the resulting penalty score of the MN reaches the max penalty, it is banned. * Punishes a MN for misbehavior. If the resulting penalty score of the MN reaches the max penalty, it is banned.
@ -338,9 +338,9 @@ public:
*/ */
void PoSeDecrease(const uint256& proTxHash); void PoSeDecrease(const uint256& proTxHash);
CDeterministicMNListDiff BuildDiff(const CDeterministicMNList& to) const; [[nodiscard]] CDeterministicMNListDiff BuildDiff(const CDeterministicMNList& to) const;
CSimplifiedMNListDiff BuildSimplifiedDiff(const CDeterministicMNList& to, bool extended) const; [[nodiscard]] CSimplifiedMNListDiff BuildSimplifiedDiff(const CDeterministicMNList& to, bool extended) const;
CDeterministicMNList ApplyDiff(const CBlockIndex* pindex, const CDeterministicMNListDiff& diff) const; [[nodiscard]] CDeterministicMNList ApplyDiff(const CBlockIndex* pindex, const CDeterministicMNListDiff& diff) const;
void AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTotalCount = true); void AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTotalCount = true);
void UpdateMN(const CDeterministicMN& oldDmn, const std::shared_ptr<const CDeterministicMNState>& pdmnState); void UpdateMN(const CDeterministicMN& oldDmn, const std::shared_ptr<const CDeterministicMNState>& pdmnState);
@ -349,12 +349,12 @@ public:
void RemoveMN(const uint256& proTxHash); void RemoveMN(const uint256& proTxHash);
template <typename T> template <typename T>
bool HasUniqueProperty(const T& v) const [[nodiscard]] bool HasUniqueProperty(const T& v) const
{ {
return mnUniquePropertyMap.count(::SerializeHash(v)) != 0; return mnUniquePropertyMap.count(::SerializeHash(v)) != 0;
} }
template <typename T> template <typename T>
CDeterministicMNCPtr GetUniquePropertyMN(const T& v) const [[nodiscard]] CDeterministicMNCPtr GetUniquePropertyMN(const T& v) const
{ {
auto p = mnUniquePropertyMap.find(::SerializeHash(v)); auto p = mnUniquePropertyMap.find(::SerializeHash(v));
if (!p) { if (!p) {

View File

@ -32,7 +32,7 @@ public:
class CEvoDB class CEvoDB
{ {
public: public:
CCriticalSection cs; Mutex cs;
private: private:
CDBWrapper db; CDBWrapper db;
@ -46,41 +46,41 @@ private:
public: public:
explicit CEvoDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false); explicit CEvoDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false);
std::unique_ptr<CEvoDBScopedCommitter> BeginTransaction() std::unique_ptr<CEvoDBScopedCommitter> BeginTransaction() LOCKS_EXCLUDED(cs)
{ {
LOCK(cs); LOCK(cs);
return std::make_unique<CEvoDBScopedCommitter>(*this); return std::make_unique<CEvoDBScopedCommitter>(*this);
} }
CurTransaction& GetCurTransaction() CurTransaction& GetCurTransaction() EXCLUSIVE_LOCKS_REQUIRED(cs)
{ {
AssertLockHeld(cs); // lock must be held from outside as long as the DB transaction is used AssertLockHeld(cs); // lock must be held from outside as long as the DB transaction is used
return curDBTransaction; return curDBTransaction;
} }
template <typename K, typename V> template <typename K, typename V>
bool Read(const K& key, V& value) bool Read(const K& key, V& value) LOCKS_EXCLUDED(cs)
{ {
LOCK(cs); LOCK(cs);
return curDBTransaction.Read(key, value); return curDBTransaction.Read(key, value);
} }
template <typename K, typename V> template <typename K, typename V>
void Write(const K& key, const V& value) void Write(const K& key, const V& value) LOCKS_EXCLUDED(cs)
{ {
LOCK(cs); LOCK(cs);
curDBTransaction.Write(key, value); curDBTransaction.Write(key, value);
} }
template <typename K> template <typename K>
bool Exists(const K& key) bool Exists(const K& key) LOCKS_EXCLUDED(cs)
{ {
LOCK(cs); LOCK(cs);
return curDBTransaction.Exists(key); return curDBTransaction.Exists(key);
} }
template <typename K> template <typename K>
void Erase(const K& key) void Erase(const K& key) LOCKS_EXCLUDED(cs)
{ {
LOCK(cs); LOCK(cs);
curDBTransaction.Erase(key); curDBTransaction.Erase(key);
@ -91,12 +91,12 @@ public:
return db; return db;
} }
size_t GetMemoryUsage() const [[nodiscard]] size_t GetMemoryUsage() const
{ {
return rootDBTransaction.GetMemoryUsage(); return rootDBTransaction.GetMemoryUsage();
} }
bool CommitRootTransaction(); bool CommitRootTransaction() LOCKS_EXCLUDED(cs);
bool IsEmpty() { return db.IsEmpty(); } bool IsEmpty() { return db.IsEmpty(); }
@ -106,8 +106,8 @@ public:
private: private:
// only CEvoDBScopedCommitter is allowed to invoke these // only CEvoDBScopedCommitter is allowed to invoke these
friend class CEvoDBScopedCommitter; friend class CEvoDBScopedCommitter;
void CommitCurTransaction(); void CommitCurTransaction() LOCKS_EXCLUDED(cs);
void RollbackCurTransaction(); void RollbackCurTransaction() LOCKS_EXCLUDED(cs);
}; };
extern std::unique_ptr<CEvoDB> evoDb; extern std::unique_ptr<CEvoDB> evoDb;

View File

@ -502,6 +502,7 @@ CFinalCommitmentPtr CQuorumBlockProcessor::GetMinedCommitment(Consensus::LLMQTyp
// The returned quorums are in reversed order, so the most recent one is at index 0 // The returned quorums are in reversed order, so the most recent one is at index 0
std::vector<const CBlockIndex*> CQuorumBlockProcessor::GetMinedCommitmentsUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, size_t maxCount) const std::vector<const CBlockIndex*> CQuorumBlockProcessor::GetMinedCommitmentsUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, size_t maxCount) const
{ {
AssertLockNotHeld(evoDb.cs);
LOCK(evoDb.cs); LOCK(evoDb.cs);
auto dbIt = evoDb.GetCurTransaction().NewIteratorUniquePtr(); auto dbIt = evoDb.GetCurTransaction().NewIteratorUniquePtr();
@ -545,6 +546,7 @@ std::vector<const CBlockIndex*> CQuorumBlockProcessor::GetMinedCommitmentsUntilB
std::optional<const CBlockIndex*> CQuorumBlockProcessor::GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const std::optional<const CBlockIndex*> CQuorumBlockProcessor::GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const
{ {
AssertLockNotHeld(evoDb.cs);
LOCK(evoDb.cs); LOCK(evoDb.cs);
auto dbIt = evoDb.GetCurTransaction().NewIteratorUniquePtr(); auto dbIt = evoDb.GetCurTransaction().NewIteratorUniquePtr();

View File

@ -377,7 +377,6 @@ std::optional<CQuorumSnapshot> CQuorumSnapshotManager::GetSnapshotForBlock(const
if (quorumSnapshotCache.get(snapshotHash, snapshot)) { if (quorumSnapshotCache.get(snapshotHash, snapshot)) {
return snapshot; return snapshot;
} }
LOCK(evoDb.cs);
if (evoDb.Read(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot)) { if (evoDb.Read(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot)) {
quorumSnapshotCache.insert(snapshotHash, snapshot); quorumSnapshotCache.insert(snapshotHash, snapshot);
return snapshot; return snapshot;
@ -391,6 +390,7 @@ void CQuorumSnapshotManager::StoreSnapshotForBlock(const Consensus::LLMQType llm
auto snapshotHash = ::SerializeHash(std::make_pair(llmqType, pindex->GetBlockHash())); auto snapshotHash = ::SerializeHash(std::make_pair(llmqType, pindex->GetBlockHash()));
// LOCK(cs_main); // LOCK(cs_main);
AssertLockNotHeld(evoDb.cs);
LOCK2(snapshotCacheCs, evoDb.cs); LOCK2(snapshotCacheCs, evoDb.cs);
evoDb.GetRawDB().Write(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot); evoDb.GetRawDB().Write(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot);
quorumSnapshotCache.insert(snapshotHash, snapshot); quorumSnapshotCache.insert(snapshotHash, snapshot);