Properly handle conflicts between ChainLocks and InstantSend (#2904)

* Move code to write archived ISLOCKs into its own method

We'll need this from another method as well later.

* Return ISLOCK instead of conflicting txid in GetConflictingTx/GetConflictingLock

* Implement GetInstantSendLocksByParent and RemoveChainedInstantSendLocks

These allow to easily delete multiple chains (actually trees) of ISLOCKs
in one go.

* Implement RemoveConflictedTx and call it from RemoveMempoolConflictsForLock

Also add "retryChildren" parameter to RemoveNonLockedTx so that we can
skip retrying of non-locked children TXs.

* Properly handle/remove conflicted TXs (between mempool and new blocks)

* Track non-locked TXs by inputs

* Implement and call ResolveBlockConflicts

* Also call ResolveBlockConflicts from ConnectBlock

But only when a block is known to have a conflict and at the same time is
ChainLocked, which causes the ISLOCK to be pruned.

* Split out RemoveChainLockConflictingLock from ResolveBlockConflicts

* Implement "quorum getrecsig" RPC

* Include decoded TX data in result of create_raw_tx

* Implement support for CLSIG in mininode.py

* Fix condition for update of nonLockedTxs.pindexMined

* Only add entries to nonLockedTxsByInputs when AddNonLockedTx is called for the first time

* Implement support for ISLOCK in mininode.py

* Implement tests for ChainLock vs InstantSend lock conflict resolution

* Handle review comment

Bail out (continue) early

qa/pull-tester/rpc-tests.py

@@ -47,6 +47,7 @@ BASE_SCRIPTS= [
     'llmq-signing.py', # NOTE: needs dash_hash to pass
     'llmq-chainlocks.py', # NOTE: needs dash_hash to pass
     'llmq-simplepose.py', # NOTE: needs dash_hash to pass
+    'llmq-is-cl-conflicts.py', # NOTE: needs dash_hash to pass
     'dip4-coinbasemerkleroots.py', # NOTE: needs dash_hash to pass
     # vv Tests less than 60s vv
     'sendheaders.py', # NOTE: needs dash_hash to pass

qa/rpc-tests/llmq-is-cl-conflicts.py

@@ -0,0 +1,338 @@
+#!/usr/bin/env python3
+# Copyright (c) 2015-2018 The Dash Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+from test_framework.blocktools import get_masternode_payment, create_coinbase, create_block
+from test_framework.mininode import *
+from test_framework.test_framework import DashTestFramework
+from test_framework.util import *
+from time import *
+
+'''
+llmq-is-cl-conflicts.py
+
+Checks conflict handling between ChainLocks and InstantSend
+
+'''
+
+class TestNode(SingleNodeConnCB):
+    def __init__(self):
+        SingleNodeConnCB.__init__(self)
+        self.clsigs = {}
+        self.islocks = {}
+
+    def send_clsig(self, clsig):
+        hash = uint256_from_str(hash256(clsig.serialize()))
+        self.clsigs[hash] = clsig
+
+        inv = msg_inv([CInv(29, hash)])
+        self.send_message(inv)
+
+    def send_islock(self, islock):
+        hash = uint256_from_str(hash256(islock.serialize()))
+        self.islocks[hash] = islock
+
+        inv = msg_inv([CInv(30, hash)])
+        self.send_message(inv)
+
+    def on_getdata(self, conn, message):
+        for inv in message.inv:
+            if inv.hash in self.clsigs:
+                self.send_message(self.clsigs[inv.hash])
+            if inv.hash in self.islocks:
+                self.send_message(self.islocks[inv.hash])
+
+
+class LLMQ_IS_CL_Conflicts(DashTestFramework):
+    def __init__(self):
+        super().__init__(6, 5, [], fast_dip3_enforcement=True)
+        #disable_mocktime()
+
+    def run_test(self):
+
+        while self.nodes[0].getblockchaininfo()["bip9_softforks"]["dip0008"]["status"] != "active":
+            self.nodes[0].generate(10)
+        sync_blocks(self.nodes, timeout=60*5)
+
+        self.test_node = TestNode()
+        self.test_node.add_connection(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], self.test_node))
+        NetworkThread().start()  # Start up network handling in another thread
+        self.test_node.wait_for_verack()
+
+        self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 0)
+        self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0)
+        self.wait_for_sporks_same()
+
+        self.mine_quorum()
+
+        # mine single block, wait for chainlock
+        self.nodes[0].generate(1)
+        self.wait_for_chainlock_tip_all_nodes()
+
+        self.test_chainlock_overrides_islock(False)
+        self.test_chainlock_overrides_islock(True)
+        self.test_islock_overrides_nonchainlock()
+
+    def test_chainlock_overrides_islock(self, test_block_conflict):
+        # create three raw TXs, they will conflict with each other
+        rawtx1 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex']
+        rawtx2 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex']
+        rawtx3 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex']
+        rawtx1_obj = FromHex(CTransaction(), rawtx1)
+        rawtx2_obj = FromHex(CTransaction(), rawtx2)
+        rawtx3_obj = FromHex(CTransaction(), rawtx3)
+
+        rawtx1_txid = self.nodes[0].sendrawtransaction(rawtx1)
+        rawtx2_txid = encode(hash256(hex_str_to_bytes(rawtx2))[::-1], 'hex_codec').decode('ascii')
+        rawtx3_txid = encode(hash256(hex_str_to_bytes(rawtx3))[::-1], 'hex_codec').decode('ascii')
+
+        # Create a chained TX on top of tx1
+        inputs = []
+        n = 0
+        for out in rawtx1_obj.vout:
+            if out.nValue == 100000000:
+                inputs.append({"txid": rawtx1_txid, "vout": n})
+            n += 1
+        rawtx4 = self.nodes[0].createrawtransaction(inputs, {self.nodes[0].getnewaddress(): 0.999})
+        rawtx4 = self.nodes[0].signrawtransaction(rawtx4)['hex']
+        rawtx4_txid = self.nodes[0].sendrawtransaction(rawtx4)
+
+        for node in self.nodes:
+            self.wait_for_instantlock(rawtx1_txid, node)
+            self.wait_for_instantlock(rawtx4_txid, node)
+
+        block = self.create_block(self.nodes[0], [rawtx2_obj])
+        if test_block_conflict:
+            submit_result = self.nodes[0].submitblock(ToHex(block))
+            assert(submit_result == "conflict-tx-lock")
+
+        cl = self.create_chainlock(self.nodes[0].getblockcount() + 1, block.sha256)
+        self.test_node.send_clsig(cl)
+
+        # Give the CLSIG some time to propagate. We unfortunately can't check propagation here as "getblock/getblockheader"
+        # is required to check for CLSIGs, but this requires the block header to be propagated already
+        sleep(1)
+
+        # The block should get accepted now, and at the same time prune the conflicting ISLOCKs
+        submit_result = self.nodes[1].submitblock(ToHex(block))
+        if test_block_conflict:
+            assert(submit_result == "duplicate")
+        else:
+            assert(submit_result is None)
+
+        for node in self.nodes:
+            self.wait_for_chainlock(node, "%064x" % block.sha256)
+
+        # Create a chained TX on top of tx2
+        inputs = []
+        n = 0
+        for out in rawtx2_obj.vout:
+            if out.nValue == 100000000:
+                inputs.append({"txid": rawtx2_txid, "vout": n})
+            n += 1
+        rawtx5 = self.nodes[0].createrawtransaction(inputs, {self.nodes[0].getnewaddress(): 0.999})
+        rawtx5 = self.nodes[0].signrawtransaction(rawtx5)['hex']
+        rawtx5_txid = self.nodes[0].sendrawtransaction(rawtx5)
+        for node in self.nodes:
+            self.wait_for_instantlock(rawtx5_txid, node)
+
+        # Lets verify that the ISLOCKs got pruned
+        for node in self.nodes:
+            assert_raises_jsonrpc(-5, "No such mempool or blockchain transaction", node.getrawtransaction, rawtx1_txid, True)
+            assert_raises_jsonrpc(-5, "No such mempool or blockchain transaction", node.getrawtransaction, rawtx4_txid, True)
+            rawtx = node.getrawtransaction(rawtx2_txid, True)
+            assert(rawtx['chainlock'])
+            assert(rawtx['instantlock'])
+            assert(not rawtx['instantlock_internal'])
+
+    def test_islock_overrides_nonchainlock(self):
+        # create two raw TXs, they will conflict with each other
+        rawtx1 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex']
+        rawtx2 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex']
+
+        rawtx1_txid = encode(hash256(hex_str_to_bytes(rawtx1))[::-1], 'hex_codec').decode('ascii')
+        rawtx2_txid = encode(hash256(hex_str_to_bytes(rawtx2))[::-1], 'hex_codec').decode('ascii')
+
+        # Create an ISLOCK but don't broadcast it yet
+        islock = self.create_islock(rawtx2)
+
+        # Stop enough MNs so that ChainLocks don't work anymore
+        for i in range(3):
+            self.stop_node(len(self.nodes) - 1)
+            self.nodes.pop(len(self.nodes) - 1)
+            self.mninfo.pop(len(self.mninfo) - 1)
+
+        # Send tx1, which will later conflict with the ISLOCK
+        self.nodes[0].sendrawtransaction(rawtx1)
+
+        # fast forward 11 minutes, so that the TX is considered safe and included in the next block
+        set_mocktime(get_mocktime() + int(60 * 11))
+        set_node_times(self.nodes, get_mocktime())
+
+        # Mine the conflicting TX into a block
+        good_tip = self.nodes[0].getbestblockhash()
+        self.nodes[0].generate(2)
+        self.sync_all()
+
+        # Assert that the conflicting tx got mined and the locked TX is not valid
+        assert(self.nodes[0].getrawtransaction(rawtx1_txid, True)['confirmations'] > 0)
+        assert_raises_jsonrpc(-25, "Missing inputs", self.nodes[0].sendrawtransaction, rawtx2)
+
+        # Send the ISLOCK, which should result in the last 2 blocks to be invalidated, even though the nodes don't know
+        # the locked transaction yet
+        self.test_node.send_islock(islock)
+        sleep(5)
+
+        assert(self.nodes[0].getbestblockhash() == good_tip)
+        assert(self.nodes[1].getbestblockhash() == good_tip)
+
+        # Send the actual transaction and mine it
+        self.nodes[0].sendrawtransaction(rawtx2)
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        assert(self.nodes[0].getrawtransaction(rawtx2_txid, True)['confirmations'] > 0)
+        assert(self.nodes[1].getrawtransaction(rawtx2_txid, True)['confirmations'] > 0)
+        assert(self.nodes[0].getrawtransaction(rawtx2_txid, True)['instantlock'])
+        assert(self.nodes[1].getrawtransaction(rawtx2_txid, True)['instantlock'])
+        assert(self.nodes[0].getbestblockhash() != good_tip)
+        assert(self.nodes[1].getbestblockhash() != good_tip)
+
+    def wait_for_chainlock_tip_all_nodes(self):
+        for node in self.nodes:
+            tip = node.getbestblockhash()
+            self.wait_for_chainlock(node, tip)
+
+    def wait_for_chainlock_tip(self, node):
+        tip = node.getbestblockhash()
+        self.wait_for_chainlock(node, tip)
+
+    def wait_for_chainlock(self, node, block_hash):
+        t = time()
+        while time() - t < 15:
+            try:
+                block = node.getblockheader(block_hash)
+                if block["confirmations"] > 0 and block["chainlock"]:
+                    return
+            except:
+                # block might not be on the node yet
+                pass
+            sleep(0.1)
+        raise AssertionError("wait_for_chainlock timed out")
+
+    def create_block(self, node, vtx=[]):
+        bt = node.getblocktemplate()
+        height = bt['height']
+        tip_hash = bt['previousblockhash']
+
+        coinbasevalue = bt['coinbasevalue']
+        miner_address = node.getnewaddress()
+        mn_payee = bt['masternode'][0]['payee']
+
+        # calculate fees that the block template included (we'll have to remove it from the coinbase as we won't
+        # include the template's transactions
+        bt_fees = 0
+        for tx in bt['transactions']:
+            bt_fees += tx['fee']
+
+        new_fees = 0
+        for tx in vtx:
+            in_value = 0
+            out_value = 0
+            for txin in tx.vin:
+                txout = node.gettxout("%064x" % txin.prevout.hash, txin.prevout.n, False)
+                in_value += int(txout['value'] * COIN)
+            for txout in tx.vout:
+                out_value += txout.nValue
+            new_fees += in_value - out_value
+
+        # fix fees
+        coinbasevalue -= bt_fees
+        coinbasevalue += new_fees
+
+        mn_amount = get_masternode_payment(height, coinbasevalue)
+        miner_amount = coinbasevalue - mn_amount
+
+        outputs = {miner_address: str(Decimal(miner_amount) / COIN)}
+        if mn_amount > 0:
+            outputs[mn_payee] = str(Decimal(mn_amount) / COIN)
+
+        coinbase = FromHex(CTransaction(), node.createrawtransaction([], outputs))
+        coinbase.vin = create_coinbase(height).vin
+
+        # We can't really use this one as it would result in invalid merkle roots for masternode lists
+        if len(bt['coinbase_payload']) != 0:
+            cbtx = FromHex(CCbTx(version=1), bt['coinbase_payload'])
+            coinbase.nVersion = 3
+            coinbase.nType = 5 # CbTx
+            coinbase.vExtraPayload = cbtx.serialize()
+
+        coinbase.calc_sha256()
+
+        block = create_block(int(tip_hash, 16), coinbase, nTime=bt['curtime'])
+        block.vtx += vtx
+
+        # Add quorum commitments from template
+        for tx in bt['transactions']:
+            tx2 = FromHex(CTransaction(), tx['data'])
+            if tx2.nType == 6:
+                block.vtx.append(tx2)
+
+        block.hashMerkleRoot = block.calc_merkle_root()
+        block.solve()
+        return block
+
+    def create_chainlock(self, height, blockHash):
+        request_id = "%064x" % uint256_from_str(hash256(ser_string(b"clsig") + struct.pack("<I", height)))
+        message_hash = "%064x" % blockHash
+
+        for mn in self.mninfo:
+            mn.node.quorum('sign', 100, request_id, message_hash)
+
+        recSig = None
+
+        t = time()
+        while time() - t < 10:
+            try:
+                recSig = self.nodes[0].quorum('getrecsig', 100, request_id, message_hash)
+                break
+            except:
+                sleep(0.1)
+        assert(recSig is not None)
+
+        clsig = msg_clsig(height, blockHash, hex_str_to_bytes(recSig['sig']))
+        return clsig
+
+    def create_islock(self, hextx):
+        tx = FromHex(CTransaction(), hextx)
+        tx.rehash()
+
+        request_id_buf = ser_string(b"islock") + ser_compact_size(len(tx.vin))
+        inputs = []
+        for txin in tx.vin:
+            request_id_buf += txin.prevout.serialize()
+            inputs.append(txin.prevout)
+        request_id = "%064x" % uint256_from_str(hash256(request_id_buf))
+        message_hash = "%064x" % tx.sha256
+
+        for mn in self.mninfo:
+            mn.node.quorum('sign', 100, request_id, message_hash)
+
+        recSig = None
+
+        t = time()
+        while time() - t < 10:
+            try:
+                recSig = self.nodes[0].quorum('getrecsig', 100, request_id, message_hash)
+                break
+            except:
+                sleep(0.1)
+        assert(recSig is not None)
+
+        islock = msg_islock(inputs, tx.sha256, hex_str_to_bytes(recSig['sig']))
+        return islock
+
+
+if __name__ == '__main__':
+    LLMQ_IS_CL_Conflicts().main()

qa/rpc-tests/test_framework/mininode.py

@@ -1547,6 +1547,54 @@ class msg_mnlistdiff(object):
         return "msg_mnlistdiff(baseBlockHash=%064x, blockHash=%064x)" % (self.baseBlockHash, self.blockHash)
 
 
+class msg_clsig(object):
+    command = b"clsig"
+
+    def __init__(self, height=0, blockHash=0, sig=b'\\x0' * 96):
+        self.height = height
+        self.blockHash = blockHash
+        self.sig = sig
+
+    def deserialize(self, f):
+        self.height = struct.unpack('<i', f.read(4))[0]
+        self.blockHash = deser_uint256(f)
+        self.sig = f.read(96)
+
+    def serialize(self):
+        r = b""
+        r += struct.pack('<i', self.height)
+        r += ser_uint256(self.blockHash)
+        r += self.sig
+        return r
+
+    def __repr__(self):
+        return "msg_clsig(height=%d, blockHash=%064x)" % (self.height, self.blockHash)
+
+
+class msg_islock(object):
+    command = b"islock"
+
+    def __init__(self, inputs=[], txid=0, sig=b'\\x0' * 96):
+        self.inputs = inputs
+        self.txid = txid
+        self.sig = sig
+
+    def deserialize(self, f):
+        self.inputs = deser_vector(f, COutPoint)
+        self.txid = deser_uint256(f)
+        self.sig = f.read(96)
+
+    def serialize(self):
+        r = b""
+        r += ser_vector(self.inputs)
+        r += ser_uint256(self.txid)
+        r += self.sig
+        return r
+
+    def __repr__(self):
+        return "msg_islock(inputs=%s, txid=%064x)" % (repr(self.inputs), self.txid)
+
+
 # This is what a callback should look like for NodeConn
 # Reimplement the on_* functions to provide handling for events
 class NodeConnCB(object):
@@ -1631,6 +1679,8 @@ class NodeConnCB(object):
     def on_getblocktxn(self, conn, message): pass
     def on_blocktxn(self, conn, message): pass
     def on_mnlistdiff(self, conn, message): pass
+    def on_clsig(self, conn, message): pass
+    def on_islock(self, conn, message): pass
 
 # More useful callbacks and functions for NodeConnCB's which have a single NodeConn
 class SingleNodeConnCB(NodeConnCB):
@@ -1688,7 +1738,9 @@ class NodeConn(asyncore.dispatcher):
         b"cmpctblock": msg_cmpctblock,
         b"getblocktxn": msg_getblocktxn,
         b"blocktxn": msg_blocktxn,
-        b"mnlistdiff": msg_mnlistdiff
+        b"mnlistdiff": msg_mnlistdiff,
+        b"clsig": msg_clsig,
+        b"islock": msg_islock
     }
     MAGIC_BYTES = {
         "mainnet": b"\xbf\x0c\x6b\xbd",   # mainnet

qa/rpc-tests/test_framework/test_framework.py

@@ -515,7 +515,10 @@ class DashTestFramework(BitcoinTestFramework):
         outputs[receiver_address] = satoshi_round(amount)
         outputs[change_address] = satoshi_round(in_amount - amount - fee)
         rawtx = node_from.createrawtransaction(inputs, outputs)
-        return node_from.signrawtransaction(rawtx)
+        ret = node_from.signrawtransaction(rawtx)
+        decoded = node_from.decoderawtransaction(ret['hex'])
+        ret = {**decoded, **ret}
+        return ret
 
     # sends regular instantsend with high fee
     def send_regular_instantsend(self, sender, receiver, check_fee = True):

src/llmq/quorums_instantsend.cpp

@@ -105,6 +105,12 @@ void CInstantSendDb::RemoveInstantSendLockMined(const uint256& hash, int nHeight
     db.Erase(BuildInversedISLockKey("is_m", nHeight, hash));
 }
 
+void CInstantSendDb::WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight)
+{
+    batch.Write(BuildInversedISLockKey("is_a1", nHeight, hash), true);
+    batch.Write(std::make_tuple(std::string("is_a2"), hash), true);
+}
+
 std::unordered_map<uint256, CInstantSendLockPtr> CInstantSendDb::RemoveConfirmedInstantSendLocks(int nUntilHeight)
 {
     auto it = std::unique_ptr<CDBIterator>(db.NewIterator());
@@ -133,8 +139,7 @@ std::unordered_map<uint256, CInstantSendLockPtr> CInstantSendDb::RemoveConfirmed
         }
 
         // archive the islock hash, so that we're still able to check if we've seen the islock in the past
-        batch.Write(BuildInversedISLockKey("is_a1", nHeight, islockHash), true);
+        WriteInstantSendLockArchived(batch, islockHash, nHeight);
-        batch.Write(std::make_tuple(std::string("is_a2"), islockHash), true);
 
         batch.Erase(curKey);
 
@@ -244,6 +249,73 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& o
     return GetInstantSendLockByHash(islockHash);
 }
 
+std::vector<uint256> CInstantSendDb::GetInstantSendLocksByParent(const uint256& parent)
+{
+    auto it = std::unique_ptr<CDBIterator>(db.NewIterator());
+    auto firstKey = std::make_tuple(std::string("is_in"), COutPoint(parent, 0));
+    it->Seek(firstKey);
+
+    std::vector<uint256> result;
+
+    while (it->Valid()) {
+        decltype(firstKey) curKey;
+        if (!it->GetKey(curKey) || std::get<0>(curKey) != "is_in") {
+            break;
+        }
+        auto& outpoint = std::get<1>(curKey);
+        if (outpoint.hash != parent) {
+            break;
+        }
+
+        uint256 islockHash;
+        if (!it->GetValue(islockHash)) {
+            break;
+        }
+        result.emplace_back(islockHash);
+        it->Next();
+    }
+
+    return result;
+}
+
+std::vector<uint256> CInstantSendDb::RemoveChainedInstantSendLocks(const uint256& islockHash, const uint256& txid, int nHeight)
+{
+    std::vector<uint256> result;
+
+    std::vector<uint256> stack;
+    std::unordered_set<uint256, StaticSaltedHasher> added;
+    stack.emplace_back(txid);
+
+    CDBBatch batch(db);
+    while (!stack.empty()) {
+        auto children = GetInstantSendLocksByParent(stack.back());
+        stack.pop_back();
+
+        for (auto& childIslockHash : children) {
+            auto childIsLock = GetInstantSendLockByHash(childIslockHash);
+            if (!childIsLock) {
+                continue;
+            }
+
+            RemoveInstantSendLock(batch, childIslockHash, childIsLock);
+            WriteInstantSendLockArchived(batch, childIslockHash, nHeight);
+            result.emplace_back(childIslockHash);
+
+            if (added.emplace(childIsLock->txid).second) {
+                stack.emplace_back(childIsLock->txid);
+            }
+        }
+    }
+
+    RemoveInstantSendLock(batch, islockHash, nullptr);
+    WriteInstantSendLockArchived(batch, islockHash, nHeight);
+    result.emplace_back(islockHash);
+
+    db.WriteBatch(batch);
+
+    return result;
+}
+
 ////////////////
 
 CInstantSendManager::CInstantSendManager(CDBWrapper& _llmqDb) :
@@ -807,7 +879,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
         }
 
         // This will also add children TXs to pendingRetryTxs
-        RemoveNonLockedTx(islock.txid);
+        RemoveNonLockedTx(islock.txid, true);
     }
 
     CInv inv(MSG_ISLOCK, hash);
@@ -820,6 +892,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
     }
 
     RemoveMempoolConflictsForLock(hash, islock);
+    ResolveBlockConflicts(hash, islock);
     UpdateWalletTransaction(islock.txid, tx);
 }
 
@@ -858,6 +931,19 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn
         return;
     }
 
+    bool inMempool = mempool.get(tx.GetHash()) != nullptr;
+
+    // Are we called from validation.cpp/MemPoolConflictRemovalTracker?
+    // TODO refactor this when we backport the BlockConnected signal from Bitcoin, as it gives better info about
+    // conflicted TXs
+    bool isConflictRemoved = !pindex && posInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK && !inMempool;
+
+    if (isConflictRemoved) {
+        LOCK(cs);
+        RemoveConflictedTx(tx);
+        return;
+    }
+
     uint256 islockHash;
     {
         LOCK(cs);
@@ -882,18 +968,18 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn
     if (!chainlocked && islockHash.IsNull()) {
         // TX is not locked, so make sure it is tracked
         AddNonLockedTx(MakeTransactionRef(tx));
-        nonLockedTxs.at(tx.GetHash()).pindexMined = posInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK ? pindex : nullptr;
+        nonLockedTxs.at(tx.GetHash()).pindexMined = posInBlock != CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK ? pindex : nullptr;
     } else {
         // TX is locked, so make sure we don't track it anymore
-        RemoveNonLockedTx(tx.GetHash());
+        RemoveNonLockedTx(tx.GetHash(), true);
     }
 }
 
 void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx)
 {
     AssertLockHeld(cs);
-    auto it = nonLockedTxs.emplace(tx->GetHash(), NonLockedTxInfo()).first;
+    auto res = nonLockedTxs.emplace(tx->GetHash(), NonLockedTxInfo());
-    auto& info = it->second;
+    auto& info = res.first->second;
 
     if (!info.tx) {
         info.tx = tx;
@@ -901,9 +987,15 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx)
             nonLockedTxs[in.prevout.hash].children.emplace(tx->GetHash());
         }
     }
+
+    if (res.second) {
+        for (auto& in : tx->vin) {
+            nonLockedTxsByInputs.emplace(in.prevout.hash, std::make_pair(in.prevout.n, tx->GetHash()));
+        }
+    }
 }
 
-void CInstantSendManager::RemoveNonLockedTx(const uint256& txid)
+void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChildren)
 {
     AssertLockHeld(cs);
 
@@ -913,9 +1005,11 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid)
     }
     auto& info = it->second;
 
-    // TX got locked, so we can retry locking children
+    if (retryChildren) {
-    for (auto& childTxid : info.children) {
+        // TX got locked, so we can retry locking children
-        pendingRetryTxs.emplace(childTxid);
+        for (auto& childTxid : info.children) {
+            pendingRetryTxs.emplace(childTxid);
+        }
     }
 
     if (info.tx) {
@@ -927,12 +1021,33 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid)
                     nonLockedTxs.erase(jt);
                 }
             }
+
+            auto its = nonLockedTxsByInputs.equal_range(in.prevout.hash);
+            for (auto kt = its.first; kt != its.second; ) {
+                if (kt->second.first != in.prevout.n) {
+                    ++kt;
+                    continue;
+                } else {
+                    kt = nonLockedTxsByInputs.erase(kt);
+                }
+            }
         }
     }
 
     nonLockedTxs.erase(it);
 }
 
+void CInstantSendManager::RemoveConflictedTx(const CTransaction& tx)
+{
+    AssertLockHeld(cs);
+    RemoveNonLockedTx(tx.GetHash(), false);
+
+    for (const auto& in : tx.vin) {
+        auto inputRequestId = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in));
+        inputRequestIds.erase(inputRequestId);
+    }
+}
+
 void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock)
 {
     HandleFullyConfirmedBlock(pindexChainLock);
@@ -990,7 +1105,7 @@ void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex)
         }
         for (auto& txid : toRemove) {
             // This will also add children to pendingRetryTxs
-            RemoveNonLockedTx(txid);
+            RemoveNonLockedTx(txid, true);
         }
     }
 
@@ -1025,10 +1140,119 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, con
     }
 
     if (!toDelete.empty()) {
+        {
+            LOCK(cs);
+            for (auto& p : toDelete) {
+                RemoveConflictedTx(*p.second);
+            }
+        }
         AskNodesForLockedTx(islock.txid);
     }
 }
 
+void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const llmq::CInstantSendLock& islock)
+{
+    // Lets first collect all non-locked TXs which conflict with the given ISLOCK
+    std::unordered_map<const CBlockIndex*, std::unordered_map<uint256, CTransactionRef, StaticSaltedHasher>> conflicts;
+    {
+        LOCK(cs);
+        for (auto& in : islock.inputs) {
+            auto its = nonLockedTxsByInputs.equal_range(in.hash);
+            for (auto it = its.first; it != its.second; ++it) {
+                if (it->second.first != in.n) {
+                    continue;
+                }
+                auto& conflictTxid = it->second.second;
+                if (conflictTxid == islock.txid) {
+                    continue;
+                }
+                auto jt = nonLockedTxs.find(conflictTxid);
+                if (jt == nonLockedTxs.end()) {
+                    continue;
+                }
+                auto& info = jt->second;
+                if (!info.pindexMined || !info.tx) {
+                    continue;
+                }
+                LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: mined TX %s with input %s and mined in block %s conflicts with islock\n", __func__,
+                          islock.txid.ToString(), islockHash.ToString(), conflictTxid.ToString(), in.ToStringShort(), info.pindexMined->GetBlockHash().ToString());
+                conflicts[info.pindexMined].emplace(conflictTxid, info.tx);
+            }
+        }
+    }
+
+    // Lets see if any of the conflicts was already mined into a ChainLocked block
+    bool hasChainLockedConflict = false;
+    for (const auto& p : conflicts) {
+        auto pindex = p.first;
+        if (chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) {
+            hasChainLockedConflict = true;
+            break;
+        }
+    }
+
+    // If a conflict was mined into a ChainLocked block, then we have no other choice and must prune the ISLOCK and all
+    // chained ISLOCKs that build on top of this one. The probability of this is practically zero and can only happen
+    // when large parts of the masternode network are controlled by an attacker. In this case we must still find consensus
+    // and its better to sacrifice individual ISLOCKs then to sacrifice whole ChainLocks.
+    if (hasChainLockedConflict) {
+        RemoveChainLockConflictingLock(islockHash, islock);
+        return;
+    }
+
+    bool activateBestChain = false;
+    for (const auto& p : conflicts) {
+        auto pindex = p.first;
+        {
+            LOCK(cs);
+            for (auto& p2 : p.second) {
+                const auto& tx = *p2.second;
+                RemoveConflictedTx(tx);
+            }
+        }
+
+        LogPrintf("CInstantSendManager::%s -- invalidating block %s\n", __func__, pindex->GetBlockHash().ToString());
+
+        LOCK(cs_main);
+        CValidationState state;
+        // need non-const pointer
+        auto pindex2 = mapBlockIndex.at(pindex->GetBlockHash());
+        if (!InvalidateBlock(state, Params(), pindex2)) {
+            LogPrintf("CInstantSendManager::%s -- InvalidateBlock failed: %s\n", __func__, FormatStateMessage(state));
+            // This should not have happened and we are in a state were it's not safe to continue anymore
+            assert(false);
+        }
+        activateBestChain = true;
+    }
+
+    if (activateBestChain) {
+        CValidationState state;
+        if (!ActivateBestChain(state, Params())) {
+            LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state));
+            // This should not have happened and we are in a state were it's not safe to continue anymore
+            assert(false);
+        }
+    }
+}
+
+void CInstantSendManager::RemoveChainLockConflictingLock(const uint256& islockHash, const llmq::CInstantSendLock& islock)
+{
+    LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: at least one conflicted TX already got a ChainLock. Removing ISLOCK and its chained children.\n", __func__,
+              islock.txid.ToString(), islockHash.ToString());
+    int tipHeight;
+    {
+        LOCK(cs_main);
+        tipHeight = chainActive.Height();
+    }
+
+    LOCK(cs);
+    auto removedIslocks = db.RemoveChainedInstantSendLocks(islockHash, islock.txid, tipHeight);
+    for (auto& h : removedIslocks) {
+        LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: removed (child) ISLOCK %s\n", __func__,
+                  islock.txid.ToString(), islockHash.ToString(), h.ToString());
+    }
+}
+
 void CInstantSendManager::AskNodesForLockedTx(const uint256& txid)
 {
     std::vector<CNode*> nodesToAskFor;
@@ -1158,15 +1382,13 @@ bool CInstantSendManager::IsLocked(const uint256& txHash)
 
 bool CInstantSendManager::IsConflicted(const CTransaction& tx)
 {
-    LOCK(cs);
+    return GetConflictingLock(tx) != nullptr;
-    uint256 dummy;
-    return GetConflictingTx(tx, dummy);
 }
 
-bool CInstantSendManager::GetConflictingTx(const CTransaction& tx, uint256& retConflictTxHash)
+CInstantSendLockPtr CInstantSendManager::GetConflictingLock(const CTransaction& tx)
 {
     if (!IsNewInstantSendEnabled()) {
-        return false;
+        return nullptr;
     }
 
     LOCK(cs);
@@ -1177,11 +1399,10 @@ bool CInstantSendManager::GetConflictingTx(const CTransaction& tx, uint256& retC
         }
 
         if (otherIsLock->txid != tx.GetHash()) {
-            retConflictTxHash = otherIsLock->txid;
+            return otherIsLock;
-            return true;
         }
     }
-    return false;
+    return nullptr;
 }
 
 void CInstantSendManager::WorkThreadMain()

src/llmq/quorums_instantsend.h

@@ -58,6 +58,7 @@ public:
 
     void WriteInstantSendLockMined(const uint256& hash, int nHeight);
     void RemoveInstantSendLockMined(const uint256& hash, int nHeight);
+    void WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight);
     std::unordered_map<uint256, CInstantSendLockPtr> RemoveConfirmedInstantSendLocks(int nUntilHeight);
     void RemoveArchivedInstantSendLocks(int nUntilHeight);
     bool HasArchivedInstantSendLock(const uint256& islockHash);
@@ -66,6 +67,9 @@ public:
     uint256 GetInstantSendLockHashByTxid(const uint256& txid);
     CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid);
     CInstantSendLockPtr GetInstantSendLockByInput(const COutPoint& outpoint);
+
+    std::vector<uint256> GetInstantSendLocksByParent(const uint256& parent);
+    std::vector<uint256> RemoveChainedInstantSendLocks(const uint256& islockHash, const uint256& txid, int nHeight);
 };
 
 class CInstantSendManager : public CRecoveredSigsListener
@@ -103,6 +107,7 @@ private:
         std::unordered_set<uint256, StaticSaltedHasher> children;
     };
     std::unordered_map<uint256, NonLockedTxInfo, StaticSaltedHasher> nonLockedTxs;
+    std::unordered_multimap<uint256, std::pair<uint32_t, uint256>> nonLockedTxsByInputs;
 
     std::unordered_set<uint256, StaticSaltedHasher> pendingRetryTxs;
 
@@ -120,7 +125,7 @@ public:
     bool CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, CAmount* retValue, const Consensus::Params& params);
     bool IsLocked(const uint256& txHash);
     bool IsConflicted(const CTransaction& tx);
-    bool GetConflictingTx(const CTransaction& tx, uint256& retConflictTxHash);
+    CInstantSendLockPtr GetConflictingLock(const CTransaction& tx);
 
     virtual void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig);
     void HandleNewInputLockRecoveredSig(const CRecoveredSig& recoveredSig, const uint256& txid);
@@ -137,7 +142,8 @@ public:
 
     void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock);
     void AddNonLockedTx(const CTransactionRef& tx);
-    void RemoveNonLockedTx(const uint256& txid);
+    void RemoveNonLockedTx(const uint256& txid, bool retryChildren);
+    void RemoveConflictedTx(const CTransaction& tx);
 
     void NotifyChainLock(const CBlockIndex* pindexChainLock);
     void UpdatedBlockTip(const CBlockIndex* pindexNew);
@@ -145,6 +151,8 @@ public:
     void HandleFullyConfirmedBlock(const CBlockIndex* pindex);
 
     void RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock);
+    void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock);
+    void RemoveChainLockConflictingLock(const uint256& islockHash, const CInstantSendLock& islock);
     void AskNodesForLockedTx(const uint256& txid);
     bool ProcessPendingRetryLockTxs();
 

src/llmq/quorums_signing.cpp

@@ -24,6 +24,18 @@ namespace llmq
 
 CSigningManager* quorumSigningManager;
 
+UniValue CRecoveredSig::ToJson() const
+{
+    UniValue ret(UniValue::VOBJ);
+    ret.push_back(Pair("llmqType", (int)llmqType));
+    ret.push_back(Pair("quorumHash", quorumHash.ToString()));
+    ret.push_back(Pair("id", id.ToString()));
+    ret.push_back(Pair("msgHash", msgHash.ToString()));
+    ret.push_back(Pair("sig", sig.GetSig().ToString()));
+    ret.push_back(Pair("hash", sig.GetSig().GetHash().ToString()));
+    return ret;
+}
+
 CRecoveredSigsDb::CRecoveredSigsDb(CDBWrapper& _db) :
     db(_db)
 {
@@ -637,6 +649,14 @@ bool CSigningManager::HasRecoveredSigForSession(const uint256& signHash)
     return db.HasRecoveredSigForSession(signHash);
 }
 
+bool CSigningManager::GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id, llmq::CRecoveredSig& retRecSig)
+{
+    if (!db.GetRecoveredSigById(llmqType, id, retRecSig)) {
+        return false;
+    }
+    return true;
+}
+
 bool CSigningManager::IsConflicting(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash)
 {
     if (!db.HasRecoveredSigForId(llmqType, id)) {

src/llmq/quorums_signing.h

@@ -10,6 +10,7 @@
 #include "net.h"
 #include "chainparams.h"
 #include "saltedhasher.h"
+#include "univalue.h"
 #include "unordered_lru_cache.h"
 
 #include <unordered_map>
@@ -56,6 +57,8 @@ public:
         assert(!hash.IsNull());
         return hash;
     }
+
+    UniValue ToJson() const;
 };
 
 class CRecoveredSigsDb
@@ -157,6 +160,7 @@ public:
     bool HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash);
     bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id);
     bool HasRecoveredSigForSession(const uint256& signHash);
+    bool GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& retRecSig);
     bool IsConflicting(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash);
 
     bool HasVotedOnId(Consensus::LLMQType llmqType, const uint256& id);

src/rpc/rpcquorums.cpp

@@ -204,6 +204,17 @@ void quorum_hasrecsig_help()
     );
 }
 
+void quorum_getrecsig_help()
+{
+    throw std::runtime_error(
+            "quorum getrecsig llmqType \"id\" \"msgHash\"\n"
+            "\nArguments:\n"
+            "1. llmqType              (int, required) LLMQ type.\n"
+            "2. \"id\"                  (string, required) Request id.\n"
+            "3. \"msgHash\"             (string, required) Message hash.\n"
+    );
+}
+
 void quorum_isconflicting_help()
 {
     throw std::runtime_error(
@@ -223,6 +234,8 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request)
             quorum_sign_help();
         } else if (cmd == "hasrecsig") {
             quorum_hasrecsig_help();
+        } else if (cmd == "getrecsig") {
+            quorum_getrecsig_help();
         } else if (cmd == "isconflicting") {
             quorum_isconflicting_help();
         } else {
@@ -243,6 +256,15 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request)
         return llmq::quorumSigningManager->AsyncSignIfMember(llmqType, id, msgHash);
     } else if (cmd == "hasrecsig") {
         return llmq::quorumSigningManager->HasRecoveredSig(llmqType, id, msgHash);
+    } else if (cmd == "getrecsig") {
+        llmq::CRecoveredSig recSig;
+        if (!llmq::quorumSigningManager->GetRecoveredSigForId(llmqType, id, recSig)) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "recovered signature not found");
+        }
+        if (recSig.msgHash != msgHash) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "recovered signature not found");
+        }
+        return recSig.ToJson();
     } else if (cmd == "isconflicting") {
         return llmq::quorumSigningManager->IsConflicting(llmqType, id, msgHash);
     } else {
@@ -265,6 +287,7 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request)
             "  dkgstatus         - Return the status of the current DKG process\n"
             "  sign              - Threshold-sign a message\n"
             "  hasrecsig         - Test if a valid recovered signature is present\n"
+            "  getrecsig         - Get a recovered signature\n"
             "  isconflicting     - Test if a conflict exists\n"
     );
 }
@@ -286,7 +309,7 @@ UniValue quorum(const JSONRPCRequest& request)
         return quorum_info(request);
     } else if (command == "dkgstatus") {
         return quorum_dkgstatus(request);
-    } else if (command == "sign" || command == "hasrecsig" || command == "isconflicting") {
+    } else if (command == "sign" || command == "hasrecsig" || command == "getrecsig" || command == "isconflicting") {
         return quorum_sigs_cmd(request);
     } else {
         quorum_help();

src/validation.cpp

@@ -692,15 +692,15 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
                             REJECT_INVALID, "tx-txlock-conflict");
     }
 
-    uint256 txConflictHash;
+    llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(tx);
-    if (llmq::quorumInstantSendManager->GetConflictingTx(tx, txConflictHash)) {
+    if (conflictLock) {
         CTransactionRef txConflict;
         uint256 hashBlock;
-        if (GetTransaction(txConflictHash, txConflict, Params().GetConsensus(), hashBlock)) {
+        if (GetTransaction(conflictLock->txid, txConflict, Params().GetConsensus(), hashBlock)) {
             GetMainSignals().NotifyInstantSendDoubleSpendAttempt(tx, *txConflict);
         }
         return state.DoS(10, error("AcceptToMemoryPool : Transaction %s conflicts with locked TX %s",
-                                   hash.ToString(), txConflictHash.ToString()),
+                                   hash.ToString(), conflictLock->txid.ToString()),
                          REJECT_INVALID, "tx-txlock-conflict");
     }
 
@@ -2217,13 +2217,19 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
                                      REJECT_INVALID, "conflict-tx-lock");
                 }
             }
-            uint256 txConflict;
+            llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(*tx);
-            if (llmq::quorumInstantSendManager->GetConflictingTx(*tx, txConflict)) {
+            if (!conflictLock) {
+                continue;
+            }
+            if (llmq::chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) {
+                llmq::quorumInstantSendManager->RemoveChainLockConflictingLock(::SerializeHash(*conflictLock), *conflictLock);
+                assert(llmq::quorumInstantSendManager->GetConflictingLock(*tx) == nullptr);
+            } else {
                 // The node which relayed this should switch to correct chain.
                 // TODO: relay instantsend data/proof.
                 LOCK(cs_main);
                 mapRejectedBlocks.insert(std::make_pair(block.GetHash(), GetTime()));
-                return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), txConflict.ToString()),
+                return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()),
                                  REJECT_INVALID, "conflict-tx-lock");
             }
         }