From a197cb7bcf84b049650350727bc59389ec403293 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:54:55 +0000 Subject: [PATCH] refactor: make `Get*MN*` and friends return `std::optional` --- src/coinjoin/client.cpp | 14 ++- src/coinjoin/server.cpp | 14 ++- src/evo/deterministicmns.cpp | 146 +++++++++++++----------- src/evo/deterministicmns.h | 26 ++--- src/evo/mnauth.cpp | 11 +- src/evo/simplifiedmns.cpp | 10 +- src/governance/governance.cpp | 17 +-- src/governance/object.cpp | 14 ++- src/governance/vote.cpp | 9 +- src/llmq/utils.cpp | 12 +- src/masternode/node.cpp | 14 ++- src/masternode/payments.cpp | 8 +- src/net.cpp | 34 +++--- src/net_processing.cpp | 17 +-- src/qt/masternodelist.cpp | 2 +- src/qt/rpcconsole.cpp | 6 +- src/rpc/evo.cpp | 34 +++--- src/rpc/governance.cpp | 11 +- src/rpc/masternode.cpp | 15 +-- src/rpc/quorums.cpp | 5 +- src/test/evo_deterministicmns_tests.cpp | 38 +++--- src/txmempool.cpp | 26 +++-- 22 files changed, 267 insertions(+), 216 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 9804b1be5bee9..0c7ac0a0e964d 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -59,8 +59,8 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS const auto tip_mn_list = m_dmnman.GetListAtChainTip(); if (dsq.masternodeOutpoint.IsNull()) { - if (auto dmn = tip_mn_list.GetValidMN(dsq.m_protxHash)) { - dsq.masternodeOutpoint = dmn->collateralOutpoint; + if (auto dmn_opt = tip_mn_list.GetValidMN(dsq.m_protxHash); dmn_opt.has_value()) { + dsq.masternodeOutpoint = dmn_opt.value()->collateralOutpoint; } else { return tl::unexpected{10}; } @@ -90,9 +90,10 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataS if (dsq.IsTimeOutOfBounds()) return {}; - auto dmn = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); - if (!dmn) return {}; + auto dmn_opt = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); + if (!dmn_opt.has_value()) return {}; + auto dmn = dmn_opt.value(); if (dsq.m_protxHash.IsNull()) { dsq.m_protxHash = dmn->proTxHash; } @@ -1085,14 +1086,15 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, // Look through the queues and see if anything matches CCoinJoinQueue dsq; while (m_queueman->GetQueueItemAndTry(dsq)) { - auto dmn = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint); + auto dmn_opt = mnList.GetValidMNByCollateral(dsq.masternodeOutpoint); - if (!dmn) { + if (!dmn_opt) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- dsq masternode is not in masternode list, masternode=%s\n", dsq.masternodeOutpoint.ToStringShort()); continue; } // skip next mn payments winners + auto dmn = dmn_opt.value(); if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) { WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- skipping winner, masternode=%s\n", dmn->proTxHash.ToString()); continue; diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index bb65990dd9006..96187fa40927e 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -60,12 +60,13 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s) txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */ auto mnList = m_dmnman.GetListAtChainTip(); - auto dmn = mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint()); - if (!dmn) { + auto dmn_opt = mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint()); + if (!dmn_opt.has_value()) { PushStatus(peer, STATUS_REJECTED, ERR_MN_LIST); return; } + auto dmn = dmn_opt.value(); if (vecSessionCollaterals.empty()) { { TRY_LOCK(cs_vecqueue, lockRecv); @@ -129,8 +130,8 @@ PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv const auto tip_mn_list = m_dmnman.GetListAtChainTip(); if (dsq.masternodeOutpoint.IsNull()) { - if (auto dmn = tip_mn_list.GetValidMN(dsq.m_protxHash)) { - dsq.masternodeOutpoint = dmn->collateralOutpoint; + if (auto dmn_opt = tip_mn_list.GetValidMN(dsq.m_protxHash); dmn_opt.has_value()) { + dsq.masternodeOutpoint = dmn_opt.value()->collateralOutpoint; } else { return tl::unexpected{10}; } @@ -157,9 +158,10 @@ PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv if (dsq.IsTimeOutOfBounds()) return {}; - auto dmn = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); - if (!dmn) return {}; + auto dmn_opt = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); + if (!dmn_opt.has_value()) return {}; + auto dmn = dmn_opt.value(); if (dsq.m_protxHash.IsNull()) { dsq.m_protxHash = dmn->proTxHash; } diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 96fbe4c02cf02..a2e4d9beeb958 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -92,58 +92,58 @@ bool CDeterministicMNList::IsMNPoSeBanned(const CDeterministicMN& dmn) return dmn.pdmnState->IsBanned(); } -CDeterministicMNCPtr CDeterministicMNList::GetMN(const uint256& proTxHash) const +std::optional CDeterministicMNList::GetMN(const uint256& proTxHash) const { auto p = mnMap.find(proTxHash); if (p == nullptr) { - return nullptr; + return std::nullopt; } return *p; } -CDeterministicMNCPtr CDeterministicMNList::GetValidMN(const uint256& proTxHash) const +std::optional CDeterministicMNList::GetValidMN(const uint256& proTxHash) const { auto dmn = GetMN(proTxHash); - if (dmn && !IsMNValid(*dmn)) { - return nullptr; + if (dmn.has_value() && !IsMNValid(*dmn.value())) { + return std::nullopt; } return dmn; } -CDeterministicMNCPtr CDeterministicMNList::GetMNByOperatorKey(const CBLSPublicKey& pubKey) const +std::optional CDeterministicMNList::GetMNByOperatorKey(const CBLSPublicKey& pubKey) const { const auto it = ranges::find_if(mnMap, [&pubKey](const auto& p){return p.second->pdmnState->pubKeyOperator.Get() == pubKey;}); if (it == mnMap.end()) { - return nullptr; + return std::nullopt; } return it->second; } -CDeterministicMNCPtr CDeterministicMNList::GetMNByCollateral(const COutPoint& collateralOutpoint) const +std::optional CDeterministicMNList::GetMNByCollateral(const COutPoint& collateralOutpoint) const { return GetUniquePropertyMN(collateralOutpoint); } -CDeterministicMNCPtr CDeterministicMNList::GetValidMNByCollateral(const COutPoint& collateralOutpoint) const +std::optional CDeterministicMNList::GetValidMNByCollateral(const COutPoint& collateralOutpoint) const { - auto dmn = GetMNByCollateral(collateralOutpoint); - if (dmn && !IsMNValid(*dmn)) { - return nullptr; + auto dmn_opt = GetMNByCollateral(collateralOutpoint); + if (dmn_opt.has_value() && !IsMNValid(*dmn_opt.value())) { + return std::nullopt; } - return dmn; + return dmn_opt; } -CDeterministicMNCPtr CDeterministicMNList::GetMNByService(const CService& service) const +std::optional CDeterministicMNList::GetMNByService(const CService& service) const { return GetUniquePropertyMN(service); } -CDeterministicMNCPtr CDeterministicMNList::GetMNByInternalId(uint64_t internalId) const +std::optional CDeterministicMNList::GetMNByInternalId(uint64_t internalId) const { auto proTxHash = mnInternalIdMap.find(internalId); if (!proTxHash) { - return nullptr; + return std::nullopt; } return GetMN(*proTxHash); } @@ -174,10 +174,10 @@ static bool CompareByLastPaid(const CDeterministicMN* _a, const CDeterministicMN return CompareByLastPaid(*_a, *_b); } -CDeterministicMNCPtr CDeterministicMNList::GetMNPayee(gsl::not_null pindexPrev) const +std::optional CDeterministicMNList::GetMNPayee(gsl::not_null pindexPrev) const { if (mnMap.size() == 0) { - return nullptr; + return std::nullopt; } const bool isv19Active{DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; @@ -208,6 +208,7 @@ CDeterministicMNCPtr CDeterministicMNList::GetMNPayee(gsl::not_null 0); - auto dmn = GetMN(proTxHash); - if (!dmn) { + auto dmn_opt = GetMN(proTxHash); + if (!dmn_opt.has_value()) { throw(std::runtime_error(strprintf("%s: Can't find a masternode with proTxHash=%s", __func__, proTxHash.ToString()))); } + auto dmn = dmn_opt.value(); int maxPenalty = CalcMaxPoSePenalty(); auto newState = std::make_shared(*dmn->pdmnState); @@ -394,12 +396,15 @@ CDeterministicMNListDiff CDeterministicMNList::BuildDiff(const CDeterministicMNL to.ForEachMNShared(false, [this, &diffRet](const CDeterministicMNCPtr& toPtr) { auto fromPtr = GetMN(toPtr->proTxHash); - if (fromPtr == nullptr) { + if (!fromPtr.has_value()) { diffRet.addedMNs.emplace_back(toPtr); - } else if (fromPtr != toPtr || fromPtr->pdmnState != toPtr->pdmnState) { - CDeterministicMNStateDiff stateDiff(*fromPtr->pdmnState, *toPtr->pdmnState); - if (stateDiff.fields) { - diffRet.updatedMNs.emplace(toPtr->GetInternalId(), std::move(stateDiff)); + } else { + auto fromPdmnState = fromPtr.value()->pdmnState; + if (fromPtr != toPtr || fromPdmnState != toPtr->pdmnState) { + CDeterministicMNStateDiff stateDiff(*fromPdmnState, *toPtr->pdmnState); + if (stateDiff.fields) { + diffRet.updatedMNs.emplace(toPtr->GetInternalId(), std::move(stateDiff)); + } } } }); @@ -426,21 +431,21 @@ CDeterministicMNList CDeterministicMNList::ApplyDiff(gsl::not_nullnHeight; for (const auto& id : diff.removedMns) { - auto dmn = result.GetMNByInternalId(id); - if (!dmn) { + auto dmn_opt = result.GetMNByInternalId(id); + if (!dmn_opt.has_value()) { throw std::runtime_error(strprintf("%s: can't find a removed masternode, id=%d", __func__, id)); } - result.RemoveMN(dmn->proTxHash); + result.RemoveMN(dmn_opt.value()->proTxHash); } for (const auto& dmn : diff.addedMNs) { result.AddMN(dmn); } for (const auto& p : diff.updatedMNs) { - auto dmn = result.GetMNByInternalId(p.first); - if (!dmn) { + auto dmn_opt = result.GetMNByInternalId(p.first); + if (!dmn_opt.has_value()) { throw std::runtime_error(strprintf("%s: can't find an updated masternode, id=%d", __func__, p.first)); } - result.UpdateMN(*dmn, p.second); + result.UpdateMN(*dmn_opt.value(), p.second); } return result; @@ -553,8 +558,8 @@ void CDeterministicMNList::UpdateMN(const CDeterministicMN& oldDmn, const CDeter void CDeterministicMNList::RemoveMN(const uint256& proTxHash) { - auto dmn = GetMN(proTxHash); - if (!dmn) { + auto dmn_opt = GetMN(proTxHash); + if (!dmn_opt.has_value()) { throw(std::runtime_error(strprintf("%s: Can't find a masternode with proTxHash=%s", __func__, proTxHash.ToString()))); } @@ -562,6 +567,7 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash) // Using this temporary map as a checkpoint to roll back to in case of any issues. decltype(mnUniquePropertyMap) mnUniquePropertyMapSaved = mnUniquePropertyMap; + auto dmn = dmn_opt.value(); if (!DeleteUniqueProperty(*dmn, dmn->collateralOutpoint)) { mnUniquePropertyMap = mnUniquePropertyMapSaved; throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with a collateralOutpoint=%s", __func__, @@ -709,7 +715,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no newList.SetBlockHash(uint256()); // we can't know the final block hash, so better not return a (invalid) block hash newList.SetHeight(nHeight); - auto payee = oldList.GetMNPayee(pindexPrev); + auto payee_opt = oldList.GetMNPayee(pindexPrev); // we iterate the oldList here and update the newList // this is only valid as long these have not diverged at this point, which is the case as long as we don't add @@ -772,8 +778,9 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-collateral"); } - auto replacedDmn = newList.GetMNByCollateral(dmn->collateralOutpoint); - if (replacedDmn != nullptr) { + auto replacedDmnOpt = newList.GetMNByCollateral(dmn->collateralOutpoint); + if (replacedDmnOpt.has_value()) { + auto replacedDmn = replacedDmnOpt.value(); // This might only happen with a ProRegTx that refers an external collateral // In that case the new ProRegTx will replace the old one. This means the old one is removed // and the new one is added like a completely fresh one, which is also at the bottom of the payment list @@ -817,14 +824,16 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - if (newList.HasUniqueProperty(opt_proTx->addr) && newList.GetUniquePropertyMN(opt_proTx->addr)->proTxHash != opt_proTx->proTxHash) { + if (auto uniq_opt = newList.GetUniquePropertyMN(opt_proTx->addr); uniq_opt.has_value() && uniq_opt.value()->proTxHash != opt_proTx->proTxHash) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-addr"); } - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { + auto dmn_opt = newList.GetMN(opt_proTx->proTxHash); + if (!dmn_opt.has_value()) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); } + + auto dmn = dmn_opt.value(); if (opt_proTx->nType != dmn->nType) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type-mismatch"); } @@ -862,11 +871,11 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { + auto dmn_opt = newList.GetMN(opt_proTx->proTxHash); + if (!dmn_opt.has_value()) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); } - auto newState = std::make_shared(*dmn->pdmnState); + auto newState = std::make_shared(*dmn_opt.value()->pdmnState); if (newState->pubKeyOperator != opt_proTx->pubKeyOperator) { // reset all operator related fields and put MN into PoSe-banned state in case the operator key changes newState->ResetOperatorFields(); @@ -890,11 +899,11 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { + auto dmn_opt = newList.GetMN(opt_proTx->proTxHash); + if (!dmn_opt.has_value()) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); } - auto newState = std::make_shared(*dmn->pdmnState); + auto newState = std::make_shared(*dmn_opt.value()->pdmnState); newState->ResetOperatorFields(); newState->BanIfNotBanned(nHeight); newState->nRevocationReason = opt_proTx->nReason; @@ -934,8 +943,9 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no // check if any existing MN collateral is spent by this transaction for (const auto& in : tx.vin) { - auto dmn = newList.GetMNByCollateral(in.prevout); - if (dmn && dmn->collateralOutpoint == in.prevout) { + auto dmn_opt = newList.GetMNByCollateral(in.prevout); + if (dmn_opt.has_value() && dmn_opt.value()->collateralOutpoint == in.prevout) { + auto dmn = dmn_opt.value(); newList.RemoveMN(dmn->proTxHash); if (debugLogs) { @@ -948,10 +958,12 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no // The payee for the current block was determined by the previous block's list, but it might have disappeared in the // current block. We still pay that MN one last time, however. - if (payee && newList.HasMN(payee->proTxHash)) { - auto dmn = newList.GetMN(payee->proTxHash); + if (payee_opt.has_value() && newList.HasMN(payee_opt.value()->proTxHash)) { + auto payee = payee_opt.value(); + auto dmn_opt = newList.GetMN(payee->proTxHash); // HasMN has reported that GetMN should succeed, enforce that. - assert(dmn); + assert(dmn_opt.has_value()); + auto dmn = dmn_opt.value(); auto newState = std::make_shared(*dmn->pdmnState); newState->nLastPaidHeight = nHeight; // Starting from v19 and until MNRewardReallocation, EvoNodes will be paid 4 blocks in a row @@ -966,10 +978,11 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no } newList.UpdateMN(payee->proTxHash, newState); if (debugLogs) { - dmn = newList.GetMN(payee->proTxHash); + dmn_opt = newList.GetMN(payee->proTxHash); // Since the previous GetMN query returned a value, after an update, querying the same // hash *must* give us a result. If it doesn't, that would be a potential logic bug. - assert(dmn); + assert(dmn_opt.has_value()); + dmn = dmn_opt.value(); LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, nConsecutivePayments=%d\n", __func__, dmn->proTxHash.ToString(), dmn->pdmnState->nConsecutivePayments); } @@ -979,7 +992,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no auto newList2 = newList; newList2.ForEachMN(false, [&](auto& dmn) { if (dmn.nType != MnType::Evo) return; - if (payee != nullptr && dmn.proTxHash == payee->proTxHash && !isMNRewardReallocation) return; + if (payee_opt.has_value() && dmn.proTxHash == payee_opt.value()->proTxHash && !isMNRewardReallocation) return; if (dmn.pdmnState->nConsecutivePayments == 0) return; if (debugLogs) { LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, reset nConsecutivePayments %d->0\n", @@ -1609,7 +1622,7 @@ bool CheckProRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl: auto mnList = dmnman.GetListForBlock(pindexPrev); // only allow reusing of addresses when it's for the same collateral (which replaces the old MN) - if (mnList.HasUniqueProperty(opt_ptx->addr) && mnList.GetUniquePropertyMN(opt_ptx->addr)->collateralOutpoint != collateralOutpoint) { + if (auto unique_opt = mnList.GetUniquePropertyMN(opt_ptx->addr); unique_opt.has_value() && unique_opt.value()->collateralOutpoint != collateralOutpoint) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-addr"); } @@ -1673,19 +1686,20 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g } auto mnList = dmnman.GetListForBlock(pindexPrev); - auto mn = mnList.GetMN(opt_ptx->proTxHash); - if (!mn) { + auto mn_opt = mnList.GetMN(opt_ptx->proTxHash); + if (!mn_opt.has_value()) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } + auto mn = mn_opt.value(); // don't allow updating to addresses already used by other MNs - if (mnList.HasUniqueProperty(opt_ptx->addr) && mnList.GetUniquePropertyMN(opt_ptx->addr)->proTxHash != opt_ptx->proTxHash) { + if (auto unique_opt = mnList.GetUniquePropertyMN(opt_ptx->addr); unique_opt.has_value() && unique_opt.value()->proTxHash != opt_ptx->proTxHash) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-addr"); } // don't allow updating to platformNodeIds already used by other EvoNodes if (opt_ptx->nType == MnType::Evo) { - if (mnList.HasUniqueProperty(opt_ptx->platformNodeID) && mnList.GetUniquePropertyMN(opt_ptx->platformNodeID)->proTxHash != opt_ptx->proTxHash) { + if (auto unique_opt = mnList.GetUniquePropertyMN(opt_ptx->platformNodeID); unique_opt.has_value() && unique_opt.value()->proTxHash != opt_ptx->proTxHash) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-platformnodeid"); } } @@ -1728,11 +1742,12 @@ bool CheckProUpRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gs } auto mnList = dmnman.GetListForBlock(pindexPrev); - auto dmn = mnList.GetMN(opt_ptx->proTxHash); - if (!dmn) { + auto dmn_opt = mnList.GetMN(opt_ptx->proTxHash); + if (!dmn_opt.has_value()) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); } + auto dmn = dmn_opt.value(); // don't allow reuse of payee key for other keys (don't allow people to put the payee key onto an online server) if (payoutDest == CTxDestination(PKHash(dmn->pdmnState->keyIDOwner)) || payoutDest == CTxDestination(PKHash(opt_ptx->keyIDVoting))) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse"); @@ -1753,9 +1768,8 @@ bool CheckProUpRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gs return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-reuse"); } - if (mnList.HasUniqueProperty(opt_ptx->pubKeyOperator)) { - auto otherDmn = mnList.GetUniquePropertyMN(opt_ptx->pubKeyOperator); - if (opt_ptx->proTxHash != otherDmn->proTxHash) { + if (auto otherDmnOpt = mnList.GetUniquePropertyMN(opt_ptx->pubKeyOperator); otherDmnOpt.has_value()) { + if (opt_ptx->proTxHash != otherDmnOpt.value()->proTxHash) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-key"); } } @@ -1787,15 +1801,15 @@ bool CheckProUpRevTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gs } auto mnList = dmnman.GetListForBlock(pindexPrev); - auto dmn = mnList.GetMN(opt_ptx->proTxHash); - if (!dmn) + auto dmn_opt = mnList.GetMN(opt_ptx->proTxHash); + if (!dmn_opt) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-protx-hash"); if (!CheckInputsHash(tx, *opt_ptx, state)) { // pass the state returned by the function above return false; } - if (check_sigs && !CheckHashSig(*opt_ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) { + if (check_sigs && !CheckHashSig(*opt_ptx, dmn_opt.value()->pdmnState->pubKeyOperator.Get(), state)) { // pass the state returned by the function above return false; } diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 52119350f16eb..e1fd25a92b24e 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -326,24 +326,24 @@ class CDeterministicMNList [[nodiscard]] bool HasMN(const uint256& proTxHash) const { - return GetMN(proTxHash) != nullptr; + return GetMN(proTxHash).has_value(); } [[nodiscard]] bool HasMNByCollateral(const COutPoint& collateralOutpoint) const { - return GetMNByCollateral(collateralOutpoint) != nullptr; + return GetMNByCollateral(collateralOutpoint).has_value(); } [[nodiscard]] bool HasValidMNByCollateral(const COutPoint& collateralOutpoint) const { - return GetValidMNByCollateral(collateralOutpoint) != nullptr; + return GetValidMNByCollateral(collateralOutpoint).has_value(); } - [[nodiscard]] CDeterministicMNCPtr GetMN(const uint256& proTxHash) const; - [[nodiscard]] CDeterministicMNCPtr GetValidMN(const uint256& proTxHash) const; - [[nodiscard]] CDeterministicMNCPtr GetMNByOperatorKey(const CBLSPublicKey& pubKey) const; - [[nodiscard]] CDeterministicMNCPtr GetMNByCollateral(const COutPoint& collateralOutpoint) const; - [[nodiscard]] CDeterministicMNCPtr GetValidMNByCollateral(const COutPoint& collateralOutpoint) const; - [[nodiscard]] CDeterministicMNCPtr GetMNByService(const CService& service) const; - [[nodiscard]] CDeterministicMNCPtr GetMNByInternalId(uint64_t internalId) const; - [[nodiscard]] CDeterministicMNCPtr GetMNPayee(gsl::not_null pindexPrev) const; + [[nodiscard]] std::optional GetMN(const uint256& proTxHash) const; + [[nodiscard]] std::optional GetValidMN(const uint256& proTxHash) const; + [[nodiscard]] std::optional GetMNByOperatorKey(const CBLSPublicKey& pubKey) const; + [[nodiscard]] std::optional GetMNByCollateral(const COutPoint& collateralOutpoint) const; + [[nodiscard]] std::optional GetValidMNByCollateral(const COutPoint& collateralOutpoint) const; + [[nodiscard]] std::optional GetMNByService(const CService& service) const; + [[nodiscard]] std::optional GetMNByInternalId(uint64_t internalId) const; + [[nodiscard]] std::optional GetMNPayee(gsl::not_null pindexPrev) const; /** * Calculates the projected MN payees for the next *count* blocks. The result is not guaranteed to be correct @@ -401,11 +401,11 @@ class CDeterministicMNList return mnUniquePropertyMap.count(GetUniquePropertyHash(v)) != 0; } template - [[nodiscard]] CDeterministicMNCPtr GetUniquePropertyMN(const T& v) const + [[nodiscard]] std::optional GetUniquePropertyMN(const T& v) const { auto p = mnUniquePropertyMap.find(GetUniquePropertyHash(v)); if (!p) { - return nullptr; + return std::nullopt; } return GetMN(p->first); } diff --git a/src/evo/mnauth.cpp b/src/evo/mnauth.cpp index 63a9cd32043d3..cb7ef1c76aebe 100644 --- a/src/evo/mnauth.cpp +++ b/src/evo/mnauth.cpp @@ -93,14 +93,15 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, ServiceFlags node_services, CCon return tl::unexpected{MisbehavingError{100, "invalid mnauth signature"}}; } - const auto dmn = tip_mn_list.GetMN(mnauth.proRegTxHash); - if (!dmn) { + const auto dmn_opt = tip_mn_list.GetMN(mnauth.proRegTxHash); + if (!dmn_opt.has_value()) { // in case node was unlucky and not up to date, just let it be connected as a regular node, which gives it // a chance to get up-to-date and thus realize that it's not a MN anymore. We still give it a // low DoS score. return tl::unexpected{MisbehavingError{10, "missing mnauth masternode"}}; } + auto dmn = dmn_opt.value(); uint256 signHash; int nOurNodeVersion{PROTOCOL_VERSION}; if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) { @@ -206,10 +207,12 @@ void CMNAuth::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& if (verifiedProRegTxHash.IsNull()) { return; } - const auto verifiedDmn = oldMNList.GetMN(verifiedProRegTxHash); - if (!verifiedDmn) { + const auto verifiedDmnOpt = oldMNList.GetMN(verifiedProRegTxHash); + if (!verifiedDmnOpt.has_value()) { return; } + + const auto verifiedDmn = verifiedDmnOpt.value(); bool doRemove = false; if (diff.removedMns.count(verifiedDmn->GetInternalId())) { doRemove = true; diff --git a/src/evo/simplifiedmns.cpp b/src/evo/simplifiedmns.cpp index 573edbc1f10ff..3d629090f7cf4 100644 --- a/src/evo/simplifiedmns.cpp +++ b/src/evo/simplifiedmns.cpp @@ -308,13 +308,13 @@ CSimplifiedMNListDiff BuildSimplifiedDiff(const CDeterministicMNList& from, cons diffRet.blockHash = to.GetBlockHash(); to.ForEachMN(false, [&](const auto& toPtr) { - auto fromPtr = from.GetMN(toPtr.proTxHash); - if (fromPtr == nullptr) { + auto fromPtrOpt = from.GetMN(toPtr.proTxHash); + if (!fromPtrOpt.has_value()) { CSimplifiedMNListEntry sme(toPtr); diffRet.mnList.push_back(std::move(sme)); } else { CSimplifiedMNListEntry sme1(toPtr); - CSimplifiedMNListEntry sme2(*fromPtr); + CSimplifiedMNListEntry sme2(*fromPtrOpt.value()); if ((sme1 != sme2) || (extended && (sme1.scriptPayout != sme2.scriptPayout || sme1.scriptOperatorPayout != sme2.scriptOperatorPayout))) { diffRet.mnList.push_back(std::move(sme1)); @@ -323,8 +323,8 @@ CSimplifiedMNListDiff BuildSimplifiedDiff(const CDeterministicMNList& from, cons }); from.ForEachMN(false, [&](auto& fromPtr) { - auto toPtr = to.GetMN(fromPtr.proTxHash); - if (toPtr == nullptr) { + auto toPtrOpt = to.GetMN(fromPtr.proTxHash); + if (!toPtrOpt.has_value()) { diffRet.deletedMNs.emplace_back(fromPtr.proTxHash); } }); diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index deea85ce26844..8181136fa3560 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -536,9 +536,9 @@ std::vector CGovernanceManager::GetCurrentVotes(const uint256& mapMasternodes.emplace(dmn->collateralOutpoint, dmn); }); } else { - auto dmn = tip_mn_list.GetMNByCollateral(mnCollateralOutpointFilter); - if (dmn) { - mapMasternodes.emplace(dmn->collateralOutpoint, dmn); + auto dmn_opt = tip_mn_list.GetMNByCollateral(mnCollateralOutpointFilter); + if (dmn_opt.has_value()) { + mapMasternodes.emplace(dmn_opt.value()->collateralOutpoint, dmn_opt.value()); } } @@ -1600,10 +1600,11 @@ void CGovernanceManager::RemoveInvalidVotes() std::vector changedKeyMNs; for (const auto& p : diff.updatedMNs) { - auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(p.first); + auto oldDmnOpt = lastMNListForVotingKeys->GetMNByInternalId(p.first); // BuildDiff will construct itself with MNs that we already have knowledge // of, meaning that fetch operations should never fail. - assert(oldDmn); + assert(oldDmnOpt.has_value()); + auto oldDmn = oldDmnOpt.value(); if ((p.second.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && p.second.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) { changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); } else if ((p.second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && p.second.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) { @@ -1611,11 +1612,11 @@ void CGovernanceManager::RemoveInvalidVotes() } } for (const auto& id : diff.removedMns) { - auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(id); + auto oldDmnOpt = lastMNListForVotingKeys->GetMNByInternalId(id); // BuildDiff will construct itself with MNs that we already have knowledge // of, meaning that fetch operations should never fail. - assert(oldDmn); - changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); + assert(oldDmnOpt.has_value()); + changedKeyMNs.emplace_back(oldDmnOpt.value()->collateralOutpoint); } for (const auto& outpoint : changedKeyMNs) { diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 6ae0ed619b464..52caa13070d72 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -72,14 +72,15 @@ bool CGovernanceObject::ProcessVote(CMasternodeMetaMan& mn_metaman, CGovernanceM return false; } - auto dmn = tip_mn_list.GetMNByCollateral(vote.GetMasternodeOutpoint()); - if (!dmn) { + auto dmn_opt = tip_mn_list.GetMNByCollateral(vote.GetMasternodeOutpoint()); + if (!dmn_opt.has_value()) { std::ostringstream ostr; ostr << "CGovernanceObject::ProcessVote -- Masternode " << vote.GetMasternodeOutpoint().ToStringShort() << " not found"; exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20); return false; } + auto dmn = dmn_opt.value(); auto it = mapCurrentMNVotes.emplace(vote_m_t::value_type(vote.GetMasternodeOutpoint(), vote_rec_t())).first; vote_rec_t& voteRecordRef = it->second; vote_signal_enum_t eSignal = vote.GetSignal(); @@ -421,12 +422,13 @@ bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, } std::string strOutpoint = m_obj.masternodeOutpoint.ToStringShort(); - auto dmn = tip_mn_list.GetMNByCollateral(m_obj.masternodeOutpoint); - if (!dmn) { + auto dmn_opt = tip_mn_list.GetMNByCollateral(m_obj.masternodeOutpoint); + if (!dmn_opt.has_value()) { strError = "Failed to find Masternode by UTXO, missing masternode=" + strOutpoint; return false; } + auto dmn = dmn_opt.value(); // Check that we have a valid MN signature if (!CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) { strError = "Invalid masternode signature for: " + strOutpoint + ", pubkey = " + dmn->pdmnState->pubKeyOperator.ToString(); @@ -556,8 +558,8 @@ int CGovernanceObject::CountMatchingVotes(const CDeterministicMNList& tip_mn_lis if (it2 != recVote.mapInstances.end() && it2->second.eOutcome == eVoteOutcomeIn) { // 4x times weight vote for EvoNode owners. // No need to check if v19 is active since no EvoNode are allowed to register before v19s - auto dmn = tip_mn_list.GetMNByCollateral(votepair.first); - if (dmn != nullptr) nCount += GetMnType(dmn->nType).voting_weight; + auto dmn_opt = tip_mn_list.GetMNByCollateral(votepair.first); + if (dmn_opt.has_value()) nCount += GetMnType(dmn_opt.value()->nType).voting_weight; } } return nCount; diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index 5cfa2b1ac1869..d4fe7b10a76a4 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -98,8 +98,8 @@ CGovernanceVote::CGovernanceVote(const COutPoint& outpointMasternodeIn, const ui std::string CGovernanceVote::ToString(const CDeterministicMNList& tip_mn_list) const { - auto dmn = tip_mn_list.GetMNByCollateral(masternodeOutpoint); - int voteWeight = dmn != nullptr ? GetMnType(dmn->nType).voting_weight : 0; + auto dmn_opt = tip_mn_list.GetMNByCollateral(masternodeOutpoint); + int voteWeight = dmn_opt.has_value() ? GetMnType(dmn_opt.value()->nType).voting_weight : 0; std::ostringstream ostr; ostr << masternodeOutpoint.ToStringShort() << ":" << nTime << ":" @@ -209,12 +209,13 @@ bool CGovernanceVote::IsValid(const CDeterministicMNList& tip_mn_list, bool useV return false; } - auto dmn = tip_mn_list.GetMNByCollateral(masternodeOutpoint); - if (!dmn) { + auto dmn_opt = tip_mn_list.GetMNByCollateral(masternodeOutpoint); + if (!dmn_opt.has_value()) { LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- Unknown Masternode - %s\n", masternodeOutpoint.ToStringShort()); return false; } + auto dmn = dmn_opt.value(); if (useVotingKey) { return CheckSignature(dmn->pdmnState->keyIDVoting); } else { diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 59147750ee657..5cf80285d406c 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -792,11 +792,11 @@ bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman& LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) { std::string debugMsg = strprintf("%s -- adding masternodes quorum connections for quorum %s:\n", __func__, pQuorumBaseBlockIndex->GetBlockHash().ToString()); for (const auto& c : connections) { - auto dmn = tip_mn_list.GetValidMN(c); - if (!dmn) { + auto dmn_opt = tip_mn_list.GetValidMN(c); + if (!dmn_opt.has_value()) { debugMsg += strprintf(" %s (not in valid MN set anymore)\n", c.ToString()); } else { - debugMsg += strprintf(" %s (%s)\n", c.ToString(), dmn->pdmnState->addr.ToStringAddrPort()); + debugMsg += strprintf(" %s (%s)\n", c.ToString(), dmn_opt.value()->pdmnState->addr.ToStringAddrPort()); } } LogPrint(BCLog::NET_NETCONN, debugMsg.c_str()); /* Continued */ @@ -839,11 +839,11 @@ void AddQuorumProbeConnections(const Consensus::LLMQParams& llmqParams, CConnman if (LogAcceptCategory(BCLog::LLMQ, BCLog::Level::Debug)) { std::string debugMsg = strprintf("%s -- adding masternodes probes for quorum %s:\n", __func__, pQuorumBaseBlockIndex->GetBlockHash().ToString()); for (const auto& c : probeConnections) { - auto dmn = tip_mn_list.GetValidMN(c); - if (!dmn) { + auto dmn_opt = tip_mn_list.GetValidMN(c); + if (!dmn_opt.has_value()) { debugMsg += strprintf(" %s (not in valid MN set anymore)\n", c.ToString()); } else { - debugMsg += strprintf(" %s (%s)\n", c.ToString(), dmn->pdmnState->addr.ToStringAddrPort()); + debugMsg += strprintf(" %s (%s)\n", c.ToString(), dmn_opt.value()->pdmnState->addr.ToStringAddrPort()); } } LogPrint(BCLog::NET_NETCONN, debugMsg.c_str()); /* Continued */ diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp index 0a0b51deef7d8..d5ab28526a82b 100644 --- a/src/masternode/node.cpp +++ b/src/masternode/node.cpp @@ -129,12 +129,13 @@ void CActiveMasternodeManager::InitInternal(const CBlockIndex* pindex) CDeterministicMNList mnList = Assert(m_dmnman)->GetListForBlock(pindex); - auto dmn = mnList.GetMNByOperatorKey(m_info.blsPubKeyOperator); - if (!dmn) { + auto dmn_opt = mnList.GetMNByOperatorKey(m_info.blsPubKeyOperator); + if (!dmn_opt.has_value()) { // MN not appeared on the chain yet return; } + auto dmn = dmn_opt.value(); if (!mnList.IsMNValid(dmn->proTxHash)) { if (mnList.IsMNPoSeBanned(dmn->proTxHash)) { m_state = MasternodeState::POSE_BANNED; @@ -200,11 +201,14 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con return reset(MasternodeState::REMOVED); } - auto oldDmn = oldMNList.GetMN(cur_protx_hash); - auto newDmn = newMNList.GetMN(cur_protx_hash); - if (!oldDmn || !newDmn) { + auto oldDmnOpt = oldMNList.GetMN(cur_protx_hash); + auto newDmnOpt = newMNList.GetMN(cur_protx_hash); + if (!oldDmnOpt.has_value() || !newDmnOpt.has_value()) { return reset(MasternodeState::SOME_ERROR); } + + auto oldDmn = oldDmnOpt.value(); + auto newDmn = newDmnOpt.value(); if (newDmn->pdmnState->pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) { // MN operator key changed or revoked return reset(MasternodeState::OPERATOR_KEY_CHANGED); diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index b90dc41936168..99767d96e6d47 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -51,18 +51,20 @@ CAmount PlatformShare(const CAmount reward) LogPrint(BCLog::MNPAYMENTS, "CMNPaymentsProcessor::%s -- MN reward %lld reallocated to credit pool\n", __func__, platformReward); voutMasternodePaymentsRet.emplace_back(platformReward, CScript() << OP_RETURN); } + const auto mnList = m_dmnman.GetListForBlock(pindexPrev); if (mnList.GetAllMNsCount() == 0) { LogPrint(BCLog::MNPAYMENTS, "CMNPaymentsProcessor::%s -- no masternode registered to receive a payment\n", __func__); return true; } - const auto dmnPayee = mnList.GetMNPayee(pindexPrev); - if (!dmnPayee) { + + auto dmnPayeeOpt = m_dmnman.GetListForBlock(pindexPrev).GetMNPayee(pindexPrev); + if (!dmnPayeeOpt.has_value()) { return false; } CAmount operatorReward = 0; - + auto dmnPayee = dmnPayeeOpt.value(); if (dmnPayee->nOperatorReward != 0 && dmnPayee->pdmnState->scriptOperatorPayout != CScript()) { // This calculation might eventually turn out to result in 0 even if an operator reward percentage is given. // This will however only happen in a few years when the block rewards drops very low. diff --git a/src/net.cpp b/src/net.cpp index 45ebdafbf98f7..0e466dd4ae8f5 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3503,8 +3503,8 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe std::tie(addr, addr_last_try) = addrman.Select(false, preferred_net); } - auto dmn = mnList.GetMNByService(addr); - bool isMasternode = dmn != nullptr; + auto dmn_opt = mnList.GetMNByService(addr); + bool isMasternode = dmn_opt.has_value(); // Require outbound IPv4/IPv6 connections, other than feelers, to be to distinct network groups if (!fFeeler && outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) { @@ -3516,7 +3516,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe break; // don't try to connect to masternodes that we already have a connection to (most likely inbound) - if (isMasternode && setConnectedMasternodes.count(dmn->proTxHash)) + if (isMasternode && setConnectedMasternodes.count(dmn_opt.value()->proTxHash)) break; // if we selected a local address, restart (local addresses are allowed in regtest and devnet) @@ -3745,10 +3745,12 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, std::vector ret; for (const auto& group : masternodeQuorumNodes) { for (const auto& proRegTxHash : group.second) { - auto dmn = mnList.GetMN(proRegTxHash); - if (!dmn) { + auto dmn_opt = mnList.GetMN(proRegTxHash); + if (!dmn_opt.has_value()) { continue; } + + auto dmn = dmn_opt.value(); const auto& addr2 = dmn->pdmnState->addr; if (connectedNodes.count(addr2) && !connectedProRegTxHashes.count(proRegTxHash)) { // we probably connected to it before it became a masternode @@ -3783,11 +3785,13 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, AssertLockHeld(cs_vPendingMasternodes); std::vector ret; for (auto it = masternodePendingProbes.begin(); it != masternodePendingProbes.end(); ) { - auto dmn = mnList.GetMN(*it); - if (!dmn) { + auto dmn_opt = mnList.GetMN(*it); + if (!dmn_opt.has_value()) { it = masternodePendingProbes.erase(it); continue; } + + auto dmn = dmn_opt.value(); bool connectedAndOutbound = connectedProRegTxHashes.count(dmn->proTxHash) && !connectedProRegTxHashes[dmn->proTxHash]; if (connectedAndOutbound) { // we already have an outbound connection to this MN so there is no theed to probe it again @@ -3813,11 +3817,13 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, LOCK2(m_nodes_mutex, cs_vPendingMasternodes); if (!vPendingMasternodes.empty()) { - auto dmn = mnList.GetValidMN(vPendingMasternodes.front()); + auto dmn_opt = mnList.GetValidMN(vPendingMasternodes.front()); vPendingMasternodes.erase(vPendingMasternodes.begin()); - if (dmn && !connectedNodes.count(dmn->pdmnState->addr) && !IsMasternodeOrDisconnectRequested(dmn->pdmnState->addr)) { - LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- opening pending masternode connection to %s, service=%s\n", _func_, dmn->proTxHash.ToString(), dmn->pdmnState->addr.ToStringAddrPort()); - return dmn; + if (dmn_opt.has_value()) { + if (auto dmn = dmn_opt.value(); !connectedNodes.count(dmn->pdmnState->addr) && !IsMasternodeOrDisconnectRequested(dmn->pdmnState->addr)) { + LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- opening pending masternode connection to %s, service=%s\n", _func_, dmn->proTxHash.ToString(), dmn->pdmnState->addr.ToStringAddrPort()); + return dmn; + } } } @@ -4706,12 +4712,12 @@ bool CConnman::IsMasternodeQuorumNode(const CNode* pnode, const CDeterministicMN // We however only need to know this if the node did not authenticate itself as a MN yet uint256 assumedProTxHash; if (pnode->GetVerifiedProRegTxHash().IsNull() && !pnode->IsInboundConn()) { - auto dmn = tip_mn_list.GetMNByService(pnode->addr); - if (dmn == nullptr) { + auto dmn_opt = tip_mn_list.GetMNByService(pnode->addr); + if (!dmn_opt.has_value()) { // This is definitely not a masternode return false; } - assumedProTxHash = dmn->proTxHash; + assumedProTxHash = dmn_opt.value()->proTxHash; } LOCK(cs_vPendingMasternodes); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0fc191983e28c..559ca41cda911 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3349,7 +3349,7 @@ std::pair static ValidateDSTX(CDeterministicMN } const CBlockIndex* pindex{nullptr}; - CDeterministicMNCPtr dmn{nullptr}; + std::optional dmn_opt{std::nullopt}; { LOCK(cs_main); pindex = chainman.ActiveChain().Tip(); @@ -3358,29 +3358,30 @@ std::pair static ValidateDSTX(CDeterministicMN // Try to find a MN up to 24 blocks deep to make sure such dstx-es are relayed and processed correctly. if (dstx.masternodeOutpoint.IsNull()) { for (int i = 0; i < 24 && pindex; ++i) { - dmn = dmnman.GetListForBlock(pindex).GetMN(dstx.m_protxHash); - if (dmn) { - dstx.masternodeOutpoint = dmn->collateralOutpoint; + dmn_opt = dmnman.GetListForBlock(pindex).GetMN(dstx.m_protxHash); + if (dmn_opt.has_value()) { + dstx.masternodeOutpoint = dmn_opt.value()->collateralOutpoint; break; } pindex = pindex->pprev; } } else { for (int i = 0; i < 24 && pindex; ++i) { - dmn = dmnman.GetListForBlock(pindex).GetMNByCollateral(dstx.masternodeOutpoint); - if (dmn) { - dstx.m_protxHash = dmn->proTxHash; + dmn_opt = dmnman.GetListForBlock(pindex).GetMNByCollateral(dstx.masternodeOutpoint); + if (dmn_opt.has_value()) { + dstx.m_protxHash = dmn_opt.value()->proTxHash; break; } pindex = pindex->pprev; } } - if (!dmn) { + if (!dmn_opt.has_value()) { LogPrint(BCLog::COINJOIN, "DSTX -- Can't find masternode %s to verify %s\n", dstx.masternodeOutpoint.ToStringShort(), hashTx.ToString()); return {false, true}; } + auto dmn = dmn_opt.value(); if (!mn_metaman.GetMetaInfo(dmn->proTxHash)->IsValidForMixingTxes()) { LogPrint(BCLog::COINJOIN, "DSTX -- Masternode %s is sending too many transactions %s\n", dstx.masternodeOutpoint.ToStringShort(), hashTx.ToString()); return {true, true}; diff --git a/src/qt/masternodelist.cpp b/src/qt/masternodelist.cpp index 7e3eabe4107d6..ecd5e5bdcde5c 100644 --- a/src/qt/masternodelist.cpp +++ b/src/qt/masternodelist.cpp @@ -355,7 +355,7 @@ CDeterministicMNCPtr MasternodeList::GetSelectedDIP3MN() proTxHash.SetHex(strProTxHash); // Caller is responsible for nullptr checking return value - return clientModel->getMasternodeList().first.GetMN(proTxHash); + return clientModel->getMasternodeList().first.GetMN(proTxHash).value_or(nullptr); } void MasternodeList::extraInfoDIP3_clicked() diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index a69aee89099ce..2d0ccf550d0ce 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1321,8 +1321,8 @@ void RPCConsole::updateDetailWidget() ui->peerPermissions->setText(permissions.join(" & ")); } ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as != 0 ? QString::number(stats->nodeStats.m_mapped_as) : ts.na); - auto dmn = clientModel->getMasternodeList().first.GetMNByService(stats->nodeStats.addr); - if (dmn == nullptr) { + auto dmn_opt = clientModel->getMasternodeList().first.GetMNByService(stats->nodeStats.addr); + if (!dmn_opt.has_value()) { ui->peerNodeType->setText(tr("Regular")); ui->peerPoSeScore->setText(ts.na); } else { @@ -1331,7 +1331,7 @@ void RPCConsole::updateDetailWidget() } else { ui->peerNodeType->setText(tr("Verified Masternode")); } - ui->peerPoSeScore->setText(QString::number(dmn->pdmnState->nPoSePenalty)); + ui->peerPoSeScore->setText(QString::number(dmn_opt.value()->pdmnState->nPoSePenalty)); } // This check fails for example if the lock was busy and diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index a7d040a059baf..ed74352cd2a9b 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1003,10 +1003,12 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques paramIdx += 3; } - auto dmn = dmnman.GetListAtChainTip().GetMN(ptx.proTxHash); - if (!dmn) { + auto dmn_opt = dmnman.GetListAtChainTip().GetMN(ptx.proTxHash); + if (!dmn_opt.has_value()) { throw std::runtime_error(strprintf("masternode with proTxHash %s not found", ptx.proTxHash.ToString())); } + + auto dmn = dmn_opt.value(); if (dmn->nType != mnType) { throw std::runtime_error(strprintf("masternode with proTxHash %s is not a %s", ptx.proTxHash.ToString(), GetMnType(mnType).description)); } @@ -1100,10 +1102,12 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme CProUpRegTx ptx; ptx.proTxHash = ParseHashV(request.params[0], "proTxHash"); - auto dmn = dmnman.GetListAtChainTip().GetMN(ptx.proTxHash); - if (!dmn) { + auto dmn_opt = dmnman.GetListAtChainTip().GetMN(ptx.proTxHash); + if (!dmn_opt.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("masternode %s not found", ptx.proTxHash.ToString())); } + + auto dmn = dmn_opt.value(); ptx.keyIDVoting = dmn->pdmnState->keyIDVoting; ptx.scriptPayout = dmn->pdmnState->scriptPayout; @@ -1225,11 +1229,12 @@ static RPCHelpMan protx_revoke() ptx.nReason = (uint16_t)nReason; } - auto dmn = dmnman.GetListAtChainTip().GetMN(ptx.proTxHash); - if (!dmn) { + auto dmn_opt = dmnman.GetListAtChainTip().GetMN(ptx.proTxHash); + if (!dmn_opt.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("masternode %s not found", ptx.proTxHash.ToString())); } + auto dmn = dmn_opt.value(); if (keyOperator.GetPublicKey() != dmn->pdmnState->pubKeyOperator.Get()) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("the operator key does not belong to the registered public key")); } @@ -1505,11 +1510,11 @@ static RPCHelpMan protx_info() } auto mnList = dmnman.GetListForBlock(pindex); - auto dmn = mnList.GetMN(proTxHash); - if (!dmn) { + auto dmn_opt = mnList.GetMN(proTxHash); + if (!dmn_opt.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s not found", proTxHash.ToString())); } - return BuildDMNListEntry(wallet.get(), *dmn, mn_metaman, true, chainman, pindex); + return BuildDMNListEntry(wallet.get(), *dmn_opt.value(), mn_metaman, true, chainman, pindex); }, }; } @@ -1634,20 +1639,21 @@ static RPCHelpMan protx_listdiff() UniValue jremovedMNs(UniValue::VARR); for(const auto& internal_id : mnDiff.removedMns) { - auto dmn = baseBlockMNList.GetMNByInternalId(internal_id); + auto dmn_opt = baseBlockMNList.GetMNByInternalId(internal_id); // BuildDiff will construct itself with MNs that we already have knowledge // of, meaning that fetch operations should never fail. - CHECK_NONFATAL(dmn); - jremovedMNs.push_back(dmn->proTxHash.ToString()); + CHECK_NONFATAL(dmn_opt.has_value()); + jremovedMNs.push_back(dmn_opt.value()->proTxHash.ToString()); } ret.pushKV("removedMNs", jremovedMNs); UniValue jupdatedMNs(UniValue::VARR); for(const auto& [internal_id, stateDiff] : mnDiff.updatedMNs) { - auto dmn = baseBlockMNList.GetMNByInternalId(internal_id); + auto dmn_opt = baseBlockMNList.GetMNByInternalId(internal_id); // BuildDiff will construct itself with MNs that we already have knowledge // of, meaning that fetch operations should never fail. - CHECK_NONFATAL(dmn); + CHECK_NONFATAL(dmn_opt.has_value()); + auto dmn = dmn_opt.value(); UniValue obj(UniValue::VOBJ); obj.pushKV(dmn->proTxHash.ToString(), stateDiff.ToJson(dmn->nType)); jupdatedMNs.push_back(obj); diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 5123945c3f323..48da2260dd420 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -458,8 +458,8 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet UniValue statusObj(UniValue::VOBJ); - auto dmn = mnList.GetValidMN(proTxHash); - if (!dmn) { + auto dmn_opt = mnList.GetValidMN(proTxHash); + if (!dmn_opt.has_value()) { nFailed++; statusObj.pushKV("result", "failed"); statusObj.pushKV("errorMessage", "Can't find masternode by proTxHash"); @@ -467,6 +467,7 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet continue; } + auto dmn = dmn_opt.value(); CGovernanceVote vote(dmn->collateralOutpoint, hash, eVoteSignal, eVoteOutcome); if (!SignVote(wallet, keyID, vote) || !vote.CheckSignature(keyID)) { @@ -598,12 +599,12 @@ static RPCHelpMan gobject_vote_alias() EnsureWalletIsUnlocked(*wallet); uint256 proTxHash(ParseHashV(request.params[3], "protx-hash")); - auto dmn = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetValidMN(proTxHash); - if (!dmn) { + auto dmn_opt = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetValidMN(proTxHash); + if (!dmn_opt.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid or unknown proTxHash"); } - + auto dmn = dmn_opt.value(); const bool is_mine = CheckWalletOwnsKey(*wallet, dmn->pdmnState->keyIDVoting); if (!is_mine) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Private key for voting address %s not known by wallet", EncodeDestination(PKHash(dmn->pdmnState->keyIDVoting)))); diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index b18f7ef06d2fa..14644e8391109 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -229,8 +229,9 @@ static RPCHelpMan masternode_status() // keep compatibility with legacy status for now (might get deprecated/removed later) mnObj.pushKV("outpoint", node.mn_activeman->GetOutPoint().ToStringShort()); mnObj.pushKV("service", node.mn_activeman->GetService().ToStringAddrPort()); - auto dmn = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetMN(node.mn_activeman->GetProTxHash()); - if (dmn) { + auto dmn_opt = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetMN(node.mn_activeman->GetProTxHash()); + if (dmn_opt.has_value()) { + auto dmn = dmn_opt.value(); mnObj.pushKV("proTxHash", dmn->proTxHash.ToString()); mnObj.pushKV("type", std::string(GetMnType(dmn->nType).description)); mnObj.pushKV("collateralHash", dmn->collateralOutpoint.hash.ToString()); @@ -321,9 +322,9 @@ static RPCHelpMan masternode_winners() const auto tip_mn_list = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip(); for (int h = nStartHeight; h <= nChainTipHeight; h++) { const CBlockIndex* pIndex = pindexTip->GetAncestor(h - 1); - auto payee = node.dmnman->GetListForBlock(pIndex).GetMNPayee(pIndex); - if (payee) { - std::string strPayments = GetRequiredPaymentsString(*CHECK_NONFATAL(node.govman), tip_mn_list, h, payee); + auto payee_opt = node.dmnman->GetListForBlock(pIndex).GetMNPayee(pIndex); + if (payee_opt.has_value()) { + std::string strPayments = GetRequiredPaymentsString(*CHECK_NONFATAL(node.govman), tip_mn_list, h, payee_opt.value()); if (strFilter != "" && strPayments.find(strFilter) == std::string::npos) continue; obj.pushKV(strprintf("%d", h), strPayments); } @@ -458,8 +459,8 @@ static RPCHelpMan masternode_payments() } // NOTE: we use _previous_ block to find a payee for the current one - const auto dmnPayee = node.dmnman->GetListForBlock(pindex->pprev).GetMNPayee(pindex->pprev); - protxObj.pushKV("proTxHash", dmnPayee == nullptr ? "" : dmnPayee->proTxHash.ToString()); + auto dmnPayeeOpt = node.dmnman->GetListForBlock(pindex->pprev).GetMNPayee(pindex->pprev); + protxObj.pushKV("proTxHash", !dmnPayeeOpt.has_value() ? "" : dmnPayeeOpt.value()->proTxHash.ToString()); protxObj.pushKV("amount", payedPerMasternode); protxObj.pushKV("payees", payeesArr); payedPerBlock += payedPerMasternode; diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index c685118c5e6fc..977be7bc5c3a4 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -397,11 +397,12 @@ static RPCHelpMan quorum_memberof() const CBlockIndex* pindexTip = WITH_LOCK(cs_main, return chainman.ActiveChain().Tip()); auto mnList = CHECK_NONFATAL(node.dmnman)->GetListForBlock(pindexTip); - auto dmn = mnList.GetMN(protxHash); - if (!dmn) { + auto dmn_opt = mnList.GetMN(protxHash); + if (!dmn_opt.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "masternode not found"); } + auto dmn = dmn_opt.value(); UniValue result(UniValue::VARR); for (const auto& type : llmq::GetEnabledQuorumTypes(pindexTip)) { const auto llmq_params_opt = Params().GetLLMQ(type); diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index ae82700ad2da6..68f184204e53c 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -466,8 +466,8 @@ void FuncDIP3Protx(TestChainSetup& setup) // check MN reward payments for (size_t i = 0; i < 20; i++) { - auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); - BOOST_ASSERT(dmnExpectedPayee); + auto dmnExpectedPayeeOpt = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); + BOOST_ASSERT(dmnExpectedPayeeOpt.has_value()); CBlock block = setup.CreateAndProcessBlock({}, setup.coinbaseKey); dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); @@ -475,7 +475,7 @@ void FuncDIP3Protx(TestChainSetup& setup) auto dmnPayout = FindPayoutDmn(dmnman, block); BOOST_REQUIRE(dmnPayout != nullptr); - BOOST_CHECK_EQUAL(dmnPayout->proTxHash.ToString(), dmnExpectedPayee->proTxHash.ToString()); + BOOST_CHECK_EQUAL(dmnPayout->proTxHash.ToString(), dmnExpectedPayeeOpt.value()->proTxHash.ToString()); nHeight++; } @@ -510,8 +510,8 @@ void FuncDIP3Protx(TestChainSetup& setup) BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); nHeight++; - auto dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); - BOOST_REQUIRE(dmn != nullptr && dmn->pdmnState->addr.GetPort() == 1000); + auto dmn_opt = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); + BOOST_REQUIRE(dmn_opt.has_value() && dmn_opt.value()->pdmnState->addr.GetPort() == 1000); // test ProUpRevTx tx = CreateProUpRevTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], operatorKeys[dmnHashes[0]], setup.coinbaseKey); @@ -520,13 +520,13 @@ void FuncDIP3Protx(TestChainSetup& setup) BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); nHeight++; - dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); - BOOST_REQUIRE(dmn != nullptr && dmn->pdmnState->GetBannedHeight() == nHeight); + dmn_opt = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); + BOOST_REQUIRE(dmn_opt.has_value() && dmn_opt.value()->pdmnState->GetBannedHeight() == nHeight); // test that the revoked MN does not get paid anymore for (size_t i = 0; i < 20; i++) { - auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); - BOOST_REQUIRE(dmnExpectedPayee && dmnExpectedPayee->proTxHash != dmnHashes[0]); + auto dmnExpectedPayeeOpt = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); + BOOST_REQUIRE(dmnExpectedPayeeOpt.has_value() && dmnExpectedPayeeOpt.value()->proTxHash != dmnHashes[0]); CBlock block = setup.CreateAndProcessBlock({}, setup.coinbaseKey); dmnman.UpdatedBlockTip(chainman.ActiveChain().Tip()); @@ -534,7 +534,7 @@ void FuncDIP3Protx(TestChainSetup& setup) auto dmnPayout = FindPayoutDmn(dmnman, block); BOOST_REQUIRE(dmnPayout != nullptr); - BOOST_CHECK_EQUAL(dmnPayout->proTxHash.ToString(), dmnExpectedPayee->proTxHash.ToString()); + BOOST_CHECK_EQUAL(dmnPayout->proTxHash.ToString(), dmnExpectedPayeeOpt.value()->proTxHash.ToString()); nHeight++; } @@ -542,8 +542,8 @@ void FuncDIP3Protx(TestChainSetup& setup) // test reviving the MN CBLSSecretKey newOperatorKey; newOperatorKey.MakeNewKey(); - dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); - tx = CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], ownerKeys[dmnHashes[0]], newOperatorKey.GetPublicKey(), ownerKeys[dmnHashes[0]].GetPubKey().GetID(), dmn->pdmnState->scriptPayout, setup.coinbaseKey); + dmn_opt = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); + tx = CreateProUpRegTx(chainman.ActiveChain(), *(setup.m_node.mempool), utxos, dmnHashes[0], ownerKeys[dmnHashes[0]], newOperatorKey.GetPublicKey(), ownerKeys[dmnHashes[0]].GetPubKey().GetID(), dmn_opt.value()->pdmnState->scriptPayout, setup.coinbaseKey); // check malleability protection again, but this time by also relying on the signature inside the ProUpRegTx auto tx2 = MalleateProTxPayout(tx); TxValidationState dummy_state; @@ -568,16 +568,16 @@ void FuncDIP3Protx(TestChainSetup& setup) BOOST_CHECK_EQUAL(chainman.ActiveChain().Height(), nHeight + 1); nHeight++; - dmn = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); - BOOST_REQUIRE(dmn != nullptr && dmn->pdmnState->addr.GetPort() == 100); - BOOST_REQUIRE(dmn != nullptr && !dmn->pdmnState->IsBanned()); + dmn_opt = dmnman.GetListAtChainTip().GetMN(dmnHashes[0]); + BOOST_REQUIRE(dmn_opt.has_value() && dmn_opt.value()->pdmnState->addr.GetPort() == 100); + BOOST_REQUIRE(dmn_opt.has_value() && !dmn_opt.value()->pdmnState->IsBanned()); // test that the revived MN gets payments again bool foundRevived = false; for (size_t i = 0; i < 20; i++) { - auto dmnExpectedPayee = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); - BOOST_ASSERT(dmnExpectedPayee); - if (dmnExpectedPayee->proTxHash == dmnHashes[0]) { + auto dmnExpectedPayeeOpt = dmnman.GetListAtChainTip().GetMNPayee(chainman.ActiveChain().Tip()); + BOOST_ASSERT(dmnExpectedPayeeOpt.has_value()); + if (dmnExpectedPayeeOpt.value()->proTxHash == dmnHashes[0]) { foundRevived = true; } @@ -587,7 +587,7 @@ void FuncDIP3Protx(TestChainSetup& setup) auto dmnPayout = FindPayoutDmn(dmnman, block); BOOST_REQUIRE(dmnPayout != nullptr); - BOOST_CHECK_EQUAL(dmnPayout->proTxHash.ToString(), dmnExpectedPayee->proTxHash.ToString()); + BOOST_CHECK_EQUAL(dmnPayout->proTxHash.ToString(), dmnExpectedPayeeOpt.value()->proTxHash.ToString()); nHeight++; } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 291599aea201e..d5a46a504fdb5 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -702,7 +702,9 @@ void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, con auto proTx = *Assert(GetTxPayload(tx)); mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash()); mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx.GetHash()); - auto dmn = Assert(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash)); + // addUncheckedProTx is called after AcceptToMemoryPool() has run appropriate checks. So + // we should assume that the contents of this transaction are valid. See comment in addUnchecked. + auto dmn = Assert(*(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash))); newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator); if (dmn->pdmnState->pubKeyOperator != proTx.pubKeyOperator) { newit->isKeyChangeProTx = true; @@ -710,7 +712,9 @@ void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, con } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { auto proTx = *Assert(GetTxPayload(tx)); mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash()); - auto dmn = Assert(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash)); + // addUncheckedProTx is called after AcceptToMemoryPool() has run appropriate checks. So + // we should assume that the contents of this transaction are valid. See comment in addUnchecked. + auto dmn = Assert(*(m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash))); newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator); if (dmn->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()) { newit->isKeyChangeProTx = true; @@ -981,10 +985,10 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx) // These are not yet mined ProRegTxs removeSpentCollateralConflict(collateralIt->second); } - auto dmn = mnList.GetMNByCollateral(in.prevout); - if (dmn) { + auto dmn_opt = mnList.GetMNByCollateral(in.prevout); + if (dmn_opt.has_value()) { // These are updates referring to a mined ProRegTx - removeSpentCollateralConflict(dmn->proTxHash); + removeSpentCollateralConflict(dmn_opt.value()->proTxHash); } } } @@ -1404,13 +1408,13 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { auto& proTx = *opt_proTx; // this method should only be called with validated ProTxs - auto dmn = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash); - if (!dmn) { + auto dmn_opt = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash); + if (!dmn_opt) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString()); return true; // i.e. failed to find validated ProTx == conflict } // only allow one operator key change in the mempool - if (dmn->pdmnState->pubKeyOperator != proTx.pubKeyOperator) { + if (dmn_opt.value()->pdmnState->pubKeyOperator != proTx.pubKeyOperator) { if (hasKeyChangeInMempool(proTx.proTxHash)) { return true; } @@ -1426,13 +1430,13 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { } auto& proTx = *opt_proTx; // this method should only be called with validated ProTxs - auto dmn = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash); - if (!dmn) { + auto dmn_opt = m_dmnman->GetListAtChainTip().GetMN(proTx.proTxHash); + if (!dmn_opt.has_value()) { LogPrint(BCLog::MEMPOOL, "%s: ERROR: Masternode is not in the list, proTxHash: %s\n", __func__, proTx.proTxHash.ToString()); return true; // i.e. failed to find validated ProTx == conflict } // only allow one operator key change in the mempool - if (dmn->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()) { + if (dmn_opt.value()->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()) { if (hasKeyChangeInMempool(proTx.proTxHash)) { return true; }