From a9c4e702139e61838fb0486abffeb1bec2e0e1ec Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 8 Mar 2022 19:14:49 +0000
Subject: [PATCH 1/9] Give `ChannelManager`s `channel_udpate`s for pub chans in
 test

This makes tests slightly more realistic by delivering
`channel_update`s to `ChannelManager`s, ensuring we have
forwarding data stored locally for all channels, including public
ones.
---
 lightning/src/ln/functional_test_utils.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index a908cd41658..64cbff1158a 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -748,6 +748,11 @@ pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, '
 		node.net_graph_msg_handler.handle_channel_update(upd_2).unwrap();
 		node.net_graph_msg_handler.handle_node_announcement(&a_node_announcement).unwrap();
 		node.net_graph_msg_handler.handle_node_announcement(&b_node_announcement).unwrap();
+
+		// Note that channel_updates are also delivered to ChannelManagers to ensure we have
+		// forwarding info for local channels even if its not accepted in the network graph.
+		node.node.handle_channel_update(&nodes[a].node.get_our_node_id(), &upd_1);
+		node.node.handle_channel_update(&nodes[b].node.get_our_node_id(), &upd_2);
 	}
 }
 

From 3fe76e61a34d2b71ff4bc713e00edf9c60087184 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 15 Feb 2022 21:05:20 +0000
Subject: [PATCH 2/9] Add a new functional test utility to open an unannounced
 channel

---
 lightning/src/ln/functional_test_utils.rs | 55 +++++++++++++++++++++++
 lightning/src/ln/functional_tests.rs      | 36 +--------------
 2 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 64cbff1158a..7a3a269316e 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -693,6 +693,61 @@ pub fn create_announced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a
 	(chan_announcement.1, chan_announcement.2, chan_announcement.3, chan_announcement.4)
 }
 
+pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> (msgs::FundingLocked, Transaction) {
+	let mut no_announce_cfg = test_default_channel_config();
+	no_announce_cfg.channel_options.announced_channel = false;
+	nodes[a].node.create_channel(nodes[b].node.get_our_node_id(), channel_value, push_msat, 42, Some(no_announce_cfg)).unwrap();
+	let open_channel = get_event_msg!(nodes[a], MessageSendEvent::SendOpenChannel, nodes[b].node.get_our_node_id());
+	nodes[b].node.handle_open_channel(&nodes[a].node.get_our_node_id(), a_flags, &open_channel);
+	let accept_channel = get_event_msg!(nodes[b], MessageSendEvent::SendAcceptChannel, nodes[a].node.get_our_node_id());
+	nodes[a].node.handle_accept_channel(&nodes[b].node.get_our_node_id(), b_flags, &accept_channel);
+
+	let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[a], channel_value, 42);
+	nodes[a].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
+	nodes[b].node.handle_funding_created(&nodes[a].node.get_our_node_id(), &get_event_msg!(nodes[a], MessageSendEvent::SendFundingCreated, nodes[b].node.get_our_node_id()));
+	check_added_monitors!(nodes[b], 1);
+
+	let cs_funding_signed = get_event_msg!(nodes[b], MessageSendEvent::SendFundingSigned, nodes[a].node.get_our_node_id());
+	nodes[a].node.handle_funding_signed(&nodes[b].node.get_our_node_id(), &cs_funding_signed);
+	check_added_monitors!(nodes[a], 1);
+
+	let conf_height = core::cmp::max(nodes[a].best_block_info().1 + 1, nodes[b].best_block_info().1 + 1);
+	confirm_transaction_at(&nodes[a], &tx, conf_height);
+	connect_blocks(&nodes[a], CHAN_CONFIRM_DEPTH - 1);
+	confirm_transaction_at(&nodes[b], &tx, conf_height);
+	connect_blocks(&nodes[b], CHAN_CONFIRM_DEPTH - 1);
+	let as_funding_locked = get_event_msg!(nodes[a], MessageSendEvent::SendFundingLocked, nodes[b].node.get_our_node_id());
+	nodes[a].node.handle_funding_locked(&nodes[b].node.get_our_node_id(), &get_event_msg!(nodes[b], MessageSendEvent::SendFundingLocked, nodes[a].node.get_our_node_id()));
+	let as_update = get_event_msg!(nodes[a], MessageSendEvent::SendChannelUpdate, nodes[b].node.get_our_node_id());
+	nodes[b].node.handle_funding_locked(&nodes[a].node.get_our_node_id(), &as_funding_locked);
+	let bs_update = get_event_msg!(nodes[b], MessageSendEvent::SendChannelUpdate, nodes[a].node.get_our_node_id());
+
+	nodes[a].node.handle_channel_update(&nodes[b].node.get_our_node_id(), &bs_update);
+	nodes[b].node.handle_channel_update(&nodes[a].node.get_our_node_id(), &as_update);
+
+	let mut found_a = false;
+	for chan in nodes[a].node.list_usable_channels() {
+		if chan.channel_id == as_funding_locked.channel_id {
+			assert!(!found_a);
+			found_a = true;
+			assert!(!chan.is_public);
+		}
+	}
+	assert!(found_a);
+
+	let mut found_b = false;
+	for chan in nodes[b].node.list_usable_channels() {
+		if chan.channel_id == as_funding_locked.channel_id {
+			assert!(!found_b);
+			found_b = true;
+			assert!(!chan.is_public);
+		}
+	}
+	assert!(found_b);
+
+	(as_funding_locked, tx)
+}
+
 pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, ann: &msgs::ChannelAnnouncement, upd_1: &msgs::ChannelUpdate, upd_2: &msgs::ChannelUpdate) {
 	nodes[a].node.broadcast_node_announcement([0, 0, 0], [0; 32], Vec::new());
 	let a_events = nodes[a].node.get_and_clear_pending_msg_events();
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 00db1f5ca55..95a515c95f7 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -7577,7 +7577,6 @@ fn test_priv_forwarding_rejection() {
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
 	let mut no_announce_cfg = test_default_channel_config();
-	no_announce_cfg.channel_options.announced_channel = false;
 	no_announce_cfg.accept_forwards_to_priv_channels = false;
 	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(no_announce_cfg), None]);
 	let persister: test_utils::TestPersister;
@@ -7586,38 +7585,7 @@ fn test_priv_forwarding_rejection() {
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 
 	let chan_id_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).2;
-
-	// Note that the create_*_chan functions in utils requires announcement_signatures, which we do
-	// not send for private channels.
-	nodes[1].node.create_channel(nodes[2].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap();
-	let open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[2].node.get_our_node_id());
-	nodes[2].node.handle_open_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel);
-	let accept_channel = get_event_msg!(nodes[2], MessageSendEvent::SendAcceptChannel, nodes[1].node.get_our_node_id());
-	nodes[1].node.handle_accept_channel(&nodes[2].node.get_our_node_id(), InitFeatures::known(), &accept_channel);
-
-	let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[1], 1_000_000, 42);
-	nodes[1].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
-	nodes[2].node.handle_funding_created(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingCreated, nodes[2].node.get_our_node_id()));
-	check_added_monitors!(nodes[2], 1);
-
-	let cs_funding_signed = get_event_msg!(nodes[2], MessageSendEvent::SendFundingSigned, nodes[1].node.get_our_node_id());
-	nodes[1].node.handle_funding_signed(&nodes[2].node.get_our_node_id(), &cs_funding_signed);
-	check_added_monitors!(nodes[1], 1);
-
-	let conf_height = core::cmp::max(nodes[1].best_block_info().1 + 1, nodes[2].best_block_info().1 + 1);
-	confirm_transaction_at(&nodes[1], &tx, conf_height);
-	connect_blocks(&nodes[1], CHAN_CONFIRM_DEPTH - 1);
-	confirm_transaction_at(&nodes[2], &tx, conf_height);
-	connect_blocks(&nodes[2], CHAN_CONFIRM_DEPTH - 1);
-	let as_funding_locked = get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[2].node.get_our_node_id());
-	nodes[1].node.handle_funding_locked(&nodes[2].node.get_our_node_id(), &get_event_msg!(nodes[2], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id()));
-	get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[2].node.get_our_node_id());
-	nodes[2].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &as_funding_locked);
-	get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
-
-	assert!(nodes[0].node.list_usable_channels()[0].is_public);
-	assert_eq!(nodes[1].node.list_usable_channels().len(), 2);
-	assert!(!nodes[2].node.list_usable_channels()[0].is_public);
+	let chan_id_2 = create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).0.channel_id;
 
 	// We should always be able to forward through nodes[1] as long as its out through a public
 	// channel:
@@ -7662,7 +7630,7 @@ fn test_priv_forwarding_rejection() {
 	let mut monitor_a_serialized = test_utils::TestVecWriter(Vec::new());
 	let mut monitor_b_serialized = test_utils::TestVecWriter(Vec::new());
 	get_monitor!(nodes[1], chan_id_1).write(&mut monitor_a_serialized).unwrap();
-	get_monitor!(nodes[1], cs_funding_signed.channel_id).write(&mut monitor_b_serialized).unwrap();
+	get_monitor!(nodes[1], chan_id_2).write(&mut monitor_b_serialized).unwrap();
 
 	persister = test_utils::TestPersister::new();
 	let keys_manager = &chanmon_cfgs[1].keys_manager;

From 09f8abad9377b1fa66fff523b5612f357b38c1ff Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Wed, 16 Feb 2022 00:19:50 +0000
Subject: [PATCH 3/9] Move private channel and short-conf tests to new module

---
 lightning/src/ln/functional_tests.rs      | 209 +------------------
 lightning/src/ln/mod.rs                   |   3 +
 lightning/src/ln/priv_short_conf_tests.rs | 239 ++++++++++++++++++++++
 3 files changed, 243 insertions(+), 208 deletions(-)
 create mode 100644 lightning/src/ln/priv_short_conf_tests.rs

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 95a515c95f7..ed220a51887 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -23,8 +23,7 @@ use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAAC
 use ln::channel::{Channel, ChannelError};
 use ln::{chan_utils, onion_utils};
 use ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment};
-use routing::network_graph::RoutingFees;
-use routing::router::{PaymentParameters, Route, RouteHop, RouteHint, RouteHintHop, RouteParameters, find_route, get_route};
+use routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, find_route, get_route};
 use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
@@ -463,88 +462,6 @@ fn test_multi_flight_update_fee() {
 	check_added_monitors!(nodes[1], 1);
 }
 
-fn do_test_1_conf_open(connect_style: ConnectStyle) {
-	// Previously, if the minium_depth config was set to 1, we'd never send a funding_locked. This
-	// tests that we properly send one in that case.
-	let mut alice_config = UserConfig::default();
-	alice_config.own_channel_config.minimum_depth = 1;
-	alice_config.channel_options.announced_channel = true;
-	alice_config.peer_channel_config_limits.force_announced_channel_preference = false;
-	let mut bob_config = UserConfig::default();
-	bob_config.own_channel_config.minimum_depth = 1;
-	bob_config.channel_options.announced_channel = true;
-	bob_config.peer_channel_config_limits.force_announced_channel_preference = false;
-	let chanmon_cfgs = create_chanmon_cfgs(2);
-	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(alice_config), Some(bob_config)]);
-	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-	*nodes[0].connect_style.borrow_mut() = connect_style;
-
-	let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
-	mine_transaction(&nodes[1], &tx);
-	nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id()));
-	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
-
-	mine_transaction(&nodes[0], &tx);
-	let as_msg_events = nodes[0].node.get_and_clear_pending_msg_events();
-	assert_eq!(as_msg_events.len(), 2);
-	let as_funding_locked = if let MessageSendEvent::SendFundingLocked { ref node_id, ref msg } = as_msg_events[0] {
-		assert_eq!(*node_id, nodes[1].node.get_our_node_id());
-		msg.clone()
-	} else { panic!("Unexpected event"); };
-	if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = as_msg_events[1] {
-		assert_eq!(*node_id, nodes[1].node.get_our_node_id());
-	} else { panic!("Unexpected event"); }
-
-	nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked);
-	let bs_msg_events = nodes[1].node.get_and_clear_pending_msg_events();
-	assert_eq!(bs_msg_events.len(), 1);
-	if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = bs_msg_events[0] {
-		assert_eq!(*node_id, nodes[0].node.get_our_node_id());
-	} else { panic!("Unexpected event"); }
-
-	send_payment(&nodes[0], &[&nodes[1]], 100_000);
-
-	// After 6 confirmations, as required by the spec, we'll send announcement_signatures and
-	// broadcast the channel_announcement (but not before exactly 6 confirmations).
-	connect_blocks(&nodes[0], 4);
-	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
-	connect_blocks(&nodes[0], 1);
-	nodes[1].node.handle_announcement_signatures(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendAnnouncementSignatures, nodes[1].node.get_our_node_id()));
-	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
-
-	connect_blocks(&nodes[1], 5);
-	let bs_announce_events = nodes[1].node.get_and_clear_pending_msg_events();
-	assert_eq!(bs_announce_events.len(), 2);
-	let bs_announcement_sigs = if let MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } = bs_announce_events[0] {
-		assert_eq!(*node_id, nodes[0].node.get_our_node_id());
-		msg.clone()
-	} else { panic!("Unexpected event"); };
-	let (bs_announcement, bs_update) = if let MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } = bs_announce_events[1] {
-		(msg.clone(), update_msg.clone())
-	} else { panic!("Unexpected event"); };
-
-	nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs);
-	let as_announce_events = nodes[0].node.get_and_clear_pending_msg_events();
-	assert_eq!(as_announce_events.len(), 1);
-	let (announcement, as_update) = if let MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } = as_announce_events[0] {
-		(msg.clone(), update_msg.clone())
-	} else { panic!("Unexpected event"); };
-	assert_eq!(announcement, bs_announcement);
-
-	for node in nodes {
-		assert!(node.net_graph_msg_handler.handle_channel_announcement(&announcement).unwrap());
-		node.net_graph_msg_handler.handle_channel_update(&as_update).unwrap();
-		node.net_graph_msg_handler.handle_channel_update(&bs_update).unwrap();
-	}
-}
-#[test]
-fn test_1_conf_open() {
-	do_test_1_conf_open(ConnectStyle::BestBlockFirst);
-	do_test_1_conf_open(ConnectStyle::TransactionsFirst);
-	do_test_1_conf_open(ConnectStyle::FullBlockViaListen);
-}
-
 fn do_test_sanity_on_in_flight_opens(steps: u8) {
 	// Previously, we had issues deserializing channels when we hadn't connected the first block
 	// after creation. To catch that and similar issues, we lean on the Node::drop impl to test
@@ -7569,130 +7486,6 @@ fn test_announce_disable_channels() {
 	assert!(chans_disabled.is_empty());
 }
 
-#[test]
-fn test_priv_forwarding_rejection() {
-	// If we have a private channel with outbound liquidity, and
-	// UserConfig::accept_forwards_to_priv_channels is set to false, we should reject any attempts
-	// to forward through that channel.
-	let chanmon_cfgs = create_chanmon_cfgs(3);
-	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
-	let mut no_announce_cfg = test_default_channel_config();
-	no_announce_cfg.accept_forwards_to_priv_channels = false;
-	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(no_announce_cfg), None]);
-	let persister: test_utils::TestPersister;
-	let new_chain_monitor: test_utils::TestChainMonitor;
-	let nodes_1_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
-	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
-
-	let chan_id_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).2;
-	let chan_id_2 = create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).0.channel_id;
-
-	// We should always be able to forward through nodes[1] as long as its out through a public
-	// channel:
-	send_payment(&nodes[2], &[&nodes[1], &nodes[0]], 10_000);
-
-	// ... however, if we send to nodes[2], we will have to pass the private channel from nodes[1]
-	// to nodes[2], which should be rejected:
-	let route_hint = RouteHint(vec![RouteHintHop {
-		src_node_id: nodes[1].node.get_our_node_id(),
-		short_channel_id: nodes[2].node.list_channels()[0].short_channel_id.unwrap(),
-		fees: RoutingFees { base_msat: 1000, proportional_millionths: 0 },
-		cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA,
-		htlc_minimum_msat: None,
-		htlc_maximum_msat: None,
-	}]);
-	let last_hops = vec![route_hint];
-	let (route, our_payment_hash, our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], last_hops, 10_000, TEST_FINAL_CLTV);
-
-	nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
-	check_added_monitors!(nodes[0], 1);
-	let payment_event = SendEvent::from_event(nodes[0].node.get_and_clear_pending_msg_events().remove(0));
-	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
-	commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true);
-
-	let htlc_fail_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
-	assert!(htlc_fail_updates.update_add_htlcs.is_empty());
-	assert_eq!(htlc_fail_updates.update_fail_htlcs.len(), 1);
-	assert!(htlc_fail_updates.update_fail_malformed_htlcs.is_empty());
-	assert!(htlc_fail_updates.update_fee.is_none());
-
-	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_fail_updates.update_fail_htlcs[0]);
-	commitment_signed_dance!(nodes[0], nodes[1], htlc_fail_updates.commitment_signed, true, true);
-	expect_payment_failed_with_update!(nodes[0], our_payment_hash, false, nodes[2].node.list_channels()[0].short_channel_id.unwrap(), true);
-
-	// Now disconnect nodes[1] from its peers and restart with accept_forwards_to_priv_channels set
-	// to true. Sadly there is currently no way to change it at runtime.
-
-	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
-	nodes[2].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
-
-	let nodes_1_serialized = nodes[1].node.encode();
-	let mut monitor_a_serialized = test_utils::TestVecWriter(Vec::new());
-	let mut monitor_b_serialized = test_utils::TestVecWriter(Vec::new());
-	get_monitor!(nodes[1], chan_id_1).write(&mut monitor_a_serialized).unwrap();
-	get_monitor!(nodes[1], chan_id_2).write(&mut monitor_b_serialized).unwrap();
-
-	persister = test_utils::TestPersister::new();
-	let keys_manager = &chanmon_cfgs[1].keys_manager;
-	new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[1].chain_source), nodes[1].tx_broadcaster.clone(), nodes[1].logger, node_cfgs[1].fee_estimator, &persister, keys_manager);
-	nodes[1].chain_monitor = &new_chain_monitor;
-
-	let mut monitor_a_read = &monitor_a_serialized.0[..];
-	let mut monitor_b_read = &monitor_b_serialized.0[..];
-	let (_, mut monitor_a) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut monitor_a_read, keys_manager).unwrap();
-	let (_, mut monitor_b) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut monitor_b_read, keys_manager).unwrap();
-	assert!(monitor_a_read.is_empty());
-	assert!(monitor_b_read.is_empty());
-
-	no_announce_cfg.accept_forwards_to_priv_channels = true;
-
-	let mut nodes_1_read = &nodes_1_serialized[..];
-	let (_, nodes_1_deserialized_tmp) = {
-		let mut channel_monitors = HashMap::new();
-		channel_monitors.insert(monitor_a.get_funding_txo().0, &mut monitor_a);
-		channel_monitors.insert(monitor_b.get_funding_txo().0, &mut monitor_b);
-		<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_1_read, ChannelManagerReadArgs {
-			default_config: no_announce_cfg,
-			keys_manager,
-			fee_estimator: node_cfgs[1].fee_estimator,
-			chain_monitor: nodes[1].chain_monitor,
-			tx_broadcaster: nodes[1].tx_broadcaster.clone(),
-			logger: nodes[1].logger,
-			channel_monitors,
-		}).unwrap()
-	};
-	assert!(nodes_1_read.is_empty());
-	nodes_1_deserialized = nodes_1_deserialized_tmp;
-
-	assert!(nodes[1].chain_monitor.watch_channel(monitor_a.get_funding_txo().0, monitor_a).is_ok());
-	assert!(nodes[1].chain_monitor.watch_channel(monitor_b.get_funding_txo().0, monitor_b).is_ok());
-	check_added_monitors!(nodes[1], 2);
-	nodes[1].node = &nodes_1_deserialized;
-
-	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() });
-	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
-	let as_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
-	let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
-	nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reestablish);
-	nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
-	get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
-	get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id());
-
-	nodes[1].node.peer_connected(&nodes[2].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() });
-	nodes[2].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
-	let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[2].node.get_our_node_id());
-	let cs_reestablish = get_event_msg!(nodes[2], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
-	nodes[2].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
-	nodes[1].node.handle_channel_reestablish(&nodes[2].node.get_our_node_id(), &cs_reestablish);
-	get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[2].node.get_our_node_id());
-	get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
-
-	nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
-	check_added_monitors!(nodes[0], 1);
-	pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], 10_000, our_payment_hash, our_payment_secret);
-	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], our_payment_preimage);
-}
-
 #[test]
 fn test_bump_penalty_txn_on_revoked_commitment() {
 	// In case of penalty txn with too low feerates for getting into mempools, RBF-bump them to be sure
diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs
index 44704ae5040..444c9685fdd 100644
--- a/lightning/src/ln/mod.rs
+++ b/lightning/src/ln/mod.rs
@@ -54,6 +54,9 @@ mod functional_tests;
 mod payment_tests;
 #[cfg(test)]
 #[allow(unused_mut)]
+mod priv_short_conf_tests;
+#[cfg(test)]
+#[allow(unused_mut)]
 mod chanmon_update_fail_tests;
 #[cfg(test)]
 #[allow(unused_mut)]
diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs
new file mode 100644
index 00000000000..f8a1a70e2d3
--- /dev/null
+++ b/lightning/src/ln/priv_short_conf_tests.rs
@@ -0,0 +1,239 @@
+// This file is Copyright its original authors, visible in version control
+// history.
+//
+// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
+// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
+// You may not use this file except in accordance with one or both of these
+// licenses.
+
+//! Tests that test ChannelManager behavior with fewer confirmations required than the default and
+//! other behavior that exists only on private channels or with a semi-trusted counterparty (eg
+//! LSP).
+
+use chain::Watch;
+use chain::channelmonitor::ChannelMonitor;
+use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, MIN_CLTV_EXPIRY_DELTA};
+use routing::network_graph::RoutingFees;
+use routing::router::{RouteHint, RouteHintHop};
+use ln::features::InitFeatures;
+use ln::msgs;
+use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler};
+use util::enforcing_trait_impls::EnforcingSigner;
+use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
+use util::config::UserConfig;
+use util::ser::{Writeable, ReadableArgs};
+use util::test_utils;
+
+use prelude::*;
+use core::default::Default;
+
+use ln::functional_test_utils::*;
+
+use bitcoin::hash_types::BlockHash;
+
+#[test]
+fn test_priv_forwarding_rejection() {
+	// If we have a private channel with outbound liquidity, and
+	// UserConfig::accept_forwards_to_priv_channels is set to false, we should reject any attempts
+	// to forward through that channel.
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let mut no_announce_cfg = test_default_channel_config();
+	no_announce_cfg.accept_forwards_to_priv_channels = false;
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(no_announce_cfg), None]);
+	let persister: test_utils::TestPersister;
+	let new_chain_monitor: test_utils::TestChainMonitor;
+	let nodes_1_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
+	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+	let chan_id_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).2;
+	let chan_id_2 = create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).0.channel_id;
+
+	// We should always be able to forward through nodes[1] as long as its out through a public
+	// channel:
+	send_payment(&nodes[2], &[&nodes[1], &nodes[0]], 10_000);
+
+	// ... however, if we send to nodes[2], we will have to pass the private channel from nodes[1]
+	// to nodes[2], which should be rejected:
+	let route_hint = RouteHint(vec![RouteHintHop {
+		src_node_id: nodes[1].node.get_our_node_id(),
+		short_channel_id: nodes[2].node.list_channels()[0].short_channel_id.unwrap(),
+		fees: RoutingFees { base_msat: 1000, proportional_millionths: 0 },
+		cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA,
+		htlc_minimum_msat: None,
+		htlc_maximum_msat: None,
+	}]);
+	let last_hops = vec![route_hint];
+	let (route, our_payment_hash, our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], last_hops, 10_000, TEST_FINAL_CLTV);
+
+	nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
+	check_added_monitors!(nodes[0], 1);
+	let payment_event = SendEvent::from_event(nodes[0].node.get_and_clear_pending_msg_events().remove(0));
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+	commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true);
+
+	let htlc_fail_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+	assert!(htlc_fail_updates.update_add_htlcs.is_empty());
+	assert_eq!(htlc_fail_updates.update_fail_htlcs.len(), 1);
+	assert!(htlc_fail_updates.update_fail_malformed_htlcs.is_empty());
+	assert!(htlc_fail_updates.update_fee.is_none());
+
+	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_fail_updates.update_fail_htlcs[0]);
+	commitment_signed_dance!(nodes[0], nodes[1], htlc_fail_updates.commitment_signed, true, true);
+	expect_payment_failed_with_update!(nodes[0], our_payment_hash, false, nodes[2].node.list_channels()[0].short_channel_id.unwrap(), true);
+
+	// Now disconnect nodes[1] from its peers and restart with accept_forwards_to_priv_channels set
+	// to true. Sadly there is currently no way to change it at runtime.
+
+	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+	nodes[2].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+
+	let nodes_1_serialized = nodes[1].node.encode();
+	let mut monitor_a_serialized = test_utils::TestVecWriter(Vec::new());
+	let mut monitor_b_serialized = test_utils::TestVecWriter(Vec::new());
+	get_monitor!(nodes[1], chan_id_1).write(&mut monitor_a_serialized).unwrap();
+	get_monitor!(nodes[1], chan_id_2).write(&mut monitor_b_serialized).unwrap();
+
+	persister = test_utils::TestPersister::new();
+	let keys_manager = &chanmon_cfgs[1].keys_manager;
+	new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[1].chain_source), nodes[1].tx_broadcaster.clone(), nodes[1].logger, node_cfgs[1].fee_estimator, &persister, keys_manager);
+	nodes[1].chain_monitor = &new_chain_monitor;
+
+	let mut monitor_a_read = &monitor_a_serialized.0[..];
+	let mut monitor_b_read = &monitor_b_serialized.0[..];
+	let (_, mut monitor_a) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut monitor_a_read, keys_manager).unwrap();
+	let (_, mut monitor_b) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut monitor_b_read, keys_manager).unwrap();
+	assert!(monitor_a_read.is_empty());
+	assert!(monitor_b_read.is_empty());
+
+	no_announce_cfg.accept_forwards_to_priv_channels = true;
+
+	let mut nodes_1_read = &nodes_1_serialized[..];
+	let (_, nodes_1_deserialized_tmp) = {
+		let mut channel_monitors = HashMap::new();
+		channel_monitors.insert(monitor_a.get_funding_txo().0, &mut monitor_a);
+		channel_monitors.insert(monitor_b.get_funding_txo().0, &mut monitor_b);
+		<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_1_read, ChannelManagerReadArgs {
+			default_config: no_announce_cfg,
+			keys_manager,
+			fee_estimator: node_cfgs[1].fee_estimator,
+			chain_monitor: nodes[1].chain_monitor,
+			tx_broadcaster: nodes[1].tx_broadcaster.clone(),
+			logger: nodes[1].logger,
+			channel_monitors,
+		}).unwrap()
+	};
+	assert!(nodes_1_read.is_empty());
+	nodes_1_deserialized = nodes_1_deserialized_tmp;
+
+	assert!(nodes[1].chain_monitor.watch_channel(monitor_a.get_funding_txo().0, monitor_a).is_ok());
+	assert!(nodes[1].chain_monitor.watch_channel(monitor_b.get_funding_txo().0, monitor_b).is_ok());
+	check_added_monitors!(nodes[1], 2);
+	nodes[1].node = &nodes_1_deserialized;
+
+	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() });
+	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+	let as_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+	let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+	nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &as_reestablish);
+	nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
+	get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
+	get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id());
+
+	nodes[1].node.peer_connected(&nodes[2].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() });
+	nodes[2].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+	let bs_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[2].node.get_our_node_id());
+	let cs_reestablish = get_event_msg!(nodes[2], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
+	nodes[2].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish);
+	nodes[1].node.handle_channel_reestablish(&nodes[2].node.get_our_node_id(), &cs_reestablish);
+	get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[2].node.get_our_node_id());
+	get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
+
+	nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
+	check_added_monitors!(nodes[0], 1);
+	pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], 10_000, our_payment_hash, our_payment_secret);
+	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], our_payment_preimage);
+}
+
+fn do_test_1_conf_open(connect_style: ConnectStyle) {
+	// Previously, if the minium_depth config was set to 1, we'd never send a funding_locked. This
+	// tests that we properly send one in that case.
+	let mut alice_config = UserConfig::default();
+	alice_config.own_channel_config.minimum_depth = 1;
+	alice_config.channel_options.announced_channel = true;
+	alice_config.peer_channel_config_limits.force_announced_channel_preference = false;
+	let mut bob_config = UserConfig::default();
+	bob_config.own_channel_config.minimum_depth = 1;
+	bob_config.channel_options.announced_channel = true;
+	bob_config.peer_channel_config_limits.force_announced_channel_preference = false;
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(alice_config), Some(bob_config)]);
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+	*nodes[0].connect_style.borrow_mut() = connect_style;
+
+	let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
+	mine_transaction(&nodes[1], &tx);
+	nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id()));
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+
+	mine_transaction(&nodes[0], &tx);
+	let as_msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(as_msg_events.len(), 2);
+	let as_funding_locked = if let MessageSendEvent::SendFundingLocked { ref node_id, ref msg } = as_msg_events[0] {
+		assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+		msg.clone()
+	} else { panic!("Unexpected event"); };
+	if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = as_msg_events[1] {
+		assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+	} else { panic!("Unexpected event"); }
+
+	nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &as_funding_locked);
+	let bs_msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+	assert_eq!(bs_msg_events.len(), 1);
+	if let MessageSendEvent::SendChannelUpdate { ref node_id, msg: _ } = bs_msg_events[0] {
+		assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+	} else { panic!("Unexpected event"); }
+
+	send_payment(&nodes[0], &[&nodes[1]], 100_000);
+
+	// After 6 confirmations, as required by the spec, we'll send announcement_signatures and
+	// broadcast the channel_announcement (but not before exactly 6 confirmations).
+	connect_blocks(&nodes[0], 4);
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+	connect_blocks(&nodes[0], 1);
+	nodes[1].node.handle_announcement_signatures(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendAnnouncementSignatures, nodes[1].node.get_our_node_id()));
+	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+	connect_blocks(&nodes[1], 5);
+	let bs_announce_events = nodes[1].node.get_and_clear_pending_msg_events();
+	assert_eq!(bs_announce_events.len(), 2);
+	let bs_announcement_sigs = if let MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } = bs_announce_events[0] {
+		assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+		msg.clone()
+	} else { panic!("Unexpected event"); };
+	let (bs_announcement, bs_update) = if let MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } = bs_announce_events[1] {
+		(msg.clone(), update_msg.clone())
+	} else { panic!("Unexpected event"); };
+
+	nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs);
+	let as_announce_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(as_announce_events.len(), 1);
+	let (announcement, as_update) = if let MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } = as_announce_events[0] {
+		(msg.clone(), update_msg.clone())
+	} else { panic!("Unexpected event"); };
+	assert_eq!(announcement, bs_announcement);
+
+	for node in nodes {
+		assert!(node.net_graph_msg_handler.handle_channel_announcement(&announcement).unwrap());
+		node.net_graph_msg_handler.handle_channel_update(&as_update).unwrap();
+		node.net_graph_msg_handler.handle_channel_update(&bs_update).unwrap();
+	}
+}
+#[test]
+fn test_1_conf_open() {
+	do_test_1_conf_open(ConnectStyle::BestBlockFirst);
+	do_test_1_conf_open(ConnectStyle::TransactionsFirst);
+	do_test_1_conf_open(ConnectStyle::FullBlockViaListen);
+}

From a274261c59d5e4fb30b2ed4e49b59830ecb89bb3 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 1 Feb 2022 23:42:05 +0000
Subject: [PATCH 4/9] Clean up the `handle_monitor_err!()` macro argument forms
 somewhat

`handle_monitor_err!()` has a number of different forms depending
on which messages and actions were outstanding when the monitor
updating first failed. Instead of matching by argument count, its
much more readable to put an explicit string in the arguments to
make it easy to scan for the called form.
---
 lightning/src/ln/channelmanager.rs | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index d206dee4950..6d067c24a1e 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1458,9 +1458,6 @@ macro_rules! remove_channel {
 }
 
 macro_rules! handle_monitor_err {
-	($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr) => {
-		handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, Vec::new(), Vec::new())
-	};
 	($self: ident, $err: expr, $short_to_id: expr, $chan: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr, $failed_finalized_fulfills: expr, $chan_id: expr) => {
 		match $err {
 			ChannelMonitorUpdateErr::PermanentFailure => {
@@ -1513,9 +1510,19 @@ macro_rules! handle_monitor_err {
 		}
 		res
 	} };
+	($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $chan_id: expr, COMMITMENT_UPDATE_ONLY) => { {
+		debug_assert!($action_type == RAACommitmentOrder::CommitmentFirst);
+		handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, false, true, Vec::new(), Vec::new(), Vec::new(), $chan_id)
+	} };
+	($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $chan_id: expr, NO_UPDATE) => {
+		handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, false, false, Vec::new(), Vec::new(), Vec::new(), $chan_id)
+	};
+	($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr) => {
+		handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, Vec::new(), Vec::new(), Vec::new())
+	};
 	($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => {
 		handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new())
-	}
+	};
 }
 
 macro_rules! return_monitor_err {
@@ -1911,7 +1918,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					if let Some(monitor_update) = monitor_update {
 						if let Err(e) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) {
 							let (result, is_permanent) =
-								handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), Vec::new(), chan_entry.key());
+								handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
 							if is_permanent {
 								remove_channel!(channel_state, chan_entry);
 								break result;
@@ -3455,7 +3462,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		let ret_err = match res {
 			Ok(Some((update_fee, commitment_signed, monitor_update))) => {
 				if let Err(e) = self.chain_monitor.update_channel(chan.get_funding_txo().unwrap(), monitor_update) {
-					let (res, drop) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, false, true, Vec::new(), Vec::new(), Vec::new(), chan_id);
+					let (res, drop) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, chan_id, COMMITMENT_UPDATE_ONLY);
 					if drop { retain_channel = false; }
 					res
 				} else {
@@ -4399,7 +4406,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					if let Some(monitor_update) = monitor_update {
 						if let Err(e) = self.chain_monitor.update_channel(chan_entry.get().get_funding_txo().unwrap(), monitor_update) {
 							let (result, is_permanent) =
-								handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, false, false, Vec::new(), Vec::new(), Vec::new(), chan_entry.key());
+								handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
 							if is_permanent {
 								remove_channel!(channel_state, chan_entry);
 								break result;
@@ -4965,7 +4972,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 						if let Some((commitment_update, monitor_update)) = commitment_opt {
 							if let Err(e) = self.chain_monitor.update_channel(chan.get_funding_txo().unwrap(), monitor_update) {
 								has_monitor_update = true;
-								let (res, close_channel) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, false, true, Vec::new(), Vec::new(), Vec::new(), channel_id);
+								let (res, close_channel) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, channel_id, COMMITMENT_UPDATE_ONLY);
 								handle_errors.push((chan.get_counterparty_node_id(), res));
 								if close_channel { return false; }
 							} else {

From f54ebf78f69f08f7b12556b5abe5eb7a3e54c466 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Fri, 4 Feb 2022 21:35:41 +0000
Subject: [PATCH 5/9] Add support for deserializing the new SCID alias in
 funding_locked

---
 lightning/src/ln/channel.rs | 4 ++++
 lightning/src/ln/msgs.rs    | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index dd9fcbea9a8..9935d3c019c 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -3457,6 +3457,7 @@ impl<Signer: Sign> Channel<Signer> {
 			Some(msgs::FundingLocked {
 				channel_id: self.channel_id(),
 				next_per_commitment_point,
+				short_channel_id_alias: None,
 			})
 		} else { None };
 
@@ -3678,6 +3679,7 @@ impl<Signer: Sign> Channel<Signer> {
 				funding_locked: Some(msgs::FundingLocked {
 					channel_id: self.channel_id(),
 					next_per_commitment_point,
+					short_channel_id_alias: None,
 				}),
 				raa: None, commitment_update: None, mon_update: None,
 				order: RAACommitmentOrder::CommitmentFirst,
@@ -3713,6 +3715,7 @@ impl<Signer: Sign> Channel<Signer> {
 			Some(msgs::FundingLocked {
 				channel_id: self.channel_id(),
 				next_per_commitment_point,
+				short_channel_id_alias: None,
 			})
 		} else { None };
 
@@ -4447,6 +4450,7 @@ impl<Signer: Sign> Channel<Signer> {
 					return Some(msgs::FundingLocked {
 						channel_id: self.channel_id,
 						next_per_commitment_point,
+						short_channel_id_alias: None,
 					});
 				}
 			} else {
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 7295c40d544..fab396e3c71 100644
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -241,6 +241,9 @@ pub struct FundingLocked {
 	pub channel_id: [u8; 32],
 	/// The per-commitment point of the second commitment transaction
 	pub next_per_commitment_point: PublicKey,
+	/// If set, provides a short_channel_id alias for this channel. The sender will accept payments
+	/// to be forwarded over this SCID and forward them to this messages' recipient.
+	pub short_channel_id_alias: Option<u64>,
 }
 
 /// A shutdown message to be sent or received from a peer
@@ -1155,7 +1158,9 @@ impl_writeable_msg!(FundingSigned, {
 impl_writeable_msg!(FundingLocked, {
 	channel_id,
 	next_per_commitment_point,
-}, {});
+}, {
+	(1, short_channel_id_alias, option),
+});
 
 impl Writeable for Init {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
@@ -2246,6 +2251,7 @@ mod tests {
 		let funding_locked = msgs::FundingLocked {
 			channel_id: [2; 32],
 			next_per_commitment_point: pubkey_1,
+			short_channel_id_alias: None,
 		};
 		let encoded_value = funding_locked.encode();
 		let target_value = hex::decode("0202020202020202020202020202020202020202020202020202020202020202031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f").unwrap();

From b2629afd88b0874ca1ede986a9c789eea20c6be8 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 1 Feb 2022 17:37:16 +0000
Subject: [PATCH 6/9] Track SCID aliases from our counterparty and use them in
 invoices

New `funding_locked` messages can include SCID aliases which our
counterparty will recognize as "ours" for the purposes of relaying
transactions to us. This avoids telling the world about our
on-chain transactions every time we want to receive a payment, and
will allow for receiving payments before the funding transaction
appears on-chain.

Here we store the new SCID aliases and use them in invoices instead
of he "standard" SCIDs.
---
 fuzz/src/router.rs                 |  1 +
 lightning-invoice/src/utils.rs     | 11 +++++++++--
 lightning/src/ln/channel.rs        | 30 ++++++++++++++++++++++++++++++
 lightning/src/ln/channelmanager.rs | 24 ++++++++++++++++++++++++
 lightning/src/routing/router.rs    |  2 ++
 5 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
index 095059c5183..20ca399449c 100644
--- a/fuzz/src/router.rs
+++ b/fuzz/src/router.rs
@@ -216,6 +216,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 								},
 								funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 								short_channel_id: Some(scid),
+								inbound_scid_alias: None,
 								channel_value_satoshis: slice_to_be64(get_slice!(8)),
 								user_channel_id: 0, inbound_capacity_msat: 0,
 								unspendable_punishment_reserve: None,
diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs
index fc82541a564..73f0138824d 100644
--- a/lightning-invoice/src/utils.rs
+++ b/lightning-invoice/src/utils.rs
@@ -65,7 +65,7 @@ pub fn create_phantom_invoice<Signer: Sign, K: Deref>(
 
 	for hint in phantom_route_hints {
 		for channel in &hint.channels {
-			let short_channel_id = match channel.short_channel_id {
+			let short_channel_id = match channel.get_inbound_payment_scid() {
 				Some(id) => id,
 				None => continue,
 			};
@@ -162,7 +162,7 @@ where
 	let our_channels = channelmanager.list_usable_channels();
 	let mut route_hints = vec![];
 	for channel in our_channels {
-		let short_channel_id = match channel.short_channel_id {
+		let short_channel_id = match channel.get_inbound_payment_scid() {
 			Some(id) => id,
 			None => continue,
 		};
@@ -313,6 +313,13 @@ mod test {
 		assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64);
 		assert_eq!(invoice.description(), InvoiceDescription::Direct(&Description("test".to_string())));
 
+		// Invoice SCIDs should always use inbound SCID aliases over the real channel ID, if one is
+		// available.
+		assert_eq!(invoice.route_hints().len(), 1);
+		assert_eq!(invoice.route_hints()[0].0.len(), 1);
+		assert_eq!(invoice.route_hints()[0].0[0].short_channel_id,
+			nodes[1].node.list_usable_channels()[0].inbound_scid_alias.unwrap());
+
 		let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
 			.with_features(invoice.features().unwrap().clone())
 			.with_route_hints(invoice.route_hints());
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 9935d3c019c..99fb345be7d 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -695,6 +695,13 @@ pub(super) struct Channel<Signer: Sign> {
 
 	/// This channel's type, as negotiated during channel open
 	channel_type: ChannelTypeFeatures,
+
+	// Our counterparty can offer us SCID aliases which they will map to this channel when routing
+	// outbound payments. These can be used in invoice route hints to avoid explicitly revealing
+	// the channel's funding UTXO.
+	// We only bother storing the most recent SCID alias at any time, though our counterparty has
+	// to store all of them.
+	latest_inbound_scid_alias: Option<u64>,
 }
 
 #[cfg(any(test, fuzzing))]
@@ -947,6 +954,8 @@ impl<Signer: Sign> Channel<Signer> {
 
 			workaround_lnd_bug_4006: None,
 
+			latest_inbound_scid_alias: None,
+
 			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills: HashSet::new(),
 
@@ -1252,6 +1261,8 @@ impl<Signer: Sign> Channel<Signer> {
 
 			workaround_lnd_bug_4006: None,
 
+			latest_inbound_scid_alias: None,
+
 			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills: HashSet::new(),
 
@@ -2151,6 +2162,15 @@ impl<Signer: Sign> Channel<Signer> {
 			return Err(ChannelError::Ignore("Peer sent funding_locked when we needed a channel_reestablish. The peer is likely lnd, see https://github.com/lightningnetwork/lnd/issues/4006".to_owned()));
 		}
 
+		if let Some(scid_alias) = msg.short_channel_id_alias {
+			if Some(scid_alias) != self.short_channel_id {
+				// The scid alias provided can be used to route payments *from* our counterparty,
+				// i.e. can be used for inbound payments and provided in invoices, but is not used
+				// when routing outbound payments.
+				self.latest_inbound_scid_alias = Some(scid_alias);
+			}
+		}
+
 		let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
 
 		if non_shutdown_state == ChannelState::FundingSent as u32 {
@@ -4198,6 +4218,11 @@ impl<Signer: Sign> Channel<Signer> {
 		self.short_channel_id
 	}
 
+	/// Allowed in any state (including after shutdown)
+	pub fn latest_inbound_scid_alias(&self) -> Option<u64> {
+		self.latest_inbound_scid_alias
+	}
+
 	/// Returns the funding_txo we either got from our peer, or were given by
 	/// get_outbound_funding_created.
 	pub fn get_funding_txo(&self) -> Option<OutPoint> {
@@ -5769,6 +5794,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(13, self.channel_creation_height, required),
 			(15, preimages, vec_type),
 			(17, self.announcement_sigs_state, required),
+			(19, self.latest_inbound_scid_alias, option),
 		});
 
 		Ok(())
@@ -6024,6 +6050,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		// If we read an old Channel, for simplicity we just treat it as "we never sent an
 		// AnnouncementSignatures" which implies we'll re-send it on reconnect, but that's fine.
 		let mut announcement_sigs_state = Some(AnnouncementSigsState::NotSent);
+		let mut latest_inbound_scid_alias = None;
 
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
@@ -6039,6 +6066,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			(13, channel_creation_height, option),
 			(15, preimages_opt, vec_type),
 			(17, announcement_sigs_state, option),
+			(19, latest_inbound_scid_alias, option),
 		});
 
 		if let Some(preimages) = preimages_opt {
@@ -6173,6 +6201,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 
 			workaround_lnd_bug_4006: None,
 
+			latest_inbound_scid_alias,
+
 			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills,
 
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 6d067c24a1e..46bad1f5656 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1188,7 +1188,20 @@ pub struct ChannelDetails {
 	pub funding_txo: Option<OutPoint>,
 	/// The position of the funding transaction in the chain. None if the funding transaction has
 	/// not yet been confirmed and the channel fully opened.
+	///
+	/// Note that if [`inbound_scid_alias`] is set, it must be used for invoices and inbound
+	/// payments instead of this. See [`get_inbound_payment_scid`].
+	///
+	/// [`inbound_scid_alias`]: Self::inbound_scid_alias
+	/// [`get_inbound_payment_scid`]: Self::get_inbound_payment_scid
 	pub short_channel_id: Option<u64>,
+	/// An optional [`short_channel_id`] alias for this channel, randomly generated by our
+	/// counterparty and usable in place of [`short_channel_id`] in invoice route hints. Our
+	/// counterparty will recognize the alias provided here in place of the [`short_channel_id`]
+	/// when they see a payment to be routed to us.
+	///
+	/// [`short_channel_id`]: Self::short_channel_id
+	pub inbound_scid_alias: Option<u64>,
 	/// The value, in satoshis, of this channel as appears in the funding output
 	pub channel_value_satoshis: u64,
 	/// The value, in satoshis, that must always be held in the channel for us. This value ensures
@@ -1274,6 +1287,15 @@ pub struct ChannelDetails {
 	pub is_public: bool,
 }
 
+impl ChannelDetails {
+	/// Gets the SCID which should be used to identify this channel for inbound payments. This
+	/// should be used for providing invoice hints or in any other context where our counterparty
+	/// will forward a payment to us.
+	pub fn get_inbound_payment_scid(&self) -> Option<u64> {
+		self.inbound_scid_alias.or(self.short_channel_id)
+	}
+}
+
 /// If a payment fails to send, it can be in one of several states. This enum is returned as the
 /// Err() type describing which state the payment is in, see the description of individual enum
 /// states for more.
@@ -1833,6 +1855,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					},
 					funding_txo: channel.get_funding_txo(),
 					short_channel_id: channel.get_short_channel_id(),
+					inbound_scid_alias: channel.latest_inbound_scid_alias(),
 					channel_value_satoshis: channel.get_value_satoshis(),
 					unspendable_punishment_reserve: to_self_reserve_satoshis,
 					balance_msat,
@@ -5973,6 +5996,7 @@ impl_writeable_tlv_based!(ChannelCounterparty, {
 });
 
 impl_writeable_tlv_based!(ChannelDetails, {
+	(1, inbound_scid_alias, option),
 	(2, channel_id, required),
 	(4, counterparty, required),
 	(6, funding_txo, option),
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index c45ce59ac13..870b1e1261f 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -1578,6 +1578,7 @@ mod tests {
 			},
 			funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
 			short_channel_id,
+			inbound_scid_alias: None,
 			channel_value_satoshis: 0,
 			user_channel_id: 0,
 			balance_msat: 0,
@@ -5101,6 +5102,7 @@ mod benches {
 				txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0
 			}),
 			short_channel_id: Some(1),
+			inbound_scid_alias: None,
 			channel_value_satoshis: 10_000_000,
 			user_channel_id: 0,
 			balance_msat: 10_000_000,

From 10be993f9458ff14c2085b1abe53eceb8870ce6a Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 8 Feb 2022 21:43:14 +0000
Subject: [PATCH 7/9] Handle `short_to_id` state updates on channel closure via
 macros

This avoids needing to update channel closure code in many places
as we add multiple SCIDs for each channel and have to track them.
---
 lightning/src/ln/channelmanager.rs | 65 ++++++++++--------------------
 1 file changed, 22 insertions(+), 43 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 46bad1f5656..94d78e798ca 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1405,6 +1405,14 @@ macro_rules! handle_error {
 	}
 }
 
+macro_rules! update_maps_on_chan_removal {
+	($short_to_id: expr, $channel: expr) => {
+		if let Some(short_id) = $channel.get_short_channel_id() {
+			$short_to_id.remove(&short_id);
+		}
+	}
+}
+
 /// Returns (boolean indicating if we should remove the Channel object from memory, a mapped error)
 macro_rules! convert_chan_err {
 	($self: ident, $err: expr, $short_to_id: expr, $channel: expr, $channel_id: expr) => {
@@ -1417,18 +1425,14 @@ macro_rules! convert_chan_err {
 			},
 			ChannelError::Close(msg) => {
 				log_error!($self.logger, "Closing channel {} due to close-required error: {}", log_bytes!($channel_id[..]), msg);
-				if let Some(short_id) = $channel.get_short_channel_id() {
-					$short_to_id.remove(&short_id);
-				}
+				update_maps_on_chan_removal!($short_to_id, $channel);
 				let shutdown_res = $channel.force_shutdown(true);
 				(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
 					shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
 			},
 			ChannelError::CloseDelayBroadcast(msg) => {
 				log_error!($self.logger, "Channel {} need to be shutdown but closing transactions not broadcast due to {}", log_bytes!($channel_id[..]), msg);
-				if let Some(short_id) = $channel.get_short_channel_id() {
-					$short_to_id.remove(&short_id);
-				}
+				update_maps_on_chan_removal!($short_to_id, $channel);
 				let shutdown_res = $channel.force_shutdown(false);
 				(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
 					shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
@@ -1471,9 +1475,7 @@ macro_rules! remove_channel {
 	($channel_state: expr, $entry: expr) => {
 		{
 			let channel = $entry.remove_entry().1;
-			if let Some(short_id) = channel.get_short_channel_id() {
-				$channel_state.short_to_id.remove(&short_id);
-			}
+			update_maps_on_chan_removal!($channel_state.short_to_id, channel);
 			channel
 		}
 	}
@@ -1484,9 +1486,7 @@ macro_rules! handle_monitor_err {
 		match $err {
 			ChannelMonitorUpdateErr::PermanentFailure => {
 				log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateErr::PermanentFailure", log_bytes!($chan_id[..]));
-				if let Some(short_id) = $chan.get_short_channel_id() {
-					$short_to_id.remove(&short_id);
-				}
+				update_maps_on_chan_removal!($short_to_id, $chan);
 				// TODO: $failed_fails is dropped here, which will cause other channels to hit the
 				// chain in a confused state! We need to move them into the ChannelMonitor which
 				// will be responsible for failing backwards once things confirm on-chain.
@@ -2049,9 +2049,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 						return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
 					}
 				}
-				if let Some(short_id) = chan.get().get_short_channel_id() {
-					channel_state.short_to_id.remove(&short_id);
-				}
 				if peer_node_id.is_some() {
 					if let Some(peer_msg) = peer_msg {
 						self.issue_channel_close_events(chan.get(),ClosureReason::CounterpartyForceClosed { peer_msg: peer_msg.to_string() });
@@ -2059,7 +2056,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 				} else {
 					self.issue_channel_close_events(chan.get(),ClosureReason::HolderForceClosed);
 				}
-				chan.remove_entry().1
+				remove_channel!(channel_state, chan)
 			} else {
 				return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
 			}
@@ -3206,12 +3203,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 										}
 										ChannelError::Close(msg) => {
 											log_trace!(self.logger, "Closing channel {} due to Close-required error: {}", log_bytes!(chan.key()[..]), msg);
-											let (channel_id, mut channel) = chan.remove_entry();
-											if let Some(short_id) = channel.get_short_channel_id() {
-												channel_state.short_to_id.remove(&short_id);
-											}
+											let mut channel = remove_channel!(channel_state, chan);
 											// ChannelClosed event is generated by handle_error for us.
-											Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
+											Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel.channel_id(), channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
 										},
 										ChannelError::CloseDelayBroadcast(_) => { panic!("Wait is only generated on receipt of channel_reestablish, which is handled by try_chan_entry, we don't bother to support it here"); }
 									};
@@ -4479,10 +4473,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 						// also implies there are no pending HTLCs left on the channel, so we can
 						// fully delete it from tracking (the channel monitor is still around to
 						// watch for old state broadcasts)!
-						if let Some(short_id) = chan_entry.get().get_short_channel_id() {
-							channel_state.short_to_id.remove(&short_id);
-						}
-						(tx, Some(chan_entry.remove_entry().1))
+						(tx, Some(remove_channel!(channel_state, chan_entry)))
 					} else { (tx, None) }
 				},
 				hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
@@ -4920,12 +4911,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					let mut channel_lock = self.channel_state.lock().unwrap();
 					let channel_state = &mut *channel_lock;
 					let by_id = &mut channel_state.by_id;
-					let short_to_id = &mut channel_state.short_to_id;
 					let pending_msg_events = &mut channel_state.pending_msg_events;
-					if let Some(mut chan) = by_id.remove(&funding_outpoint.to_channel_id()) {
-						if let Some(short_id) = chan.get_short_channel_id() {
-							short_to_id.remove(&short_id);
-						}
+					if let hash_map::Entry::Occupied(chan_entry) = by_id.entry(funding_outpoint.to_channel_id()) {
+						let mut chan = remove_channel!(channel_state, chan_entry);
 						failed_channels.push(chan.force_shutdown(false));
 						if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
 							pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
@@ -5054,10 +5042,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 						if let Some(tx) = tx_opt {
 							// We're done with this channel. We got a closing_signed and sent back
 							// a closing_signed with a closing transaction to broadcast.
-							if let Some(short_id) = chan.get_short_channel_id() {
-								short_to_id.remove(&short_id);
-							}
-
 							if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
 								pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
 									msg: update
@@ -5068,6 +5052,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
 							log_info!(self.logger, "Broadcasting {}", log_tx!(tx));
 							self.tx_broadcaster.broadcast_transaction(&tx);
+							update_maps_on_chan_removal!(short_to_id, chan);
 							false
 						} else { true }
 					},
@@ -5585,9 +5570,7 @@ where
 						}
 					}
 				} else if let Err(reason) = res {
-					if let Some(short_id) = channel.get_short_channel_id() {
-						short_to_id.remove(&short_id);
-					}
+					update_maps_on_chan_removal!(short_to_id, channel);
 					// It looks like our counterparty went on-chain or funding transaction was
 					// reorged out of the main chain. Close the channel.
 					failed_channels.push(channel.force_shutdown(true));
@@ -5783,9 +5766,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
 				log_debug!(self.logger, "Failing all channels with {} due to no_connection_possible", log_pubkey!(counterparty_node_id));
 				channel_state.by_id.retain(|_, chan| {
 					if chan.get_counterparty_node_id() == *counterparty_node_id {
-						if let Some(short_id) = chan.get_short_channel_id() {
-							short_to_id.remove(&short_id);
-						}
+						update_maps_on_chan_removal!(short_to_id, chan);
 						failed_channels.push(chan.force_shutdown(true));
 						if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
 							pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
@@ -5804,9 +5785,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
 					if chan.get_counterparty_node_id() == *counterparty_node_id {
 						chan.remove_uncommitted_htlcs_and_mark_paused(&self.logger);
 						if chan.is_shutdown() {
-							if let Some(short_id) = chan.get_short_channel_id() {
-								short_to_id.remove(&short_id);
-							}
+							update_maps_on_chan_removal!(short_to_id, chan);
 							self.issue_channel_close_events(chan, ClosureReason::DisconnectedPeer);
 							return false;
 						} else {

From 84fa127661ff09de94ac6129dd0790d6de79f8b7 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 15 Feb 2022 23:27:07 +0000
Subject: [PATCH 8/9] Provide our peers with SCID aliases and forward payments
 with them

This creates an SCID alias for all of our outbound channels, which
we send to our counterparties as a part of the `funding_locked`
message and then recognize in any HTLC forwarding instructions.

Note that we generate an SCID alias for all channels, including
already open ones, even though we currently have no way of
communicating to our peers the SCID alias for already-open
channels.
---
 lightning/src/ln/channel.rs               |  56 +++++--
 lightning/src/ln/channelmanager.rs        | 177 +++++++++++++++++-----
 lightning/src/ln/functional_tests.rs      |  20 ++-
 lightning/src/ln/onion_route_tests.rs     |   2 +-
 lightning/src/ln/priv_short_conf_tests.rs |  34 +++++
 lightning/src/ln/reorg_tests.rs           |   2 +-
 lightning/src/util/scid_utils.rs          |  15 +-
 7 files changed, 236 insertions(+), 70 deletions(-)

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 99fb345be7d..65f1be00008 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -702,6 +702,12 @@ pub(super) struct Channel<Signer: Sign> {
 	// We only bother storing the most recent SCID alias at any time, though our counterparty has
 	// to store all of them.
 	latest_inbound_scid_alias: Option<u64>,
+
+	// We always offer our counterparty a static SCID alias, which we recognize as for this channel
+	// if we see it in HTLC forwarding instructions. We don't bother rotating the alias given we
+	// don't currently support node id aliases and eventually privacy should be provided with
+	// blinded paths instead of simple scid+node_id aliases.
+	outbound_scid_alias: u64,
 }
 
 #[cfg(any(test, fuzzing))]
@@ -807,7 +813,8 @@ impl<Signer: Sign> Channel<Signer> {
 	// Constructors:
 	pub fn new_outbound<K: Deref, F: Deref>(
 		fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
-		channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig, current_chain_height: u32
+		channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig, current_chain_height: u32,
+		outbound_scid_alias: u64
 	) -> Result<Channel<Signer>, APIError>
 	where K::Target: KeysInterface<Signer = Signer>,
 	      F::Target: FeeEstimator,
@@ -955,6 +962,7 @@ impl<Signer: Sign> Channel<Signer> {
 			workaround_lnd_bug_4006: None,
 
 			latest_inbound_scid_alias: None,
+			outbound_scid_alias,
 
 			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills: HashSet::new(),
@@ -993,7 +1001,8 @@ impl<Signer: Sign> Channel<Signer> {
 	/// Assumes chain_hash has already been checked and corresponds with what we expect!
 	pub fn new_from_req<K: Deref, F: Deref, L: Deref>(
 		fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
-		msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32, logger: &L
+		msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32, logger: &L,
+		outbound_scid_alias: u64
 	) -> Result<Channel<Signer>, ChannelError>
 		where K::Target: KeysInterface<Signer = Signer>,
 		      F::Target: FeeEstimator,
@@ -1262,6 +1271,7 @@ impl<Signer: Sign> Channel<Signer> {
 			workaround_lnd_bug_4006: None,
 
 			latest_inbound_scid_alias: None,
+			outbound_scid_alias,
 
 			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills: HashSet::new(),
@@ -3477,7 +3487,7 @@ impl<Signer: Sign> Channel<Signer> {
 			Some(msgs::FundingLocked {
 				channel_id: self.channel_id(),
 				next_per_commitment_point,
-				short_channel_id_alias: None,
+				short_channel_id_alias: Some(self.outbound_scid_alias),
 			})
 		} else { None };
 
@@ -3699,7 +3709,7 @@ impl<Signer: Sign> Channel<Signer> {
 				funding_locked: Some(msgs::FundingLocked {
 					channel_id: self.channel_id(),
 					next_per_commitment_point,
-					short_channel_id_alias: None,
+					short_channel_id_alias: Some(self.outbound_scid_alias),
 				}),
 				raa: None, commitment_update: None, mon_update: None,
 				order: RAACommitmentOrder::CommitmentFirst,
@@ -3735,7 +3745,7 @@ impl<Signer: Sign> Channel<Signer> {
 			Some(msgs::FundingLocked {
 				channel_id: self.channel_id(),
 				next_per_commitment_point,
-				short_channel_id_alias: None,
+				short_channel_id_alias: Some(self.outbound_scid_alias),
 			})
 		} else { None };
 
@@ -4223,6 +4233,17 @@ impl<Signer: Sign> Channel<Signer> {
 		self.latest_inbound_scid_alias
 	}
 
+	/// Allowed in any state (including after shutdown)
+	pub fn outbound_scid_alias(&self) -> u64 {
+		self.outbound_scid_alias
+	}
+	/// Only allowed immediately after deserialization if get_outbound_scid_alias returns 0,
+	/// indicating we were written by an old LDK which did not set outbound SCID aliases.
+	pub fn set_outbound_scid_alias(&mut self, outbound_scid_alias: u64) {
+		assert_eq!(self.outbound_scid_alias, 0);
+		self.outbound_scid_alias = outbound_scid_alias;
+	}
+
 	/// Returns the funding_txo we either got from our peer, or were given by
 	/// get_outbound_funding_created.
 	pub fn get_funding_txo(&self) -> Option<OutPoint> {
@@ -4475,7 +4496,7 @@ impl<Signer: Sign> Channel<Signer> {
 					return Some(msgs::FundingLocked {
 						channel_id: self.channel_id,
 						next_per_commitment_point,
-						short_channel_id_alias: None,
+						short_channel_id_alias: Some(self.outbound_scid_alias),
 					});
 				}
 			} else {
@@ -5795,6 +5816,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(15, preimages, vec_type),
 			(17, self.announcement_sigs_state, required),
 			(19, self.latest_inbound_scid_alias, option),
+			(21, self.outbound_scid_alias, required),
 		});
 
 		Ok(())
@@ -6051,6 +6073,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		// AnnouncementSignatures" which implies we'll re-send it on reconnect, but that's fine.
 		let mut announcement_sigs_state = Some(AnnouncementSigsState::NotSent);
 		let mut latest_inbound_scid_alias = None;
+		let mut outbound_scid_alias = None;
 
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
@@ -6067,6 +6090,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			(15, preimages_opt, vec_type),
 			(17, announcement_sigs_state, option),
 			(19, latest_inbound_scid_alias, option),
+			(21, outbound_scid_alias, option),
 		});
 
 		if let Some(preimages) = preimages_opt {
@@ -6202,6 +6226,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			workaround_lnd_bug_4006: None,
 
 			latest_inbound_scid_alias,
+			// Later in the ChannelManager deserialization phase we scan for channels and assign scid aliases if its missing
+			outbound_scid_alias: outbound_scid_alias.unwrap_or(0),
 
 			#[cfg(any(test, fuzzing))]
 			historical_inbound_htlc_fulfills,
@@ -6325,7 +6351,7 @@ mod tests {
 		let secp_ctx = Secp256k1::new();
 		let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let config = UserConfig::default();
-		match Channel::<EnforcingSigner>::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config, 0) {
+		match Channel::<EnforcingSigner>::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config, 0, 42) {
 			Err(APIError::IncompatibleShutdownScript { script }) => {
 				assert_eq!(script.into_inner(), non_v0_segwit_shutdown_script.into_inner());
 			},
@@ -6347,7 +6373,7 @@ mod tests {
 
 		let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let config = UserConfig::default();
-		let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();
+		let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap();
 
 		// Now change the fee so we can check that the fee in the open_channel message is the
 		// same as the old fee.
@@ -6373,13 +6399,13 @@ mod tests {
 		// Create Node A's channel pointing to Node B's pubkey
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let config = UserConfig::default();
-		let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();
+		let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap();
 
 		// Create Node B's channel by receiving Node A's open_channel message
 		// Make sure A's dust limit is as we expect.
 		let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-		let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap();
+		let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
 
 		// Node B --> Node A: accept channel, explicitly setting B's dust limit.
 		let mut accept_channel_msg = node_b_chan.accept_inbound_channel();
@@ -6443,7 +6469,7 @@ mod tests {
 
 		let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let config = UserConfig::default();
-		let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();
+		let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap();
 
 		let commitment_tx_fee_0_htlcs = Channel::<EnforcingSigner>::commit_tx_fee_msat(chan.feerate_per_kw, 0, chan.opt_anchors());
 		let commitment_tx_fee_1_htlc = Channel::<EnforcingSigner>::commit_tx_fee_msat(chan.feerate_per_kw, 1, chan.opt_anchors());
@@ -6492,12 +6518,12 @@ mod tests {
 		// Create Node A's channel pointing to Node B's pubkey
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let config = UserConfig::default();
-		let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();
+		let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap();
 
 		// Create Node B's channel by receiving Node A's open_channel message
 		let open_channel_msg = node_a_chan.get_open_channel(chain_hash);
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-		let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap();
+		let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
 
 		// Node B --> Node A: accept channel
 		let accept_channel_msg = node_b_chan.accept_inbound_channel();
@@ -6554,7 +6580,7 @@ mod tests {
 		// Create a channel.
 		let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let config = UserConfig::default();
-		let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();
+		let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap();
 		assert!(node_a_chan.counterparty_forwarding_info.is_none());
 		assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default
 		assert!(node_a_chan.counterparty_forwarding_info().is_none());
@@ -6619,7 +6645,7 @@ mod tests {
 		let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let mut config = UserConfig::default();
 		config.channel_options.announced_channel = false;
-		let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config, 0).unwrap(); // Nothing uses their network key in this test
+		let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config, 0, 42).unwrap(); // Nothing uses their network key in this test
 		chan.holder_dust_limit_satoshis = 546;
 		chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel
 
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 94d78e798ca..699941586e6 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -660,8 +660,16 @@ pub(super) enum RAACommitmentOrder {
 // Note this is only exposed in cfg(test):
 pub(super) struct ChannelHolder<Signer: Sign> {
 	pub(super) by_id: HashMap<[u8; 32], Channel<Signer>>,
+	/// SCIDs (and outbound SCID aliases) to the real channel id. Outbound SCID aliases are added
+	/// here once the channel is available for normal use, with SCIDs being added once the funding
+	/// transaction is confirmed at the channel's required confirmation depth.
 	pub(super) short_to_id: HashMap<u64, [u8; 32]>,
-	/// short channel id -> forward infos. Key of 0 means payments received
+	/// SCID/SCID Alias -> forward infos. Key of 0 means payments received.
+	///
+	/// Note that because we may have an SCID Alias as the key we can have two entries per channel,
+	/// though in practice we probably won't be receiving HTLCs for a channel both via the alias
+	/// and via the classic SCID.
+	///
 	/// Note that while this is held in the same mutex as the channels themselves, no consistency
 	/// guarantees are made about the existence of a channel with the short id here, nor the short
 	/// ids in the PendingHTLCInfo!
@@ -971,6 +979,12 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
 	/// Locked *after* channel_state.
 	pending_outbound_payments: Mutex<HashMap<PaymentId, PendingOutboundPayment>>,
 
+	/// The set of outbound SCID aliases across all our channels, including unconfirmed channels
+	/// and some closed channels which reached a usable state prior to being closed. This is used
+	/// only to avoid duplicates, and is not persisted explicitly to disk, but rebuilt from the
+	/// active channel list on load.
+	outbound_scid_aliases: Mutex<HashSet<u64>>,
+
 	our_network_key: SecretKey,
 	our_network_pubkey: PublicKey,
 
@@ -1406,10 +1420,20 @@ macro_rules! handle_error {
 }
 
 macro_rules! update_maps_on_chan_removal {
-	($short_to_id: expr, $channel: expr) => {
+	($self: expr, $short_to_id: expr, $channel: expr) => {
 		if let Some(short_id) = $channel.get_short_channel_id() {
 			$short_to_id.remove(&short_id);
+		} else {
+			// If the channel was never confirmed on-chain prior to its closure, remove the
+			// outbound SCID alias we used for it from the collision-prevention set. While we
+			// generally want to avoid ever re-using an outbound SCID alias across all channels, we
+			// also don't want a counterparty to be able to trivially cause a memory leak by simply
+			// opening a million channels with us which are closed before we ever reach the funding
+			// stage.
+			let alias_removed = $self.outbound_scid_aliases.lock().unwrap().remove(&$channel.outbound_scid_alias());
+			debug_assert!(alias_removed);
 		}
+		$short_to_id.remove(&$channel.outbound_scid_alias());
 	}
 }
 
@@ -1425,14 +1449,14 @@ macro_rules! convert_chan_err {
 			},
 			ChannelError::Close(msg) => {
 				log_error!($self.logger, "Closing channel {} due to close-required error: {}", log_bytes!($channel_id[..]), msg);
-				update_maps_on_chan_removal!($short_to_id, $channel);
+				update_maps_on_chan_removal!($self, $short_to_id, $channel);
 				let shutdown_res = $channel.force_shutdown(true);
 				(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
 					shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
 			},
 			ChannelError::CloseDelayBroadcast(msg) => {
 				log_error!($self.logger, "Channel {} need to be shutdown but closing transactions not broadcast due to {}", log_bytes!($channel_id[..]), msg);
-				update_maps_on_chan_removal!($short_to_id, $channel);
+				update_maps_on_chan_removal!($self, $short_to_id, $channel);
 				let shutdown_res = $channel.force_shutdown(false);
 				(true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
 					shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
@@ -1472,10 +1496,10 @@ macro_rules! try_chan_entry {
 }
 
 macro_rules! remove_channel {
-	($channel_state: expr, $entry: expr) => {
+	($self: expr, $channel_state: expr, $entry: expr) => {
 		{
 			let channel = $entry.remove_entry().1;
-			update_maps_on_chan_removal!($channel_state.short_to_id, channel);
+			update_maps_on_chan_removal!($self, $channel_state.short_to_id, channel);
 			channel
 		}
 	}
@@ -1486,7 +1510,7 @@ macro_rules! handle_monitor_err {
 		match $err {
 			ChannelMonitorUpdateErr::PermanentFailure => {
 				log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateErr::PermanentFailure", log_bytes!($chan_id[..]));
-				update_maps_on_chan_removal!($short_to_id, $chan);
+				update_maps_on_chan_removal!($self, $short_to_id, $chan);
 				// TODO: $failed_fails is dropped here, which will cause other channels to hit the
 				// chain in a confused state! We need to move them into the ChannelMonitor which
 				// will be responsible for failing backwards once things confirm on-chain.
@@ -1568,15 +1592,34 @@ macro_rules! maybe_break_monitor_err {
 	}
 }
 
+macro_rules! send_funding_locked {
+	($short_to_id: expr, $pending_msg_events: expr, $channel: expr, $funding_locked_msg: expr) => {
+		$pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
+			node_id: $channel.get_counterparty_node_id(),
+			msg: $funding_locked_msg,
+		});
+		// Note that we may send a funding locked multiple times for a channel if we reconnect, so
+		// we allow collisions, but we shouldn't ever be updating the channel ID pointed to.
+		let outbound_alias_insert = $short_to_id.insert($channel.outbound_scid_alias(), $channel.channel_id());
+		assert!(outbound_alias_insert.is_none() || outbound_alias_insert.unwrap() == $channel.channel_id(),
+			"SCIDs should never collide - ensure you weren't behind the chain tip by a full month when creating channels");
+		if let Some(real_scid) = $channel.get_short_channel_id() {
+			let scid_insert = $short_to_id.insert(real_scid, $channel.channel_id());
+			assert!(scid_insert.is_none() || scid_insert.unwrap() == $channel.channel_id(),
+				"SCIDs should never collide - ensure you weren't behind the chain tip by a full month when creating channels");
+		}
+	}
+}
+
 macro_rules! handle_chan_restoration_locked {
 	($self: ident, $channel_lock: expr, $channel_state: expr, $channel_entry: expr,
 	 $raa: expr, $commitment_update: expr, $order: expr, $chanmon_update: expr,
 	 $pending_forwards: expr, $funding_broadcastable: expr, $funding_locked: expr, $announcement_sigs: expr) => { {
 		let mut htlc_forwards = None;
-		let counterparty_node_id = $channel_entry.get().get_counterparty_node_id();
 
 		let chanmon_update: Option<ChannelMonitorUpdate> = $chanmon_update; // Force type-checking to resolve
 		let chanmon_update_is_none = chanmon_update.is_none();
+		let counterparty_node_id = $channel_entry.get().get_counterparty_node_id();
 		let res = loop {
 			let forwards: Vec<(PendingHTLCInfo, u64)> = $pending_forwards; // Force type-checking to resolve
 			if !forwards.is_empty() {
@@ -1602,11 +1645,7 @@ macro_rules! handle_chan_restoration_locked {
 				// Similar to the above, this implies that we're letting the funding_locked fly
 				// before it should be allowed to.
 				assert!(chanmon_update.is_none());
-				$channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
-					node_id: counterparty_node_id,
-					msg,
-				});
-				$channel_state.short_to_id.insert($channel_entry.get().get_short_channel_id().unwrap(), $channel_entry.get().channel_id());
+				send_funding_locked!($channel_state.short_to_id, $channel_state.pending_msg_events, $channel_entry.get(), msg);
 			}
 			if let Some(msg) = $announcement_sigs {
 				$channel_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
@@ -1735,6 +1774,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 				claimable_htlcs: HashMap::new(),
 				pending_msg_events: Vec::new(),
 			}),
+			outbound_scid_aliases: Mutex::new(HashSet::new()),
 			pending_inbound_payments: Mutex::new(HashMap::new()),
 			pending_outbound_payments: Mutex::new(HashMap::new()),
 
@@ -1766,6 +1806,25 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		&self.default_configuration
 	}
 
+	fn create_and_insert_outbound_scid_alias(&self) -> u64 {
+		let height = self.best_block.read().unwrap().height();
+		let mut outbound_scid_alias = 0;
+		let mut i = 0;
+		loop {
+			if cfg!(fuzzing) { // fuzzing chacha20 doesn't use the key at all so we always get the same alias
+				outbound_scid_alias += 1;
+			} else {
+				outbound_scid_alias = fake_scid::Namespace::OutboundAlias.get_fake_scid(height, &self.genesis_hash, &self.fake_scid_rand_bytes, &self.keys_manager);
+			}
+			if outbound_scid_alias != 0 && self.outbound_scid_aliases.lock().unwrap().insert(outbound_scid_alias) {
+				break;
+			}
+			i += 1;
+			if i > 1_000_000 { panic!("Your RNG is busted or we ran out of possible outbound SCID aliases (which should never happen before we run out of memory to store channels"); }
+		}
+		outbound_scid_alias
+	}
+
 	/// Creates a new outbound channel to the given remote node and with the given value.
 	///
 	/// `user_channel_id` will be provided back as in
@@ -1801,11 +1860,20 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 			let per_peer_state = self.per_peer_state.read().unwrap();
 			match per_peer_state.get(&their_network_key) {
 				Some(peer_state) => {
+					let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
 					let peer_state = peer_state.lock().unwrap();
 					let their_features = &peer_state.latest_features;
 					let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
-					Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features,
-						channel_value_satoshis, push_msat, user_channel_id, config, self.best_block.read().unwrap().height())?
+					match Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key,
+						their_features, channel_value_satoshis, push_msat, user_channel_id, config,
+						self.best_block.read().unwrap().height(), outbound_scid_alias)
+					{
+						Ok(res) => res,
+						Err(e) => {
+							self.outbound_scid_aliases.lock().unwrap().remove(&outbound_scid_alias);
+							return Err(e);
+						},
+					}
 				},
 				None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
 			}
@@ -1943,7 +2011,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 							let (result, is_permanent) =
 								handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
 							if is_permanent {
-								remove_channel!(channel_state, chan_entry);
+								remove_channel!(self, channel_state, chan_entry);
 								break result;
 							}
 						}
@@ -1955,7 +2023,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					});
 
 					if chan_entry.get().is_shutdown() {
-						let channel = remove_channel!(channel_state, chan_entry);
+						let channel = remove_channel!(self, channel_state, chan_entry);
 						if let Ok(channel_update) = self.get_channel_update_for_broadcast(&channel) {
 							channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
 								msg: channel_update
@@ -2056,7 +2124,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 				} else {
 					self.issue_channel_close_events(chan.get(),ClosureReason::HolderForceClosed);
 				}
-				remove_channel!(channel_state, chan)
+				remove_channel!(self, channel_state, chan)
 			} else {
 				return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
 			}
@@ -3203,7 +3271,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 										}
 										ChannelError::Close(msg) => {
 											log_trace!(self.logger, "Closing channel {} due to Close-required error: {}", log_bytes!(chan.key()[..]), msg);
-											let mut channel = remove_channel!(channel_state, chan);
+											let mut channel = remove_channel!(self, channel_state, chan);
 											// ChannelClosed event is generated by handle_error for us.
 											Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel.channel_id(), channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
 										},
@@ -4215,13 +4283,24 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 			return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone()));
 		}
 
-		let mut channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(),
-				&their_features, msg, 0, &self.default_configuration, self.best_block.read().unwrap().height(), &self.logger)
-			.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
+		let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
+		let mut channel = match Channel::new_from_req(&self.fee_estimator, &self.keys_manager,
+			counterparty_node_id.clone(), &their_features, msg, 0, &self.default_configuration,
+			self.best_block.read().unwrap().height(), &self.logger, outbound_scid_alias)
+		{
+			Err(e) => {
+				self.outbound_scid_aliases.lock().unwrap().remove(&outbound_scid_alias);
+				return Err(MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id));
+			},
+			Ok(res) => res
+		};
 		let mut channel_state_lock = self.channel_state.lock().unwrap();
 		let channel_state = &mut *channel_state_lock;
 		match channel_state.by_id.entry(channel.channel_id()) {
-			hash_map::Entry::Occupied(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision!".to_owned(), msg.temporary_channel_id.clone())),
+			hash_map::Entry::Occupied(_) => {
+				self.outbound_scid_aliases.lock().unwrap().remove(&outbound_scid_alias);
+				return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision!".to_owned(), msg.temporary_channel_id.clone()))
+			},
 			hash_map::Entry::Vacant(entry) => {
 				if !self.default_configuration.manually_accept_inbound_channels {
 					channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
@@ -4425,7 +4504,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 							let (result, is_permanent) =
 								handle_monitor_err!(self, e, channel_state.short_to_id, chan_entry.get_mut(), RAACommitmentOrder::CommitmentFirst, chan_entry.key(), NO_UPDATE);
 							if is_permanent {
-								remove_channel!(channel_state, chan_entry);
+								remove_channel!(self, channel_state, chan_entry);
 								break result;
 							}
 						}
@@ -4473,7 +4552,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 						// also implies there are no pending HTLCs left on the channel, so we can
 						// fully delete it from tracking (the channel monitor is still around to
 						// watch for old state broadcasts)!
-						(tx, Some(remove_channel!(channel_state, chan_entry)))
+						(tx, Some(remove_channel!(self, channel_state, chan_entry)))
 					} else { (tx, None) }
 				},
 				hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
@@ -4913,7 +4992,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					let by_id = &mut channel_state.by_id;
 					let pending_msg_events = &mut channel_state.pending_msg_events;
 					if let hash_map::Entry::Occupied(chan_entry) = by_id.entry(funding_outpoint.to_channel_id()) {
-						let mut chan = remove_channel!(channel_state, chan_entry);
+						let mut chan = remove_channel!(self, channel_state, chan_entry);
 						failed_channels.push(chan.force_shutdown(false));
 						if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
 							pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
@@ -5052,7 +5131,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
 							log_info!(self.logger, "Broadcasting {}", log_tx!(tx));
 							self.tx_broadcaster.broadcast_transaction(&tx);
-							update_maps_on_chan_removal!(short_to_id, chan);
+							update_maps_on_chan_removal!(self, short_to_id, chan);
 							false
 						} else { true }
 					},
@@ -5249,7 +5328,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		let mut channel_state = self.channel_state.lock().unwrap();
 		let best_block = self.best_block.read().unwrap();
 		loop {
-			let scid_candidate = fake_scid::get_phantom_scid(&self.fake_scid_rand_bytes, best_block.height(), &self.genesis_hash, &self.keys_manager);
+			let scid_candidate = fake_scid::Namespace::Phantom.get_fake_scid(best_block.height(), &self.genesis_hash, &self.fake_scid_rand_bytes, &self.keys_manager);
 			// Ensure the generated scid doesn't conflict with a real channel.
 			match channel_state.short_to_id.entry(scid_candidate) {
 				hash_map::Entry::Occupied(_) => continue,
@@ -5537,10 +5616,7 @@ where
 						}));
 					}
 					if let Some(funding_locked) = funding_locked_opt {
-						pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
-							node_id: channel.get_counterparty_node_id(),
-							msg: funding_locked,
-						});
+						send_funding_locked!(short_to_id, pending_msg_events, channel, funding_locked);
 						if channel.is_usable() {
 							log_trace!(self.logger, "Sending funding_locked with private initial channel_update for our counterparty on channel {}", log_bytes!(channel.channel_id()));
 							pending_msg_events.push(events::MessageSendEvent::SendChannelUpdate {
@@ -5550,7 +5626,6 @@ where
 						} else {
 							log_trace!(self.logger, "Sending funding_locked WITHOUT channel_update for {}", log_bytes!(channel.channel_id()));
 						}
-						short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
 					}
 					if let Some(announcement_sigs) = announcement_sigs {
 						log_trace!(self.logger, "Sending announcement_signatures for channel {}", log_bytes!(channel.channel_id()));
@@ -5570,7 +5645,7 @@ where
 						}
 					}
 				} else if let Err(reason) = res {
-					update_maps_on_chan_removal!(short_to_id, channel);
+					update_maps_on_chan_removal!(self, short_to_id, channel);
 					// It looks like our counterparty went on-chain or funding transaction was
 					// reorged out of the main chain. Close the channel.
 					failed_channels.push(channel.force_shutdown(true));
@@ -5760,13 +5835,13 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
 		{
 			let mut channel_state_lock = self.channel_state.lock().unwrap();
 			let channel_state = &mut *channel_state_lock;
-			let short_to_id = &mut channel_state.short_to_id;
 			let pending_msg_events = &mut channel_state.pending_msg_events;
+			let short_to_id = &mut channel_state.short_to_id;
 			if no_connection_possible {
 				log_debug!(self.logger, "Failing all channels with {} due to no_connection_possible", log_pubkey!(counterparty_node_id));
 				channel_state.by_id.retain(|_, chan| {
 					if chan.get_counterparty_node_id() == *counterparty_node_id {
-						update_maps_on_chan_removal!(short_to_id, chan);
+						update_maps_on_chan_removal!(self, short_to_id, chan);
 						failed_channels.push(chan.force_shutdown(true));
 						if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
 							pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
@@ -5785,7 +5860,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
 					if chan.get_counterparty_node_id() == *counterparty_node_id {
 						chan.remove_uncommitted_htlcs_and_mark_paused(&self.logger);
 						if chan.is_shutdown() {
-							update_maps_on_chan_removal!(short_to_id, chan);
+							update_maps_on_chan_removal!(self, short_to_id, chan);
 							self.issue_channel_close_events(chan, ClosureReason::DisconnectedPeer);
 							return false;
 						} else {
@@ -6785,6 +6860,32 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 			}
 		}
 
+		let mut outbound_scid_aliases = HashSet::new();
+		for (chan_id, chan) in by_id.iter_mut() {
+			if chan.outbound_scid_alias() == 0 {
+				let mut outbound_scid_alias;
+				loop {
+					outbound_scid_alias = fake_scid::Namespace::OutboundAlias
+						.get_fake_scid(best_block_height, &genesis_hash, fake_scid_rand_bytes.as_ref().unwrap(), &args.keys_manager);
+					if outbound_scid_aliases.insert(outbound_scid_alias) { break; }
+				}
+				chan.set_outbound_scid_alias(outbound_scid_alias);
+			} else if !outbound_scid_aliases.insert(chan.outbound_scid_alias()) {
+				// Note that in rare cases its possible to hit this while reading an older
+				// channel if we just happened to pick a colliding outbound alias above.
+				log_error!(args.logger, "Got duplicate outbound SCID alias; {}", chan.outbound_scid_alias());
+				return Err(DecodeError::InvalidValue);
+			}
+			if chan.is_usable() {
+				if short_to_id.insert(chan.outbound_scid_alias(), *chan_id).is_some() {
+					// Note that in rare cases its possible to hit this while reading an older
+					// channel if we just happened to pick a colliding outbound alias above.
+					log_error!(args.logger, "Got duplicate outbound SCID alias; {}", chan.outbound_scid_alias());
+					return Err(DecodeError::InvalidValue);
+				}
+			}
+		}
+
 		let inbound_pmt_key_material = args.keys_manager.get_inbound_payment_key_material();
 		let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material);
 		let channel_manager = ChannelManager {
@@ -6805,6 +6906,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 			inbound_payment_key: expanded_inbound_key,
 			pending_inbound_payments: Mutex::new(pending_inbound_payments),
 			pending_outbound_payments: Mutex::new(pending_outbound_payments.unwrap()),
+
+			outbound_scid_aliases: Mutex::new(outbound_scid_aliases),
 			fake_scid_rand_bytes: fake_scid_rand_bytes.unwrap(),
 
 			our_network_key,
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index ed220a51887..598a51b0be6 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -5916,7 +5916,7 @@ fn bolt2_open_channel_sending_node_checks_part1() { //This test needs to be on i
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-	//Force duplicate channel ids
+	// Force duplicate randomness for every get-random call
 	for node in nodes.iter() {
 		*node.keys_manager.override_random_bytes.lock().unwrap() = Some([0; 32]);
 	}
@@ -5929,7 +5929,8 @@ fn bolt2_open_channel_sending_node_checks_part1() { //This test needs to be on i
 	nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &node0_to_1_send_open_channel);
 	get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
 
-	//Create a second channel with a channel_id collision
+	// Create a second channel with the same random values. This used to panic due to a colliding
+	// channel_id, but now panics due to a colliding outbound SCID alias.
 	assert!(nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), channel_value_satoshis, push_msat, 42, None).is_err());
 }
 
@@ -7179,7 +7180,10 @@ fn test_user_configurable_csv_delay() {
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
 	// We test config.our_to_self > BREAKDOWN_TIMEOUT is enforced in Channel::new_outbound()
-	if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), 1000000, 1000000, 0, &low_our_to_self_config, 0) {
+	if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) },
+		&nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), 1000000, 1000000, 0,
+		&low_our_to_self_config, 0, 42)
+	{
 		match error {
 			APIError::APIMisuseError { err } => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str())); },
 			_ => panic!("Unexpected event"),
@@ -7190,7 +7194,10 @@ fn test_user_configurable_csv_delay() {
 	nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
 	let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
 	open_channel.to_self_delay = 200;
-	if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &low_our_to_self_config, 0, &nodes[0].logger) {
+	if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) },
+		&nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0,
+		&low_our_to_self_config, 0, &nodes[0].logger, 42)
+	{
 		match error {
 			ChannelError::Close(err) => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str()));  },
 			_ => panic!("Unexpected event"),
@@ -7219,7 +7226,10 @@ fn test_user_configurable_csv_delay() {
 	nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
 	let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
 	open_channel.to_self_delay = 200;
-	if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &high_their_to_self_config, 0, &nodes[0].logger) {
+	if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) },
+		&nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0,
+		&high_their_to_self_config, 0, &nodes[0].logger, 42)
+	{
 		match error {
 			ChannelError::Close(err) => { assert!(regex::Regex::new(r"They wanted our payments to be delayed by a needlessly long period\. Upper limit: \d+\. Actual: \d+").unwrap().is_match(err.as_str())); },
 			_ => panic!("Unexpected event"),
diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index d67abeefdad..af82a275d9a 100644
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/lightning/src/ln/onion_route_tests.rs
@@ -319,10 +319,10 @@ fn test_onion_failure() {
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(node_2_cfg)]);
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+	let channels = [create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()), create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known())];
 	for node in nodes.iter() {
 		*node.keys_manager.override_random_bytes.lock().unwrap() = Some([3; 32]);
 	}
-	let channels = [create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()), create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known())];
 	let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], 40000);
 	// positive case
 	send_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 40000);
diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs
index f8a1a70e2d3..4fadfc915d4 100644
--- a/lightning/src/ln/priv_short_conf_tests.rs
+++ b/lightning/src/ln/priv_short_conf_tests.rs
@@ -237,3 +237,37 @@ fn test_1_conf_open() {
 	do_test_1_conf_open(ConnectStyle::TransactionsFirst);
 	do_test_1_conf_open(ConnectStyle::FullBlockViaListen);
 }
+
+#[test]
+fn test_routed_scid_alias() {
+	// Trivially test sending a payment which is routed through an SCID alias.
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let mut no_announce_cfg = test_default_channel_config();
+	no_announce_cfg.accept_forwards_to_priv_channels = true;
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(no_announce_cfg), None]);
+	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).2;
+	create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known());
+
+	let last_hop = nodes[2].node.list_usable_channels();
+	let hop_hints = vec![RouteHint(vec![RouteHintHop {
+		src_node_id: nodes[1].node.get_our_node_id(),
+		short_channel_id: last_hop[0].inbound_scid_alias.unwrap(),
+		fees: RoutingFees {
+			base_msat: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_base_msat,
+			proportional_millionths: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_proportional_millionths,
+		},
+		cltv_expiry_delta: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().cltv_expiry_delta,
+		htlc_maximum_msat: None,
+		htlc_minimum_msat: None,
+	}])];
+	let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], hop_hints, 100_000, 42);
+	assert_eq!(route.paths[0][1].short_channel_id, last_hop[0].inbound_scid_alias.unwrap());
+	nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
+	check_added_monitors!(nodes[0], 1);
+
+	pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], 100_000, payment_hash, payment_secret);
+	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
+}
diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index 6c39efb87b5..8eb39cfe92a 100644
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -201,7 +201,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_
 
 	let channel_state = nodes[0].node.channel_state.lock().unwrap();
 	assert_eq!(channel_state.by_id.len(), 1);
-	assert_eq!(channel_state.short_to_id.len(), 1);
+	assert_eq!(channel_state.short_to_id.len(), 2);
 	mem::drop(channel_state);
 
 	if !reorg_after_reload {
diff --git a/lightning/src/util/scid_utils.rs b/lightning/src/util/scid_utils.rs
index 11b8d0c95e0..8552358c35a 100644
--- a/lightning/src/util/scid_utils.rs
+++ b/lightning/src/util/scid_utils.rs
@@ -61,7 +61,7 @@ pub fn scid_from_parts(block: u64, tx_index: u64, vout_index: u64) -> Result<u64
 }
 
 /// LDK has multiple reasons to generate fake short channel ids:
-/// 1) zero-conf channels that don't have a confirmed channel id yet
+/// 1) outbound SCID aliases we use for private channels
 /// 2) phantom node payments, to get an scid for the phantom node's phantom channel
 pub(crate) mod fake_scid {
 	use bitcoin::hash_types::BlockHash;
@@ -84,9 +84,9 @@ pub(crate) mod fake_scid {
 	/// receipt, and handle the HTLC accordingly. The namespace identifier is encrypted when encoded
 	/// into the fake scid.
 	#[derive(Copy, Clone)]
-	pub(super) enum Namespace {
+	pub(crate) enum Namespace {
 		Phantom,
-		// Coming soon: a variant for the zero-conf scid namespace
+		OutboundAlias,
 	}
 
 	impl Namespace {
@@ -94,7 +94,7 @@ pub(crate) mod fake_scid {
 		/// between segwit activation and the current best known height, and the tx index and output
 		/// index are also selected from a "reasonable" range. We add this logic because it makes it
 		/// non-obvious at a glance that the scid is fake, e.g. if it appears in invoice route hints.
-		pub(super) fn get_fake_scid<Signer: Sign, K: Deref>(&self, highest_seen_blockheight: u32, genesis_hash: &BlockHash, fake_scid_rand_bytes: &[u8; 32], keys_manager: &K) -> u64
+		pub(crate) fn get_fake_scid<Signer: Sign, K: Deref>(&self, highest_seen_blockheight: u32, genesis_hash: &BlockHash, fake_scid_rand_bytes: &[u8; 32], keys_manager: &K) -> u64
 			where K::Target: KeysInterface<Signer = Signer>,
 		{
 			// Ensure we haven't created a namespace that doesn't fit into the 3 bits we've allocated for
@@ -138,13 +138,6 @@ pub(crate) mod fake_scid {
 		}
 	}
 
-	pub fn get_phantom_scid<Signer: Sign, K: Deref>(fake_scid_rand_bytes: &[u8; 32], highest_seen_blockheight: u32, genesis_hash: &BlockHash, keys_manager: &K) -> u64
-		where K::Target: KeysInterface<Signer = Signer>,
-	{
-		let namespace = Namespace::Phantom;
-		namespace.get_fake_scid(highest_seen_blockheight, genesis_hash, fake_scid_rand_bytes, keys_manager)
-	}
-
 	fn segwit_activation_height(genesis: &BlockHash) -> u32 {
 		const MAINNET_GENESIS_STR: &'static str = "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f";
 		if BlockHash::from_hex(MAINNET_GENESIS_STR).unwrap() == *genesis {

From e4486fe9f4cf682f8cf84f7b4eefe5515cbd3d4e Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Tue, 1 Feb 2022 17:37:28 +0000
Subject: [PATCH 9/9] Support receiving multiple funding_locked messages

As a part of adding SCID aliases to channels, we now have to accept
otherwise-redundant funding_locked messages which serve only to
update the SCID alias. Previously, we'd failt he channel as such
an update used to be bogus.
---
 lightning/src/ln/channel.rs               | 34 +++++++++++++++--------
 lightning/src/ln/priv_short_conf_tests.rs | 15 +++++++++-
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 65f1be00008..71e0ea121a9 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2188,17 +2188,28 @@ impl<Signer: Sign> Channel<Signer> {
 		} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
 			self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
 			self.update_time_counter += 1;
-		} else if (self.channel_state & (ChannelState::ChannelFunded as u32) != 0 &&
-				 // Note that funding_signed/funding_created will have decremented both by 1!
-				 self.cur_holder_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 &&
-				 self.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1) ||
-				// If we reconnected before sending our funding locked they may still resend theirs:
-				(self.channel_state & (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) ==
-				                      (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32)) {
-			if self.counterparty_cur_commitment_point != Some(msg.next_per_commitment_point) {
+		} else if self.channel_state & (ChannelState::ChannelFunded as u32) != 0 ||
+			// If we reconnected before sending our funding locked they may still resend theirs:
+			(self.channel_state & (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) ==
+			                      (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32))
+		{
+			// They probably disconnected/reconnected and re-sent the funding_locked, which is
+			// required, or they're sending a fresh SCID alias.
+			let expected_point =
+				if self.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 {
+					// If they haven't ever sent an updated point, the point they send should match
+					// the current one.
+					self.counterparty_cur_commitment_point
+				} else {
+					// If they have sent updated points, funding_locked is always supposed to match
+					// their "first" point, which we re-derive here.
+					Some(PublicKey::from_secret_key(&self.secp_ctx, &SecretKey::from_slice(
+							&self.commitment_secrets.get_secret(INITIAL_COMMITMENT_NUMBER - 1).expect("We should have all prev secrets available")
+						).expect("We already advanced, so previous secret keys should have been validated already")))
+				};
+			if expected_point != Some(msg.next_per_commitment_point) {
 				return Err(ChannelError::Close("Peer sent a reconnect funding_locked with a different point".to_owned()));
 			}
-			// They probably disconnected/reconnected and re-sent the funding_locked, which is required
 			return Ok(None);
 		} else {
 			return Err(ChannelError::Close("Peer sent a funding_locked at a strange time".to_owned()));
@@ -4238,7 +4249,7 @@ impl<Signer: Sign> Channel<Signer> {
 		self.outbound_scid_alias
 	}
 	/// Only allowed immediately after deserialization if get_outbound_scid_alias returns 0,
-	/// indicating we were written by an old LDK which did not set outbound SCID aliases.
+	/// indicating we were written by LDK prior to 0.0.106 which did not set outbound SCID aliases.
 	pub fn set_outbound_scid_alias(&mut self, outbound_scid_alias: u64) {
 		assert_eq!(self.outbound_scid_alias, 0);
 		self.outbound_scid_alias = outbound_scid_alias;
@@ -4492,7 +4503,8 @@ impl<Signer: Sign> Channel<Signer> {
 		if need_commitment_update {
 			if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
 				if self.channel_state & (ChannelState::PeerDisconnected as u32) == 0 {
-					let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
+					let next_per_commitment_point =
+						self.holder_signer.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &self.secp_ctx);
 					return Some(msgs::FundingLocked {
 						channel_id: self.channel_id,
 						next_per_commitment_point,
diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs
index 4fadfc915d4..a6f5a1b2ef7 100644
--- a/lightning/src/ln/priv_short_conf_tests.rs
+++ b/lightning/src/ln/priv_short_conf_tests.rs
@@ -249,7 +249,7 @@ fn test_routed_scid_alias() {
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).2;
-	create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known());
+	let mut as_funding_locked = create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 500_000_000, InitFeatures::known(), InitFeatures::known()).0;
 
 	let last_hop = nodes[2].node.list_usable_channels();
 	let hop_hints = vec![RouteHint(vec![RouteHintHop {
@@ -270,4 +270,17 @@ fn test_routed_scid_alias() {
 
 	pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], 100_000, payment_hash, payment_secret);
 	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
+
+	// Now test that if a peer sends us a second funding_locked after the channel is operational we
+	// will use the new alias.
+	as_funding_locked.short_channel_id_alias = Some(0xdeadbeef);
+	nodes[2].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &as_funding_locked);
+	// Note that we always respond to a funding_locked with a channel_update. Not a lot of reason
+	// to bother updating that code, so just drop the message here.
+	get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id());
+	let updated_channel_info = nodes[2].node.list_usable_channels();
+	assert_eq!(updated_channel_info.len(), 1);
+	assert_eq!(updated_channel_info[0].inbound_scid_alias.unwrap(), 0xdeadbeef);
+	// Note that because we never send a duplicate funding_locked we can't send a payment through
+	// the 0xdeadbeef SCID alias.
 }