From 0f8cf34ce1c78d0ee88376bb8e6b2d118aeee35c Mon Sep 17 00:00:00 2001 From: Peter John Bushnell Date: Wed, 8 Feb 2023 10:45:56 +0000 Subject: [PATCH] Fix staker crash on invalidateblock (#1741) * Fix staker crash on rollback * Make GetTimelock return optional * Invert logic for require --- src/masternodes/masternodes.cpp | 18 +++++++++++++----- src/masternodes/masternodes.h | 2 +- src/masternodes/rpc_masternodes.cpp | 12 ++++++------ src/miner.cpp | 6 +++++- src/pos.cpp | 5 ++++- src/rpc/mining.cpp | 10 +++++----- 6 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/masternodes/masternodes.cpp b/src/masternodes/masternodes.cpp index e17d40346b..375277ca7f 100644 --- a/src/masternodes/masternodes.cpp +++ b/src/masternodes/masternodes.cpp @@ -325,7 +325,11 @@ Res CMasternodesView::ResignMasternode(CMasternode &node, const uint256 &nodeId, return Res::Err("node %s state is not 'PRE_ENABLED' or 'ENABLED'", nodeId.ToString()); } - Require(!GetTimelock(nodeId, node, height), "Trying to resign masternode before timelock expiration."); + const auto timelock = GetTimelock(nodeId, node, height); + if (!timelock) { + return Res::Err("Failed to get timelock for masternode"); + } + Require(timelock.value() == CMasternode::ZEROYEAR, "Trying to resign masternode before timelock expiration."); node.resignTx = txid; node.resignHeight = height; @@ -522,9 +526,8 @@ void CMasternodesView::EraseSubNodesLastBlockTime(const uint256 &nodeId, const u } } -uint16_t CMasternodesView::GetTimelock(const uint256 &nodeId, const CMasternode &node, const uint64_t height) const { - auto timelock = ReadBy(nodeId); - if (timelock) { +std::optional CMasternodesView::GetTimelock(const uint256 &nodeId, const CMasternode &node, const uint64_t height) const { + if (const auto timelock = ReadBy(nodeId); timelock) { LOCK(cs_main); // Get last height auto lastHeight = height - 1; @@ -540,7 +543,12 @@ uint16_t CMasternodesView::GetTimelock(const uint256 &nodeId, const CMasternode // Get average time of the last two times the activation delay worth of blocks uint64_t totalTime{0}; for (; lastHeight + Params().GetConsensus().mn.newResignDelay >= height; --lastHeight) { - totalTime += ::ChainActive()[lastHeight]->nTime; + const auto &blockIndex{::ChainActive()[lastHeight]}; + // Last height might not be available due to rollback or call to invalidateblock + if (!blockIndex) { + return {}; + } + totalTime += blockIndex->nTime; } const uint32_t averageTime = totalTime / Params().GetConsensus().mn.newResignDelay; diff --git a/src/masternodes/masternodes.h b/src/masternodes/masternodes.h index 80e4871fe6..08f86e3ede 100644 --- a/src/masternodes/masternodes.h +++ b/src/masternodes/masternodes.h @@ -263,7 +263,7 @@ class CMasternodesView : public virtual CStorageView { uint8_t{}, std::numeric_limits::max()}); - uint16_t GetTimelock(const uint256 &nodeId, const CMasternode &node, const uint64_t height) const; + std::optional GetTimelock(const uint256 &nodeId, const CMasternode &node, const uint64_t height) const; // tags struct ID { diff --git a/src/masternodes/rpc_masternodes.cpp b/src/masternodes/rpc_masternodes.cpp index 310686f1d1..39e22d47ca 100644 --- a/src/masternodes/rpc_masternodes.cpp +++ b/src/masternodes/rpc_masternodes.cpp @@ -45,15 +45,15 @@ UniValue mnToJSON(CCustomCSView& view, uint256 const & nodeId, CMasternode const } obj.pushKV("localMasternode", localMasternode); - uint16_t timelock = pcustomcsview->GetTimelock(nodeId, node, currentHeight); + const auto timelock = pcustomcsview->GetTimelock(nodeId, node, currentHeight); // Only get targetMultiplier for active masternodes - if (node.IsActive(currentHeight, view)) { + if (timelock && node.IsActive(currentHeight, view)) { // Get block times with next block as height - const auto subNodesBlockTime = pcustomcsview->GetBlockTimes(node.operatorAuthAddress, currentHeight + 1, node.creationHeight, timelock); + const auto subNodesBlockTime = pcustomcsview->GetBlockTimes(node.operatorAuthAddress, currentHeight + 1, node.creationHeight, *timelock); if (currentHeight >= Params().GetConsensus().EunosPayaHeight) { - const uint8_t loops = timelock == CMasternode::TENYEAR ? 4 : timelock == CMasternode::FIVEYEAR ? 3 : 2; + const uint8_t loops = *timelock == CMasternode::TENYEAR ? 4 : *timelock == CMasternode::FIVEYEAR ? 3 : 2; UniValue multipliers(UniValue::VARR); for (uint8_t i{0}; i < loops; ++i) { multipliers.push_back(pos::CalcCoinDayWeight(Params().GetConsensus(), GetTime(), subNodesBlockTime[i]).getdouble()); @@ -64,8 +64,8 @@ UniValue mnToJSON(CCustomCSView& view, uint256 const & nodeId, CMasternode const } } - if (timelock) { - obj.pushKV("timelock", strprintf("%d years", timelock / 52)); + if (timelock && *timelock) { + obj.pushKV("timelock", strprintf("%d years", *timelock / 52)); } /// @todo add unlock height and|or real resign height diff --git a/src/miner.cpp b/src/miner.cpp index 48cc0079de..2237a2aeb3 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -777,7 +777,11 @@ namespace pos { blockHeight = tip->nHeight + 1; creationHeight = int64_t(nodePtr->creationHeight); blockTime = std::max(tip->GetMedianTimePast() + 1, GetAdjustedTime()); - timelock = pcustomcsview->GetTimelock(masternodeID, *nodePtr, blockHeight); + const auto optTimeLock = pcustomcsview->GetTimelock(masternodeID, *nodePtr, blockHeight); + if (!optTimeLock) + return Status::stakeWaiting; + + timelock = *optTimeLock; // Get block times subNodesBlockTime = pcustomcsview->GetBlockTimes(args.operatorID, blockHeight, creationHeight, timelock); diff --git a/src/pos.cpp b/src/pos.cpp index 0a014a3935..d9f0290d33 100644 --- a/src/pos.cpp +++ b/src/pos.cpp @@ -75,7 +75,10 @@ bool ContextualCheckProofOfStake(const CBlockHeader& blockHeader, const Consensu creationHeight = int64_t(nodePtr->creationHeight); if (height >= params.EunosPayaHeight) { - timelock = mnView->GetTimelock(masternodeID, *nodePtr, height); + const auto optTimeLock = mnView->GetTimelock(masternodeID, *nodePtr, height); + if (!optTimeLock) + return false; + timelock = *optTimeLock; } // Check against EunosPayaHeight here for regtest, does not hurt other networks. diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 2729afa8f2..dc27f4421a 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -300,12 +300,12 @@ static UniValue getmininginfo(const JSONRPCRequest& request) const auto timelock = pcustomcsview->GetTimelock(mnId.second, *nodePtr, height); // Get targetMultiplier if node is active - if (nodePtr->IsActive(height, *pcustomcsview)) { + if (timelock && nodePtr->IsActive(height, *pcustomcsview)) { // Get block times - const auto subNodesBlockTime = pcustomcsview->GetBlockTimes(nodePtr->operatorAuthAddress, height, nodePtr->creationHeight, timelock); + const auto subNodesBlockTime = pcustomcsview->GetBlockTimes(nodePtr->operatorAuthAddress, height, nodePtr->creationHeight, *timelock); if (height >= Params().GetConsensus().EunosPayaHeight) { - const uint8_t loops = timelock == CMasternode::TENYEAR ? 4 : timelock == CMasternode::FIVEYEAR ? 3 : 2; + const uint8_t loops = *timelock == CMasternode::TENYEAR ? 4 : *timelock == CMasternode::FIVEYEAR ? 3 : 2; UniValue multipliers(UniValue::VARR); for (uint8_t i{0}; i < loops; ++i) { multipliers.push_back(pos::CalcCoinDayWeight(Params().GetConsensus(), GetTime(), subNodesBlockTime[i]).getdouble()); @@ -316,8 +316,8 @@ static UniValue getmininginfo(const JSONRPCRequest& request) } } - if (timelock) { - obj.pushKV("timelock", strprintf("%d years", timelock / 52)); + if (timelock && *timelock) { + obj.pushKV("timelock", strprintf("%d years", *timelock / 52)); } mnArr.push_back(subObj);