Merge pull request #1053 from getlipa/feature/remove-unnecessary-none…

…-recommended-action Remove `RecommendedAction`
getlipa · May 7, 2024 · e8792cc · e8792cc
2 parents f2a68f5 + 6569e5d
commit e8792cc
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 50 deletions.
diff --git a/examples/notification_handler/main.rs b/examples/notification_handler/main.rs
@@ -145,9 +145,9 @@ fn start_payment_received(words: &mut dyn Iterator<Item = &str>) -> Result<()> {
         }}"
     );
 
-    let action = handle_notification(config, notification_payload).unwrap();
+    let notification = handle_notification(config, notification_payload).unwrap();
 
-    println!("The recommended action is {action:?}");
+    println!("The returned notification is {notification:?}");
 
     Ok(())
 }
@@ -172,9 +172,9 @@ fn start_address_txs_confirmed(words: &mut dyn Iterator<Item = &str>) -> Result<
         }}"
     );
 
-    let action = handle_notification(config, notification_payload).unwrap();
+    let notification = handle_notification(config, notification_payload).unwrap();
 
-    println!("The recommended action is {action:?}");
+    println!("The returned notification is {notification:?}");
 
     Ok(())
 }
diff --git a/src/errors.rs b/src/errors.rs
@@ -300,10 +300,15 @@ pub(crate) fn map_lnurl_withdraw_error(
 /// A code that specifies the NotificationHandlingError that occurred.
 #[derive(PartialEq, Eq, Debug, Clone)]
 pub enum NotificationHandlingErrorCode {
-    /// Information about the remote node isn't cached and couldn't be accessed. Could be a network error.
+    /// Information about the remote node isn't cached and couldn't be accessed.
+    /// Could be a network error.
     NodeUnavailable,
-    /// The specificed in-progress swap couldn't be found.
+    /// The notification payload implied the existence of an in-progress swap, but it couldn't be
+    /// found. Maybe another instance of the wallet completed the swap.
     InProgressSwapNotFound,
+    /// The notification payload implied the existence of an incoming payment, but it was not
+    /// received in time. Starting the app might help complete the payment.
+    ExpectedPaymentNotReceived,
 }
 
 impl Display for NotificationHandlingErrorCode {

diff --git a/src/lib.rs b/src/lib.rs
@@ -75,7 +75,7 @@ use crate::key_derivation::derive_persistence_encryption_key;
 pub use crate::limits::{LiquidityLimit, PaymentAmountLimits};
 pub use crate::lnurl::{LnUrlPayDetails, LnUrlWithdrawDetails};
 use crate::locker::Locker;
-pub use crate::notification_handling::{handle_notification, Notification, RecommendedAction};
+pub use crate::notification_handling::{handle_notification, Notification};
 pub use crate::offer::{OfferInfo, OfferKind, OfferStatus};
 pub use crate::payment::{
     IncomingPaymentInfo, OutgoingPaymentInfo, PaymentInfo, PaymentState, Recipient,

diff --git a/src/lipalightninglib.udl b/src/lipalightninglib.udl
@@ -624,7 +624,7 @@ namespace lipalightninglib {
     void parse_lightning_address([ByRef] string address);
 
     [Throws=NotificationHandlingError]
-    RecommendedAction handle_notification(Config config, string notification_payload);
+    Notification handle_notification(Config config, string notification_payload);
 };
 
 dictionary Secret {
@@ -633,12 +633,6 @@ dictionary Secret {
     bytes seed;
 };
 
-[Enum]
-interface RecommendedAction {
-    None();
-    ShowNotification(Notification notification);
-};
-
 [Enum]
 interface Notification {
     Bolt11PaymentReceived(u64 amount_sat, string payment_hash);
@@ -770,4 +764,5 @@ interface NotificationHandlingError {
 enum NotificationHandlingErrorCode {
     "NodeUnavailable",
     "InProgressSwapNotFound",
+    "ExpectedPaymentNotReceived",
 };
diff --git a/src/notification_handling.rs b/src/notification_handling.rs
@@ -13,9 +13,11 @@ use crate::{
 use breez_sdk_core::{
     BreezEvent, BreezServices, EventListener, InvoicePaidDetails, Payment, PaymentStatus,
 };
-use log::{debug, error};
+use log::debug;
 use parrot::AnalyticsClient;
-use perro::{ensure, permanent_failure, runtime_error, MapToError, OptionToError, ResultTrait};
+use perro::{
+    ensure, invalid_input, permanent_failure, runtime_error, MapToError, OptionToError, ResultTrait,
+};
 use serde::Deserialize;
 use std::path::Path;
 use std::sync::mpsc::{Receiver, RecvTimeoutError, Sender};
@@ -50,24 +52,18 @@ pub enum Notification {
     },
 }
 
-/// An action to be taken by the consumer of this library upon calling [`handle_notification`].
-#[derive(Debug)]
-pub enum RecommendedAction {
-    None,
-    ShowNotification { notification: Notification },
-}
-
 /// Handles a notification.
 ///
 /// Notifications are used to wake up the node in order to process some request. Currently supported
 /// requests are:
 /// * Receive a payment from a previously issued bolt11 invoice.
+/// * Receive a payment from a confirmed swap.
 ///
 /// Requires network: **yes**
 pub fn handle_notification(
     config: Config,
     notification_payload: String,
-) -> NotificationHandlingResult<RecommendedAction> {
+) -> NotificationHandlingResult<Notification> {
     enable_backtrace();
     if let Some(level) = config.file_logging_level {
         init_logger_once(
@@ -81,8 +77,7 @@ pub fn handle_notification(
     let payload = match serde_json::from_str::<Payload>(&notification_payload) {
         Ok(p) => p,
         Err(e) => {
-            error!("Notification payload not recognized. Error: {e} - JSON Payload: {notification_payload}");
-            return Ok(RecommendedAction::None);
+            invalid_input!("The provided payload was not recognized. Error: {e} - JSON Payload: {notification_payload}")
         }
     };
 
@@ -156,36 +151,36 @@ fn handle_payment_received_notification(
     sdk: Arc<BreezServices>,
     event_receiver: Receiver<BreezEvent>,
     payment_hash: String,
-) -> NotificationHandlingResult<RecommendedAction> {
+) -> NotificationHandlingResult<Notification> {
     // Check if the payment was already received
     if let Some(payment) = get_confirmed_payment(&rt, &sdk, &payment_hash)? {
-        return Ok(RecommendedAction::ShowNotification {
-            notification: Notification::Bolt11PaymentReceived {
-                amount_sat: payment.amount_msat / 1000,
-                payment_hash,
-            },
+        return Ok(Notification::Bolt11PaymentReceived {
+            amount_sat: payment.amount_msat / 1000,
+            payment_hash,
         });
     }
 
     // Wait for payment to be received
     if let Some(details) = wait_for_payment_with_timeout(event_receiver, &payment_hash)? {
-        return Ok(RecommendedAction::ShowNotification {
-            notification: Notification::Bolt11PaymentReceived {
-                amount_sat: details.payment.map(|p| p.amount_msat).unwrap_or(0) / 1000, // payment will only be None for corrupted GL payments. This is unlikely, so giving an optional amount seems overkill.
-                payment_hash,
-            },
+        return Ok(Notification::Bolt11PaymentReceived {
+            amount_sat: details.payment.map(|p| p.amount_msat).unwrap_or(0) / 1000, // payment will only be None for corrupted GL payments. This is unlikely, so giving an optional amount seems overkill.
+            payment_hash,
         });
     }
 
-    Ok(RecommendedAction::None)
+    runtime_error!(
+        NotificationHandlingErrorCode::ExpectedPaymentNotReceived,
+        "Expected incoming payment with hash {} but it was not received",
+        payment_hash
+    )
 }
 
 fn handle_address_txs_confirmed_notification(
     rt: AsyncRuntime,
     sdk: Arc<BreezServices>,
     event_receiver: Receiver<BreezEvent>,
     address: String,
-) -> NotificationHandlingResult<RecommendedAction> {
+) -> NotificationHandlingResult<Notification> {
     let in_progress_swap = rt
         .handle()
         .block_on(sdk.in_progress_swap())
@@ -218,25 +213,25 @@ fn handle_address_txs_confirmed_notification(
     // Check if the payment was already received
     let payment_hash = hex::encode(in_progress_swap.payment_hash);
     if let Some(payment) = get_confirmed_payment(&rt, &sdk, &payment_hash)? {
-        return Ok(RecommendedAction::ShowNotification {
-            notification: Notification::OnchainPaymentSwappedIn {
-                amount_sat: payment.amount_msat / 1000,
-                payment_hash,
-            },
+        return Ok(Notification::OnchainPaymentSwappedIn {
+            amount_sat: payment.amount_msat / 1000,
+            payment_hash,
         });
     }
 
     // Wait for payment to arrive
     if let Some(details) = wait_for_payment_with_timeout(event_receiver, &payment_hash)? {
-        return Ok(RecommendedAction::ShowNotification {
-            notification: Notification::OnchainPaymentSwappedIn {
-                amount_sat: details.payment.map(|p| p.amount_msat).unwrap_or(0) / 1000, // payment will only be None for corrupted GL payments. This is unlikely, so giving an optional amount seems overkill.
-                payment_hash,
-            },
+        return Ok(Notification::OnchainPaymentSwappedIn {
+            amount_sat: details.payment.map(|p| p.amount_msat).unwrap_or(0) / 1000, // payment will only be None for corrupted GL payments. This is unlikely, so giving an optional amount seems overkill.
+            payment_hash,
         });
     }
 
-    Ok(RecommendedAction::None)
+    runtime_error!(
+        NotificationHandlingErrorCode::ExpectedPaymentNotReceived,
+        "Expected incoming payment with hash {} but it was not received",
+        payment_hash
+    )
 }
 
 fn get_confirmed_payment(