Browse Source

lightningd/channel.c: clearer functions names, better return values.

We call channel_sent_commit *before* sending (so we know if we need
to), so the name is wrong.  Similarly channel_sent_revoke_and_ack.

We can usefully have them tell is if there is outstanding work to do,
too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ppa-0.6.1
Rusty Russell 8 years ago
parent
commit
3041cd5915
  1. 46
      lightningd/channel.c
  2. 21
      lightningd/channel.h
  3. 2
      lightningd/channel/channel.c
  4. 69
      lightningd/test/run-channel.c

46
lightningd/channel.c

@ -635,7 +635,8 @@ static void check_lockedin(const struct htlc *h,
} }
/* FIXME: Commit to storage when this happens. */ /* 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, enum side sidechanged,
const enum htlc_state *htlc_states, const enum htlc_state *htlc_states,
size_t n_hstates, size_t n_hstates,
@ -646,7 +647,7 @@ static bool change_htlcs(struct channel *channel,
{ {
struct htlc_map_iter it; struct htlc_map_iter it;
struct htlc *h; struct htlc *h;
bool changed = false; int cflags = 0;
size_t i; size_t i;
for (h = htlc_map_first(&channel->htlcs, &it); for (h = htlc_map_first(&channel->htlcs, &it);
@ -660,23 +661,26 @@ static bool change_htlcs(struct channel *channel,
theirslocked, theirslocked,
theirsfulfilled, theirsfulfilled,
cbarg); cbarg);
changed = true; cflags |= (htlc_state_flags(htlc_states[i])
^ htlc_state_flags(h->state));
} }
} }
} }
return changed; return cflags;
} }
/* FIXME: Handle fee changes too. */ /* 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, const enum htlc_state states[] = { SENT_ADD_HTLC,
SENT_REMOVE_REVOCATION, SENT_REMOVE_REVOCATION,
SENT_ADD_REVOCATION, SENT_ADD_REVOCATION,
SENT_REMOVE_HTLC }; SENT_REMOVE_HTLC };
status_trace("sent commit"); status_trace("Trying commit");
return change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), change = change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states),
NULL, NULL, NULL, NULL); NULL, NULL, NULL, NULL);
return change & HTLC_REMOTE_F_COMMITTED;
} }
bool channel_rcvd_revoke_and_ack_(struct channel *channel, 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),
void *cbarg) void *cbarg)
{ {
int change;
const enum htlc_state states[] = { SENT_ADD_COMMIT, const enum htlc_state states[] = { SENT_ADD_COMMIT,
SENT_REMOVE_ACK_COMMIT, SENT_REMOVE_ACK_COMMIT,
SENT_ADD_ACK_COMMIT, SENT_ADD_ACK_COMMIT,
SENT_REMOVE_COMMIT }; SENT_REMOVE_COMMIT };
status_trace("received revoke_and_ack"); status_trace("Received revoke_and_ack");
return change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), change = change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states),
oursfail, theirslocked, NULL, cbarg); oursfail, theirslocked, NULL, cbarg);
return change & HTLC_LOCAL_F_COMMITTED;
} }
/* FIXME: We can actually merge these two... */ /* FIXME: We can actually merge these two... */
@ -702,25 +708,29 @@ bool channel_rcvd_commit_(struct channel *channel,
void *cbarg), void *cbarg),
void *cbarg) void *cbarg)
{ {
int change;
const enum htlc_state states[] = { RCVD_ADD_REVOCATION, const enum htlc_state states[] = { RCVD_ADD_REVOCATION,
RCVD_REMOVE_HTLC, RCVD_REMOVE_HTLC,
RCVD_ADD_HTLC, RCVD_ADD_HTLC,
RCVD_REMOVE_REVOCATION }; RCVD_REMOVE_REVOCATION };
status_trace("received commit"); status_trace("Received commit");
return change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), change = change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states),
NULL, NULL, theirsfulfilled, cbarg); 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, const enum htlc_state states[] = { RCVD_ADD_ACK_COMMIT,
RCVD_REMOVE_COMMIT, RCVD_REMOVE_COMMIT,
RCVD_ADD_COMMIT, RCVD_ADD_COMMIT,
RCVD_REMOVE_ACK_COMMIT }; RCVD_REMOVE_ACK_COMMIT };
status_trace("sent revoke_and_ack"); status_trace("Sending revoke_and_ack");
return change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), change = change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states),
NULL, NULL, NULL, NULL); NULL, NULL, NULL, NULL);
return change & HTLC_REMOTE_F_PENDING;
} }
static char *fmt_channel_view(const tal_t *ctx, const struct channel_view *view) static char *fmt_channel_view(const tal_t *ctx, const struct channel_view *view)

21
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); 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 * @channel: the channel
* *
* This is where we commit to pending changes we've added; returns true if * This is where we commit to pending changes we've added; returns true if
* anything changed. */ * anything changed for the remote side (if not, don't send!) */
bool channel_sent_commit(struct channel *channel); bool channel_sending_commit(struct channel *channel);
/** /**
* channel_rcvd_revoke_and_ack: accept ack on remote committed changes. * 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 * @cbarg: argument to pass through to @ourhtlcfail & @theirhtlclocked
* *
* This is where we commit to pending changes we've added; returns true if * 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) \ #define channel_rcvd_revoke_and_ack(channel, oursfail, theirslocked, cbarg) \
channel_rcvd_revoke_and_ack_((channel), \ 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 * @cbarg: argument to pass through to @theirsfulfilled
* *
* This is where we commit to pending changes we've added; returns true if * This is where we commit to pending changes we've added; returns true if
* anything changed. @theirsfulfilled is called for any HTLC we fulfilled * anything changed for our local commitment (ie. we had pending changes).
* which they are irrevocably committed to, and is in our current commitment. * @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) \ #define channel_rcvd_commit(channel, theirsfulfilled, cbarg) \
channel_rcvd_commit_((channel), \ channel_rcvd_commit_((channel), \
@ -381,11 +382,11 @@ bool channel_rcvd_commit_(struct channel *channel,
void *cbarg); 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 * @channel: the channel
* *
* This is where we commit to pending changes we've added; returns true if * This is where we commit to pending changes we've added. Returns true if
* anything changed. */ * anything changed for the remote commitment (ie. send a new commit).*/
bool channel_sent_revoke_and_ack(struct channel *channel); bool channel_sending_revoke_and_ack(struct channel *channel);
#endif /* LIGHTNING_DAEMON_CHANNEL_H */ #endif /* LIGHTNING_DAEMON_CHANNEL_H */

2
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 * A node MUST NOT send a `commitment_signed` message which does not
* include any updates. * include any updates.
*/ */
if (!channel_sent_commit(peer->channel)) { if (!channel_sending_commit(peer->channel)) {
tal_free(tmpctx); tal_free(tmpctx);
return; return;
} }

69
lightningd/test/run-channel.c

@ -113,6 +113,7 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side
int i; int i;
const struct htlc **htlcs = tal_arr(channel, const struct htlc *, 5); const struct htlc **htlcs = tal_arr(channel, const struct htlc *, 5);
u8 *dummy_routing = tal_arr(htlcs, u8, 1254); u8 *dummy_routing = tal_arr(htlcs, u8, 1254);
bool ret;
for (i = 0; i < 5; i++) { for (i = 0; i < 5; i++) {
struct preimage preimage; struct preimage preimage;
@ -153,12 +154,18 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side
tal_free(dummy_routing); tal_free(dummy_routing);
/* Now make HTLCs fully committed. */ /* Now make HTLCs fully committed. */
channel_sent_commit(channel); ret = channel_sending_commit(channel);
channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); assert(ret);
channel_rcvd_commit(channel, NULL, NULL); ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL);
channel_sent_revoke_and_ack(channel); assert(!ret);
channel_sent_commit(channel); ret = channel_rcvd_commit(channel, NULL, NULL);
channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, 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; return htlcs;
} }
@ -230,6 +237,7 @@ static void send_and_fulfill_htlc(struct channel *channel,
struct preimage r; struct preimage r;
struct sha256 rhash; struct sha256 rhash;
u8 *dummy_routing = tal_arr(channel, u8, 1254); u8 *dummy_routing = tal_arr(channel, u8, 1254);
bool ret;
memset(&r, 0, sizeof(r)); memset(&r, 0, sizeof(r));
sha256(&rhash, &r, sizeof(r)); sha256(&rhash, &r, sizeof(r));
@ -239,29 +247,46 @@ static void send_and_fulfill_htlc(struct channel *channel,
if (sender == LOCAL) { if (sender == LOCAL) {
/* Step through a complete cycle. */ /* Step through a complete cycle. */
channel_sent_commit(channel); ret = channel_sending_commit(channel);
channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); assert(ret);
channel_rcvd_commit(channel, NULL, NULL); ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL);
channel_sent_revoke_and_ack(channel); 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) assert(channel_fulfill_htlc(channel, REMOTE, 1337, &r)
== CHANNEL_ERR_REMOVE_OK); == CHANNEL_ERR_REMOVE_OK);
channel_rcvd_commit(channel, NULL, NULL); ret = channel_rcvd_commit(channel, NULL, NULL);
channel_sent_revoke_and_ack(channel); assert(ret);
channel_sent_commit(channel); ret = channel_sending_revoke_and_ack(channel);
channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); 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 assert(channel_get_htlc(channel, sender, 1337)->state
== RCVD_REMOVE_ACK_REVOCATION); == RCVD_REMOVE_ACK_REVOCATION);
} else { } else {
channel_rcvd_commit(channel, NULL, NULL); ret = channel_rcvd_commit(channel, NULL, NULL);
channel_sent_revoke_and_ack(channel); assert(ret);
channel_sent_commit(channel); ret = channel_sending_revoke_and_ack(channel);
channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, NULL); 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) assert(channel_fulfill_htlc(channel, LOCAL, 1337, &r)
== CHANNEL_ERR_REMOVE_OK); == CHANNEL_ERR_REMOVE_OK);
channel_sent_commit(channel); ret = channel_sending_commit(channel);
channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); assert(ret);
channel_rcvd_commit(channel, do_nothing, NULL); ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL);
channel_sent_revoke_and_ack(channel); 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 assert(channel_get_htlc(channel, sender, 1337)->state
== SENT_REMOVE_ACK_REVOCATION); == SENT_REMOVE_ACK_REVOCATION);
} }

Loading…
Cancel
Save