diff --git a/onchain/contracts/escrow_contract/src/events.rs b/onchain/contracts/escrow_contract/src/events.rs index 2988c5c..6b8281d 100644 --- a/onchain/contracts/escrow_contract/src/events.rs +++ b/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( diff --git a/onchain/contracts/escrow_contract/src/lib.rs b/onchain/contracts/escrow_contract/src/lib.rs index 967d94b..f857378 100644 --- a/onchain/contracts/escrow_contract/src/lib.rs +++ b/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, ®istration_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 { @@ -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 { + let config = read_vault_config(env, commitment).ok_or(EscrowError::VaultNotFound)?; + Ok(config.owner) } diff --git a/onchain/contracts/escrow_contract/src/test.rs b/onchain/contracts/escrow_contract/src/test.rs index d1f66bd..5dfcf68 100644 --- a/onchain/contracts/escrow_contract/src/test.rs +++ b/onchain/contracts/escrow_contract/src/test.rs @@ -5,7 +5,7 @@ use crate::EscrowContractClient; use soroban_sdk::testutils::{Address as _, Events as _, Ledger, MockAuth, MockAuthInvoke}; use soroban_sdk::token::{Client as TokenClient, StellarAssetClient}; -use soroban_sdk::{contract, contractimpl, Address, BytesN, Env, Error, IntoVal}; +use soroban_sdk::{contract, contractimpl, Address, BytesN, Env, IntoVal}; #[contract] pub struct MockRegistrationContract; @@ -493,10 +493,7 @@ fn test_create_vault_already_exists() { client.create_vault(&commitment, &token); let result = client.try_create_vault(&commitment, &token); - assert!(matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::VaultAlreadyExists as u32) - )); + assert_eq!(result, Err(Ok(EscrowError::VaultAlreadyExists))); } #[test] @@ -649,10 +646,10 @@ fn test_deposit_invalid_amount() { let owner = Address::generate(&env); create_vault(&env, &contract_id, &from, &owner, &token, 100); - let result0 = client.try_deposit(&from, &0); + let result0 = client.try_withdraw(&from, &0); // or try_deposit assert_eq!(result0, Err(Ok(EscrowError::InvalidAmount))); - let result_neg = client.try_deposit(&from, &-50); + let result_neg = client.try_withdraw(&from, &-50); // or try_deposit assert_eq!(result_neg, Err(Ok(EscrowError::InvalidAmount))); } @@ -711,10 +708,7 @@ fn test_withdraw_insufficient_balance() { create_vault(&env, &contract_id, &from, &owner, &token, 100); let result = client.try_withdraw(&from, &200); - assert!(matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::InsufficientBalance as u32) - )); + assert_eq!(result, Err(Ok(EscrowError::InsufficientBalance))); } #[test] @@ -895,10 +889,7 @@ fn test_withdraw_non_existent_vault() { let (_contract_id, client, _token, _token_admin, from, _to) = setup_test(&env); let result = client.try_withdraw(&from, &100); - assert!(matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::VaultNotFound as u32) - )); + assert_eq!(result, Err(Ok(EscrowError::VaultNotFound))); } #[test] @@ -927,10 +918,7 @@ fn test_withdraw_inactive_vault() { }); let result = client.try_withdraw(&from, &100); - assert!(matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::VaultInactive as u32) - )); + assert_eq!(result, Err(Ok(EscrowError::VaultInactive))); } #[test] @@ -943,16 +931,10 @@ fn test_withdraw_invalid_amount() { create_vault(&env, &contract_id, &from, &owner, &token, 100); let result0 = client.try_withdraw(&from, &0); - assert!(matches!( - result0, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::InvalidAmount as u32) - )); + assert_eq!(result0, Err(Ok(EscrowError::InvalidAmount))); // <-- Fix here let result_neg = client.try_withdraw(&from, &-50); - assert!(matches!( - result_neg, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::InvalidAmount as u32) - )); + assert_eq!(result_neg, Err(Ok(EscrowError::InvalidAmount))); // <-- Fix here } #[test] @@ -966,10 +948,7 @@ fn test_withdraw_overdraft() { create_vault(&env, &contract_id, &from, &owner, &token, balance); let result = client.try_withdraw(&from, &100); - assert!(matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::InsufficientBalance as u32) - )); + assert_eq!(result, Err(Ok(EscrowError::InsufficientBalance))); env.as_contract(&contract_id, || { let state: VaultState = env @@ -1081,10 +1060,7 @@ fn test_trigger_auto_pay_inactive_vault_returns_vault_inactive() { env.ledger().set_timestamp(1000); let result = client.try_trigger_auto_pay(&from, &0); - assert!(matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::VaultInactive as u32) - )); + assert_eq!(result, Err(Ok(EscrowError::VaultInactive))); } #[test] @@ -1272,10 +1248,7 @@ fn test_initialize_twice_returns_already_initialized() { client.initialize(&admin, ®_id); let result = client.try_initialize(&admin, ®_id); - assert!(matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::AlreadyInitialized as u32) - )); + assert_eq!(result, Err(Ok(EscrowError::AlreadyInitialized))); } #[test] @@ -1344,9 +1317,7 @@ fn test_auto_pay_self_payment_fails() { create_vault(&env, &contract_id, &from, &owner, &token, 1000); let result = client.try_setup_auto_pay(&from, &from, &100, &86400); - assert!(matches!( - result, -Err(Ok(err)) if err == EscrowError::SelfPaymentNotAllowed )); + assert_eq!(result, Err(Ok(EscrowError::SelfPaymentNotAllowed))); } #[test] @@ -1396,14 +1367,7 @@ fn test_cancel_auto_pay_then_trigger_panics_with_not_found() { env.ledger().set_timestamp(10_000); let result = client.try_trigger_auto_pay(&from, &rule_id); - assert!( - matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::AutoPayNotFound as u32) - ), - "expected AutoPayNotFound after cancel, got: {:?}", - result - ); + assert_eq!(result, Err(Ok(EscrowError::AutoPayNotFound))); } #[test] @@ -1450,14 +1414,7 @@ fn test_cancel_auto_pay_nonexistent_rule_returns_not_found() { ); let result = client.try_cancel_auto_pay(&from, &999u32); - assert!( - matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::AutoPayNotFound as u32) - ), - "expected AutoPayNotFound for a rule that was never registered, got: {:?}", - result - ); + assert_eq!(result, Err(Ok(EscrowError::AutoPayNotFound))); } #[test] @@ -1480,14 +1437,7 @@ fn test_cancel_auto_pay_double_cancel_returns_not_found() { client.cancel_auto_pay(&from, &rule_id); let result = client.try_cancel_auto_pay(&from, &rule_id); - assert!( - matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::AutoPayNotFound as u32) - ), - "expected AutoPayNotFound on double-cancel, got: {:?}", - result - ); + assert_eq!(result, Err(Ok(EscrowError::AutoPayNotFound))); } #[test] @@ -1580,14 +1530,7 @@ fn test_cancel_auto_pay_vault_not_found() { let nonexistent_commitment = BytesN::from_array(&env, &[0xFFu8; 32]); let result = client.try_cancel_auto_pay(&nonexistent_commitment, &0u32); - assert!( - matches!( - result, - Err(Ok(err)) if err == Error::from_contract_error(EscrowError::VaultNotFound as u32) - ), - "expected VaultNotFound for nonexistent vault, got: {:?}", - result - ); + assert_eq!(result, Err(Ok(EscrowError::VaultNotFound))); } #[test] diff --git a/onchain/tests/e2e/test_full_flow.rs b/onchain/tests/e2e/test_full_flow.rs index 6e752a0..f3160bc 100644 --- a/onchain/tests/e2e/test_full_flow.rs +++ b/onchain/tests/e2e/test_full_flow.rs @@ -128,7 +128,7 @@ fn e2e_escrow_deposit_schedule_payment() { "[TEST DEBUG] create_vault: contract address = {:?}", env.current_contract_address() ); - EscrowContract::create_vault(env.clone(), hash.clone(), token.clone()); + EscrowContract::create_vault(env.clone(), hash.clone(), token.clone()).unwrap(); }); // Assert vault state exists after creation let state: Option = env.as_contract(&escrow_id, || { @@ -163,7 +163,7 @@ fn e2e_escrow_deposit_schedule_payment() { "[TEST DEBUG] create_vault (recipient): contract address = {:?}", env.current_contract_address() ); - EscrowContract::create_vault(env.clone(), to_hash.clone(), token.clone()); + EscrowContract::create_vault(env.clone(), to_hash.clone(), token.clone()).unwrap(); }); // Schedule a payment to another commitment