X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=c8209a9eac6f984f76514987729d9ca04dde192f;hb=d2a9ae0b8eb5bdb1724be0b8d398d0fdf9870266;hp=ca0836f63275308d2a78b9d28d89fe15f5b809d3;hpb=b0e8b739b73cc25f3e1ab00695a14d972162a140;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ca0836f6..c8209a9e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -795,6 +795,9 @@ pub const MAX_CHAN_DUST_LIMIT_SATOSHIS: u64 = MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS /// See https://github.com/lightning/bolts/issues/905 for more details. pub const MIN_CHAN_DUST_LIMIT_SATOSHIS: u64 = 354; +// Just a reasonable implementation-specific safe lower bound, higher than the dust limit. +pub const MIN_THEIR_CHAN_RESERVE_SATOSHIS: u64 = 1000; + /// Used to return a simple Error back to ChannelManager. Will get converted to a /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our /// channel_id in ChannelManager. @@ -843,16 +846,25 @@ impl Channel { } /// Returns a minimum channel reserve value the remote needs to maintain, - /// required by us. + /// required by us according to the configured or default + /// [`ChannelHandshakeConfig::their_channel_reserve_proportional_millionths`] /// /// Guaranteed to return a value no larger than channel_value_satoshis /// - /// This is used both for new channels and to figure out what reserve value we sent to peers - /// for channels serialized before we included our selected reserve value in the serialized - /// data explicitly. - pub(crate) fn get_holder_selected_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 { + /// This is used both for outbound and inbound channels and has lower bound + /// of `MIN_THEIR_CHAN_RESERVE_SATOSHIS`. + pub(crate) fn get_holder_selected_channel_reserve_satoshis(channel_value_satoshis: u64, config: &UserConfig) -> u64 { + let calculated_reserve = channel_value_satoshis.saturating_mul(config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64) / 1_000_000; + cmp::min(channel_value_satoshis, cmp::max(calculated_reserve, MIN_THEIR_CHAN_RESERVE_SATOSHIS)) + } + + /// This is for legacy reasons, present for forward-compatibility. + /// LDK versions older than 0.0.104 don't know how read/handle values other than default + /// from storage. Hence, we use this function to not persist default values of + /// `holder_selected_channel_reserve_satoshis` for channels into storage. + pub(crate) fn get_legacy_default_holder_selected_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 { let (q, _) = channel_value_satoshis.overflowing_div(100); - cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO + cmp::min(channel_value_satoshis, cmp::max(q, 1000)) } pub(crate) fn opt_anchors(&self) -> bool { @@ -912,12 +924,14 @@ impl Channel { if holder_selected_contest_delay < BREAKDOWN_TIMEOUT { return Err(APIError::APIMisuseError {err: format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks", holder_selected_contest_delay)}); } - let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(channel_value_satoshis); + let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(channel_value_satoshis, config); if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { + // Protocol level safety check in place, although it should never happen because + // of `MIN_THEIR_CHAN_RESERVE_SATOSHIS` return Err(APIError::APIMisuseError { err: format!("Holder selected channel reserve below implemention limit dust_limit_satoshis {}", holder_selected_channel_reserve_satoshis) }); } - let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); + let feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal); let value_to_self_msat = channel_value_satoshis * 1000 - push_msat; let commitment_tx_fee = Self::commit_tx_fee_msat(feerate, MIN_AFFORDABLE_HTLC_COUNT, opt_anchors); @@ -1064,11 +1078,11 @@ impl Channel { // We generally don't care too much if they set the feerate to something very high, but it // could result in the channel being useless due to everything being dust. let upper_limit = cmp::max(250 * 25, - fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10); + fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10); if feerate_per_kw as u64 > upper_limit { return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit))); } - let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); + let lower_limit = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background); // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing // occasional issues with feerate disagreements between an initiator that wants a feerate // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250 @@ -1204,12 +1218,14 @@ impl Channel { } } - let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis); + let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis, config); if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { + // Protocol level safety check in place, although it should never happen because + // of `MIN_THEIR_CHAN_RESERVE_SATOSHIS` return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). dust_limit_satoshis is ({}).", holder_selected_channel_reserve_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); } if holder_selected_channel_reserve_satoshis * 1000 >= full_channel_value_msat { - return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). Channel value is ({} - {}).", holder_selected_channel_reserve_satoshis, full_channel_value_msat, msg.push_msat))); + return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({})msats. Channel value is ({} - {})msats.", holder_selected_channel_reserve_satoshis * 1000, full_channel_value_msat, msg.push_msat))); } if msg.channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { log_debug!(logger, "channel_reserve_satoshis ({}) is smaller than our dust limit ({}). We can broadcast stale states without any risk, implying this channel is very insecure for our counterparty.", @@ -3534,6 +3550,12 @@ impl Channel { return; } + if self.channel_state & (ChannelState::PeerDisconnected as u32) == (ChannelState::PeerDisconnected as u32) { + // While the below code should be idempotent, it's simpler to just return early, as + // redundant disconnect events can fire, though they should be rare. + return; + } + if self.announcement_sigs_state == AnnouncementSigsState::MessageSent || self.announcement_sigs_state == AnnouncementSigsState::Committed { self.announcement_sigs_state = AnnouncementSigsState::NotSent; } @@ -4022,8 +4044,8 @@ impl Channel { // Propose a range from our current Background feerate to our Normal feerate plus our // force_close_avoidance_max_fee_satoshis. // If we fail to come to consensus, we'll have to force-close. - let mut proposed_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); - let normal_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); + let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background); + let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal); let mut proposed_max_feerate = if self.is_outbound() { normal_feerate } else { u32::max_value() }; // The spec requires that (when the channel does not have anchors) we only send absolute @@ -4755,6 +4777,9 @@ impl Channel { } fn check_get_channel_ready(&mut self, height: u32) -> Option { + // Called: + // * always when a new block/transactions are confirmed with the new height + // * when funding is signed with a height of 0 if self.funding_tx_confirmation_height == 0 && self.minimum_depth != Some(0) { return None; } @@ -4780,7 +4805,7 @@ impl Channel { // We got a reorg but not enough to trigger a force close, just ignore. false } else { - if self.channel_state < ChannelState::ChannelFunded as u32 { + if self.funding_tx_confirmation_height != 0 && self.channel_state < ChannelState::ChannelFunded as u32 { // We should never see a funding transaction on-chain until we've received // funding_signed (if we're an outbound channel), or seen funding_generated (if we're // an inbound channel - before that we have no known funding TXID). The fuzzer, @@ -5765,7 +5790,7 @@ impl Channel { /// those explicitly stated to be allowed after shutdown completes, eg some simple getters). /// Also returns the list of payment_hashes for channels which we can safely fail backwards /// immediately (others we will have to allow to time out). - pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash)>) { + pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash, PublicKey, [u8; 32])>) { // Note that we MUST only generate a monitor update that indicates force-closure - we're // called during initialization prior to the chain_monitor in the encompassing ChannelManager // being fully configured in some cases. Thus, its likely any monitor events we generate will @@ -5775,10 +5800,11 @@ impl Channel { // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and // return them to fail the payment. let mut dropped_outbound_htlcs = Vec::with_capacity(self.holding_cell_htlc_updates.len()); + let counterparty_node_id = self.get_counterparty_node_id(); for htlc_update in self.holding_cell_htlc_updates.drain(..) { match htlc_update { HTLCUpdateAwaitingACK::AddHTLC { source, payment_hash, .. } => { - dropped_outbound_htlcs.push((source, payment_hash)); + dropped_outbound_htlcs.push((source, payment_hash, counterparty_node_id, self.channel_id)); }, _ => {} } @@ -6106,7 +6132,7 @@ impl Writeable for Channel { // a different percentage of the channel value then 10%, which older versions of LDK used // to set it to before the percentage was made configurable. let serialized_holder_selected_reserve = - if self.holder_selected_channel_reserve_satoshis != Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) + if self.holder_selected_channel_reserve_satoshis != Self::get_legacy_default_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) { Some(self.holder_selected_channel_reserve_satoshis) } else { None }; let mut old_max_in_flight_percent_config = UserConfig::default().channel_handshake_config; @@ -6381,7 +6407,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel let mut announcement_sigs = None; let mut target_closing_feerate_sats_per_kw = None; let mut monitor_pending_finalized_fulfills = Some(Vec::new()); - let mut holder_selected_channel_reserve_satoshis = Some(Self::get_holder_selected_channel_reserve_satoshis(channel_value_satoshis)); + let mut holder_selected_channel_reserve_satoshis = Some(Self::get_legacy_default_holder_selected_channel_reserve_satoshis(channel_value_satoshis)); let mut holder_max_htlc_value_in_flight_msat = Some(Self::get_holder_max_htlc_value_in_flight_msat(channel_value_satoshis, &UserConfig::default().channel_handshake_config)); // Prior to supporting channel type negotiation, all of our channels were static_remotekey // only, so we default to that if none was written. @@ -6561,6 +6587,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel #[cfg(test)] mod tests { + use std::cmp; use bitcoin::blockdata::script::{Script, Builder}; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::blockdata::constants::genesis_block; @@ -6570,7 +6597,7 @@ mod tests { use ln::PaymentHash; use ln::channelmanager::{HTLCSource, PaymentId}; use ln::channel::{Channel, InboundHTLCOutput, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator}; - use ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS}; + use ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS, MIN_THEIR_CHAN_RESERVE_SATOSHIS}; use ln::features::{InitFeatures, ChannelTypeFeatures}; use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate, MAX_VALUE_MSAT}; use ln::script::ShutdownScript; @@ -6585,14 +6612,16 @@ mod tests { use util::errors::APIError; use util::test_utils; use util::test_utils::OnGetShutdownScriptpubkey; - use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; + use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature, Scalar}; use bitcoin::secp256k1::ffi::Signature as FFISignature; use bitcoin::secp256k1::{SecretKey,PublicKey}; + use bitcoin::secp256k1::ecdh::SharedSecret; use bitcoin::secp256k1::ecdsa::RecoverableSignature; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; use bitcoin::hash_types::WPubkeyHash; use bitcoin::bech32::u5; + use bitcoin::PackedLockTime; use bitcoin::util::address::WitnessVersion; use prelude::*; @@ -6628,6 +6657,7 @@ mod tests { type Signer = InMemorySigner; fn get_node_secret(&self, _recipient: Recipient) -> Result { panic!(); } + fn ecdh(&self, _recipient: Recipient, _other_key: &PublicKey, _tweak: Option<&Scalar>) -> Result { panic!(); } fn get_inbound_payment_key_material(&self) -> KeyMaterial { panic!(); } fn get_destination_script(&self) -> Script { let secp_ctx = Secp256k1::signing_only(); @@ -6852,7 +6882,7 @@ mod tests { // Node A --> Node B: funding created let output_script = node_a_chan.get_funding_redeemscript(); - let tx = Transaction { version: 1, lock_time: 0, input: Vec::new(), output: vec![TxOut { + let tx = Transaction { version: 1, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: vec![TxOut { value: 10000000, script_pubkey: output_script.clone(), }]}; let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; @@ -6962,6 +6992,64 @@ mod tests { assert_eq!(chan_8.holder_max_htlc_value_in_flight_msat, chan_8_value_msat); } + #[test] + fn test_configured_holder_selected_channel_reserve_satoshis() { + + // Test that `new_outbound` and `new_from_req` create a channel with the correct + // channel reserves, when `their_channel_reserve_proportional_millionths` is configured. + test_self_and_counterparty_channel_reserve(10_000_000, 0.02, 0.02); + + // Test with valid but unreasonably high channel reserves + // Requesting and accepting parties have requested for 49%-49% and 60%-30% channel reserve + test_self_and_counterparty_channel_reserve(10_000_000, 0.49, 0.49); + test_self_and_counterparty_channel_reserve(10_000_000, 0.60, 0.30); + + // Test with calculated channel reserve less than lower bound + // i.e `MIN_THEIR_CHAN_RESERVE_SATOSHIS` + test_self_and_counterparty_channel_reserve(100_000, 0.00002, 0.30); + + // Test with invalid channel reserves since sum of both is greater than or equal + // to channel value + test_self_and_counterparty_channel_reserve(10_000_000, 0.50, 0.50); + test_self_and_counterparty_channel_reserve(10_000_000, 0.60, 0.50); + } + + fn test_self_and_counterparty_channel_reserve(channel_value_satoshis: u64, outbound_selected_channel_reserve_perc: f64, inbound_selected_channel_reserve_perc: f64) { + let fee_est = LowerBoundedFeeEstimator::new(&TestFeeEstimator { fee_est: 15_000 }); + let logger = test_utils::TestLogger::new(); + let secp_ctx = Secp256k1::new(); + let seed = [42; 32]; + let network = Network::Testnet; + let keys_provider = test_utils::TestKeysInterface::new(&seed, network); + let outbound_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let inbound_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); + + + let mut outbound_node_config = UserConfig::default(); + outbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (outbound_selected_channel_reserve_perc * 1_000_000.0) as u32; + let chan = Channel::::new_outbound(&&fee_est, &&keys_provider, outbound_node_id, &InitFeatures::known(), channel_value_satoshis, 100_000, 42, &outbound_node_config, 0, 42).unwrap(); + + let expected_outbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.channel_value_satoshis as f64 * outbound_selected_channel_reserve_perc) as u64); + assert_eq!(chan.holder_selected_channel_reserve_satoshis, expected_outbound_selected_chan_reserve); + + let chan_open_channel_msg = chan.get_open_channel(genesis_block(network).header.block_hash()); + let mut inbound_node_config = UserConfig::default(); + inbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (inbound_selected_channel_reserve_perc * 1_000_000.0) as u32; + + if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 { + let chan_inbound_node = Channel::::new_from_req(&&fee_est, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42).unwrap(); + + let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.channel_value_satoshis as f64 * inbound_selected_channel_reserve_perc) as u64); + + assert_eq!(chan_inbound_node.holder_selected_channel_reserve_satoshis, expected_inbound_selected_chan_reserve); + assert_eq!(chan_inbound_node.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve); + } else { + // Channel Negotiations failed + let result = Channel::::new_from_req(&&fee_est, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42); + assert!(result.is_err()); + } + } + #[test] fn channel_update() { let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000});