From f83c04fdbeb6ce86c1f8a8ff7e098559b5631d9a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 1 Apr 2017 21:27:07 +1030 Subject: [PATCH] lightningd/channel.c: make callbacks clearly generic Passing through 'struct peer *' was a layering violation. Reported-by: Christian Decker Signed-off-by: Rusty Russell --- lightningd/channel.c | 57 ++++++++++++++++------------------- lightningd/channel.h | 46 ++++++++++++++++++++-------- lightningd/channel/channel.c | 2 +- lightningd/test/run-channel.c | 8 ++--- 4 files changed, 64 insertions(+), 49 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 461b28cca..709221a5d 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -620,21 +620,18 @@ static void htlc_incstate(struct channel *channel, } static void check_lockedin(const struct htlc *h, - struct peer *peer, - void (*oursfail)(struct peer *peer, - const struct htlc *htlc), - void (*theirslocked)(struct peer *peer, - const struct htlc *htlc), - void (*theirsfulfilled)(struct peer *peer, - const struct htlc *htlc)) + void (*oursfail)(const struct htlc *, void *), + void (*theirslocked)(const struct htlc *, void *), + void (*theirsfulfilled)(const struct htlc *, void *), + void *cbarg) { /* If it was fulfilled, we handled it immediately. */ if (h->state == RCVD_REMOVE_ACK_REVOCATION && !h->r) - oursfail(peer, h); + oursfail(h, cbarg); else if (h->state == RCVD_ADD_ACK_REVOCATION) - theirslocked(peer, h); + theirslocked(h, cbarg); else if (h->state == RCVD_REMOVE_ACK_COMMIT && h->r) - theirsfulfilled(peer, h); + theirsfulfilled(h, cbarg); } /* FIXME: Commit to storage when this happens. */ @@ -642,13 +639,10 @@ static bool change_htlcs(struct channel *channel, enum side sidechanged, const enum htlc_state *htlc_states, size_t n_hstates, - struct peer *peer, - void (*oursfail)(struct peer *peer, - const struct htlc *htlc), - void (*theirslocked)(struct peer *peer, - const struct htlc *htlc), - void (*theirsfulfilled)(struct peer *peer, - const struct htlc *htlc)) + void (*oursfail)(const struct htlc *, void *), + void (*theirslocked)(const struct htlc *, void *), + void (*theirsfulfilled)(const struct htlc *, void *), + void *cbarg) { struct htlc_map_iter it; struct htlc *h; @@ -661,10 +655,11 @@ static bool 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, peer, + check_lockedin(h, oursfail, theirslocked, - theirsfulfilled); + theirsfulfilled, + cbarg); changed = true; } } @@ -684,12 +679,12 @@ bool channel_sent_commit(struct channel *channel) NULL, NULL, NULL, NULL); } -bool channel_rcvd_revoke_and_ack(struct channel *channel, - struct peer *peer, - void (*oursfail)(struct peer *peer, - const struct htlc *htlc), - void (*theirslocked)(struct peer *peer, - const struct htlc *htlc)) +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) { const enum htlc_state states[] = { SENT_ADD_COMMIT, SENT_REMOVE_ACK_COMMIT, @@ -698,14 +693,14 @@ bool channel_rcvd_revoke_and_ack(struct channel *channel, status_trace("received revoke_and_ack"); return change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), - peer, oursfail, theirslocked, NULL); + oursfail, theirslocked, NULL, cbarg); } /* FIXME: We can actually merge these two... */ -bool channel_rcvd_commit(struct channel *channel, - struct peer *peer, - void (*theirsfulfilled)(struct peer *peer, - const struct htlc *htlc)) +bool channel_rcvd_commit_(struct channel *channel, + void (*theirsfulfilled)(const struct htlc *htlc, + void *cbarg), + void *cbarg) { const enum htlc_state states[] = { RCVD_ADD_REVOCATION, RCVD_REMOVE_HTLC, @@ -714,7 +709,7 @@ bool channel_rcvd_commit(struct channel *channel, status_trace("received commit"); return change_htlcs(channel, LOCAL, states, ARRAY_SIZE(states), - peer, NULL, NULL, theirsfulfilled); + NULL, NULL, theirsfulfilled, cbarg); } bool channel_sent_revoke_and_ack(struct channel *channel) diff --git a/lightningd/channel.h b/lightningd/channel.h index d02ab3f1d..31bdc7fda 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -5,12 +5,12 @@ #include #include #include +#include #include #include #include #include -struct peer; struct signature; /* View from each side */ @@ -331,34 +331,54 @@ bool channel_sent_commit(struct channel *channel); /** * channel_rcvd_revoke_and_ack: accept ack on remote committed changes. * @channel: the channel - * @peer: argument to pass through to @ourhtlcfail & @theirhtlclocked * @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 * * This is where we commit to pending changes we've added; returns true if * anything changed. */ -bool channel_rcvd_revoke_and_ack(struct channel *channel, - struct peer *peer, - void (*oursfail)(struct peer *peer, - const struct htlc *htlc), - void (*theirslocked)(struct peer *peer, - const struct htlc *htlc)); +#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); /** * channel_rcvd_commit: commit all local outstanding changes. * @channel: the channel - * @peer: argument to pass through to @theirsfulfilled * @theirsfulfilled: they are irrevocably committed to removal of htlc. + * @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. */ -bool channel_rcvd_commit(struct channel *channel, - struct peer *peer, - void (*theirsfulfilled)(struct peer *peer, - const struct htlc *htlc)); +#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); /** * channel_sent_revoke_and_ack: sent ack on local committed changes. diff --git a/lightningd/channel/channel.c b/lightningd/channel/channel.c index abc7210ec..6d0b147a5 100644 --- a/lightningd/channel/channel.c +++ b/lightningd/channel/channel.c @@ -370,7 +370,7 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) size_t i; /* FIXME: Handle theirsfulfilled! */ - if (!channel_rcvd_commit(peer->channel, peer, NULL)) { + if (!channel_rcvd_commit(peer->channel, NULL, peer)) { /* BOLT #2: * * A node MUST NOT send a `commitment_signed` message which diff --git a/lightningd/test/run-channel.c b/lightningd/test/run-channel.c index c64adc917..eeb3e4cac 100644 --- a/lightningd/test/run-channel.c +++ b/lightningd/test/run-channel.c @@ -81,7 +81,7 @@ static u64 feerates[] = { 9651936 }; -static void do_nothing(struct peer *peer, const struct htlc *htlc) +static void do_nothing(const struct htlc *htlc, void *unused) { } @@ -158,7 +158,7 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side channel_rcvd_commit(channel, NULL, NULL); channel_sent_revoke_and_ack(channel); channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, NULL, do_nothing); + channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, NULL); return htlcs; } @@ -255,12 +255,12 @@ static void send_and_fulfill_htlc(struct channel *channel, channel_rcvd_commit(channel, NULL, NULL); channel_sent_revoke_and_ack(channel); channel_sent_commit(channel); - channel_rcvd_revoke_and_ack(channel, NULL, NULL, do_nothing); + channel_rcvd_revoke_and_ack(channel, NULL, do_nothing, NULL); 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, NULL, do_nothing); + channel_rcvd_commit(channel, do_nothing, NULL); channel_sent_revoke_and_ack(channel); assert(channel_get_htlc(channel, sender, 1337)->state == SENT_REMOVE_ACK_REVOCATION);