Merge remote-tracking branch 'xyephy/support-multiple-coinbase-outputs'

Author: Sjors

src/sv2/messages.cpp

@@ -4,8 +4,9 @@
 #include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <consensus/validation.h> // NO_WITNESS_COMMITMENT
+#include <script/script.h>
 
-node::Sv2NewTemplateMsg::Sv2NewTemplateMsg(const CBlockHeader& header, const CTransactionRef coinbase_tx, std::vector<uint256> coinbase_merkle_path, int witness_commitment_index, uint64_t template_id, bool future_template)
+node::Sv2NewTemplateMsg::Sv2NewTemplateMsg(const CBlockHeader& header, const CTransactionRef coinbase_tx, std::vector<uint256> coinbase_merkle_path, uint64_t template_id, bool future_template)
     : m_template_id{template_id}, m_future_template{future_template}
 {
     m_version = header.nVersion;
@@ -17,11 +18,16 @@ node::Sv2NewTemplateMsg::Sv2NewTemplateMsg(const CBlockHeader& header, const CTr
     // The coinbase nValue already contains the nFee + the Block Subsidy when built using CreateBlock().
     m_coinbase_tx_value_remaining = static_cast<uint64_t>(coinbase_tx->vout[0].nValue);
 
-    m_coinbase_tx_outputs_count = 0;
-    if (witness_commitment_index != NO_WITNESS_COMMITMENT) {
-        m_coinbase_tx_outputs_count = 1;
-        m_coinbase_tx_outputs = {coinbase_tx->vout[witness_commitment_index]};
+    // Extract only OP_RETURN coinbase outputs (witness commitment, merge mining, etc.)
+    // Bitcoin Core adds a dummy output with the full reward that we must exclude,
+    // otherwise the pool would create an invalid block trying to spend that amount again.
+    m_coinbase_tx_outputs.clear();
+    for (const auto& output : coinbase_tx->vout) {
+        if (!output.scriptPubKey.empty() && output.scriptPubKey[0] == OP_RETURN) {
+            m_coinbase_tx_outputs.push_back(output);
+        }
     }
+    m_coinbase_tx_outputs_count = m_coinbase_tx_outputs.size();
 
     m_coinbase_tx_locktime = coinbase_tx->nLockTime;
 

src/sv2/messages.h

@@ -314,7 +314,7 @@ struct Sv2NewTemplateMsg
     std::vector<uint256> m_merkle_path;
 
     Sv2NewTemplateMsg() = default;
-    explicit Sv2NewTemplateMsg(const CBlockHeader& header, const CTransactionRef coinbase_tx, std::vector<uint256> coinbase_merkle_path, int witness_commitment_index, uint64_t template_id, bool future_template);
+    explicit Sv2NewTemplateMsg(const CBlockHeader& header, const CTransactionRef coinbase_tx, std::vector<uint256> coinbase_merkle_path, uint64_t template_id, bool future_template);
 
     template <typename Stream>
     void Serialize(Stream& s) const
@@ -332,8 +332,12 @@ struct Sv2NewTemplateMsg
         // as [B0_64K](https://github.com/stratum-mining/sv2-spec/blob/main/03-Protocol-Overview.md#31-data-types-mapping)
         if (m_coinbase_tx_outputs_count > 0) {
             std::vector<uint8_t> outputs_bytes;
-            // TODO: support more than 1 output
-            VectorWriter{outputs_bytes, 0, m_coinbase_tx_outputs.at(0)};
+            for (const auto& output : m_coinbase_tx_outputs) {
+                VectorWriter{outputs_bytes, outputs_bytes.size(), output};
+            }
+
+            LogPrintLevel(BCLog::SV2, BCLog::Level::Trace, "Serialized %zu coinbase output(s), total size: %zu bytes\n",
+                          m_coinbase_tx_outputs.size(), outputs_bytes.size());
 
             s << static_cast<uint16_t>(outputs_bytes.size());
             s.write(MakeByteSpan(outputs_bytes));

src/sv2/template_provider.cpp

@@ -558,7 +558,6 @@ bool Sv2TemplateProvider::SendWork(Sv2Client& client, uint64_t template_id, Bloc
     node::Sv2NewTemplateMsg new_template{header,
                                         block_template.getCoinbaseTx(),
                                         block_template.getCoinbaseMerklePath(),
-                                        block_template.getWitnessCommitmentIndex(),
                                         template_id,
                                         future_template};
 

src/test/sv2_messages_tests.cpp

@@ -2,6 +2,9 @@
 #include <boost/test/unit_test.hpp>
 #include <sv2/messages.h>
 #include <util/strencodings.h>
+#include <primitives/transaction.h>
+#include <primitives/block.h>
+#include <script/script.h>
 
 BOOST_AUTO_TEST_SUITE(sv2_messages_tests)
 
@@ -153,6 +156,74 @@ BOOST_AUTO_TEST_CASE(Sv2NewTemplate_test)
     BOOST_CHECK_EQUAL(HexStr(ss), expected);
 }
 
+BOOST_AUTO_TEST_CASE(Sv2NewTemplate_MultipleOutputs_test)
+{
+    // NewTemplate with realistic coinbase: 1 dummy anyone-can-spend output + 3 OP_RETURN outputs
+    // Tests that only OP_RETURN outputs are serialized (dummy output filtered out)
+    //
+    // U64              0100000000000000    template_id
+    // BOOL             00                  future_template
+    // U32              00000030            version
+    // U32              02000000            coinbase tx version
+    // B0_255           04                  coinbase_prefix len
+    //                  03012100            coinbase prefix
+    // U32              ffffffff            coinbase tx input sequence
+    // U64              0040075af0750700    coinbase tx value remaining
+    // U32              03000000            coinbase tx outputs count (3 OP_RETURN outputs, dummy filtered)
+    // B0_64K           2100                coinbase_tx_outputs (33 bytes total)
+    //                  6400000000000000    output 1: 100 sats
+    //                  026a51              output 1: script (OP_RETURN OP_1)
+    //                  c800000000000000    output 2: 200 sats
+    //                  026a52              output 2: script (OP_RETURN OP_2)
+    //                  2c01000000000000    output 3: 300 sats
+    //                  026a53              output 3: script (OP_RETURN OP_3)
+    // U32              dbc80d00            coinbase lock time (height 903,387)
+    // SEQ0_255[U256]   01                  merkle path length
+    //                  1a6240823de4c8d6aaf826851bdf2b0e8d5acf7c31e8578cff4c394b5a32bd4e - merkle path
+    std::string expected{"01000000000000000000000030020000000403012100ffffffff0040075af07507000300000021006400000000000000026a51c800000000000000026a522c01000000000000026a53dbc80d00011a6240823de4c8d6aaf826851bdf2b0e8d5acf7c31e8578cff4c394b5a32bd4e"};
+
+    // Create realistic coinbase transaction with dummy anyone-can-spend output
+    CMutableTransaction coinbase_tx;
+    coinbase_tx.version = 2;
+    coinbase_tx.vin.resize(1);
+    coinbase_tx.vin[0].prevout.SetNull();
+    std::vector<uint8_t> coinbase_prefix_bytes{0x03, 0x01, 0x21, 0x00};
+    coinbase_tx.vin[0].scriptSig = CScript(coinbase_prefix_bytes.begin(), coinbase_prefix_bytes.end());
+    coinbase_tx.vin[0].nSequence = 4294967295;
+
+    coinbase_tx.vout.resize(4);
+    // Output 0: Dummy anyone-can-spend output with full reward (will be filtered out by sv2-tp)
+    coinbase_tx.vout[0].nValue = MAX_MONEY;
+    coinbase_tx.vout[0].scriptPubKey = CScript() << OP_TRUE;
+
+    // Outputs 1-3: OP_RETURN outputs (will be included in NewTemplate message)
+    coinbase_tx.vout[1] = CTxOut(100, CScript() << OP_RETURN << 1);
+    coinbase_tx.vout[2] = CTxOut(200, CScript() << OP_RETURN << 2);
+    coinbase_tx.vout[3] = CTxOut(300, CScript() << OP_RETURN << 3);
+
+    coinbase_tx.nLockTime = 903387;
+
+    CBlockHeader header;
+    header.nVersion = 805306368;
+    header.hashPrevBlock.SetNull();
+    header.nTime = 0;
+    header.nBits = 0;
+    header.nNonce = 0;
+
+    std::vector<uint256> merkle_path;
+    CMutableTransaction mtx_tx;
+    CTransaction tx{mtx_tx};
+    merkle_path.push_back(tx.GetHash().ToUint256());
+
+    // Use constructor which filters outputs to only OP_RETURN
+    node::Sv2NewTemplateMsg new_template{header, MakeTransactionRef(coinbase_tx), merkle_path, 1, false};
+
+    DataStream ss{};
+    ss << new_template;
+
+    BOOST_CHECK_EQUAL(HexStr(ss), expected);
+}
+
 BOOST_AUTO_TEST_CASE(Sv2NetHeader_NewTemplate_test)
 {
     // 0000 - extension type

src/test/sv2_mock_mining.cpp

@@ -21,14 +21,24 @@ MockBlockTemplate::MockBlockTemplate(std::shared_ptr<MockState> st, uint256 prev
 {
     // Simple internal consistency assertion: constructor sequence should not exceed state counter.
     assert(m_sequence <= state->chain.template_seq);
-    // Build a dummy coinbase
+    // Build a realistic coinbase with multiple outputs
     CMutableTransaction cb;
     cb.vin.resize(1);
     cb.vin[0].prevout.SetNull();
     cb.vin[0].scriptSig = CScript() << OP_0;
-    cb.vout.resize(1);
+
+    cb.vout.resize(3);
+    // Output 0: Dummy anyone-can-spend output with full reward (will be filtered out by sv2-tp)
     cb.vout[0].nValue = 50 * COIN;
-    cb.vout[0].scriptPubKey = CScript() << OP_RETURN;
+    cb.vout[0].scriptPubKey = CScript() << OP_TRUE;
+
+    // Output 1: Fake witness commitment (will be included)
+    cb.vout[1].nValue = 0;
+    cb.vout[1].scriptPubKey = CScript() << OP_RETURN << std::vector<unsigned char>(32, 0xaa);
+
+    // Output 2: Fake merge mining commitment (will be included)
+    cb.vout[2].nValue = 0;
+    cb.vout[2].scriptPubKey = CScript() << OP_RETURN << std::vector<unsigned char>{'M', 'M'};
 
     block = CBlock{};
     block.vtx.clear();

src/test/sv2_template_provider_tests.cpp

@@ -62,11 +62,11 @@ BOOST_AUTO_TEST_CASE(client_tests)
         1 +                 // future_template
         4 +                 // version
         4 +                 // coinbase_tx_version
-    2 +                 // coinbase_prefix (CompactSize(1) + 1-byte OP_0)
+        2 +                 // coinbase_prefix (CompactSize(1) + 1-byte OP_0)
         4 +                 // coinbase_tx_input_sequence
         8 +                 // coinbase_tx_value_remaining
-        4 +                 // coinbase_tx_outputs_count (0)
-        2 +                 // B0_64K length for outputs array (0)
+        4 +                 // coinbase_tx_outputs_count (2 - mock creates 3, only 2 OP_RETURN outputs pass filter)
+        2 + 56 +            // B0_64K: length prefix (2 bytes) + 2 outputs (witness commitment 43 bytes + merge mining 13 bytes)
         4 +                 // coinbase_tx_locktime
         1;                  // merkle_path count (CompactSize(0))