From 7105085801b232056a6984976f5626a2d55da5ce Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 20 Jun 2017 15:11:03 +0930 Subject: [PATCH] lightningd/channel: hand back changed htlcs, not callbacks. Means caller has to do some more work, but this is closer to what we want: we're going to want to send them to the master daemon for atomic commit. Signed-off-by: Rusty Russell --- lightningd/channel.c | 65 +++++++++++++---------------------- lightningd/channel.h | 48 +++++--------------------- lightningd/channel/channel.c | 31 ++++++++--------- lightningd/test/run-channel.c | 44 ++++++++++++------------ 4 files changed, 69 insertions(+), 119 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index dceb55cec..7d4ad9109 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -627,31 +627,24 @@ static void htlc_incstate(struct channel *channel, } } -static void check_lockedin(const struct htlc *h, - void (*oursfail)(const struct htlc *, void *), - void (*theirslocked)(const struct htlc *, void *), - void (*theirsfulfilled)(const struct htlc *, void *), - void *cbarg) +static void append_htlc(const struct htlc ***htlcs, const struct htlc *h) { - /* If it was fulfilled, we handled it immediately. */ - if (h->state == RCVD_REMOVE_ACK_REVOCATION && !h->r) - oursfail(h, cbarg); - else if (h->state == RCVD_ADD_ACK_REVOCATION) - theirslocked(h, cbarg); - else if (h->state == RCVD_REMOVE_ACK_COMMIT && h->r) - theirsfulfilled(h, cbarg); + size_t n; + + if (!htlcs) + return; + + n = tal_count(*htlcs); + tal_resize(htlcs, n+1); + (*htlcs)[n] = h; } -/* FIXME: Commit to storage when this happens. */ /* 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, - void (*oursfail)(const struct htlc *, void *), - void (*theirslocked)(const struct htlc *, void *), - void (*theirsfulfilled)(const struct htlc *, void *), - void *cbarg) + enum side sidechanged, + const enum htlc_state *htlc_states, + size_t n_hstates, + const struct htlc ***htlcs) { struct htlc_map_iter it; struct htlc *h; @@ -664,11 +657,7 @@ static int change_htlcs(struct channel *channel, for (i = 0; i < n_hstates; i++) { if (h->state == htlc_states[i]) { htlc_incstate(channel, h, sidechanged); - check_lockedin(h, - oursfail, - theirslocked, - theirsfulfilled, - cbarg); + append_htlc(htlcs, h); cflags |= (htlc_state_flags(htlc_states[i]) ^ htlc_state_flags(h->state)); } @@ -678,7 +667,8 @@ static int change_htlcs(struct channel *channel, } /* FIXME: Handle fee changes too. */ -bool channel_sending_commit(struct channel *channel) +bool channel_sending_commit(struct channel *channel, + const struct htlc ***htlcs) { int change; const enum htlc_state states[] = { SENT_ADD_HTLC, @@ -687,16 +677,12 @@ bool channel_sending_commit(struct channel *channel) SENT_REMOVE_HTLC }; status_trace("Trying commit"); change = change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), - NULL, NULL, NULL, NULL); + htlcs); return change & HTLC_REMOTE_F_COMMITTED; } -bool channel_rcvd_revoke_and_ack_(struct channel *channel, - void (*oursfail)(const struct htlc *htlc, - void *cbarg), - void (*theirslocked)(const struct htlc *htlc, - void *cbarg), - void *cbarg) +bool channel_rcvd_revoke_and_ack(struct channel *channel, + const struct htlc ***htlcs) { int change; const enum htlc_state states[] = { SENT_ADD_COMMIT, @@ -706,15 +692,12 @@ bool channel_rcvd_revoke_and_ack_(struct channel *channel, status_trace("Received revoke_and_ack"); change = change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), - oursfail, theirslocked, NULL, cbarg); + htlcs); return change & HTLC_LOCAL_F_COMMITTED; } /* FIXME: We can actually merge these two... */ -bool channel_rcvd_commit_(struct channel *channel, - void (*theirsfulfilled)(const struct htlc *htlc, - void *cbarg), - void *cbarg) +bool channel_rcvd_commit(struct channel *channel, const struct htlc ***htlcs) { int change; const enum htlc_state states[] = { RCVD_ADD_REVOCATION, @@ -723,8 +706,7 @@ bool channel_rcvd_commit_(struct channel *channel, RCVD_REMOVE_REVOCATION }; status_trace("Received commit"); - change = change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), - NULL, NULL, theirsfulfilled, cbarg); + change = change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), htlcs); return change & HTLC_LOCAL_F_COMMITTED; } @@ -736,8 +718,7 @@ bool channel_sending_revoke_and_ack(struct channel *channel) RCVD_ADD_COMMIT, RCVD_REMOVE_ACK_COMMIT }; status_trace("Sending revoke_and_ack"); - change = change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), - NULL, NULL, NULL, NULL); + change = change_htlcs(channel, REMOTE, states, ARRAY_SIZE(states), NULL); return change & HTLC_REMOTE_F_PENDING; } diff --git a/lightningd/channel.h b/lightningd/channel.h index 1d498f25d..701169edc 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -324,63 +323,34 @@ bool force_fee(struct channel *channel, u64 fee); /** * channel_sending_commit: commit all remote outstanding changes. * @channel: the channel + * @htlcs: initially-empty tal_arr() for htlcs which changed state. * * This is where we commit to pending changes we've added; returns true if * anything changed for the remote side (if not, don't send!) */ -bool channel_sending_commit(struct channel *channel); +bool channel_sending_commit(struct channel *channel, + const struct htlc ***htlcs); /** * channel_rcvd_revoke_and_ack: accept ack on remote committed changes. * @channel: the channel - * @oursfail: callback for any unfilfilled htlcs which are now fully removed. - * @theirslocked: callback for any new htlcs which are now fully committed. - * @cbarg: argument to pass through to @ourhtlcfail & @theirhtlclocked + * @htlcs: initially-empty tal_arr() for htlcs which changed state. * * This is where we commit to pending changes we've added; returns true if * 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), \ - typesafe_cb_preargs(void, void *, \ - (oursfail), \ - (cbarg), \ - const struct htlc *), \ - typesafe_cb_preargs(void, void *, \ - (theirslocked), \ - (cbarg), \ - const struct htlc *), \ - (cbarg)) - -bool channel_rcvd_revoke_and_ack_(struct channel *channel, - void (*oursfail)(const struct htlc *htlc, - void *cbarg), - void (*theirslocked)(const struct htlc *htlc, - void *cbarg), - void *cbarg); +bool channel_rcvd_revoke_and_ack(struct channel *channel, + const struct htlc ***htlcs); /** * channel_rcvd_commit: commit all local outstanding changes. * @channel: the channel - * @theirsfulfilled: they are irrevocably committed to removal of htlc. - * @cbarg: argument to pass through to @theirsfulfilled + * @htlcs: initially-empty tal_arr() for htlcs which changed state. * * This is where we commit to pending changes we've added; returns true if * 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), \ - typesafe_cb_preargs(void, void *, \ - (theirsfulfilled), \ - (cbarg), \ - const struct htlc *), \ - (cbarg)) - -bool channel_rcvd_commit_(struct channel *channel, - void (*theirsfulfilled)(const struct htlc *htlc, - void *cbarg), - void *cbarg); +bool channel_rcvd_commit(struct channel *channel, + const struct htlc ***htlcs); /** * channel_sending_revoke_and_ack: sending ack on local committed changes. diff --git a/lightningd/channel/channel.c b/lightningd/channel/channel.c index 7c555d1a2..5cc81a20b 100644 --- a/lightningd/channel/channel.c +++ b/lightningd/channel/channel.c @@ -370,7 +370,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_sending_commit(peer->channel)) { + if (!channel_sending_commit(peer->channel, NULL)) { status_trace("Can't send commit: nothing to send"); tal_free(tmpctx); return; @@ -449,11 +449,6 @@ static void start_commit_timer(struct peer *peer) send_commit, peer); } -static void theirs_fulfilled(const struct htlc *htlc, struct peer *peer) -{ - /* FIXME: Tell master, so it can disarm timer. */ -} - static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) { tal_t *tmpctx = tal_tmpctx(peer); @@ -462,11 +457,12 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) secp256k1_ecdsa_signature commit_sig, *htlc_sigs; struct pubkey remotekey; struct bitcoin_tx **txs; - const struct htlc **htlc_map; + const struct htlc **htlc_map, **changed_htlcs; const u8 **wscripts; size_t i; - if (!channel_rcvd_commit(peer->channel, theirs_fulfilled, peer)) { + changed_htlcs = tal_arr(msg, const struct htlc *, 0); + if (!channel_rcvd_commit(peer->channel, &changed_htlcs)) { /* BOLT #2: * * A node MUST NOT send a `commitment_signed` message which @@ -479,6 +475,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) "commit_sig with no changes"); } + /* FIXME: Tell master about HTLC changes. */ + if (!fromwire_commitment_signed(tmpctx, msg, NULL, &channel_id, &commit_sig, &htlc_sigs)) peer_failed(io_conn_fd(peer->peer_conn), @@ -588,11 +586,6 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) tal_free(tmpctx); } -static void our_htlc_failed(const struct htlc *htlc, struct peer *peer) -{ - status_trace("FIXME: our htlc %"PRIu64" failed", htlc->id); -} - static void their_htlc_locked(const struct htlc *htlc, struct peer *peer) { tal_t *tmpctx = tal_tmpctx(peer); @@ -689,6 +682,7 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) struct privkey privkey; struct channel_id channel_id; struct pubkey per_commit_point, next_per_commit; + const struct htlc **changed_htlcs = tal_arr(msg, const struct htlc *, 0); if (!fromwire_revoke_and_ack(msg, NULL, &channel_id, &old_commit_secret, &next_per_commit)) { @@ -747,13 +741,18 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) /* We start timer even if this returns false: we might have delayed * commit because we were waiting for this! */ - if (channel_rcvd_revoke_and_ack(peer->channel, - our_htlc_failed, their_htlc_locked, - peer)) + if (channel_rcvd_revoke_and_ack(peer->channel, &changed_htlcs)) status_trace("Commits outstanding after recv revoke_and_ack"); else status_trace("No commits outstanding after recv revoke_and_ack"); + /* Tell master about locked-in htlcs. */ + for (size_t i = 0; i < tal_count(changed_htlcs); i++) { + if (changed_htlcs[i]->state == RCVD_ADD_ACK_REVOCATION) { + their_htlc_locked(changed_htlcs[i], peer); + } + } + start_commit_timer(peer); } diff --git a/lightningd/test/run-channel.c b/lightningd/test/run-channel.c index e7e2f83e6..33b44e740 100644 --- a/lightningd/test/run-channel.c +++ b/lightningd/test/run-channel.c @@ -82,10 +82,6 @@ static u64 feerates[] = { 9651936 }; -static void do_nothing(const struct htlc *htlc, void *unused) -{ -} - /* BOLT #3: * * htlc 0 direction: remote->local @@ -113,6 +109,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); + const struct htlc **changed_htlcs; u8 *dummy_routing = tal_arr(htlcs, u8, TOTAL_PACKET_SIZE); bool ret; @@ -155,17 +152,18 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side tal_free(dummy_routing); /* Now make HTLCs fully committed. */ - ret = channel_sending_commit(channel); + changed_htlcs = tal_arr(htlcs, const struct htlc *, 0); + ret = channel_sending_commit(channel, &changed_htlcs); assert(ret); - ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + ret = channel_rcvd_revoke_and_ack(channel, &changed_htlcs); assert(!ret); - ret = channel_rcvd_commit(channel, NULL, NULL); + ret = channel_rcvd_commit(channel, &changed_htlcs); assert(ret); ret = channel_sending_revoke_and_ack(channel); assert(ret); - ret = channel_sending_commit(channel); + ret = channel_sending_commit(channel, &changed_htlcs); assert(ret); - ret = channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, NULL); + ret = channel_rcvd_revoke_and_ack(channel, &changed_htlcs); assert(!ret); return htlcs; } @@ -239,6 +237,7 @@ static void send_and_fulfill_htlc(struct channel *channel, struct sha256 rhash; u8 *dummy_routing = tal_arr(channel, u8, TOTAL_PACKET_SIZE); bool ret; + const struct htlc **changed_htlcs; memset(&r, 0, sizeof(r)); sha256(&rhash, &r, sizeof(r)); @@ -246,45 +245,46 @@ static void send_and_fulfill_htlc(struct channel *channel, assert(channel_add_htlc(channel, sender, 1337, msatoshi, 900, &rhash, dummy_routing) == CHANNEL_ERR_ADD_OK); + changed_htlcs = tal_arr(channel, const struct htlc *, 0); + if (sender == LOCAL) { /* Step through a complete cycle. */ - ret = channel_sending_commit(channel); + ret = channel_sending_commit(channel, &changed_htlcs); assert(ret); - ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + ret = channel_rcvd_revoke_and_ack(channel, &changed_htlcs); assert(!ret); - ret = channel_rcvd_commit(channel, NULL, NULL); + ret = channel_rcvd_commit(channel, &changed_htlcs); assert(ret); ret = channel_sending_revoke_and_ack(channel); assert(!ret); assert(channel_fulfill_htlc(channel, LOCAL, 1337, &r) == CHANNEL_ERR_REMOVE_OK); - ret = channel_rcvd_commit(channel, NULL, NULL); + ret = channel_rcvd_commit(channel, &changed_htlcs); assert(ret); ret = channel_sending_revoke_and_ack(channel); assert(ret); - ret = channel_sending_commit(channel); + ret = channel_sending_commit(channel, &changed_htlcs); assert(ret); - ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + ret = channel_rcvd_revoke_and_ack(channel, &changed_htlcs); assert(!ret); assert(channel_get_htlc(channel, sender, 1337)->state == RCVD_REMOVE_ACK_REVOCATION); } else { - ret = channel_rcvd_commit(channel, NULL, NULL); + ret = channel_rcvd_commit(channel, &changed_htlcs); assert(ret); ret = channel_sending_revoke_and_ack(channel); assert(ret); - ret = channel_sending_commit(channel); + ret = channel_sending_commit(channel, &changed_htlcs); assert(ret); - ret = channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, - NULL); + ret = channel_rcvd_revoke_and_ack(channel, &changed_htlcs); assert(!ret); assert(channel_fulfill_htlc(channel, REMOTE, 1337, &r) == CHANNEL_ERR_REMOVE_OK); - ret = channel_sending_commit(channel); + ret = channel_sending_commit(channel, &changed_htlcs); assert(ret); - ret = channel_rcvd_revoke_and_ack(channel, NULL, NULL, NULL); + ret = channel_rcvd_revoke_and_ack(channel, &changed_htlcs); assert(!ret); - ret = channel_rcvd_commit(channel, do_nothing, NULL); + ret = channel_rcvd_commit(channel, &changed_htlcs); assert(ret); ret = channel_sending_revoke_and_ack(channel); assert(!ret);