From 30f3f509282632370bc2dfa7f365cbf2ce318a18 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Mon, 9 Oct 2023 19:14:51 +0300
Subject: [PATCH] fix: Let CDeterministicMN::ToJson() return correct
 `collateralAddress` for spent collaterals (#5607)

## Issue being fixed or feature implemented
Historical masternode data returned via rpcs like `protx listdiff` can
be broken because some collaterals might be spent already and
`GetUTXOCoin` wasn't able to get any info.

## What was done?
Use `GetTransaction` as a fallback.

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---
 src/evo/deterministicmns.cpp                    | 17 +++++++++++------
 .../functional/feature_dip3_deterministicmns.py |  4 ++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp
index e99c711055..87df855cae 100644
--- a/src/evo/deterministicmns.cpp
+++ b/src/evo/deterministicmns.cpp
@@ -50,12 +50,17 @@ UniValue CDeterministicMN::ToJson() const
     obj.pushKV("collateralHash", collateralOutpoint.hash.ToString());
     obj.pushKV("collateralIndex", (int)collateralOutpoint.n);
 
-    Coin coin;
-    if (GetUTXOCoin(collateralOutpoint, coin)) {
-        CTxDestination dest;
-        if (ExtractDestination(coin.out.scriptPubKey, dest)) {
-            obj.pushKV("collateralAddress", EncodeDestination(dest));
-        }
+    CScript scriptPubKey;
+    if (Coin coin; GetUTXOCoin(collateralOutpoint, coin)) {
+        scriptPubKey = coin.out.scriptPubKey;
+    } else {
+        uint256 tmpHashBlock;
+        CTransactionRef collateralTx = GetTransaction(/* block_index */ nullptr,  /* mempool */ nullptr, collateralOutpoint.hash, Params().GetConsensus(), tmpHashBlock);
+        scriptPubKey = collateralTx->vout[collateralOutpoint.n].scriptPubKey;
+    }
+    CTxDestination dest;
+    if (ExtractDestination(scriptPubKey, dest)) {
+        obj.pushKV("collateralAddress", EncodeDestination(dest));
     }
 
     obj.pushKV("operatorReward", (double)nOperatorReward / 100);
diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py
index 9a39ab1040..357901121b 100755
--- a/test/functional/feature_dip3_deterministicmns.py
+++ b/test/functional/feature_dip3_deterministicmns.py
@@ -112,6 +112,8 @@ class DIP3Test(BitcoinTestFramework):
         spend_mns_count = 3
         mns_tmp = [] + mns
         dummy_txins = []
+        old_tip = self.nodes[0].getblockcount()
+        old_listdiff = self.nodes[0].protx("listdiff", 1, old_tip)
         for i in range(spend_mns_count):
             dummy_txin = self.spend_mn_collateral(mns[i], with_dummy_input_output=True)
             dummy_txins.append(dummy_txin)
@@ -119,6 +121,8 @@ class DIP3Test(BitcoinTestFramework):
             self.sync_all()
             mns_tmp.remove(mns[i])
             self.assert_mnlists(mns_tmp)
+        new_listdiff = self.nodes[0].protx("listdiff", 1, old_tip)
+        assert_equal(new_listdiff, old_listdiff)
 
         self.log.info("test that reverting the blockchain on a single node results in the mnlist to be reverted as well")
         for i in range(spend_mns_count):