Update activemn if protx info changed (#3176)

* Update activemn if protx info changed

* Add `==` and `!=` operators to CDeterministicMNState

* Only re-init active MN if its IP changed, changes to payout, voting etc. can be done without it

* Test `masternode status` updates

* Don't track mnListEntry anymore and instead get the DMN on demand

* Revert "Add `==` and `!=` operators to CDeterministicMNState"

This reverts commit fba4687.

Author: nmarley

src/masternode/activemasternode.cpp

@@ -26,6 +26,8 @@ std::string CActiveMasternodeManager::GetStateString() const
         return "REMOVED";
     case MASTERNODE_OPERATOR_KEY_CHANGED:
         return "OPERATOR_KEY_CHANGED";
+    case MASTERNODE_PROTX_IP_CHANGED:
+        return "PROTX_IP_CHANGED";
     case MASTERNODE_READY:
         return "READY";
     case MASTERNODE_ERROR:
@@ -46,6 +48,8 @@ std::string CActiveMasternodeManager::GetStatus() const
         return "Masternode removed from list";
     case MASTERNODE_OPERATOR_KEY_CHANGED:
         return "Operator key changed or revoked";
+    case MASTERNODE_PROTX_IP_CHANGED:
+        return "IP address specified in ProTx changed";
     case MASTERNODE_READY:
         return "Ready";
     case MASTERNODE_ERROR:
@@ -94,11 +98,9 @@ void CActiveMasternodeManager::Init()
         return;
     }
 
-    mnListEntry = dmn;
+    LogPrintf("CActiveMasternodeManager::Init -- proTxHash=%s, proTx=%s\n", dmn->proTxHash.ToString(), dmn->ToString());
 
-    LogPrintf("CActiveMasternodeManager::Init -- proTxHash=%s, proTx=%s\n", mnListEntry->proTxHash.ToString(), mnListEntry->ToString());
-
-    if (activeMasternodeInfo.service != mnListEntry->pdmnState->addr) {
+    if (activeMasternodeInfo.service != dmn->pdmnState->addr) {
         state = MASTERNODE_ERROR;
         strError = "Local address does not match the address from ProTx";
         LogPrintf("CActiveMasternodeManager::Init -- ERROR: %s", strError);
@@ -120,8 +122,8 @@ void CActiveMasternodeManager::Init()
         }
     }
 
-    activeMasternodeInfo.proTxHash = mnListEntry->proTxHash;
-    activeMasternodeInfo.outpoint = mnListEntry->collateralOutpoint;
+    activeMasternodeInfo.proTxHash = dmn->proTxHash;
+    activeMasternodeInfo.outpoint = dmn->collateralOutpoint;
     state = MASTERNODE_READY;
 }
 
@@ -134,24 +136,41 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con
     if (!deterministicMNManager->IsDIP3Enforced(pindexNew->nHeight)) return;
 
     if (state == MASTERNODE_READY) {
-        auto mnList = deterministicMNManager->GetListForBlock(pindexNew);
-        if (!mnList.IsMNValid(mnListEntry->proTxHash)) {
+        auto oldMNList = deterministicMNManager->GetListForBlock(pindexNew->pprev);
+        auto newMNList = deterministicMNManager->GetListForBlock(pindexNew);
+        if (!newMNList.IsMNValid(activeMasternodeInfo.proTxHash)) {
             // MN disappeared from MN list
             state = MASTERNODE_REMOVED;
             activeMasternodeInfo.proTxHash = uint256();
             activeMasternodeInfo.outpoint.SetNull();
             // MN might have reappeared in same block with a new ProTx
             Init();
-        } else if (mnList.GetMN(mnListEntry->proTxHash)->pdmnState->pubKeyOperator != mnListEntry->pdmnState->pubKeyOperator) {
+            return;
+        }
+
+        auto oldDmn = oldMNList.GetMN(activeMasternodeInfo.proTxHash);
+        auto newDmn = newMNList.GetMN(activeMasternodeInfo.proTxHash);
+        if (newDmn->pdmnState->pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) {
             // MN operator key changed or revoked
             state = MASTERNODE_OPERATOR_KEY_CHANGED;
             activeMasternodeInfo.proTxHash = uint256();
             activeMasternodeInfo.outpoint.SetNull();
             // MN might have reappeared in same block with a new ProTx
             Init();
+            return;
+        }
+
+        if (newDmn->pdmnState->addr != oldDmn->pdmnState->addr) {
+            // MN IP changed
+            state = MASTERNODE_PROTX_IP_CHANGED;
+            activeMasternodeInfo.proTxHash = uint256();
+            activeMasternodeInfo.outpoint.SetNull();
+            Init();
+            return;
         }
     } else {
-        // MN might have (re)appeared with a new ProTx or we've found some peers and figured out our local address
+        // MN might have (re)appeared with a new ProTx or we've found some peers
+        // and figured out our local address
         Init();
     }
 }

src/masternode/activemasternode.h

@@ -40,12 +40,12 @@ class CActiveMasternodeManager : public CValidationInterface
         MASTERNODE_POSE_BANNED,
         MASTERNODE_REMOVED,
         MASTERNODE_OPERATOR_KEY_CHANGED,
+        MASTERNODE_PROTX_IP_CHANGED,
         MASTERNODE_READY,
         MASTERNODE_ERROR,
     };
 
 private:
-    CDeterministicMNCPtr mnListEntry;
     masternode_state_t state{MASTERNODE_WAITING_FOR_PROTX};
     std::string strError;
 
@@ -54,8 +54,6 @@ class CActiveMasternodeManager : public CValidationInterface
 
     void Init();
 
-    CDeterministicMNCPtr GetDMN() const { return mnListEntry; }
-
     std::string GetStateString() const;
     std::string GetStatus() const;
 

src/rpc/masternode.cpp

@@ -402,7 +402,7 @@ UniValue masternode_status(const JSONRPCRequest& request)
     mnObj.push_back(Pair("outpoint", activeMasternodeInfo.outpoint.ToStringShort()));
     mnObj.push_back(Pair("service", activeMasternodeInfo.service.ToString()));
 
-    auto dmn = activeMasternodeManager->GetDMN();
+    auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(activeMasternodeInfo.proTxHash);
     if (dmn) {
         mnObj.push_back(Pair("proTxHash", dmn->proTxHash.ToString()));
         mnObj.push_back(Pair("collateralHash", dmn->collateralOutpoint.hash.ToString()));

test/functional/dip3-deterministicmns.py

@@ -184,6 +184,23 @@ def run_test(self):
             self.start_mn(new_mn)
             self.sync_all()
 
+        self.log.info("testing masternode status updates")
+        # change voting address and see if changes are reflected in `masternode status` rpc output
+        mn = mns[0]
+        node = self.nodes[0]
+        old_dmnState = mn.node.masternode("status")["dmnState"]
+        old_voting_address = old_dmnState["votingAddress"]
+        new_voting_address = node.getnewaddress()
+        assert(old_voting_address != new_voting_address)
+        # also check if funds from payout address are used when no fee source address is specified
+        node.sendtoaddress(mn.rewards_address, 0.001)
+        node.protx('update_registrar', mn.protx_hash, "", new_voting_address, mn.rewards_address)
+        node.generate(1)
+        self.sync_all()
+        new_dmnState = mn.node.masternode("status")["dmnState"]
+        new_voting_address_from_rpc = new_dmnState["votingAddress"]
+        assert(new_voting_address_from_rpc == new_voting_address)
+
     def prepare_mn(self, node, idx, alias):
         mn = Masternode()
         mn.idx = idx