From 4fcb698bc2bb74171cd3a14b94f9882d8e19e9fb Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Sat, 27 Apr 2019 19:44:38 +0200 Subject: [PATCH] [rpc] walletcreatefundedpsbt: use wallet default RBF --- src/rpc/rawtransaction.cpp | 12 ++++++++++-- src/rpc/rawtransaction_util.cpp | 8 +++----- src/rpc/rawtransaction_util.h | 2 +- src/wallet/rpcwallet.cpp | 10 ++++++++-- test/functional/rpc_psbt.py | 25 +++++++++++++++---------- 5 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 532765b3d..f9e5c436f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -406,7 +406,11 @@ static UniValue createrawtransaction(const JSONRPCRequest& request) }, true ); - CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]); + bool rbf = false; + if (!request.params[3].isNull()) { + rbf = request.params[3].isTrue(); + } + CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); return EncodeHexTx(CTransaction(rawTx)); } @@ -1362,7 +1366,11 @@ UniValue createpsbt(const JSONRPCRequest& request) }, true ); - CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]); + bool rbf = false; + if (!request.params[3].isNull()) { + rbf = request.params[3].isTrue(); + } + CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); // Make a blank psbt PartiallySignedTransaction psbtx; diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 1c96d0123..55425cca3 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -19,7 +19,7 @@ #include #include -CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, const UniValue& rbf) +CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, bool rbf) { if (inputs_in.isNull() || outputs_in.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, arguments 1 and 2 must be non-null"); @@ -37,8 +37,6 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal rawTx.nLockTime = nLockTime; } - bool rbfOptIn = rbf.isTrue(); - for (unsigned int idx = 0; idx < inputs.size(); idx++) { const UniValue& input = inputs[idx]; const UniValue& o = input.get_obj(); @@ -53,7 +51,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout must be positive"); uint32_t nSequence; - if (rbfOptIn) { + if (rbf) { nSequence = MAX_BIP125_RBF_SEQUENCE; /* CTxIn::SEQUENCE_FINAL - 2 */ } else if (rawTx.nLockTime) { nSequence = CTxIn::SEQUENCE_FINAL - 1; @@ -125,7 +123,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal } } - if (!rbf.isNull() && rawTx.vin.size() > 0 && rbfOptIn != SignalsOptInRBF(CTransaction(rawTx))) { + if (rbf && rawTx.vin.size() > 0 && !SignalsOptInRBF(CTransaction(rawTx))) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter combination: Sequence number(s) contradict replaceable option"); } diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index d198887b9..c85593e71 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -27,6 +27,6 @@ class COutPoint; UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxs, FillableSigningProvider* keystore, std::map& 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); +CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, bool rbf); #endif // BITCOIN_RPC_RAWTRANSACTION_UTIL_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7af009f43..7c78d485b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4055,7 +4055,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, - {"replaceable", RPCArg::Type::BOOL, /* default */ "false", "Marks this transaction as BIP125 replaceable.\n" + {"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n" " Allows this transaction to be replaced by a transaction with higher fees"}, {"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" @@ -4090,7 +4090,13 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) CAmount fee; int change_position; - CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]["replaceable"]); + bool rbf = pwallet->m_signal_rbf; + const UniValue &replaceable_arg = request.params[3]["replaceable"]; + if (!replaceable_arg.isNull()) { + RPCTypeCheckArgument(replaceable_arg, UniValue::VBOOL); + rbf = replaceable_arg.isTrue(); + } + CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]); // Make a blank psbt diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index b3d869620..6efb415c5 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -27,6 +27,11 @@ class PSBTTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = False self.num_nodes = 3 + self.extra_args = [ + ["-walletrbf=1"], + [], + [] + ] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -207,18 +212,18 @@ class PSBTTest(BitcoinTestFramework): # replaceable arg block_height = self.nodes[0].getblockcount() unspent = self.nodes[0].listunspent()[0] - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable":True}, False) - decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) - for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): - assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) - assert "bip32_derivs" not in psbt_in - assert_equal(decoded_psbt["tx"]["locktime"], block_height+2) - - # Same construction with only locktime set - psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height, {}, True) + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable": False}, False) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE + assert "bip32_derivs" not in psbt_in + assert_equal(decoded_psbt["tx"]["locktime"], block_height+2) + + # Same construction with only locktime set and RBF explicitly enabled + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height, {"replaceable": True}, True) + decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) + for tx_in, psbt_in in zip(decoded_psbt["tx"]["vin"], decoded_psbt["inputs"]): + assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) assert "bip32_derivs" in psbt_in assert_equal(decoded_psbt["tx"]["locktime"], block_height) @@ -226,7 +231,7 @@ class PSBTTest(BitcoinTestFramework): psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) for tx_in in decoded_psbt["tx"]["vin"]: - assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE + assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) assert_equal(decoded_psbt["tx"]["locktime"], 0) # Make sure change address wallet does not have P2SH innerscript access to results in success