X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannelmanager.rs;h=d591788f64a68209349b2d7b86c269997a63fbab;hb=783e8188a7bd6470d5926bf5d1bae1350996801a;hp=2c62cc74d698212b4bc918a4a383859d282746c7;hpb=bd4eb0da76207a68396de8b36fb640467255cc31;p=rust-lightning diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2c62cc74..d591788f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -35,6 +35,8 @@ use crate::chain::{Confirm, ChannelMonitorUpdateStatus, Watch, BestBlock}; use crate::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator}; use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID}; use crate::chain::transaction::{OutPoint, TransactionData}; +use crate::events; +use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination}; // Since this struct is returned in `list_channels` methods, expose it here in case users want to // construct one themselves. use crate::ln::{inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret}; @@ -55,10 +57,9 @@ use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutb use crate::ln::wire::Encode; use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, ChannelSigner, WriteableEcdsaChannelSigner}; use crate::util::config::{UserConfig, ChannelConfig}; -use crate::util::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination}; -use crate::util::events; use crate::util::wakers::{Future, Notifier}; use crate::util::scid_utils::fake_scid; +use crate::util::string::UntrustedString; use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter}; use crate::util::logger::{Level, Logger}; use crate::util::errors::APIError; @@ -119,7 +120,10 @@ pub(super) struct PendingHTLCInfo { pub(super) routing: PendingHTLCRouting, pub(super) incoming_shared_secret: [u8; 32], payment_hash: PaymentHash, + /// Amount received pub(super) incoming_amt_msat: Option, // Added in 0.0.113 + /// Sender intended amount to forward or receive (actual amount received + /// may overshoot this in either case) pub(super) outgoing_amt_msat: u64, pub(super) outgoing_cltv_value: u32, } @@ -191,14 +195,21 @@ struct ClaimableHTLC { cltv_expiry: u32, /// The amount (in msats) of this MPP part value: u64, + /// The amount (in msats) that the sender intended to be sent in this MPP + /// part (used for validating total MPP amount) + sender_intended_value: u64, onion_payload: OnionPayload, timer_ticks: u8, - /// The sum total of all MPP parts + /// The total value received for a payment (sum of all MPP parts if the payment is a MPP). + /// Gets set to the amount reported when pushing [`Event::PaymentClaimable`]. + total_value_received: Option, + /// The sender intended sum total of all MPP parts specified in the onion total_msat: u64, } /// A payment identifier used to uniquely identify a payment to LDK. -/// (C-not exported) as we just use [u8; 32] directly +/// +/// This is not exported to bindings users as we just use [u8; 32] directly #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] pub struct PaymentId(pub [u8; 32]); @@ -216,7 +227,8 @@ impl Readable for PaymentId { } /// An identifier used to uniquely identify an intercepted HTLC to LDK. -/// (C-not exported) as we just use [u8; 32] directly +/// +/// This is not exported to bindings users as we just use [u8; 32] directly #[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] pub struct InterceptId(pub [u8; 32]); @@ -568,7 +580,7 @@ struct PendingInboundPayment { /// or, respectively, [`Router`] for its router, but this type alias chooses the concrete types /// of [`KeysManager`] and [`DefaultRouter`]. /// -/// (C-not exported) as Arcs don't make sense in bindings +/// This is not exported to bindings users as Arcs don't make sense in bindings pub type SimpleArcChannelManager = ChannelManager< Arc, Arc, @@ -594,7 +606,7 @@ pub type SimpleArcChannelManager = ChannelManager< /// or, respectively, [`Router`] for its router, but this type alias chooses the concrete types /// of [`KeysManager`] and [`DefaultRouter`]. /// -/// (C-not exported) as Arcs don't make sense in bindings +/// This is not exported to bindings users as Arcs don't make sense in bindings pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = ChannelManager<&'a M, &'b T, &'c KeysManager, &'c KeysManager, &'c KeysManager, &'d F, &'e DefaultRouter<&'f NetworkGraph<&'g L>, &'g L, &'h Mutex, &'g L>>>, &'g L>; /// Manager which keeps track of a number of channels and sends messages to the appropriate @@ -1946,7 +1958,7 @@ where /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal - /// [`SendShutdown`]: crate::util::events::MessageSendEvent::SendShutdown + /// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown pub fn close_channel(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey) -> Result<(), APIError> { self.close_channel_internal(channel_id, counterparty_node_id, None) } @@ -1970,7 +1982,7 @@ where /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal - /// [`SendShutdown`]: crate::util::events::MessageSendEvent::SendShutdown + /// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown pub fn close_channel_with_target_feerate(&self, channel_id: &[u8; 32], counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: u32) -> Result<(), APIError> { self.close_channel_internal(channel_id, counterparty_node_id, Some(target_feerate_sats_per_1000_weight)) } @@ -2006,7 +2018,7 @@ where let peer_state = &mut *peer_state_lock; if let hash_map::Entry::Occupied(chan) = peer_state.channel_by_id.entry(channel_id.clone()) { if let Some(peer_msg) = peer_msg { - self.issue_channel_close_events(chan.get(),ClosureReason::CounterpartyForceClosed { peer_msg: peer_msg.to_string() }); + self.issue_channel_close_events(chan.get(),ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(peer_msg.to_string()) }); } else { self.issue_channel_close_events(chan.get(),ClosureReason::HolderForceClosed); } @@ -2089,9 +2101,9 @@ where payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result { // final_incorrect_cltv_expiry - if hop_data.outgoing_cltv_value != cltv_expiry { + if hop_data.outgoing_cltv_value > cltv_expiry { return Err(ReceiveError { - msg: "Upstream node set CLTV to the wrong value", + msg: "Upstream node set CLTV to less than the CLTV set by the sender", err_code: 18, err_data: cltv_expiry.to_be_bytes().to_vec() }) @@ -2175,7 +2187,7 @@ where payment_hash, incoming_shared_secret: shared_secret, incoming_amt_msat: Some(amt_msat), - outgoing_amt_msat: amt_msat, + outgoing_amt_msat: hop_data.amt_to_forward, outgoing_cltv_value: hop_data.outgoing_cltv_value, }) } @@ -2657,7 +2669,7 @@ where } #[cfg(test)] - fn test_send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option, keysend_preimage: Option, payment_id: PaymentId, recv_value_msat: Option, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> { + pub(super) fn test_send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option, keysend_preimage: Option, payment_id: PaymentId, recv_value_msat: Option, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height(); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); self.pending_outbound_payments.test_send_payment_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, &self.node_signer, best_block_height, @@ -2844,8 +2856,8 @@ where /// implemented by Bitcoin Core wallet. See /// for more details. /// - /// [`Event::FundingGenerationReady`]: crate::util::events::Event::FundingGenerationReady - /// [`Event::ChannelClosed`]: crate::util::events::Event::ChannelClosed + /// [`Event::FundingGenerationReady`]: crate::events::Event::FundingGenerationReady + /// [`Event::ChannelClosed`]: crate::events::Event::ChannelClosed pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction) -> Result<(), APIError> { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); @@ -3255,7 +3267,7 @@ where HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id, forward_info: PendingHTLCInfo { - routing, incoming_shared_secret, payment_hash, outgoing_amt_msat, .. + routing, incoming_shared_secret, payment_hash, incoming_amt_msat, outgoing_amt_msat, .. } }) => { let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret) = match routing { @@ -3269,7 +3281,7 @@ where panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive"); } }; - let claimable_htlc = ClaimableHTLC { + let mut claimable_htlc = ClaimableHTLC { prev_hop: HTLCPreviousHopData { short_channel_id: prev_short_channel_id, outpoint: prev_funding_outpoint, @@ -3277,8 +3289,13 @@ where incoming_packet_shared_secret: incoming_shared_secret, phantom_shared_secret, }, - value: outgoing_amt_msat, + // We differentiate the received value from the sender intended value + // if possible so that we don't prematurely mark MPP payments complete + // if routing nodes overpay + value: incoming_amt_msat.unwrap_or(outgoing_amt_msat), + sender_intended_value: outgoing_amt_msat, timer_ticks: 0, + total_value_received: None, total_msat: if let Some(data) = &payment_data { data.total_msat } else { outgoing_amt_msat }, cltv_expiry, onion_payload, @@ -3323,7 +3340,7 @@ where fail_htlc!(claimable_htlc, payment_hash); continue } - let (_, htlcs) = claimable_payments.claimable_htlcs.entry(payment_hash) + let (_, ref mut htlcs) = claimable_payments.claimable_htlcs.entry(payment_hash) .or_insert_with(|| (purpose(), Vec::new())); if htlcs.len() == 1 { if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload { @@ -3332,9 +3349,9 @@ where continue } } - let mut total_value = claimable_htlc.value; + let mut total_value = claimable_htlc.sender_intended_value; for htlc in htlcs.iter() { - total_value += htlc.value; + total_value += htlc.sender_intended_value; match &htlc.onion_payload { OnionPayload::Invoice { .. } => { if htlc.total_msat != $payment_data.total_msat { @@ -3347,18 +3364,24 @@ where _ => unreachable!(), } } - if total_value >= msgs::MAX_VALUE_MSAT || total_value > $payment_data.total_msat { - log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the total value {} ran over expected value {} (or HTLCs were inconsistent)", - log_bytes!(payment_hash.0), total_value, $payment_data.total_msat); + // The condition determining whether an MPP is complete must + // match exactly the condition used in `timer_tick_occurred` + if total_value >= msgs::MAX_VALUE_MSAT { fail_htlc!(claimable_htlc, payment_hash); - } else if total_value == $payment_data.total_msat { + } else if total_value - claimable_htlc.sender_intended_value >= $payment_data.total_msat { + log_trace!(self.logger, "Failing HTLC with payment_hash {} as payment is already claimable", + log_bytes!(payment_hash.0)); + fail_htlc!(claimable_htlc, payment_hash); + } else if total_value >= $payment_data.total_msat { let prev_channel_id = prev_funding_outpoint.to_channel_id(); htlcs.push(claimable_htlc); + let amount_msat = htlcs.iter().map(|htlc| htlc.value).sum(); + htlcs.iter_mut().for_each(|htlc| htlc.total_value_received = Some(amount_msat)); new_events.push(events::Event::PaymentClaimable { receiver_node_id: Some(receiver_node_id), payment_hash, purpose: purpose(), - amount_msat: total_value, + amount_msat, via_channel_id: Some(prev_channel_id), via_user_channel_id: Some(prev_user_channel_id), }); @@ -3412,13 +3435,15 @@ where } match claimable_payments.claimable_htlcs.entry(payment_hash) { hash_map::Entry::Vacant(e) => { + let amount_msat = claimable_htlc.value; + claimable_htlc.total_value_received = Some(amount_msat); let purpose = events::PaymentPurpose::SpontaneousPayment(preimage); e.insert((purpose.clone(), vec![claimable_htlc])); let prev_channel_id = prev_funding_outpoint.to_channel_id(); new_events.push(events::Event::PaymentClaimable { receiver_node_id: Some(receiver_node_id), payment_hash, - amount_msat: outgoing_amt_msat, + amount_msat, purpose, via_channel_id: Some(prev_channel_id), via_user_channel_id: Some(prev_user_channel_id), @@ -3678,7 +3703,9 @@ where if let OnionPayload::Invoice { .. } = htlcs[0].onion_payload { // Check if we've received all the parts we need for an MPP (the value of the parts adds to total_msat). // In this case we're not going to handle any timeouts of the parts here. - if htlcs[0].total_msat == htlcs.iter().fold(0, |total, htlc| total + htlc.value) { + // This condition determining whether the MPP is complete here must match + // exactly the condition used in `process_pending_htlc_forwards`. + if htlcs[0].total_msat <= htlcs.iter().fold(0, |total, htlc| total + htlc.sender_intended_value) { return true; } else if htlcs.into_iter().any(|htlc| { htlc.timer_ticks += 1; @@ -3904,8 +3931,8 @@ where /// event matches your expectation. If you fail to do so and call this method, you may provide /// the sender "proof-of-payment" when they did not fulfill the full expected payment. /// - /// [`Event::PaymentClaimable`]: crate::util::events::Event::PaymentClaimable - /// [`Event::PaymentClaimed`]: crate::util::events::Event::PaymentClaimed + /// [`Event::PaymentClaimable`]: crate::events::Event::PaymentClaimable + /// [`Event::PaymentClaimed`]: crate::events::Event::PaymentClaimed /// [`process_pending_events`]: EventsProvider::process_pending_events /// [`create_inbound_payment`]: Self::create_inbound_payment /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash @@ -3957,6 +3984,7 @@ where // provide the preimage, so worrying too much about the optimal handling isn't worth // it. let mut claimable_amt_msat = 0; + let mut prev_total_msat = None; let mut expected_amt_msat = None; let mut valid_mpp = true; let mut errs = Vec::new(); @@ -3984,14 +4012,22 @@ where break; } - if expected_amt_msat.is_some() && expected_amt_msat != Some(htlc.total_msat) { - log_error!(self.logger, "Somehow ended up with an MPP payment with different total amounts - this should not be reachable!"); + if prev_total_msat.is_some() && prev_total_msat != Some(htlc.total_msat) { + log_error!(self.logger, "Somehow ended up with an MPP payment with different expected total amounts - this should not be reachable!"); + debug_assert!(false); + valid_mpp = false; + break; + } + prev_total_msat = Some(htlc.total_msat); + + if expected_amt_msat.is_some() && expected_amt_msat != htlc.total_value_received { + log_error!(self.logger, "Somehow ended up with an MPP payment with different received total amounts - this should not be reachable!"); debug_assert!(false); valid_mpp = false; break; } + expected_amt_msat = htlc.total_value_received; - expected_amt_msat = Some(htlc.total_msat); if let OnionPayload::Spontaneous(_) = &htlc.onion_payload { // We don't currently support MPP for spontaneous payments, so just check // that there's one payment here and move on. @@ -4148,6 +4184,7 @@ where claim_from_onchain_tx: from_onchain, prev_channel_id, next_channel_id, + outbound_amount_forwarded_msat: forwarded_htlc_value_msat, }}) } else { None } }); @@ -6791,7 +6828,9 @@ impl Writeable for ClaimableHTLC { (0, self.prev_hop, required), (1, self.total_msat, required), (2, self.value, required), + (3, self.sender_intended_value, required), (4, payment_data, option), + (5, self.total_value_received, option), (6, self.cltv_expiry, required), (8, keysend_preimage, option), }); @@ -6803,15 +6842,19 @@ impl Readable for ClaimableHTLC { fn read(reader: &mut R) -> Result { let mut prev_hop = crate::util::ser::RequiredWrapper(None); let mut value = 0; + let mut sender_intended_value = None; let mut payment_data: Option = None; let mut cltv_expiry = 0; + let mut total_value_received = None; let mut total_msat = None; let mut keysend_preimage: Option = None; read_tlv_fields!(reader, { (0, prev_hop, required), (1, total_msat, option), (2, value, required), + (3, sender_intended_value, option), (4, payment_data, option), + (5, total_value_received, option), (6, cltv_expiry, required), (8, keysend_preimage, option) }); @@ -6839,6 +6882,8 @@ impl Readable for ClaimableHTLC { prev_hop: prev_hop.0.unwrap(), timer_ticks: 0, value, + sender_intended_value: sender_intended_value.unwrap_or(value), + total_value_received, total_msat: total_msat.unwrap(), onion_payload, cltv_expiry, @@ -7235,7 +7280,7 @@ where /// In such cases the latest local transactions will be sent to the tx_broadcaster included in /// this struct. /// - /// (C-not exported) because we have no HashMap bindings + /// This is not exported to bindings users because we have no HashMap bindings pub channel_monitors: HashMap::Signer>>, } @@ -7886,6 +7931,7 @@ mod tests { use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use core::time::Duration; use core::sync::atomic::Ordering; + use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use crate::ln::channelmanager::{inbound_payment, PaymentId, PaymentSendFailure, InterceptId}; use crate::ln::functional_test_utils::*; @@ -7893,7 +7939,6 @@ mod tests { use crate::ln::msgs::ChannelMessageHandler; use crate::routing::router::{PaymentParameters, RouteParameters, find_route}; use crate::util::errors::APIError; - use crate::util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason}; use crate::util::test_utils; use crate::util::config::ChannelConfig; use crate::chain::keysinterface::EntropySource; @@ -8794,6 +8839,7 @@ pub mod bench { use crate::chain::Listen; use crate::chain::chainmonitor::{ChainMonitor, Persist}; use crate::chain::keysinterface::{EntropySource, KeysManager, InMemorySigner}; + use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider}; use crate::ln::channelmanager::{self, BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage, PaymentId}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::{ChannelMessageHandler, Init}; @@ -8801,7 +8847,6 @@ pub mod bench { use crate::routing::router::{PaymentParameters, get_route}; use crate::util::test_utils; use crate::util::config::UserConfig; - use crate::util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256;