From bb9b761dda538bb99e95b473519ad82ddc23b990 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 18 Feb 2020 10:25:58 +1030 Subject: [PATCH] channeld: don't get details of our own failed htlcs at init. For incoming htlcs, we need failure details in case we need to re-xmit them. But for outgoing htlcs, lightningd is telling us it already knows they've failed, so we just need to flag them failed and don't need the details. Internally, we set the ->fail to a dummy non-NULL value; this is cleaned up next. This matters for the next patch, which moves onion handling into lightningd. Signed-off-by: Rusty Russell --- channeld/channel_wire.csv | 7 +-- channeld/channeld.c | 17 +++---- channeld/full_channel.c | 88 +++++++++++++++++++++++------------- channeld/full_channel.h | 8 ++-- lightningd/channel_control.c | 8 ++-- lightningd/peer_htlcs.c | 23 ++++------ lightningd/peer_htlcs.h | 4 +- 7 files changed, 89 insertions(+), 66 deletions(-) diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index cc4342b3b..27767d352 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -45,9 +45,10 @@ msgdata,channel_init,htlc_states,enum htlc_state,num_htlcs msgdata,channel_init,num_fulfilled,u16, msgdata,channel_init,fulfilled,fulfilled_htlc,num_fulfilled msgdata,channel_init,fulfilled_sides,enum side,num_fulfilled -msgdata,channel_init,num_failed,u16, -msgdata,channel_init,failed,failed_htlc,num_failed -msgdata,channel_init,failed_sides,enum side,num_failed +msgdata,channel_init,num_failed_in,u16, +msgdata,channel_init,failed_in,failed_htlc,num_failed_in +msgdata,channel_init,num_failed_out,u16, +msgdata,channel_init,failed_out,u64,num_failed_out msgdata,channel_init,failheight,u32, msgdata,channel_init,local_funding_locked,bool, msgdata,channel_init,remote_funding_locked,bool, diff --git a/channeld/channeld.c b/channeld/channeld.c index efe9f7319..6049fb65e 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3005,8 +3005,8 @@ static void init_channel(struct peer *peer) enum htlc_state *hstates; struct fulfilled_htlc *fulfilled; enum side *fulfilled_sides; - struct failed_htlc **failed; - enum side *failed_sides; + struct failed_htlc **failed_in; + u64 *failed_out; struct added_htlc *htlcs; bool reconnected; u8 *funding_signed; @@ -3060,8 +3060,8 @@ static void init_channel(struct peer *peer) &hstates, &fulfilled, &fulfilled_sides, - &failed, - &failed_sides, + &failed_in, + &failed_out, &failheight, &peer->funding_locked[LOCAL], &peer->funding_locked[REMOTE], @@ -3143,8 +3143,9 @@ static void init_channel(struct peer *peer) if (!channel_force_htlcs(peer->channel, htlcs, hstates, fulfilled, fulfilled_sides, cast_const2(const struct failed_htlc **, - failed), - failed_sides, failheight)) + failed_in), + failed_out, + failheight)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Could not restore HTLCs"); @@ -3157,8 +3158,8 @@ static void init_channel(struct peer *peer) tal_free(hstates); tal_free(fulfilled); tal_free(fulfilled_sides); - tal_free(failed); - tal_free(failed_sides); + tal_free(failed_in); + tal_free(failed_out); tal_free(remote_ann_node_sig); tal_free(remote_ann_bitcoin_sig); diff --git a/channeld/full_channel.c b/channeld/full_channel.c index efaab0d44..b1c7f111b 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -1188,8 +1188,8 @@ bool channel_force_htlcs(struct channel *channel, const enum htlc_state *hstates, const struct fulfilled_htlc *fulfilled, const enum side *fulfilled_sides, - const struct failed_htlc **failed, - const enum side *failed_sides, + const struct failed_htlc **failed_in, + const u64 *failed_out, u32 failheight) { size_t i; @@ -1209,12 +1209,6 @@ bool channel_force_htlcs(struct channel *channel, return false; } - if (tal_count(failed) != tal_count(failed_sides)) { - status_broken("#failed sides %zu != #failed %zu", - tal_count(failed_sides), tal_count(failed)); - return false; - } - for (i = 0; i < tal_count(htlcs); i++) { enum channel_add_err e; struct htlc *htlc; @@ -1282,58 +1276,88 @@ bool channel_force_htlcs(struct channel *channel, &fulfilled[i].payment_preimage); } - for (i = 0; i < tal_count(failed); i++) { + for (i = 0; i < tal_count(failed_in); i++) { struct htlc *htlc; - htlc = channel_get_htlc(channel, failed_sides[i], - failed[i]->id); + htlc = channel_get_htlc(channel, REMOTE, failed_in[i]->id); if (!htlc) { - status_broken("Fail %s HTLC %"PRIu64" not found", - failed_sides[i] == LOCAL ? "out" : "in", - failed[i]->id); + status_broken("Fail in HTLC %"PRIu64" not found", + failed_in[i]->id); return false; } if (htlc->r) { - status_broken("Fail %s HTLC %"PRIu64" already fulfilled", - failed_sides[i] == LOCAL ? "out" : "in", - failed[i]->id); + status_broken("Fail in HTLC %"PRIu64" already fulfilled", + failed_in[i]->id); return false; } if (htlc->fail) { - status_broken("Fail %s HTLC %"PRIu64" already failed", - failed_sides[i] == LOCAL ? "out" : "in", - failed[i]->id); + status_broken("Fail in HTLC %"PRIu64" already failed_in", + failed_in[i]->id); return false; } if (htlc->failcode) { - status_broken("Fail %s HTLC %"PRIu64" already fail %u", - failed_sides[i] == LOCAL ? "out" : "in", - failed[i]->id, htlc->failcode); + status_broken("Fail in HTLC %"PRIu64" already fail %u", + failed_in[i]->id, htlc->failcode); return false; } if (!htlc_has(htlc, HTLC_REMOVING)) { - status_broken("Fail %s HTLC %"PRIu64" state %s", - failed_sides[i] == LOCAL ? "out" : "in", - failed[i]->id, + status_broken("Fail in HTLC %"PRIu64" state %s", + failed_in[i]->id, htlc_state_name(htlc->state)); return false; } - htlc->failcode = failed[i]->failcode; - /* We assume they all failed at the same height, which is + htlc->failcode = failed_in[i]->failcode; + /* We assume they all failed_in at the same height, which is * not necessarily true in case of restart. But it's only * a hint. */ htlc->failblock = failheight; - if (failed[i]->failreason) - htlc->fail = dup_onionreply(htlc, failed[i]->failreason); + if (failed_in[i]->failreason) + htlc->fail = dup_onionreply(htlc, failed_in[i]->failreason); else htlc->fail = NULL; - if (failed[i]->scid) + if (failed_in[i]->scid) htlc->failed_scid = tal_dup(htlc, struct short_channel_id, - failed[i]->scid); + failed_in[i]->scid); else htlc->failed_scid = NULL; } + for (i = 0; i < tal_count(failed_out); i++) { + struct htlc *htlc; + + htlc = channel_get_htlc(channel, LOCAL, failed_out[i]); + if (!htlc) { + status_broken("Fail out HTLC %"PRIu64" not found", + failed_out[i]); + return false; + } + if (htlc->r) { + status_broken("Fail out HTLC %"PRIu64" already fulfilled", + failed_out[i]); + return false; + } + if (htlc->fail) { + status_broken("Fail out HTLC %"PRIu64" already failed", + failed_out[i]); + return false; + } + if (htlc->failcode) { + status_broken("Fail out HTLC %"PRIu64" already fail %u", + failed_out[i], htlc->failcode); + return false; + } + if (!htlc_has(htlc, HTLC_REMOVING)) { + status_broken("Fail out HTLC %"PRIu64" state %s", + failed_out[i], + htlc_state_name(htlc->state)); + return false; + } + + /* Now, we don't really care why our htlcs failed: lightningd + * already knows. Just mark it failed using anything. */ + htlc->fail = tal(htlc, struct onionreply); + } + /* You'd think, since we traverse HTLCs in ID order, this would never * go negative. But this ignores the fact that HTLCs ids from each * side have no correlation with each other. Copy into struct balance, diff --git a/channeld/full_channel.h b/channeld/full_channel.h index 1cfd950c1..5c44460a7 100644 --- a/channeld/full_channel.h +++ b/channeld/full_channel.h @@ -231,8 +231,8 @@ size_t num_channel_htlcs(const struct channel *channel); * @hstates: the states for the htlcs (tal_arr of same size) * @fulfilled: htlcs of those which are fulfilled * @fulfilled_sides: sides for ids in @fulfilled - * @failed: htlcs of those which are failed - * @failed_sides: sides for ids in @failed + * @failed_in: incoming htlcs which are failed + * @failed_out: outgoing htlc ids which are failed * @failheight: block number which htlcs failed at. * * This is used for restoring a channel state. @@ -242,8 +242,8 @@ bool channel_force_htlcs(struct channel *channel, const enum htlc_state *hstates, const struct fulfilled_htlc *fulfilled, const enum side *fulfilled_sides, - const struct failed_htlc **failed, - const enum side *failed_sides, + const struct failed_htlc **failed_in, + const u64 *failed_out, u32 failheight); /** diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 7a3bd3479..4ce3a80eb 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -369,8 +369,8 @@ void peer_start_channeld(struct channel *channel, enum htlc_state *htlc_states; struct fulfilled_htlc *fulfilled_htlcs; enum side *fulfilled_sides; - const struct failed_htlc **failed_htlcs; - enum side *failed_sides; + const struct failed_htlc **failed_in; + u64 *failed_out; struct short_channel_id scid; u64 num_revocations; struct lightningd *ld = channel->peer->ld; @@ -409,7 +409,7 @@ void peer_start_channeld(struct channel *channel, } peer_htlcs(tmpctx, channel, &htlcs, &htlc_states, &fulfilled_htlcs, - &fulfilled_sides, &failed_htlcs, &failed_sides); + &fulfilled_sides, &failed_in, &failed_out); if (channel->scid) { scid = *channel->scid; @@ -492,7 +492,7 @@ void peer_start_channeld(struct channel *channel, channel->next_htlc_id, htlcs, htlc_states, fulfilled_htlcs, fulfilled_sides, - failed_htlcs, failed_sides, + failed_in, failed_out, /* This is an approximation, but failing * on restart is a corner case */ get_block_height(ld->topology), diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index b02b83b2d..19c5890e0 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -1799,12 +1799,11 @@ static void add_fulfill(u64 id, enum side side, tal_arr_expand(fulfilled_sides, side); } -static void add_fail(u64 id, enum side side, +static void add_fail(u64 id, enum onion_type failcode, const struct short_channel_id *failing_channel, const struct onionreply *failonion, - const struct failed_htlc ***failed_htlcs, - enum side **failed_sides) + const struct failed_htlc ***failed_htlcs) { struct failed_htlc *newf; @@ -1824,7 +1823,6 @@ static void add_fail(u64 id, enum side side, newf->failreason = NULL; tal_arr_expand(failed_htlcs, newf); - tal_arr_expand(failed_sides, side); } /* FIXME: Load direct from db. */ @@ -1834,8 +1832,8 @@ void peer_htlcs(const tal_t *ctx, enum htlc_state **htlc_states, struct fulfilled_htlc **fulfilled_htlcs, enum side **fulfilled_sides, - const struct failed_htlc ***failed_htlcs, - enum side **failed_sides) + const struct failed_htlc ***failed_in, + u64 **failed_out) { struct htlc_in_map_iter ini; struct htlc_out_map_iter outi; @@ -1847,8 +1845,8 @@ void peer_htlcs(const tal_t *ctx, *htlc_states = tal_arr(ctx, enum htlc_state, 0); *fulfilled_htlcs = tal_arr(ctx, struct fulfilled_htlc, 0); *fulfilled_sides = tal_arr(ctx, enum side, 0); - *failed_htlcs = tal_arr(ctx, const struct failed_htlc *, 0); - *failed_sides = tal_arr(ctx, enum side, 0); + *failed_in = tal_arr(ctx, const struct failed_htlc *, 0); + *failed_out = tal_arr(ctx, u64, 0); for (hin = htlc_in_map_first(&ld->htlcs_in, &ini); hin; @@ -1862,9 +1860,9 @@ void peer_htlcs(const tal_t *ctx, hin->hstate); if (hin->failonion || hin->failcode) - add_fail(hin->key.id, REMOTE, hin->failcode, + add_fail(hin->key.id, hin->failcode, &hin->failoutchannel, - hin->failonion, failed_htlcs, failed_sides); + hin->failonion, failed_in); if (hin->preimage) add_fulfill(hin->key.id, REMOTE, hin->preimage, fulfilled_htlcs, fulfilled_sides); @@ -1882,9 +1880,8 @@ void peer_htlcs(const tal_t *ctx, hout->hstate); if (hout->failonion || hout->failcode) - add_fail(hout->key.id, LOCAL, hout->failcode, - hout->key.channel->scid, - hout->failonion, failed_htlcs, failed_sides); + tal_arr_expand(failed_out, hout->key.id); + if (hout->preimage) add_fulfill(hout->key.id, LOCAL, hout->preimage, fulfilled_htlcs, fulfilled_sides); diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h index c3fecb29b..14b6e172f 100644 --- a/lightningd/peer_htlcs.h +++ b/lightningd/peer_htlcs.h @@ -35,8 +35,8 @@ void peer_htlcs(const tal_t *ctx, enum htlc_state **htlc_states, struct fulfilled_htlc **fulfilled_htlcs, enum side **fulfilled_sides, - const struct failed_htlc ***failed_htlcs, - enum side **failed_sides); + const struct failed_htlc ***failed_in, + u64 **failed_out); void free_htlcs(struct lightningd *ld, const struct channel *channel);