diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 8fdd7b03e21..421a0d09a48 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -47,8 +47,7 @@ use lightning::events::MessageSendEventsProvider; use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; use lightning::ln::channel_state::ChannelDetails; use lightning::ln::channelmanager::{ - ChainParameters, ChannelManager, ChannelManagerReadArgs, PaymentId, PaymentSendFailure, - RecipientOnionFields, + ChainParameters, ChannelManager, ChannelManagerReadArgs, PaymentId, RecipientOnionFields, Retry, }; use lightning::ln::functional_test_utils::*; use lightning::ln::msgs::{ @@ -58,7 +57,9 @@ use lightning::ln::script::ShutdownScript; use lightning::ln::types::ChannelId; use lightning::offers::invoice::UnsignedBolt12Invoice; use lightning::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath}; -use lightning::routing::router::{InFlightHtlcs, Path, Route, RouteHop, RouteParameters, Router}; +use lightning::routing::router::{ + InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters, Router, +}; use lightning::sign::{ EntropySource, InMemorySigner, KeyMaterial, NodeSigner, Recipient, SignerProvider, }; @@ -82,6 +83,7 @@ use bitcoin::secp256k1::{self, Message, PublicKey, Scalar, Secp256k1, SecretKey} use lightning::io::Cursor; use std::cmp::{self, Ordering}; +use std::collections::VecDeque; use std::mem; use std::sync::atomic; use std::sync::{Arc, Mutex}; @@ -112,13 +114,18 @@ impl FeeEstimator for FuzzEstimator { } } -struct FuzzRouter {} +struct FuzzRouter { + pub next_routes: Mutex>, +} impl Router for FuzzRouter { fn find_route( &self, _payer: &PublicKey, _params: &RouteParameters, _first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs, ) -> Result { + if let Some(route) = self.next_routes.lock().unwrap().pop_front() { + return Ok(route); + } Err(msgs::LightningError { err: String::from("Not implemented"), action: msgs::ErrorAction::IgnoreError, @@ -434,6 +441,34 @@ impl KeyProvider { } } +// Returns a bool indicating whether the payment failed. +#[inline] +fn check_payment_send_events( + source: &ChanMan, amt: u64, min_sendable: u64, max_sendable: u64, +) -> bool { + let mut payment_failed = false; + let events = source.get_and_clear_pending_events(); + assert!(events.len() == 2 || events.len() == 0); + for ev in events { + match ev { + events::Event::PaymentPathFailed { + failure: events::PathFailure::InitialSend { err }, + .. + } => { + check_api_err(err, amt > max_sendable || amt < min_sendable); + }, + events::Event::PaymentFailed { .. } => {}, + _ => panic!(), + }; + payment_failed = true; + } + // Note that while the max is a strict upper-bound, we can occasionally send substantially + // below the minimum, with some gap which is unusable immediately below the minimum. Thus, + // we don't check against min_value_sendable here. + assert!(payment_failed || (amt <= max_sendable)); + payment_failed +} + #[inline] fn check_api_err(api_err: APIError, sendable_bounds_violated: bool) { match api_err { @@ -460,34 +495,6 @@ fn check_api_err(api_err: APIError, sendable_bounds_violated: bool) { }, } } -#[inline] -fn check_payment_err(send_err: PaymentSendFailure, sendable_bounds_violated: bool) { - match send_err { - PaymentSendFailure::ParameterError(api_err) => { - check_api_err(api_err, sendable_bounds_violated) - }, - PaymentSendFailure::PathParameterError(per_path_results) => { - for res in per_path_results { - if let Err(api_err) = res { - check_api_err(api_err, sendable_bounds_violated); - } - } - }, - PaymentSendFailure::AllFailedResendSafe(per_path_results) => { - for api_err in per_path_results { - check_api_err(api_err, sendable_bounds_violated); - } - }, - PaymentSendFailure::PartialFailure { results, .. } => { - for res in results { - if let Err(api_err) = res { - check_api_err(api_err, sendable_bounds_violated); - } - } - }, - PaymentSendFailure::DuplicatePayment => panic!(), - } -} type ChanMan<'a> = ChannelManager< Arc, @@ -546,34 +553,36 @@ fn send_payment( .find(|chan| chan.short_channel_id == Some(dest_chan_id)) .map(|chan| (chan.next_outbound_htlc_minimum_msat, chan.next_outbound_htlc_limit_msat)) .unwrap_or((0, 0)); - if let Err(err) = source.send_payment_with_route( - Route { - paths: vec![Path { - hops: vec![RouteHop { - pubkey: dest.get_our_node_id(), - node_features: dest.node_features(), - short_channel_id: dest_chan_id, - channel_features: dest.channel_features(), - fee_msat: amt, - cltv_expiry_delta: 200, - maybe_announced_channel: true, - }], - blinded_tail: None, + let mut next_routes = source.router.next_routes.lock().unwrap(); + next_routes.push_back(Route { + paths: vec![Path { + hops: vec![RouteHop { + pubkey: dest.get_our_node_id(), + node_features: dest.node_features(), + short_channel_id: dest_chan_id, + channel_features: dest.channel_features(), + fee_msat: amt, + cltv_expiry_delta: 200, + maybe_announced_channel: true, }], - route_params: None, - }, + blinded_tail: None, + }], + route_params: None, + }); + let route_params = RouteParameters::from_payment_params_and_value( + PaymentParameters::from_node_id(source.get_our_node_id(), TEST_FINAL_CLTV), + amt, + ); + if let Err(err) = source.send_payment( payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id), + route_params, + Retry::Attempts(0), ) { - check_payment_err(err, amt > max_value_sendable || amt < min_value_sendable); - false + panic!("Errored with {:?} on initial payment send", err); } else { - // Note that while the max is a strict upper-bound, we can occasionally send substantially - // below the minimum, with some gap which is unusable immediately below the minimum. Thus, - // we don't check against min_value_sendable here. - assert!(amt <= max_value_sendable); - true + check_payment_send_events(source, amt, min_value_sendable, max_value_sendable) } } @@ -615,46 +624,48 @@ fn send_hop_payment( .map(|chan| (chan.next_outbound_htlc_minimum_msat, chan.next_outbound_htlc_limit_msat)) .unwrap_or((0, 0)); let first_hop_fee = 50_000; - if let Err(err) = source.send_payment_with_route( - Route { - paths: vec![Path { - hops: vec![ - RouteHop { - pubkey: middle.get_our_node_id(), - node_features: middle.node_features(), - short_channel_id: middle_chan_id, - channel_features: middle.channel_features(), - fee_msat: first_hop_fee, - cltv_expiry_delta: 100, - maybe_announced_channel: true, - }, - RouteHop { - pubkey: dest.get_our_node_id(), - node_features: dest.node_features(), - short_channel_id: dest_chan_id, - channel_features: dest.channel_features(), - fee_msat: amt, - cltv_expiry_delta: 200, - maybe_announced_channel: true, - }, - ], - blinded_tail: None, - }], - route_params: None, - }, + let mut next_routes = source.router.next_routes.lock().unwrap(); + next_routes.push_back(Route { + paths: vec![Path { + hops: vec![ + RouteHop { + pubkey: middle.get_our_node_id(), + node_features: middle.node_features(), + short_channel_id: middle_chan_id, + channel_features: middle.channel_features(), + fee_msat: first_hop_fee, + cltv_expiry_delta: 100, + maybe_announced_channel: true, + }, + RouteHop { + pubkey: dest.get_our_node_id(), + node_features: dest.node_features(), + short_channel_id: dest_chan_id, + channel_features: dest.channel_features(), + fee_msat: amt, + cltv_expiry_delta: 200, + maybe_announced_channel: true, + }, + ], + blinded_tail: None, + }], + route_params: None, + }); + let route_params = RouteParameters::from_payment_params_and_value( + PaymentParameters::from_node_id(source.get_our_node_id(), TEST_FINAL_CLTV), + amt, + ); + if let Err(err) = source.send_payment( payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id), + route_params, + Retry::Attempts(0), ) { - let sent_amt = amt + first_hop_fee; - check_payment_err(err, sent_amt < min_value_sendable || sent_amt > max_value_sendable); - false + panic!("Errored with {:?} on initial payment send", err); } else { - // Note that while the max is a strict upper-bound, we can occasionally send substantially - // below the minimum, with some gap which is unusable immediately below the minimum. Thus, - // we don't check against min_value_sendable here. - assert!(amt + first_hop_fee <= max_value_sendable); - true + let sent_amt = amt + first_hop_fee; + check_payment_send_events(source, sent_amt, min_value_sendable, max_value_sendable) } } @@ -662,7 +673,7 @@ fn send_hop_payment( pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { let out = SearchingOutput::new(underlying_out); let broadcast = Arc::new(TestBroadcaster {}); - let router = FuzzRouter {}; + let router = FuzzRouter { next_routes: Mutex::new(VecDeque::new()) }; macro_rules! make_node { ($node_id: expr, $fee_estimator: expr) => {{ diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a01caec3699..7385c829852 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -125,7 +125,9 @@ use core::time::Duration; use core::ops::Deref; use bitcoin::hex::impl_fmt_traits; // Re-export this for use in the public API. -pub use crate::ln::outbound_payment::{Bolt12PaymentError, PaymentSendFailure, ProbeSendFailure, Retry, RetryableSendFailure, RecipientOnionFields}; +pub use crate::ln::outbound_payment::{Bolt12PaymentError, ProbeSendFailure, Retry, RetryableSendFailure, RecipientOnionFields}; +#[cfg(test)] +pub(crate) use crate::ln::outbound_payment::PaymentSendFailure; use crate::ln::script::ShutdownScript; // We hold various information about HTLC relay in the HTLC objects in Channel itself: @@ -2363,7 +2365,9 @@ where fee_estimator: LowerBoundedFeeEstimator, chain_monitor: M, tx_broadcaster: T, - #[allow(unused)] + #[cfg(fuzzing)] + pub router: R, + #[cfg(not(fuzzing))] router: R, message_router: MR, @@ -4525,8 +4529,11 @@ where // [`TestRouter::expect_find_route`] instead. // // [`TestRouter::expect_find_route`]: crate::util::test_utils::TestRouter::expect_find_route - #[cfg(any(test, fuzzing))] - pub fn send_payment_with_route(&self, route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId) -> Result<(), PaymentSendFailure> { + #[cfg(test)] + pub(crate) fn send_payment_with_route( + &self, route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, + payment_id: PaymentId + ) -> Result<(), PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height; let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); self.pending_outbound_payments diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 2cfee6221c0..2f3bb3378ed 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -482,7 +482,7 @@ pub enum RetryableSendFailure { /// as the Err() type describing which state the payment is in, see the description of individual /// enum states for more. #[derive(Clone, Debug, PartialEq, Eq)] -pub enum PaymentSendFailure { +pub(crate) enum PaymentSendFailure { /// A parameter which was passed to send_payment was invalid, preventing us from attempting to /// send the payment at all. ///