diff --git a/lightningd/channel.c b/lightningd/channel.c index 709221a5d..ccf6430e6 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -635,7 +635,8 @@ static void check_lockedin(const struct htlc *h, } /* FIXME: Commit to storage when this happens. */ -static bool change_htlcs(struct channel *channel, +/* Returns flags which were changed. */ +static int change_htlcs(struct channel *channel, enum side sidechanged, const enum htlc_state *htlc_states, size_t n_hstates, @@ -646,7 +647,7 @@ static bool change_htlcs(struct channel *channel, { struct htlc_map_iter it; struct htlc *h; - bool changed = false; + int cflags = 0; size_t i; for (h = htlc_map_first(&channel->htlcs, &it); @@ -660,23 +661,26 @@ static bool change_htlcs(struct channel *channel, theirslocked, theirsfulfilled, cbarg); - changed = true; + cflags |= (htlc_state_flags(htlc_states[i]) + ^ htlc_state_flags(h->state)); } } } - return changed; + return cflags; } /* FIXME: Handle fee changes too. */ -bool channel_sent_commit(struct channel *channel) +bool channel_sending_commit(struct channel *channel) { + int change; const enum htlc_state states[] = { SENT_ADD_HTLC, SENT_REMOVE_REVOCATION, SENT_ADD_REVOCATION, SENT_REMOVE_HTLC }; - status_trace("sent commit"); - return change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), - NULL, NULL, NULL, NULL); + status_trace("Trying commit"); + change = change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), + NULL, NULL, NULL, NULL); + return change & HTLC_REMOTE_F_COMMITTED; } bool channel_rcvd_revoke_and_ack_(struct channel *channel, @@ -686,14 +690,16 @@ bool channel_rcvd_revoke_and_ack_(struct channel *channel, void *cbarg), void *cbarg) { + int change; const enum htlc_state states[] = { SENT_ADD_COMMIT, SENT_REMOVE_ACK_COMMIT, SENT_ADD_ACK_COMMIT, SENT_REMOVE_COMMIT }; - status_trace("received revoke_and_ack"); - return change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), - oursfail, theirslocked, NULL, cbarg); + status_trace("Received revoke_and_ack"); + change = change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), + oursfail, theirslocked, NULL, cbarg); + return change & HTLC_LOCAL_F_COMMITTED; } /* FIXME: We can actually merge these two... */ @@ -702,25 +708,29 @@ bool channel_rcvd_commit_(struct channel *channel, void *cbarg), void *cbarg) { + int change; const enum htlc_state states[] = { RCVD_ADD_REVOCATION, RCVD_REMOVE_HTLC, RCVD_ADD_HTLC, RCVD_REMOVE_REVOCATION }; - status_trace("received commit"); - return change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), - NULL, NULL, theirsfulfilled, cbarg); + status_trace("Received commit"); + change = change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), + NULL, NULL, theirsfulfilled, cbarg); + return change & HTLC_LOCAL_F_COMMITTED; } -bool channel_sent_revoke_and_ack(struct channel *channel) +bool channel_sending_revoke_and_ack(struct channel *channel) { + int change; const enum htlc_state states[] = { RCVD_ADD_ACK_COMMIT, RCVD_REMOVE_COMMIT, RCVD_ADD_COMMIT, RCVD_REMOVE_ACK_COMMIT }; - status_trace("sent revoke_and_ack"); - return change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), - NULL, NULL, NULL, NULL); + status_trace("Sending revoke_and_ack"); + change = change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), + NULL, NULL, NULL, NULL); + return change & HTLC_REMOTE_F_PENDING; } static char *fmt_channel_view(const tal_t *ctx, const struct channel_view *view) diff --git a/lightningd/channel.h b/lightningd/channel.h index 31bdc7fda..16a49accc 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -321,12 +321,12 @@ void adjust_fee(struct channel *channel, u64 feerate_per_kw, enum side side); bool force_fee(struct channel *channel, u64 fee); /** - * channel_sent_commit: commit all remote outstanding changes. + * channel_sending_commit: commit all remote outstanding changes. * @channel: the channel * * This is where we commit to pending changes we've added; returns true if - * anything changed. */ -bool channel_sent_commit(struct channel *channel); + * anything changed for the remote side (if not, don't send!) */ +bool channel_sending_commit(struct channel *channel); /** * channel_rcvd_revoke_and_ack: accept ack on remote committed changes. @@ -336,7 +336,7 @@ bool channel_sent_commit(struct channel *channel); * @cbarg: argument to pass through to @ourhtlcfail & @theirhtlclocked * * This is where we commit to pending changes we've added; returns true if - * anything changed. + * anything changed for our local commitment (ie. we have pending changes). */ #define channel_rcvd_revoke_and_ack(channel, oursfail, theirslocked, cbarg) \ channel_rcvd_revoke_and_ack_((channel), \ @@ -364,8 +364,9 @@ bool channel_rcvd_revoke_and_ack_(struct channel *channel, * @cbarg: argument to pass through to @theirsfulfilled * * This is where we commit to pending changes we've added; returns true if - * anything changed. @theirsfulfilled is called for any HTLC we fulfilled - * which they are irrevocably committed to, and is in our current commitment. + * anything changed for our local commitment (ie. we had pending changes). + * @theirsfulfilled is called for any HTLC we fulfilled which they are + * irrevocably committed to, and is in our current commitment. */ #define channel_rcvd_commit(channel, theirsfulfilled, cbarg) \ channel_rcvd_commit_((channel), \ @@ -381,11 +382,11 @@ bool channel_rcvd_commit_(struct channel *channel, void *cbarg); /** - * channel_sent_revoke_and_ack: sent ack on local committed changes. + * channel_sending_revoke_and_ack: sending ack on local committed changes. * @channel: the channel * - * This is where we commit to pending changes we've added; returns true if - * anything changed. */ -bool channel_sent_revoke_and_ack(struct channel *channel); + * This is where we commit to pending changes we've added. Returns true if + * anything changed for the remote commitment (ie. send a new commit).*/ +bool channel_sending_revoke_and_ack(struct channel *channel); #endif /* LIGHTNING_DAEMON_CHANNEL_H */ diff --git a/lightningd/channel/channel.c b/lightningd/channel/channel.c index 6d0b147a5..e7d927730 100644 --- a/lightningd/channel/channel.c +++ b/lightningd/channel/channel.c @@ -280,7 +280,7 @@ static void send_commit(struct peer *peer) * A node MUST NOT send a `commitment_signed` message which does not * include any updates. */ - if (!channel_sent_commit(peer->channel)) { + if (!channel_sending_commit(peer->channel)) { tal_free(tmpctx); return; } diff --git a/lightningd/test/run-channel.c b/lightningd/test/run-channel.c index eeb3e4cac..048a30c58 100644 --- a/lightningd/test/run-channel.c +++ b/lightningd/test/run-channel.c @@ -113,6 +113,7 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side int i; const struct htlc **htlcs = tal_arr(channel, const struct htlc *, 5); u8 *dummy_routing = tal_arr(htlcs, u8, 1254); + bool ret; for (i = 0; i < 5; i++) { struct preimage preimage; @@ -153,12 +154,18 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side tal_free(dummy_routing); /* Now make HTLCs fully committed. */ - channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); - channel_rcvd_commit(channel, NULL, NULL); - channel_sent_revoke_and_ack(channel); - channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, NULL); + ret = channel_sending_commit(channel); + assert(ret); + ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + assert(!ret); + ret = channel_rcvd_commit(channel, NULL, NULL); + assert(ret); + ret = channel_sending_revoke_and_ack(channel); + assert(ret); + ret = channel_sending_commit(channel); + assert(ret); + ret = channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, NULL); + assert(!ret); return htlcs; } @@ -230,6 +237,7 @@ static void send_and_fulfill_htlc(struct channel *channel, struct preimage r; struct sha256 rhash; u8 *dummy_routing = tal_arr(channel, u8, 1254); + bool ret; memset(&r, 0, sizeof(r)); sha256(&rhash, &r, sizeof(r)); @@ -239,29 +247,46 @@ static void send_and_fulfill_htlc(struct channel *channel, if (sender == LOCAL) { /* Step through a complete cycle. */ - channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); - channel_rcvd_commit(channel, NULL, NULL); - channel_sent_revoke_and_ack(channel); + ret = channel_sending_commit(channel); + assert(ret); + ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + assert(!ret); + ret = channel_rcvd_commit(channel, NULL, NULL); + assert(ret); + ret = channel_sending_revoke_and_ack(channel); + assert(!ret); assert(channel_fulfill_htlc(channel, REMOTE, 1337, &r) == CHANNEL_ERR_REMOVE_OK); - channel_rcvd_commit(channel, NULL, NULL); - channel_sent_revoke_and_ack(channel); - channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + ret = channel_rcvd_commit(channel, NULL, NULL); + assert(ret); + ret = channel_sending_revoke_and_ack(channel); + assert(ret); + ret = channel_sending_commit(channel); + assert(ret); + ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + assert(!ret); assert(channel_get_htlc(channel, sender, 1337)->state == RCVD_REMOVE_ACK_REVOCATION); } else { - channel_rcvd_commit(channel, NULL, NULL); - channel_sent_revoke_and_ack(channel); - channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, NULL); + ret = channel_rcvd_commit(channel, NULL, NULL); + assert(ret); + ret = channel_sending_revoke_and_ack(channel); + assert(ret); + ret = channel_sending_commit(channel); + assert(ret); + ret = channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, + NULL); + assert(!ret); assert(channel_fulfill_htlc(channel, LOCAL, 1337, &r) == CHANNEL_ERR_REMOVE_OK); - channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); - channel_rcvd_commit(channel, do_nothing, NULL); - channel_sent_revoke_and_ack(channel); + ret = channel_sending_commit(channel); + assert(ret); + ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + assert(!ret); + ret = channel_rcvd_commit(channel, do_nothing, NULL); + assert(ret); + ret = channel_sending_revoke_and_ack(channel); + assert(!ret); assert(channel_get_htlc(channel, sender, 1337)->state == SENT_REMOVE_ACK_REVOCATION); }