Dont ignore first scid for sendpay, sendonion

contrib/pyln-testing/pyln/testing/utils.py

@@ -1102,11 +1102,15 @@ def pay(self, dst, amt, label=None):
         invoices = dst.rpc.listinvoices(label)['invoices']
         assert len(invoices) == 1 and invoices[0]['status'] == 'unpaid'
 
+        # Pick first normal channel.
+        scid = [c['short_channel_id'] for c in only_one(self.rpc.listpeers(dst_id)['peers'])['channels']
+                if c['state'] == 'CHANNELD_NORMAL'][0]
+
         routestep = {
             'amount_msat': amt,
             'id': dst_id,
             'delay': 5,
-            'channel': '1x1x1'  # note: can be bogus for 1-hop direct payments
+            'channel': scid
         }
 
         # sendpay is async now

doc/lightning-sendonion.7.md

@@ -42,6 +42,10 @@ to add an HTLC for 1002 millisatoshis and a delay of 21 blocks on top of the cur
 }
 ```
 
+If the first element of *route* does not have "channel" set, a
+suitable channel (if any) will be chosen, otherwise that specific
+short-channel-id is used.
+
 The *payment_hash* parameter specifies the 32 byte hex-encoded hash to use as
 a challenge to the HTLC that we are sending. It is specific to the onion and
 has to match the one the onion was created with.

lightningd/pay.c

@@ -1,4 +1,5 @@
 #include "config.h"
+#include <ccan/mem/mem.h>
 #include <ccan/tal/str/str.h>
 #include <common/bolt12_merkle.h>
 #include <common/configdir.h>
@@ -827,19 +828,26 @@ static struct command_result *check_offer_usage(struct command *cmd,
 }
 
 static struct channel *find_channel_for_htlc_add(struct lightningd *ld,
-						 const struct node_id *node)
+						 const struct node_id *node,
+						 const struct short_channel_id *scid)
 {
 	struct channel *channel;
 	struct peer *peer = peer_by_id(ld, node);
 	if (!peer)
 		return NULL;
 
-	list_for_each(&peer->channels, channel, list) {
-		if (channel_can_add_htlc(channel)) {
-			return channel;
+	channel = find_channel_by_scid(peer, scid);
+	if (channel && channel_can_add_htlc(channel))
+		return channel;
+
+	/* We used to ignore scid: now all-zero means "any" */
+	if (!channel && (deprecated_apis || memeqzero(scid, sizeof(*scid)))) {
+		list_for_each(&peer->channels, channel, list) {
+			if (channel_can_add_htlc(channel)) {
+				return channel;
+			}
 		}
 	}
-
 	return NULL;
 }
 
@@ -1032,7 +1040,8 @@ send_payment_core(struct lightningd *ld,
 	if (offer_err)
 		return offer_err;
 
-	channel = find_channel_for_htlc_add(ld, &first_hop->node_id);
+	channel = find_channel_for_htlc_add(ld, &first_hop->node_id,
+					    &first_hop->scid);
 	if (!channel) {
 		struct json_stream *data
 			= json_stream_fail(cmd, PAY_TRY_OTHER_ROUTE,

tests/test_closing.py

@@ -7,7 +7,8 @@
     account_balance, first_channel_id, closing_fee, TEST_NETWORK,
     scriptpubkey_addr, calc_lease_fee, EXPERIMENTAL_FEATURES,
     check_utxos_channel, anchor_expected, check_coin_moves,
-    check_balance_snaps, mine_funding_to_announce, check_inspect_channel
+    check_balance_snaps, mine_funding_to_announce, check_inspect_channel,
+    first_scid
 )
 
 import os
@@ -1887,7 +1888,7 @@ def test_onchaind_replay(node_factory, bitcoind):
         'amount_msat': 10**8 - 1,
         'id': l2.info['id'],
         'delay': 101,
-        'channel': '1x1x1'
+        'channel': first_scid(l1, l2)
     }
     l1.rpc.sendpay([routestep], rhash, payment_secret=inv['payment_secret'])
     l1.daemon.wait_for_log('sendrawtx exit 0')
@@ -1947,7 +1948,7 @@ def test_onchain_dust_out(node_factory, bitcoind, executor):
         'amount_msat': 1,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'
+        'channel': first_scid(l1, l2)
     }
 
     l1.rpc.sendpay([routestep], rhash, payment_secret=inv['payment_secret'])
@@ -2019,7 +2020,7 @@ def test_onchain_timeout(node_factory, bitcoind, executor):
         'amount_msat': 10**8 - 1,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'
+        'channel': first_scid(l1, l2)
     }
 
     l1.rpc.sendpay([routestep], rhash, payment_secret=inv['payment_secret'], groupid=1)
@@ -2505,7 +2506,7 @@ def test_onchain_feechange(node_factory, bitcoind, executor):
         'amount_msat': 10**8 - 1,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'
+        'channel': first_scid(l1, l2)
     }
 
     executor.submit(l1.rpc.sendpay, [routestep], rhash, payment_secret=inv['payment_secret'])
@@ -2589,7 +2590,7 @@ def test_onchain_all_dust(node_factory, bitcoind, executor):
         'amount_msat': 10**7 - 1,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'
+        'channel': first_scid(l1, l2)
     }
 
     executor.submit(l1.rpc.sendpay, [routestep], rhash, payment_secret=inv['payment_secret'])

tests/test_connection.py

@@ -9,7 +9,7 @@
     expected_channel_features,
     check_coin_moves, first_channel_id, account_balance, basic_fee,
     scriptpubkey_addr, default_ln_port,
-    EXPERIMENTAL_FEATURES, mine_funding_to_announce
+    EXPERIMENTAL_FEATURES, mine_funding_to_announce, first_scid
 )
 from pyln.testing.utils import SLOW_MACHINE, VALGRIND, EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT
 
@@ -749,7 +749,7 @@ def test_reconnect_sender_add1(node_factory):
     rhash = inv['payment_hash']
     assert only_one(l2.rpc.listinvoices('test_reconnect_sender_add1')['invoices'])['status'] == 'unpaid'
 
-    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': '1x1x1'}]
+    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}]
 
     for i in range(0, len(disconnects)):
         with pytest.raises(RpcError):
@@ -788,7 +788,7 @@ def test_reconnect_sender_add(node_factory):
     rhash = inv['payment_hash']
     assert only_one(l2.rpc.listinvoices('testpayment')['invoices'])['status'] == 'unpaid'
 
-    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': '1x1x1'}]
+    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}]
 
     # This will send commit, so will reconnect as required.
     l1.rpc.sendpay(route, rhash, payment_secret=inv['payment_secret'])
@@ -822,7 +822,7 @@ def test_reconnect_receiver_add(node_factory):
     rhash = inv['payment_hash']
     assert only_one(l2.rpc.listinvoices('testpayment2')['invoices'])['status'] == 'unpaid'
 
-    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': '1x1x1'}]
+    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}]
     l1.rpc.sendpay(route, rhash, payment_secret=inv['payment_secret'])
     for i in range(len(disconnects)):
         l1.daemon.wait_for_log('Already have funding locked in')
@@ -852,7 +852,7 @@ def test_reconnect_receiver_fulfill(node_factory):
     rhash = inv['payment_hash']
     assert only_one(l2.rpc.listinvoices('testpayment2')['invoices'])['status'] == 'unpaid'
 
-    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': '1x1x1'}]
+    route = [{'amount_msat': amt, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}]
     l1.rpc.sendpay(route, rhash, payment_secret=inv['payment_secret'])
     for i in range(len(disconnects)):
         l1.daemon.wait_for_log('Already have funding locked in')
@@ -3480,7 +3480,7 @@ def test_htlc_retransmit_order(node_factory, executor):
         'amount_msat': 1000,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'  # note: can be bogus for 1-hop direct payments
+        'channel': first_scid(l1, l2)
     }
     for inv in invoices:
         executor.submit(l1.rpc.sendpay, [routestep], inv['payment_hash'], payment_secret=inv['payment_secret'])
@@ -3600,7 +3600,7 @@ def test_upgrade_statickey_onchaind(node_factory, executor, bitcoind):
         'amount_msat': 1,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'  # note: can be bogus for 1-hop direct payments
+        'channel': first_scid(l1, l2)
     }
     l1.rpc.sendpay([routestep], '00' * 32, payment_secret='00' * 32)
     with pytest.raises(RpcError, match=r'WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS'):
@@ -3725,7 +3725,7 @@ def test_upgrade_statickey_fail(node_factory, executor, bitcoind):
                                                'hold-result': 'fail'}])
 
     # This HTLC will fail
-    l1.rpc.sendpay([{'amount_msat': 1000, 'id': l2.info['id'], 'delay': 5, 'channel': '1x1x1'}], '00' * 32, payment_secret='00' * 32)
+    l1.rpc.sendpay([{'amount_msat': 1000, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}], '00' * 32, payment_secret='00' * 32)
 
     # Each one should cause one disconnection, no upgrade.
     for d in l1_disconnects + l2_disconnects:
@@ -3801,7 +3801,7 @@ def test_htlc_failed_noclose(node_factory):
         'amount_msat': FUNDAMOUNT * 1000,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'  # note: can be bogus for 1-hop direct payments
+        'channel': first_scid(l1, l2)
     }
 
     # This fails at channeld

tests/test_pay.py

@@ -6,7 +6,7 @@
 from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT
 from utils import (
     DEVELOPER, wait_for, only_one, sync_blockheight, TIMEOUT,
-    EXPERIMENTAL_FEATURES, VALGRIND, mine_funding_to_announce
+    EXPERIMENTAL_FEATURES, VALGRIND, mine_funding_to_announce, first_scid
 )
 import copy
 import os
@@ -572,7 +572,7 @@ def invoice_unpaid(dst, label):
         'amount_msat': amt,
         'id': l2.info['id'],
         'delay': 5,
-        'channel': '1x1x1'
+        'channel': first_scid(l1, l2)
     }
 
     # Insufficient funds.
@@ -671,7 +671,7 @@ def check_balances():
     inv = l2.rpc.invoice(amt, 'testpayment3', 'desc')
     rhash = inv['payment_hash']
     assert only_one(l2.rpc.listinvoices('testpayment3')['invoices'])['status'] == 'unpaid'
-    routestep = {'amount_msat': amt * 2, 'id': l2.info['id'], 'delay': 5, 'channel': '1x1x1'}
+    routestep = {'amount_msat': amt * 2, 'id': l2.info['id'], 'delay': 5, 'channel': first_scid(l1, l2)}
     l1.rpc.sendpay([routestep], rhash, payment_secret=inv['payment_secret'])
     preimage3 = l1.rpc.waitsendpay(rhash)['payment_preimage']
     assert only_one(l2.rpc.listinvoices('testpayment3')['invoices'])['status'] == 'paid'

tests/utils.py

@@ -412,6 +412,10 @@ def first_channel_id(n1, n2):
     return only_one(only_one(n1.rpc.listpeers(n2.info['id'])['peers'])['channels'])['channel_id']
 
 
+def first_scid(n1, n2):
+    return only_one(only_one(n1.rpc.listpeers(n2.info['id'])['peers'])['channels'])['short_channel_id']
+
+
 def basic_fee(feerate):
     if EXPERIMENTAL_FEATURES or EXPERIMENTAL_DUAL_FUND:
         # option_anchor_outputs