From 049529542abbc9baa8ad2da2cb2f2dba4a2a1cec Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 10 Aug 2019 14:54:57 +0930 Subject: [PATCH] lightningd: delay reprocessing of incoming htlcs at startup until plugins ready. Fixes: #2923 Signed-off-by: Rusty Russell --- lightningd/lightningd.c | 8 ++++- lightningd/peer_control.c | 4 +-- lightningd/peer_control.h | 5 +-- lightningd/peer_htlcs.c | 35 ++++++++++++++------- lightningd/peer_htlcs.h | 8 +++-- lightningd/test/run-find_my_abspath.c | 5 ++- lightningd/test/run-invoice-select-inchan.c | 6 ++-- tests/test_plugin.py | 2 -- wallet/test/run-wallet.c | 2 +- 9 files changed, 48 insertions(+), 27 deletions(-) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index e66f7c857..67f1900a9 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -632,6 +632,7 @@ int main(int argc, char *argv[]) int stop_fd; struct timers *timers; const char *stop_response; + struct htlc_in_map *unprocessed_htlcs; /*~ What happens in strange locales should stay there. */ setup_locale(); @@ -769,7 +770,7 @@ int main(int argc, char *argv[]) * topology is initialized since some decisions rely on being able to * know the blockheight. */ db_begin_transaction(ld->wallet->db); - load_channels_from_wallet(ld); + unprocessed_htlcs = load_channels_from_wallet(ld); db_commit_transaction(ld->wallet->db); /*~ Create RPC socket: now lightning-cli can send us JSON RPC commands @@ -780,6 +781,11 @@ int main(int argc, char *argv[]) * can start talking to us. */ plugins_config(ld->plugins); + /*~ Process any HTLCs we were in the middle of when we exited, now + * that plugins (who might want to know via htlc_accepted hook) are + * active. */ + htlcs_resubmit(ld, unprocessed_htlcs); + /*~ Activate connect daemon. Needs to be after the initialization of * chaintopology, otherwise peers may connect and ask for * uninitialized data. */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 172cd0b25..abddb3a04 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1419,7 +1419,7 @@ void activate_peers(struct lightningd *ld) } /* Pull peers, channels and HTLCs from db, and wire them up. */ -void load_channels_from_wallet(struct lightningd *ld) +struct htlc_in_map *load_channels_from_wallet(struct lightningd *ld) { struct peer *peer; @@ -1442,7 +1442,7 @@ void load_channels_from_wallet(struct lightningd *ld) } /* Now connect HTLC pointers together */ - htlcs_reconnect(ld, &ld->htlcs_in, &ld->htlcs_out); + return htlcs_reconnect(ld, &ld->htlcs_in, &ld->htlcs_out); } static struct command_result *json_disconnect(struct command *cmd, diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 6c52aacf1..a89e738de 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -90,8 +90,9 @@ void drop_to_chain(struct lightningd *ld, struct channel *channel, bool cooperat void channel_watch_funding(struct lightningd *ld, struct channel *channel); -/* Pull peers, channels and HTLCs from db, and wire them up. */ -void load_channels_from_wallet(struct lightningd *ld); +/* Pull peers, channels and HTLCs from db, and wire them up. + * Returns any HTLCs we have to resubmit via htlcs_resubmit. */ +struct htlc_in_map *load_channels_from_wallet(struct lightningd *ld); #if DEVELOPER void peer_dev_memleak(struct command *cmd); diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 597fe860f..9cfc299a7 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -2018,17 +2018,18 @@ static void fixup_hout(struct lightningd *ld, struct htlc_out *hout) * For each outgoing HTLC find the incoming HTLC that triggered it. If * we are the origin of the transfer then we cannot resolve the * incoming HTLC in which case we just leave it `NULL`. + * + * Returns a map of any htlcs we need to retry. */ -void htlcs_reconnect(struct lightningd *ld, - struct htlc_in_map *htlcs_in, - struct htlc_out_map *htlcs_out) +struct htlc_in_map *htlcs_reconnect(struct lightningd *ld, + struct htlc_in_map *htlcs_in, + struct htlc_out_map *htlcs_out) { struct htlc_in_map_iter ini; struct htlc_out_map_iter outi; struct htlc_in *hin; struct htlc_out *hout; - struct htlc_in_map unprocessed; - enum onion_type failcode COMPILER_WANTS_INIT("gcc7.4.0 bad, 8.3 OK"); + struct htlc_in_map *unprocessed = tal(NULL, struct htlc_in_map); /* Any HTLCs which happened to be incoming and weren't forwarded before * we shutdown/crashed: fail them now. @@ -2036,11 +2037,11 @@ void htlcs_reconnect(struct lightningd *ld, * Note that since we do local processing synchronously, so this never * captures local payments. But if it did, it would be a tiny corner * case. */ - htlc_in_map_init(&unprocessed); + htlc_in_map_init(unprocessed); for (hin = htlc_in_map_first(htlcs_in, &ini); hin; hin = htlc_in_map_next(htlcs_in, &ini)) { if (hin->hstate == RCVD_ADD_ACK_REVOCATION) - htlc_in_map_add(&unprocessed, hin); + htlc_in_map_add(unprocessed, hin); } for (hout = htlc_out_map_first(htlcs_out, &outi); hout; @@ -2081,12 +2082,22 @@ void htlcs_reconnect(struct lightningd *ld, #endif if (hout->in) - htlc_in_map_del(&unprocessed, hout->in); + htlc_in_map_del(unprocessed, hout->in); } + return unprocessed; +} + +void htlcs_resubmit(struct lightningd *ld, struct htlc_in_map *unprocessed) +{ + struct htlc_in *hin; + struct htlc_in_map_iter ini; + enum onion_type failcode COMPILER_WANTS_INIT("gcc7.4.0 bad, 8.3 OK"); + /* Now fail any which were stuck. */ - for (hin = htlc_in_map_first(&unprocessed, &ini); hin; - hin = htlc_in_map_next(&unprocessed, &ini)) { + for (hin = htlc_in_map_first(unprocessed, &ini); + hin; + hin = htlc_in_map_next(unprocessed, &ini)) { log_unusual(hin->key.channel->log, "Replaying old unprocessed HTLC #%"PRIu64, hin->key.id); @@ -2102,10 +2113,10 @@ void htlcs_reconnect(struct lightningd *ld, } /* Don't leak memory! */ - htlc_in_map_clear(&unprocessed); + htlc_in_map_clear(unprocessed); + tal_free(unprocessed); } - #if DEVELOPER static struct command_result *json_dev_ignore_htlcs(struct command *cmd, const char *buffer, diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h index 5a2ec845c..0d54b8cad 100644 --- a/lightningd/peer_htlcs.h +++ b/lightningd/peer_htlcs.h @@ -63,9 +63,11 @@ void onchain_fulfilled_htlc(struct channel *channel, void htlcs_notify_new_block(struct lightningd *ld, u32 height); -void htlcs_reconnect(struct lightningd *ld, - struct htlc_in_map *htlcs_in, - struct htlc_out_map *htlcs_out); +struct htlc_in_map *htlcs_reconnect(struct lightningd *ld, + struct htlc_in_map *htlcs_in, + struct htlc_out_map *htlcs_out); + +void htlcs_resubmit(struct lightningd *ld, struct htlc_in_map *unprocessed); /* For HTLCs which terminate here, invoice payment calls one of these. */ void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage); diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index 649b0f4a7..678e4ee4a 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -96,6 +96,9 @@ void hsm_init(struct lightningd *ld UNNEEDED) /* Generated stub for htlcs_notify_new_block */ void htlcs_notify_new_block(struct lightningd *ld UNNEEDED, u32 height UNNEEDED) { fprintf(stderr, "htlcs_notify_new_block called!\n"); abort(); } +/* Generated stub for htlcs_resubmit */ +void htlcs_resubmit(struct lightningd *ld UNNEEDED, struct htlc_in_map *unprocessed UNNEEDED) +{ fprintf(stderr, "htlcs_resubmit called!\n"); abort(); } /* Generated stub for jsonrpc_listen */ void jsonrpc_listen(struct jsonrpc *rpc UNNEEDED, struct lightningd *ld UNNEEDED) { fprintf(stderr, "jsonrpc_listen called!\n"); abort(); } @@ -103,7 +106,7 @@ void jsonrpc_listen(struct jsonrpc *rpc UNNEEDED, struct lightningd *ld UNNEEDED void jsonrpc_setup(struct lightningd *ld UNNEEDED) { fprintf(stderr, "jsonrpc_setup called!\n"); abort(); } /* Generated stub for load_channels_from_wallet */ -void load_channels_from_wallet(struct lightningd *ld UNNEEDED) +struct htlc_in_map *load_channels_from_wallet(struct lightningd *ld UNNEEDED) { fprintf(stderr, "load_channels_from_wallet called!\n"); abort(); } /* Generated stub for log_ */ void log_(struct log *log UNNEEDED, enum log_level level UNNEEDED, bool call_notifier UNNEEDED, const char *fmt UNNEEDED, ...) diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 13cee785e..b74078fbd 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -126,9 +126,9 @@ bool htlc_is_trimmed(enum side htlc_owner UNNEEDED, enum side side UNNEEDED) { fprintf(stderr, "htlc_is_trimmed called!\n"); abort(); } /* Generated stub for htlcs_reconnect */ -void htlcs_reconnect(struct lightningd *ld UNNEEDED, - struct htlc_in_map *htlcs_in UNNEEDED, - struct htlc_out_map *htlcs_out UNNEEDED) +struct htlc_in_map *htlcs_reconnect(struct lightningd *ld UNNEEDED, + struct htlc_in_map *htlcs_in UNNEEDED, + struct htlc_out_map *htlcs_out UNNEEDED) { fprintf(stderr, "htlcs_reconnect called!\n"); abort(); } /* Generated stub for json_add_address */ void json_add_address(struct json_stream *response UNNEEDED, const char *fieldname UNNEEDED, diff --git a/tests/test_plugin.py b/tests/test_plugin.py index ddd430a12..863208a12 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -466,7 +466,6 @@ def test_htlc_accepted_hook_resolve(node_factory): assert len(inv) == 1 and inv[0]['status'] == 'unpaid' -@pytest.mark.xfail(strict=True) def test_htlc_accepted_hook_direct_restart(node_factory, executor): """l2 restarts while it is pondering what to do with an HTLC. """ @@ -492,7 +491,6 @@ def test_htlc_accepted_hook_direct_restart(node_factory, executor): @unittest.skipIf(not DEVELOPER, "without DEVELOPER=1, gossip v slow") -@pytest.mark.xfail(strict=True) def test_htlc_accepted_hook_forward_restart(node_factory, executor): """l2 restarts while it is pondering what to do with an HTLC. """ diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index be8dc36e6..a1a9b2a39 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1195,7 +1195,7 @@ static bool test_htlc_crud(struct lightningd *ld, const tal_t *ctx) "Failed loading HTLCs"); db_commit_transaction(w->db); - htlcs_reconnect(w->ld, htlcs_in, htlcs_out); + htlcs_resubmit(w->ld, htlcs_reconnect(w->ld, htlcs_in, htlcs_out)); CHECK(!wallet_err); hin = htlc_in_map_get(htlcs_in, &in.key);