Fix staker crash on invalidateblock (#1741)

* Fix staker crash on rollback * Make GetTimelock return optional * Invert logic for require
DeFiCh · Feb 8, 2023 · 0f8cf34 · 0f8cf34
1 parent d2eaa8a
commit 0f8cf34
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 19 deletions.
diff --git 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<Timelock, uint16_t>(nodeId);
-    if (timelock) {
+std::optional<uint16_t> CMasternodesView::GetTimelock(const uint256 &nodeId, const CMasternode &node, const uint64_t height) const {
+    if (const auto timelock = ReadBy<Timelock, uint16_t>(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
@@ -263,7 +263,7 @@ class CMasternodesView : public virtual CStorageView {
                                                             uint8_t{},
                                                             std::numeric_limits<uint32_t>::max()});
 
-    uint16_t GetTimelock(const uint256 &nodeId, const CMasternode &node, const uint64_t height) const;
+    std::optional<uint16_t> 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
@@ -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
@@ -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
@@ -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
@@ -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);