From 41469504966f8f9f501bb64ef385b90dbf9bfcef Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 9 Oct 2018 19:27:52 +1030 Subject: [PATCH] lightningd: don't access htlc_in's failoutchannel on db restore. failoutchannel tells us which channel to send an update for (specifically for temporary_channel_failure); but we don't save it into the db. It's not even clear we should, since it's a corner case and the channel might not even exist when we come back. So on db restore, change such errors to WIRE_TEMPORARY_NODE_FAILURE which doesn't need an update. We also don't memset it to 0 in the normal case (we only access if it failcode has the UPDATE bit set) so valgrind will trigger if we're wrong. Signed-off-by: Rusty Russell --- lightningd/peer_htlcs.c | 2 -- wallet/wallet.c | 20 +++++++++++++------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 27d204cab..3d691e37c 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -100,8 +100,6 @@ static void fail_in_htlc(struct htlc_in *hin, /* We need this set, since we send it to channeld. */ if (hin->failcode & UPDATE) hin->failoutchannel = *out_channelid; - else - memset(&hin->failoutchannel, 0, sizeof(hin->failoutchannel)); /* We update state now to signal it's in progress, for persistence. */ htlc_in_update_state(hin->key.channel, hin, SENT_REMOVE_HTLC); diff --git a/wallet/wallet.c b/wallet/wallet.c index bba4a9621..bd3ca3760 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1362,10 +1362,17 @@ static bool wallet_stmt2htlc_out(struct channel *channel, return ok; } -#ifdef COMPAT_V061 -/* We didn't used to save failcore, failuremsg... */ static void fixup_hin(struct wallet *wallet, struct htlc_in *hin) { + /* We don't save the outgoing channel which failed; probably not worth + * it for this corner case. So we can't set hin->failoutchannel to + * tell channeld what update to send, thus we turn those into a + * WIRE_TEMPORARY_NODE_FAILURE. */ + if (hin->failcode & UPDATE) + hin->failcode = WIRE_TEMPORARY_NODE_FAILURE; + + /* We didn't used to save failcore, failuremsg... */ +#ifdef COMPAT_V061 /* We care about HTLCs being removed only, not those being added. */ if (hin->hstate < SENT_REMOVE_HTLC) return; @@ -1378,19 +1385,20 @@ static void fixup_hin(struct wallet *wallet, struct htlc_in *hin) if (hin->failcode || hin->failuremsg) return; - hin->failcode = WIRE_TEMPORARY_CHANNEL_FAILURE; + hin->failcode = WIRE_TEMPORARY_NODE_FAILURE; log_broken(wallet->log, "HTLC #%"PRIu64" (%s) " " for amount %"PRIu64 " from %s" " is missing a resolution:" - " subsituting temporary channel failure", + " subsituting temporary node failure", hin->key.id, htlc_state_name(hin->hstate), hin->msatoshi, type_to_string(tmpctx, struct pubkey, &hin->key.channel->peer->id)); -} #endif +} + bool wallet_htlcs_load_for_channel(struct wallet *wallet, struct channel *chan, @@ -1416,9 +1424,7 @@ bool wallet_htlcs_load_for_channel(struct wallet *wallet, struct htlc_in *in = tal(chan, struct htlc_in); ok &= wallet_stmt2htlc_in(chan, stmt, in); connect_htlc_in(htlcs_in, in); -#ifdef COMPAT_V061 fixup_hin(wallet, in); -#endif ok &= htlc_in_check(in, NULL) != NULL; incount++; }