From 1d1d2726e979fff55589140d19534c3496370ea9 Mon Sep 17 00:00:00 2001
From: Anthony Fieroni <bvbfan@abv.bg>
Date: Thu, 2 Dec 2021 00:33:23 +0000
Subject: [PATCH] Well define masternode states (#936)

Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

Co-authored-by: Prasanna Loganathar <pvl@prasannavl.com>
---
 src/masternodes/anchors.cpp         |  4 ++--
 src/masternodes/masternodes.cpp     | 36 ++++++++++++++---------------
 src/masternodes/masternodes.h       |  6 ++---
 src/masternodes/mn_checks.cpp       |  2 +-
 src/masternodes/rpc_masternodes.cpp |  8 +++----
 src/miner.cpp                       | 11 ++++-----
 src/rpc/mining.cpp                  |  6 ++---
 src/validation.cpp                  |  2 +-
 8 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/src/masternodes/anchors.cpp b/src/masternodes/anchors.cpp
index 092161981c..cd88b9a54c 100644
--- a/src/masternodes/anchors.cpp
+++ b/src/masternodes/anchors.cpp
@@ -1011,7 +1011,7 @@ bool CAnchorAwaitingConfirms::Validate(CAnchorConfirmMessage const &confirmMessa
     }
 
     auto it = pcustomcsview->GetMasternodeIdByOperator(signer);
-    if (!it || !pcustomcsview->GetMasternode(*it)->IsActive()) {
+    if (!it || !pcustomcsview->GetMasternode(*it)->IsActive(height)) {
         LogPrint(BCLog::ANCHORING, "%s: Warning! Masternode with operator key %s does not exist or not active!\n", __func__, signer.ToString());
         return false;
     }
@@ -1062,7 +1062,7 @@ void CAnchorAwaitingConfirms::ReVote()
         return;
     }
 
-    const auto operatorDetails = AmISignerNow(currentTeam);
+    const auto operatorDetails = AmISignerNow(height, currentTeam);
 
     for (const auto& keys : operatorDetails) {
         CAnchorIndex::UnrewardedResult unrewarded = panchors->GetUnrewarded();
diff --git a/src/masternodes/masternodes.cpp b/src/masternodes/masternodes.cpp
index a9be5ab7bc..2810375627 100644
--- a/src/masternodes/masternodes.cpp
+++ b/src/masternodes/masternodes.cpp
@@ -86,16 +86,15 @@ CMasternode::CMasternode()
 {
 }
 
-CMasternode::State CMasternode::GetState() const
-{
-    return GetState(::ChainActive().Height());
-}
-
 CMasternode::State CMasternode::GetState(int height) const
 {
     int EunosPayaHeight = Params().GetConsensus().EunosPayaHeight;
 
-    if (resignHeight == -1) { // enabled or pre-enabled
+    if (height < creationHeight) {
+        return State::UNKNOWN;
+    }
+
+    if (resignHeight == -1 || height < resignHeight) { // enabled or pre-enabled
         // Special case for genesis block
         int activationDelay = height < EunosPayaHeight ? GetMnActivationDelay(height) : GetMnActivationDelay(creationHeight);
         if (creationHeight == 0 || height >= creationHeight + activationDelay) {
@@ -114,11 +113,6 @@ CMasternode::State CMasternode::GetState(int height) const
     return State::UNKNOWN;
 }
 
-bool CMasternode::IsActive() const
-{
-    return IsActive(::ChainActive().Height());
-}
-
 bool CMasternode::IsActive(int height) const
 {
     State state = GetState(height);
@@ -742,7 +736,7 @@ void CCustomCSView::SetDbVersion(int version)
     Write(DbVersion::prefix(), version);
 }
 
-CTeamView::CTeam CCustomCSView::CalcNextTeam(const uint256 & stakeModifier)
+CTeamView::CTeam CCustomCSView::CalcNextTeam(int height, const uint256 & stakeModifier)
 {
     if (stakeModifier == uint256())
         return Params().GetGenesisTeam();
@@ -750,8 +744,8 @@ CTeamView::CTeam CCustomCSView::CalcNextTeam(const uint256 & stakeModifier)
     int anchoringTeamSize = Params().GetConsensus().mn.anchoringTeamSize;
 
     std::map<arith_uint256, CKeyID, std::less<arith_uint256>> priorityMN;
-    ForEachMasternode([&stakeModifier, &priorityMN] (uint256 const & id, CMasternode node) {
-        if(!node.IsActive())
+    ForEachMasternode([&] (uint256 const & id, CMasternode node) {
+        if(!node.IsActive(height))
             return true;
 
         CDataStream ss{SER_GETHASH, PROTOCOL_VERSION};
@@ -790,8 +784,8 @@ void CCustomCSView::CalcAnchoringTeams(const uint256 & stakeModifier, const CBlo
 
     std::map<arith_uint256, CKeyID, std::less<arith_uint256>> authMN;
     std::map<arith_uint256, CKeyID, std::less<arith_uint256>> confirmMN;
-    ForEachMasternode([&masternodeIDs, &stakeModifier, &authMN, &confirmMN] (uint256 const & id, CMasternode node) {
-        if(!node.IsActive())
+    ForEachMasternode([&] (uint256 const & id, CMasternode node) {
+        if(!node.IsActive(pindexNew->height))
             return true;
 
         // Not in our list of MNs from last week, skip.
@@ -1065,7 +1059,7 @@ uint256 CCustomCSView::MerkleRoot() {
     return ComputeMerkleRoot(std::move(hashes));
 }
 
-std::map<CKeyID, CKey> AmISignerNow(CAnchorData::CTeam const & team)
+std::map<CKeyID, CKey> AmISignerNow(int height, CAnchorData::CTeam const & team)
 {
     AssertLockHeld(cs_main);
 
@@ -1073,8 +1067,12 @@ std::map<CKeyID, CKey> AmISignerNow(CAnchorData::CTeam const & team)
     auto const mnIds = pcustomcsview->GetOperatorsMulti();
     for (const auto& mnId : mnIds)
     {
-        if (pcustomcsview->GetMasternode(mnId.second)->IsActive() && team.find(mnId.first) != team.end())
-        {
+        auto node = pcustomcsview->GetMasternode(mnId.second);
+        if (!node) {
+            continue;
+        }
+
+        if (node->IsActive(height) && team.find(mnId.first) != team.end()) {
             CKey masternodeKey;
             std::vector<std::shared_ptr<CWallet>> wallets = GetWallets();
             for (auto const & wallet : wallets) {
diff --git a/src/masternodes/masternodes.h b/src/masternodes/masternodes.h
index 35ca2b27ee..50623bc3f0 100644
--- a/src/masternodes/masternodes.h
+++ b/src/masternodes/masternodes.h
@@ -95,9 +95,7 @@ class CMasternode
     //! empty constructor
     CMasternode();
 
-    State GetState() const;
     State GetState(int height) const;
-    bool IsActive() const;
     bool IsActive(int height) const;
 
     static std::string GetHumanReadableState(State state);
@@ -414,7 +412,7 @@ class CCustomCSView
     }
 
     // cause depends on current mns:
-    CTeamView::CTeam CalcNextTeam(uint256 const & stakeModifier);
+    CTeamView::CTeam CalcNextTeam(int height, uint256 const & stakeModifier);
 
     // Generate auth and custom anchor teams based on current block
     void CalcAnchoringTeams(uint256 const & stakeModifier, const CBlockIndex *pindexNew);
@@ -447,7 +445,7 @@ class CCustomCSView
     struct DbVersion { static constexpr uint8_t prefix() { return 'D'; } };
 };
 
-std::map<CKeyID, CKey> AmISignerNow(CAnchorData::CTeam const & team);
+std::map<CKeyID, CKey> AmISignerNow(int height, CAnchorData::CTeam const & team);
 
 /** Global DB and view that holds enhanced chainstate data (should be protected by cs_main) */
 extern std::unique_ptr<CStorageLevelDB> pcustomcsDB;
diff --git a/src/masternodes/mn_checks.cpp b/src/masternodes/mn_checks.cpp
index 752419503c..a9d7215ae8 100644
--- a/src/masternodes/mn_checks.cpp
+++ b/src/masternodes/mn_checks.cpp
@@ -3245,7 +3245,7 @@ ResVal<uint256> ApplyAnchorRewardTx(CCustomCSView & mnview, CTransaction const &
         return Res::ErrDbg("bad-ar-curteam", "anchor wrong current team");
     }
 
-    if (finMsg.nextTeam != mnview.CalcNextTeam(prevStakeModifier)) {
+    if (finMsg.nextTeam != mnview.CalcNextTeam(height, prevStakeModifier)) {
         return Res::ErrDbg("bad-ar-nextteam", "anchor wrong next team");
     }
     mnview.SetTeam(finMsg.nextTeam);
diff --git a/src/masternodes/rpc_masternodes.cpp b/src/masternodes/rpc_masternodes.cpp
index 556013e64c..618813021b 100644
--- a/src/masternodes/rpc_masternodes.cpp
+++ b/src/masternodes/rpc_masternodes.cpp
@@ -6,8 +6,9 @@
 UniValue mnToJSON(uint256 const & nodeId, CMasternode const& node, bool verbose, const std::set<std::pair<CKeyID, uint256>>& mnIds, const CWallet* pwallet)
 {
     UniValue ret(UniValue::VOBJ);
+    auto currentHeight = ChainActive().Height();
     if (!verbose) {
-        ret.pushKV(nodeId.GetHex(), CMasternode::GetHumanReadableState(node.GetState()));
+        ret.pushKV(nodeId.GetHex(), CMasternode::GetHumanReadableState(node.GetState(currentHeight)));
     }
     else {
         UniValue obj(UniValue::VOBJ);
@@ -30,7 +31,7 @@ UniValue mnToJSON(uint256 const & nodeId, CMasternode const& node, bool verbose,
         obj.pushKV("resignHeight", node.resignHeight);
         obj.pushKV("resignTx", node.resignTx.GetHex());
         obj.pushKV("banTx", node.banTx.GetHex());
-        obj.pushKV("state", CMasternode::GetHumanReadableState(node.GetState()));
+        obj.pushKV("state", CMasternode::GetHumanReadableState(node.GetState(currentHeight)));
         obj.pushKV("mintedBlocks", (uint64_t) node.mintedBlocks);
         isminetype ownerMine = IsMineCached(*pwallet, ownerDest);
         obj.pushKV("ownerIsMine", bool(ownerMine & ISMINE_SPENDABLE));
@@ -44,11 +45,10 @@ UniValue mnToJSON(uint256 const & nodeId, CMasternode const& node, bool verbose,
         }
         obj.pushKV("localMasternode", localMasternode);
 
-        auto currentHeight = ChainActive().Height();
         uint16_t timelock = pcustomcsview->GetTimelock(nodeId, node, currentHeight);
 
         // Only get targetMultiplier for active masternodes
-        if (node.IsActive()) {
+        if (node.IsActive(currentHeight)) {
             // Get block times with next block as height
             const auto subNodesBlockTime = pcustomcsview->GetBlockTimes(node.operatorAuthAddress, currentHeight + 1, node.creationHeight, timelock);
 
diff --git a/src/miner.cpp b/src/miner.cpp
index 9587adea08..1447e41c7a 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -112,6 +112,9 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     pblocktemplate->vTxSigOpsCost.push_back(-1); // updated at end
 
     LOCK2(cs_main, mempool.cs);
+    CBlockIndex* pindexPrev = ::ChainActive().Tip();
+    assert(pindexPrev != nullptr);
+    nHeight = pindexPrev->nHeight + 1;
     // in fact, this may be redundant cause it was checked upthere in the miner
     boost::optional<std::pair<CKeyID, uint256>> myIDs;
     if (!blockTime) {
@@ -119,15 +122,11 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
         if (!myIDs)
             return nullptr;
         auto nodePtr = pcustomcsview->GetMasternode(myIDs->second);
-        if (!nodePtr || !nodePtr->IsActive())
+        if (!nodePtr || !nodePtr->IsActive(nHeight))
             return nullptr;
     }
 
-    CBlockIndex* pindexPrev = ::ChainActive().Tip();
-    assert(pindexPrev != nullptr);
-    nHeight = pindexPrev->nHeight + 1;
     auto consensus = chainparams.GetConsensus();
-
     pblock->nVersion = ComputeBlockVersion(pindexPrev, consensus);
     // -regtest only: allow overriding block.nVersion with
     // -blockversion=N to test forking scenarios
@@ -722,7 +721,7 @@ namespace pos {
             tip = ::ChainActive().Tip();
             masternodeID = *optMasternodeID;
             auto nodePtr = pcustomcsview->GetMasternode(masternodeID);
-            if (!nodePtr || !nodePtr->IsActive(tip->height)) /// @todo miner: height+1 or nHeight+1 ???
+            if (!nodePtr || !nodePtr->IsActive(tip->height + 1))
             {
                 /// @todo may be new status for not activated (or already resigned) MN??
                 return Status::initWaiting;
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 014830ce4e..ef2d4605d7 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -281,12 +281,12 @@ static UniValue getmininginfo(const JSONRPCRequest& request)
             //should not come here if the database has correct data.
             throw JSONRPCError(RPC_DATABASE_ERROR, strprintf("The masternode %s does not exist", mnId.second.GetHex()));
         }
-        auto state = nodePtr->GetState();
+        auto state = nodePtr->GetState(height);
         CTxDestination operatorDest = nodePtr->operatorType == 1 ? CTxDestination(PKHash(nodePtr->operatorAuthAddress)) :
                                       CTxDestination(WitnessV0KeyHash(nodePtr->operatorAuthAddress));
         subObj.pushKV("operator", EncodeDestination(operatorDest));// NOTE(sp) : Should this also be encoded? not the HEX
         subObj.pushKV("state", CMasternode::GetHumanReadableState(state));
-        auto generate = nodePtr->IsActive() && genCoins;
+        auto generate = nodePtr->IsActive(height) && genCoins;
         subObj.pushKV("generate", generate);
         subObj.pushKV("mintedblocks", (uint64_t)nodePtr->mintedBlocks);
 
@@ -302,7 +302,7 @@ static UniValue getmininginfo(const JSONRPCRequest& request)
         const auto timelock = pcustomcsview->GetTimelock(mnId.second, *nodePtr, height);
 
         // Get targetMultiplier if node is active
-        if (nodePtr->IsActive()) {
+        if (nodePtr->IsActive(height)) {
             // Get block times
             const auto subNodesBlockTime = pcustomcsview->GetBlockTimes(nodePtr->operatorAuthAddress, height, nodePtr->creationHeight, timelock);
 
diff --git a/src/validation.cpp b/src/validation.cpp
index b9c848ed33..2649cc2acd 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5088,7 +5088,7 @@ void ProcessAuthsIfTipChanged(CBlockIndex const * oldTip, CBlockIndex const * ti
     }
 
     // masternode key and operator auth address
-    auto operatorDetails = AmISignerNow(team);
+    auto operatorDetails = AmISignerNow(tip->nHeight, team);
 
     if (operatorDetails.empty()) {
         return;