fundpsbt: make parameters more usable.

fundpsbt forces the caller to manually add their weight * feerate
to the satoshis they ask for.  That means no named feerates.

Instead, create a startweight parameter and do the calc for them
internally, and return the feerate we used (and, while we're at it,
the estimated final weight).

This API change is best done now, as it would otherwise have to
be appended as a parameter.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Author: rustyrussell

contrib/pyln-client/pyln/client/lightning.py

@@ -1126,13 +1126,14 @@ def unreserveinputs(self, psbt):
         }
         return self.call("unreserveinputs", payload)
 
-    def fundpsbt(self, satoshi, feerate, minconf=None, reserve=True):
+    def fundpsbt(self, satoshi, feerate, startweight, minconf=None, reserve=True):
         """
         Create a PSBT with inputs sufficient to give an output of satoshi.
         """
         payload = {
             "satoshi": satoshi,
             "feerate": feerate,
+            "startweight": startweight,
             "minconf": minconf,
             "reserve": reserve,
         }

doc/lightning-fundpsbt.7

@@ -4,7 +4,7 @@ lightning-fundpsbt -- Command to populate PSBT inputs from the wallet
 SYNOPSIS
 --------
 
-**fundpsbt** *satoshi* *feerate* \[*minconf*\] \[*reserve*\]
+**fundpsbt** *satoshi* *feerate* *startweight* \[*minconf*\] \[*reserve*\]
 
 DESCRIPTION
 -----------
@@ -18,30 +18,54 @@ be a whole number, a whole number ending in *sat*, a whole number
 ending in *000msat*, or a number with 1 to 8 decimal places ending in
 *btc*.
 
-You calculate the value by starting with the amount you want to pay
-and adding the fee which will be needed to pay for the base of the
-transaction plus that output, and any other outputs and inputs you
-will add to the final transaction.
+*feerate* can be one of the feerates listed in lightning-feerates(7),
+or one of the strings *urgent* (aim for next block), *normal* (next 4
+blocks or so) or *slow* (next 100 blocks or so) to use lightningd’s
+internal estimates.  It can also be a *feerate* is a number, with an
+optional suffix: *perkw* means the number is interpreted as
+satoshi-per-kilosipa (weight), and *perkb* means it is interpreted
+bitcoind-style as satoshi-per-kilobyte. Omitting the suffix is
+equivalent to *perkb*.
 
-*feerate* is a number, with an optional suffix: *perkw* means the
-number is interpreted as satoshi-per-kilosipa (weight), and *perkb*
-means it is interpreted bitcoind-style as
-satoshi-per-kilobyte. Omitting the suffix is equivalent to *perkb*.
+*startweight* is the weight of the transaction before *fundpsbt* has
+added any inputs.
 
 *minconf* specifies the minimum number of confirmations that used
 outputs should have. Default is 1.
 
 *reserve* is a boolean: if true (the default), then *reserveinputs* is
 called (successfully, with *exclusive* true) on the returned PSBT.
 
+EXAMPLE USAGE
+-------------
+
+Let's assume the caller is trying to produce a 100,000 satoshi output.
+
+First, the caller estimates the weight of the core (typically 42) and
+known outputs of the transaction (typically (9 + scriptlen) * 4).  For
+a simple P2WPKH it's a 22 byte scriptpubkey, so that's 164 weight.
+
+It calls "*fundpsbt* 100000sat slow 206", which succeeds, and returns
+the *psbt* and *feerate_per_kw* it used, the *estimated_final_weight*
+and any *excess_msat*.
+
+If *excess_msat* is greater than the cost of adding a change output,
+the caller adds a change output randomly to position 0 or 1 in the
+PSBT.  Say *feerate_per_kw* is 253, and the change output is a P2WPKH
+(weight 164), that would cost the cost is around 41 sats.  With the
+dust limit disallowing payments below 546 satoshis, we would only create
+a change output if *excess_msat* was greater or equal to 41 + 546.
+
 RETURN VALUE
 ------------
 
-On success, returns the *psbt* containing the inputs, and
+On success, returns the *psbt* containing the inputs, *feerate_per_kw*
+showing the exact numeric feerate it used, *estimated_final_weight* for
+the estimated weight of the transaction once fully signed, and
 *excess_msat* containing the amount above *satoshi* which is
 available.  This could be zero, or dust.  If *satoshi* was "all",
 then *excess_msat* is the entire amount once fees are subtracted
-for the weights of the inputs.
+for the weights of the inputs and startweight.
 
 If *reserve* was true, then a *reservations* array is returned,
 exactly like *reserveinputs*.

doc/lightning-fundpsbt.7.md

@@ -506,41 +506,53 @@ def test_fundpsbt(node_factory, bitcoind, chainparams):
     feerate = '7500perkw'
 
     # Should get one input, plus some excess
-    funding = l1.rpc.fundpsbt(amount // 2, feerate, reserve=False)
+    funding = l1.rpc.fundpsbt(amount // 2, feerate, 0, reserve=False)
     psbt = bitcoind.rpc.decodepsbt(funding['psbt'])
     assert len(psbt['tx']['vin']) == 1
     assert funding['excess_msat'] > Millisatoshi(0)
     assert funding['excess_msat'] < Millisatoshi(amount // 2 * 1000)
+    assert funding['feerate_per_kw'] == 7500
+    assert 'estimated_final_weight' in funding
+    assert 'reservations' not in funding
+
+    # This should add 99 to the weight, but otherwise be identical (might choose different inputs though!)
+    funding2 = l1.rpc.fundpsbt(amount // 2, feerate, 99, reserve=False)
+    psbt2 = bitcoind.rpc.decodepsbt(funding2['psbt'])
+    assert len(psbt2['tx']['vin']) == 1
+    assert funding2['excess_msat'] < funding['excess_msat']
+    assert funding2['feerate_per_kw'] == 7500
+    # Naively you'd expect this to be +99, but it might have selected a non-p2sh output...
+    assert funding2['estimated_final_weight'] > funding['estimated_final_weight']
 
     # Cannot afford this one (too much)
     with pytest.raises(RpcError, match=r"not afford"):
-        l1.rpc.fundpsbt(amount * total_outs, feerate)
+        l1.rpc.fundpsbt(amount * total_outs, feerate, 0)
 
     # Nor this (depth insufficient)
     with pytest.raises(RpcError, match=r"not afford"):
-        l1.rpc.fundpsbt(amount // 2, feerate, minconf=2)
+        l1.rpc.fundpsbt(amount // 2, feerate, 0, minconf=2)
 
     # Should get two inputs.
-    psbt = bitcoind.rpc.decodepsbt(l1.rpc.fundpsbt(amount, feerate, reserve=False)['psbt'])
+    psbt = bitcoind.rpc.decodepsbt(l1.rpc.fundpsbt(amount, feerate, 0, reserve=False)['psbt'])
     assert len(psbt['tx']['vin']) == 2
 
     # Should not use reserved outputs.
     psbt = bitcoind.rpc.createpsbt([{'txid': out[0], 'vout': out[1]} for out in outputs], [])
     l1.rpc.reserveinputs(psbt)
     with pytest.raises(RpcError, match=r"not afford"):
-        l1.rpc.fundpsbt(amount // 2, feerate)
+        l1.rpc.fundpsbt(amount // 2, feerate, 0)
 
     # Will use first one if unreserved.
     l1.rpc.unreserveinputs(bitcoind.rpc.createpsbt([{'txid': outputs[0][0], 'vout': outputs[0][1]}], []))
-    psbt = l1.rpc.fundpsbt(amount // 2, feerate)['psbt']
+    psbt = l1.rpc.fundpsbt(amount // 2, feerate, 0)['psbt']
 
     # Should have passed to reserveinputs.
     with pytest.raises(RpcError, match=r"already reserved"):
         l1.rpc.reserveinputs(psbt)
 
     # And now we can't afford any more.
     with pytest.raises(RpcError, match=r"not afford"):
-        l1.rpc.fundpsbt(amount // 2, feerate)
+        l1.rpc.fundpsbt(amount // 2, feerate, 0)
 
 
 def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
@@ -564,9 +576,10 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
     bitcoind.generate_block(1)
     wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == total_outs)
 
-    # Make a PSBT out of our inputs (FIXME: satoshi amount should include fees!)
+    # Make a PSBT out of our inputs
     funding = l1.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000),
                               feerate=7500,
+                              startweight=42,
                               reserve=True)
     assert len([x for x in l1.rpc.listfunds()['outputs'] if x['reserved']]) == 4
     psbt = bitcoind.rpc.decodepsbt(funding['psbt'])
@@ -628,6 +641,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
     wait_for(lambda: len(l2.rpc.listfunds()['outputs']) == total_outs)
     l2_funding = l2.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000),
                                  feerate=7500,
+                                 startweight=42,
                                  reserve=True)
 
     # Try to get L1 to sign it
@@ -637,6 +651,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
     # Add some of our own PSBT inputs to it
     l1_funding = l1.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000),
                                  feerate=7500,
+                                 startweight=42,
                                  reserve=True)
 
     # Join and add an output
@@ -652,6 +667,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams):
     # Send a PSBT that's not ours
     l2_funding = l2.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000),
                                  feerate=7500,
+                                 startweight=42,
                                  reserve=True)
     psbt = bitcoind.rpc.joinpsbts([l2_funding['psbt'], output_pbst])
     l2_signed_psbt = l2.rpc.signpsbt(psbt)['signed_psbt']

tests/test_wallet.py

@@ -184,15 +184,16 @@ static struct command_result *json_fundpsbt(struct command *cmd,
 	struct json_stream *response;
 	struct utxo **utxos;
 	u32 *feerate_per_kw;
-	u32 *minconf;
+	u32 *minconf, *weight;
 	struct amount_sat *amount, input, needed, excess, total_fee;
 	bool all, *reserve;
 	u32 locktime, maxheight, current_height;
 	struct bitcoin_tx *tx;
 
 	if (!param(cmd, buffer, params,
 		   p_req("satoshi", param_sat_or_all, &amount),
-		   p_req("feerate", param_feerate_val, &feerate_per_kw),
+		   p_req("feerate", param_feerate, &feerate_per_kw),
+		   p_req("startweight", param_number, &weight),
 		   p_opt_def("minconf", param_number, &minconf, 1),
 		   p_opt_def("reserve", param_bool, &reserve, true),
 		   NULL))
@@ -203,10 +204,15 @@ static struct command_result *json_fundpsbt(struct command *cmd,
 
 	current_height = get_block_height(cmd->ld->topology);
 
+	/* Can overflow if amount is "all" */
+	if (!amount_sat_add(amount, *amount,
+			    amount_tx_fee(*feerate_per_kw, *weight)))
+		;
+
 	/* We keep adding until we meet their output requirements. */
 	input = AMOUNT_SAT(0);
 	utxos = tal_arr(cmd, struct utxo *, 0);
-	total_fee = AMOUNT_SAT(0);
+	total_fee = amount_tx_fee(*feerate_per_kw, *weight);
 	while (amount_sat_sub(&needed, *amount, input)
 	       && !amount_sat_eq(needed, AMOUNT_SAT(0))) {
 		struct utxo *utxo;
@@ -227,6 +233,7 @@ static struct command_result *json_fundpsbt(struct command *cmd,
 						    "impossible UTXO value");
 
 			/* But increase amount needed, to pay for new input */
+			*weight += utxo_spend_weight(utxo);
 			fee = amount_tx_fee(*feerate_per_kw,
 					    utxo_spend_weight(utxo));
 			if (!amount_sat_add(amount, *amount, fee))
@@ -300,6 +307,8 @@ static struct command_result *json_fundpsbt(struct command *cmd,
 
 	response = json_stream_success(cmd);
 	json_add_psbt(response, "psbt", tx->psbt);
+	json_add_num(response, "feerate_per_kw", *feerate_per_kw);
+	json_add_num(response, "estimated_final_weight", *weight);
 	json_add_amount_sat_only(response, "excess_msat", excess);
 	if (*reserve)
 		reserve_and_report(response, cmd->ld->wallet, current_height,