lightningd/channel.c: make callbacks clearly generic

Passing through 'struct peer *' was a layering violation.

Reported-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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)

lightningd/channel.h

@@ -5,12 +5,12 @@
 #include <bitcoin/shadouble.h>
 #include <ccan/short_types/short_types.h>
 #include <ccan/tal/tal.h>
+#include <ccan/typesafe_cb/typesafe_cb.h>
 #include <daemon/htlc.h>
 #include <lightningd/channel_config.h>
 #include <lightningd/derive_basepoints.h>
 #include <stdbool.h>
 
-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.

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

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);