From f1bea55395c23f5d79401b05b95835d772e946e7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 28 Jun 2019 11:28:31 +0930 Subject: [PATCH] lightningd: fix occasional missing txid detection. I was working on rewriting our (somewhat chaotic) tx watching code for 0.7.2, when I found this bug: we don't always notice the funding tx in corner cases where more than one block is detected at once. This is just the one commit needed to fix the problem: it has some unnecessary changes, but I'd prefer not to diverge too far from my cleanup-txwatch branch. Fixes: #2352 Signed-off-by: Rusty Russell --- lightningd/chaintopology.c | 21 +++++++++++++ lightningd/onchain_control.c | 22 ++++++++++--- lightningd/peer_control.c | 34 +++++++++++++++++++++ lightningd/peer_htlcs.h | 2 ++ lightningd/test/run-invoice-select-inchan.c | 1 + lightningd/watch.c | 26 ++++++++++++++-- lightningd/watch.h | 7 +++++ wallet/test/run-wallet.c | 1 + 8 files changed, 108 insertions(+), 6 deletions(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index ba953303b..c24f5d18e 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -4,6 +4,7 @@ #include "bitcoin/tx.h" #include "bitcoind.h" #include "chaintopology.h" +#include "channel.h" #include "jsonrpc.h" #include "lightningd.h" #include "log.h" @@ -100,6 +101,8 @@ static void filter_block_txs(struct chain_topology *topo, struct block *b) wallet_transaction_add(topo->ld->wallet, tx, b->height, i); } + + txwatch_inform(topo, &txid, tx); } b->full_txs = tal_free(b->full_txs); } @@ -240,8 +243,26 @@ void broadcast_tx(struct chain_topology *topo, static enum watch_result closeinfo_txid_confirmed(struct lightningd *ld, struct channel *channel, const struct bitcoin_txid *txid, + const struct bitcoin_tx *tx, unsigned int depth) { + /* Sanity check. */ + if (tx != NULL) { + struct bitcoin_txid txid2; + + bitcoin_txid(tx, &txid2); + if (!bitcoin_txid_eq(txid, &txid2)) { + channel_internal_error(channel, "Txid for %s is not %s", + type_to_string(tmpctx, + struct bitcoin_tx, + tx), + type_to_string(tmpctx, + struct bitcoin_txid, + txid)); + return DELETE_WATCH; + } + } + /* We delete ourselves first time, so should not be reorged out!! */ assert(depth > 0); /* Subtle: depth 1 == current block. */ diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 8bf60116f..25c891fb3 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -79,17 +79,31 @@ static void onchain_tx_depth(struct channel *channel, static enum watch_result onchain_tx_watched(struct lightningd *ld, struct channel *channel, const struct bitcoin_txid *txid, + const struct bitcoin_tx *tx, unsigned int depth) { u32 blockheight = get_block_height(ld->topology); + + if (tx != NULL) { + struct bitcoin_txid txid2; + + bitcoin_txid(tx, &txid2); + if (!bitcoin_txid_eq(txid, &txid2)) { + channel_internal_error(channel, "Txid for %s is not %s", + type_to_string(tmpctx, + struct bitcoin_tx, + tx), + type_to_string(tmpctx, + struct bitcoin_txid, + txid)); + return DELETE_WATCH; + } + } + if (depth == 0) { log_unusual(channel->log, "Chain reorganization!"); channel_set_owner(channel, NULL, false); - /* FIXME! - topology_rescan(peer->ld->topology, peer->funding_txid); - */ - /* We will most likely be freed, so this is a noop */ return KEEP_WATCHING; } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 575654806..c3a519e64 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -945,14 +945,48 @@ void peer_connected(struct lightningd *ld, const u8 *msg, plugin_hook_call_peer_connected(ld, hook_payload, hook_payload); } +/* FIXME: Unify our watch code so we get notified by txout, instead, like + * the wallet code does. */ +static bool check_funding_tx(const struct bitcoin_tx *tx, + const struct channel *channel) +{ + u8 *wscript; + + if (channel->funding_outnum >= tx->wtx->num_outputs) + return false; + + if (!amount_sat_eq(bitcoin_tx_output_get_amount(tx, + channel->funding_outnum), + channel->funding)) + return false; + + wscript = bitcoin_redeem_2of2(tmpctx, + &channel->local_funding_pubkey, + &channel->channel_info.remote_fundingkey); + return scripteq(scriptpubkey_p2wsh(tmpctx, wscript), + bitcoin_tx_output_get_script(tmpctx, tx, + channel->funding_outnum)); +} + static enum watch_result funding_depth_cb(struct lightningd *ld, struct channel *channel, const struct bitcoin_txid *txid, + const struct bitcoin_tx *tx, unsigned int depth) { const char *txidstr; struct short_channel_id scid; + /* Sanity check */ + if (tx && !check_funding_tx(tx, channel)) { + channel_internal_error(channel, "Bad tx %s: %s", + type_to_string(tmpctx, + struct bitcoin_txid, txid), + type_to_string(tmpctx, + struct bitcoin_tx, tx)); + return DELETE_WATCH; + } + txidstr = type_to_string(tmpctx, struct bitcoin_txid, txid); log_debug(channel->log, "Funding tx %s depth %u of %u", txidstr, depth, channel->minimum_depth); diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h index dd57c99f3..d8f9285ed 100644 --- a/lightningd/peer_htlcs.h +++ b/lightningd/peer_htlcs.h @@ -9,7 +9,9 @@ struct channel; struct htlc_in; +struct htlc_in_map; struct htlc_out; +struct htlc_out_map; struct htlc_stub; struct lightningd; diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index d3dd5387e..a2e615ba7 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -578,6 +578,7 @@ struct txwatch *watch_txid(const tal_t *ctx UNNEEDED, enum watch_result (*cb)(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED, const struct bitcoin_txid * UNNEEDED, + const struct bitcoin_tx * UNNEEDED, unsigned int depth)) { fprintf(stderr, "watch_txid called!\n"); abort(); } /* Generated stub for watch_txo */ diff --git a/lightningd/watch.c b/lightningd/watch.c index d1ee5d1f1..c6cc27ff0 100644 --- a/lightningd/watch.c +++ b/lightningd/watch.c @@ -63,12 +63,17 @@ struct txwatch { /* Transaction to watch. */ struct bitcoin_txid txid; + + /* May be NULL if we haven't seen it yet. */ + const struct bitcoin_tx *tx; + unsigned int depth; /* A new depth (0 if kicked out, otherwise 1 = tip, etc.) */ enum watch_result (*cb)(struct lightningd *ld, struct channel *channel, const struct bitcoin_txid *txid, + const struct bitcoin_tx *tx, unsigned int depth); }; @@ -124,7 +129,8 @@ struct txwatch *watch_txid(const tal_t *ctx, const struct bitcoin_txid *txid, enum watch_result (*cb)(struct lightningd *ld, struct channel *channel, - const struct bitcoin_txid *, + const struct bitcoin_txid *txid, + const struct bitcoin_tx *tx, unsigned int depth)) { struct txwatch *w; @@ -133,6 +139,7 @@ struct txwatch *watch_txid(const tal_t *ctx, w->topo = topo; w->depth = 0; w->txid = *txid; + w->tx = NULL; w->channel = channel; w->cb = cb; @@ -173,11 +180,13 @@ struct txwatch *watch_tx(const tal_t *ctx, enum watch_result (*cb)(struct lightningd *ld, struct channel *channel, const struct bitcoin_txid *, + const struct bitcoin_tx *, unsigned int depth)) { struct bitcoin_txid txid; bitcoin_txid(tx, &txid); + /* FIXME: Save populate txwatch->tx here, too! */ return watch_txid(ctx, topo, channel, &txid, cb); } @@ -227,7 +236,8 @@ static bool txw_fire(struct txwatch *txw, type_to_string(tmpctx, struct bitcoin_txid, &txw->txid), depth ? "" : " REORG"); txw->depth = depth; - r = txw->cb(txw->topo->bitcoind->ld, txw->channel, txid, txw->depth); + r = txw->cb(txw->topo->bitcoind->ld, txw->channel, txid, txw->tx, + txw->depth); switch (r) { case DELETE_WATCH: tal_free(txw); @@ -295,3 +305,15 @@ void watch_topology_changed(struct chain_topology *topo) } } while (needs_rerun); } + +void txwatch_inform(const struct chain_topology *topo, + const struct bitcoin_txid *txid, + const struct bitcoin_tx *tx_may_steal) +{ + struct txwatch *txw; + + txw = txwatch_hash_get(&topo->txwatches, txid); + + if (txw && !txw->tx) + txw->tx = tal_steal(txw, tx_may_steal); +} diff --git a/lightningd/watch.h b/lightningd/watch.h index a5e97ad9f..b5c7f2544 100644 --- a/lightningd/watch.h +++ b/lightningd/watch.h @@ -47,6 +47,7 @@ struct txwatch *watch_txid(const tal_t *ctx, enum watch_result (*cb)(struct lightningd *ld, struct channel *channel, const struct bitcoin_txid *, + const struct bitcoin_tx *, unsigned int depth)); struct txwatch *watch_tx(const tal_t *ctx, @@ -56,6 +57,7 @@ struct txwatch *watch_tx(const tal_t *ctx, enum watch_result (*cb)(struct lightningd *ld, struct channel *channel, const struct bitcoin_txid *, + const struct bitcoin_tx *, unsigned int depth)); struct txowatch *watch_txo(const tal_t *ctx, @@ -83,5 +85,10 @@ void txowatch_fire(const struct txowatch *txow, bool watching_txid(const struct chain_topology *topo, const struct bitcoin_txid *txid); +/* FIXME: Implement bitcoin_tx_dup() so we tx arg can be TAKEN */ +void txwatch_inform(const struct chain_topology *topo, + const struct bitcoin_txid *txid, + const struct bitcoin_tx *tx_may_steal); + void watch_topology_changed(struct chain_topology *topo); #endif /* LIGHTNING_LIGHTNINGD_WATCH_H */ diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 2263197d6..867a5deb2 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -569,6 +569,7 @@ struct txwatch *watch_txid(const tal_t *ctx UNNEEDED, enum watch_result (*cb)(struct lightningd *ld UNNEEDED, struct channel *channel UNNEEDED, const struct bitcoin_txid * UNNEEDED, + const struct bitcoin_tx * UNNEEDED, unsigned int depth)) { fprintf(stderr, "watch_txid called!\n"); abort(); } /* Generated stub for watch_txo */