Merge #15784: rpc: Remove dependency on interfaces::Chain in SignTransaction

99e88a372 rpc: Remove dependency on interfaces::Chain in SignTransaction (Antoine Riard)

Pull request description:

  Assuming wallet RPCs and node RPCs will go into different processes, signrawtransactionwithkey doesn't need to access Coins via interfaces::Chain, it may use directly utility in node/coins.cpp

  Obviously will need rebase after #15638

Tree-SHA512: 42ee8fcbcd38643bbd82210db6f68249bed5ee036a4c930a1db534d0469a133e287b8869c977bf0cc79a7296dde04f72adb74d24e1cd20f4a280f4c2b7fceb74

4 files changed, 34 insertions, 19 deletions

diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 1a7216ceeb..78d7bbc80c 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -15,6 +15,7 @@
 #include <key_io.h>
 #include <keystore.h>
 #include <merkleblock.h>
+#include <node/coin.h>
 #include <node/psbt.h>
 #include <node/transaction.h>
 #include <policy/policy.h>
@@ -770,7 +771,14 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request)
         keystore.AddKey(key);
     }
 
-    return SignTransaction(*g_rpc_interfaces->chain, mtx, request.params[2], &keystore, true, request.params[3]);
+    // Fetch previous transactions (inputs):
+    std::map<COutPoint, Coin> coins;
+    for (const CTxIn& txin : mtx.vin) {
+        coins[txin.prevout]; // Create empty map entry keyed by prevout.
+    }
+    FindCoins(coins);
+
+    return SignTransaction(mtx, request.params[2], &keystore, coins, true, request.params[3]);
 }
 
 static UniValue sendrawtransaction(const JSONRPCRequest& request)
diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp
index 728fc62e25..8af491977c 100644
--- a/src/rpc/rawtransaction_util.cpp
+++ b/src/rpc/rawtransaction_util.cpp
@@ -149,18 +149,8 @@ static void TxInErrorToJSON(const CTxIn& txin, UniValue& vErrorsRet, const std::
     vErrorsRet.push_back(entry);
 }
 
-// TODO(https://github.com/bitcoin/bitcoin/pull/10973#discussion_r267084237):
-// The dependency on interfaces::Chain should be removed, so
-// signrawtransactionwithkey doesn't need access to a Chain instance.
-UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, const UniValue& prevTxsUnival, CBasicKeyStore *keystore, bool is_temp_keystore, const UniValue& hashType)
+UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival, CBasicKeyStore* keystore, std::map<COutPoint, Coin>& coins, bool is_temp_keystore, const UniValue& hashType)
 {
-    // Fetch previous transactions (inputs):
-    std::map<COutPoint, Coin> coins;
-    for (const CTxIn& txin : mtx.vin) {
-        coins[txin.prevout]; // Create empty map entry keyed by prevout.
-    }
-    chain.findCoins(coins);
-
     // Add previous txouts given in the RPC call:
     if (!prevTxsUnival.isNull()) {
         UniValue prevTxs = prevTxsUnival.get_array();
diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h
index 5529dedbd4..c115d33a77 100644
--- a/src/rpc/rawtransaction_util.h
+++ b/src/rpc/rawtransaction_util.h
@@ -5,16 +5,26 @@
 #ifndef BITCOIN_RPC_RAWTRANSACTION_UTIL_H
 #define BITCOIN_RPC_RAWTRANSACTION_UTIL_H
 
+#include <map>
+
 class CBasicKeyStore;
 class UniValue;
 struct CMutableTransaction;
+class Coin;
+class COutPoint;
 
-namespace interfaces {
-class Chain;
-} // namespace interfaces
-
-/** Sign a transaction with the given keystore and previous transactions */
-UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, const UniValue& prevTxs, CBasicKeyStore *keystore, bool tempKeystore, const UniValue& hashType);
+/**
+ * Sign a transaction with the given keystore and previous transactions
+ *
+ * @param  mtx           The transaction to-be-signed
+ * @param  prevTxs       Array of previous txns outputs that tx depends on but may not yet be in the block chain
+ * @param  keystore      Temporary keystore containing signing keys
+ * @param  coins         Map of unspent outputs - coins in mempool and current chain UTXO set, may be extended by previous txns outputs after call
+ * @param  tempKeystore  Whether to use temporary keystore
+ * @param  hashType      The signature hash type
+ * @returns JSON object with details of signed transaction
+ */
+UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxs, CBasicKeyStore* keystore, std::map<COutPoint, Coin>& coins, bool tempKeystore, const UniValue& hashType);
 
 /** Create a transaction from univalue parameters */
 CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, const UniValue& rbf);
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 7de74bac4d..5a22508c4b 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3150,7 +3150,14 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request)
     LOCK(pwallet->cs_wallet);
     EnsureWalletIsUnlocked(pwallet);
 
-    return SignTransaction(pwallet->chain(), mtx, request.params[1], pwallet, false, request.params[2]);
+    // Fetch previous transactions (inputs):
+    std::map<COutPoint, Coin> coins;
+    for (const CTxIn& txin : mtx.vin) {
+        coins[txin.prevout]; // Create empty map entry keyed by prevout.
+    }
+    pwallet->chain().findCoins(coins);
+
+    return SignTransaction(mtx, request.params[1], pwallet, coins, false, request.params[2]);
 }
 
 static UniValue bumpfee(const JSONRPCRequest& request)