Skip to content

Commit

Permalink
fix: possible assert call if nHeight in CDeterministicMNListDiff is h…
Browse files Browse the repository at this point in the history
…igher than Tip - cummulative from dashpay#5590

Example of new log:
```
2023-09-28T17:35:50Z GetProjectedMNPayeesAtChainTip WARNING pindex is nullptr due to height=914160 chain height=914159
```

instead assert call:
```
...
 dashpay#6  0x00007ffff7a33b86 in __assert_fail (assertion=0x55555783afd2 "pindex", file=0x5555577f2ed8 "llmq/utils.cpp", line=730,
        function=0x5555577f2448 "bool llmq::utils::IsMNRewardReallocationActive(const CBlockIndex*)") at ./assert/assert.c:101
 dashpay#7  0x0000555555ab7daf in llmq::utils::IsMNRewardReallocationActive (pindex=<optimized out>) at llmq/utils.cpp:730
 dashpay#8  0x00005555559458ad in CDeterministicMNList::GetProjectedMNPayees (this=this@entry=0x7fffffffc690, pindex=0x0, nCount=<optimized out>, nCount@entry=2147483647)
        at evo/deterministicmns.cpp:231
 dashpay#9  0x000055555594614f in CDeterministicMNList::GetProjectedMNPayeesAtChainTip (this=this@entry=0x7fffffffc690, nCount=nCount@entry=2147483647) at evo/deterministicmns.cpp:216
 dashpay#10 0x00005555558c9f51 in MasternodeList::updateDIP3List (this=this@entry=0x55555908cfd0) at qt/masternodelist.cpp:194
 dashpay#11 0x00005555558ca9a0 in MasternodeList::updateDIP3ListScheduled (this=0x55555908cfd0) at qt/masternodelist.cpp:157
 dashpay#12 0x000055555684a60f in void doActivate<false>(QObject*, int, void**) ()
 dashpay#13 0x00005555568525b1 in QTimer::timerEvent(QTimerEvent*) ()
 dashpay#14 0x0000555556844ce5 in QObject::event(QEvent*) ()
 dashpay#15 0x0000555556ac3252 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
 dashpay#16 0x000055555681e6b8 in QCoreApplication::sendEvent(QObject*, QEvent*) ()
 dashpay#17 0x000055555686de2a in QTimerInfoList::activateTimers() ()
 dashpay#18 0x000055555686be84 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
 dashpay#19 0x00005555569bf8a2 in QXcbUnixEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
 dashpay#20 0x000055555681caf6 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
 dashpay#21 0x0000555556825f8a in QCoreApplication::exec() ()
...
```

fix: use g_chainman.m_blockman.LookupBlockIndex for any known block (even wrong tip)

fix: handle GetProjectedMNPayees's failures in `MasternodeList::updateDIP3List()`

fix: resulting vector in GetProjectedMNPayees grows to the weighted count, should reserve memory accordingly

fix: avoid useless loop in GetProjectedMNPayees after mn_rr activation

fix: pass proper CBlockIndex from ClientModel

fix statusItem
  • Loading branch information
knst committed Oct 27, 2023
1 parent 3e732a9 commit 2bb63f0
Show file tree
Hide file tree
Showing 13 changed files with 70 additions and 54 deletions.
45 changes: 21 additions & 24 deletions src/evo/deterministicmns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,38 +214,35 @@ CDeterministicMNCPtr CDeterministicMNList::GetMNPayee(const CBlockIndex* pIndex)
return best;
}

std::vector<CDeterministicMNCPtr> CDeterministicMNList::GetProjectedMNPayeesAtChainTip(int nCount) const
{
return GetProjectedMNPayees(::ChainActive()[nHeight], nCount);
}

std::vector<CDeterministicMNCPtr> CDeterministicMNList::GetProjectedMNPayees(const CBlockIndex* const pindex, int nCount) const
{
if (nCount < 0 ) {
return {};
}
nCount = std::min(nCount, int(GetValidWeightedMNsCount()));
const auto weighted_count = GetValidWeightedMNsCount();
nCount = std::min(nCount, int(weighted_count));

std::vector<CDeterministicMNCPtr> result;
result.reserve(nCount);
result.reserve(weighted_count);

auto remaining_evo_payments = 0;
CDeterministicMNCPtr evo_to_be_skipped = nullptr;
bool isMNRewardReallocation = llmq::utils::IsMNRewardReallocationActive(pindex);
ForEachMNShared(true, [&](const CDeterministicMNCPtr& dmn) {
if (dmn->pdmnState->nLastPaidHeight == nHeight) {
// We found the last MN Payee.
// If the last payee is an EvoNode, we need to check its consecutive payments and pay him again if needed
if (!isMNRewardReallocation && dmn->nType == MnType::Evo && dmn->pdmnState->nConsecutivePayments < dmn_types::Evo.voting_weight) {
remaining_evo_payments = dmn_types::Evo.voting_weight - dmn->pdmnState->nConsecutivePayments;
for ([[maybe_unused]] auto _ : irange::range(remaining_evo_payments)) {
result.emplace_back(dmn);
evo_to_be_skipped = dmn;
int remaining_evo_payments{0};
CDeterministicMNCPtr evo_to_be_skipped{nullptr};
const bool isMNRewardReallocation = llmq::utils::IsMNRewardReallocationActive(pindex);
if (!isMNRewardReallocation) {
ForEachMNShared(true, [&](const CDeterministicMNCPtr& dmn) {
if (dmn->pdmnState->nLastPaidHeight == nHeight) {
// We found the last MN Payee.
// If the last payee is an EvoNode, we need to check its consecutive payments and pay him again if needed
if (dmn->nType == MnType::Evo && dmn->pdmnState->nConsecutivePayments < dmn_types::Evo.voting_weight) {
remaining_evo_payments = dmn_types::Evo.voting_weight - dmn->pdmnState->nConsecutivePayments;
for ([[maybe_unused]] auto _ : irange::range(remaining_evo_payments)) {
result.emplace_back(dmn);
evo_to_be_skipped = dmn;
}
}
}
}
return;
});
});
}

ForEachMNShared(true, [&](const CDeterministicMNCPtr& dmn) {
if (dmn == evo_to_be_skipped) return;
Expand Down Expand Up @@ -647,7 +644,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
// Don't hold cs while calling signals
if (diff.HasChanges()) {
GetMainSignals().NotifyMasternodeListChanged(false, oldList, diff, connman);
uiInterface.NotifyMasternodeListChanged(newList);
uiInterface.NotifyMasternodeListChanged(newList, pindex);
}

if (nHeight == consensusParams.DIP0003EnforcementHeight) {
Expand Down Expand Up @@ -688,7 +685,7 @@ bool CDeterministicMNManager::UndoBlock(const CBlockIndex* pindex)
if (diff.HasChanges()) {
auto inversedDiff = curList.BuildDiff(prevList);
GetMainSignals().NotifyMasternodeListChanged(true, curList, inversedDiff, connman);
uiInterface.NotifyMasternodeListChanged(prevList);
uiInterface.NotifyMasternodeListChanged(prevList, pindex->pprev);
}

const auto& consensusParams = Params().GetConsensus();
Expand Down
1 change: 0 additions & 1 deletion src/evo/deterministicmns.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,6 @@ class CDeterministicMNList
* @return
*/
[[nodiscard]] std::vector<CDeterministicMNCPtr> GetProjectedMNPayees(const CBlockIndex* const pindex, int nCount = std::numeric_limits<int>::max()) const;
[[nodiscard]] std::vector<CDeterministicMNCPtr> GetProjectedMNPayeesAtChainTip(int nCount = std::numeric_limits<int>::max()) const;

/**
* Calculate a quorum based on the modifier. The resulting list is deterministically sorted by score
Expand Down
6 changes: 4 additions & 2 deletions src/governance/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -676,8 +676,10 @@ void CGovernanceManager::CreateGovernanceTrigger(const CSuperblock& sb, CConnman
if (identical_sb == nullptr) {
// Nobody submitted a trigger we'd like to see,
// so let's do it but only if we are the payee
auto mnList = deterministicMNManager->GetListAtChainTip();
auto mn_payees = mnList.GetProjectedMNPayeesAtChainTip();

const CBlockIndex *tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
auto mnList = deterministicMNManager->GetListForBlock(tip);
auto mn_payees = mnList.GetProjectedMNPayees(tip);
if (mn_payees.empty()) return;
{
LOCK(activeMasternodeInfoCs);
Expand Down
6 changes: 4 additions & 2 deletions src/interfaces/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <vector>

class BanMan;
class CBlockIndex;
class CCoinControl;
class CDeterministicMNList;
class CFeeRate;
Expand All @@ -45,7 +46,7 @@ class EVO
{
public:
virtual ~EVO() {}
virtual CDeterministicMNList getListAtChainTip() = 0;
virtual std::pair<CDeterministicMNList, const CBlockIndex*> getListAtChainTip() = 0;
};

//! Interface for the src/governance part of a dash node (dashd process).
Expand Down Expand Up @@ -355,7 +356,8 @@ class Node

//! Register handler for masternode list update messages.
using NotifyMasternodeListChangedFn =
std::function<void(const CDeterministicMNList& newList)>;
std::function<void(const CDeterministicMNList& newList,
const CBlockIndex* pindex)>;
virtual std::unique_ptr<Handler> handleNotifyMasternodeListChanged(NotifyMasternodeListChangedFn fn) = 0;

//! Register handler for additional data sync progress update messages.
Expand Down
13 changes: 9 additions & 4 deletions src/node/interfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,14 @@ namespace {
class EVOImpl : public EVO
{
public:
CDeterministicMNList getListAtChainTip() override
std::pair<CDeterministicMNList, const CBlockIndex*> getListAtChainTip() override
{
return deterministicMNManager == nullptr ? CDeterministicMNList() : deterministicMNManager->GetListAtChainTip();
const CBlockIndex *tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
CDeterministicMNList mnList{};
if (tip != nullptr && deterministicMNManager != nullptr) {
mnList = deterministicMNManager->GetListForBlock(tip);
}
return {std::move(mnList), tip};
}
};

Expand Down Expand Up @@ -499,8 +504,8 @@ class NodeImpl : public Node
std::unique_ptr<Handler> handleNotifyMasternodeListChanged(NotifyMasternodeListChangedFn fn) override
{
return MakeHandler(
::uiInterface.NotifyMasternodeListChanged_connect([fn](const CDeterministicMNList& newList) {
fn(newList);
::uiInterface.NotifyMasternodeListChanged_connect([fn](const CDeterministicMNList& newList, const CBlockIndex* pindex) {
fn(newList, pindex);
}));
}
std::unique_ptr<Handler> handleNotifyAdditionalDataSyncProgressChanged(NotifyAdditionalDataSyncProgressChangedFn fn) override
Expand Down
17 changes: 10 additions & 7 deletions src/qt/clientmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,26 +82,29 @@ int ClientModel::getNumConnections(unsigned int flags) const
return m_node.getNodeCount(connections);
}

void ClientModel::setMasternodeList(const CDeterministicMNList& mnList)
void ClientModel::setMasternodeList(const CDeterministicMNList& mnList, const CBlockIndex* tip)
{
LOCK(cs_mnlinst);
if (mnListCached->GetBlockHash() == mnList.GetBlockHash()) {
return;
}
mnListCached = std::make_shared<CDeterministicMNList>(mnList);
mnListTip = tip;
Q_EMIT masternodeListChanged();
}

CDeterministicMNList ClientModel::getMasternodeList() const
std::pair<CDeterministicMNList, const CBlockIndex*> ClientModel::getMasternodeList() const
{
LOCK(cs_mnlinst);
return *mnListCached;
return {*mnListCached, mnListTip};
}

void ClientModel::refreshMasternodeList()
{
auto [mnList, tip] = m_node.evo().getListAtChainTip();

LOCK(cs_mnlinst);
setMasternodeList(m_node.evo().getListAtChainTip());
setMasternodeList(mnList, tip);
}

int ClientModel::getHeaderTipHeight() const
Expand Down Expand Up @@ -332,9 +335,9 @@ static void NotifyChainLock(ClientModel *clientmodel, const std::string& bestCha
assert(invoked);
}

static void NotifyMasternodeListChanged(ClientModel *clientmodel, const CDeterministicMNList& newList)
static void NotifyMasternodeListChanged(ClientModel *clientmodel, const CDeterministicMNList& newList, const CBlockIndex* pindex)
{
clientmodel->setMasternodeList(newList);
clientmodel->setMasternodeList(newList, pindex);
}

static void NotifyAdditionalDataSyncProgressChanged(ClientModel *clientmodel, double nSyncProgress)
Expand All @@ -355,7 +358,7 @@ void ClientModel::subscribeToCoreSignals()
m_handler_notify_block_tip = m_node.handleNotifyBlockTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, false));
m_handler_notify_chainlock = m_node.handleNotifyChainLock(std::bind(NotifyChainLock, this, std::placeholders::_1, std::placeholders::_2));
m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, true));
m_handler_notify_masternodelist_changed = m_node.handleNotifyMasternodeListChanged(std::bind(NotifyMasternodeListChanged, this, std::placeholders::_1));
m_handler_notify_masternodelist_changed = m_node.handleNotifyMasternodeListChanged(std::bind(NotifyMasternodeListChanged, this, std::placeholders::_1, std::placeholders::_2));
m_handler_notify_additional_data_sync_progess_changed = m_node.handleNotifyAdditionalDataSyncProgressChanged(std::bind(NotifyAdditionalDataSyncProgressChanged, this, std::placeholders::_1));
}

Expand Down
5 changes: 3 additions & 2 deletions src/qt/clientmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ class ClientModel : public QObject
int getHeaderTipHeight() const;
int64_t getHeaderTipTime() const;

void setMasternodeList(const CDeterministicMNList& mnList);
CDeterministicMNList getMasternodeList() const;
void setMasternodeList(const CDeterministicMNList& mnList, const CBlockIndex* tip);
std::pair<CDeterministicMNList, const CBlockIndex*> getMasternodeList() const;
void refreshMasternodeList();

void getAllGovernanceObjects(std::vector<CGovernanceObject> &obj);
Expand Down Expand Up @@ -119,6 +119,7 @@ class ClientModel : public QObject
// representation of the list in UI during initial sync/reindex, so we cache it here too.
mutable RecursiveMutex cs_mnlinst; // protects mnListCached
CDeterministicMNListPtr mnListCached;
const CBlockIndex* mnListTip;

void subscribeToCoreSignals();
void unsubscribeFromCoreSignals();
Expand Down
2 changes: 1 addition & 1 deletion src/qt/governancelist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ void GovernanceList::updateProposalList()
// A proposal is considered passing if (YES votes - NO votes) >= (Total Weight of Masternodes / 10),
// count total valid (ENABLED) masternodes to determine passing threshold.
// Need to query number of masternodes here with access to clientModel.
const int nWeightedMnCount = clientModel->getMasternodeList().GetValidWeightedMNsCount();
const int nWeightedMnCount = clientModel->getMasternodeList().first.GetValidWeightedMNsCount();
const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10);
proposalModel->setVotingParams(nAbsVoteReq);

Expand Down
16 changes: 11 additions & 5 deletions src/qt/masternodelist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,15 @@ void MasternodeList::updateDIP3List()
return;
}

auto mnList = clientModel->getMasternodeList();
auto [mnList, pindex] = clientModel->getMasternodeList();
auto projectedPayees = mnList.GetProjectedMNPayees(pindex);

if (projectedPayees.empty() && mnList.GetValidMNsCount() > 0) {
// GetProjectedMNPayees failed to provide results for a list with valid mns.
// Keep current list and let it try again later.
return;
}

std::map<uint256, CTxDestination> mapCollateralDests;

{
Expand All @@ -191,7 +199,6 @@ void MasternodeList::updateDIP3List()

nTimeUpdatedDIP3 = GetTime();

auto projectedPayees = mnList.GetProjectedMNPayeesAtChainTip();
std::map<uint256, int> nextPayments;
for (size_t i = 0; i < projectedPayees.size(); i++) {
const auto& dmn = projectedPayees[i];
Expand Down Expand Up @@ -222,7 +229,7 @@ void MasternodeList::updateDIP3List()
QByteArray addr_ba(reinterpret_cast<const char*>(addr_key.data()), addr_key.size());
QTableWidgetItem* addressItem = new CMasternodeListWidgetItem<QByteArray>(QString::fromStdString(dmn.pdmnState->addr.ToString()), addr_ba);
QTableWidgetItem* typeItem = new QTableWidgetItem(QString::fromStdString(std::string(GetMnType(dmn.nType).description)));
QTableWidgetItem* statusItem = new QTableWidgetItem(mnList.IsMNValid(dmn) ? tr("ENABLED") : (mnList.IsMNPoSeBanned(dmn) ? tr("POSE_BANNED") : tr("UNKNOWN")));
QTableWidgetItem* statusItem = new QTableWidgetItem(dmn.pdmnState->IsBanned() ? tr("POSE_BANNED") : tr("ENABLED"));
QTableWidgetItem* PoSeScoreItem = new CMasternodeListWidgetItem<int>(QString::number(dmn.pdmnState->nPoSePenalty), dmn.pdmnState->nPoSePenalty);
QTableWidgetItem* registeredItem = new CMasternodeListWidgetItem<int>(QString::number(dmn.pdmnState->nRegisteredHeight), dmn.pdmnState->nRegisteredHeight);
QTableWidgetItem* lastPaidItem = new CMasternodeListWidgetItem<int>(QString::number(dmn.pdmnState->nLastPaidHeight), dmn.pdmnState->nLastPaidHeight);
Expand Down Expand Up @@ -349,8 +356,7 @@ CDeterministicMNCPtr MasternodeList::GetSelectedDIP3MN()
uint256 proTxHash;
proTxHash.SetHex(strProTxHash);

auto mnList = clientModel->getMasternodeList();
return mnList.GetMN(proTxHash);
return clientModel->getMasternodeList().first.GetMN(proTxHash);;
}

void MasternodeList::extraInfoDIP3_clicked()
Expand Down
4 changes: 2 additions & 2 deletions src/qt/rpcconsole.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ void RPCConsole::updateMasternodeCount()
if (!clientModel) {
return;
}
auto mnList = clientModel->getMasternodeList();
auto mnList = clientModel->getMasternodeList().first;
size_t total_mn_count = mnList.GetAllMNsCount();
size_t total_enabled_mn_count = mnList.GetValidMNsCount();
size_t total_evo_count = mnList.GetAllEvoCount();
Expand Down Expand Up @@ -1247,7 +1247,7 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats)
ui->peerHeight->setText(QString::number(stats->nodeStats.nStartingHeight));
ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No"));
ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as != 0 ? QString::number(stats->nodeStats.m_mapped_as) : tr("N/A"));
auto dmn = clientModel->getMasternodeList().GetMNByService(stats->nodeStats.addr);
auto dmn = clientModel->getMasternodeList().first.GetMNByService(stats->nodeStats.addr);
if (dmn == nullptr) {
ui->peerNodeType->setText(tr("Regular"));
ui->peerPoSeScore->setText(tr("N/A"));
Expand Down
5 changes: 3 additions & 2 deletions src/rpc/masternode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ static UniValue masternode_count(const JSONRPCRequest& request)

static UniValue GetNextMasternodeForPayment(int heightShift)
{
auto mnList = deterministicMNManager->GetListAtChainTip();
auto payees = mnList.GetProjectedMNPayeesAtChainTip(heightShift);
const CBlockIndex *tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
auto mnList = deterministicMNManager->GetListForBlock(tip);
auto payees = mnList.GetProjectedMNPayees(tip, heightShift);
if (payees.empty())
return "unknown";
auto payee = payees.back();
Expand Down
2 changes: 1 addition & 1 deletion src/ui_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void CClientUIInterface::ShowProgress(const std::string& title, int nProgress, b
void CClientUIInterface::NotifyBlockTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(b, i); }
void CClientUIInterface::NotifyChainLock(const std::string& bestChainLockHash, int bestChainLockHeight) { return g_ui_signals.NotifyChainLock(bestChainLockHash, bestChainLockHeight); }
void CClientUIInterface::NotifyHeaderTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(b, i); }
void CClientUIInterface::NotifyMasternodeListChanged(const CDeterministicMNList& list) { return g_ui_signals.NotifyMasternodeListChanged(list); }
void CClientUIInterface::NotifyMasternodeListChanged(const CDeterministicMNList& list, const CBlockIndex* i) { return g_ui_signals.NotifyMasternodeListChanged(list, i); }
void CClientUIInterface::NotifyAdditionalDataSyncProgressChanged(double nSyncProgress) { return g_ui_signals.NotifyAdditionalDataSyncProgressChanged(nSyncProgress); }
void CClientUIInterface::BannedListChanged() { return g_ui_signals.BannedListChanged(); }

Expand Down
2 changes: 1 addition & 1 deletion src/ui_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class CClientUIInterface
ADD_SIGNALS_DECL_WRAPPER(NotifyHeaderTip, void, bool, const CBlockIndex*);

/** Masternode list has changed */
ADD_SIGNALS_DECL_WRAPPER(NotifyMasternodeListChanged, void, const CDeterministicMNList&);
ADD_SIGNALS_DECL_WRAPPER(NotifyMasternodeListChanged, void, const CDeterministicMNList&, const CBlockIndex*);

/** Additional data sync progress changed */
ADD_SIGNALS_DECL_WRAPPER(NotifyAdditionalDataSyncProgressChanged, void, double nSyncProgress);
Expand Down

0 comments on commit 2bb63f0

Please sign in to comment.