lightningd: pass signed tx through to close callback.

Thread the signed tx through so close's JSON return contains that, rather than the unsigned channel->last_tx. We have to split the "get cmd_id" from "resolve the close commands" though; and of course, as before, we don't actually print the txids of multiple transactions even though we may have multi in flight due to splice! Signed-off-by: Rusty Russell <[email protected]> Changelog-Fixed: JSON-RPC: `close` returns a `tx` field with witness data populated (i.e. signed). Fixes: #6440
ElementsProject · Jul 31, 2023 · c9dbc9e · c9dbc9e
1 parent f26faf9
commit c9dbc9e
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 38 deletions.
diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c
@@ -51,14 +51,15 @@ struct close_command {
 
 /* Resolve a single close command. */
 static void
-resolve_one_close_command(struct close_command *cc, bool cooperative)
+resolve_one_close_command(struct close_command *cc, bool cooperative,
+			  const struct bitcoin_tx *close_tx)
 {
 	struct json_stream *result = json_stream_success(cc->cmd);
 
-	json_add_tx(result, "tx", cc->channel->last_tx);
-	if (!invalid_last_tx(cc->channel->last_tx)) {
+	json_add_tx(result, "tx", close_tx);
+	if (!invalid_last_tx(close_tx)) {
 		struct bitcoin_txid txid;
-		bitcoin_txid(cc->channel->last_tx, &txid);
+		bitcoin_txid(close_tx, &txid);
 		json_add_txid(result, "txid", &txid);
 	}
 	if (cooperative)
@@ -69,24 +70,31 @@ resolve_one_close_command(struct close_command *cc, bool cooperative)
 	was_pending(command_success(cc->cmd, result));
 }
 
-/* Resolve a close command for a channel that will be closed soon: returns
- * the cmd_id of one, if any (allocated off ctx). */
-const char *resolve_close_command(const tal_t *ctx,
-				  struct lightningd *ld, struct channel *channel,
-				  bool cooperative)
+const char *cmd_id_from_close_command(const tal_t *ctx,
+				      struct lightningd *ld, struct channel *channel)
+{
+	struct close_command *cc;
+
+	list_for_each(&ld->close_commands, cc, list) {
+		if (cc->channel != channel)
+			continue;
+		return tal_strdup(ctx, cc->cmd->id);
+	}
+	return NULL;
+}
+
+/* Resolve a close command for a channel that will be closed soon. */
+void resolve_close_command(struct lightningd *ld, struct channel *channel,
+			   bool cooperative, const struct bitcoin_tx *close_tx)
 {
 	struct close_command *cc;
 	struct close_command *n;
-	const char *cmd_id = NULL;
 
 	list_for_each_safe(&ld->close_commands, cc, n, list) {
 		if (cc->channel != channel)
 			continue;
-		if (!cmd_id)
-			cmd_id = tal_strdup(ctx, cc->cmd->id);
-		resolve_one_close_command(cc, cooperative);
+		resolve_one_close_command(cc, cooperative, close_tx);
 	}
-	return cmd_id;
 }
 
 /* Destroy the close command structure in reaction to the

diff --git a/lightningd/closing_control.h b/lightningd/closing_control.h
@@ -8,11 +8,13 @@ struct channel;
 struct lightningd;
 struct peer_fd;
 
-/* Resolve a close command for a channel that will be closed soon: returns
- * the cmd_id of one, if any (allocated off ctx). */
-const char *resolve_close_command(const tal_t *ctx,
-				  struct lightningd *ld, struct channel *channel,
-				  bool cooperative);
+/* Find cmd_id for closing command, if any. */
+const char *cmd_id_from_close_command(const tal_t *ctx,
+				      struct lightningd *ld, struct channel *channel);
+
+/* Resolve a close command for a channel that will be closed soon. */
+void resolve_close_command(struct lightningd *ld, struct channel *channel,
+			   bool cooperative, const struct bitcoin_tx *close_tx);
 
 void peer_start_closingd(struct channel *channel,
 			 struct peer_fd *peer_fd);

diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c
@@ -287,17 +287,18 @@ static bool commit_tx_send_finished(struct channel *channel,
 	return false;
 }
 
-static void sign_and_send_last(struct lightningd *ld,
-			       struct channel *channel,
-			       const char *cmd_id,
-			       const struct bitcoin_tx *last_tx,
-			       const struct bitcoin_signature *last_sig)
+static struct bitcoin_tx *sign_and_send_last(const tal_t *ctx,
+					     struct lightningd *ld,
+					     struct channel *channel,
+					     const char *cmd_id,
+					     const struct bitcoin_tx *last_tx,
+					     const struct bitcoin_signature *last_sig)
 {
 	struct bitcoin_txid txid;
 	struct anchor_details *adet;
 	struct bitcoin_tx *tx;
 
-	tx = sign_last_tx(tmpctx, channel, last_tx, last_sig);
+	tx = sign_last_tx(ctx, channel, last_tx, last_sig);
 	bitcoin_txid(tx, &txid);
 	wallet_transaction_add(ld->wallet, tx->wtx, 0, 0);
 
@@ -308,6 +309,8 @@ static void sign_and_send_last(struct lightningd *ld,
 	 * if they beat us to the broadcast). */
 	broadcast_tx(ld->topology, channel, tx, cmd_id, false, 0,
 		     commit_tx_send_finished, commit_tx_boost, take(adet));
+
+	return tx;
 }
 
 void drop_to_chain(struct lightningd *ld, struct channel *channel,
@@ -317,7 +320,7 @@ void drop_to_chain(struct lightningd *ld, struct channel *channel,
 	const char *cmd_id;
 
 	/* If this was triggered by a close command, get a copy of the cmd id */
-	cmd_id = resolve_close_command(tmpctx, ld, channel, cooperative);
+	cmd_id = cmd_id_from_close_command(tmpctx, ld, channel);
 
 	/* BOLT #2:
 	 *
@@ -335,15 +338,19 @@ void drop_to_chain(struct lightningd *ld, struct channel *channel,
 			   "Cannot broadcast our commitment tx:"
 			   " it's invalid! (ancient channel?)");
 	} else {
+		struct bitcoin_tx *tx;
+
 		/* We need to drop *every* commitment transaction to chain */
 		if (!cooperative && !list_empty(&channel->inflights)) {
 			list_for_each(&channel->inflights, inflight, list)
-				sign_and_send_last(ld, channel, cmd_id,
-						   inflight->last_tx,
-						   &inflight->last_sig);
+				tx = sign_and_send_last(tmpctx, ld, channel, cmd_id,
+							inflight->last_tx,
+							&inflight->last_sig);
 		} else
-			sign_and_send_last(ld, channel, cmd_id, channel->last_tx,
-					   &channel->last_sig);
+			tx = sign_and_send_last(tmpctx, ld, channel, cmd_id, channel->last_tx,
+						&channel->last_sig);
+
+		resolve_close_command(ld, channel, cooperative, tx);
 	}
 
 }

diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c
@@ -132,6 +132,10 @@ void channel_update_reserve(struct channel *channel UNNEEDED,
 			    struct channel_config *their_config UNNEEDED,
 			    struct amount_sat funding_total UNNEEDED)
 { fprintf(stderr, "channel_update_reserve called!\n"); abort(); }
+/* Generated stub for cmd_id_from_close_command */
+const char *cmd_id_from_close_command(const tal_t *ctx UNNEEDED,
+				      struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED)
+{ fprintf(stderr, "cmd_id_from_close_command called!\n"); abort(); }
 /* Generated stub for command_fail */
 struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED,
 				    const char *fmt UNNEEDED, ...)
@@ -841,9 +845,8 @@ bool pubkey_from_node_id(struct pubkey *key UNNEEDED, const struct node_id *id U
 void report_subd_memleak(struct leak_detect *leak_detect UNNEEDED, struct subd *leaker UNNEEDED)
 { fprintf(stderr, "report_subd_memleak called!\n"); abort(); }
 /* Generated stub for resolve_close_command */
-const char *resolve_close_command(const tal_t *ctx UNNEEDED,
-				  struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED,
-				  bool cooperative UNNEEDED)
+void resolve_close_command(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED,
+			   bool cooperative UNNEEDED, const struct bitcoin_tx *close_tx UNNEEDED)
 { fprintf(stderr, "resolve_close_command called!\n"); abort(); }
 /* Generated stub for start_leak_request */
 void start_leak_request(const struct subd_req *req UNNEEDED,

diff --git a/tests/test_closing.py b/tests/test_closing.py
@@ -3872,7 +3872,6 @@ def censoring_sendrawtx(r):
     # FIXME: l2 should complain!
 
 
-@pytest.mark.xfail(strict=True)
 @pytest.mark.developer("needs dev-no-reconnect")
 def test_closing_tx_valid(node_factory, bitcoind):
     l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True,

diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
@@ -89,6 +89,10 @@ void channel_update_reserve(struct channel *channel UNNEEDED,
 			    struct channel_config *their_config UNNEEDED,
 			    struct amount_sat funding_total UNNEEDED)
 { fprintf(stderr, "channel_update_reserve called!\n"); abort(); }
+/* Generated stub for cmd_id_from_close_command */
+const char *cmd_id_from_close_command(const tal_t *ctx UNNEEDED,
+				      struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED)
+{ fprintf(stderr, "cmd_id_from_close_command called!\n"); abort(); }
 /* Generated stub for command_fail */
 struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED,
 				    const char *fmt UNNEEDED, ...)
@@ -677,9 +681,8 @@ const u8 *psbt_fixup(const tal_t *ctx UNNEEDED, const u8 *psbtblob UNNEEDED)
 void report_subd_memleak(struct leak_detect *leak_detect UNNEEDED, struct subd *leaker UNNEEDED)
 { fprintf(stderr, "report_subd_memleak called!\n"); abort(); }
 /* Generated stub for resolve_close_command */
-const char *resolve_close_command(const tal_t *ctx UNNEEDED,
-				  struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED,
-				  bool cooperative UNNEEDED)
+void resolve_close_command(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED,
+			   bool cooperative UNNEEDED, const struct bitcoin_tx *close_tx UNNEEDED)
 { fprintf(stderr, "resolve_close_command called!\n"); abort(); }
 /* Generated stub for rune_is_ours */
 const char *rune_is_ours(struct lightningd *ld UNNEEDED, const struct rune *rune UNNEEDED)