From a3d04ed72e77135600af7ae1949950b85e7911f1 Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Wed, 7 Oct 2026 01:09:10 -0500
Subject: [PATCH 1/2] fix(coinjoin): bind signatures to the approved
 transaction

CoinJoin entries reach the coordinator without their memory-only
prevPubKey, so IsInputScriptSigValid verified each scriptSig against an
empty script on a reconstructed, unsorted transaction. Any push-only
scriptSig that left true on the stack passed, so the signature check
did not authenticate the input.

Record each input's scriptPubKey from the coin view under cs_main when
the entry is admitted, and reject the entry if an input is missing or
spent. When a signature arrives, require the signing state, bind it to
an input of the sender's own admitted entry, and verify it with
standard script flags against the canonical final transaction before
changing any accepted state.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 src/coinjoin/server.cpp            | 123 +++++++++++-----------------
 src/coinjoin/server.h              |   4 +-
 src/test/coinjoin_inouts_tests.cpp | 127 ++++++++++++++++++++++++++++-
 3 files changed, 176 insertions(+), 78 deletions(-)

diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp
index b06652025431..91490990e630 100644
--- a/src/coinjoin/server.cpp
+++ b/src/coinjoin/server.cpp
@@ -10,10 +10,12 @@
 #include <masternode/meta.h>
 #include <masternode/sync.h>
 
+#include <coins.h>
 #include <core_io.h>
 #include <net.h>
 #include <net_processing.h>
 #include <netmessagemaker.h>
+#include <policy/policy.h>
 #include <scheduler.h>
 #include <script/interpreter.h>
 #include <serialize.h>
@@ -26,6 +28,8 @@
 
 #include <univalue.h>
 
+#include <algorithm>
+#include <iterator>
 #include <ranges>
 
 CCoinJoinServer::CCoinJoinServer(PeerManagerInternal* peer_manager, ChainstateManager& chainman, CConnman& _connman,
@@ -281,7 +285,7 @@ void CCoinJoinServer::ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv)
         int nTxInsCount = static_cast<int>(vecTxIn.size());
         for (const auto& txin : vecTxIn) {
             nTxInIndex++;
-            if (!AddScriptSig(txin)) {
+            if (!AddScriptSig(txin, peer.addr)) {
                 LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- AddScriptSig() failed at %d/%d, session: %d\n", nTxInIndex,
                          nTxInsCount, session_id);
                 RelayStatus(STATUS_REJECTED);
@@ -778,51 +782,6 @@ void CCoinJoinServer::CheckForCompleteQueue()
     m_queueman.AddQueue(std::move(dsq));
 }
 
-// Check to make sure a given input matches an input in the pool and its scriptSig is valid
-bool CCoinJoinServer::IsInputScriptSigValid(const CTxIn& txin) const
-{
-    AssertLockHeld(cs_coinjoin);
-    CMutableTransaction txNew;
-    txNew.vin.clear();
-    txNew.vout.clear();
-
-    int nTxInIndex = -1;
-    CScript sigPubKey = CScript();
-
-    {
-        int i = 0;
-        for (const auto &entry: vecEntries) {
-            for (const auto &txout: entry.vecTxOut) {
-                txNew.vout.push_back(txout);
-            }
-            for (const auto &txdsin: entry.vecTxDSIn) {
-                txNew.vin.push_back(txdsin);
-
-                if (txdsin.prevout == txin.prevout) {
-                    nTxInIndex = i;
-                    sigPubKey = txdsin.prevPubKey;
-                }
-                i++;
-            }
-        }
-    }
-    if (nTxInIndex >= 0) { //might have to do this one input at a time?
-        txNew.vin[nTxInIndex].scriptSig = txin.scriptSig;
-        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::IsInputScriptSigValid -- verifying scriptSig %s\n", ScriptToAsmStr(txin.scriptSig).substr(0, 24));
-        // TODO we're using amount=0 here but we should use the correct amount. This works because Dash ignores the amount while signing/verifying (only used in Bitcoin/Segwit)
-        if (!VerifyScript(txNew.vin[nTxInIndex].scriptSig, sigPubKey, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, MutableTransactionSignatureChecker(&txNew, nTxInIndex, 0, MissingDataBehavior::ASSERT_FAIL))) {
-            LogPrint(BCLog::COINJOIN, "CCoinJoinServer::IsInputScriptSigValid -- VerifyScript() failed on input %d\n", nTxInIndex);
-            return false;
-        }
-    } else {
-        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::IsInputScriptSigValid -- Failed to find matching input in pool, %s\n", txin.ToString());
-        return false;
-    }
-
-    LogPrint(BCLog::COINJOIN, "CCoinJoinServer::IsInputScriptSigValid -- Successfully validated input and scriptSig\n");
-    return true;
-}
-
 //
 // Add a client's transaction inputs/outputs to the pool
 //
@@ -966,6 +925,21 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
         return false;
     }
 
+    CCoinJoinEntry admitted_entry{entry};
+    {
+        LOCK(cs_main);
+        CCoinsViewMemPool view{&m_chainman.ActiveChainstate().CoinsTip(), mempool};
+        for (auto& input : admitted_entry.vecTxDSIn) {
+            Coin coin;
+            if (!view.GetCoin(input.prevout, coin) || coin.IsSpent()) {
+                nMessageIDRet = ERR_MISSING_TX;
+                return false;
+            }
+            // prevPubKey is memory-only; keep the admitted script even if chainstate changes before signing.
+            input.prevPubKey = coin.out.scriptPubKey;
+        }
+    }
+
     {
         LOCK(cs_coinjoin);
         // cs_coinjoin was released around the UTXO checks above, so the scheduler thread may
@@ -979,7 +953,7 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
             nMessageIDRet = ERR_SESSION;
             return false;
         }
-        vecEntries.push_back(entry);
+        vecEntries.push_back(std::move(admitted_entry));
     }
 
     LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- adding entry %d of %d required\n", __func__, GetEntriesCount(), CoinJoin::GetMaxPoolParticipants());
@@ -988,41 +962,42 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
     return true;
 }
 
-bool CCoinJoinServer::AddScriptSig(const CTxIn& txinNew)
+bool CCoinJoinServer::AddScriptSig(const CTxIn& txinNew, const CService& sender)
 {
     AssertLockHeld(cs_coinjoin);
-    LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- scriptSig=%s\n", ScriptToAsmStr(txinNew.scriptSig).substr(0, 24));
-
-    for (const auto& entry : vecEntries) {
-        if (std::ranges::any_of(entry.vecTxDSIn,
-                                [&txinNew](const auto& txdsin) { return txdsin.scriptSig == txinNew.scriptSig; })) {
-            LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- already exists\n");
-            return false;
-        }
-    }
+    if (nState != POOL_STATE_SIGNING) return false;
 
-    if (!IsInputScriptSigValid(txinNew)) {
+    const auto matches_input = [&txinNew](const CTxIn& input) {
+        return input.prevout == txinNew.prevout && input.nSequence == txinNew.nSequence;
+    };
+    const auto entry = std::ranges::find_if(vecEntries, [&](const auto& candidate) {
+        return candidate.addr == sender && std::ranges::any_of(candidate.vecTxDSIn, matches_input);
+    });
+    if (entry == vecEntries.end()) return false;
+
+    const auto admitted_input = std::ranges::find_if(entry->vecTxDSIn, matches_input);
+    const auto final_input = std::ranges::find_if(finalMutableTransaction.vin, matches_input);
+    if (final_input == finalMutableTransaction.vin.end() || admitted_input->prevPubKey.empty()) return false;
+    if (admitted_input->fHasSig && admitted_input->scriptSig == txinNew.scriptSig) return true;
+
+    const auto index = static_cast<unsigned int>(std::distance(finalMutableTransaction.vin.begin(), final_input));
+    // Dash's legacy signature hash does not use the input amount.
+    if (!VerifyScript(txinNew.scriptSig, admitted_input->prevPubKey,
+                      STANDARD_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_SIGPUSHONLY,
+                      MutableTransactionSignatureChecker(&finalMutableTransaction, index, 0, MissingDataBehavior::ASSERT_FAIL))) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- Invalid scriptSig\n");
         return false;
     }
-
-    LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- scriptSig=%s new\n", ScriptToAsmStr(txinNew.scriptSig).substr(0, 24));
-
-    for (auto& txin : finalMutableTransaction.vin) {
-        if (txin.prevout == txinNew.prevout && txin.nSequence == txinNew.nSequence) {
-            txin.scriptSig = txinNew.scriptSig;
-            LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- adding to finalMutableTransaction, scriptSig=%s\n", ScriptToAsmStr(txinNew.scriptSig).substr(0, 24));
-        }
-    }
-    for (auto& entry : vecEntries) {
-        if (entry.AddScriptSig(txinNew)) {
-            LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- adding to entries, scriptSig=%s\n", ScriptToAsmStr(txinNew.scriptSig).substr(0, 24));
-            return true;
+    if (admitted_input->fHasSig) return true;
+    for (const auto& other_entry : vecEntries) {
+        if (std::ranges::any_of(other_entry.vecTxDSIn,
+                               [&txinNew](const auto& input) { return input.scriptSig == txinNew.scriptSig; })) {
+            return false;
         }
     }
-
-    LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- Couldn't set sig!\n");
-    return false;
+    if (!entry->AddScriptSig(txinNew)) return false;
+    final_input->scriptSig = txinNew.scriptSig;
+    return true;
 }
 
 // Check to make sure everything is signed
diff --git a/src/coinjoin/server.h b/src/coinjoin/server.h
index d5abb3d218b7..fc5af7c380c2 100644
--- a/src/coinjoin/server.h
+++ b/src/coinjoin/server.h
@@ -147,7 +147,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler
     Mutex cs_check_pool;
 
     /// Add signature to a txin
-    bool AddScriptSig(const CTxIn& txin) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
+    bool AddScriptSig(const CTxIn& txin, const CService& sender) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
 
     /// Collaterals chosen to be charged whose consumption has not finished yet. They are chosen
     /// under cs_coinjoin, but ConsumeCollateral() takes cs_main and runs after cs_coinjoin is
@@ -210,8 +210,6 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler
 
     /// Check that all inputs are signed. (Are all inputs signed?)
     bool IsSignaturesComplete() const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
-    /// Check to make sure a given input matches an input in the pool and its scriptSig is valid
-    bool IsInputScriptSigValid(const CTxIn& txin) const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
 
     // Set the 'state' value, with some logging and capturing when the state changed.
     // Requires cs_coinjoin so that a transition and the session data it describes are always
diff --git a/src/test/coinjoin_inouts_tests.cpp b/src/test/coinjoin_inouts_tests.cpp
index c5dea2930c42..1ef47ebd6741 100644
--- a/src/test/coinjoin_inouts_tests.cpp
+++ b/src/test/coinjoin_inouts_tests.cpp
@@ -10,15 +10,24 @@
 #include <coinjoin/common.h>
 #include <coinjoin/options.h>
 #include <coinjoin/server.h>
+#include <coins.h>
+#include <key.h>
 #include <llmq/context.h>
 #include <masternode/sync.h>
 #include <net.h>
 #include <node/connection_types.h>
+#include <policy/policy.h>
 #include <protocol.h>
+#include <script/interpreter.h>
 #include <script/script.h>
+#include <script/sign.h>
+#include <script/signingprovider.h>
+#include <script/standard.h>
 #include <streams.h>
+#include <test/util/net.h>
 #include <test/util/setup_common.h>
 #include <tinyformat.h>
+#include <txmempool.h>
 #include <uint256.h>
 #include <util/check.h>
 #include <util/time.h>
@@ -29,7 +38,9 @@
 #include <algorithm>
 #include <array>
 #include <cstdint>
+#include <iterator>
 #include <memory>
+#include <ranges>
 #include <vector>
 
 BOOST_FIXTURE_TEST_SUITE(coinjoin_inouts_tests, TestingSetup)
@@ -187,7 +198,13 @@ class TestableCoinJoinServer : public CCoinJoinServer
     // A live session always carries a non-zero id, and AddEntry rejects entries that don't
     // belong to one, so seed an id along with the state.
     void EnterSigningState() { nSessionID = 1; nState = POOL_STATE_SIGNING; }
-    void EnterAcceptingEntriesState() { nSessionID = 1; nState = POOL_STATE_ACCEPTING_ENTRIES; }
+    void EnterAcceptingEntriesState(int denom = 0) { nSessionID = 1; nSessionDenom = denom; nState = POOL_STATE_ACCEPTING_ENTRIES; }
+
+    CMutableTransaction FinalTransaction() const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin)
+    {
+        LOCK(cs_coinjoin);
+        return finalMutableTransaction;
+    }
 
     void SeedParticipant(const CService& addr) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin)
     {
@@ -230,6 +247,114 @@ static std::unique_ptr<CNode> MakePeer(NodeId id, uint32_t ipv4)
     return peer;
 }
 
+BOOST_AUTO_TEST_CASE(server_signfinaltx_completes_with_legacy_and_full_signatures)
+{
+    m_node.mn_sync->SwitchToNextAsset();
+    BOOST_REQUIRE(m_node.mn_sync->IsBlockchainSynced());
+    auto& connman{static_cast<ConnmanTestMsg&>(*Assert(m_node.connman))};
+    CActiveMasternodeManager mn_activeman(connman, *Assert(m_node.dmnman), MakeSecretKey());
+    TestableCoinJoinServer server(m_node.peerman.get(), *Assert(m_node.chainman), connman,
+                                  *Assert(m_node.dmnman), *Assert(m_node.dstxman), *Assert(m_node.mn_metaman),
+                                  *Assert(m_node.mempool), mn_activeman, *Assert(m_node.mn_sync), *Assert(m_node.isman));
+    struct PeerCleanup {
+        ConnmanTestMsg& connman;
+        ~PeerCleanup() { connman.ClearTestNodes(); }
+    } cleanup{connman};
+    FillableSigningProvider provider;
+    const CAmount amount{CoinJoin::GetSmallestDenomination()};
+    const int participants{std::max(3, CoinJoin::GetMinPoolParticipants())};
+    std::vector<CNode*> peers;
+    std::vector<CScript> previous_scripts;
+    std::vector<CCoinJoinEntry> entries;
+
+    for (int i{0}; i < participants; ++i) {
+        auto peer = MakePeer(i + 100, 0x0a000010 + i);
+        peer->fSuccessfullyConnected = true;
+        peers.push_back(peer.get());
+        connman.AddTestNode(*peer.release());
+        CKey key;
+        key.MakeNewKey(true);
+        BOOST_REQUIRE(provider.AddKey(key));
+        const CScript previous_script{GetScriptForDestination(PKHash(key.GetPubKey()))};
+        previous_scripts.push_back(previous_script);
+        const COutPoint input{uint256::ONE, static_cast<uint32_t>(100 + participants - i)};
+        const COutPoint collateral_input{uint256::TWO, static_cast<uint32_t>(100 + i)};
+        {
+            LOCK(cs_main);
+            auto& coins{m_node.chainman->ActiveChainstate().CoinsTip()};
+            coins.AddCoin(input, Coin{CTxOut{amount, previous_script}, 0, false}, false);
+            coins.AddCoin(collateral_input, Coin{CTxOut{COIN / 10, previous_script}, 0, false}, false);
+        }
+        CMutableTransaction collateral;
+        collateral.vin.emplace_back(collateral_input);
+        collateral.vout.emplace_back(COIN / 10 - CoinJoin::GetCollateralAmount(), P2PKHScript(100 + i));
+        BOOST_REQUIRE(SignSignature(provider, previous_script, collateral, 0, COIN / 10, SIGHASH_ALL));
+        server.SeedSessionCollateral(collateral, CoinJoin::MixShape::STANDARD);
+        // Descending output order requires verification against the canonical BIP69 transaction.
+        CCoinJoinEntry original{{CTxDSIn{CTxIn{input}, previous_script, 0}},
+                               {CTxOut{amount, P2PKHScript(participants - i)}}, CTransaction{collateral}};
+        CDataStream wire{SER_NETWORK, PROTOCOL_VERSION};
+        wire << original;
+        CCoinJoinEntry entry;
+        wire >> entry;
+        BOOST_REQUIRE(entry.vecTxDSIn[0].prevPubKey.empty());
+        entry.addr = peers[i == participants - 1 ? 0 : i]->addr;
+        entries.push_back(std::move(entry));
+    }
+    server.EnterAcceptingEntriesState(CoinJoin::AmountToDenomination(amount));
+    for (const auto& entry : entries) {
+        PoolMessage message{MSG_NOERR};
+        BOOST_REQUIRE(server.AddEntry(entry, message));
+    }
+    server.CreateFinalTransaction(/*session_id=*/1, /*charge_fees=*/false);
+    BOOST_REQUIRE_EQUAL(server.GetState(), int{POOL_STATE_SIGNING});
+    CMutableTransaction final{server.FinalTransaction()};
+    BOOST_REQUIRE(final.vout.front().scriptPubKey == P2PKHScript(1));
+
+    const auto input_index = [&final](const COutPoint& prevout) {
+        return std::distance(final.vin.begin(), std::ranges::find(final.vin, prevout, &CTxIn::prevout));
+    };
+    std::vector<CTxIn> signed_inputs;
+    for (int i{0}; i < participants; ++i) {
+        const auto index{input_index(entries[i].vecTxDSIn[0].prevout)};
+        BOOST_REQUIRE(SignSignature(provider, previous_scripts[i], final, index, amount,
+                                    SIGHASH_ALL | (i == 0 ? SIGHASH_ANYONECANPAY : 0)));
+        signed_inputs.push_back(final.vin[index]);
+    }
+    const auto rejected = [](CNode& peer) {
+        LOCK(peer.cs_vSend);
+        for (const auto& msg : peer.vSendMsg) {
+            if (msg.m_type != NetMsgType::DSSTATUSUPDATE) continue;
+            CDataStream wire{msg.data, SER_NETWORK, PROTOCOL_VERSION};
+            CCoinJoinStatusUpdate status;
+            wire >> status;
+            if (status.nStatusUpdate == STATUS_REJECTED) return true;
+        }
+        return false;
+    };
+    const auto submit = [&server, &peers, participants](int participant, const CTxIn& input) {
+        for (auto* peer : peers) {
+            LOCK(peer->cs_vSend);
+            peer->vSendMsg.clear();
+        }
+        CDataStream wire{SER_NETWORK, PROTOCOL_VERSION};
+        wire << std::vector<CTxIn>{input};
+        server.ProcessMessage(*peers[participant == participants - 1 ? 0 : participant], NetMsgType::DSSIGNFINALTX, wire);
+    };
+
+    submit(0, signed_inputs[0]);
+    BOOST_CHECK(!rejected(*peers[0]));
+    // Multiple entries on this connection can receive the same final proposal again.
+    submit(0, signed_inputs[0]);
+    BOOST_CHECK(!rejected(*peers[0]));
+    for (int i{1}; i < participants; ++i) {
+        submit(i, signed_inputs[i]);
+        BOOST_CHECK(!rejected(*peers[i == participants - 1 ? 0 : i]));
+    }
+    BOOST_CHECK(m_node.mempool->exists(final.GetHash()));
+    BOOST_CHECK_EQUAL(server.GetState(), int{POOL_STATE_IDLE});
+}
+
 BOOST_AUTO_TEST_CASE(server_signfinaltx_nonparticipant_cannot_abort_session)
 {
     BOOST_REQUIRE(m_node.mn_sync);

From c485e32db67b9b2f11407bcfb5ca5cfc29ceb6aa Mon Sep 17 00:00:00 2001
From: pasta <pasta@dashboost.org>
Date: Wed, 7 Oct 2026 03:15:35 -0500
Subject: [PATCH 2/2] test: cover CoinJoin signature admission rejection

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 src/test/coinjoin_inouts_tests.cpp | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/test/coinjoin_inouts_tests.cpp b/src/test/coinjoin_inouts_tests.cpp
index 1ef47ebd6741..f27f9ebd923e 100644
--- a/src/test/coinjoin_inouts_tests.cpp
+++ b/src/test/coinjoin_inouts_tests.cpp
@@ -341,12 +341,35 @@ BOOST_AUTO_TEST_CASE(server_signfinaltx_completes_with_legacy_and_full_signature
         wire << std::vector<CTxIn>{input};
         server.ProcessMessage(*peers[participant == participants - 1 ? 0 : participant], NetMsgType::DSSIGNFINALTX, wire);
     };
+    const auto check_rejected = [&](int participant, const CTxIn& input) {
+        const auto before{server.FinalTransaction()};
+        submit(participant, input);
+        BOOST_CHECK(rejected(*peers[participant == participants - 1 ? 0 : participant]));
+        BOOST_CHECK(server.FinalTransaction().GetHash() == before.GetHash());
+        BOOST_CHECK_EQUAL(server.GetState(), int{POOL_STATE_SIGNING});
+    };
+
+    // An admitted connection cannot sign an input belonging to another connection.
+    check_rejected(1, signed_inputs[0]);
+    CMutableTransaction modified_proposal{final};
+    modified_proposal.vout.front().scriptPubKey = P2PKHScript(0xff);
+    const auto first_index{input_index(signed_inputs[0].prevout)};
+    constexpr int legacy_mode{SIGHASH_ALL | SIGHASH_ANYONECANPAY};
+    BOOST_REQUIRE(SignSignature(provider, previous_scripts[0], modified_proposal, first_index, amount, legacy_mode));
+    BOOST_REQUIRE(VerifyScript(modified_proposal.vin[first_index].scriptSig, previous_scripts[0],
+                               STANDARD_SCRIPT_VERIFY_FLAGS,
+                               MutableTransactionSignatureChecker(&modified_proposal, first_index, amount,
+                                                                  MissingDataBehavior::ASSERT_FAIL)));
+    check_rejected(0, modified_proposal.vin[first_index]);
 
     submit(0, signed_inputs[0]);
     BOOST_CHECK(!rejected(*peers[0]));
     // Multiple entries on this connection can receive the same final proposal again.
     submit(0, signed_inputs[0]);
     BOOST_CHECK(!rejected(*peers[0]));
+    CTxIn invalid_retry{signed_inputs[0]};
+    invalid_retry.scriptSig.clear();
+    check_rejected(0, invalid_retry);
     for (int i{1}; i < participants; ++i) {
         submit(i, signed_inputs[i]);
         BOOST_CHECK(!rejected(*peers[i == participants - 1 ? 0 : i]));