From 795a5aaf1ed7d8bb59e2331913f8c1887f5a535b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 31 Jul 2023 16:53:46 +0930 Subject: [PATCH] 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 Changelog-Fixed: JSON-RPC: `close` returns a `tx` field with witness data populated (i.e. signed). Fixes: #6440 --- lightningd/closing_control.c | 36 +++++++++++++-------- lightningd/closing_control.h | 12 ++++--- lightningd/peer_control.c | 31 +++++++++++------- lightningd/test/run-invoice-select-inchan.c | 9 ++++-- tests/test_closing.py | 1 - wallet/test/run-wallet.c | 9 ++++-- 6 files changed, 60 insertions(+), 38 deletions(-) diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index f5ba5babb281..99678e52adf2 100644 --- 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 index c536e9b569c6..a4be9ad171e2 100644 --- 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 index 094601f4dca6..4efee7ba2d66 100644 --- 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 index 3af8a13c6f52..1a932094bdf2 100644 --- 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, ...) @@ -831,9 +835,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 index f9cedfafbcb1..aca67f264c4e 100644 --- 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 index 59586848a1ac..f52513ef2bed 100644 --- 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, ...) @@ -673,9 +677,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)