From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue, 10 Aug 2021 22:11:18 +0000 (+0000) Subject: Merge pull request #1009 from ariard/2021-07-add-forward-dust-limit X-Git-Tag: v0.0.100~4 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=9d8d24f6906d6fbdc6c02a88e5e2298c1fa50825;hp=-c;p=rust-lightning Merge pull request #1009 from ariard/2021-07-add-forward-dust-limit Add new config setting `max_balance_dust_htlc_msat` --- 9d8d24f6906d6fbdc6c02a88e5e2298c1fa50825 diff --combined fuzz/src/chanmon_consistency.rs index ef9ce82d,36d12ad5..4ce0df09 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@@ -39,7 -39,6 +39,7 @@@ use lightning::ln::{PaymentHash, Paymen use lightning::ln::channelmanager::{ChainParameters, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs}; use lightning::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures}; use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, DecodeError, UpdateAddHTLC, Init}; +use lightning::ln::script::ShutdownScript; use lightning::util::enforcing_trait_impls::{EnforcingSigner, INITIAL_REVOKED_COMMITMENT_NUMBER}; use lightning::util::errors::APIError; use lightning::util::events; @@@ -165,11 -164,9 +165,11 @@@ impl KeysInterface for KeyProvider Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_monitor_claim_key_hash[..]).into_script() } - fn get_shutdown_pubkey(&self) -> PublicKey { + fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { let secp_ctx = Secp256k1::signing_only(); - PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, self.node_id]).unwrap()) + let secret_key = SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, self.node_id]).unwrap(); + let pubkey_hash = WPubkeyHash::hash(&PublicKey::from_secret_key(&secp_ctx, &secret_key).serialize()); + ShutdownScript::new_p2wpkh(&pubkey_hash) } fn get_channel_signer(&self, _inbound: bool, channel_value_satoshis: u64) -> EnforcingSigner { @@@ -247,13 -244,13 +247,14 @@@ fn check_api_err(api_err: APIError) _ if err.starts_with("Cannot send value that would put counterparty balance under holder-announced channel reserve value") => {}, _ if err.starts_with("Cannot send value that would overdraw remaining funds.") => {}, _ if err.starts_with("Cannot send value that would not leave enough to pay for fees.") => {}, + _ if err.starts_with("Cannot send value that would put our exposure to dust HTLCs at") => {}, _ => panic!("{}", err), } }, APIError::MonitorUpdateFailed => { // We can (obviously) temp-fail a monitor update }, + APIError::IncompatibleShutdownScript { .. } => panic!("Cannot send an incompatible shutdown script"), } } #[inline] @@@ -397,9 -394,6 +398,9 @@@ pub fn do_test { { + $source.peer_connected(&$dest.get_our_node_id(), &Init { features: InitFeatures::known() }); + $dest.peer_connected(&$source.get_our_node_id(), &Init { features: InitFeatures::known() }); + $source.create_channel($dest.get_our_node_id(), 100_000, 42, 0, None).unwrap(); let open_channel = { let events = $source.get_and_clear_pending_msg_events(); diff --combined lightning/src/ln/channel.rs index b678f0f4,d14b4fa9..e685c15d --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@@ -9,15 -9,15 +9,15 @@@ use bitcoin::blockdata::script::{Script,Builder}; use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType}; -use bitcoin::blockdata::opcodes; use bitcoin::util::bip143; use bitcoin::consensus::encode; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::sha256d::Hash as Sha256d; -use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash}; +use bitcoin::hash_types::{Txid, BlockHash}; +use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE; use bitcoin::secp256k1::key::{PublicKey,SecretKey}; use bitcoin::secp256k1::{Secp256k1,Signature}; use bitcoin::secp256k1; @@@ -26,7 -26,6 +26,7 @@@ use ln::{PaymentPreimage, PaymentHash} use ln::features::{ChannelFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; +use ln::script::ShutdownScript; use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor}; use ln::chan_utils; @@@ -45,11 -44,11 +45,11 @@@ use util::scid_utils::scid_from_parts use io; use prelude::*; use core::{cmp,mem,fmt}; +use core::convert::TryFrom; use core::ops::Deref; #[cfg(any(test, feature = "fuzztarget", debug_assertions))] use sync::Mutex; use bitcoin::hashes::hex::ToHex; -use bitcoin::blockdata::opcodes::all::OP_PUSHBYTES_0; #[cfg(test)] pub struct ChannelValueStat { @@@ -275,6 -274,14 +275,14 @@@ enum HTLCInitiator RemoteOffered, } + /// An enum gathering stats on pending HTLCs, either inbound or outbound side. + struct HTLCStats { + pending_htlcs: u32, + pending_htlcs_value_msat: u64, + on_counterparty_tx_dust_exposure_msat: u64, + on_holder_tx_dust_exposure_msat: u64, + } + /// Used when calculating whether we or the remote can afford an additional HTLC. struct HTLCCandidate { amount_msat: u64, @@@ -356,7 -363,7 +364,7 @@@ pub(super) struct Channel, destination_script: Script, // Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction @@@ -574,7 -581,7 +582,7 @@@ impl Channel } // Constructors: - pub fn new_outbound(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result, APIError> + pub fn new_outbound(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, APIError> where K::Target: KeysInterface, F::Target: FeeEstimator, { @@@ -602,16 -609,6 +610,16 @@@ 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 }; + + if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey { + if !shutdown_scriptpubkey.is_compatible(&their_features) { + return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() }); + } + } + Ok(Channel { user_id, config: config.channel_options.clone(), @@@ -624,7 -621,7 +632,7 @@@ latest_monitor_update_id: 0, holder_signer, - shutdown_pubkey: keys_provider.get_shutdown_pubkey(), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@@ -720,7 -717,7 +728,7 @@@ /// Creates a new channel from a remote sides' request for one. /// Assumes chain_hash has already been checked and corresponds with what we expect! - pub fn new_from_req(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig) -> Result, ChannelError> + pub fn new_from_req(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig) -> Result, ChannelError> where K::Target: KeysInterface, F::Target: FeeEstimator { @@@ -840,11 -837,11 +848,11 @@@ // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything if script.len() == 0 { None - // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel - } else if is_unsupported_shutdown_script(&their_features, script) { - return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))); } else { - Some(script.clone()) + match ShutdownScript::try_from((script.clone(), their_features)) { + Ok(shutdown_script) => Some(shutdown_script.into_inner()), + Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))), + } } }, // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel @@@ -854,16 -851,6 +862,16 @@@ } } else { None }; + let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + Some(keys_provider.get_shutdown_scriptpubkey()) + } 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: {}", shutdown_scriptpubkey))); + } + } + let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); @@@ -878,7 -865,7 +886,7 @@@ latest_monitor_update_id: 0, holder_signer, - shutdown_pubkey: keys_provider.get_shutdown_pubkey(), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@@ -1157,10 -1144,8 +1165,10 @@@ #[inline] fn get_closing_scriptpubkey(&self) -> Script { - let channel_close_key_hash = WPubkeyHash::hash(&self.shutdown_pubkey.serialize()); - Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_close_key_hash[..]).into_script() + // The shutdown scriptpubkey is set on channel opening when option_upfront_shutdown_script + // is signaled. Otherwise, it is set when sending a shutdown message. Calling this method + // outside of those situations will fail. + self.shutdown_scriptpubkey.clone().unwrap().into_inner() } #[inline] @@@ -1226,7 -1211,6 +1234,7 @@@ }, ())); } + assert!(self.shutdown_scriptpubkey.is_some()); if value_to_self as u64 > self.holder_dust_limit_satoshis { txouts.push((TxOut { script_pubkey: self.get_closing_scriptpubkey(), @@@ -1513,7 -1497,7 +1521,7 @@@ // Message handlers: - pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: InitFeatures) -> Result<(), ChannelError> { + pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: &InitFeatures) -> Result<(), ChannelError> { // Check sanity of message fields: if !self.is_outbound() { return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned())); @@@ -1584,11 -1568,11 +1592,11 @@@ // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything if script.len() == 0 { None - // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel - } else if is_unsupported_shutdown_script(&their_features, script) { - return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))); } else { - Some(script.clone()) + match ShutdownScript::try_from((script.clone(), their_features)) { + Ok(shutdown_script) => Some(shutdown_script.into_inner()), + Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))), + } } }, // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel @@@ -1710,9 -1694,8 +1718,9 @@@ let funding_redeemscript = self.get_funding_redeemscript(); let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()); + let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(), - &self.shutdown_pubkey, self.get_holder_selected_contest_delay(), + shutdown_script, self.get_holder_selected_contest_delay(), &self.destination_script, (funding_txo, funding_txo_script.clone()), &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, @@@ -1784,9 -1767,8 +1792,9 @@@ let funding_txo = self.get_funding_txo().unwrap(); let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()); + let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(), - &self.shutdown_pubkey, self.get_holder_selected_contest_delay(), + shutdown_script, self.get_holder_selected_contest_delay(), &self.destination_script, (funding_txo, funding_txo_script), &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, @@@ -1842,32 -1824,63 +1850,63 @@@ Ok(()) } - /// Returns (inbound_htlc_count, htlc_inbound_value_msat) - fn get_inbound_pending_htlc_stats(&self) -> (u32, u64) { - let mut htlc_inbound_value_msat = 0; + /// Returns a HTLCStats about inbound pending htlcs + fn get_inbound_pending_htlc_stats(&self) -> HTLCStats { + let mut stats = HTLCStats { + pending_htlcs: self.pending_inbound_htlcs.len() as u32, + pending_htlcs_value_msat: 0, + on_counterparty_tx_dust_exposure_msat: 0, + on_holder_tx_dust_exposure_msat: 0, + }; + + let counterparty_dust_limit_timeout_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + let holder_dust_limit_success_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; for ref htlc in self.pending_inbound_htlcs.iter() { - htlc_inbound_value_msat += htlc.amount_msat; + stats.pending_htlcs_value_msat += htlc.amount_msat; + if htlc.amount_msat / 1000 < counterparty_dust_limit_timeout_sat { + stats.on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; + } + if htlc.amount_msat / 1000 < holder_dust_limit_success_sat { + stats.on_holder_tx_dust_exposure_msat += htlc.amount_msat; + } } - (self.pending_inbound_htlcs.len() as u32, htlc_inbound_value_msat) + stats } - /// Returns (outbound_htlc_count, htlc_outbound_value_msat) *including* pending adds in our - /// holding cell. - fn get_outbound_pending_htlc_stats(&self) -> (u32, u64) { - let mut htlc_outbound_value_msat = 0; + /// Returns a HTLCStats about pending outbound htlcs, *including* pending adds in our holding cell. + fn get_outbound_pending_htlc_stats(&self) -> HTLCStats { + let mut stats = HTLCStats { + pending_htlcs: self.pending_outbound_htlcs.len() as u32, + pending_htlcs_value_msat: 0, + on_counterparty_tx_dust_exposure_msat: 0, + on_holder_tx_dust_exposure_msat: 0, + }; + + let counterparty_dust_limit_success_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + let holder_dust_limit_timeout_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; for ref htlc in self.pending_outbound_htlcs.iter() { - htlc_outbound_value_msat += htlc.amount_msat; + stats.pending_htlcs_value_msat += htlc.amount_msat; + if htlc.amount_msat / 1000 < counterparty_dust_limit_success_sat { + stats.on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; + } + if htlc.amount_msat / 1000 < holder_dust_limit_timeout_sat { + stats.on_holder_tx_dust_exposure_msat += htlc.amount_msat; + } } - let mut htlc_outbound_count = self.pending_outbound_htlcs.len(); for update in self.holding_cell_htlc_updates.iter() { if let &HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, .. } = update { - htlc_outbound_count += 1; - htlc_outbound_value_msat += amount_msat; + stats.pending_htlcs += 1; + stats.pending_htlcs_value_msat += amount_msat; + if *amount_msat / 1000 < counterparty_dust_limit_success_sat { + stats.on_counterparty_tx_dust_exposure_msat += amount_msat; + } + if *amount_msat / 1000 < holder_dust_limit_timeout_sat { + stats.on_holder_tx_dust_exposure_msat += amount_msat; + } } } - - (htlc_outbound_count as u32, htlc_outbound_value_msat) + stats } /// Get the available (ie not including pending HTLCs) inbound and outbound balance in msat. @@@ -1879,11 -1892,11 +1918,11 @@@ ( cmp::max(self.channel_value_satoshis as i64 * 1000 - self.value_to_self_msat as i64 - - self.get_inbound_pending_htlc_stats().1 as i64 + - self.get_inbound_pending_htlc_stats().pending_htlcs_value_msat as i64 - Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64 * 1000, 0) as u64, cmp::max(self.value_to_self_msat as i64 - - self.get_outbound_pending_htlc_stats().1 as i64 + - self.get_outbound_pending_htlc_stats().pending_htlcs_value_msat as i64 - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, 0) as u64 ) @@@ -2095,12 -2108,13 +2134,13 @@@ return Err(ChannelError::Close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.holder_htlc_minimum_msat, msg.amount_msat))); } - let (inbound_htlc_count, htlc_inbound_value_msat) = self.get_inbound_pending_htlc_stats(); - if inbound_htlc_count + 1 > OUR_MAX_HTLCS as u32 { + let inbound_stats = self.get_inbound_pending_htlc_stats(); + let outbound_stats = self.get_outbound_pending_htlc_stats(); + if inbound_stats.pending_htlcs + 1 > OUR_MAX_HTLCS as u32 { return Err(ChannelError::Close(format!("Remote tried to push more than our max accepted HTLCs ({})", OUR_MAX_HTLCS))); } let holder_max_htlc_value_in_flight_msat = Channel::::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis); - if htlc_inbound_value_msat + msg.amount_msat > holder_max_htlc_value_in_flight_msat { + if inbound_stats.pending_htlcs_value_msat + msg.amount_msat > holder_max_htlc_value_in_flight_msat { return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", holder_max_htlc_value_in_flight_msat))); } // Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet @@@ -2124,8 -2138,28 +2164,28 @@@ } } + let exposure_dust_limit_timeout_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + if msg.amount_msat / 1000 < exposure_dust_limit_timeout_sats { + let on_counterparty_tx_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + msg.amount_msat; + if on_counterparty_tx_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", + on_counterparty_tx_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()); + pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7); + } + } + + let exposure_dust_limit_success_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; + if msg.amount_msat / 1000 < exposure_dust_limit_success_sats { + let on_holder_tx_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + msg.amount_msat; + if on_holder_tx_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", + on_holder_tx_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()); + pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7); + } + } + let pending_value_to_self_msat = - self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat; + self.value_to_self_msat + inbound_stats.pending_htlcs_value_msat - removed_outbound_total_msat; let pending_remote_value_msat = self.channel_value_satoshis * 1000 - pending_value_to_self_msat; if pending_remote_value_msat < msg.amount_msat { @@@ -3117,7 -3151,6 +3177,7 @@@ self.channel_state &= !(ChannelState::PeerDisconnected as u32); let shutdown_msg = if self.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 { + assert!(self.shutdown_scriptpubkey.is_some()); Some(msgs::Shutdown { channel_id: self.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), @@@ -3229,7 -3262,6 +3289,7 @@@ if self.feerate_per_kw > proposed_feerate { proposed_feerate = self.feerate_per_kw; } + assert!(self.shutdown_scriptpubkey.is_some()); let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap())); let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000; @@@ -3248,12 -3280,8 +3308,12 @@@ }) } - pub fn shutdown(&mut self, fee_estimator: &F, their_features: &InitFeatures, msg: &msgs::Shutdown) -> Result<(Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> - where F::Target: FeeEstimator + pub fn shutdown( + &mut self, fee_estimator: &F, keys_provider: &K, their_features: &InitFeatures, msg: &msgs::Shutdown + ) -> Result<(Option, Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> + where + F::Target: FeeEstimator, + K::Target: KeysInterface { if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 { return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish".to_owned())); @@@ -3271,58 -3299,23 +3331,58 @@@ } assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0); - if is_unsupported_shutdown_script(&their_features, &msg.scriptpubkey) { - return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex()))); - } + let shutdown_scriptpubkey = match ShutdownScript::try_from((msg.scriptpubkey.clone(), their_features)) { + Ok(script) => script.into_inner(), + Err(_) => return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex()))), + }; if self.counterparty_shutdown_scriptpubkey.is_some() { - if Some(&msg.scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() { - return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", msg.scriptpubkey.to_bytes().to_hex()))); + if Some(&shutdown_scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() { + return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", shutdown_scriptpubkey.to_bytes().to_hex()))); } } else { - self.counterparty_shutdown_scriptpubkey = Some(msg.scriptpubkey.clone()); + self.counterparty_shutdown_scriptpubkey = Some(shutdown_scriptpubkey); } + // If we have any LocalAnnounced updates we'll probably just get back an update_fail_htlc + // immediately after the commitment dance, but we can send a Shutdown because 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 update_shutdown_script = match self.shutdown_scriptpubkey { + Some(_) => false, + 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: {}", shutdown_scriptpubkey))); + } + self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + true + }, + }; + // From here on out, we may not fail! self.channel_state |= ChannelState::RemoteShutdownSent as u32; self.update_time_counter += 1; + let monitor_update = if update_shutdown_script { + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + } else { None }; + let shutdown = if send_shutdown { + Some(msgs::Shutdown { + channel_id: self.channel_id, + scriptpubkey: self.get_closing_scriptpubkey(), + }) + } else { None }; + // We can't send our shutdown until we've committed all of our pending HTLCs, but the // remote side is unlikely to accept any new HTLCs, so we go ahead and "free" any holding // cell HTLCs and return them to fail the payment. @@@ -3337,11 -3330,23 +3397,11 @@@ _ => true } }); - // 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 shutdown = if (self.channel_state & ChannelState::LocalShutdownSent as u32) == ChannelState::LocalShutdownSent as u32 { - None - } else { - Some(msgs::Shutdown { - channel_id: self.channel_id, - scriptpubkey: self.get_closing_scriptpubkey(), - }) - }; self.channel_state |= ChannelState::LocalShutdownSent as u32; self.update_time_counter += 1; - Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs)) + Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, dropped_outbound_htlcs)) } fn build_signed_closing_transaction(&self, tx: &mut Transaction, counterparty_sig: &Signature, sig: &Signature) { @@@ -3416,7 -3421,6 +3476,7 @@@ macro_rules! propose_new_feerate { ($new_feerate: expr) => { + assert!(self.shutdown_scriptpubkey.is_some()); let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap())); let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * tx_weight / 1000, false); let sig = self.holder_signer @@@ -3558,11 -3562,24 +3618,24 @@@ cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) } + pub fn get_max_dust_htlc_exposure_msat(&self) -> u64 { + self.config.max_dust_htlc_exposure_msat + } + #[cfg(test)] pub fn get_feerate(&self) -> u32 { self.feerate_per_kw } + pub fn get_dust_buffer_feerate(&self) -> u32 { + // When calculating our exposure to dust HTLCs, we assume that the channel feerate + // may, at any point, increase by at least 10 sat/vB (i.e 2530 sat/kWU) or 25%, + // whichever is higher. This ensures that we aren't suddenly exposed to significantly + // more dust balance if the feerate increases when we have several HTLCs pending + // which are near the dust limit. + cmp::max(2530, self.feerate_per_kw * 1250 / 1000) + } + pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 { self.cur_holder_commitment_transaction_number + 1 } @@@ -3915,10 -3932,7 +3988,10 @@@ htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, channel_flags: if self.config.announced_channel {1} else {0}, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } @@@ -3951,10 -3965,7 +4024,10 @@@ delayed_payment_basepoint: keys.delayed_payment_basepoint, htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } @@@ -4207,12 -4218,13 +4280,13 @@@ return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned())); } - let (outbound_htlc_count, htlc_outbound_value_msat) = self.get_outbound_pending_htlc_stats(); - if outbound_htlc_count + 1 > self.counterparty_max_accepted_htlcs as u32 { + let inbound_stats = self.get_inbound_pending_htlc_stats(); + let outbound_stats = self.get_outbound_pending_htlc_stats(); + if outbound_stats.pending_htlcs + 1 > self.counterparty_max_accepted_htlcs as u32 { return Err(ChannelError::Ignore(format!("Cannot push more than their max accepted HTLCs ({})", self.counterparty_max_accepted_htlcs))); } // Check their_max_htlc_value_in_flight_msat - if htlc_outbound_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_msat { + if outbound_stats.pending_htlcs_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_msat { return Err(ChannelError::Ignore(format!("Cannot send value that would put us over the max HTLC value in flight our peer will accept ({})", self.counterparty_max_htlc_value_in_flight_msat))); } @@@ -4227,7 -4239,25 +4301,25 @@@ } } - let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat; + let exposure_dust_limit_success_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + if amount_msat / 1000 < exposure_dust_limit_success_sats { + let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + amount_msat; + if on_counterparty_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", + on_counterparty_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); + } + } + + let exposure_dust_limit_timeout_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; + if amount_msat / 1000 < exposure_dust_limit_timeout_sats { + let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + amount_msat; + if on_holder_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", + on_holder_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); + } + } + + let pending_value_to_self_msat = self.value_to_self_msat - outbound_stats.pending_htlcs_value_msat; if pending_value_to_self_msat < amount_msat { return Err(ChannelError::Ignore(format!("Cannot send value that would overdraw remaining funds. Amount: {}, pending value to self {}", amount_msat, pending_value_to_self_msat))); } @@@ -4462,8 -4492,7 +4554,8 @@@ /// Begins the shutdown process, getting a message for the remote peer and returning all /// holding cell HTLCs for payment failure. - pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, PaymentHash)>), APIError> { + pub fn get_shutdown(&mut self, keys_provider: &K, their_features: &InitFeatures) -> Result<(msgs::Shutdown, Option, Vec<(HTLCSource, PaymentHash)>), APIError> + where K::Target: KeysInterface { for htlc in self.pending_outbound_htlcs.iter() { if let OutboundHTLCState::LocalAnnounced(_) = htlc.state { return Err(APIError::APIMisuseError{err: "Cannot begin shutdown with pending HTLCs. Process pending events first".to_owned()}); @@@ -4482,17 -4511,7 +4574,17 @@@ return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()}); } - let closing_script = self.get_closing_scriptpubkey(); + let update_shutdown_script = match self.shutdown_scriptpubkey { + Some(_) => false, + None => { + let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey(); + if !shutdown_scriptpubkey.is_compatible(their_features) { + return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() }); + } + self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + true + }, + }; // From here on out, we may not fail! if self.channel_state < ChannelState::FundingSent as u32 { @@@ -4502,20 -4521,6 +4594,20 @@@ } self.update_time_counter += 1; + let monitor_update = if update_shutdown_script { + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + } else { None }; + let shutdown = msgs::Shutdown { + channel_id: self.channel_id, + scriptpubkey: self.get_closing_scriptpubkey(), + }; + // Go ahead and drop holding cell updates as we'd rather fail payments than wait to send // our shutdown until we've committed all of the pending changes. self.holding_cell_update_fee = None; @@@ -4530,7 -4535,10 +4622,7 @@@ } }); - Ok((msgs::Shutdown { - channel_id: self.channel_id, - scriptpubkey: closing_script, - }, dropped_outbound_htlcs)) + Ok((shutdown, monitor_update, dropped_outbound_htlcs)) } /// Gets the latest commitment transaction and any dependent transactions for relay (forcing @@@ -4579,6 -4587,24 +4671,6 @@@ } } -fn is_unsupported_shutdown_script(their_features: &InitFeatures, script: &Script) -> bool { - // We restrain shutdown scripts to standards forms to avoid transactions not propagating on the p2p tx-relay network - - // BOLT 2 says we must only send a scriptpubkey of certain standard forms, - // which for a a BIP-141-compliant witness program is at max 42 bytes in length. - // So don't let the remote peer feed us some super fee-heavy script. - let is_script_too_long = script.len() > 42; - if is_script_too_long { - return true; - } - - if their_features.supports_shutdown_anysegwit() && script.is_witness_program() && script.as_bytes()[0] != OP_PUSHBYTES_0.into_u8() { - return false; - } - - return !script.is_p2pkh() && !script.is_p2sh() && !script.is_v0_p2wpkh() && !script.is_v0_p2wsh() -} - const SERIALIZATION_VERSION: u8 = 2; const MIN_SERIALIZATION_VERSION: u8 = 1; @@@ -4642,12 -4668,7 +4734,12 @@@ impl Writeable for Channe (key_data.0.len() as u32).write(writer)?; writer.write_all(&key_data.0[..])?; - self.shutdown_pubkey.write(writer)?; + // Write out the old serialization for shutdown_pubkey for backwards compatibility, if + // deserialized from that format. + match self.shutdown_scriptpubkey.as_ref().and_then(|script| script.as_legacy_pubkey()) { + Some(shutdown_pubkey) => shutdown_pubkey.write(writer)?, + None => [0u8; PUBLIC_KEY_SIZE].write(writer)?, + } self.destination_script.write(writer)?; self.cur_holder_commitment_transaction_number.write(writer)?; @@@ -4843,7 -4864,6 +4935,7 @@@ (1, self.minimum_depth, option), (3, self.counterparty_selected_channel_reserve_satoshis, option), (5, self.config, required), + (7, self.shutdown_scriptpubkey, option), }); Ok(()) @@@ -4887,11 -4907,7 +4979,11 @@@ impl<'a, Signer: Sign, K: Deref> Readab } let holder_signer = keys_source.read_chan_signer(&keys_data)?; - let shutdown_pubkey = Readable::read(reader)?; + // Read the old serialization for shutdown_pubkey, preferring the TLV field later if set. + let mut shutdown_scriptpubkey = match ::read(reader) { + Ok(pubkey) => Some(ShutdownScript::new_p2wpkh_from_pubkey(pubkey)), + Err(_) => None, + }; let destination_script = Readable::read(reader)?; let cur_holder_commitment_transaction_number = Readable::read(reader)?; @@@ -5062,7 -5078,6 +5154,7 @@@ (1, minimum_depth, option), (3, counterparty_selected_channel_reserve_satoshis, option), (5, config, option), // Note that if none is provided we will *not* overwrite the existing one. + (7, shutdown_scriptpubkey, option), }); let mut secp_ctx = Secp256k1::new(); @@@ -5080,7 -5095,7 +5172,7 @@@ latest_monitor_update_id, holder_signer, - shutdown_pubkey, + shutdown_scriptpubkey, destination_script, cur_holder_commitment_transaction_number, @@@ -5173,7 -5188,6 +5265,7 @@@ mod tests use ln::channel::MAX_FUNDING_SATOSHIS; use ln::features::InitFeatures; use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate}; + use ln::script::ShutdownScript; use ln::chan_utils; use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT}; use chain::BestBlock; @@@ -5182,9 -5196,7 +5274,9 @@@ use chain::transaction::OutPoint; use util::config::UserConfig; use util::enforcing_trait_impls::EnforcingSigner; + use util::errors::APIError; use util::test_utils; + use util::test_utils::OnGetShutdownScriptpubkey; use util::logger::Logger; use bitcoin::secp256k1::{Secp256k1, Message, Signature, All}; use bitcoin::secp256k1::ffi::Signature as FFISignature; @@@ -5193,7 -5205,6 +5285,7 @@@ use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; use bitcoin::hash_types::{Txid, WPubkeyHash}; + use core::num::NonZeroU8; use sync::Arc; use prelude::*; @@@ -5226,10 -5237,10 +5318,10 @@@ Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_monitor_claim_key_hash[..]).into_script() } - fn get_shutdown_pubkey(&self) -> PublicKey { + fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { let secp_ctx = Secp256k1::signing_only(); let channel_close_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(); - PublicKey::from_secret_key(&secp_ctx, &channel_close_key) + ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key)) } fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> InMemorySigner { @@@ -5244,32 -5255,6 +5336,32 @@@ PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode(hex).unwrap()[..]).unwrap()) } + #[test] + fn upfront_shutdown_script_incompatibility() { + let features = InitFeatures::known().clear_shutdown_anysegwit(); + let non_v0_segwit_shutdown_script = + ShutdownScript::new_witness_program(NonZeroU8::new(16).unwrap(), &[0, 40]).unwrap(); + + let seed = [42; 32]; + let network = Network::Testnet; + let keys_provider = test_utils::TestKeysInterface::new(&seed, network); + keys_provider.expect(OnGetShutdownScriptpubkey { + returns: non_v0_segwit_shutdown_script.clone(), + }); + + let fee_estimator = TestFeeEstimator { fee_est: 253 }; + 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::::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config) { + Err(APIError::IncompatibleShutdownScript { script }) => { + assert_eq!(script.into_inner(), non_v0_segwit_shutdown_script.into_inner()); + }, + Err(e) => panic!("Unexpected error: {:?}", e), + Ok(_) => panic!("Expected error"), + } + } + // Check that, during channel creation, we use the same feerate in the open channel message // as we do in the Channel object creation itself. #[test] @@@ -5283,7 -5268,7 +5375,7 @@@ 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::::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, 10000000, 100000, 42, &config).unwrap(); + let node_a_chan = Channel::::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. @@@ -5308,18 -5293,18 +5400,18 @@@ // 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::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::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. 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 node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); + let node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); // Node B --> Node A: accept channel, explicitly setting B's dust limit. let mut accept_channel_msg = node_b_chan.get_accept_channel(); accept_channel_msg.dust_limit_satoshis = 546; - node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap(); + node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap(); node_a_chan.holder_dust_limit_satoshis = 1560; // Put some inbound and outbound HTLCs in A's channel. @@@ -5375,7 -5360,7 +5467,7 @@@ let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_id, 10000000, 100000, 42, &config).unwrap(); + let mut chan = Channel::::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); @@@ -5424,16 -5409,16 +5516,16 @@@ // 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::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::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); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); + let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); // Node B --> Node A: accept channel let accept_channel_msg = node_b_chan.get_accept_channel(); - node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap(); + node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap(); // Node A --> Node B: funding created let output_script = node_a_chan.get_funding_redeemscript(); @@@ -5486,7 -5471,7 +5578,7 @@@ // 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::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::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()); @@@ -5550,7 -5535,7 +5642,7 @@@ 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::::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::::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 --combined lightning/src/ln/functional_test_utils.rs index 79f8562c,dfda381b..6c08065b --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@@ -199,7 -199,6 +199,7 @@@ pub struct NodeCfg<'a> pub keys_manager: &'a test_utils::TestKeysInterface, pub logger: &'a test_utils::TestLogger, pub node_seed: [u8; 32], + pub features: InitFeatures, } pub struct Node<'a, 'b: 'a, 'c: 'b> { @@@ -1369,16 -1368,7 +1369,16 @@@ pub fn create_node_cfgs<'a>(node_count for i in 0..node_count { let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[i].chain_source), &chanmon_cfgs[i].tx_broadcaster, &chanmon_cfgs[i].logger, &chanmon_cfgs[i].fee_estimator, &chanmon_cfgs[i].persister, &chanmon_cfgs[i].keys_manager); let seed = [i as u8; 32]; - nodes.push(NodeCfg { chain_source: &chanmon_cfgs[i].chain_source, logger: &chanmon_cfgs[i].logger, tx_broadcaster: &chanmon_cfgs[i].tx_broadcaster, fee_estimator: &chanmon_cfgs[i].fee_estimator, chain_monitor, keys_manager: &chanmon_cfgs[i].keys_manager, node_seed: seed }); + nodes.push(NodeCfg { + chain_source: &chanmon_cfgs[i].chain_source, + logger: &chanmon_cfgs[i].logger, + tx_broadcaster: &chanmon_cfgs[i].tx_broadcaster, + fee_estimator: &chanmon_cfgs[i].fee_estimator, + chain_monitor, + keys_manager: &chanmon_cfgs[i].keys_manager, + node_seed: seed, + features: InitFeatures::known(), + }); } nodes @@@ -1394,6 -1384,9 +1394,9 @@@ pub fn test_default_channel_config() - // When most of our tests were written, the default HTLC minimum was fixed at 1000. // It now defaults to 1, so we simply set it to the expected value here. default_config.own_channel_config.our_htlc_minimum_msat = 1000; + // When most of our tests were written, we didn't have the notion of a `max_dust_htlc_exposure_msat`, + // It now defaults to 5_000_000 msat; to avoid interfering with tests we bump it to 50_000_000 msat. + default_config.channel_options.max_dust_htlc_exposure_msat = 50_000_000; default_config } @@@ -1433,8 -1426,8 +1436,8 @@@ pub fn create_network<'a, 'b: 'a, 'c: ' for i in 0..node_count { for j in (i+1)..node_count { - nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() }); - nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &msgs::Init { features: InitFeatures::known() }); + nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &msgs::Init { features: cfgs[j].features.clone() }); + nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &msgs::Init { features: cfgs[i].features.clone() }); } } diff --combined lightning/src/ln/functional_tests.rs index 1d149b83,0c60622e..754ef563 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@@ -28,10 -28,8 +28,10 @@@ use routing::network_graph::RoutingFees use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures}; use ln::msgs; use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction}; +use ln::script::ShutdownScript; use util::enforcing_trait_impls::EnforcingSigner; use util::{byte_utils, test_utils}; +use util::test_utils::OnGetShutdownScriptpubkey; use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; use util::errors::APIError; use util::ser::{Writeable, ReadableArgs}; @@@ -57,7 -55,6 +57,7 @@@ use io use prelude::*; use alloc::collections::BTreeSet; use core::default::Default; +use core::num::NonZeroU8; use sync::{Arc, Mutex}; use ln::functional_test_utils::*; @@@ -5896,7 -5893,7 +5896,7 @@@ fn test_key_derivation_params() let seed = [42; 32]; let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet); let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[0].chain_source), &chanmon_cfgs[0].tx_broadcaster, &chanmon_cfgs[0].logger, &chanmon_cfgs[0].fee_estimator, &chanmon_cfgs[0].persister, &keys_manager); - let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, chain_monitor, keys_manager: &keys_manager, node_seed: seed }; + let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, chain_monitor, keys_manager: &keys_manager, node_seed: seed, features: InitFeatures::known() }; let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs); node_cfgs.remove(0); node_cfgs.insert(0, node); @@@ -7480,9 -7477,9 +7480,9 @@@ fn test_upfront_shutdown_script() let flags_no = InitFeatures::known().clear_upfront_shutdown_script(); let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, flags_no, flags.clone()); nodes[0].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); - let mut node_1_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()); - node_1_shutdown.scriptpubkey = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script().to_p2sh(); + let node_1_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()); nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &InitFeatures::known(), &node_1_shutdown); + check_added_monitors!(nodes[1], 1); let events = nodes[1].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); match events[0] { @@@ -7494,8 -7491,8 +7494,8 @@@ // channel smoothly, opt-out is from channel initiator here let chan = create_announced_chan_between_nodes_with_value(&nodes, 1, 0, 1000000, 1000000, flags.clone(), flags.clone()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); - let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); - node_0_shutdown.scriptpubkey = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script().to_p2sh(); + check_added_monitors!(nodes[1], 1); + let node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_0_shutdown); let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); @@@ -7508,8 -7505,8 +7508,8 @@@ //// channel smoothly let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, flags.clone(), flags.clone()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); - let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); - node_0_shutdown.scriptpubkey = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script().to_p2sh(); + check_added_monitors!(nodes[1], 1); + let node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_0_shutdown); let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 2); @@@ -7524,56 -7521,10 +7524,56 @@@ } #[test] -fn test_upfront_shutdown_script_unsupport_segwit() { - // We test that channel is closed early - // if a segwit program is passed as upfront shutdown script, - // but the peer does not support segwit. +fn test_unsupported_anysegwit_upfront_shutdown_script() { + 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, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // Use a non-v0 segwit script supported by option_shutdown_anysegwit + let node_features = InitFeatures::known().clear_shutdown_anysegwit(); + let anysegwit_shutdown_script = Builder::new() + .push_int(16) + .push_slice(&[0, 40]) + .into_script(); + + // Check script when handling an open_channel message + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap(); + let mut open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + open_channel.shutdown_scriptpubkey = Present(anysegwit_shutdown_script.clone()); + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), node_features.clone(), &open_channel); + + let events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id } => { + assert_eq!(node_id, nodes[0].node.get_our_node_id()); + assert_eq!(msg.data, "Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: Script(OP_PUSHNUM_16 OP_PUSHBYTES_2 0028)"); + }, + _ => panic!("Unexpected event"), + } + + // Check script when handling an accept_channel message + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap(); + let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel); + let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + accept_channel.shutdown_scriptpubkey = Present(anysegwit_shutdown_script.clone()); + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), node_features, &accept_channel); + + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id } => { + assert_eq!(node_id, nodes[1].node.get_our_node_id()); + assert_eq!(msg.data, "Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: Script(OP_PUSHNUM_16 OP_PUSHBYTES_2 0028)"); + }, + _ => panic!("Unexpected event"), + } +} + +#[test] +fn test_invalid_upfront_shutdown_script() { 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, &[None, None]); @@@ -7581,26 -7532,27 +7581,26 @@@ nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap(); + // Use a segwit v0 script with an unsupported witness program let mut open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); - open_channel.shutdown_scriptpubkey = Present(Builder::new().push_int(16) + open_channel.shutdown_scriptpubkey = Present(Builder::new().push_int(0) .push_slice(&[0, 0]) .into_script()); - - let features = InitFeatures::known().clear_shutdown_anysegwit(); - nodes[0].node.handle_open_channel(&nodes[0].node.get_our_node_id(), features, &open_channel); + nodes[0].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel); let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); match events[0] { MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id } => { assert_eq!(node_id, nodes[0].node.get_our_node_id()); - assert!(regex::Regex::new(r"Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: (\([A-Fa-f0-9]+\))").unwrap().is_match(&*msg.data)); + assert_eq!(msg.data, "Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: Script(OP_0 OP_PUSHBYTES_2 0000)"); }, _ => panic!("Unexpected event"), } } #[test] -fn test_shutdown_script_any_segwit_allowed() { +fn test_segwit_v0_shutdown_script() { let mut config = UserConfig::default(); config.channel_options.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; @@@ -7611,17 -7563,14 +7611,17 @@@ let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &user_cfgs); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); - //// We test if the remote peer accepts opt_shutdown_anysegwit, a witness program can be used on shutdown - let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, InitFeatures::known(), InitFeatures::known()); + let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); + check_added_monitors!(nodes[1], 1); + + // Use a segwit v0 script supported even without option_shutdown_anysegwit let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); - node_0_shutdown.scriptpubkey = Builder::new().push_int(16) - .push_slice(&[0, 0]) + node_0_shutdown.scriptpubkey = Builder::new().push_int(0) + .push_slice(&[0; 20]) .into_script(); nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_0_shutdown); + let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 2); match events[0] { @@@ -7635,7 -7584,7 +7635,7 @@@ } #[test] -fn test_shutdown_script_any_segwit_not_allowed() { +fn test_anysegwit_shutdown_script() { let mut config = UserConfig::default(); config.channel_options.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; @@@ -7646,73 -7595,22 +7646,73 @@@ let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &user_cfgs); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); - //// We test that if the remote peer does not accept opt_shutdown_anysegwit, the witness program cannot be used on shutdown - let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, InitFeatures::known(), InitFeatures::known()); + let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); + check_added_monitors!(nodes[1], 1); + + // Use a non-v0 segwit script supported by option_shutdown_anysegwit let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); - // Make an any segwit version script node_0_shutdown.scriptpubkey = Builder::new().push_int(16) .push_slice(&[0, 0]) .into_script(); - let flags_no = InitFeatures::known().clear_shutdown_anysegwit(); - nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &flags_no, &node_0_shutdown); + nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_0_shutdown); + + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 2); + match events[0] { + MessageSendEvent::SendShutdown { node_id, .. } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()) } + _ => panic!("Unexpected event"), + } + match events[1] { + MessageSendEvent::SendClosingSigned { node_id, .. } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()) } + _ => panic!("Unexpected event"), + } +} + +#[test] +fn test_unsupported_anysegwit_shutdown_script() { + let mut config = UserConfig::default(); + config.channel_options.announced_channel = true; + config.peer_channel_config_limits.force_announced_channel_preference = false; + config.channel_options.commit_upfront_shutdown_pubkey = false; + let user_cfgs = [None, Some(config), None]; + let chanmon_cfgs = create_chanmon_cfgs(3); + let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + node_cfgs[0].features = InitFeatures::known().clear_shutdown_anysegwit(); + node_cfgs[1].features = InitFeatures::known().clear_shutdown_anysegwit(); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &user_cfgs); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + // Check that using an unsupported shutdown script fails and a supported one succeeds. + let supported_shutdown_script = chanmon_cfgs[1].keys_manager.get_shutdown_scriptpubkey(); + let unsupported_shutdown_script = + ShutdownScript::new_witness_program(NonZeroU8::new(16).unwrap(), &[0, 40]).unwrap(); + chanmon_cfgs[1].keys_manager + .expect(OnGetShutdownScriptpubkey { returns: unsupported_shutdown_script.clone() }) + .expect(OnGetShutdownScriptpubkey { returns: supported_shutdown_script }); + + let chan = create_announced_chan_between_nodes(&nodes, 0, 1, node_cfgs[0].features.clone(), node_cfgs[1].features.clone()); + match nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()) { + Err(APIError::IncompatibleShutdownScript { script }) => { + assert_eq!(script.into_inner(), unsupported_shutdown_script.clone().into_inner()); + }, + Err(e) => panic!("Unexpected error: {:?}", e), + Ok(_) => panic!("Expected error"), + } + nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); + check_added_monitors!(nodes[1], 1); + + // Use a non-v0 segwit script unsupported without option_shutdown_anysegwit + let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); + node_0_shutdown.scriptpubkey = unsupported_shutdown_script.into_inner(); + nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &node_cfgs[1].features, &node_0_shutdown); + let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 2); match events[1] { MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()); - assert_eq!(msg.data, "Got a nonstandard scriptpubkey (60020000) from remote peer".to_owned()) + assert_eq!(msg.data, "Got a nonstandard scriptpubkey (60020028) from remote peer".to_owned()); }, _ => panic!("Unexpected event"), } @@@ -7720,7 -7618,7 +7720,7 @@@ } #[test] -fn test_shutdown_script_segwit_but_not_anysegwit() { +fn test_invalid_shutdown_script() { let mut config = UserConfig::default(); config.channel_options.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; @@@ -7731,17 -7629,15 +7731,17 @@@ let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &user_cfgs); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); - //// We test that if shutdown any segwit is supported and we send a witness script with 0 version, this is not accepted - let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 1000000, InitFeatures::known(), InitFeatures::known()); + let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); nodes[1].node.close_channel(&OutPoint { txid: chan.3.txid(), index: 0 }.to_channel_id()).unwrap(); + check_added_monitors!(nodes[1], 1); + + // Use a segwit v0 script with an unsupported witness program let mut node_0_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()); - // Make a segwit script that is not a valid as any segwit node_0_shutdown.scriptpubkey = Builder::new().push_int(0) .push_slice(&[0, 0]) .into_script(); nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_0_shutdown); + let events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 2); match events[1] { @@@ -7769,7 -7665,7 +7769,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"), @@@ -7780,7 -7676,7 +7780,7 @@@ 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) { + 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) { 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"), @@@ -7806,7 -7702,7 +7806,7 @@@ 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) { + 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) { 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"), @@@ -9764,3 -9660,116 +9764,116 @@@ fn test_keysend_payments_to_private_nod pass_along_path(&nodes[0], &path, 10000, payment_hash, None, event, true, Some(test_preimage)); claim_payment(&nodes[0], &path, test_preimage); } + + fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, at_forward: bool, on_holder_tx: bool) { + // Test that we properly reject dust HTLC violating our `max_dust_htlc_exposure_msat` policy. + // + // At HTLC forward (`send_payment()`), if the sum of the trimmed-to-dust HTLC inbound and + // trimmed-to-dust HTLC outbound balance and this new payment as included on next counterparty + // commitment are above our `max_dust_htlc_exposure_msat`, we'll reject the update. + // At HTLC reception (`update_add_htlc()`), if the sum of the trimmed-to-dust HTLC inbound + // and trimmed-to-dust HTLC outbound balance and this new received HTLC as included on next + // counterparty commitment are above our `max_dust_htlc_exposure_msat`, we'll fail the update. + // Note, we return a `temporary_channel_failure` (0x1000 | 7), as the channel might be + // available again for HTLC processing once the dust bandwidth has cleared up. + + let chanmon_cfgs = create_chanmon_cfgs(2); + let mut config = test_default_channel_config(); + config.channel_options.max_dust_htlc_exposure_msat = 5_000_000; // default setting value + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(config)]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap(); + let mut open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + open_channel.max_htlc_value_in_flight_msat = 50_000_000; + open_channel.max_accepted_htlcs = 60; + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel); + let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + if on_holder_tx { + accept_channel.dust_limit_satoshis = 660; + } + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel); + + let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], 1_000_000, 42); + + if on_holder_tx { + if let Some(mut chan) = nodes[1].node.channel_state.lock().unwrap().by_id.get_mut(&temporary_channel_id) { + chan.holder_dust_limit_satoshis = 660; + } + } + + nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap(); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id())); + check_added_monitors!(nodes[1], 1); + + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id())); + check_added_monitors!(nodes[0], 1); + + let (funding_locked, _) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &tx); + let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked); + update_nodes_with_chan_announce(&nodes, 0, 1, &announcement, &as_update, &bs_update); + + if on_holder_tx { + if dust_outbound_balance { + for i in 0..2 { + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 2_300_000); + if let Err(_) = nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret)) { panic!("Unexpected event at dust HTLC {}", i); } + } + } else { + for _ in 0..2 { + route_payment(&nodes[0], &[&nodes[1]], 2_300_000); + } + } + } else { + if dust_outbound_balance { + for i in 0..25 { + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 200_000); // + 177_000 msat of HTLC-success tx at 253 sats/kWU + if let Err(_) = nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret)) { panic!("Unexpected event at dust HTLC {}", i); } + } + } else { + for _ in 0..25 { + route_payment(&nodes[0], &[&nodes[1]], 200_000); // + 167_000 msat of HTLC-timeout tx at 253 sats/kWU + } + } + } + + if at_forward { + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], if on_holder_tx { 2_300_000 } else { 200_000 }); + let mut config = UserConfig::default(); + if on_holder_tx { + unwrap_send_err!(nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable { ref err }, assert_eq!(err, &format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", 6_900_000, config.channel_options.max_dust_htlc_exposure_msat))); + } else { + unwrap_send_err!(nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable { ref err }, assert_eq!(err, &format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", 5_200_000, config.channel_options.max_dust_htlc_exposure_msat))); + } + } else { + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1 ], if on_holder_tx { 2_300_000 } else { 200_000 }); + nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap(); + check_added_monitors!(nodes[0], 1); + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let payment_event = SendEvent::from_event(events.remove(0)); + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]); + if on_holder_tx { + nodes[1].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", 6_900_000, config.channel_options.max_dust_htlc_exposure_msat), 1); + } else { + nodes[1].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", 5_200_000, config.channel_options.max_dust_htlc_exposure_msat), 1); + } + } + + let _ = nodes[1].node.get_and_clear_pending_msg_events(); + let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); + added_monitors.clear(); + } + + #[test] + fn test_max_dust_htlc_exposure() { + do_test_max_dust_htlc_exposure(true, true, true); + do_test_max_dust_htlc_exposure(false, true, true); + do_test_max_dust_htlc_exposure(false, false, true); + do_test_max_dust_htlc_exposure(false, false, false); + do_test_max_dust_htlc_exposure(true, true, false); + do_test_max_dust_htlc_exposure(true, false, false); + do_test_max_dust_htlc_exposure(true, false, true); + do_test_max_dust_htlc_exposure(false, true, false); + }