From e4486fe9f4cf682f8cf84f7b4eefe5515cbd3d4e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 1 Feb 2022 17:37:28 +0000 Subject: [PATCH] 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 65f1be00..71e0ea12 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2188,17 +2188,28 @@ impl Channel { } 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 Channel { 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 Channel { 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 4fadfc91..a6f5a1b2 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. } -- 2.30.2