Merge branch 'lightningdevkit:main' into main

Author: Sharmalm

fuzz/src/chanmon_consistency.rs

@@ -295,7 +295,7 @@ fn check_api_err(api_err: APIError) {
 			// all others. If you hit this panic, the list of acceptable errors
 			// is probably just stale and you should add new messages here.
 			match err.as_str() {
-				"Peer for first hop currently disconnected/pending monitor update!" => {},
+				"Peer for first hop currently disconnected" => {},
 				_ if err.starts_with("Cannot push more than their max accepted HTLCs ") => {},
 				_ if err.starts_with("Cannot send value that would put us over the max HTLC value in flight our peer will accept ") => {},
 				_ if err.starts_with("Cannot send value that would put our balance under counterparty-announced channel reserve value") => {},
@@ -474,8 +474,8 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 	let mut channel_txn = Vec::new();
 	macro_rules! make_channel {
 		($source: expr, $dest: expr, $chan_id: expr) => { {
-			$source.peer_connected(&$dest.get_our_node_id(), &Init { features: $dest.init_features(), remote_network_address: None }).unwrap();
-			$dest.peer_connected(&$source.get_our_node_id(), &Init { features: $source.init_features(), remote_network_address: None }).unwrap();
+			$source.peer_connected(&$dest.get_our_node_id(), &Init { features: $dest.init_features(), remote_network_address: None }, true).unwrap();
+			$dest.peer_connected(&$source.get_our_node_id(), &Init { features: $source.init_features(), remote_network_address: None }, false).unwrap();
 
 			$source.create_channel($dest.get_our_node_id(), 100_000, 42, 0, None).unwrap();
 			let open_channel = {
@@ -979,31 +979,31 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 
 			0x0c => {
 				if !chan_a_disconnected {
-					nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
-					nodes[1].peer_disconnected(&nodes[0].get_our_node_id(), false);
+					nodes[0].peer_disconnected(&nodes[1].get_our_node_id());
+					nodes[1].peer_disconnected(&nodes[0].get_our_node_id());
 					chan_a_disconnected = true;
 					drain_msg_events_on_disconnect!(0);
 				}
 			},
 			0x0d => {
 				if !chan_b_disconnected {
-					nodes[1].peer_disconnected(&nodes[2].get_our_node_id(), false);
-					nodes[2].peer_disconnected(&nodes[1].get_our_node_id(), false);
+					nodes[1].peer_disconnected(&nodes[2].get_our_node_id());
+					nodes[2].peer_disconnected(&nodes[1].get_our_node_id());
 					chan_b_disconnected = true;
 					drain_msg_events_on_disconnect!(2);
 				}
 			},
 			0x0e => {
 				if chan_a_disconnected {
-					nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }).unwrap();
-					nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: nodes[0].init_features(), remote_network_address: None }).unwrap();
+					nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }, true).unwrap();
+					nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: nodes[0].init_features(), remote_network_address: None }, false).unwrap();
 					chan_a_disconnected = false;
 				}
 			},
 			0x0f => {
 				if chan_b_disconnected {
-					nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: nodes[2].init_features(), remote_network_address: None }).unwrap();
-					nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }).unwrap();
+					nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: nodes[2].init_features(), remote_network_address: None }, true).unwrap();
+					nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }, false).unwrap();
 					chan_b_disconnected = false;
 				}
 			},
@@ -1040,7 +1040,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 
 			0x2c => {
 				if !chan_a_disconnected {
-					nodes[1].peer_disconnected(&nodes[0].get_our_node_id(), false);
+					nodes[1].peer_disconnected(&nodes[0].get_our_node_id());
 					chan_a_disconnected = true;
 					drain_msg_events_on_disconnect!(0);
 				}
@@ -1054,14 +1054,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 			},
 			0x2d => {
 				if !chan_a_disconnected {
-					nodes[0].peer_disconnected(&nodes[1].get_our_node_id(), false);
+					nodes[0].peer_disconnected(&nodes[1].get_our_node_id());
 					chan_a_disconnected = true;
 					nodes[0].get_and_clear_pending_msg_events();
 					ab_events.clear();
 					ba_events.clear();
 				}
 				if !chan_b_disconnected {
-					nodes[2].peer_disconnected(&nodes[1].get_our_node_id(), false);
+					nodes[2].peer_disconnected(&nodes[1].get_our_node_id());
 					chan_b_disconnected = true;
 					nodes[2].get_and_clear_pending_msg_events();
 					bc_events.clear();
@@ -1073,7 +1073,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 			},
 			0x2e => {
 				if !chan_b_disconnected {
-					nodes[1].peer_disconnected(&nodes[2].get_our_node_id(), false);
+					nodes[1].peer_disconnected(&nodes[2].get_our_node_id());
 					chan_b_disconnected = true;
 					drain_msg_events_on_disconnect!(2);
 				}
@@ -1198,13 +1198,13 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 
 				// Next, make sure peers are all connected to each other
 				if chan_a_disconnected {
-					nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }).unwrap();
-					nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: nodes[0].init_features(), remote_network_address: None }).unwrap();
+					nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }, true).unwrap();
+					nodes[1].peer_connected(&nodes[0].get_our_node_id(), &Init { features: nodes[0].init_features(), remote_network_address: None }, false).unwrap();
 					chan_a_disconnected = false;
 				}
 				if chan_b_disconnected {
-					nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: nodes[2].init_features(), remote_network_address: None }).unwrap();
-					nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }).unwrap();
+					nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init { features: nodes[2].init_features(), remote_network_address: None }, true).unwrap();
+					nodes[2].peer_connected(&nodes[1].get_our_node_id(), &Init { features: nodes[1].init_features(), remote_network_address: None }, false).unwrap();
 					chan_b_disconnected = false;
 				}
 

fuzz/src/full_stack.rs

@@ -634,11 +634,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 					if let Err(e) = channelmanager.funding_transaction_generated(&funding_generation.0, &funding_generation.1, tx.clone()) {
 						// It's possible the channel has been closed in the mean time, but any other
 						// failure may be a bug.
-						if let APIError::ChannelUnavailable { err } = e {
-							if !err.starts_with("Can't find a peer matching the passed counterparty node_id ") {
-								assert_eq!(err, "No such channel");
-							}
-						} else { panic!(); }
+						if let APIError::ChannelUnavailable { .. } = e { } else { panic!(); }
 					}
 					pending_funding_signatures.insert(funding_output, tx);
 				}

fuzz/src/indexedmap.rs

@@ -13,14 +13,27 @@ use hashbrown::HashSet;
 
 use crate::utils::test_logger;
 
-fn check_eq(btree: &BTreeMap<u8, u8>, indexed: &IndexedMap<u8, u8>) {
+use std::ops::{RangeBounds, Bound};
+
+struct ExclLowerInclUpper(u8, u8);
+impl RangeBounds<u8> for ExclLowerInclUpper {
+	fn start_bound(&self) -> Bound<&u8> { Bound::Excluded(&self.0) }
+	fn end_bound(&self) -> Bound<&u8> { Bound::Included(&self.1) }
+}
+struct ExclLowerExclUpper(u8, u8);
+impl RangeBounds<u8> for ExclLowerExclUpper {
+	fn start_bound(&self) -> Bound<&u8> { Bound::Excluded(&self.0) }
+	fn end_bound(&self) -> Bound<&u8> { Bound::Excluded(&self.1) }
+}
+
+fn check_eq(btree: &BTreeMap<u8, u8>, mut indexed: IndexedMap<u8, u8>) {
 	assert_eq!(btree.len(), indexed.len());
 	assert_eq!(btree.is_empty(), indexed.is_empty());
 
 	let mut btree_clone = btree.clone();
 	assert!(btree_clone == *btree);
 	let mut indexed_clone = indexed.clone();
-	assert!(indexed_clone == *indexed);
+	assert!(indexed_clone == indexed);
 
 	for k in 0..=255 {
 		assert_eq!(btree.contains_key(&k), indexed.contains_key(&k));
@@ -43,16 +56,27 @@ fn check_eq(btree: &BTreeMap<u8, u8>, indexed: &IndexedMap<u8, u8>) {
 	}
 
 	const STRIDE: u8 = 16;
-	for k in 0..=255/STRIDE {
-		let lower_bound = k * STRIDE;
-		let upper_bound = lower_bound + (STRIDE - 1);
-		let mut btree_iter = btree.range(lower_bound..=upper_bound);
-		let mut indexed_iter = indexed.range(lower_bound..=upper_bound);
-		loop {
-			let b_v = btree_iter.next();
-			let i_v = indexed_iter.next();
-			assert_eq!(b_v, i_v);
-			if b_v.is_none() { break; }
+	for range_type in 0..4 {
+		for k in 0..=255/STRIDE {
+			let lower_bound = k * STRIDE;
+			let upper_bound = lower_bound + (STRIDE - 1);
+			macro_rules! range { ($map: expr) => {
+				match range_type {
+					0 => $map.range(lower_bound..upper_bound),
+					1 => $map.range(lower_bound..=upper_bound),
+					2 => $map.range(ExclLowerInclUpper(lower_bound, upper_bound)),
+					3 => $map.range(ExclLowerExclUpper(lower_bound, upper_bound)),
+					_ => unreachable!(),
+				}
+			} }
+			let mut btree_iter = range!(btree);
+			let mut indexed_iter = range!(indexed);
+			loop {
+				let b_v = btree_iter.next();
+				let i_v = indexed_iter.next();
+				assert_eq!(b_v, i_v);
+				if b_v.is_none() { break; }
+			}
 		}
 	}
 
@@ -91,15 +115,15 @@ pub fn do_test(data: &[u8]) {
 		let prev_value_i = indexed.insert(tuple[0], tuple[1]);
 		assert_eq!(prev_value_b, prev_value_i);
 	}
-	check_eq(&btree, &indexed);
+	check_eq(&btree, indexed.clone());
 
 	// Now, modify the maps in all the ways we have to do so, checking that the maps remain
 	// equivalent as we go.
 	for (k, v) in indexed.unordered_iter_mut() {
 		*v = *k;
 		*btree.get_mut(k).unwrap() = *k;
 	}
-	check_eq(&btree, &indexed);
+	check_eq(&btree, indexed.clone());
 
 	for k in 0..=255 {
 		match btree.entry(k) {
@@ -124,7 +148,7 @@ pub fn do_test(data: &[u8]) {
 			},
 		}
 	}
-	check_eq(&btree, &indexed);
+	check_eq(&btree, indexed);
 }
 
 pub fn indexedmap_test<Out: test_logger::Output>(data: &[u8], _out: Out) {

lightning-background-processor/src/lib.rs

@@ -37,7 +37,6 @@ use lightning::util::events::{Event, EventHandler, EventsProvider};
 use lightning::util::logger::Logger;
 use lightning::util::persist::Persister;
 use lightning_rapid_gossip_sync::RapidGossipSync;
-use lightning::io;
 
 use core::ops::Deref;
 use core::time::Duration;
@@ -431,7 +430,7 @@ pub async fn process_events_async<
 	persister: PS, event_handler: EventHandler, chain_monitor: M, channel_manager: CM,
 	gossip_sync: GossipSync<PGS, RGS, G, UL, L>, peer_manager: PM, logger: L, scorer: Option<S>,
 	sleeper: Sleeper,
-) -> Result<(), io::Error>
+) -> Result<(), lightning::io::Error>
 where
 	UL::Target: 'static + UtxoLookup,
 	CF::Target: 'static + chain::Filter,
@@ -963,8 +962,8 @@ mod tests {
 
 		for i in 0..num_nodes {
 			for j in (i+1)..num_nodes {
-				nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &Init { features: nodes[j].node.init_features(), remote_network_address: None }).unwrap();
-				nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &Init { features: nodes[i].node.init_features(), remote_network_address: None }).unwrap();
+				nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &Init { features: nodes[j].node.init_features(), remote_network_address: None }, true).unwrap();
+				nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &Init { features: nodes[i].node.init_features(), remote_network_address: None }, false).unwrap();
 			}
 		}
 

lightning-invoice/src/payment.rs

@@ -17,7 +17,7 @@ use lightning::chain;
 use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use lightning::chain::keysinterface::{NodeSigner, SignerProvider, EntropySource};
 use lightning::ln::{PaymentHash, PaymentSecret};
-use lightning::ln::channelmanager::{ChannelManager, PaymentId, PaymentSendFailure, Retry};
+use lightning::ln::channelmanager::{ChannelManager, PaymentId, Retry, RetryableSendFailure};
 use lightning::routing::router::{PaymentParameters, RouteParameters, Router};
 use lightning::util::logger::Logger;
 
@@ -172,7 +172,7 @@ pub enum PaymentError {
 	/// An error resulting from the provided [`Invoice`] or payment hash.
 	Invoice(&'static str),
 	/// An error occurring when sending a payment.
-	Sending(PaymentSendFailure),
+	Sending(RetryableSendFailure),
 }
 
 /// A trait defining behavior of an [`Invoice`] payer.

lightning-invoice/src/utils.rs

@@ -842,13 +842,13 @@ mod test {
 
 		// With only one sufficient-value peer connected we should only get its hint
 		scid_aliases.remove(&chan_b.0.short_channel_id_alias.unwrap());
-		nodes[0].node.peer_disconnected(&nodes[2].node.get_our_node_id(), false);
+		nodes[0].node.peer_disconnected(&nodes[2].node.get_our_node_id());
 		match_invoice_routes(Some(1_000_000_000), &nodes[0], scid_aliases.clone());
 
 		// If we don't have any sufficient-value peers connected we should get all hints with
 		// sufficient value, even though there is a connected insufficient-value peer.
 		scid_aliases.insert(chan_b.0.short_channel_id_alias.unwrap());
-		nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+		nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
 		match_invoice_routes(Some(1_000_000_000), &nodes[0], scid_aliases);
 	}
 

lightning-net-tokio/src/lib.rs

@@ -617,7 +617,7 @@ mod tests {
 		fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
 		fn get_next_channel_announcement(&self, _starting_point: u64) -> Option<(ChannelAnnouncement, Option<ChannelUpdate>, Option<ChannelUpdate>)> { None }
 		fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<NodeAnnouncement> { None }
-		fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> { Ok(()) }
+		fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init, _inbound: bool) -> Result<(), ()> { Ok(()) }
 		fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: ReplyChannelRange) -> Result<(), LightningError> { Ok(()) }
 		fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) }
 		fn handle_query_channel_range(&self, _their_node_id: &PublicKey, _msg: QueryChannelRange) -> Result<(), LightningError> { Ok(()) }
@@ -643,13 +643,13 @@ mod tests {
 		fn handle_update_fee(&self, _their_node_id: &PublicKey, _msg: &UpdateFee) {}
 		fn handle_announcement_signatures(&self, _their_node_id: &PublicKey, _msg: &AnnouncementSignatures) {}
 		fn handle_channel_update(&self, _their_node_id: &PublicKey, _msg: &ChannelUpdate) {}
-		fn peer_disconnected(&self, their_node_id: &PublicKey, _no_connection_possible: bool) {
+		fn peer_disconnected(&self, their_node_id: &PublicKey) {
 			if *their_node_id == self.expected_pubkey {
 				self.disconnected_flag.store(true, Ordering::SeqCst);
 				self.pubkey_disconnected.clone().try_send(()).unwrap();
 			}
 		}
-		fn peer_connected(&self, their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> {
+		fn peer_connected(&self, their_node_id: &PublicKey, _init_msg: &Init, _inbound: bool) -> Result<(), ()> {
 			if *their_node_id == self.expected_pubkey {
 				self.pubkey_connected.clone().try_send(()).unwrap();
 			}

lightning/src/chain/chainmonitor.rs

@@ -796,7 +796,7 @@ mod tests {
 	use crate::ln::functional_test_utils::*;
 	use crate::ln::msgs::ChannelMessageHandler;
 	use crate::util::errors::APIError;
-	use crate::util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
+	use crate::util::events::{Event, ClosureReason, MessageSendEvent, MessageSendEventsProvider};
 
 	#[test]
 	fn test_async_ooo_offchain_updates() {
@@ -819,10 +819,8 @@ mod tests {
 
 		nodes[1].node.claim_funds(payment_preimage_1);
 		check_added_monitors!(nodes[1], 1);
-		expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 		nodes[1].node.claim_funds(payment_preimage_2);
 		check_added_monitors!(nodes[1], 1);
-		expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
 
 		let persistences = chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clone();
 		assert_eq!(persistences.len(), 1);
@@ -850,8 +848,24 @@ mod tests {
 			.find(|(txo, _)| txo == funding_txo).unwrap().1.contains(&next_update));
 		assert!(nodes[1].chain_monitor.release_pending_monitor_events().is_empty());
 		assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+		assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
 		nodes[1].chain_monitor.chain_monitor.channel_monitor_updated(*funding_txo, update_iter.next().unwrap().clone()).unwrap();
 
+		let claim_events = nodes[1].node.get_and_clear_pending_events();
+		assert_eq!(claim_events.len(), 2);
+		match claim_events[0] {
+			Event::PaymentClaimed { ref payment_hash, amount_msat: 1_000_000, .. } => {
+				assert_eq!(payment_hash_1, *payment_hash);
+			},
+			_ => panic!("Unexpected event"),
+		}
+		match claim_events[1] {
+			Event::PaymentClaimed { ref payment_hash, amount_msat: 1_000_000, .. } => {
+				assert_eq!(payment_hash_2, *payment_hash);
+			},
+			_ => panic!("Unexpected event"),
+		}
+
 		// Now manually walk the commitment signed dance - because we claimed two payments
 		// back-to-back it doesn't fit into the neat walk commitment_signed_dance does.