Alias privacy, as per BOLT 2

doc/lightning-decode.7.md

@@ -156,7 +156,7 @@ If **type** is "bolt11 invoice", and **valid** is *true*:
     - hops in the route:
       - **pubkey** (pubkey): the public key of the node
       - **short_channel_id** (short_channel_id): a channel to the next peer
-      - **fee_base_msat** (u32): the base fee for payments
+      - **fee_base_msat** (msat): the base fee for payments
       - **fee_proportional_millionths** (u32): the parts-per-million fee for payments
       - **cltv_expiry_delta** (u32): the CLTV delta across this hop
   - **extra** (array of objects, optional): Any extra fields we didn't know how to parse:
@@ -201,4 +201,4 @@ RESOURCES
 
 Main web site: <https://github.com/ElementsProject/lightning>
 
-[comment]: # ( SHA256STAMP:a3963c3e0061b0d42a1f9e2f2a9012df780fce0264c6785f0311909b01f78af2)
+[comment]: # ( SHA256STAMP:3e522a9788bb79302e4c4386c3937b7dcd8359d1b894364ac3e884bd3f695850)

doc/lightning-invoice.7.md

@@ -54,7 +54,7 @@ If specified, *exposeprivatechannels* overrides the default route hint
 logic, which will use unpublished channels only if there are no
 published channels. If *true* unpublished channels are always considered
 as a route hint candidate; if *false*, never.  If it is a short channel id
-(e.g. *1x1x3*) or array of short channel ids, only those specific channels
+(e.g. *1x1x3*) or array of short channel ids (or a remote alias), only those specific channels
 will be considered candidates, even if they are public or dead-ends.
 
 The route hint is selected from the set of incoming channels of which:

doc/schemas/decode.schema.json

@@ -869,7 +869,7 @@
                     "description": "a channel to the next peer"
                   },
                   "fee_base_msat": {
-                    "type": "u32",
+                    "type": "msat",
                     "description": "the base fee for payments"
                   },
                   "fee_proportional_millionths": {

lightningd/channel.c

@@ -17,6 +17,7 @@
 #include <lightningd/peer_control.h>
 #include <lightningd/subd.h>
 #include <wallet/txfilter.h>
+#include <wire/peer_wire.h>
 #include <wire/wire_sync.h>
 
 void channel_set_owner(struct channel *channel, struct subd *owner)
@@ -601,20 +602,35 @@ struct channel_inflight *channel_inflight_find(struct channel *channel,
 }
 
 struct channel *any_channel_by_scid(struct lightningd *ld,
-				    const struct short_channel_id *scid)
+				    const struct short_channel_id *scid,
+				    bool privacy_leak_ok)
 {
 	struct peer *p;
 	struct channel *chan;
 	list_for_each(&ld->peers, p, list) {
 		list_for_each(&p->channels, chan, list) {
-			if (chan->scid
-			    && short_channel_id_eq(scid, chan->scid))
-				return chan;
-			/* We also want to find the channel by its local alias
-			 * when we forward. */
+			/* BOLT-channel-type #2:
+			 * - MUST always recognize the `alias` as a
+			 *   `short_channel_id` for incoming HTLCs to this
+			 *   channel.
+			 */
 			if (chan->alias[LOCAL] &&
 			    short_channel_id_eq(scid, chan->alias[LOCAL]))
 				return chan;
+			/* BOLT-channel-type #2:
+			 * - if `channel_type` has `option_scid_alias` set:
+			 *   - MUST NOT allow incoming HTLCs to this channel
+			 *     using the real `short_channel_id`
+			 */
+			/* FIXME: We don't keep type is db, so assume all
+			 * private channels which support aliases want this! */
+			if (!privacy_leak_ok
+			    && chan->alias[REMOTE]
+			    && !(chan->channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL))
+				continue;
+			if (chan->scid
+			    && short_channel_id_eq(scid, chan->scid))
+				return chan;
 		}
 	}
 	return NULL;

lightningd/channel.h

@@ -411,8 +411,11 @@ struct channel *peer_any_unsaved_channel(struct peer *peer, bool *others);
 
 struct channel *channel_by_dbid(struct lightningd *ld, const u64 dbid);
 
+/* Includes both real scids and aliases.  If !privacy_leak_ok, then private
+ * channels' real scids are not included. */
 struct channel *any_channel_by_scid(struct lightningd *ld,
-				    const struct short_channel_id *scid);
+				    const struct short_channel_id *scid,
+				    bool privacy_leak_ok);
 
 /* Get channel by channel_id */
 struct channel *channel_by_cid(struct lightningd *ld,

lightningd/gossip_control.c

@@ -123,7 +123,7 @@ static void handle_local_channel_update(struct lightningd *ld, const u8 *msg)
 
 	/* In theory this could vanish before gossipd gets around to telling
 	 * us. */
-	channel = any_channel_by_scid(ld, &scid);
+	channel = any_channel_by_scid(ld, &scid, true);
 	if (!channel) {
 		log_broken(ld->log, "Local update for bad scid %s",
 			   type_to_string(tmpctx, struct short_channel_id,

lightningd/peer_control.c

@@ -2027,7 +2027,7 @@ command_find_channel(struct command *cmd,
 				    tok->end - tok->start,
 				    buffer + tok->start);
 	} else if (json_to_short_channel_id(buffer, tok, &scid)) {
-		*channel = any_channel_by_scid(ld, &scid);
+		*channel = any_channel_by_scid(ld, &scid, true);
 		if (!*channel)
 			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 					    "Short channel ID not found: '%.*s'",

lightningd/peer_htlcs.c

@@ -663,7 +663,7 @@ static void forward_htlc(struct htlc_in *hin,
 
 	/* This is a shortcut for specifying next peer; doesn't mean
 	 * the actual channel! */
-	next = any_channel_by_scid(ld, scid);
+	next = any_channel_by_scid(ld, scid, false);
 	if (next) {
 		struct peer *peer = next->peer;
 		struct channel *channel;

lightningd/routehint.c

@@ -171,7 +171,10 @@ routehint_candidates(const tal_t *ctx,
 		/* Consider only hints they gave */
 		if (hints) {
 			log_debug(ld->log, "We have hints!");
-			if (!scid_in_arr(hints, &r->short_channel_id)) {
+			/* Allow specification by alias, too */
+			if (!scid_in_arr(hints, &r->short_channel_id)
+			    && (!candidate.c->alias[REMOTE]
+				|| !scid_in_arr(hints, candidate.c->alias[REMOTE]))) {
 				log_debug(ld->log, "scid %s not in hints",
 					  type_to_string(tmpctx,
 							 struct short_channel_id,
@@ -204,6 +207,22 @@ routehint_candidates(const tal_t *ctx,
 			continue;
 		}
 
+		/* BOLT-channel-type #2:
+		 *   - if `channel_type` has `option_scid_alias` set:
+		 *       - MUST NOT use the real `short_channel_id` in
+		 *         BOLT 11 `r` fields.
+		 */
+		/* FIXME: We don't remember the type explicitly, so
+		 * we just assume all private channels negotiated since
+		 * we had alias support want this. */
+
+		/* Note explicit flag test here: if we're told to expose all
+		 * private channels, then "is_public" is forced true */
+		if (!(candidate.c->channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL)
+		    && candidate.c->alias[REMOTE]) {
+			r->short_channel_id = *candidate.c->alias[REMOTE];
+		}
+
 		/* OK, finish it and append to one of the arrays. */
 		if (is_public) {
 			log_debug(ld->log, "%s: added to public",

lightningd/test/run-invoice-select-inchan.c

@@ -10,7 +10,8 @@
 /* AUTOGENERATED MOCKS START */
 /* Generated stub for any_channel_by_scid */
 struct channel *any_channel_by_scid(struct lightningd *ld UNNEEDED,
-				    const struct short_channel_id *scid UNNEEDED)
+				    const struct short_channel_id *scid UNNEEDED,
+				    bool privacy_leak_ok UNNEEDED)
 { fprintf(stderr, "any_channel_by_scid called!\n"); abort(); }
 /* Generated stub for bitcoind_getutxout_ */
 void bitcoind_getutxout_(struct bitcoind *bitcoind UNNEEDED,

tests/test_invoices.py

@@ -226,6 +226,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Make sure channel is totally public.
     wait_for(lambda: [c['public'] for c in l2.rpc.listchannels(scid_dummy)['channels']] == [True, True])
 
+    alias = only_one(only_one(l1.rpc.listpeers(l2.info['id'])['peers'])['channels'])['alias']['local']
     # Since there's only one route, it will reluctantly hint that even
     # though it's private
     inv = l2.rpc.invoice(amount_msat=123456, label="inv0", description="?")
@@ -237,7 +238,9 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Route array has single route with single element.
     r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
     assert r['pubkey'] == l1.info['id']
-    assert r['short_channel_id'] == l1.rpc.listchannels()['channels'][0]['short_channel_id']
+    # It uses our private alias!
+    assert r['short_channel_id'] != l1.rpc.listchannels()['channels'][0]['short_channel_id']
+    assert r['short_channel_id'] == alias
     assert r['fee_base_msat'] == 1
     assert r['fee_proportional_millionths'] == 10
     assert r['cltv_expiry_delta'] == 6
@@ -261,7 +264,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Route array has single route with single element.
     r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
     assert r['pubkey'] == l1.info['id']
-    assert r['short_channel_id'] == l1.rpc.listchannels()['channels'][0]['short_channel_id']
+    assert r['short_channel_id'] == alias
     assert r['fee_base_msat'] == 1
     assert r['fee_proportional_millionths'] == 10
     assert r['cltv_expiry_delta'] == 6
@@ -276,7 +279,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Route array has single route with single element.
     r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
     assert r['pubkey'] == l1.info['id']
-    assert r['short_channel_id'] == l1.rpc.listchannels()['channels'][0]['short_channel_id']
+    assert r['short_channel_id'] == alias
     assert r['fee_base_msat'] == 1
     assert r['fee_proportional_millionths'] == 10
     assert r['cltv_expiry_delta'] == 6
@@ -308,7 +311,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Route array has single route with single element.
     r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
     assert r['pubkey'] == l1.info['id']
-    assert r['short_channel_id'] == l1.rpc.listchannels()['channels'][0]['short_channel_id']
+    assert r['short_channel_id'] == alias
     assert r['fee_base_msat'] == 1
     assert r['fee_proportional_millionths'] == 10
     assert r['cltv_expiry_delta'] == 6
@@ -322,7 +325,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Route array has single route with single element.
     r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
     assert r['pubkey'] == l1.info['id']
-    assert r['short_channel_id'] == scid
+    assert r['short_channel_id'] == alias
     assert r['fee_base_msat'] == 1
     assert r['fee_proportional_millionths'] == 10
     assert r['cltv_expiry_delta'] == 6
@@ -345,7 +348,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Route array has single route with single element.
     r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
     assert r['pubkey'] == l1.info['id']
-    assert r['short_channel_id'] == scid
+    assert r['short_channel_id'] == alias
     assert r['fee_base_msat'] == 1
     assert r['fee_proportional_millionths'] == 10
     assert r['cltv_expiry_delta'] == 6
@@ -365,7 +368,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     # Route array has single route with single element.
     r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
     assert r['pubkey'] == l1.info['id']
-    assert r['short_channel_id'] == l1.rpc.listchannels()['channels'][0]['short_channel_id']
+    assert r['short_channel_id'] == alias
     assert r['fee_base_msat'] == 1
     assert r['fee_proportional_millionths'] == 10
     assert r['cltv_expiry_delta'] == 6

tests/test_opening.py

@@ -1476,3 +1476,67 @@ def test_buy_liquidity_ad_no_v2(node_factory, bitcoind):
         l1.rpc.fundchannel(l2.info['id'], amount, request_amt=amount,
                            feerate='{}perkw'.format(feerate),
                            compact_lease='029a002d000000004b2003e8')
+
+
+def test_scid_alias_private(node_factory, bitcoind):
+    """Test that we don't allow use of real scid for scid_alias-type channels"""
+    l1, l2, l3 = node_factory.line_graph(3, fundchannel=False, opts=[{}, {},
+                                                                     {'log-level': 'io'}])
+
+    l2.fundwallet(5000000)
+    l2.rpc.fundchannel(l3.info['id'], 'all', announce=False)
+
+    bitcoind.generate_block(1, wait_for_mempool=1)
+    wait_for(lambda: only_one(only_one(l2.rpc.listpeers(l3.info['id'])['peers'])['channels'])['state'] == 'CHANNELD_NORMAL')
+
+    chan = only_one(only_one(l2.rpc.listpeers(l3.info['id'])['peers'])['channels'])
+    assert chan['private'] is True
+    scid23 = chan['short_channel_id']
+    alias23 = chan['alias']['local']
+
+    # Create l1<->l2 channel, make sure l3 sees it so it will routehint via
+    # l2 (otherwise it sees it as a deadend!)
+    l1.fundwallet(5000000)
+    l1.rpc.fundchannel(l2.info['id'], 'all')
+    bitcoind.generate_block(6, wait_for_mempool=1)
+    wait_for(lambda: len(l3.rpc.listchannels(source=l1.info['id'])['channels']) == 1)
+
+    chan = only_one(only_one(l1.rpc.listpeers(l2.info['id'])['peers'])['channels'])
+    assert chan['private'] is False
+    scid12 = chan['short_channel_id']
+
+    # Make sure it sees both sides of private channel in gossmap!
+    wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 4)
+
+    # BOLT #2:
+    # - if `channel_type` has `option_scid_alias` set:
+    #    - MUST NOT use the real `short_channel_id` in BOLT 11 `r` fields.
+    inv = l3.rpc.invoice(10, 'test_scid_alias_private', 'desc')
+    assert only_one(only_one(l1.rpc.decode(inv['bolt11'])['routes']))['short_channel_id'] == alias23
+
+    # BOLT #2:
+    # - if `channel_type` has `option_scid_alias` set:
+    #   - MUST NOT allow incoming HTLCs to this channel using the real `short_channel_id`
+    route = [{'amount_msat': 11,
+              'id': l2.info['id'],
+              'delay': 12,
+              'channel': scid12},
+             {'amount_msat': 10,
+              'id': l3.info['id'],
+              'delay': 6,
+              'channel': scid23}]
+    l1.rpc.sendpay(route, inv['payment_hash'], payment_secret=inv['payment_secret'])
+    with pytest.raises(RpcError) as err:
+        l1.rpc.waitsendpay(inv['payment_hash'])
+
+    # PERM|10
+    WIRE_UNKNOWN_NEXT_PEER = 0x4000 | 10
+    assert err.value.error['data']['failcode'] == WIRE_UNKNOWN_NEXT_PEER
+    assert err.value.error['data']['erring_node'] == l2.info['id']
+    assert err.value.error['data']['erring_channel'] == scid23
+
+    # BOLT #2
+    # - MUST always recognize the `alias` as a `short_channel_id` for incoming HTLCs to this channel.
+    route[1]['channel'] = alias23
+    l1.rpc.sendpay(route, inv['payment_hash'], payment_secret=inv['payment_secret'])
+    l1.rpc.waitsendpay(inv['payment_hash'])

tests/test_pay.py

@@ -1857,7 +1857,7 @@ def test_pay_routeboost(node_factory, bitcoind):
     assert 'routehint_modifications' not in only_one(status['pay'])
     assert 'local_exclusions' not in only_one(status['pay'])
     attempts = only_one(status['pay'])['attempts']
-    scid34 = only_one(l3.rpc.listpeers(l4.info['id'])['peers'])['channels'][0]['short_channel_id']
+    scid34 = only_one(l3.rpc.listpeers(l4.info['id'])['peers'])['channels'][0]['alias']['local']
     assert(len(attempts) == 1)
     a = attempts[0]
     assert(a['strategy'] == "Initial attempt")
@@ -1866,7 +1866,7 @@ def test_pay_routeboost(node_factory, bitcoind):
 
     # With dev-route option we can test longer routehints.
     if DEVELOPER:
-        scid45 = only_one(l4.rpc.listpeers(l5.info['id'])['peers'])['channels'][0]['short_channel_id']
+        scid45 = only_one(l4.rpc.listpeers(l5.info['id'])['peers'])['channels'][0]['alias']['local']
         routel3l4l5 = [{'id': l3.info['id'],
                         'short_channel_id': scid34,
                         'fee_base_msat': 1000,
@@ -3606,7 +3606,7 @@ def test_keysend_routehint(node_factory):
     routehints = [
         [
             {
-                'scid': l3.rpc.listpeers()['peers'][0]['channels'][0]['short_channel_id'],
+                'scid': l3.rpc.listpeers()['peers'][0]['channels'][0]['alias']['remote'],
                 'id': l2.info['id'],
                 'feebase': '1msat',
                 'feeprop': 10,
@@ -5062,7 +5062,8 @@ def test_routehint_tous(node_factory, bitcoind):
 
     inv = l3.rpc.invoice(10, "test", "test")['bolt11']
     decoded = l3.rpc.decodepay(inv)
-    assert(only_one(only_one(decoded['routes']))['short_channel_id'] == scid23)
+    assert(only_one(only_one(decoded['routes']))['short_channel_id']
+           == only_one(only_one(l3.rpc.listpeers()['peers'])['channels'])['alias']['remote'])
 
     l3.stop()
     with pytest.raises(RpcError, match=r'Destination .* is not reachable directly and all routehints were unusable'):