From c2f0c79ae976e8821e0e60cc1ca4289bf332d969 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 24 Oct 2023 12:11:30 +1030 Subject: [PATCH] lightningd: call finished callback *every* time we re-xmit a transaction. We have to work quite hard to do this, since we don't want to call finish if the broadcast has been freed in the meantime. Signed-off-by: Rusty Russell --- lightningd/chaintopology.c | 95 ++++++++++++++++++++------------------ lightningd/chaintopology.h | 2 +- 2 files changed, 51 insertions(+), 46 deletions(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 557cb77149c3..f8b77fdfafd9 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -115,40 +116,44 @@ size_t get_tx_depth(const struct chain_topology *topo, return topo->tip->height - blockheight + 1; } -struct txs_to_broadcast { - /* We just sent txs[cursor] */ - size_t cursor; - /* These are hex encoded already, for bitcoind_sendrawtx */ - const char **txs; - - /* IDs to attach to each tx (could be NULL!) */ - const char **cmd_id; +struct tx_rebroadcast { + /* otx destructor sets this to NULL if it's been freed */ + struct outgoing_tx *otx; - /* allowhighfees flags for each tx */ - bool *allowhighfees; + /* Pointer to how many are remaining: last one frees! */ + size_t *num_rebroadcast_remaining; }; -/* We just sent the last entry in txs[]. Shrink and send the next last. */ -static void broadcast_remainder(struct bitcoind *bitcoind, - bool success, const char *msg, - struct txs_to_broadcast *txs) +/* Timer recursion: declare now. */ +static void rebroadcast_txs(struct chain_topology *topo); + +/* We are last. Refresh timer, and free refcnt */ +static void rebroadcasts_complete(struct chain_topology *topo, + size_t *num_rebroadcast_remaining) +{ + tal_free(num_rebroadcast_remaining); + topo->rebroadcast_timer = new_reltimer(topo->ld->timers, topo, + time_from_sec(30 + pseudorand(30)), + rebroadcast_txs, topo); +} + +static void destroy_tx_broadcast(struct tx_rebroadcast *txrb, struct chain_topology *topo) +{ + if (--*txrb->num_rebroadcast_remaining == 0) + rebroadcasts_complete(topo, txrb->num_rebroadcast_remaining); +} + +static void rebroadcast_done(struct bitcoind *bitcoind, + bool success, const char *msg, + struct tx_rebroadcast *txrb) { if (!success) log_debug(bitcoind->log, "Expected error broadcasting tx %s: %s", - txs->txs[txs->cursor], msg); - - txs->cursor++; - if (txs->cursor == tal_count(txs->txs)) { - tal_free(txs); - return; - } + fmt_bitcoin_tx(tmpctx, txrb->otx->tx), msg); - /* Broadcast next one. */ - bitcoind_sendrawtx(bitcoind, bitcoind, - txs->cmd_id[txs->cursor], txs->txs[txs->cursor], - txs->allowhighfees[txs->cursor], - broadcast_remainder, txs); + /* Last one freed calls rebroadcasts_complete */ + tal_free(txrb); } /* FIXME: This is dumb. We can group txs and avoid bothering bitcoind @@ -156,20 +161,16 @@ static void broadcast_remainder(struct bitcoind *bitcoind, static void rebroadcast_txs(struct chain_topology *topo) { /* Copy txs now (peers may go away, and they own txs). */ - struct txs_to_broadcast *txs; struct outgoing_tx *otx; struct outgoing_tx_map_iter it; tal_t *cleanup_ctx = tal(NULL, char); + size_t *num_rebroadcast_remaining = notleak(tal(topo, size_t)); - txs = tal(topo, struct txs_to_broadcast); - txs->cmd_id = tal_arr(txs, const char *, 0); - - /* Put any txs we want to broadcast in ->txs. */ - txs->txs = tal_arr(txs, const char *, 0); - txs->allowhighfees = tal_arr(txs, bool, 0); - + *num_rebroadcast_remaining = 0; for (otx = outgoing_tx_map_first(topo->outgoing_txs, &it); otx; otx = outgoing_tx_map_next(topo->outgoing_txs, &it)) { + struct tx_rebroadcast *txrb; + /* Already sent? */ if (wallet_transaction_height(topo->ld->wallet, &otx->txid)) continue; @@ -185,22 +186,26 @@ static void rebroadcast_txs(struct chain_topology *topo) continue; } - tal_arr_expand(&txs->txs, fmt_bitcoin_tx(txs->txs, otx->tx)); - tal_arr_expand(&txs->allowhighfees, otx->allowhighfees); - tal_arr_expand(&txs->cmd_id, tal_strdup_or_null(txs, otx->cmd_id)); + txrb = tal(otx, struct tx_rebroadcast); + txrb->otx = otx; + txrb->num_rebroadcast_remaining = num_rebroadcast_remaining; + (*num_rebroadcast_remaining)++; + tal_add_destructor2(txrb, destroy_tx_broadcast, topo); + bitcoind_sendrawtx(txrb, topo->bitcoind, + tal_strdup_or_null(tmpctx, otx->cmd_id), + fmt_bitcoin_tx(tmpctx, otx->tx), + otx->allowhighfees, + rebroadcast_done, + txrb); } tal_free(cleanup_ctx); - /* Free explicitly in case we were called because a block came in. - * Then set a new timer 30-60 seconds away */ + /* Free explicitly in case we were called because a block came in. */ tal_free(topo->rebroadcast_timer); - topo->rebroadcast_timer = new_reltimer(topo->ld->timers, topo, - time_from_sec(30 + pseudorand(30)), - rebroadcast_txs, topo); - /* Let this do the dirty work. */ - txs->cursor = (size_t)-1; - broadcast_remainder(topo->bitcoind, true, "", txs); + /* Nothing to broadcast? Reset timer immediately */ + if (*num_rebroadcast_remaining == 0) + rebroadcasts_complete(topo, num_rebroadcast_remaining); } static void destroy_outgoing_tx(struct outgoing_tx *otx, struct chain_topology *topo) diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 014250b48067..4d7e03e51666 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -212,7 +212,7 @@ u32 default_locktime(const struct chain_topology *topo); * @cmd_id: the JSON command id which triggered this (or NULL). * @allowhighfees: set to true to override the high-fee checks in the backend. * @minblock: minimum block we can send it at (or 0). - * @finished: if non-NULL, call that when sendrawtransaction returns; if it returns true, don't rebroadcast. + * @finished: if non-NULL, call every time sendrawtransaction returns; if it returns true, don't rebroadcast. * @refresh: if non-NULL, callback before re-broadcasting (can replace tx): * if returns false, delete. * @cbarg: argument for @finished and @refresh