Remove inflight HTLC value from map on PaymentPathFailed/Successful

When processing a `PaymentPathSuccessful` event, the associated inflight
HTLC value is subtracted from the map, `total_inflight_map`.

On `PaymentPathFailed`, when a payment is abandoned, we subtract the
previously-inflight value from any existing records in the map.

Remove need to explicitly drop locks

Author: jurvis

lightning-invoice/src/payment.rs

@@ -517,6 +517,23 @@ where
 		}
 	}
 
+	fn remove_path_inflight_htlcs(&self, hops: &Vec<RouteHop>) {
+		// Here, just like `process_path_inflight_htlcs`, we are given a vector of hops that excludes
+		// any information about the payer. However, in our `total_inflight_map`, we keep track
+		// of possible inbound/outbound liquidity from the payer as well. Therefore, it is accounted
+		// for by hard-coding the same tuple as in `process_path_inflight_htlcs`
+		let our_node_id: PublicKey = self.payer.node_id();
+		let hops_with_us = core::iter::once((0u64, our_node_id)).chain(hops.iter().map(|hop| (hop.fee_msat, hop.pubkey)));
+		
+		for (next_hop, prev_hop) in hops.iter().zip(hops_with_us) {
+			println!("Subtracting {} inflight msats from {}", next_hop.fee_msat, next_hop.short_channel_id);
+			self.total_inflight_map.lock().unwrap()
+				.entry((next_hop.short_channel_id, next_hop.pubkey < prev_hop.1))
+				.and_modify(|used_liquidity_msat| *used_liquidity_msat = used_liquidity_msat.saturating_sub(next_hop.fee_msat) )
+				.or_insert(0);
+		}
+	}
+
 	fn retry_payment(
 		&self, payment_id: PaymentId, payment_hash: PaymentHash, params: &RouteParameters
 	) -> Result<(), ()> {
@@ -613,22 +630,26 @@ where
 				} else if *rejected_by_dest {
 					log_trace!(self.logger, "Payment {} rejected by destination; not retrying", log_bytes!(payment_hash.0));
 					self.payer.abandon_payment(payment_id.unwrap());
+					self.remove_path_inflight_htlcs(path);
 				} else if retry.is_none() {
 					log_trace!(self.logger, "Payment {} missing retry params; not retrying", log_bytes!(payment_hash.0));
 					self.payer.abandon_payment(payment_id.unwrap());
+					self.remove_path_inflight_htlcs(path);
 				} else if self.retry_payment(payment_id.unwrap(), *payment_hash, retry.as_ref().unwrap()).is_ok() {
 					// We retried at least somewhat, don't provide the PaymentPathFailed event to the user.
 					return;
 				} else {
 					self.payer.abandon_payment(payment_id.unwrap());
+					self.remove_path_inflight_htlcs(path);
 				}
 			},
 			Event::PaymentFailed { payment_hash, .. } => {
 				self.remove_cached_payment(&payment_hash);
 			},
 			Event::PaymentPathSuccessful { path, .. } => {
-				let path = path.iter().collect::<Vec<_>>();
-				self.scorer.lock().payment_path_successful(&path);
+				let path_with_refs = path.iter().collect::<Vec<_>>();
+				self.scorer.lock().payment_path_successful(&path_with_refs);
+				self.remove_path_inflight_htlcs(path);
 			},
 			Event::PaymentSent { payment_hash, .. } => {
 				let mut payment_cache = self.payment_cache.lock().unwrap();
@@ -766,11 +787,32 @@ mod tests {
 		let payment_id = Some(invoice_payer.pay_invoice(&invoice).unwrap());
 		assert_eq!(*payer.attempts.borrow(), 1);
 
+		{
+			let inflight_map = invoice_payer.total_inflight_map.lock().unwrap();
+			// TestRouter returns two hops, with `fee_msat` divided by two. The invoice instantiated above
+			// is hard-coded to test sending an invoice with a value of 128 msats.
+			assert_eq!(inflight_map.get(&(0, true)).unwrap().clone(), 64);
+			assert_eq!(inflight_map.get(&(1, true)).unwrap().clone(), 64);
+		}
+
 		invoice_payer.handle_event(&Event::PaymentSent {
 			payment_id, payment_preimage, payment_hash, fee_paid_msat: None
 		});
 		assert_eq!(*event_handled.borrow(), true);
 		assert_eq!(*payer.attempts.borrow(), 1);
+
+		invoice_payer.handle_event(&Event::PaymentPathSuccessful {
+			payment_id: payment_id.unwrap(), payment_hash: Some(payment_hash),
+			path: TestRouter::route_for_value(final_value_msat).paths[0].clone()
+		});
+		invoice_payer.handle_event(&Event::PaymentPathSuccessful {
+			payment_id: payment_id.unwrap(), payment_hash: Some(payment_hash),
+			path: TestRouter::route_for_value(final_value_msat).paths[1].clone()
+		});
+
+		let inflight_map = invoice_payer.total_inflight_map.lock().unwrap();
+		assert_eq!(inflight_map.get(&(0, true)).unwrap().clone(), 0);
+		assert_eq!(inflight_map.get(&(1, true)).unwrap().clone(), 0);
 	}
 
 	#[test]
@@ -808,12 +850,28 @@ mod tests {
 		invoice_payer.handle_event(&event);
 		assert_eq!(*event_handled.borrow(), false);
 		assert_eq!(*payer.attempts.borrow(), 2);
+		let inflight_map = invoice_payer.total_inflight_map.lock().unwrap();
+		assert_eq!(inflight_map.get(&(0, true)).unwrap().clone(), 64);
+		assert_eq!(inflight_map.get(&(1, true)).unwrap().clone(), 64);
+		drop(inflight_map);
 
 		invoice_payer.handle_event(&Event::PaymentSent {
 			payment_id, payment_preimage, payment_hash, fee_paid_msat: None
 		});
 		assert_eq!(*event_handled.borrow(), true);
 		assert_eq!(*payer.attempts.borrow(), 2);
+
+		invoice_payer.handle_event(&Event::PaymentPathSuccessful {
+			payment_id: payment_id.unwrap(), payment_hash: Some(payment_hash),
+			path: TestRouter::route_for_value(final_value_msat).paths[0].clone()
+		});
+		invoice_payer.handle_event(&Event::PaymentPathSuccessful {
+			payment_id: payment_id.unwrap(), payment_hash: Some(payment_hash),
+			path: TestRouter::route_for_value(final_value_msat).paths[1].clone()
+		});
+		let inflight_map = invoice_payer.total_inflight_map.lock().unwrap();
+		assert_eq!(inflight_map.get(&(0, true)).unwrap().clone(), 0);
+		assert_eq!(inflight_map.get(&(1, true)).unwrap().clone(), 0);
 	}
 
 	#[test]