sendpsbt: just reserve (maybe bump) inputs on send, don't mark spent.

Marking spent means if the transaction doesn't confirm for some
reason, the user will need to force a rescan to find the funds.  Now
we have timed reservations, reserving for (an additional) 12 hours
should be sufficient.

We also take this opportunity (now we have our own callback path)
to record the tx in the wallet only on success.

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

tests/test_closing.py

@@ -1543,7 +1543,7 @@ Make sure we show the address.
     channel_id = first_channel_id(l1, l2)
 
     # listfunds will show 1 output change, and channels.
-    assert len(l1.rpc.listfunds()['outputs']) == 1
+    assert len([o for o in l1.rpc.listfunds()['outputs'] if not o['reserved']]) == 1
 
     l1.stop()
     l2.rpc.close(l1.info['id'], unilateraltimeout=1)

tests/test_connection.py

@@ -703,6 +703,8 @@ def test_funding_change(node_factory, bitcoind):
     assert only_one(outputs)['value'] == 10000000
 
     l1.rpc.fundchannel(l2.info['id'], 1000000)
+    bitcoind.generate_block(1, wait_for_mempool=1)
+    sync_blockheight(bitcoind, [l1])
     outputs = {r['status']: r['value'] for r in l1.db_query(
         'SELECT status, SUM(value) AS value FROM outputs GROUP BY status;')}
 
@@ -734,10 +736,21 @@ def test_funding_all_too_much(node_factory):
     """
     l1, l2 = node_factory.line_graph(2, fundchannel=False)
 
-    l1.fundwallet(2**24 + 10000)
+    addr, txid = l1.fundwallet(2**24 + 10000)
     l1.rpc.fundchannel(l2.info['id'], "all")
 
-    assert only_one(l1.rpc.listfunds()['outputs'])['status'] == 'unconfirmed'
+    # One reserved, confirmed output spent above, and one change.
+    outputs = l1.rpc.listfunds()['outputs']
+
+    spent = only_one([o for o in outputs if o['status'] == 'confirmed'])
+
+    assert spent['txid'] == txid
+    assert spent['address'] == addr
+    assert spent['reserved'] is True
+
+    pending = only_one([o for o in outputs if o['status'] != 'confirmed'])
+    assert pending['status'] == 'unconfirmed'
+    assert pending['reserved'] is False
     assert only_one(l1.rpc.listfunds()['channels'])['channel_total_sat'] == 2**24 - 1
 
 
@@ -877,6 +890,13 @@ def test_funding_by_utxos(node_factory, bitcoind):
     l1.rpc.connect(l3.info["id"], "localhost", l3.port)
     l1.rpc.fundchannel(l3.info["id"], int(0.007 * 10**8), utxos=[utxos[2]])
 
+    # Fund another channel with already reserved utxos
+    with pytest.raises(RpcError, match=r"UTXO.*already reserved"):
+        l1.rpc.fundchannel(l3.info["id"], int(0.01 * 10**8), utxos=utxos)
+
+    bitcoind.generate_block(1, wait_for_mempool=1)
+    sync_blockheight(bitcoind, [l1])
+
     # Fund another channel with already spent utxos
     with pytest.raises(RpcError, match=r"Already spent UTXO "):
         l1.rpc.fundchannel(l3.info["id"], int(0.01 * 10**8), utxos=utxos)

wallet/wallet.c

@@ -3208,13 +3208,15 @@ wallet_outpoint_spend(struct wallet *w, const tal_t *ctx, const u32 blockheight,
 	int changes;
 	if (outpointfilter_matches(w->owned_outpoints, txid, outnum)) {
 		stmt = db_prepare_v2(w->db, SQL("UPDATE outputs "
-						"SET spend_height = ? "
+						"SET spend_height = ?, "
+						" status = ? "
 						"WHERE prev_out_tx = ?"
 						" AND prev_out_index = ?"));
 
 		db_bind_int(stmt, 0, blockheight);
-		db_bind_sha256d(stmt, 1, &txid->shad);
-		db_bind_int(stmt, 2, outnum);
+		db_bind_int(stmt, 1, output_status_in_db(output_state_spent));
+		db_bind_sha256d(stmt, 2, &txid->shad);
+		db_bind_int(stmt, 3, outnum);
 
 		db_exec_prepared_v2(take(stmt));
 

wallet/walletrpc.c

@@ -1043,25 +1043,69 @@ static const struct json_command signpsbt_command = {
 
 AUTODATA(json_command, &signpsbt_command);
 
+struct sending_psbt {
+	struct command *cmd;
+	struct utxo **utxos;
+	struct wally_tx *wtx;
+};
+
+static void sendpsbt_done(struct bitcoind *bitcoind UNUSED,
+			  bool success, const char *msg,
+			  struct sending_psbt *sending)
+{
+	struct lightningd *ld = sending->cmd->ld;
+	struct json_stream *response;
+	struct bitcoin_txid txid;
+	struct amount_sat change;
+
+	if (!success) {
+		/* Unreserve the inputs again. */
+		for (size_t i = 0; i < tal_count(sending->utxos); i++) {
+			wallet_unreserve_utxo(ld->wallet,
+					      sending->utxos[i],
+					      get_block_height(ld->topology));
+		}
+
+		was_pending(command_fail(sending->cmd, LIGHTNINGD,
+					 "Error broadcasting transaction: %s."
+					 " Unsent tx discarded %s",
+					 msg,
+					 type_to_string(tmpctx, struct wally_tx,
+							sending->wtx)));
+		return;
+	}
+
+	wallet_transaction_add(ld->wallet, sending->wtx, 0, 0);
+
+	/* Extract the change output and add it to the DB */
+	wallet_extract_owned_outputs(ld->wallet, sending->wtx, NULL, &change);
+
+	response = json_stream_success(sending->cmd);
+	wally_txid(sending->wtx, &txid);
+	json_add_hex_talarr(response, "tx", linearize_wtx(tmpctx, sending->wtx));
+	json_add_txid(response, "txid", &txid);
+	was_pending(command_success(sending->cmd, response));
+}
+
 static struct command_result *json_sendpsbt(struct command *cmd,
 					    const char *buffer,
 					    const jsmntok_t *obj UNNEEDED,
 					    const jsmntok_t *params)
 {
 	struct command_result *res;
+	struct sending_psbt *sending;
 	struct wally_psbt *psbt;
-	struct wally_tx *w_tx;
-	struct tx_broadcast *txb;
-	struct utxo **utxos;
-	struct bitcoin_txid txid;
+	struct lightningd *ld = cmd->ld;
 
 	if (!param(cmd, buffer, params,
 		   p_req("psbt", param_psbt, &psbt),
 		   NULL))
 		return command_param_failed();
 
-	w_tx = psbt_finalize(psbt, true);
-	if (!w_tx)
+	sending = tal(cmd, struct sending_psbt);
+	sending->cmd = cmd;
+	sending->wtx = tal_steal(sending, psbt_finalize(psbt, true));
+	if (!sending->wtx)
 		return command_fail(cmd, LIGHTNINGD,
 				    "PSBT not finalizeable %s",
 				    type_to_string(tmpctx, struct wally_psbt,
@@ -1070,27 +1114,21 @@ static struct command_result *json_sendpsbt(struct command *cmd,
 	/* We have to find/locate the utxos that are ours on this PSBT,
 	 * so that we know who to mark as used.
 	 */
-	res = match_psbt_inputs_to_utxos(cmd, psbt, NULL, &utxos);
+	res = match_psbt_inputs_to_utxos(cmd, psbt, NULL, &sending->utxos);
 	if (res)
 		return res;
 
-	txb = tal(cmd, struct tx_broadcast);
-	txb->utxos = cast_const2(const struct utxo **,
-				tal_steal(txb, utxos));
-	txb->wtx = tal_steal(txb, w_tx);
-	txb->cmd = cmd;
-	txb->expected_change = NULL;
-
-	/* FIXME: Do this *after* successful broadcast! */
-	wallet_transaction_add(cmd->ld->wallet, txb->wtx, 0, 0);
-	wally_txid(txb->wtx, &txid);
-	wallet_transaction_annotate(cmd->ld->wallet, &txid,
-				    TX_UNKNOWN, 0);
+	for (size_t i = 0; i < tal_count(sending->utxos); i++) {
+		if (!wallet_reserve_utxo(ld->wallet, sending->utxos[i],
+					 get_block_height(ld->topology)))
+			fatal("UTXO not reservable?");
+	}
 
 	/* Now broadcast the transaction */
 	bitcoind_sendrawtx(cmd->ld->topology->bitcoind,
-			   tal_hex(tmpctx, linearize_wtx(tmpctx, w_tx)),
-			   wallet_withdrawal_broadcast, txb);
+			   tal_hex(tmpctx,
+				   linearize_wtx(tmpctx, sending->wtx)),
+			   sendpsbt_done, sending);
 
 	return command_still_pending(cmd);
 }