test: improve CoinJoin unit coverage

src/test/coinjoin_dstxmanager_tests.cpp

@@ -122,4 +122,71 @@ BOOST_AUTO_TEST_CASE(update_heights_block_connect_disconnect)
     }
 }
 
+BOOST_AUTO_TEST_CASE(mempool_reentry_clears_confirmed_height)
+{
+    // After a block connect sets the confirmed height, the same tx being
+    // (re)added to the mempool, e.g. via a reorg, should clear it. This
+    // drives DSTX expiry logic that watches GetConfirmedHeight().
+    CCoinJoinBroadcastTx dstx = MakeDSTX();
+    auto& man = *Assert(m_node.dstxman);
+    man.AddDSTX(dstx);
+
+    auto block = std::make_shared<CBlock>();
+    block->vtx.push_back(dstx.tx);
+    CBlockIndex index;
+    index.nHeight = 200;
+    uint256 bh = uint256S("0c");
+    index.phashBlock = &bh;
+
+    man.BlockConnected(block, &index);
+    {
+        auto got = man.GetDSTX(dstx.tx->GetHash());
+        BOOST_REQUIRE(static_cast<bool>(got));
+        BOOST_REQUIRE(got.GetConfirmedHeight().has_value());
+        BOOST_CHECK_EQUAL(*got.GetConfirmedHeight(), 200);
+    }
+
+    // Re-entry to the mempool clears the confirmed height (without removing the entry).
+    man.TransactionAddedToMempool(dstx.tx);
+    {
+        auto got = man.GetDSTX(dstx.tx->GetHash());
+        BOOST_REQUIRE(static_cast<bool>(got));
+        BOOST_CHECK(!got.GetConfirmedHeight().has_value());
+    }
+}
+
+BOOST_AUTO_TEST_CASE(mempool_event_unrelated_tx_does_not_mutate_dstx)
+{
+    // TransactionAddedToMempool for a transaction that is not a tracked DSTX
+    // must be a no-op for any stored DSTX state.
+    CCoinJoinBroadcastTx dstx = MakeDSTX();
+    auto& man = *Assert(m_node.dstxman);
+    man.AddDSTX(dstx);
+
+    // Establish a confirmed height to detect any unwanted mutation.
+    auto block = std::make_shared<CBlock>();
+    block->vtx.push_back(dstx.tx);
+    CBlockIndex index;
+    index.nHeight = 321;
+    uint256 bh = uint256S("0d");
+    index.phashBlock = &bh;
+    man.BlockConnected(block, &index);
+
+    // Build an unrelated transaction by mutating one input of the tracked tx so it hashes differently.
+    CMutableTransaction other_mtx(*dstx.tx);
+    other_mtx.vin[0].prevout = COutPoint(uint256S("ee"), 7);
+    auto other_tx = MakeTransactionRef(other_mtx);
+    BOOST_REQUIRE(other_tx->GetHash() != dstx.tx->GetHash());
+
+    man.TransactionAddedToMempool(other_tx);
+
+    // Stored DSTX still has its confirmed height.
+    auto got = man.GetDSTX(dstx.tx->GetHash());
+    BOOST_REQUIRE(static_cast<bool>(got));
+    BOOST_REQUIRE(got.GetConfirmedHeight().has_value());
+    BOOST_CHECK_EQUAL(*got.GetConfirmedHeight(), 321);
+    // Unrelated tx was not inserted as a DSTX.
+    BOOST_CHECK(!static_cast<bool>(man.GetDSTX(other_tx->GetHash())));
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/coinjoin_inouts_tests.cpp

@@ -84,6 +84,77 @@ BOOST_AUTO_TEST_CASE(broadcasttx_isvalidstructure_good_and_bad)
     BOOST_CHECK(!bad_amount.IsValidStructure());
 }
 
+BOOST_AUTO_TEST_CASE(entry_addscriptsig_matches_and_rejects)
+{
+    // Build an entry with two distinct inputs so we can check both the match
+    // and the isolation (only the matching input mutates).
+    const COutPoint op0(uint256::ONE, 0);
+    const COutPoint op1(uint256::ONE, 1);
+    const uint32_t seq0 = 0xfffffffeU;
+    const uint32_t seq1 = 0xfffffffdU;
+
+    auto make_dsin = [](const COutPoint& op, uint32_t seq) {
+        CTxIn in(op);
+        in.nSequence = seq;
+        return CTxDSIn(in, P2PKHScript(0x10), /*nRounds=*/0);
+    };
+
+    std::vector<CTxDSIn> dsins{make_dsin(op0, seq0), make_dsin(op1, seq1)};
+    CCoinJoinEntry entry(dsins, /*vecTxOut=*/{}, CTransaction{CMutableTransaction{}});
+
+    // The scriptSig we expect to be copied across on a successful match.
+    const CScript scriptSig0 = CScript() << std::vector<unsigned char>{0xde, 0xad} << std::vector<unsigned char>{0xbe, 0xef};
+
+    // Matching prevout + matching sequence -> copies scriptSig, sets fHasSig.
+    {
+        CTxIn signed_in(op0, scriptSig0, seq0);
+        BOOST_CHECK(entry.AddScriptSig(signed_in));
+        BOOST_CHECK(entry.vecTxDSIn[0].fHasSig);
+        BOOST_CHECK(entry.vecTxDSIn[0].scriptSig == scriptSig0);
+        // Other input is untouched.
+        BOOST_CHECK(!entry.vecTxDSIn[1].fHasSig);
+        BOOST_CHECK(entry.vecTxDSIn[1].scriptSig.empty());
+    }
+
+    // Duplicate signature for the already-signed input -> rejected, no overwrite.
+    {
+        const CScript scriptSig_other = CScript() << std::vector<unsigned char>{0x01};
+        CTxIn dup_in(op0, scriptSig_other, seq0);
+        BOOST_CHECK(!entry.AddScriptSig(dup_in));
+        // Still holds the original signature.
+        BOOST_CHECK(entry.vecTxDSIn[0].scriptSig == scriptSig0);
+        BOOST_CHECK(entry.vecTxDSIn[0].fHasSig);
+    }
+
+    // Wrong prevout (sequence matches an existing input) -> rejected.
+    {
+        const COutPoint op_wrong(uint256S("ff"), 9);
+        CTxIn wrong_in(op_wrong, scriptSig0, seq1);
+        BOOST_CHECK(!entry.AddScriptSig(wrong_in));
+        BOOST_CHECK(!entry.vecTxDSIn[1].fHasSig);
+        BOOST_CHECK(entry.vecTxDSIn[1].scriptSig.empty());
+    }
+
+    // Right prevout but wrong sequence -> rejected (guards against malleated nSequence).
+    {
+        CTxIn badseq_in(op1, scriptSig0, /*nSequence=*/seq1 ^ 0xffU);
+        BOOST_CHECK(!entry.AddScriptSig(badseq_in));
+        BOOST_CHECK(!entry.vecTxDSIn[1].fHasSig);
+        BOOST_CHECK(entry.vecTxDSIn[1].scriptSig.empty());
+    }
+
+    // Correct prevout + correct sequence on the second input -> succeeds, doesn't disturb the first.
+    {
+        const CScript scriptSig1 = CScript() << std::vector<unsigned char>{0xca, 0xfe};
+        CTxIn signed_in(op1, scriptSig1, seq1);
+        BOOST_CHECK(entry.AddScriptSig(signed_in));
+        BOOST_CHECK(entry.vecTxDSIn[1].fHasSig);
+        BOOST_CHECK(entry.vecTxDSIn[1].scriptSig == scriptSig1);
+        // First input unchanged.
+        BOOST_CHECK(entry.vecTxDSIn[0].scriptSig == scriptSig0);
+    }
+}
+
 BOOST_AUTO_TEST_CASE(queue_timeout_bounds)
 {
     CCoinJoinQueue dsq;

src/test/coinjoin_queue_tests.cpp

@@ -188,4 +188,59 @@ BOOST_AUTO_TEST_CASE(queuemanager_getqueueitem_marks_tried_once)
     BOOST_CHECK(!man.GetQueueItemAndTry(picked2));
 }
 
+BOOST_AUTO_TEST_CASE(queuemanager_has_queue_from_masternode_readiness)
+{
+    CoinJoinQueueManager man;
+    const int denom = CoinJoin::AmountToDenomination(CoinJoin::GetSmallestDenomination());
+    const int64_t now = GetAdjustedTime();
+    const COutPoint mn(uint256S("31"), 0);
+    // Single queue from `mn` with fReady=false.
+    man.AddQueue(MakeQueue(denom, now, /*fReady=*/false, mn));
+
+    // Readiness-aware lookup honors fReady.
+    BOOST_CHECK(man.HasQueueFromMasternode(mn, /*fReady=*/false));
+    BOOST_CHECK(!man.HasQueueFromMasternode(mn, /*fReady=*/true));
+
+    // Unrelated masternode is never matched.
+    const COutPoint other(uint256S("32"), 0);
+    BOOST_CHECK(!man.HasQueueFromMasternode(other, /*fReady=*/false));
+    BOOST_CHECK(!man.HasQueueFromMasternode(other, /*fReady=*/true));
+
+    // TRY variant ignores readiness, finds by outpoint only.
+    auto try_present = man.TryHasQueueFromMasternode(mn);
+    BOOST_REQUIRE(try_present.has_value());
+    BOOST_CHECK(*try_present);
+
+    auto try_absent = man.TryHasQueueFromMasternode(other);
+    BOOST_REQUIRE(try_absent.has_value());
+    BOOST_CHECK(!*try_absent);
+}
+
+BOOST_AUTO_TEST_CASE(queuemanager_try_check_duplicate)
+{
+    CoinJoinQueueManager man;
+    const int denom = CoinJoin::AmountToDenomination(CoinJoin::GetSmallestDenomination());
+    const int64_t now = GetAdjustedTime();
+    const COutPoint mn(uint256S("41"), 0);
+    const COutPoint other_mn(uint256S("42"), 0);
+
+    const auto seed = MakeQueue(denom, now, /*fReady=*/false, mn);
+    man.AddQueue(seed);
+
+    auto is_dup = [&](const CCoinJoinQueue& q) {
+        auto r = man.TryCheckDuplicate(q);
+        BOOST_REQUIRE(r.has_value());
+        return *r;
+    };
+
+    // Exact duplicate.
+    BOOST_CHECK(is_dup(seed));
+    // Same masternode + same readiness, different nTime: the "too many dsqs" guard still flags it.
+    BOOST_CHECK(is_dup(MakeQueue(denom, now + 1, /*fReady=*/false, mn)));
+    // Same masternode but different readiness: allowed.
+    BOOST_CHECK(!is_dup(MakeQueue(denom, now, /*fReady=*/true, mn)));
+    // Different masternode: allowed.
+    BOOST_CHECK(!is_dup(MakeQueue(denom, now, /*fReady=*/false, other_mn)));
+}
+
 BOOST_AUTO_TEST_SUITE_END()