payments: Better error message if paying ourselves

At least twice now, our users have reported that they encountered
"We've already tried paying this invoice" when in fact they were trying
to pay themselves... This improves the error message to distinguish
between paying someone else's invoice twice vs trying to pay oneself.

- Adds `PaymentsManager::get_payment`.
- Removes `PaymentsManager::contains_payment_id` which requires a
  `finalized` `HashSet` which grows indefinitely

Author: MaxFangX

lexe-ln/src/command.rs

@@ -46,7 +46,7 @@ use lexe_api::{
         Empty,
         invoice::LxInvoice,
         offer::{LxOffer, MaxQuantity},
-        payments::LxPaymentId,
+        payments::{LxPaymentId, PaymentDirection},
     },
     vfs::{REVOCABLE_CLIENTS_FILE_ID, Vfs},
 };
@@ -1287,13 +1287,23 @@ where
 {
     let invoice = req.invoice;
 
-    // Fail expired invoices early.
+    // Fail early if invoice is expired.
     ensure!(!invoice.is_expired(), "Invoice has expired");
 
-    // Fail invoice double-payment early.
+    // Fail early if we already tried paying this invoice,
+    // or we are trying to pay ourselves (yes, users actually do this).
     let payment_id = LxPaymentId::Lightning(invoice.payment_hash());
-    if payments_manager.contains_payment_id(&payment_id).await {
-        bail!("We've already tried paying this invoice");
+    let maybe_existing_payment = payments_manager
+        .get_payment(payment_id)
+        .await
+        .context("Couldn't check for existing payment")?;
+    if let Some(existing_payment) = maybe_existing_payment {
+        match existing_payment.direction() {
+            PaymentDirection::Outbound =>
+                return Err(anyhow!("We've already tried paying this invoice")),
+            // Yes, users actually hit this case...
+            PaymentDirection::Inbound => bail!("We cannot pay ourselves"),
+        }
     }
 
     // Compute the amount; handle amountless invoices.
@@ -1378,7 +1388,7 @@ where
 {
     let offer = req.offer;
 
-    // Fail expired offers early.
+    // Fail early if offer is expired.
     ensure!(!offer.is_expired(), "Offer has expired");
 
     // We only support paying BTC-denominated offers at the moment.
@@ -1387,11 +1397,17 @@ where
         "Fiat-denominated offers are not supported yet"
     );
 
-    // Fail offer double-payment early.
+    // Fail early if we already tried paying with this client ID.
     let payment_id = LxPaymentId::OfferSend(req.cid);
-    if payments_manager.contains_payment_id(&payment_id).await {
-        bail!("We've already tried paying this offer");
-    }
+    let maybe_existing_payment = payments_manager
+        .get_payment(payment_id)
+        .await
+        .context("Couldn't check for existing payment")?;
+    ensure!(
+        maybe_existing_payment.is_none(),
+        "Detected duplicate attempt trying to pay this offer. \
+         Please refresh and try again."
+    );
 
     // TODO(phlip9): support user choosing quantity. For now just assume
     // quantity=1, but in a way that works.

lexe-ln/src/payments/manager.rs

@@ -290,10 +290,23 @@ impl<CM: LexeChannelManager<PS>, PS: LexePersister> PaymentsManager<CM, PS> {
         Ok(())
     }
 
-    /// Returns true if we already have a payment with the given [`LxPaymentId`]
-    /// registered.
-    pub async fn contains_payment_id(&self, id: &LxPaymentId) -> bool {
-        self.data.lock().await.contains_payment_id(id)
+    /// Get a [`Payment`] by its [`LxPaymentId`].
+    /// Returns the in-memory payment if cached, fetches from the DB otherwise.
+    pub async fn get_payment(
+        &self,
+        id: LxPaymentId,
+    ) -> anyhow::Result<Option<Payment>> {
+        {
+            let locked_data = self.data.lock().await;
+            if let Some(payment) = locked_data.pending.get(&id).cloned() {
+                return Ok(Some(payment));
+            }
+
+            // TODO(max): Maybe cache finalized payments that were fetched?
+            // Then we could early return here as well.
+        }
+
+        self.persister.get_payment_by_id(id).await
     }
 
     /// Attempt to update the personal note on a payment.
@@ -839,10 +852,6 @@ impl PaymentsData {
         }
     }
 
-    fn contains_payment_id(&self, id: &LxPaymentId) -> bool {
-        self.pending.contains_key(id) || self.finalized.contains(id)
-    }
-
     fn check_new_payment(
         &self,
         payment: Payment,