diff --git a/doc/release-notes-25375.md b/doc/release-notes-25375.md new file mode 100644 index 0000000000..24605b12f0 --- /dev/null +++ b/doc/release-notes-25375.md @@ -0,0 +1,10 @@ +Updated RPCs +-------- + +The `minconf` option, which allows a user to specify the minimum number +of confirmations a UTXO being spent has, and the `maxconf` option, +which allows specifying the maximum number of confirmations, have been +added to the following RPCs: +- `fundrawtransaction` +- `send` +- `walletcreatefundedpsbt` diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index be1ad37592..280d6a8061 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -528,6 +528,8 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, {"replaceable", UniValueType(UniValue::VBOOL)}, {"conf_target", UniValueType(UniValue::VNUM)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, + {"minconf", UniValueType(UniValue::VNUM)}, + {"maxconf", UniValueType(UniValue::VNUM)}, {"input_weights", UniValueType(UniValue::VARR)}, }, true, true); @@ -593,6 +595,22 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, if (options.exists("replaceable")) { coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); } + + if (options.exists("minconf")) { + coinControl.m_min_depth = options["minconf"].getInt(); + + if (coinControl.m_min_depth < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative minconf"); + } + } + + if (options.exists("maxconf")) { + coinControl.m_max_depth = options["maxconf"].getInt(); + + if (coinControl.m_max_depth < coinControl.m_min_depth) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("maxconf can't be lower than minconf: %d < %d", coinControl.m_max_depth, coinControl.m_min_depth)); + } + } SetFeeEstimateMode(wallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); } } else { @@ -744,6 +762,8 @@ RPCHelpMan fundrawtransaction() {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n" "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n" "If that happens, you will need to fund the transaction with different inputs and republish it."}, + {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "If add_inputs is specified, require inputs with at least this many confirmations."}, + {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If add_inputs is specified, require inputs with at most this many confirmations."}, {"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."}, @@ -1147,6 +1167,8 @@ RPCHelpMan send() {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n" "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n" "If that happens, you will need to fund the transaction with different inputs and republish it."}, + {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "If add_inputs is specified, require inputs with at least this many confirmations."}, + {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If add_inputs is specified, require inputs with at most this many confirmations."}, {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns a serialized transaction which will not be added to the wallet or broadcast"}, {"change_address", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"change_position", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, @@ -1603,6 +1625,8 @@ RPCHelpMan walletcreatefundedpsbt() {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n" "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n" "If that happens, you will need to fund the transaction with different inputs and republish it."}, + {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "If add_inputs is specified, require inputs with at least this many confirmations."}, + {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If add_inputs is specified, require inputs with at most this many confirmations."}, {"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."}, diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index a50e0fb244..58a80e37a2 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -36,6 +36,7 @@ from test_framework.util import ( assert_approx, assert_equal, assert_greater_than, + assert_greater_than_or_equal, assert_raises_rpc_error, find_output, find_vout_for_address, @@ -106,6 +107,65 @@ class PSBTTest(BitcoinTestFramework): self.connect_nodes(0, 1) self.connect_nodes(0, 2) + def test_input_confs_control(self): + self.nodes[0].createwallet("minconf") + wallet = self.nodes[0].get_wallet_rpc("minconf") + + # Fund the wallet with different chain heights + for _ in range(2): + self.nodes[1].sendmany("", {wallet.getnewaddress():1, wallet.getnewaddress():1}) + self.generate(self.nodes[1], 1) + + unconfirmed_txid = wallet.sendtoaddress(wallet.getnewaddress(), 0.5) + + self.log.info("Crafting PSBT using an unconfirmed input") + target_address = self.nodes[1].getnewaddress() + psbtx1 = wallet.walletcreatefundedpsbt([], {target_address: 0.1}, 0, {'fee_rate': 1, 'maxconf': 0})['psbt'] + + # Make sure we only had the one input + tx1_inputs = self.nodes[0].decodepsbt(psbtx1)['tx']['vin'] + assert_equal(len(tx1_inputs), 1) + + utxo1 = tx1_inputs[0] + assert_equal(unconfirmed_txid, utxo1['txid']) + + signed_tx1 = wallet.walletprocesspsbt(psbtx1)['psbt'] + final_tx1 = wallet.finalizepsbt(signed_tx1)['hex'] + txid1 = self.nodes[0].sendrawtransaction(final_tx1) + + mempool = self.nodes[0].getrawmempool() + assert txid1 in mempool + + self.log.info("Fail to craft a new PSBT that sends more funds with add_inputs = False") + assert_raises_rpc_error(-4, "The preselected coins total amount does not cover the transaction target. Please allow other inputs to be automatically selected or include more coins manually", wallet.walletcreatefundedpsbt, [{'txid': utxo1['txid'], 'vout': utxo1['vout']}], {target_address: 1}, 0, {'add_inputs': False}) + + self.log.info("Fail to craft a new PSBT with minconf above highest one") + assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, [{'txid': utxo1['txid'], 'vout': utxo1['vout']}], {target_address: 1}, 0, {'add_inputs': True, 'minconf': 3, 'fee_rate': 10}) + + self.log.info("Fail to broadcast a new PSBT with maxconf 0 due to BIP125 rules to verify it actually chose unconfirmed outputs") + psbt_invalid = wallet.walletcreatefundedpsbt([{'txid': utxo1['txid'], 'vout': utxo1['vout']}], {target_address: 1}, 0, {'add_inputs': True, 'maxconf': 0, 'fee_rate': 10})['psbt'] + signed_invalid = wallet.walletprocesspsbt(psbt_invalid)['psbt'] + final_invalid = wallet.finalizepsbt(signed_invalid)['hex'] + assert_raises_rpc_error(-26, "bad-txns-spends-conflicting-tx", self.nodes[0].sendrawtransaction, final_invalid) + + self.log.info("Craft a replacement adding inputs with highest confs possible") + psbtx2 = wallet.walletcreatefundedpsbt([{'txid': utxo1['txid'], 'vout': utxo1['vout']}], {target_address: 1}, 0, {'add_inputs': True, 'minconf': 2, 'fee_rate': 10})['psbt'] + tx2_inputs = self.nodes[0].decodepsbt(psbtx2)['tx']['vin'] + assert_greater_than_or_equal(len(tx2_inputs), 2) + for vin in tx2_inputs: + if vin['txid'] != unconfirmed_txid: + assert_greater_than_or_equal(self.nodes[0].gettxout(vin['txid'], vin['vout'])['confirmations'], 2) + + signed_tx2 = wallet.walletprocesspsbt(psbtx2)['psbt'] + final_tx2 = wallet.finalizepsbt(signed_tx2)['hex'] + txid2 = self.nodes[0].sendrawtransaction(final_tx2) + + mempool = self.nodes[0].getrawmempool() + assert txid1 not in mempool + assert txid2 in mempool + + wallet.unloadwallet() + def assert_change_type(self, psbtx, expected_type): """Assert that the given PSBT has a change output with the given type.""" @@ -514,6 +574,8 @@ class PSBTTest(BitcoinTestFramework): # TODO: Re-enable this for segwit v1 # self.test_utxo_conversion() + self.test_input_confs_control() + # Test that psbts with p2pkh outputs are created properly p2pkh = self.nodes[0].getnewaddress(address_type='legacy') psbt = self.nodes[1].walletcreatefundedpsbt([], [{p2pkh : 1}], 0, {"includeWatching" : True}, True) diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 98b0f70b01..29ddb77b41 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -148,6 +148,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_external_inputs() self.test_22670() self.test_feerate_rounding() + self.test_input_confs_control() def test_change_position(self): """Ensure setting changePosition in fundraw with an exact match is handled properly.""" @@ -1403,6 +1404,66 @@ class RawTransactionsTest(BitcoinTestFramework): rawtx = w.createrawtransaction(inputs=[], outputs=[{self.nodes[0].getnewaddress(address_type="bech32"): 1 - 0.00000202}]) assert_raises_rpc_error(-4, "Insufficient funds", w.fundrawtransaction, rawtx, {"fee_rate": 1.85}) + def test_input_confs_control(self): + self.nodes[0].createwallet("minconf") + wallet = self.nodes[0].get_wallet_rpc("minconf") + + # Fund the wallet with different chain heights + for _ in range(2): + self.nodes[2].sendmany("", {wallet.getnewaddress():1, wallet.getnewaddress():1}) + self.generate(self.nodes[2], 1) + + unconfirmed_txid = wallet.sendtoaddress(wallet.getnewaddress(), 0.5) + + self.log.info("Crafting TX using an unconfirmed input") + target_address = self.nodes[2].getnewaddress() + raw_tx1 = wallet.createrawtransaction([], {target_address: 0.1}, 0, True) + funded_tx1 = wallet.fundrawtransaction(raw_tx1, {'fee_rate': 1, 'maxconf': 0})['hex'] + + # Make sure we only had the one input + tx1_inputs = self.nodes[0].decoderawtransaction(funded_tx1)['vin'] + assert_equal(len(tx1_inputs), 1) + + utxo1 = tx1_inputs[0] + assert unconfirmed_txid == utxo1['txid'] + + final_tx1 = wallet.signrawtransactionwithwallet(funded_tx1)['hex'] + txid1 = self.nodes[0].sendrawtransaction(final_tx1) + + mempool = self.nodes[0].getrawmempool() + assert txid1 in mempool + + self.log.info("Fail to craft a new TX with minconf above highest one") + # Create a replacement tx to 'final_tx1' that has 1 BTC target instead of 0.1. + raw_tx2 = wallet.createrawtransaction([{'txid': utxo1['txid'], 'vout': utxo1['vout']}], {target_address: 1}) + assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, raw_tx2, {'add_inputs': True, 'minconf': 3, 'fee_rate': 10}) + + self.log.info("Fail to broadcast a new TX with maxconf 0 due to BIP125 rules to verify it actually chose unconfirmed outputs") + # Now fund 'raw_tx2' to fulfill the total target (1 BTC) by using all the wallet unconfirmed outputs. + # As it was created with the first unconfirmed output, 'raw_tx2' only has 0.1 BTC covered (need to fund 0.9 BTC more). + # So, the selection process, to cover the amount, will pick up the 'final_tx1' output as well, which is an output of the tx that this + # new tx is replacing!. So, once we send it to the mempool, it will return a "bad-txns-spends-conflicting-tx" + # because the input will no longer exist once the first tx gets replaced by this new one). + funded_invalid = wallet.fundrawtransaction(raw_tx2, {'add_inputs': True, 'maxconf': 0, 'fee_rate': 10})['hex'] + final_invalid = wallet.signrawtransactionwithwallet(funded_invalid)['hex'] + assert_raises_rpc_error(-26, "bad-txns-spends-conflicting-tx", self.nodes[0].sendrawtransaction, final_invalid) + + self.log.info("Craft a replacement adding inputs with highest depth possible") + funded_tx2 = wallet.fundrawtransaction(raw_tx2, {'add_inputs': True, 'minconf': 2, 'fee_rate': 10})['hex'] + tx2_inputs = self.nodes[0].decoderawtransaction(funded_tx2)['vin'] + assert_greater_than_or_equal(len(tx2_inputs), 2) + for vin in tx2_inputs: + if vin['txid'] != unconfirmed_txid: + assert_greater_than_or_equal(self.nodes[0].gettxout(vin['txid'], vin['vout'])['confirmations'], 2) + + final_tx2 = wallet.signrawtransactionwithwallet(funded_tx2)['hex'] + txid2 = self.nodes[0].sendrawtransaction(final_tx2) + + mempool = self.nodes[0].getrawmempool() + assert txid1 not in mempool + assert txid2 in mempool + + wallet.unloadwallet() if __name__ == '__main__': RawTransactionsTest().main() diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 424834323f..ac3ec06eec 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -45,7 +45,7 @@ class WalletSendTest(BitcoinTestFramework): conf_target=None, estimate_mode=None, fee_rate=None, add_to_wallet=None, psbt=None, inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None, change_type=None, include_watching=None, locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None, - expect_error=None, solving_data=None): + expect_error=None, solving_data=None, minconf=None): assert (amount is None) != (data is None) from_balance_before = from_wallet.getbalances()["mine"]["trusted"] @@ -106,6 +106,8 @@ class WalletSendTest(BitcoinTestFramework): options["subtract_fee_from_outputs"] = subtract_fee_from_outputs if solving_data is not None: options["solving_data"] = solving_data + if minconf is not None: + options["minconf"] = minconf if len(options.keys()) == 0: options = None @@ -487,6 +489,16 @@ class WalletSendTest(BitcoinTestFramework): res = self.test_send(from_wallet=w5, to_wallet=w0, amount=1, include_unsafe=True) assert res["complete"] + self.log.info("Minconf") + self.nodes[1].createwallet(wallet_name="minconfw") + minconfw= self.nodes[1].get_wallet_rpc("minconfw") + self.test_send(from_wallet=w0, to_wallet=minconfw, amount=2) + self.generate(self.nodes[0], 3) + self.test_send(from_wallet=minconfw, to_wallet=w0, amount=1, minconf=4, expect_error=(-4, "Insufficient funds")) + self.test_send(from_wallet=minconfw, to_wallet=w0, amount=1, minconf=-4, expect_error=(-8, "Negative minconf")) + res = self.test_send(from_wallet=minconfw, to_wallet=w0, amount=1, minconf=3) + assert res["complete"] + self.log.info("External outputs") eckey = ECKey() eckey.generate()