Skip to content

Commit

Permalink
rpc: include_unsafe option for fundrawtransaction
Browse files Browse the repository at this point in the history
Allow RPC users to opt-in to unsafe inputs when funding a raw transaction.

Applications that need to manage a complex RBF flow (such as lightning
nodes using anchor outputs) are very limited if they can only use safe inputs.

Fixes #21299
  • Loading branch information
t-bast committed Mar 17, 2021
1 parent a9d1b40 commit 16076bb
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/wallet/coincontrol.cpp
Expand Up @@ -11,6 +11,7 @@ void CCoinControl::SetNull()
destChange = CNoDestination();
m_change_type.reset();
m_add_inputs = true;
m_include_unsafe_inputs = false;
fAllowOtherInputs = false;
fAllowWatchOnly = false;
m_avoid_partial_spends = gArgs.GetBoolArg("-avoidpartialspends", DEFAULT_AVOIDPARTIALSPENDS);
Expand Down
2 changes: 2 additions & 0 deletions src/wallet/coincontrol.h
Expand Up @@ -29,6 +29,8 @@ class CCoinControl
std::optional<OutputType> m_change_type;
//! If false, only selected inputs are used
bool m_add_inputs;
//! If false, only safe inputs will be used
bool m_include_unsafe_inputs;
//! If false, allows unselected inputs, but requires all selected inputs be used
bool fAllowOtherInputs;
//! Includes watch only addresses which are solvable
Expand Down
8 changes: 8 additions & 0 deletions src/wallet/rpcwallet.cpp
Expand Up @@ -3073,6 +3073,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
RPCTypeCheckObj(options,
{
{"add_inputs", UniValueType(UniValue::VBOOL)},
{"include_unsafe", UniValueType(UniValue::VBOOL)},
{"add_to_wallet", UniValueType(UniValue::VBOOL)},
{"changeAddress", UniValueType(UniValue::VSTR)},
{"change_address", UniValueType(UniValue::VSTR)},
Expand Down Expand Up @@ -3133,6 +3134,10 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
lockUnspents = (options.exists("lock_unspents") ? options["lock_unspents"] : options["lockUnspents"]).get_bool();
}

if (options.exists("include_unsafe")) {
coinControl.m_include_unsafe_inputs = options["include_unsafe"].get_bool();
}

if (options.exists("feeRate")) {
if (options.exists("fee_rate")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both fee_rate (" + CURRENCY_ATOM + "/vB) and feeRate (" + CURRENCY_UNIT + "/kvB)");
Expand Down Expand Up @@ -3203,6 +3208,9 @@ static RPCHelpMan fundrawtransaction()
{"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."},
{"include_unsafe", RPCArg::Type::BOOL, /* default */ "false", "Include inputs that are not safe to spend.\n"
"Beware that 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."},
{"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The bitcoin address to receive the change"},
{"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"},
{"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."},
Expand Down
3 changes: 2 additions & 1 deletion src/wallet/wallet.cpp
Expand Up @@ -2509,6 +2509,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
(m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
(m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
(m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
(m_spend_zero_conf_change && coin_control.m_include_unsafe_inputs && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 0, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) ||
(m_spend_zero_conf_change && !fRejectLongChains && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), std::numeric_limits<uint64_t>::max(), true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used));

// because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset
Expand Down Expand Up @@ -2799,7 +2800,7 @@ bool CWallet::CreateTransactionInternal(
txNew.nLockTime = GetLocktimeForNewTransaction(chain(), GetLastBlockHash(), GetLastBlockHeight());
{
std::vector<COutput> vAvailableCoins;
AvailableCoins(vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
AvailableCoins(vAvailableCoins, !coin_control.m_include_unsafe_inputs, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy
coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;

Expand Down
24 changes: 24 additions & 0 deletions test/functional/rpc_fundrawtransaction.py
Expand Up @@ -96,6 +96,7 @@ def run_test(self):
self.test_option_subtract_fee_from_outputs()
self.test_subtract_fee_with_presets()
self.test_transaction_too_large()
self.test_include_unsafe()

def test_change_position(self):
"""Ensure setting changePosition in fundraw with an exact match is handled properly."""
Expand Down Expand Up @@ -928,6 +929,29 @@ def test_transaction_too_large(self):
self.nodes[0].generate(10)
assert_raises_rpc_error(-4, "Transaction too large", recipient.fundrawtransaction, rawtx)

def test_include_unsafe(self):
self.log.info("Test fundrawtxn with unsafe inputs")

self.nodes[0].createwallet("unsafe")
wallet = self.nodes[0].get_wallet_rpc("unsafe")

# We receive unconfirmed funds from external keys (unsafe output).
addr = wallet.getnewaddress()
txid = self.nodes[2].sendtoaddress(addr, 10)
self.sync_all()
vout = find_vout_for_address(wallet, txid, addr)

# Unsafe inputs are ignored by default.
rawtx = wallet.createrawtransaction([], [{wallet.getnewaddress(): 5}])
assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, rawtx)

# But we can opt-in to use them for funding.
fundedtx = wallet.fundrawtransaction(rawtx, {"include_unsafe": True})
tx_dec = wallet.decoderawtransaction(fundedtx['hex'])
assert any([txin['txid'] == txid and txin['vout'] == vout for txin in tx_dec['vin']])
signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex'])
wallet.sendrawtransaction(signedtx['hex'])


if __name__ == '__main__':
RawTransactionsTest().main()

0 comments on commit 16076bb

Please sign in to comment.