refactor: finish EscrowContract structured errors and events

onchain/contracts/escrow_contract/src/events.rs

@@ -1,4 +1,13 @@
-use soroban_sdk::{contractevent, symbol_short, Address, BytesN, Env};
+use soroban_sdk::{contractevent, Address, BytesN, Env};
+
+#[contractevent]
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct VaultCrtEvent {
+    #[topic]
+    pub commitment: BytesN<32>,
+    pub token: Address,
+    pub owner: Address,
+}
 
 #[contractevent]
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -122,10 +131,13 @@ impl Events {
         .publish(env);
     }
 
-    #[allow(deprecated)]
     pub fn vault_crt(env: &Env, commitment: BytesN<32>, token: Address, owner: Address) {
-        env.events()
-            .publish((symbol_short!("VAULT_CRT"), commitment), (token, owner));
+        VaultCrtEvent {
+            commitment,
+            token,
+            owner,
+        }
+        .publish(env);
     }
 
     pub fn auto_set(

onchain/contracts/escrow_contract/src/lib.rs

@@ -16,19 +16,21 @@ use crate::storage::{
     write_registration_contract, write_scheduled_payment, write_vault_config, write_vault_state,
 };
 use crate::types::{AutoPay, DataKey, ScheduledPayment, VaultConfig, VaultState};
-use soroban_sdk::{
-    contract, contractimpl, panic_with_error, token, vec, Address, BytesN, Env, IntoVal, Symbol,
-};
+use soroban_sdk::{contract, contractimpl, token, vec, Address, BytesN, Env, IntoVal, Symbol};
 
 #[contract]
 pub struct EscrowContract;
 
 #[contractimpl]
 impl EscrowContract {
-    pub fn initialize(env: Env, admin: Address, registration_contract: Address) {
+    pub fn initialize(
+        env: Env,
+        admin: Address,
+        registration_contract: Address,
+    ) -> Result<(), EscrowError> {
         admin.require_auth();
         if read_registration_contract(&env).is_some() {
-            panic_with_error!(&env, EscrowError::AlreadyInitialized);
+            return Err(EscrowError::AlreadyInitialized);
         }
         write_registration_contract(&env, &registration_contract);
         write_escrow_admin(&env, &admin);
@@ -71,13 +73,12 @@ impl EscrowContract {
             &Symbol::new(&env, "get_owner"),
             vec![&env, commitment.into_val(&env)],
         );
-        let owner =
-            owner.unwrap_or_else(|| panic_with_error!(&env, EscrowError::CommitmentNotRegistered));
+        let owner = owner.ok_or(EscrowError::CommitmentNotRegistered)?;
 
         owner.require_auth();
 
         if read_vault_config(&env, &commitment).is_some() {
-            panic_with_error!(&env, EscrowError::VaultAlreadyExists);
+            return Err(EscrowError::VaultAlreadyExists);
         }
 
         write_vault_config(
@@ -100,6 +101,7 @@ impl EscrowContract {
         );
 
         Events::vault_crt(&env, commitment, token, owner);
+        Ok(())
     }
 
     pub fn deposit(env: Env, commitment: BytesN<32>, amount: i128) -> Result<(), EscrowError> {
@@ -126,16 +128,16 @@ impl EscrowContract {
         state.balance = state
             .balance
             .checked_add(amount)
-            .expect("vault balance overflow");
+            .ok_or(EscrowError::ArithmeticError)?;
         write_vault_state(&env, &commitment, &state);
 
         Events::deposit(&env, commitment, amount, state.balance);
         Ok(())
     }
 
-    pub fn withdraw(env: Env, commitment: BytesN<32>, amount: i128) {
+    pub fn withdraw(env: Env, commitment: BytesN<32>, amount: i128) -> Result<(), EscrowError> {
         if amount <= 0 {
-            panic_with_error!(&env, EscrowError::InvalidAmount);
+            return Err(EscrowError::InvalidAmount);
         }
 
         if read_paused(&env) {
@@ -150,11 +152,11 @@ impl EscrowContract {
         config.owner.require_auth();
 
         if !state.is_active {
-            panic_with_error!(&env, EscrowError::VaultInactive);
+            return Err(EscrowError::VaultInactive);
         }
 
         if state.balance < amount {
-            panic_with_error!(&env, EscrowError::InsufficientBalance);
+            return Err(EscrowError::InsufficientBalance);
         }
 
         let token_client = token::Client::new(&env, &config.token);
@@ -163,10 +165,11 @@ impl EscrowContract {
         state.balance = state
             .balance
             .checked_sub(amount)
-            .expect("vault balance underflow");
-        write_vault_state(&env, &commitment, &state);
+            .ok_or(EscrowError::ArithmeticError)?;
 
+        write_vault_state(&env, &commitment, &state);
         Events::withdraw(&env, commitment, amount, state.balance);
+        Ok(())
     }
 
     pub fn schedule_payment(
@@ -253,7 +256,7 @@ impl EscrowContract {
             return Err(EscrowError::VaultInactive);
         }
 
-        let recipient = resolve(&env, &payment.to);
+        let recipient = resolve(&env, &payment.to)?;
         let token_client = token::Client::new(&env, &payment.token);
         token_client.transfer(&env.current_contract_address(), &recipient, &payment.amount);
 
@@ -330,18 +333,17 @@ impl EscrowContract {
         Ok(rule_id)
     }
 
-    pub fn cancel_auto_pay(env: Env, from: BytesN<32>, rule_id: u32) {
-        let config = read_vault_config(&env, &from)
-            .unwrap_or_else(|| panic_with_error!(&env, EscrowError::VaultNotFound));
+    pub fn cancel_auto_pay(env: Env, from: BytesN<32>, rule_id: u32) -> Result<(), EscrowError> {
+        let config = read_vault_config(&env, &from).ok_or(EscrowError::VaultNotFound)?;
         config.owner.require_auth();
 
         if read_auto_pay(&env, &from, rule_id).is_none() {
-            panic_with_error!(&env, EscrowError::AutoPayNotFound);
+            return Err(EscrowError::AutoPayNotFound);
         }
 
         delete_auto_pay(&env, &from, rule_id);
-
         Events::auto_cancel(&env, from, rule_id);
+        Ok(())
     }
 
     pub fn trigger_auto_pay(env: Env, from: BytesN<32>, rule_id: u32) {
@@ -353,24 +355,29 @@ impl EscrowContract {
             .unwrap_or_else(|| panic_with_error!(&env, EscrowError::AutoPayNotFound));
 
         let current_time = env.ledger().timestamp();
-        let next_payment_time = auto_pay.last_paid + auto_pay.interval;
+
+        // Use checked_add for safety to prevent overflow panics
+        let next_payment_time = auto_pay
+            .last_paid
+            .checked_add(auto_pay.interval)
+            .ok_or(EscrowError::ArithmeticError)?;
 
         if current_time < next_payment_time {
-            panic_with_error!(&env, EscrowError::IntervalNotElapsed);
+            return Err(EscrowError::IntervalNotElapsed);
         }
 
-        let mut state = read_vault_state(&env, &from)
-            .unwrap_or_else(|| panic_with_error!(&env, EscrowError::VaultNotFound));
+        let mut state = read_vault_state(&env, &from).ok_or(EscrowError::VaultNotFound)?;
 
         if !state.is_active {
-            panic_with_error!(&env, EscrowError::VaultInactive);
+            return Err(EscrowError::VaultInactive);
         }
 
         if state.balance < auto_pay.amount {
-            panic_with_error!(&env, EscrowError::InsufficientBalance);
+            return Err(EscrowError::InsufficientBalance);
         }
 
-        let recipient = resolve(&env, &auto_pay.to);
+        // Add the `?` operator here because `resolve` now returns a Result
+        let recipient = resolve(&env, &auto_pay.to)?;
 
         let token_client = token::Client::new(&env, &auto_pay.token);
         token_client.transfer(
@@ -379,7 +386,12 @@ impl EscrowContract {
             &auto_pay.amount,
         );
 
-        state.balance -= auto_pay.amount;
+        // Replace the raw `-=` with checked math
+        state.balance = state
+            .balance
+            .checked_sub(auto_pay.amount)
+            .ok_or(EscrowError::ArithmeticError)?;
+
         write_vault_state(&env, &from, &state);
 
         auto_pay.last_paid = current_time;
@@ -393,6 +405,8 @@ impl EscrowContract {
             auto_pay.amount,
             current_time,
         );
+
+        Ok(())
     }
 
     pub fn get_balance(env: Env, commitment: BytesN<32>) -> Option<i128> {
@@ -418,8 +432,7 @@ impl EscrowContract {
 }
 
 /// Resolves a commitment to its owner address.
-fn resolve(env: &Env, commitment: &BytesN<32>) -> Address {
-    let config = read_vault_config(env, commitment)
-        .unwrap_or_else(|| panic_with_error!(env, EscrowError::VaultNotFound));
-    config.owner
+fn resolve(env: &Env, commitment: &BytesN<32>) -> Result<Address, EscrowError> {
+    let config = read_vault_config(env, commitment).ok_or(EscrowError::VaultNotFound)?;
+    Ok(config.owner)
 }