Move transaction combining from signrawtransaction to new RPC

Liquid369 · Liquid369 · commit d1940db24bbc · 2023-05-15T08:13:28.000-05:00
New combinerawtransaction RPC implementation
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
@@ -32,11 +32,12 @@ static const CRPCConvertParam vRPCConvertParams[] = {
     { "cleanbudget", 0, "try_sync" },
     { "createmultisig", 0, "nrequired" },
     { "createmultisig", 1, "keys" },
+    { "combinerawtransaction", 0, "txs" },
     { "createrawtransaction", 0, "inputs" },
     { "createrawtransaction", 1, "outputs" },
     { "createrawtransaction", 2, "locktime" },
-    {"createrawmnfinalbudget", 1, "blockstart"},
-    {"createrawmnfinalbudget", 2, "proposals"},
+    { "createrawmnfinalbudget", 1, "blockstart" },
+    { "createrawmnfinalbudget", 2, "proposals" },
     { "delegatestake", 1, "amount" },
     { "delegatestake", 3, "ext_owner" },
     { "delegatestake", 4, "include_delegated" },
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
@@ -502,6 +502,92 @@ static void TxInErrorToJSON(const CTxIn& txin, UniValue& vErrorsRet, const std::
     vErrorsRet.push_back(entry);
 }
 
+UniValue combinerawtransaction(const JSONRPCRequest& request)
+{
+    if (request.fHelp || request.params.size() != 1)
+        throw std::runtime_error(
+                "combinerawtransaction [\"hexstring\",...]\n"
+                "\nCombine multiple partially signed transactions into one transaction.\n"
+                "The combined transaction may be another partially signed transaction or a \n"
+                "fully signed transaction."
+
+                "\nArguments:\n"
+                "1. \"txs\"         (string) A json array of hex strings of partially signed transactions\n"
+                "    [\n"
+                "      \"hexstring\"     (string) A transaction hash\n"
+                "      ,...\n"
+                "    ]\n"
+
+                "\nResult:\n"
+                "\"hex\" : \"value\",           (string) The hex-encoded raw transaction with signature(s)\n"
+
+                "\nExamples:\n"
+                + HelpExampleCli("combinerawtransaction", "[\"myhex1\", \"myhex2\", \"myhex3\"]")
+        );
+
+
+    UniValue txs = request.params[0].get_array();
+    std::vector<CMutableTransaction> txVariants(txs.size());
+
+    for (unsigned int idx = 0; idx < txs.size(); idx++) {
+        if (!DecodeHexTx(txVariants[idx], txs[idx].get_str())) {
+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed for tx %d", idx));
+        }
+    }
+
+    if (txVariants.empty()) {
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Missing transactions");
+    }
+
+    // mergedTx will end up with all the signatures; it
+    // starts as a clone of the rawtx:
+    CMutableTransaction mergedTx(txVariants[0]);
+
+    // Fetch previous transactions (inputs):
+    CCoinsView viewDummy;
+    CCoinsViewCache view(&viewDummy);
+    {
+        LOCK(cs_main);
+        LOCK(mempool.cs);
+        CCoinsViewCache &viewChain = *pcoinsTip;
+        CCoinsViewMemPool viewMempool(&viewChain, mempool);
+        view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view
+
+        for (const CTxIn& txin : mergedTx.vin) {
+            view.AccessCoin(txin.prevout); // Load entries from viewChain into view; can fail.
+        }
+
+        view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long
+    }
+
+    // Use CTransaction for the constant parts of the
+    // transaction to avoid rehashing.
+    const CTransaction txConst(mergedTx);
+    // Sign what we can:
+    for (unsigned int i = 0; i < mergedTx.vin.size(); i++) {
+        CTxIn& txin = mergedTx.vin[i];
+        const Coin& coin = view.AccessCoin(txin.prevout);
+        if (coin.IsSpent()) {
+            throw JSONRPCError(RPC_VERIFY_ERROR, "Input not found or already spent");
+        }
+        const CScript& prevPubKey = coin.out.scriptPubKey;
+        const CAmount& amount = coin.out.nValue;
+
+        SignatureData sigdata;
+
+        // ... and merge in other signatures:
+        for (const CMutableTransaction& txv : txVariants) {
+            if (txv.vin.size() > i) {
+                sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(txv, i));
+            }
+        }
+
+        UpdateTransaction(mergedTx, i, sigdata);
+    }
+
+    return EncodeHexTx(mergedTx);
+}
+
 UniValue signrawtransaction(const JSONRPCRequest& request)
 {
     CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
@@ -570,30 +656,14 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
 #endif
     RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VARR, UniValue::VSTR}, true);
 
-    std::vector<unsigned char> txData(ParseHexV(request.params[0], "argument 1"));
-    CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION);
-    std::vector<CMutableTransaction> txVariants;
-    while (!ssData.empty()) {
-        try {
-            CMutableTransaction tx;
-            ssData >> tx;
-            txVariants.push_back(tx);
-        } catch (const std::exception&) {
-            throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
-        }
-    }
-
-    if (txVariants.empty())
-        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Missing transaction");
-
-    // mergedTx will end up with all the signatures; it
-    // starts as a clone of the rawtx:
-    CMutableTransaction mergedTx(txVariants[0]);
+    CMutableTransaction mtx;
+    if (!DecodeHexTx(mtx, request.params[0].get_str()))
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
 
     // Fetch previous transactions (inputs):
     std::map<COutPoint, std::pair<CScript, CAmount>> mapPrevOut; // todo: check why do we have this for regtest..
     if (Params().IsRegTestNet()) {
-        for (const CTxIn &txbase : mergedTx.vin)
+        for (const CTxIn &txbase : mtx.vin)
         {
             CTransactionRef tempTx;
             uint256 hashBlock;
@@ -611,7 +681,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
         CCoinsViewMemPool viewMempool(&viewChain, mempool);
         view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view
 
-        for (const CTxIn& txin : mergedTx.vin) {
+        for (const CTxIn& txin : mtx.vin) {
             view.AccessCoin(txin.prevout); // Load entries from viewChain into view; can fail.
         }
 
@@ -732,10 +802,10 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
 
     // Use CTransaction for the constant parts of the
     // transaction to avoid rehashing.
-    const CTransaction txConst(mergedTx);
+    const CTransaction txConst(mtx);
     // Sign what we can:
-    for (unsigned int i = 0; i < mergedTx.vin.size(); i++) {
-        CTxIn& txin = mergedTx.vin[i];
+    for (unsigned int i = 0; i < mtx.vin.size(); i++) {
+        CTxIn& txin = mtx.vin[i];
         const Coin& coin = view.AccessCoin(txin.prevout);
         if (Params().IsRegTestNet()) {
             if (mapPrevOut.count(txin.prevout) == 0 && coin.IsSpent())
@@ -763,18 +833,13 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
         }
 
         SignatureData sigdata;
-        SigVersion sigversion = mergedTx.GetRequiredSigVersion();
+        SigVersion sigversion = mtx.GetRequiredSigVersion();
         // Only sign SIGHASH_SINGLE if there's a corresponding output:
-        if (!fHashSingle || (i < mergedTx.vout.size()))
-            ProduceSignature(MutableTransactionSignatureCreator(&keystore, &mergedTx, i, amount, nHashType),
-                    prevPubKey, sigdata, sigversion, fColdStake);
+        if (!fHashSingle || (i < mtx.vout.size()))
+            ProduceSignature(MutableTransactionSignatureCreator(&keystore, &mtx, i, amount, nHashType), prevPubKey, sigdata, sigversion, fColdStake);
+        sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(mtx, i));
 
-        // ... and merge in other signatures:
-        for (const CMutableTransaction& txv : txVariants) {
-            sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(txv, i));
-        }
-
-        UpdateTransaction(mergedTx, i, sigdata);
+        UpdateTransaction(mtx, i, sigdata);
 
         ScriptError serror = SCRIPT_ERR_OK;
         if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS,
@@ -785,7 +850,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request)
     bool fComplete = vErrors.empty();
 
     UniValue result(UniValue::VOBJ);
-    result.pushKV("hex", EncodeHexTx(mergedTx));
+    result.pushKV("hex", EncodeHexTx(mtx));
     result.pushKV("complete", fComplete);
     if (!vErrors.empty()) {
         result.pushKV("errors", vErrors);
@@ -895,6 +960,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
 static const CRPCCommand commands[] =
 { //  category              name                      actor (function)         okSafe argNames
   //  --------------------- ------------------------  -----------------------  ------ --------
+    { "rawtransactions",    "combinerawtransaction",  &combinerawtransaction,  true,  {"txs"} },
     { "rawtransactions",    "createrawtransaction",   &createrawtransaction,   true,  {"inputs","outputs","locktime"} },
     { "rawtransactions",    "decoderawtransaction",   &decoderawtransaction,   true,  {"hexstring"}  },
     { "rawtransactions",    "decodescript",           &decodescript,           true,  {"hexstring"} },
diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py
@@ -497,11 +497,11 @@ def test_all_watched_funds(self):
         assert_greater_than(result["changepos"], -1)
         assert_equal(result["fee"] + res_dec["vout"][result["changepos"]]["value"], float(self.watchonly_amount) / 10)
 
-        signedtx = self.nodes[3].signrawtransaction(result["hex"])
-        assert not signedtx["complete"]
-        signedtx = self.nodes[0].signrawtransaction(signedtx["hex"])
-        assert signedtx["complete"]
-        self.nodes[0].sendrawtransaction(signedtx["hex"])
+        signedtxpart1 = self.nodes[3].signrawtransaction(result["hex"])
+        assert not signedtxpart1["complete"]
+        signedtxpart2 = self.nodes[0].signrawtransaction(signedtxpart1["hex"])
+        combinedtx = self.nodes[0].combinerawtransaction([signedtxpart1['hex'], signedtxpart2['hex']])
+        self.nodes[0].sendrawtransaction(combinedtx)
         self.nodes[0].generate(1)
         self.sync_all()
 
diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
@@ -284,7 +284,7 @@ def run_test(self):
                 break
 
         bal = self.nodes[0].getbalance()
-        inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex'], "amount" : vout['value']}]
+        inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex']}]
         outputs = { self.nodes[0].getnewaddress() : 2.19 }
         rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs)
         rawTxPartialSigned1 = self.nodes[1].signrawtransaction(rawTx2, inputs)
@@ -294,12 +294,10 @@ def run_test(self):
         rawTxPartialSigned2 = self.nodes[2].signrawtransaction(rawTx2, inputs)
         self.log.info(rawTxPartialSigned2)
         assert_equal(rawTxPartialSigned2['complete'], False) #node2 only has one key, can't comp. sign the tx
-
-        rawTxSignedComplete = self.nodes[2].signrawtransaction(rawTxPartialSigned1['hex'], inputs)
-        self.log.info(rawTxSignedComplete)
-        assert_equal(rawTxSignedComplete['complete'], True)
-        self.nodes[2].sendrawtransaction(rawTxSignedComplete['hex'])
-        rawTx2 = self.nodes[0].decoderawtransaction(rawTxSignedComplete['hex'])
+        rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']])
+        self.log.info(rawTxComb)
+        self.nodes[2].sendrawtransaction(rawTxComb)
+        rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb)
         self.sync_all()
         self.nodes[0].generate(1)
         self.sync_all()
