From 4b5ec85c252f3025739efbf3f5f2556d73b87ad6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 18 Aug 2016 14:23:45 +0930 Subject: [PATCH] daemon: keep enum htlc_state within struct htlc. And update the state as HTLCs get moved around. Signed-off-by: Rusty Russell --- daemon/packets.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++ daemon/peer.c | 50 +++++++++++++++------------ daemon/peer.h | 1 + 3 files changed, 119 insertions(+), 21 deletions(-) diff --git a/daemon/packets.c b/daemon/packets.c index 7e245a304..8c377991f 100644 --- a/daemon/packets.c +++ b/daemon/packets.c @@ -13,6 +13,7 @@ #include "secrets.h" #include "state.h" #include "utils.h" +#include #include #include #include @@ -219,6 +220,7 @@ void queue_pkt_htlc_fulfill(struct peer *peer, struct htlc *htlc, stage.fulfill.htlc = htlc; stage.fulfill.r = *r; add_unacked(&peer->remote, &stage); + htlc_changestate(htlc, RCVD_ADD_ACK_REVOCATION, SENT_REMOVE_HTLC); remote_changes_pending(peer); @@ -249,16 +251,69 @@ void queue_pkt_htlc_fail(struct peer *peer, struct htlc *htlc) stage.fail.fail = HTLC_FAIL; stage.fail.htlc = htlc; add_unacked(&peer->remote, &stage); + htlc_changestate(htlc, RCVD_ADD_ACK_REVOCATION, SENT_REMOVE_HTLC); remote_changes_pending(peer); queue_pkt(peer, PKT__PKT_UPDATE_FAIL_HTLC, f); } +struct state_table { + enum htlc_state from, to; +}; + +static bool htlcs_changestate(struct peer *peer, + const struct state_table *table, + size_t n) +{ + struct htlc_map_iter it; + struct htlc *h; + bool changed = false; + + for (h = htlc_map_first(&peer->local.htlcs, &it); + h; + h = htlc_map_next(&peer->local.htlcs, &it)) { + size_t i; + for (i = 0; i < n; i++) { + if (h->state == table[i].from) { + htlc_changestate(h, table[i].from, table[i].to); + changed = true; + } + } + } + + for (h = htlc_map_first(&peer->remote.htlcs, &it); + h; + h = htlc_map_next(&peer->remote.htlcs, &it)) { + size_t i; + for (i = 0; i < n; i++) { + if (h->state == table[i].from) { + htlc_changestate(h, table[i].from, table[i].to); + changed = true; + } + } + } + return changed; +} + /* OK, we're sending a signature for their pending changes. */ void queue_pkt_commit(struct peer *peer) { UpdateCommit *u = tal(peer, UpdateCommit); struct commit_info *ci = new_commit_info(peer); + static const struct state_table changes[] = { + { SENT_ADD_HTLC, SENT_ADD_COMMIT }, + { SENT_REMOVE_REVOCATION, SENT_REMOVE_ACK_COMMIT }, + { SENT_ADD_REVOCATION, SENT_ADD_ACK_COMMIT}, + { SENT_REMOVE_HTLC, SENT_REMOVE_COMMIT} + }; + + /* BOLT #2: + * + * A node MUST NOT send an `update_commit` message which does + * not include any updates. + */ + if (!htlcs_changestate(peer, changes, ARRAY_SIZE(changes))) + fatal("sent commit with no changes"); /* Create new commit info for this commit tx. */ ci->prev = peer->remote.commit; @@ -363,6 +418,12 @@ void queue_pkt_revocation(struct peer *peer) { UpdateRevocation *u = tal(peer, UpdateRevocation); struct commit_info *ci; + static const struct state_table changes[] = { + { RCVD_ADD_ACK_COMMIT, SENT_ADD_ACK_REVOCATION }, + { RCVD_REMOVE_COMMIT, SENT_REMOVE_REVOCATION }, + { RCVD_ADD_COMMIT, SENT_ADD_REVOCATION }, + { RCVD_REMOVE_ACK_COMMIT, SENT_REMOVE_ACK_REVOCATION } + }; update_revocation__init(u); @@ -374,6 +435,9 @@ void queue_pkt_revocation(struct peer *peer) /* We have their signature on the current one, right? */ assert(peer->local.commit->sig); + if (!htlcs_changestate(peer, changes, ARRAY_SIZE(changes))) + fatal("sent revoke with no changes"); + ci->revocation_preimage = tal(ci, struct sha256); peer_get_revocation_preimage(peer, ci->commit_num, ci->revocation_preimage); @@ -701,6 +765,7 @@ Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt) stage.fail.fail = HTLC_FAIL; stage.fail.htlc = htlc; add_unacked(&peer->local, &stage); + htlc_changestate(htlc, SENT_ADD_ACK_REVOCATION, RCVD_REMOVE_HTLC); return NULL; } @@ -733,6 +798,7 @@ Pkt *accept_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt) * to the unacked changeset for its local commitment. */ cstate_fulfill_htlc(peer->local.staging_cstate, htlc, OURS); + htlc_changestate(htlc, SENT_ADD_ACK_REVOCATION, RCVD_REMOVE_HTLC); stage.fulfill.fulfill = HTLC_FULFILL; stage.fulfill.htlc = htlc; @@ -746,6 +812,20 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt) const UpdateCommit *c = pkt->update_commit; Pkt *err; struct commit_info *ci = new_commit_info(peer); + static const struct state_table changes[] = { + { RCVD_ADD_REVOCATION, RCVD_ADD_ACK_COMMIT }, + { RCVD_REMOVE_HTLC, RCVD_REMOVE_COMMIT }, + { RCVD_ADD_HTLC, RCVD_ADD_COMMIT }, + { RCVD_REMOVE_REVOCATION, RCVD_REMOVE_ACK_COMMIT } + }; + + /* BOLT #2: + * + * A node MUST NOT send an `update_commit` message which does + * not include any updates. + */ + if (!htlcs_changestate(peer, changes, ARRAY_SIZE(changes))) + return pkt_err(peer, "Empty commit"); /* Create new commit info for this commit tx. */ ci->prev = peer->local.commit; @@ -805,6 +885,12 @@ Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt) { const UpdateRevocation *r = pkt->update_revocation; struct commit_info *ci = peer->remote.commit->prev; + static const struct state_table changes[] = { + { SENT_ADD_COMMIT, RCVD_ADD_REVOCATION }, + { SENT_REMOVE_ACK_COMMIT, RCVD_REMOVE_ACK_REVOCATION }, + { SENT_ADD_ACK_COMMIT, RCVD_ADD_ACK_REVOCATION }, + { SENT_REMOVE_COMMIT, RCVD_REMOVE_REVOCATION } + }; /* BOLT #2: * @@ -839,6 +925,9 @@ Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt) ci->unacked_changes, tal_count(ci->unacked_changes)); + if (!htlcs_changestate(peer, changes, ARRAY_SIZE(changes))) + fatal("Revocation received but we made empty commitment?"); + /* Should never examine these again. */ ci->unacked_changes = tal_free(ci->unacked_changes); diff --git a/daemon/peer.c b/daemon/peer.c index c563f04d5..c859584b7 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -162,30 +162,32 @@ static void peer_breakdown(struct peer *peer) /* All unrevoked commit txs must have no HTLCs in them. */ static bool committed_to_htlcs(const struct peer *peer) { - const struct commit_info *i; + struct htlc_map_iter it; + struct htlc *h; - /* Before anchor exchange, we don't even have cstate. */ - if (!peer->local.commit || !peer->local.commit->cstate) - return false; - - i = peer->local.commit; - while (i && !i->revocation_preimage) { - if (tal_count(i->cstate->side[OURS].htlcs)) - return true; - if (tal_count(i->cstate->side[THEIRS].htlcs)) - return true; - i = i->prev; + for (h = htlc_map_first(&peer->local.htlcs, &it); + h; + h = htlc_map_next(&peer->local.htlcs, &it)) { + /* FIXME: Move these dead ones to a separate hash (or + * just leave in database only). */ + if (h->state == RCVD_REMOVE_ACK_REVOCATION) + continue; + if (h->state == SENT_REMOVE_ACK_REVOCATION) + continue; + return true; } - i = peer->remote.commit; - while (i && !i->revocation_preimage) { - if (tal_count(i->cstate->side[OURS].htlcs)) - return true; - if (tal_count(i->cstate->side[THEIRS].htlcs)) - return true; - i = i->prev; + for (h = htlc_map_first(&peer->remote.htlcs, &it); + h; + h = htlc_map_next(&peer->remote.htlcs, &it)) { + /* FIXME: Move these dead ones to a separate hash (or + * just leave in database only). */ + if (h->state == RCVD_REMOVE_ACK_REVOCATION) + continue; + if (h->state == SENT_REMOVE_ACK_REVOCATION) + continue; + return true; } - return false; } @@ -557,10 +559,15 @@ static struct htlc *htlc_by_index(const struct commit_info *ci, size_t index) assert(index >= 2); index -= 2; - if (index < tal_count(ci->cstate->side[OURS].htlcs)) + if (index < tal_count(ci->cstate->side[OURS].htlcs)) { + assert(htlc_has(ci->cstate->side[OURS].htlcs[index], + HTLC_LOCAL_F_OWNER)); return ci->cstate->side[OURS].htlcs[index]; + } index -= tal_count(ci->cstate->side[OURS].htlcs); assert(index < tal_count(ci->cstate->side[THEIRS].htlcs)); + assert(htlc_has(ci->cstate->side[THEIRS].htlcs[index], + HTLC_REMOTE_F_OWNER)); return ci->cstate->side[THEIRS].htlcs[index]; } @@ -1034,6 +1041,7 @@ struct htlc *peer_new_htlc(struct peer *peer, struct htlc *h = tal(peer, struct htlc); h->peer = peer; assert(state == SENT_ADD_HTLC || state == RCVD_ADD_HTLC); + h->state = state; h->id = id; h->msatoshis = msatoshis; h->rhash = *rhash; diff --git a/daemon/peer.h b/daemon/peer.h index 34331e85b..d4a39cab0 100644 --- a/daemon/peer.h +++ b/daemon/peer.h @@ -97,6 +97,7 @@ struct peer_visible_state { /* cstate to generate next commitment tx. */ struct channel_state *staging_cstate; + /* FIXME: Use single map in struct peer. */ /* HTLCs offered by this side */ struct htlc_map htlcs; };