f - Check KeysInterface::get_shutdown_scriptpubkey

Check that the user-provided shutdown_scriptpubkey is compatible with
the counterparty's features (option_shutdown_anysegwit).

TODO: Implement for ChannelManager::close_channel
TODO: Add unit tests

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index b22f3756b0c63c95edd0e65699f9136874e8f71d..cfd0bec3d13cac4c8958ebf6ed247c7698d60ac4 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -567,7 +567,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        // Constructors:
-       pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, APIError>
+       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) -> Result<Channel<Signer>, APIError>
        where K::Target: KeysInterface<Signer = Signer>,
              F::Target: FeeEstimator,
        {
@@ -597,9 +597,13 @@ impl<Signer: Sign> Channel<Signer> {
 
                let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey {
                        Some(keys_provider.get_shutdown_scriptpubkey())
-               } else {
-                       None
-               };
+               } else { None };
+
+               if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey {
+                       if !shutdown_scriptpubkey.is_compatible(&their_features) {
+                               return Err(APIError::APIMisuseError { err: format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()) });
+                       }
+               }
 
                Ok(Channel {
                        user_id,
@@ -843,14 +847,18 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                } else { None };
 
-               let mut secp_ctx = Secp256k1::new();
-               secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
-
                let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey {
                        Some(keys_provider.get_shutdown_scriptpubkey())
-               } else {
-                       None
-               };
+               } else { None };
+
+               if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey {
+                       if !shutdown_scriptpubkey.is_compatible(&their_features) {
+                               return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex())));
+                       }
+               }
+
+               let mut secp_ctx = Secp256k1::new();
+               secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
 
                let chan = Channel {
                        user_id,
@@ -3262,6 +3270,23 @@ impl<Signer: Sign> Channel<Signer> {
                        self.counterparty_shutdown_scriptpubkey = Some(shutdown_scriptpubkey);
                }
 
+               // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc
+               // immediately after the commitment dance, but we can send a Shutdown cause we won't send
+               // any further commitment updates after we set LocalShutdownSent.
+               let send_shutdown = (self.channel_state & ChannelState::LocalShutdownSent as u32) != ChannelState::LocalShutdownSent as u32;
+
+               let shutdown_scriptpubkey = match self.shutdown_scriptpubkey {
+                       Some(_) => None,
+                       None => {
+                               assert!(send_shutdown);
+                               let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey();
+                               if !shutdown_scriptpubkey.is_compatible(their_features) {
+                                       return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex())));
+                               }
+                               Some(shutdown_scriptpubkey)
+                       },
+               };
+
                // From here on out, we may not fail!
 
                self.channel_state |= ChannelState::RemoteShutdownSent as u32;
@@ -3282,15 +3307,9 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                });
 
-               // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc
-               // immediately after the commitment dance, but we can send a Shutdown cause we won't send
-               // any further commitment updates after we set LocalShutdownSent.
-               let send_shutdown = (self.channel_state & ChannelState::LocalShutdownSent as u32) != ChannelState::LocalShutdownSent as u32;
-               let monitor_update = match self.shutdown_scriptpubkey {
-                       Some(_) => None,
-                       None => {
-                               assert!(send_shutdown);
-                               self.shutdown_scriptpubkey = Some(keys_provider.get_shutdown_scriptpubkey());
+               let monitor_update = match shutdown_scriptpubkey {
+                       Some(shutdown_scriptpubkey) => {
+                               self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey);
                                self.latest_monitor_update_id += 1;
                                Some(ChannelMonitorUpdate {
                                        update_id: self.latest_monitor_update_id,
@@ -3299,6 +3318,7 @@ impl<Signer: Sign> Channel<Signer> {
                                        }],
                                })
                        },
+                       None => None,
                };
                let shutdown = if send_shutdown {
                        Some(msgs::Shutdown {
@@ -5212,7 +5232,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, 10000000, 100000, 42, &config).unwrap();
+               let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
 
                // Now change the fee so we can check that the fee in the open_channel message is the
                // same as the old fee.
@@ -5237,7 +5257,7 @@ 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, 10000000, 100000, 42, &config).unwrap();
+               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
 
                // Create Node B's channel by receiving Node A's open_channel message
                // Make sure A's dust limit is as we expect.
@@ -5304,7 +5324,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, 10000000, 100000, 42, &config).unwrap();
+               let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
 
                let commitment_tx_fee_0_htlcs = chan.commit_tx_fee_msat(0);
                let commitment_tx_fee_1_htlc = chan.commit_tx_fee_msat(1);
@@ -5353,7 +5373,7 @@ 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, 10000000, 100000, 42, &config).unwrap();
+               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).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);
@@ -5415,7 +5435,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, 10000000, 100000, 42, &config).unwrap();
+               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).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());
@@ -5479,7 +5499,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, 10_000_000, 100000, 42, &config).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).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 0ce337f608d32706b601ad53bb2f2012cbb0e85c..971b4bc22ac2964657ee0d3eda26c41f538e80ce 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1156,8 +1156,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
                }
 
+               let their_features = {
+                       let per_peer_state = self.per_peer_state.read().unwrap();
+                       let peer_state = per_peer_state.get(&their_network_key).unwrap().lock().unwrap();
+                       peer_state.latest_features.clone()
+               };
                let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
-               let channel = Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, channel_value_satoshis, push_msat, user_id, config)?;
+               let channel = Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_id, config)?;
                let res = channel.get_open_channel(self.genesis_hash.clone());
 
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 21f871fa8d3376405c973e3cd4b7a5607a368171..32eea83b3c64f6f503ca1faa54f09c9fec5e56c8 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -7749,7 +7749,7 @@ 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(), 1000000, 1000000, 0, &low_our_to_self_config) {
+       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) {
                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"),