short_channel_id_dir: new primitive for one direction of short_channe…

…l_id

Currently only used by gossipd for channel elimination.

Also print them in canonical form (/[01]), so tests need to be
changed.

Suggested-by: @cdecker
Signed-off-by: Rusty Russell <[email protected]>

bitcoin/short_channel_id.c

@@ -61,3 +61,29 @@ char *short_channel_id_to_str(const tal_t *ctx, const struct short_channel_id *s
 		       short_channel_id_txnum(scid),
 		       short_channel_id_outnum(scid));
 }
+
+bool short_channel_id_dir_from_str(const char *str, size_t strlen,
+				   struct short_channel_id_dir *scidd)
+{
+	const char *slash = memchr(str, '/', strlen);
+	if (!slash || slash + 2 != str + strlen)
+		return false;
+	if (!short_channel_id_from_str(str, slash - str, &scidd->scid))
+		return false;
+	if (slash[1] == '0')
+		scidd->dir = 0;
+	else if (slash[1] == '1')
+		scidd->dir = 1;
+	else
+		return false;
+	return true;
+}
+
+char *short_channel_id_dir_to_str(const tal_t *ctx,
+				  const struct short_channel_id_dir *scidd)
+{
+	char *str, *scidstr = short_channel_id_to_str(NULL, &scidd->scid);
+	str = tal_fmt(ctx, "%s/%u", scidstr, scidd->dir);
+	tal_free(scidstr);
+	return str;
+}

bitcoin/short_channel_id.h

@@ -15,6 +15,23 @@ struct short_channel_id {
 /* Define short_channel_id_eq (no padding) */
 STRUCTEQ_DEF(short_channel_id, 0, u64);
 
+/* BOLT #7:
+ *
+ * - MUST set `node_id_1` and `node_id_2` to the public keys of the two nodes
+ * operating the channel, such that `node_id_1` is the numerically-lesser of the
+ * two DER-encoded keys sorted in ascending numerical order.
+ *...
+ *   - if the origin node is `node_id_1` in the message:
+ *     - MUST set the `direction` bit of `channel_flags` to 0.
+ *   - otherwise:
+ *     - MUST set the `direction` bit of `channel_flags` to 1.
+ */
+struct short_channel_id_dir {
+	struct short_channel_id scid;
+	/* 0 == from lesser id node, 1 == to lesser id node */
+	int dir;
+};
+
 static inline u32 short_channel_id_blocknum(const struct short_channel_id *scid)
 {
 	return scid->u64 >> 40;
@@ -38,4 +55,10 @@ bool short_channel_id_from_str(const char *str, size_t strlen,
 
 char *short_channel_id_to_str(const tal_t *ctx, const struct short_channel_id *scid);
 
+bool short_channel_id_dir_from_str(const char *str, size_t strlen,
+				   struct short_channel_id_dir *scidd);
+
+char *short_channel_id_dir_to_str(const tal_t *ctx,
+				  const struct short_channel_id_dir *scidd);
+
 #endif /* LIGHTNING_BITCOIN_SHORT_CHANNEL_ID_H */

common/type_to_string.h

@@ -24,6 +24,7 @@ union printable_types {
 	const secp256k1_pubkey *secp256k1_pubkey;
 	const struct channel_id *channel_id;
 	const struct short_channel_id *short_channel_id;
+	const struct short_channel_id_dir *short_channel_id_dir;
 	const struct secret *secret;
 	const struct privkey *privkey;
 	const secp256k1_ecdsa_signature *secp256k1_ecdsa_signature;

gossipd/gossip_wire.csv

@@ -33,8 +33,7 @@ gossip_getroute_request,,riskfactor,u16
 gossip_getroute_request,,final_cltv,u32
 gossip_getroute_request,,fuzz,double
 gossip_getroute_request,,num_excluded,u16
-gossip_getroute_request,,excluded,num_excluded*struct short_channel_id
-gossip_getroute_request,,excluded_dir,num_excluded*bool
+gossip_getroute_request,,excluded,num_excluded*struct short_channel_id_dir
 gossip_getroute_request,,max_hops,u32
 
 gossip_getroute_reply,3106

gossipd/gossipd.c

@@ -1871,8 +1871,7 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon,
 	u8 *out;
 	struct route_hop *hops;
 	double fuzz;
-	struct short_channel_id *excluded;
-	bool *excluded_dir;
+	struct short_channel_id_dir *excluded;
 
 	/* To choose between variations, we need to know how much we're
 	 * sending (eliminates too-small channels, and also effects the fees
@@ -1884,7 +1883,7 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon,
 					      &source, &destination,
 					      &msatoshi, &riskfactor,
 					      &final_cltv, &fuzz,
-					      &excluded, &excluded_dir,
+					      &excluded,
 					      &max_hops))
 		master_badmsg(WIRE_GOSSIP_GETROUTE_REQUEST, msg);
 
@@ -1895,7 +1894,7 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon,
 	/* routing.c does all the hard work; can return NULL. */
 	hops = get_route(tmpctx, daemon->rstate, &source, &destination,
 			 msatoshi, riskfactor, final_cltv,
-			 fuzz, siphash_seed(), excluded, excluded_dir, max_hops);
+			 fuzz, siphash_seed(), excluded, max_hops);
 
 	out = towire_gossip_getroute_reply(NULL, hops);
 	daemon_conn_send(daemon->master, take(out));

gossipd/routing.c

@@ -1003,7 +1003,7 @@ static void update_pending(struct pending_cannouncement *pending,
 			   u32 timestamp, const u8 *update,
 			   const u8 direction)
 {
-	SUPERVERBOSE("Deferring update for pending channel %s(%d)",
+	SUPERVERBOSE("Deferring update for pending channel %s/%d",
 		     type_to_string(tmpctx, struct short_channel_id,
 				    &pending->short_channel_id), direction);
 
@@ -1043,12 +1043,12 @@ static void set_connection_values(struct chan *chan,
 	/* If it was temporarily unroutable, re-enable */
 	c->unroutable_until = 0;
 
-	SUPERVERBOSE("Channel %s(%d) was updated.",
+	SUPERVERBOSE("Channel %s/%d was updated.",
 		     type_to_string(tmpctx, struct short_channel_id, &chan->scid),
 		     idx);
 
 	if (c->proportional_fee >= MAX_PROPORTIONAL_FEE) {
-		status_trace("Channel %s(%d) massive proportional fee %u:"
+		status_trace("Channel %s/%d massive proportional fee %u:"
 			     " disabling.",
 			     type_to_string(tmpctx, struct short_channel_id,
 					    &chan->scid),
@@ -1272,7 +1272,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES,
 		return err;
 	}
 
-	status_trace("Received channel_update for channel %s(%d) now %s was %s (from %s)",
+	status_trace("Received channel_update for channel %s/%d now %s was %s (from %s)",
 		     type_to_string(tmpctx, struct short_channel_id,
 				    &short_channel_id),
 		     channel_flags & 0x01,
@@ -1506,8 +1506,7 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate,
 			    const u64 msatoshi, double riskfactor,
 			    u32 final_cltv,
 			    double fuzz, const struct siphash_seed *base_seed,
-			    const struct short_channel_id *excluded,
-			    const bool *excluded_dir,
+			    const struct short_channel_id_dir *excluded,
 			    size_t max_hops)
 {
 	struct chan **route;
@@ -1518,17 +1517,16 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate,
 	struct node *n;
 	u64 *saved_capacity;
 
-	assert(tal_count(excluded) == tal_count(excluded_dir));
 	saved_capacity = tal_arr(tmpctx, u64, tal_count(excluded));
 
 	/* Temporarily set excluded channels' capacity to zero. */
 	for (size_t i = 0; i < tal_count(excluded); i++) {
-		struct chan *chan = get_channel(rstate, &excluded[i]);
+		struct chan *chan = get_channel(rstate, &excluded[i].scid);
 		if (!chan)
 			continue;
 		saved_capacity[i]
-			= chan->half[excluded_dir[i]].htlc_maximum_msat;
-		chan->half[excluded_dir[i]].htlc_maximum_msat = 0;
+			= chan->half[excluded[i].dir].htlc_maximum_msat;
+		chan->half[excluded[i].dir].htlc_maximum_msat = 0;
 	}
 
 	route = find_route(ctx, rstate, source, destination, msatoshi,
@@ -1537,10 +1535,10 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate,
 
 	/* Now restore the capacity. */
 	for (size_t i = 0; i < tal_count(excluded); i++) {
-		struct chan *chan = get_channel(rstate, &excluded[i]);
+		struct chan *chan = get_channel(rstate, &excluded[i].scid);
 		if (!chan)
 			continue;
-		chan->half[excluded_dir[i]].htlc_maximum_msat
+		chan->half[excluded[i].dir].htlc_maximum_msat
 			= saved_capacity[i];
 	}
 

gossipd/routing.h

@@ -268,8 +268,7 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate,
 			    u32 final_cltv,
 			    double fuzz,
 			    const struct siphash_seed *base_seed,
-			    const struct short_channel_id *excluded,
-			    const bool *excluded_dir,
+			    const struct short_channel_id_dir *excluded,
 			    size_t max_hops);
 /* Disable channel(s) based on the given routing failure. */
 void routing_failure(struct routing_state *rstate,

lightningd/gossip_control.c

@@ -292,27 +292,6 @@ static void json_getroute_reply(struct subd *gossip UNUSED, const u8 *reply, con
 	was_pending(command_success(cmd, response));
 }
 
-static bool json_to_short_channel_id_with_dir(const char *buffer,
-					      const jsmntok_t *tok,
-					      struct short_channel_id *scid,
-					      bool *dir)
-{
-	/* Ends in /0 or /1 */
-	if (tok->end - tok->start < 2)
-		return false;
-	if (buffer[tok->end - 2] != '/')
-		return false;
-	if (buffer[tok->end - 1] == '0')
-		*dir = false;
-	else if (buffer[tok->end - 1] == '1')
-		*dir = true;
-	else
-		return false;
-
-	return short_channel_id_from_str(buffer + tok->start,
-					 tok->end - tok->start - 2, scid);
-}
-
 static struct command_result *json_getroute(struct command *cmd,
 					    const char *buffer,
 					    const jsmntok_t *obj UNNEEDED,
@@ -325,8 +304,7 @@ static struct command_result *json_getroute(struct command *cmd,
 	u64 *msatoshi;
 	unsigned *cltv;
 	double *riskfactor;
-	struct short_channel_id *excluded;
-	bool *excluded_dir;
+	struct short_channel_id_dir *excluded;
 	u32 *max_hops;
 
 	/* Higher fuzz means that some high-fee paths can be discounted
@@ -356,16 +334,15 @@ static struct command_result *json_getroute(struct command *cmd,
 		const jsmntok_t *t, *end = json_next(excludetok);
 		size_t i;
 
-		excluded = tal_arr(cmd, struct short_channel_id,
+		excluded = tal_arr(cmd, struct short_channel_id_dir,
 				   excludetok->size);
-		excluded_dir = tal_arr(cmd, bool, excludetok->size);
 
 		for (i = 0, t = excludetok + 1;
 		     t < end;
 		     t = json_next(t), i++) {
-			if (!json_to_short_channel_id_with_dir(buffer, t,
-							       &excluded[i],
-							       &excluded_dir[i])) {
+			if (!short_channel_id_dir_from_str(buffer + t->start,
+							   t->end - t->start,
+							   &excluded[i])) {
 				return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 						    "%.*s is not a valid"
 						    " short_channel_id/direction",
@@ -375,13 +352,12 @@ static struct command_result *json_getroute(struct command *cmd,
 		}
 	} else {
 		excluded = NULL;
-		excluded_dir = NULL;
 	}
 
 	u8 *req = towire_gossip_getroute_request(cmd, source, destination,
 						 *msatoshi, *riskfactor * 1000,
 						 *cltv, fuzz,
-						 excluded, excluded_dir,
+						 excluded,
 						 *max_hops);
 	subd_req(ld->gossip, ld->gossip, req, -1, 0, json_getroute_reply, cmd);
 	return command_still_pending(cmd);

lightningd/payalgo.c

@@ -557,7 +557,7 @@ static struct command_result *json_pay_try(struct pay *pay)
 					     pay->msatoshi + overpayment,
 					     pay->riskfactor,
 					     pay->min_final_cltv_expiry,
-					     &pay->fuzz, NULL, NULL,
+					     &pay->fuzz, NULL,
 					     ROUTING_MAX_HOPS);
 	subd_req(pay->try_parent, cmd->ld->gossip, req, -1, 0, json_pay_getroute_reply, pay);
 

tests/test_connection.py

@@ -1446,23 +1446,23 @@ def test_restart_many_payments(node_factory):
     l1_logs = []
     for i in range(len(innodes)):
         scid = inchans[i]
-        l1_logs += [r'update for channel {}\(0\) now ACTIVE'.format(scid),
-                    r'update for channel {}\(1\) now ACTIVE'.format(scid),
+        l1_logs += [r'update for channel {}/0 now ACTIVE'.format(scid),
+                    r'update for channel {}/1 now ACTIVE'.format(scid),
                     'to CHANNELD_NORMAL']
-        innodes[i].daemon.wait_for_logs([r'update for channel {}\(0\) now ACTIVE'
+        innodes[i].daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE'
                                          .format(scid),
-                                         r'update for channel {}\(1\) now ACTIVE'
+                                         r'update for channel {}/1 now ACTIVE'
                                          .format(scid),
                                          'to CHANNELD_NORMAL'])
 
     for i in range(len(outnodes)):
         scid = outchans[i]
-        l1_logs += [r'update for channel {}\(0\) now ACTIVE'.format(scid),
-                    r'update for channel {}\(1\) now ACTIVE'.format(scid),
+        l1_logs += [r'update for channel {}/0 now ACTIVE'.format(scid),
+                    r'update for channel {}/1 now ACTIVE'.format(scid),
                     'to CHANNELD_NORMAL']
-        outnodes[i].daemon.wait_for_logs([r'update for channel {}\(0\) now ACTIVE'
+        outnodes[i].daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE'
                                           .format(scid),
-                                          r'update for channel {}\(1\) now ACTIVE'
+                                          r'update for channel {}/1 now ACTIVE'
                                           .format(scid),
                                           'to CHANNELD_NORMAL'])
 

tests/test_gossip.py

@@ -978,9 +978,9 @@ def test_getroute_exclude(node_factory, bitcoind):
     bitcoind.generate_block(5)
 
     # We don't wait above, because we care about it hitting l1.
-    l1.daemon.wait_for_logs([r'update for channel {}\(0\) now ACTIVE'
+    l1.daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE'
                              .format(scid),
-                             r'update for channel {}\(1\) now ACTIVE'
+                             r'update for channel {}/1 now ACTIVE'
                              .format(scid)])
 
     # l3 id is > l2 id, so 1 means l3->l2

tests/test_pay.py

@@ -151,7 +151,7 @@ def test_pay_get_error_with_update(node_factory):
     l1.daemon.wait_for_log(r'Extracted channel_update 0102.*from onionreply 10070088[0-9a-fA-F]{88}')
 
     # And now monitor for l1 to apply the channel_update we just extracted
-    l1.daemon.wait_for_log(r'Received channel_update for channel {}\(.\) now DISABLED was ACTIVE \(from error\)'.format(chanid2))
+    l1.daemon.wait_for_log(r'Received channel_update for channel {}/. now DISABLED was ACTIVE \(from error\)'.format(chanid2))
 
 
 def test_pay_optional_args(node_factory):

tests/utils.py

@@ -536,14 +536,14 @@ def fund_channel(self, l2, amount, wait_for_active=True):
         if wait_for_active:
             # We wait until gossipd sees both local updates, as well as status NORMAL,
             # so it can definitely route through.
-            self.daemon.wait_for_logs([r'update for channel {}\(0\) now ACTIVE'
+            self.daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE'
                                        .format(scid),
-                                       r'update for channel {}\(1\) now ACTIVE'
+                                       r'update for channel {}/1 now ACTIVE'
                                        .format(scid),
                                        'to CHANNELD_NORMAL'])
-            l2.daemon.wait_for_logs([r'update for channel {}\(0\) now ACTIVE'
+            l2.daemon.wait_for_logs([r'update for channel {}/0 now ACTIVE'
                                      .format(scid),
-                                     r'update for channel {}\(1\) now ACTIVE'
+                                     r'update for channel {}/1 now ACTIVE'
                                      .format(scid),
                                      'to CHANNELD_NORMAL'])
         return scid
@@ -596,9 +596,9 @@ def wait_channel_active(self, chanid):
     # (or for local channels, at least a local announcement)
     def wait_for_routes(self, channel_ids):
         # Could happen in any order...
-        self.daemon.wait_for_logs(['Received channel_update for channel {}\\(0\\)'.format(c)
+        self.daemon.wait_for_logs(['Received channel_update for channel {}/0'.format(c)
                                    for c in channel_ids]
-                                  + ['Received channel_update for channel {}\\(1\\)'.format(c)
+                                  + ['Received channel_update for channel {}/1'.format(c)
                                      for c in channel_ids])
 
     def pay(self, dst, amt, label=None):
@@ -829,7 +829,7 @@ def line_graph(self, num_nodes, fundchannel=True, fundamount=10**6, wait_for_ann
         for src, dst in connections:
             wait_for(lambda: src.channel_state(dst) == 'CHANNELD_NORMAL')
             scid = src.get_channel_scid(dst)
-            src.daemon.wait_for_log(r'Received channel_update for channel {scid}\(.\) now ACTIVE'.format(scid=scid))
+            src.daemon.wait_for_log(r'Received channel_update for channel {scid}/. now ACTIVE'.format(scid=scid))
             scids.append(scid)
 
         if not wait_for_announce:

wire/fromwire.c

@@ -161,6 +161,13 @@ void fromwire_short_channel_id(const u8 **cursor, size_t *max,
 	short_channel_id->u64 = fromwire_u64(cursor, max);
 }
 
+void fromwire_short_channel_id_dir(const u8 **cursor, size_t *max,
+				   struct short_channel_id_dir *scidd)
+{
+	fromwire_short_channel_id(cursor, max, &scidd->scid);
+	scidd->dir = fromwire_bool(cursor, max);
+}
+
 void fromwire_sha256(const u8 **cursor, size_t *max, struct sha256 *sha256)
 {
 	fromwire(cursor, max, sha256, sizeof(*sha256));
@@ -235,6 +242,7 @@ char *fromwire_wirestring(const tal_t *ctx, const u8 **cursor, size_t *max)
 }
 
 REGISTER_TYPE_TO_STRING(short_channel_id, short_channel_id_to_str);
+REGISTER_TYPE_TO_STRING(short_channel_id_dir, short_channel_id_dir_to_str);
 REGISTER_TYPE_TO_HEXSTR(channel_id);
 
 /* BOLT #2: