From 086491b3b688fa32615d50f5557b68b432e74b08 Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Thu, 15 Jun 2023 12:38:44 +0200
Subject: [PATCH] Remove unnecessary `PaymentStatus::SendingFailed`

Previously, I had a misunderstood in what event retrying payments is
safe. It turns out that it's indeed safe to retry a payment as soon as
we receive `LdkEvent::PaymentFailed`.

Therefore, we can here now only refuse to send duplicate payments if our previous status is
`Succeeded` or `Pending`, which renders the additional distinction
between `Failed` and `SendingFailed` superfluous. Accordingly, we remove
this `PaymentStatus` variant.
---
 bindings/ldk_node.udl |  1 -
 src/lib.rs            | 18 ++++++++++++------
 src/payment_store.rs  |  9 +++------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl
index 4aa1074ff..af7ddf9d9 100644
--- a/bindings/ldk_node.udl
+++ b/bindings/ldk_node.udl
@@ -128,7 +128,6 @@ enum PaymentDirection {
 
 enum PaymentStatus {
 	"Pending",
-	"SendingFailed",
 	"Succeeded",
 	"Failed",
 };
diff --git a/src/lib.rs b/src/lib.rs
index c9cbe7770..54400863c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1518,7 +1518,9 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 		let payment_hash = PaymentHash((*invoice.payment_hash()).into_inner());
 
 		if let Some(payment) = self.payment_store.get(&payment_hash) {
-			if payment.status != PaymentStatus::SendingFailed {
+			if payment.status == PaymentStatus::Pending
+				|| payment.status == PaymentStatus::Succeeded
+			{
 				log_error!(self.logger, "Payment error: an invoice must not be paid twice.");
 				return Err(Error::DuplicatePayment);
 			}
@@ -1565,7 +1567,7 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 							secret: payment_secret,
 							amount_msat: invoice.amount_milli_satoshis(),
 							direction: PaymentDirection::Outbound,
-							status: PaymentStatus::SendingFailed,
+							status: PaymentStatus::Failed,
 						};
 
 						self.payment_store.insert(payment)?;
@@ -1601,7 +1603,9 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 
 		let payment_hash = PaymentHash((*invoice.payment_hash()).into_inner());
 		if let Some(payment) = self.payment_store.get(&payment_hash) {
-			if payment.status != PaymentStatus::SendingFailed {
+			if payment.status == PaymentStatus::Pending
+				|| payment.status == PaymentStatus::Succeeded
+			{
 				log_error!(self.logger, "Payment error: an invoice must not be paid twice.");
 				return Err(Error::DuplicatePayment);
 			}
@@ -1668,7 +1672,7 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 							secret: payment_secret,
 							amount_msat: Some(amount_msat),
 							direction: PaymentDirection::Outbound,
-							status: PaymentStatus::SendingFailed,
+							status: PaymentStatus::Failed,
 						};
 						self.payment_store.insert(payment)?;
 
@@ -1692,7 +1696,9 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 		let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 
 		if let Some(payment) = self.payment_store.get(&payment_hash) {
-			if payment.status != PaymentStatus::SendingFailed {
+			if payment.status == PaymentStatus::Pending
+				|| payment.status == PaymentStatus::Succeeded
+			{
 				log_error!(self.logger, "Payment error: must not send duplicate payments.");
 				return Err(Error::DuplicatePayment);
 			}
@@ -1741,7 +1747,7 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 							hash: payment_hash,
 							preimage: Some(payment_preimage),
 							secret: None,
-							status: PaymentStatus::SendingFailed,
+							status: PaymentStatus::Failed,
 							direction: PaymentDirection::Outbound,
 							amount_msat: Some(amount_msat),
 						};
diff --git a/src/payment_store.rs b/src/payment_store.rs
index c9d867d60..65037a3ba 100644
--- a/src/payment_store.rs
+++ b/src/payment_store.rs
@@ -57,19 +57,16 @@ impl_writeable_tlv_based_enum!(PaymentDirection,
 pub enum PaymentStatus {
 	/// The payment is still pending.
 	Pending,
-	/// The sending of the payment failed and is safe to be retried.
-	SendingFailed,
 	/// The payment suceeded.
 	Succeeded,
-	/// The payment failed and is not retryable.
+	/// The payment failed.
 	Failed,
 }
 
 impl_writeable_tlv_based_enum!(PaymentStatus,
 	(0, Pending) => {},
-	(2, SendingFailed) => {},
-	(4, Succeeded) => {},
-	(6, Failed) => {};
+	(2, Succeeded) => {},
+	(4, Failed) => {};
 );
 
 #[derive(Clone, Debug, PartialEq, Eq)]