fundpsbt: deprecate reserve=true/false usage.

Make it always a number; this makes the JSON request specification
simpler.  We allowed a number since v0.10.1.

(reserve=True is the default anyway, so usually it can be omitted:
reserve=False becomes reserve=0).

Changelog-Deprecated: JSON-RPC: `fundpsbt`/`utxopsbt` `reserve` must be a number, not bool (for `true` use 72/don't specify, for `false` use 0).  Numbers have been allowed since v0.10.1.
This commit is contained in:
Rusty Russell 2022-04-01 14:42:45 +10:30
parent 3feb634d31
commit bf4d9e30d2
8 changed files with 36 additions and 41 deletions

View file

@ -1338,7 +1338,7 @@ class LightningRpc(UnixDomainSocketRpc):
} }
return self.call("unreserveinputs", payload) return self.call("unreserveinputs", payload)
def fundpsbt(self, satoshi, feerate, startweight, minconf=None, reserve=True, locktime=None, min_witness_weight=None, excess_as_change=False): def fundpsbt(self, satoshi, feerate, startweight, minconf=None, reserve=None, locktime=None, min_witness_weight=None, excess_as_change=False):
""" """
Create a PSBT with inputs sufficient to give an output of satoshi. Create a PSBT with inputs sufficient to give an output of satoshi.
""" """
@ -1354,7 +1354,7 @@ class LightningRpc(UnixDomainSocketRpc):
} }
return self.call("fundpsbt", payload) return self.call("fundpsbt", payload)
def utxopsbt(self, satoshi, feerate, startweight, utxos, reserve=True, reservedok=False, locktime=None, min_witness_weight=None, excess_as_change=False): def utxopsbt(self, satoshi, feerate, startweight, utxos, reserve=None, reservedok=False, locktime=None, min_witness_weight=None, excess_as_change=False):
""" """
Create a PSBT with given inputs, to give an output of satoshi. Create a PSBT with given inputs, to give an output of satoshi.
""" """

View file

@ -33,10 +33,9 @@ added any inputs.
*minconf* specifies the minimum number of confirmations that used *minconf* specifies the minimum number of confirmations that used
outputs should have. Default is 1. outputs should have. Default is 1.
*reserve* is either boolean or a number: if *true* or a non-zero If *reserve* if not zero, then *reserveinputs* is called (successfully, with
number then *reserveinputs* is called (successfully, with *exclusive* true) on the returned PSBT for this number of blocks (default
*exclusive* true) on the returned PSBT for this number of blocks (or 72 blocks if unspecified).
72 blocks if *reserve* is simply *true*).
*locktime* is an optional locktime: if not set, it is set to a recent *locktime* is an optional locktime: if not set, it is set to a recent
block height. block height.

View file

@ -23,10 +23,9 @@ the resulting transaction plus *startweight* at the given *feerate*,
with at least *satoshi* left over (unless *satoshi* is **all**, which with at least *satoshi* left over (unless *satoshi* is **all**, which
is equivalent to setting it to zero). is equivalent to setting it to zero).
*reserve* is either boolean or a number: if *true* or a non-zero If *reserve* if not zero, then *reserveinputs* is called (successfully, with
number then *reserveinputs* is called (successfully, with *exclusive* true) on the returned PSBT for this number of blocks (default
*exclusive* true) on the returned PSBT for this number of blocks (or 72 blocks if unspecified).
72 blocks if *reserve* is simply *true*).
Unless *reservedok* is set to true (default is false) it will also fail Unless *reservedok* is set to true (default is false) it will also fail
if any of the *utxos* are already reserved. if any of the *utxos* are already reserved.

View file

@ -455,7 +455,6 @@ listfunds_success(struct command *cmd,
&psbt_funded, &psbt_funded,
&psbt_fund_failed, &psbt_fund_failed,
info); info);
json_add_bool(req->js, "reserve", true);
json_add_string(req->js, "satoshi", json_add_string(req->js, "satoshi",
type_to_string(tmpctx, struct amount_sat, type_to_string(tmpctx, struct amount_sat,
&info->our_funding)); &info->our_funding));

View file

@ -369,7 +369,6 @@ static struct command_result *start_mw(struct multiwithdraw_command *mw)
mw); mw);
json_add_u32(req->js, "minconf", *mw->minconf); json_add_u32(req->js, "minconf", *mw->minconf);
} }
json_add_bool(req->js, "reserve", true);
if (mw->has_all) if (mw->has_all)
json_add_string(req->js, "satoshi", "all"); json_add_string(req->js, "satoshi", "all");
else { else {

View file

@ -320,7 +320,7 @@ def test_channel_abandon(node_factory, bitcoind):
l1.rpc.fundchannel(l2.info['id'], SATS, feerate='1875perkw') l1.rpc.fundchannel(l2.info['id'], SATS, feerate='1875perkw')
opening_utxo = only_one([o for o in l1.rpc.listfunds()['outputs'] if o['reserved']]) opening_utxo = only_one([o for o in l1.rpc.listfunds()['outputs'] if o['reserved']])
psbt = l1.rpc.utxopsbt(0, "253perkw", 0, [opening_utxo['txid'] + ':' + str(opening_utxo['output'])], reserve=False, reservedok=True)['psbt'] psbt = l1.rpc.utxopsbt(0, "253perkw", 0, [opening_utxo['txid'] + ':' + str(opening_utxo['output'])], reserve=0, reservedok=True)['psbt']
# We expect a reservation for 2016 blocks; unreserve it. # We expect a reservation for 2016 blocks; unreserve it.
reservations = only_one(l1.rpc.unreserveinputs(psbt, reserve=2015)['reservations']) reservations = only_one(l1.rpc.unreserveinputs(psbt, reserve=2015)['reservations'])
@ -1178,7 +1178,7 @@ def test_funding_external_wallet_corners(node_factory, bitcoind):
wait_for(lambda: len(l1.rpc.listfunds()["outputs"]) != 0) wait_for(lambda: len(l1.rpc.listfunds()["outputs"]) != 0)
# Some random (valid) psbt # Some random (valid) psbt
psbt = l1.rpc.fundpsbt(amount, '253perkw', 250, reserve=False)['psbt'] psbt = l1.rpc.fundpsbt(amount, '253perkw', 250, reserve=0)['psbt']
with pytest.raises(RpcError, match=r'Unknown peer'): with pytest.raises(RpcError, match=r'Unknown peer'):
l1.rpc.fundchannel_start(l2.info['id'], amount) l1.rpc.fundchannel_start(l2.info['id'], amount)
@ -1296,7 +1296,7 @@ def test_funding_v2_corners(node_factory, bitcoind):
wait_for(lambda: len(l1.rpc.listfunds()["outputs"]) != 0) wait_for(lambda: len(l1.rpc.listfunds()["outputs"]) != 0)
# Some random (valid) psbt # Some random (valid) psbt
psbt = l1.rpc.fundpsbt(amount, '253perkw', 250, reserve=False)['psbt'] psbt = l1.rpc.fundpsbt(amount, '253perkw', 250, reserve=0)['psbt']
nonexist_chanid = '11' * 32 nonexist_chanid = '11' * 32
with pytest.raises(RpcError, match=r'Unknown peer'): with pytest.raises(RpcError, match=r'Unknown peer'):
@ -1324,7 +1324,7 @@ def test_funding_v2_corners(node_factory, bitcoind):
# Should be able to 'restart' after canceling # Should be able to 'restart' after canceling
amount2 = 1000000 amount2 = 1000000
l1.rpc.unreserveinputs(psbt) l1.rpc.unreserveinputs(psbt)
psbt = l1.rpc.fundpsbt(amount2, '253perkw', 250, reserve=False)['psbt'] psbt = l1.rpc.fundpsbt(amount2, '253perkw', 250, reserve=0)['psbt']
l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
start = l1.rpc.openchannel_init(l2.info['id'], amount2, psbt) start = l1.rpc.openchannel_init(l2.info['id'], amount2, psbt)
@ -1455,7 +1455,7 @@ def test_funding_v2_cancel_race(node_factory, bitcoind, executor):
for count, n in enumerate(nodes): for count, n in enumerate(nodes):
l1.rpc.connect(n.info['id'], 'localhost', n.port) l1.rpc.connect(n.info['id'], 'localhost', n.port)
psbt = l1.rpc.fundpsbt(amount, '7500perkw', 250, reserve=False, psbt = l1.rpc.fundpsbt(amount, '7500perkw', 250, reserve=0,
excess_as_change=True, excess_as_change=True,
min_witness_weight=110)['psbt'] min_witness_weight=110)['psbt']
start = l1.rpc.openchannel_init(n.info['id'], amount, psbt) start = l1.rpc.openchannel_init(n.info['id'], amount, psbt)

View file

@ -507,7 +507,7 @@ def test_fundpsbt(node_factory, bitcoind, chainparams):
feerate = '7500perkw' feerate = '7500perkw'
# Should get one input, plus some excess # Should get one input, plus some excess
funding = l1.rpc.fundpsbt(amount // 2, feerate, 0, reserve=False) funding = l1.rpc.fundpsbt(amount // 2, feerate, 0, reserve=0)
psbt = bitcoind.rpc.decodepsbt(funding['psbt']) psbt = bitcoind.rpc.decodepsbt(funding['psbt'])
# We can fuzz this up to 99 blocks back. # We can fuzz this up to 99 blocks back.
assert psbt['tx']['locktime'] > bitcoind.rpc.getblockcount() - 100 assert psbt['tx']['locktime'] > bitcoind.rpc.getblockcount() - 100
@ -520,7 +520,7 @@ def test_fundpsbt(node_factory, bitcoind, chainparams):
assert 'reservations' not in funding assert 'reservations' not in funding
# This should add 99 to the weight, but otherwise be identical (might choose different inputs though!) except for locktime. # This should add 99 to the weight, but otherwise be identical (might choose different inputs though!) except for locktime.
funding2 = l1.rpc.fundpsbt(amount // 2, feerate, 99, reserve=False, locktime=bitcoind.rpc.getblockcount() + 1) funding2 = l1.rpc.fundpsbt(amount // 2, feerate, 99, reserve=0, locktime=bitcoind.rpc.getblockcount() + 1)
psbt2 = bitcoind.rpc.decodepsbt(funding2['psbt']) psbt2 = bitcoind.rpc.decodepsbt(funding2['psbt'])
assert psbt2['tx']['locktime'] == bitcoind.rpc.getblockcount() + 1 assert psbt2['tx']['locktime'] == bitcoind.rpc.getblockcount() + 1
assert len(psbt2['tx']['vin']) == 1 assert len(psbt2['tx']['vin']) == 1
@ -537,7 +537,7 @@ def test_fundpsbt(node_factory, bitcoind, chainparams):
with pytest.raises(RpcError, match=r"not afford"): with pytest.raises(RpcError, match=r"not afford"):
l1.rpc.fundpsbt(amount // 2, feerate, 0, minconf=2) l1.rpc.fundpsbt(amount // 2, feerate, 0, minconf=2)
funding3 = l1.rpc.fundpsbt(amount // 2, feerate, 0, reserve=False, excess_as_change=True) funding3 = l1.rpc.fundpsbt(amount // 2, feerate, 0, reserve=0, excess_as_change=True)
assert funding3['excess_msat'] == Millisatoshi(0) assert funding3['excess_msat'] == Millisatoshi(0)
# Should have the excess msat as the output value (minus fee for change) # Should have the excess msat as the output value (minus fee for change)
psbt = bitcoind.rpc.decodepsbt(funding3['psbt']) psbt = bitcoind.rpc.decodepsbt(funding3['psbt'])
@ -550,7 +550,7 @@ def test_fundpsbt(node_factory, bitcoind, chainparams):
assert funding['excess_msat'] == change + change_fee assert funding['excess_msat'] == change + change_fee
# Should get two inputs. # Should get two inputs.
psbt = bitcoind.rpc.decodepsbt(l1.rpc.fundpsbt(amount, feerate, 0, reserve=False)['psbt']) psbt = bitcoind.rpc.decodepsbt(l1.rpc.fundpsbt(amount, feerate, 0, reserve=0)['psbt'])
assert len(psbt['tx']['vin']) == 2 assert len(psbt['tx']['vin']) == 2
# Should not use reserved outputs. # Should not use reserved outputs.
@ -594,7 +594,7 @@ def test_utxopsbt(node_factory, bitcoind, chainparams):
# Explicitly spend the first output above. # Explicitly spend the first output above.
funding = l1.rpc.utxopsbt(amount // 2, feerate, 0, funding = l1.rpc.utxopsbt(amount // 2, feerate, 0,
['{}:{}'.format(outputs[0][0], outputs[0][1])], ['{}:{}'.format(outputs[0][0], outputs[0][1])],
reserve=False) reserve=0)
psbt = bitcoind.rpc.decodepsbt(funding['psbt']) psbt = bitcoind.rpc.decodepsbt(funding['psbt'])
# We can fuzz this up to 99 blocks back. # We can fuzz this up to 99 blocks back.
assert psbt['tx']['locktime'] > bitcoind.rpc.getblockcount() - 100 assert psbt['tx']['locktime'] > bitcoind.rpc.getblockcount() - 100
@ -610,7 +610,7 @@ def test_utxopsbt(node_factory, bitcoind, chainparams):
start_weight = 99 start_weight = 99
funding2 = l1.rpc.utxopsbt(amount // 2, feerate, start_weight, funding2 = l1.rpc.utxopsbt(amount // 2, feerate, start_weight,
['{}:{}'.format(outputs[0][0], outputs[0][1])], ['{}:{}'.format(outputs[0][0], outputs[0][1])],
reserve=False, locktime=bitcoind.rpc.getblockcount() + 1) reserve=0, locktime=bitcoind.rpc.getblockcount() + 1)
psbt2 = bitcoind.rpc.decodepsbt(funding2['psbt']) psbt2 = bitcoind.rpc.decodepsbt(funding2['psbt'])
assert psbt2['tx']['locktime'] == bitcoind.rpc.getblockcount() + 1 assert psbt2['tx']['locktime'] == bitcoind.rpc.getblockcount() + 1
assert psbt2['tx']['vin'] == psbt['tx']['vin'] assert psbt2['tx']['vin'] == psbt['tx']['vin']
@ -638,7 +638,7 @@ def test_utxopsbt(node_factory, bitcoind, chainparams):
funding3 = l1.rpc.utxopsbt(amount // 2, feerate, 0, funding3 = l1.rpc.utxopsbt(amount // 2, feerate, 0,
['{}:{}'.format(outputs[0][0], outputs[0][1])], ['{}:{}'.format(outputs[0][0], outputs[0][1])],
reserve=False, reserve=0,
excess_as_change=True) excess_as_change=True)
assert funding3['excess_msat'] == Millisatoshi(0) assert funding3['excess_msat'] == Millisatoshi(0)
# Should have the excess msat as the output value (minus fee for change) # Should have the excess msat as the output value (minus fee for change)
@ -655,7 +655,7 @@ def test_utxopsbt(node_factory, bitcoind, chainparams):
funding4 = l1.rpc.utxopsbt(amount - 3500, funding4 = l1.rpc.utxopsbt(amount - 3500,
feerate, 0, feerate, 0,
['{}:{}'.format(outputs[0][0], outputs[0][1])], ['{}:{}'.format(outputs[0][0], outputs[0][1])],
reserve=False, reserve=0,
excess_as_change=True) excess_as_change=True)
assert 'change_outnum' not in funding4 assert 'change_outnum' not in funding4
@ -713,8 +713,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
# Make a PSBT out of our inputs # Make a PSBT out of our inputs
funding = l1.rpc.fundpsbt(satoshi=out_total, funding = l1.rpc.fundpsbt(satoshi=out_total,
feerate=7500, feerate=7500,
startweight=42, startweight=42)
reserve=True)
assert len([x for x in l1.rpc.listfunds()['outputs'] if x['reserved']]) == 4 assert len([x for x in l1.rpc.listfunds()['outputs'] if x['reserved']]) == 4
psbt = bitcoind.rpc.decodepsbt(funding['psbt']) psbt = bitcoind.rpc.decodepsbt(funding['psbt'])
saved_input = psbt['tx']['vin'][0] saved_input = psbt['tx']['vin'][0]
@ -778,8 +777,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
wait_for(lambda: len(l2.rpc.listfunds()['outputs']) == total_outs) wait_for(lambda: len(l2.rpc.listfunds()['outputs']) == total_outs)
l2_funding = l2.rpc.fundpsbt(satoshi=out_total, l2_funding = l2.rpc.fundpsbt(satoshi=out_total,
feerate=7500, feerate=7500,
startweight=42, startweight=42)
reserve=True)
# Try to get L1 to sign it # Try to get L1 to sign it
with pytest.raises(RpcError, match=r"No wallet inputs to sign"): with pytest.raises(RpcError, match=r"No wallet inputs to sign"):
@ -792,8 +790,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
# Add some of our own PSBT inputs to it # Add some of our own PSBT inputs to it
l1_funding = l1.rpc.fundpsbt(satoshi=out_total, l1_funding = l1.rpc.fundpsbt(satoshi=out_total,
feerate=7500, feerate=7500,
startweight=42, startweight=42)
reserve=True)
l1_num_inputs = len(bitcoind.rpc.decodepsbt(l1_funding['psbt'])['tx']['vin']) l1_num_inputs = len(bitcoind.rpc.decodepsbt(l1_funding['psbt'])['tx']['vin'])
l2_num_inputs = len(bitcoind.rpc.decodepsbt(l2_funding['psbt'])['tx']['vin']) l2_num_inputs = len(bitcoind.rpc.decodepsbt(l2_funding['psbt'])['tx']['vin'])
@ -833,8 +830,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
# Send a PSBT that's not ours # Send a PSBT that's not ours
l2_funding = l2.rpc.fundpsbt(satoshi=out_total, l2_funding = l2.rpc.fundpsbt(satoshi=out_total,
feerate=7500, feerate=7500,
startweight=42, startweight=42)
reserve=True)
out_amt = Millisatoshi(l2_funding['excess_msat']) out_amt = Millisatoshi(l2_funding['excess_msat'])
output_psbt = bitcoind.rpc.createpsbt([], output_psbt = bitcoind.rpc.createpsbt([],
[{addr: float((out_total + out_amt).to_btc())}]) [{addr: float((out_total + out_amt).to_btc())}])

View file

@ -4,6 +4,7 @@
#include <bitcoin/script.h> #include <bitcoin/script.h>
#include <ccan/cast/cast.h> #include <ccan/cast/cast.h>
#include <ccan/mem/mem.h> #include <ccan/mem/mem.h>
#include <common/configdir.h>
#include <common/json_command.h> #include <common/json_command.h>
#include <common/json_helpers.h> #include <common/json_helpers.h>
#include <common/json_tok.h> #include <common/json_tok.h>
@ -444,14 +445,16 @@ static struct command_result *param_reserve_num(struct command *cmd,
{ {
bool flag; bool flag;
/* "reserve=true" means 6 hours */ if (deprecated_apis) {
if (json_to_bool(buffer, tok, &flag)) { /* "reserve=true" means 6 hours */
*num = tal(cmd, unsigned int); if (json_to_bool(buffer, tok, &flag)) {
if (flag) *num = tal(cmd, unsigned int);
**num = RESERVATION_DEFAULT; if (flag)
else **num = RESERVATION_DEFAULT;
**num = 0; else
return NULL; **num = 0;
return NULL;
}
} }
return param_number(cmd, name, buffer, tok, num); return param_number(cmd, name, buffer, tok, num);