fix: separate CoinJoin entry wire and semantic limits

Deserialize CoinJoin entry input and output vectors through the pool-wide 180-element wire cap while retaining the 9-element per-entry policy in AddEntry. Apply the shared pool cap to DSSIGNFINALTX and only consume an oversized entry's collateral when it was accepted into the current session.

Author: thepastaclaw

src/coinjoin/coinjoin.cpp

@@ -95,7 +95,7 @@ bool CCoinJoinBroadcastTx::IsValidStructure() const
     if (tx->vin.size() < size_t(CoinJoin::GetMinPoolParticipants())) {
         return false;
     }
-    if (tx->vin.size() > CoinJoin::GetMaxPoolParticipants() * COINJOIN_ENTRY_MAX_SIZE) {
+    if (tx->vin.size() > CoinJoin::GetMaxPoolInputOutputCount()) {
         return false;
     }
     return std::ranges::all_of(tx->vout, [](const auto& txOut) {

src/coinjoin/coinjoin.h

@@ -50,6 +50,15 @@ static constexpr int COINJOIN_SIGNING_TIMEOUT = 15;
 
 static constexpr size_t COINJOIN_ENTRY_MAX_SIZE = 9;
 
+namespace CoinJoin {
+/// Get the minimum/maximum number of participants for the pool
+int GetMinPoolParticipants();
+int GetMaxPoolParticipants();
+
+/// Maximum number of inputs or outputs across a full pool
+inline size_t GetMaxPoolInputOutputCount() { return size_t(GetMaxPoolParticipants()) * COINJOIN_ENTRY_MAX_SIZE; }
+} // namespace CoinJoin
+
 // pool responses
 enum PoolMessage : int32_t {
     ERR_ALREADY_HAVE,
@@ -155,7 +164,6 @@ class CCoinJoinEntry
     CTransactionRef txCollateral;
     // memory only
     CService addr;
-    bool fHasOversizedTxOut{false};
 
     CCoinJoinEntry() :
         txCollateral(MakeTransactionRef(CMutableTransaction{}))
@@ -178,24 +186,15 @@ class CCoinJoinEntry
     template <typename Stream>
     void Unserialize(Stream& s)
     {
-        fHasOversizedTxOut = false;
-
-        // Accept one input over the cap so AddEntry can apply the collateral
-        // penalty for an entry that is merely too large; anything bigger is
-        // rejected before any element is decoded.
-        if (!UnserializeVectorWithMaxSize(s, vecTxDSIn, COINJOIN_ENTRY_MAX_SIZE + 1)) {
+        const size_t max_count{CoinJoin::GetMaxPoolInputOutputCount()};
+        if (!UnserializeVectorWithMaxSize(s, vecTxDSIn, max_count)) {
             throw std::ios_base::failure("CCoinJoinEntry::vecTxDSIn size too large");
         }
 
         s >> txCollateral;
 
-        if (!UnserializeVectorWithMaxSize(s, vecTxOut, COINJOIN_ENTRY_MAX_SIZE)) {
-            // Outputs follow txCollateral on the wire, so an output count over the
-            // cap must not throw mid-entry: flag it and leave txCollateral intact
-            // so AddEntry can reject the entry with ERR_MAXIMUM and consume its
-            // collateral. A count above the generic MAX_SIZE cap is malformed and
-            // still throws from ReadCompactSize.
-            fHasOversizedTxOut = true;
+        if (!UnserializeVectorWithMaxSize(s, vecTxOut, max_count)) {
+            throw std::ios_base::failure("CCoinJoinEntry::vecTxOut size too large");
         }
     }
 
@@ -407,10 +406,6 @@ namespace CoinJoin
 {
     bilingual_str GetMessageByID(PoolMessage nMessageID);
 
-    /// Get the minimum/maximum number of participants for the pool
-    int GetMinPoolParticipants();
-    int GetMaxPoolParticipants();
-
     constexpr CAmount GetMaxPoolAmount() { return COINJOIN_ENTRY_MAX_SIZE * vecStandardDenominations.front(); }
 
     /// If the collateral is valid given by a client

src/coinjoin/server.cpp

@@ -246,7 +246,7 @@ void CCoinJoinServer::ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv)
         }
     }
 
-    const size_t max_txins{size_t(CoinJoin::GetMaxPoolParticipants()) * COINJOIN_ENTRY_MAX_SIZE};
+    const size_t max_txins{CoinJoin::GetMaxPoolInputOutputCount()};
     std::vector<CTxIn> vecTxIn;
     // Reject an over-cap count through this peer-local ERR_MAXIMUM path before a
     // single CTxIn is decoded or allocated. A count above the generic MAX_SIZE cap
@@ -610,11 +610,12 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
         return false;
     }
 
-    if (entry.fHasOversizedTxOut || entry.vecTxOut.size() > COINJOIN_ENTRY_MAX_SIZE) {
-        const size_t txout_size{entry.fHasOversizedTxOut ? COINJOIN_ENTRY_MAX_SIZE + 1 : entry.vecTxOut.size()};
-        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many outputs! %d/%d\n", __func__,
-                 static_cast<int>(txout_size), static_cast<int>(COINJOIN_ENTRY_MAX_SIZE));
+    if (entry.vecTxDSIn.size() > COINJOIN_ENTRY_MAX_SIZE || entry.vecTxOut.size() > COINJOIN_ENTRY_MAX_SIZE) {
+        LogPrint(BCLog::COINJOIN, /* Continued */
+                 "CCoinJoinServer::%s -- ERROR: too many inputs or outputs! inputs=%s/%s, outputs=%s/%s\n", __func__,
+                 entry.vecTxDSIn.size(), COINJOIN_ENTRY_MAX_SIZE, entry.vecTxOut.size(), COINJOIN_ENTRY_MAX_SIZE);
         nMessageIDRet = ERR_MAXIMUM;
+
         CTransactionRef txCollateralToConsume;
         {
             LOCK(cs_coinjoin);
@@ -637,13 +638,6 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
         return false;
     }
 
-    if (entry.vecTxDSIn.size() > COINJOIN_ENTRY_MAX_SIZE) {
-        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many inputs! %d/%d\n", __func__, entry.vecTxDSIn.size(), COINJOIN_ENTRY_MAX_SIZE);
-        nMessageIDRet = ERR_MAXIMUM;
-        ConsumeCollateral(entry.txCollateral);
-        return false;
-    }
-
     std::vector<CTxIn> vin;
     for (const auto& txin : entry.vecTxDSIn) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- txin=%s\n", __func__, txin.ToString());

src/test/coinjoin_inouts_tests.cpp

@@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(server_signfinaltx_nonparticipant_cannot_abort_session)
     auto nonparticipant = MakePeer(/*id=*/42, /*ipv4=*/0x01020304);
     BOOST_REQUIRE(!(nonparticipant->addr == participant->addr));
 
-    const size_t max_txins{size_t(CoinJoin::GetMaxPoolParticipants()) * COINJOIN_ENTRY_MAX_SIZE};
+    const size_t max_txins{CoinJoin::GetMaxPoolInputOutputCount()};
     CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
     WriteCompactSize(stream, max_txins + 1);
 
@@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE(server_signfinaltx_participant_oversized_count_is_rejected_
     server.SeedParticipant(participant->addr);
     server.EnterSigningState();
 
-    const size_t max_txins{size_t(CoinJoin::GetMaxPoolParticipants()) * COINJOIN_ENTRY_MAX_SIZE};
+    const size_t max_txins{CoinJoin::GetMaxPoolInputOutputCount()};
     CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
     WriteCompactSize(stream, max_txins + 1);
 
@@ -287,76 +287,53 @@ BOOST_AUTO_TEST_CASE(server_signfinaltx_participant_oversized_count_is_rejected_
     BOOST_CHECK_EQUAL(server.GetEntriesCount(), 1);
 }
 
-BOOST_AUTO_TEST_CASE(entry_allows_one_oversized_input_for_collateral_penalty)
+BOOST_AUTO_TEST_CASE(entry_deserializes_vectors_through_wire_cap)
 {
-    CCoinJoinEntry oversized_entry;
-    oversized_entry.vecTxDSIn.resize(COINJOIN_ENTRY_MAX_SIZE + 1);
-
-    CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
-    stream << oversized_entry;
-
-    CCoinJoinEntry entry;
-    BOOST_CHECK_NO_THROW(stream >> entry);
-    BOOST_CHECK_EQUAL(entry.vecTxDSIn.size(), COINJOIN_ENTRY_MAX_SIZE + 1);
-    BOOST_CHECK(entry.vecTxOut.empty());
+    const size_t wire_cap{CoinJoin::GetMaxPoolInputOutputCount()};
+    BOOST_REQUIRE_GT(wire_cap, COINJOIN_ENTRY_MAX_SIZE);
+
+    for (const size_t count : {size_t{0}, COINJOIN_ENTRY_MAX_SIZE, COINJOIN_ENTRY_MAX_SIZE + 1, wire_cap}) {
+        BOOST_TEST_CONTEXT("count=" << count)
+        {
+            CCoinJoinEntry entry;
+            entry.vecTxDSIn.resize(count);
+            entry.vecTxOut.resize(count);
+
+            CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
+            stream << entry;
+
+            CCoinJoinEntry roundtripped;
+            BOOST_CHECK_NO_THROW(stream >> roundtripped);
+            BOOST_CHECK_EQUAL(roundtripped.vecTxDSIn.size(), count);
+            BOOST_CHECK_EQUAL(roundtripped.vecTxOut.size(), count);
+        }
+    }
 }
 
-BOOST_AUTO_TEST_CASE(entry_rejects_huge_inputs_before_materializing)
+BOOST_AUTO_TEST_CASE(entry_rejects_inputs_above_wire_cap_before_materializing)
 {
+    const size_t wire_cap{CoinJoin::GetMaxPoolInputOutputCount()};
     CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
-    WriteCompactSize(stream, COINJOIN_ENTRY_MAX_SIZE + 2);
+    WriteCompactSize(stream, wire_cap + 1);
 
     CCoinJoinEntry entry;
     BOOST_CHECK_THROW(stream >> entry, std::ios_base::failure);
     BOOST_CHECK(entry.vecTxDSIn.empty());
 }
 
-BOOST_AUTO_TEST_CASE(entry_flags_oversized_outputs_before_materializing)
+BOOST_AUTO_TEST_CASE(entry_rejects_outputs_above_wire_cap_before_materializing)
 {
+    const size_t wire_cap{CoinJoin::GetMaxPoolInputOutputCount()};
     CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
     WriteCompactSize(stream, 0);
     stream << MakeTransactionRef(CMutableTransaction{});
-    WriteCompactSize(stream, COINJOIN_ENTRY_MAX_SIZE + 1);
+    WriteCompactSize(stream, wire_cap + 1);
 
-    CCoinJoinEntry entry;
-    BOOST_CHECK_NO_THROW(stream >> entry);
-    BOOST_CHECK(entry.fHasOversizedTxOut);
-    BOOST_CHECK(entry.vecTxOut.empty());
-}
-
-BOOST_AUTO_TEST_CASE(entry_throws_on_huge_outputs_before_materializing)
-{
-    CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
-    WriteCompactSize(stream, 0);
-    stream << MakeTransactionRef(CMutableTransaction{});
-    WriteCompactSize(stream, MAX_SIZE + 1);
-
-    // Above the generic CompactSize cap the message is simply malformed, so the
-    // standard deserialization error is raised. No output is materialized, and the
-    // half-read entry is not left looking like an oversized-but-chargeable one:
-    // fHasOversizedTxOut stays clear, so it cannot reach AddEntry's collateral path.
     CCoinJoinEntry entry;
     BOOST_CHECK_THROW(stream >> entry, std::ios_base::failure);
-    BOOST_CHECK(!entry.fHasOversizedTxOut);
     BOOST_CHECK(entry.vecTxOut.empty());
 }
 
-BOOST_AUTO_TEST_CASE(entry_accepts_max_sized_vectors)
-{
-    CCoinJoinEntry entry;
-    entry.vecTxDSIn.resize(COINJOIN_ENTRY_MAX_SIZE); // exactly the cap must be accepted
-    entry.vecTxOut.resize(COINJOIN_ENTRY_MAX_SIZE);
-
-    CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
-    stream << entry;
-
-    CCoinJoinEntry roundtripped;
-    BOOST_CHECK_NO_THROW(stream >> roundtripped);
-    BOOST_CHECK_EQUAL(roundtripped.vecTxDSIn.size(), COINJOIN_ENTRY_MAX_SIZE);
-    BOOST_CHECK_EQUAL(roundtripped.vecTxOut.size(), COINJOIN_ENTRY_MAX_SIZE);
-    BOOST_CHECK(!roundtripped.fHasOversizedTxOut);
-}
-
 BOOST_AUTO_TEST_CASE(queue_timeout_bounds)
 {
     CCoinJoinQueue dsq;