Merge bitcoin#12194: Add change type option to fundrawtransaction

16f6f59 [qa] Test fundrawtransaction with change_type option (João Barbosa) 536ddeb [rpc] Add change_type option to fundrawtransaction (João Barbosa) 31dbd5a [wallet] Add change type to CCoinControl (João Barbosa) Pull request description: Adds a new option `change_type` to `fundrawtransaction` RPC. This is useful to override the node `-changetype` argument. The new option is exclusive to `changeAddress` option, setting both raises a RPC error. See also bitcoin#11403, bitcoin#12119. Tree-SHA512: 654686444f6125e37015a62f167064d54ec335701534988447be4687fa5ef9c7980a8a07cc0a03fff6ea6c4c1abf0f77a8843d535c4f3fe0bf93f968a4e676e6
totothink · Jan 24, 2018 · 7abb0f0 · 7abb0f0
2 parents eadb2da + 16f6f59
commit 7abb0f0
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 5 deletions.
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
@@ -16,7 +16,10 @@
 class CCoinControl
 {
 public:
+    //! Custom change destination, if not set an address is generated
     CTxDestination destChange;
+    //! Custom change type, ignored if destChange is set, defaults to g_change_type
+    OutputType change_type;
     //! If false, allows unselected inputs, but requires all selected inputs be used
     bool fAllowOtherInputs;
     //! Includes watch only addresses which match the ISMINE_WATCH_SOLVABLE criteria
@@ -40,6 +43,7 @@ class CCoinControl
     void SetNull()
     {
         destChange = CNoDestination();
+        change_type = g_change_type;
         fAllowOtherInputs = false;
         fAllowWatchOnly = false;
         setSelected.clear();

diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -3065,6 +3065,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
                             "   {\n"
                             "     \"changeAddress\"          (string, optional, default pool address) The bitcoin address to receive the change\n"
                             "     \"changePosition\"         (numeric, optional, default random) The index of the change output\n"
+                            "     \"change_type\"            (string, optional) The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\". Default is set by -changetype.\n"
                             "     \"includeWatching\"        (boolean, optional, default false) Also select inputs which are watch only\n"
                             "     \"lockUnspents\"           (boolean, optional, default false) Lock selected unspent outputs\n"
                             "     \"feeRate\"                (numeric, optional, default not set: makes wallet determine the fee) Set a specific fee rate in " + CURRENCY_UNIT + "/kB\n"
@@ -3130,6 +3131,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
             {
                 {"changeAddress", UniValueType(UniValue::VSTR)},
                 {"changePosition", UniValueType(UniValue::VNUM)},
+                {"change_type", UniValueType(UniValue::VSTR)},
                 {"includeWatching", UniValueType(UniValue::VBOOL)},
                 {"lockUnspents", UniValueType(UniValue::VBOOL)},
                 {"reserveChangeKey", UniValueType(UniValue::VBOOL)}, // DEPRECATED (and ignored), should be removed in 0.16 or so.
@@ -3154,6 +3156,16 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
         if (options.exists("changePosition"))
             changePosition = options["changePosition"].get_int();
 
+        if (options.exists("change_type")) {
+            if (options.exists("changeAddress")) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both changeAddress and address_type options");
+            }
+            coinControl.change_type = ParseOutputType(options["change_type"].get_str(), coinControl.change_type);
+            if (coinControl.change_type == OUTPUT_TYPE_NONE) {
+                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown change type '%s'", options["change_type"].get_str()));
+            }
+        }
+
         if (options.exists("includeWatching"))
             coinControl.fAllowWatchOnly = options["includeWatching"].get_bool();
 

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -2674,11 +2674,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
     return true;
 }
 
-OutputType CWallet::TransactionChangeType(const std::vector<CRecipient>& vecSend)
+OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vector<CRecipient>& vecSend)
 {
     // If -changetype is specified, always use that change type.
-    if (g_change_type != OUTPUT_TYPE_NONE) {
-        return g_change_type;
+    if (change_type != OUTPUT_TYPE_NONE) {
+        return change_type;
     }
 
     // if g_address_type is legacy, use legacy address as change (even
@@ -2797,7 +2797,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                     return false;
                 }
 
-                const OutputType change_type = TransactionChangeType(vecSend);
+                const OutputType change_type = TransactionChangeType(coin_control.change_type, vecSend);
 
                 LearnRelatedScripts(vchPubKey, change_type);
                 scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, change_type));

diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -965,7 +965,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
     CAmount GetLegacyBalance(const isminefilter& filter, int minDepth, const std::string* account) const;
     CAmount GetAvailableBalance(const CCoinControl* coinControl = nullptr) const;
 
-    OutputType TransactionChangeType(const std::vector<CRecipient>& vecSend);
+    OutputType TransactionChangeType(OutputType change_type, const std::vector<CRecipient>& vecSend);
 
     /**
      * Insert additional inputs into the transaction by

diff --git a/test/functional/fundrawtransaction.py b/test/functional/fundrawtransaction.py
@@ -212,6 +212,19 @@ def run_test(self):
         out = dec_tx['vout'][0]
         assert_equal(change, out['scriptPubKey']['addresses'][0])
 
+        #########################################################
+        # test a fundrawtransaction with a provided change type #
+        #########################################################
+        utx = get_unspent(self.nodes[2].listunspent(), 5)
+
+        inputs  = [ {'txid' : utx['txid'], 'vout' : utx['vout']} ]
+        outputs = { self.nodes[0].getnewaddress() : Decimal(4.0) }
+        rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
+        assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[2].fundrawtransaction, rawtx, {'change_type': None})
+        assert_raises_rpc_error(-5, "Unknown change type", self.nodes[2].fundrawtransaction, rawtx, {'change_type': ''})
+        rawtx = self.nodes[2].fundrawtransaction(rawtx, {'change_type': 'bech32'})
+        tx  = self.nodes[2].decoderawtransaction(rawtx['hex'])
+        assert_equal('witness_v0_keyhash', tx['vout'][rawtx['changepos']]['scriptPubKey']['type'])
 
         #########################################################################
         # test a fundrawtransaction with a VIN smaller than the required amount #