Make helper function for creating inflight htlc map

Author: jurvis

lightning-invoice/src/payment.rs

@@ -495,43 +495,7 @@ where
 
 		let payer = self.payer.node_id();
 		let first_hops = self.payer.first_hops();
-		let inflight_htlcs = {
-			// Make an attempt at finding existing payment information from `payment_cache`. If it
-			// does not exist, it probably is a fresh payment and we can just return an empty
-			// HashMap.
-			if let Some(payment_info) = self.payment_cache.lock().unwrap().get(&payment_hash) {
-				let mut total_inflight_map: HashMap<(u64, bool), u64> = HashMap::new();
-
-				for path in &payment_info.paths {
-					if path.is_empty() { break };
-					// total_inflight_map needs to be direction-sensitive when keeping track of the HTLC value
-					// that is held up. However, the `hops` array, which is a path returned by `find_route` in
-					// the router excludes the payer node. In the following lines, the payer's information is
-					// hardcoded with an inflight value of 0 so that we can correctly represent the first hop
-					// in our sliding window of two.
-					let our_node_id: PublicKey = self.payer.node_id();
-					let reversed_hops_with_payer = core::iter::once((0u64, our_node_id)).chain(
-						path.split_last().unwrap().1.iter().map(|hop| (hop.fee_msat, hop.pubkey))).rev();
-					let mut cumulative_msat = path.last().unwrap().fee_msat;
-
-					// Taking the reversed vector from above, we zip it with just the reversed hops list to
-					// work "backwards" of the given path, since the last hop's `fee_msat` actually represents
-					// the total amount sent.
-					for (next_hop, prev_hop) in path.iter().rev().zip(reversed_hops_with_payer) {
-						cumulative_msat += prev_hop.0;
-						total_inflight_map
-							.entry((next_hop.short_channel_id, next_hop.pubkey < prev_hop.1))
-							.and_modify(|used_liquidity_msat| *used_liquidity_msat += cumulative_msat)
-							.or_insert(cumulative_msat);
-					}
-				}
-
-				total_inflight_map
-			} else {
-				HashMap::new()
-			}
-		};
-
+		let inflight_htlcs = self.create_inflight_map(&payment_hash);
 		let route = self.router.find_route(
 				&payer, &params, &payment_hash, Some(&first_hops.iter().collect::<Vec<_>>()),
 				&AccountForInFlightHtlcs { scorer: &mut self.scorer.lock(), inflight_htlcs }
@@ -631,7 +595,13 @@ where
 	) -> Result<(), ()> {
 		let attempts = self.payment_cache.lock().unwrap().entry(payment_hash)
 			.and_modify(|info| info.attempts.count += 1 )
-			.or_insert({ PaymentInfo::new() }).attempts;
+			.or_insert(PaymentInfo {
+				attempts: PaymentAttempts {
+					count: 1,
+					first_attempted_at: T::now(),
+				},
+				paths: vec![],
+			}).attempts;
 
 		if !self.retry.is_retryable_now(&attempts) {
 			log_trace!(self.logger, "Payment {} exceeded maximum attempts; not retrying ({})", log_bytes!(payment_hash.0), attempts);
@@ -647,42 +617,7 @@ where
 
 		let payer = self.payer.node_id();
 		let first_hops = self.payer.first_hops();
-		let inflight_htlcs = {
-			// Make an attempt at finding existing payment information from `payment_cache`. If it
-			// does not exist, it probably is a fresh payment and we can just return an empty
-			// HashMap.
-			if let Some(payment_info) = self.payment_cache.lock().unwrap().get(&payment_hash) {
-				let mut total_inflight_map: HashMap<(u64, bool), u64> = HashMap::new();
-
-				for hops in &payment_info.paths {
-					if hops.len() < 1 { break };
-					// total_inflight_map needs to be direction-sensitive when keeping track of the HTLC value
-					// that is held up. However, the `hops` array, which is a path returned by `find_route` in
-					// the router excludes the payer node. In the following lines, the payer's information is
-					// hardcoded with an inflight value of 0 so that we can correctly represent the first hop
-					// in our sliding window of two.
-					let our_node_id: PublicKey = self.payer.node_id();
-					let reversed_hops_with_payer = core::iter::once((0u64, our_node_id)).chain(
-						hops.split_last().unwrap().1.iter().map(|hop| (hop.fee_msat, hop.pubkey))).rev();
-					let mut cumulative_msat = hops.last().unwrap().fee_msat;
-
-					// Taking the reversed vector from above, we zip it with just the reversed hops list to
-					// work "backwards" of the given path, since the last hop's `fee_msat` actually represents
-					// the total amount sent.
-					for (next_hop, prev_hop) in hops.iter().rev().zip(reversed_hops_with_payer) {
-						cumulative_msat += prev_hop.0;
-						total_inflight_map
-							.entry((next_hop.short_channel_id, next_hop.pubkey < prev_hop.1))
-							.and_modify(|used_liquidity_msat| *used_liquidity_msat += cumulative_msat)
-							.or_insert(cumulative_msat);
-					}
-				}
-
-				total_inflight_map
-			} else {
-				HashMap::new()
-			}
-		};
+		let inflight_htlcs = self.create_inflight_map(&payment_hash);
 
 		let route = {
 			let scorer = AccountForInFlightHtlcs { scorer: &mut self.scorer.lock(), inflight_htlcs };
@@ -693,13 +628,11 @@ where
 			)
 		};
 
-
 		if route.is_err() {
 			log_trace!(self.logger, "Failed to find a route for payment {}; not retrying ({:})", log_bytes!(payment_hash.0), attempts);
 			return Err(());
 		}
 
-
 		match self.payer.retry_payment(&route.unwrap(), payment_id) {
 			Ok(()) => Ok(()),
 			Err(PaymentSendFailure::ParameterError(_)) |
@@ -727,6 +660,49 @@ where
 	pub fn remove_cached_payment(&self, payment_hash: &PaymentHash) {
 		self.payment_cache.lock().unwrap().remove(payment_hash);
 	}
+
+	/// Given a [`PaymentHash`], this function looks up inflight path attempts in the payment_cache.
+	/// Then, it uses the path information inside the cache to construct a HashMap mapping a channel's
+	/// short channel id and direction to the amount being sent through it.
+	///
+	/// This function should be called whenever we need information about currently used up liquidity
+	/// across payments.
+	pub fn create_inflight_map(&self, payment_hash: &PaymentHash) -> HashMap<(u64, bool), u64> {
+		// Make an attempt at finding existing payment information from `payment_cache`. If it
+		// does not exist, it probably is a fresh payment and we can just return an empty
+		// HashMap.
+		if let Some(payment_info) = self.payment_cache.lock().unwrap().get(&payment_hash) {
+			let mut total_inflight_map: HashMap<(u64, bool), u64> = HashMap::new();
+
+			for path in &payment_info.paths {
+				if path.is_empty() { break };
+				// total_inflight_map needs to be direction-sensitive when keeping track of the HTLC value
+				// that is held up. However, the `hops` array, which is a path returned by `find_route` in
+				// the router excludes the payer node. In the following lines, the payer's information is
+				// hardcoded with an inflight value of 0 so that we can correctly represent the first hop
+				// in our sliding window of two.
+				let our_node_id: PublicKey = self.payer.node_id();
+				let reversed_hops_with_payer = core::iter::once((0u64, our_node_id)).chain(
+					path.split_last().unwrap().1.iter().map(|hop| (hop.fee_msat, hop.pubkey))).rev();
+				let mut cumulative_msat = path.last().unwrap().fee_msat;
+
+				// Taking the reversed vector from above, we zip it with just the reversed hops list to
+				// work "backwards" of the given path, since the last hop's `fee_msat` actually represents
+				// the total amount sent.
+				for (next_hop, prev_hop) in path.iter().rev().zip(reversed_hops_with_payer) {
+					cumulative_msat += prev_hop.0;
+					total_inflight_map
+						.entry((next_hop.short_channel_id, next_hop.pubkey < prev_hop.1))
+						.and_modify(|used_liquidity_msat| *used_liquidity_msat += cumulative_msat)
+						.or_insert(cumulative_msat);
+				}
+			}
+
+			return total_inflight_map
+		} else {
+			return HashMap::new()
+		}
+	}
 }
 
 fn expiry_time_from_unix_epoch(invoice: &Invoice) -> Duration {