sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-24 19:42:46 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

merge bitcoin#16377: don't automatically append inputs in walletcreatefundedpsbt

Browse Source 
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
This commit is contained in:
Kittywhiskers Van Gogh 2024-02-06 15:41:26 +00:00
parent bbd1a54897
commit a5da10e29b
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
7 changed files with 63 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
doc/release-notes-5861.md Normal file
View File

@ -0,0 +1,9 @@
RPC changes
-----------
- The `walletcreatefundedpsbt` RPC call will now fail with
`Insufficient funds` when inputs are manually selected but are not enough to cover
the outputs and fee. Additional inputs can automatically be added through the
new `add_inputs` option.
- The `fundrawtransaction` RPC now supports `add_inputs` option that when `false`
prevents adding more inputs if necessary and consequently the RPC fails.

2
src/wallet/coincontrol.cpp
View File

@ -9,6 +9,7 @@
void CCoinControl::SetNull(bool fResetCoinType)
{
destChange = CNoDestination();
m_add_inputs = true;
fAllowOtherInputs = false;
fAllowWatchOnly = false;
m_avoid_partial_spends = gArgs.GetBoolArg("-avoidpartialspends", DEFAULT_AVOIDPARTIALSPENDS);
@ -26,4 +27,3 @@ void CCoinControl::SetNull(bool fResetCoinType)
nCoinType = CoinType::ALL_COINS;
}
}

2
src/wallet/coincontrol.h
View File

@ -37,6 +37,8 @@ class CCoinControl
{
public:
CTxDestination destChange;
//! If false, only selected inputs are used
bool m_add_inputs;
//! If false, allows unselected inputs, but requires all selected inputs be used if fAllowOtherInputs is true (default)
bool fAllowOtherInputs;
//! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with fAllowOtherInputs

29
src/wallet/rpcwallet.cpp
View File

@ -3234,13 +3234,12 @@ static UniValue listunspent(const JSONRPCRequest& request)
return results;
}
void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options)
void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, CCoinControl& coinControl)
{
// Make sure the results are valid at least up to the most recent block
// the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain();
CCoinControl coinControl;
change_position = -1;
bool lockUnspents = false;
UniValue subtractFeeFromOutputs;
@ -3255,6 +3254,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
RPCTypeCheckArgument(options, UniValue::VOBJ);
RPCTypeCheckObj(options,
{
{"add_inputs", UniValueType(UniValue::VBOOL)},
{"changeAddress", UniValueType(UniValue::VSTR)},
{"changePosition", UniValueType(UniValue::VNUM)},
{"includeWatching", UniValueType(UniValue::VBOOL)},
@ -3266,6 +3266,10 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
},
true, true);
if (options.exists("add_inputs") ) {
coinControl.m_add_inputs = options["add_inputs"].get_bool();
}
if (options.exists("changeAddress")) {
CTxDestination dest = DecodeDestination(options["changeAddress"].get_str());
@ -3334,8 +3338,8 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
static UniValue fundrawtransaction(const JSONRPCRequest& request)
{
RPCHelpMan{"fundrawtransaction",
"\nAdd inputs to a transaction until it has enough in value to meet its out value.\n"
"This will not modify existing inputs, and will add at most one change output to the outputs.\n"
"\nIf the transaction has no inputs, they will be automatically selected to meet its out value.\n"
"It will add at most one change output to the outputs.\n"
"No existing outputs will be modified unless \"subtractFeeFromOutputs\" is specified.\n"
"Note that inputs which were signed may need to be resigned after completion since in/outputs have been added.\n"
"The inputs added will not be signed, use signrawtransactionwithkey\n"
@ -3349,6 +3353,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request)
{"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"},
{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}",
{
{"add_inputs", RPCArg::Type::BOOL, /* default */ "true", "For a transaction with existing inputs, automatically include more if they are not enough."},
{"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The Dash address to receive the change"},
{"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"},
{"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n"
@ -3404,7 +3409,10 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request)
CAmount fee;
int change_position;
FundTransaction(pwallet, tx, fee, change_position, request.params[1]);
CCoinControl coin_control;
// Automatically select (additional) coins. Can be overriden by options.add_inputs.
coin_control.m_add_inputs = true;
FundTransaction(pwallet, tx, fee, change_position, request.params[1], coin_control);
UniValue result(UniValue::VOBJ);
result.pushKV("hex", EncodeHexTx(CTransaction(tx)));
@ -4066,10 +4074,10 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request)
UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
{
RPCHelpMan{"walletcreatefundedpsbt",
"\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n"
"\nCreates and funds a transaction in the Partially Signed Transaction format.\n"
"Implements the Creator and Updater roles.\n",
{
{"inputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The inputs",
{"inputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The inputs. Leave empty to add inputs automatically. See add_inputs option.",
{
{"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
{
@ -4100,6 +4108,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
{"locktime", RPCArg::Type::NUM, /* default */ "0", "Raw locktime. Non-0 value also locktime-activates inputs"},
{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
{
{"add_inputs", RPCArg::Type::BOOL, /* default */ "false", "If inputs are specified, automatically include more if they are not enough."},
{"changeAddress", RPCArg::Type::STR_HEX, /* default */ "pool address", "The Dash address to receive the change"},
{"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"},
{"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only"},
@ -4150,7 +4159,11 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
CAmount fee;
int change_position;
CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2]);
FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]);
CCoinControl coin_control;
// Automatically select coins, unless at least one is manually selected. Can
// be overriden by options.add_inputs.
coin_control.m_add_inputs = rawTx.vin.size() == 0;
FundTransaction(pwallet, rawTx, fee, change_position, request.params[3], coin_control);
// Make a blank psbt
PartiallySignedTransaction psbtx{rawTx};

5
src/wallet/wallet.cpp
View File

@ -2625,6 +2625,11 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
}
if(!found) continue;
// Only consider selected coins if add_inputs is false
if (coinControl && !coinControl->m_add_inputs && !coinControl->IsSelected(COutPoint(wtxid, i))) {
continue;
}
if (pcoin->tx->vout[i].nValue < nMinimumAmount || pcoin->tx->vout[i].nValue > nMaximumAmount)
continue;

14
test/functional/rpc_fundrawtransaction.py
View File

@ -264,7 +264,11 @@ class RawTransactionsTest(BitcoinTestFramework):
assert_equal(utx['txid'], dec_tx['vin'][0]['txid'])
assert_equal("00", dec_tx['vin'][0]['scriptSig']['hex'])
# Should fail without add_inputs:
assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False})
# add_inputs is enabled by default
rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
totalOut = 0
matchingOuts = 0
@ -292,7 +296,10 @@ class RawTransactionsTest(BitcoinTestFramework):
dec_tx = self.nodes[2].decoderawtransaction(rawtx)
assert_equal(utx['txid'], dec_tx['vin'][0]['txid'])
rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
# Should fail without add_inputs:
assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False})
rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True})
dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
totalOut = 0
matchingOuts = 0
@ -323,7 +330,10 @@ class RawTransactionsTest(BitcoinTestFramework):
dec_tx = self.nodes[2].decoderawtransaction(rawtx)
assert_equal(utx['txid'], dec_tx['vin'][0]['txid'])
rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
# Should fail without add_inputs:
assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx, {"add_inputs": False})
rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True})
dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
totalOut = 0
matchingOuts = 0

19
test/functional/rpc_psbt.py
View File

@ -8,8 +8,8 @@
from decimal import Decimal
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_approx,
assert_equal,
assert_greater_than,
assert_raises_rpc_error,
find_output
)
@ -31,6 +31,13 @@ class PSBTTest(BitcoinTestFramework):
# Create and fund a raw tx for sending 10 DASH
psbtx1 = self.nodes[0].walletcreatefundedpsbt([], {self.nodes[2].getnewaddress():10})['psbt']
# If inputs are specified, do not automatically add more:
utxo1 = self.nodes[0].listunspent()[0]
assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[0].walletcreatefundedpsbt, [{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():900})
psbtx1 = self.nodes[0].walletcreatefundedpsbt([{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():900}, 0, {"add_inputs": True})['psbt']
assert_equal(len(self.nodes[0].decodepsbt(psbtx1)['tx']['vin']), 2)
# Node 1 should not be able to add anything to it but still return the psbtx same as before
psbtx = self.nodes[1].walletprocesspsbt(psbtx1)['psbt']
assert_equal(psbtx1, psbtx)
@ -96,16 +103,16 @@ class PSBTTest(BitcoinTestFramework):
self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex'])
# feeRate of 0.1 DASH / KB produces a total fee slightly below -maxtxfee (~0.06650000):
res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 0.1}, False)
assert_greater_than(res["fee"], 0.03)
assert_greater_than(0.04, res["fee"])
res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 0.1, "add_inputs": True}, False)
assert_approx(res["fee"], 0.04, 0.03)
decoded_psbt = self.nodes[0].decodepsbt(res['psbt'])
for psbt_in in decoded_psbt["inputs"]:
assert "bip32_derivs" not in psbt_in
# feeRate of 10 DASH / KB produces a total fee well above -maxtxfee
# previously this was silently capped at -maxtxfee
assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 10})
assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}, 0, {"feeRate": 10, "add_inputs": True})
assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():1}, 0, {"feeRate": 10, "add_inputs": True})
# partially sign multisig things with node 1
psbtx = wmulti.walletcreatefundedpsbt(inputs=[{"txid":txid,"vout":p2sh_pos}], outputs={self.nodes[1].getnewaddress():9.99}, options={'changeAddress': self.nodes[1].getrawchangeaddress()})['psbt']
@ -185,7 +192,7 @@ class PSBTTest(BitcoinTestFramework):
# Regression test for 14473 (mishandling of already-signed witness transaction):
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}])
psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], 0, {"add_inputs": True})
complete_psbt = self.nodes[0].walletprocesspsbt(psbtx_info["psbt"])
double_processed_psbt = self.nodes[0].walletprocesspsbt(complete_psbt["psbt"])
assert_equal(complete_psbt, double_processed_psbt)