fix: make validation checks on payout entity (#374)

* fix: make validation checks on payout entity

* test: add test for cancel_payout

* test: refactor payout unit test

---------

Co-authored-by: bodymindarts <justin@galoy.io>

Author: thevaibhav-dixit

src/api/server/convert.rs

@@ -678,7 +678,10 @@ impl From<ApplicationError> for tonic::Status {
             ApplicationError::CouldNotParseIncomingPsbt(_) => {
                 tonic::Status::invalid_argument(err.to_string())
             }
-            ApplicationError::PayoutAlreadyCommitted => {
+            ApplicationError::PayoutError(PayoutError::PayoutAlreadyCommitted) => {
+                tonic::Status::failed_precondition(err.to_string())
+            }
+            ApplicationError::PayoutError(PayoutError::PayoutAlreadyCancelled) => {
                 tonic::Status::failed_precondition(err.to_string())
             }
             _ => tonic::Status::internal(err.to_string()),

src/app/error.rs

@@ -72,8 +72,6 @@ pub enum ApplicationError {
     SigningSessionNotFoundForXPubId(crate::primitives::XPubId),
     #[error("Could not parse incoming psbt: {0}")]
     CouldNotParseIncomingPsbt(bitcoin::psbt::PsbtParseError),
-    #[error("Payout already committed to a batch")]
-    PayoutAlreadyCommitted,
     #[error("Hex decode error: {0}")]
     HexDecodeError(#[from] hex::FromHexError),
     #[error("Could not decrypt the encrypted key: {0}")]

src/app/mod.rs

@@ -868,13 +868,7 @@ impl App {
             .payouts
             .find_by_id_for_cancellation(&mut tx, profile.account_id, id)
             .await?;
-        if payout.batch_id.is_some() {
-            return Err(ApplicationError::PayoutAlreadyCommitted);
-        }
-        if payout.is_cancelled() {
-            return Ok(());
-        }
-        payout.cancel_payout(profile.id);
+        payout.cancel_payout(profile.id)?;
         self.payouts.update(&mut tx, payout).await?;
         self.ledger
             .payout_cancelled(tx, LedgerTransactionId::new(), id)

src/entity/event.rs

@@ -109,4 +109,9 @@ impl<T: DeserializeOwned + Serialize + 'static> EntityEvents<T> {
         query.execute(&mut **tx).await?;
         Ok(())
     }
+
+    #[cfg(test)]
+    pub fn last(&self, n: usize) -> &[T] {
+        &self.events[self.events.len() - n..]
+    }
 }

src/payout/entity.rs

@@ -3,6 +3,8 @@ use serde::{Deserialize, Serialize};
 
 use crate::{entity::*, primitives::*};
 
+use super::error::PayoutError;
+
 #[derive(Serialize, Deserialize)]
 #[serde(tag = "type", rename_all = "snake_case")]
 pub enum PayoutEvent {
@@ -50,10 +52,17 @@ pub struct Payout {
 }
 
 impl Payout {
-    pub fn cancel_payout(&mut self, profile_id: ProfileId) {
+    pub fn cancel_payout(&mut self, profile_id: ProfileId) -> Result<(), PayoutError> {
+        if self.is_cancelled() {
+            return Err(PayoutError::PayoutAlreadyCancelled);
+        }
+        if self.is_already_committed() {
+            return Err(PayoutError::PayoutAlreadyCommitted);
+        }
         self.events.push(PayoutEvent::Cancelled {
             executed_by: profile_id,
-        })
+        });
+        Ok(())
     }
 
     pub fn is_cancelled(&self) -> bool {
@@ -64,6 +73,10 @@ impl Payout {
         }
         false
     }
+
+    fn is_already_committed(&self) -> bool {
+        self.batch_id.is_some()
+    }
 }
 
 #[derive(Debug, Builder, Clone)]
@@ -153,3 +166,70 @@ impl TryFrom<EntityEvents<PayoutEvent>> for Payout {
         builder.events(events).build()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use rust_decimal::Decimal;
+
+    use super::*;
+
+    fn init_events() -> EntityEvents<PayoutEvent> {
+        EntityEvents::init([
+            PayoutEvent::Initialized {
+                id: PayoutId::new(),
+                wallet_id: WalletId::new(),
+                profile_id: ProfileId::new(),
+                payout_queue_id: PayoutQueueId::new(),
+                destination: PayoutDestination::OnchainAddress {
+                    value: "bc1qwqdg6squsna38e46795at95yu9atm8azzmyvckulcc7kytlcckxswvvzej"
+                        .parse()
+                        .unwrap(),
+                },
+                satoshis: Satoshis::from(Decimal::from(21)),
+            },
+            PayoutEvent::ExternalIdUpdated {
+                external_id: "external_id".to_string(),
+            },
+        ])
+    }
+
+    #[test]
+    fn cancel_payout() {
+        let mut payout = Payout::try_from(init_events()).unwrap();
+        assert!(payout.cancel_payout(payout.profile_id).is_ok());
+        assert!(matches!(
+            payout.events.last(1)[0],
+            PayoutEvent::Cancelled { .. }
+        ));
+    }
+
+    #[test]
+    fn can_only_cancel_payout_one_time() {
+        let mut events = init_events();
+        events.push(PayoutEvent::Cancelled {
+            executed_by: ProfileId::new(),
+        });
+        let mut payout = Payout::try_from(events).unwrap();
+        let result = payout.cancel_payout(payout.profile_id);
+        assert!(matches!(result, Err(PayoutError::PayoutAlreadyCancelled)));
+    }
+
+    #[test]
+    fn errors_when_payout_already_committed() {
+        let mut events = init_events();
+        events.push(PayoutEvent::CommittedToBatch {
+            batch_id: BatchId::new(),
+            outpoint: bitcoin::OutPoint {
+                txid: "4010e27ff7dc6d9c66a5657e6b3d94b4c4e394d968398d16fefe4637463d194d"
+                    .parse()
+                    .unwrap(),
+                vout: 0,
+            },
+        });
+
+        let mut payout = Payout::try_from(events).unwrap();
+
+        let result = payout.cancel_payout(payout.profile_id);
+        assert!(matches!(result, Err(PayoutError::PayoutAlreadyCommitted)));
+    }
+}

src/payout/error.rs

@@ -10,4 +10,8 @@ pub enum PayoutError {
     PayoutIdNotFound(String),
     #[error("PayoutError - External Id does not exists")]
     ExternalIdNotFound,
+    #[error("PayoutError - Payout is already committed to batch")]
+    PayoutAlreadyCommitted,
+    #[error("PayoutError - Payout is already cancelled")]
+    PayoutAlreadyCancelled,
 }