Implement watchonly support in fundrawtransaction

Support watchonly in fundrawtransaction, CreateTransaction, SelectCoins
and coin selection in general.

Coin selection is exposed over RPC through fundrawtransaction, and
watchonly can be enabled by passing includeWatching true (defaults to
false).

fundrawtransaction will first attempt to fund a transaction without
using watchonly, even when includeWatching is enabled. When this fails,
an includeWatching attempt is made.

When an includeWatching attempt at coin selection is performed (in
CreateTransaction), and all watchonlys are consumed leaving no funds
available for a fee, then CreateTransaction will not include a fee in
the transaction.

Also, includeWatching tests (for fundrawtransaction).

See also: bitcoin/bitcoin#5503

Author: kanzure

qa/rpc-tests/rawtransactions.py

@@ -35,17 +35,26 @@ def setup_network(self, split=False):
 
         self.is_network_split=False
         self.sync_all()
-    
+
     def run_test(self):
 
         self.nodes[2].setgenerate(True, 1)
-        self.nodes[0].setgenerate(True, 101)
+        self.nodes[0].setgenerate(True, 121)
         self.sync_all()
+
+        watchonly_address = self.nodes[0].getnewaddress()
+        watchonly_amount = 200
+        self.nodes[1].importaddress(watchonly_address, "", True)
+        self.nodes[0].sendtoaddress(watchonly_address, watchonly_amount)
+
         self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.5);
         self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.0);
         self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),5.0);
+
         self.sync_all()
+
         self.nodes[0].setgenerate(True, 1)
+
         self.sync_all()
 
         ###############
@@ -283,5 +292,32 @@ def run_test(self):
         assert_equal("Insufficient" in errorString, True);
 
 
+        ##################################################
+        # test a fundrawtransaction using only watchonly #
+        ##################################################
+
+        inputs = []
+        outputs = {self.nodes[2].getnewaddress() : watchonly_amount / 2.0}
+        rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
+
+        result = self.nodes[1].fundrawtransaction(rawtx, True)
+
+        assert_equal("hex" in result.keys(), True)
+        assert_equal("fee" in result.keys(), True)
+
+        ###############################################################
+        # test fundrawtransaction using the entirety of watched funds #
+        ###############################################################
+
+        inputs = []
+        outputs = {self.nodes[2].getnewaddress() : watchonly_amount / 1.0}
+        rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
+
+        result = self.nodes[1].fundrawtransaction(rawtx, True)
+
+        assert_equal("hex" in result.keys(), True)
+        assert_equal("fee" in result.keys(), True)
+
+
 if __name__ == '__main__':
     RawTransactionsTest().main()

src/rpcrawtransaction.cpp

@@ -760,13 +760,14 @@ Value sendrawtransaction(const Array& params, bool fHelp)
 #ifdef ENABLE_WALLET
 Value fundrawtransaction(const Array& params, bool fHelp)
 {
-    if (fHelp || params.size() != 1)
+    if (fHelp || params.size() < 1 || params.size() > 2)
         throw runtime_error(
                             "fundrawtransaction \"hexstring\"\n"
                             "\nAdd vIns to a raw transaction.\n"
                             "\nAlso see createrawtransaction and signrawtransaction calls.\n"
                             "\nArguments:\n"
                             "1. \"hexstring\"    (string, required) The hex string of the raw transaction\n"
+                            "2. includeWatching    (boolean, optional, default=false) Use watchonly outputs\n"
                             "\nResult:\n"
                             "{\n"
                             "  \"hex\": \"value\",   (string) The raw transaction with vIns (hex-encoded string)\n"
@@ -792,11 +793,15 @@ Value fundrawtransaction(const Array& params, bool fHelp)
     CTransaction tx;
     if (!DecodeHexTx(tx, params[0].get_str()))
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
-    
+
+    bool includeWatching = false;
+    if (params.size() > 1)
+        includeWatching = params[1].get_bool();
+
     CMutableTransaction txNew;
     CAmount nFeeRet;
     string strFailReason;
-    if(!pwalletMain->FundTransaction(tx, txNew, nFeeRet, strFailReason))
+    if(!pwalletMain->FundTransaction(tx, txNew, nFeeRet, strFailReason, includeWatching))
         throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);
 
     Object result;

src/wallet.cpp

@@ -1152,7 +1152,7 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const
 /**
  * populate vCoins with vector of available COutputs.
  */
-void CWallet::AvailableCoins(vector<COutput>& vCoins, bool fOnlyConfirmed, const CCoinControl *coinControl) const
+void CWallet::AvailableCoins(vector<COutput>& vCoins, bool fOnlyConfirmed, const CCoinControl *coinControl, bool includeWatching) const
 {
     vCoins.clear();
 
@@ -1181,7 +1181,7 @@ void CWallet::AvailableCoins(vector<COutput>& vCoins, bool fOnlyConfirmed, const
                 if (!(IsSpent(wtxid, i)) && mine != ISMINE_NO &&
                     !IsLockedCoin((*it).first, i) && pcoin->vout[i].nValue > 0 &&
                     (!coinControl || !coinControl->HasSelected() || coinControl->IsSelected((*it).first, i)))
-                        vCoins.push_back(COutput(pcoin, i, nDepth, (mine & ISMINE_SPENDABLE) != ISMINE_NO));
+                        vCoins.push_back(COutput(pcoin, i, nDepth, ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (includeWatching && ((mine & ISMINE_WATCH_ONLY) != ISMINE_NO))));
             }
         }
     }
@@ -1334,10 +1334,10 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int
     return true;
 }
 
-bool CWallet::SelectCoins(const CAmount& nTargetValue, set<pair<const CWalletTx*,unsigned int> >& setCoinsRet, CAmount& nValueRet, vector<CTxIn> vPresetVINs, const CCoinControl* coinControl) const
+bool CWallet::SelectCoins(const CAmount& nTargetValue, set<pair<const CWalletTx*,unsigned int> >& setCoinsRet, CAmount& nValueRet, vector<CTxIn> vPresetVINs, const CCoinControl* coinControl, bool includeWatching) const
 {
     vector<COutput> vCoins;
-    AvailableCoins(vCoins, true, coinControl);
+    AvailableCoins(vCoins, true, coinControl, includeWatching);
 
     // create a empty set to store possible VINS
     set<pair<const CWalletTx*,unsigned int> > setTempCoins;
@@ -1404,37 +1404,48 @@ bool CWallet::SelectCoins(const CAmount& nTargetValue, set<pair<const CWalletTx*
 }
 
 
-bool CWallet::FundTransaction(const CTransaction& txToFund, CMutableTransaction& txNew, CAmount &nFeeRet, std::string& strFailReason)
+bool CWallet::FundTransaction(const CTransaction& txToFund, CMutableTransaction& txNew, CAmount &nFeeRet, std::string& strFailReason, bool includeWatching)
 {
-    
+
     vector<pair<CScript, CAmount> > vecSend;
     vector<CTxIn> vin;
-    
+
     BOOST_FOREACH (const CTxOut& txOut, txToFund.vout)
     {
         vecSend.push_back(make_pair(txOut.scriptPubKey, txOut.nValue));
     }
-    
+
     BOOST_FOREACH (const CTxIn& txIn, txToFund.vin)
     {
         vin.push_back(txIn);
     }
-    
+
     CReserveKey reservekey(this);
     CWalletTx wtx;
-    return CreateTransaction(vecSend, vin, wtx, txNew, reservekey, nFeeRet, strFailReason, NULL, false);
+
+    // always try first without including watchonly
+    bool result = CreateTransaction(vecSend, vin, wtx, txNew, reservekey, nFeeRet, strFailReason, NULL, false, false);
+
+    // There may be a solution including watchonly, which may also be a
+    // solution that does not pay any fees.
+    if (!result && includeWatching)
+        result = CreateTransaction(vecSend, vin, wtx, txNew, reservekey, nFeeRet, strFailReason, NULL, false, includeWatching);
+
+    return result;
 }
 
 bool CWallet::CreateTransaction(const vector<pair<CScript, CAmount> >& vecSend,
-                                CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl* coinControl, bool sign)
+                                CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl* coinControl, bool sign, bool includeWatching)
 {
     vector<CTxIn> vINs;
-    return CreateTransaction(vecSend, vINs, wtxNew, txNew, reservekey, nFeeRet, strFailReason, coinControl, sign);
+    return CreateTransaction(vecSend, vINs, wtxNew, txNew, reservekey, nFeeRet, strFailReason, coinControl, sign, includeWatching);
 }
 
 bool CWallet::CreateTransaction(const vector<pair<CScript, CAmount> >& vecSend, const vector<CTxIn> vINs,
-                                CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl* coinControl, bool sign)
+                                CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl* coinControl, bool sign, bool includeWatching)
 {
+    bool cannotFundFee = false; // used with includeWatching
+
     CAmount nValue = 0;
     BOOST_FOREACH (const PAIRTYPE(CScript, CAmount)& s, vecSend)
     {
@@ -1481,10 +1492,22 @@ bool CWallet::CreateTransaction(const vector<pair<CScript, CAmount> >& vecSend,
                 // Choose coins to use
                 set<pair<const CWalletTx*,unsigned int> > setCoins;
                 CAmount nValueIn = 0;
-                if (!SelectCoins(nTotalValue, setCoins, nValueIn, vINs, coinControl))
+                if (!SelectCoins(nTotalValue, setCoins, nValueIn, vINs, coinControl, includeWatching))
                 {
-                    strFailReason = _("Insufficient funds");
-                    return false;
+                    // fee selection is not mandatory when using includeWatching
+                    if (includeWatching && SelectCoins(nTotalValue - nFeeRet, setCoins, nValueIn, vINs, coinControl, includeWatching) && nTotalValue >= nValue)
+                    {
+                        nFeeRet = 0;
+
+                        // no more funds available for fee, but target met anyway
+                        cannotFundFee = true;
+                    }
+
+                    else
+                    {
+                        strFailReason = _("Insufficient funds");
+                        return false;
+                    }
                 }
                 BOOST_FOREACH(PAIRTYPE(const CWalletTx*, unsigned int) pcoin, setCoins)
                 {
@@ -1554,10 +1577,11 @@ bool CWallet::CreateTransaction(const vector<pair<CScript, CAmount> >& vecSend,
                 BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins)
                     txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second));
 
-                // Sign
+                // Sign (also calculate fee)
                 int nIn = 0;
                 BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins)
-                    if (!SignSignature(*this, *coin.first, txNew, nIn++))
+                    // when unsignable and watchonly enabled, this may be a watchonly so don't error
+                    if (!SignSignature(*this, *coin.first, txNew, nIn++) && !includeWatching)
                     {
                         strFailReason = _("Signing transaction failed");
                         return false;
@@ -1573,14 +1597,19 @@ bool CWallet::CreateTransaction(const vector<pair<CScript, CAmount> >& vecSend,
                     strFailReason = _("Transaction too large");
                     return false;
                 }
-                
-                //remove signature if we used the signing only for the fee calculation
+
+                // remove signature if we used the signing only for the fee calculation
                 if(!sign)
                 {
                     BOOST_FOREACH (CTxIn& vin, txNew.vin)
                         vin.scriptSig = CScript();
                 }
-                
+
+                // There are no more available funds for funding a transaction
+                // fee, and includeWatching should de-prioritize fee funding.
+                if (includeWatching && cannotFundFee)
+                    return true;
+
                 dPriority = wtxNew.ComputePriority(dPriority, nBytes);
 
                 // Can we complete this as a free transaction?
@@ -1620,13 +1649,13 @@ bool CWallet::CreateTransaction(const vector<pair<CScript, CAmount> >& vecSend,
 }
 
 bool CWallet::CreateTransaction(CScript scriptPubKey, const CAmount& nValue,
-                                CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl* coinControl, bool sign)
+                                CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl* coinControl, bool sign, bool includeWatching)
 {
     vector< pair<CScript, CAmount> > vecSend;
     vecSend.push_back(make_pair(scriptPubKey, nValue));
     CMutableTransaction txNew;
     vector<CTxIn> vINs;
-    return CreateTransaction(vecSend, vINs, wtxNew, txNew, reservekey, nFeeRet, strFailReason, coinControl, sign);
+    return CreateTransaction(vecSend, vINs, wtxNew, txNew, reservekey, nFeeRet, strFailReason, coinControl, sign, includeWatching);
 }
 
 /**

src/wallet.h

@@ -110,7 +110,7 @@ class CAddressBookData
 class CWallet : public CCryptoKeyStore, public CValidationInterface
 {
 private:
-    bool SelectCoins(const CAmount& nTargetValue, std::set<std::pair<const CWalletTx*,unsigned int> >& setCoinsRet, CAmount& nValueRet, const std::vector<CTxIn> vPresetVINs, const CCoinControl *coinControl = NULL) const;
+    bool SelectCoins(const CAmount& nTargetValue, std::set<std::pair<const CWalletTx*,unsigned int> >& setCoinsRet, CAmount& nValueRet, const std::vector<CTxIn> vPresetVINs, const CCoinControl *coinControl = NULL, bool includeWatching = false) const;
 
     CWalletDB *pwalletdbEncryption;
 
@@ -205,7 +205,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     //! check whether we are allowed to upgrade (or already support) to the named feature
     bool CanSupportFeature(enum WalletFeature wf) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; }
 
-    void AvailableCoins(std::vector<COutput>& vCoins, bool fOnlyConfirmed=true, const CCoinControl *coinControl = NULL) const;
+    void AvailableCoins(std::vector<COutput>& vCoins, bool fOnlyConfirmed=true, const CCoinControl *coinControl = NULL, bool includeWatching = false) const;
     bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, std::vector<COutput> vCoins, std::set<std::pair<const CWalletTx*,unsigned int> >& setCoinsRet, CAmount& nValueRet) const;
 
     bool IsSpent(const uint256& hash, unsigned int n) const;
@@ -288,13 +288,13 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     CAmount GetWatchOnlyBalance() const;
     CAmount GetUnconfirmedWatchOnlyBalance() const;
     CAmount GetImmatureWatchOnlyBalance() const;
-    bool FundTransaction(const CTransaction& txToFund, CMutableTransaction& txNew, CAmount& nFeeRet, std::string& strFailReason);
+    bool FundTransaction(const CTransaction& txToFund, CMutableTransaction& txNew, CAmount& nFeeRet, std::string& strFailReason, bool includeWatching = false);
     bool CreateTransaction(const std::vector<std::pair<CScript, CAmount> >& vecSend, const std::vector<CTxIn> vins,
-                           CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true);
+                           CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true, bool includeWatching = false);
     bool CreateTransaction(const std::vector<std::pair<CScript, CAmount> >& vecSend,
-                           CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true);
+                           CWalletTx& wtxNew, CMutableTransaction& txNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true, bool includeWatching = false);
     bool CreateTransaction(CScript scriptPubKey, const CAmount& nValue,
-                           CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true);
+                           CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true, bool includeWatching = false);
     bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey);
 
     static CFeeRate minTxFee;