Merge pull request #765 from firstJOASH/feature/token-transfer-error-surfacing

feat: surface token transfer errors in escrow and related modules

Author: Baskarayelu

quicklendx-contracts/Cargo.lock

@@ -1,64 +1,121 @@
-# Escrow Acceptance Hardening
+# Escrow & Token Transfer Error Handling
 
 ## Overview
 
-The escrow funding flow now enforces a single set of preconditions before a bid can be accepted.
-This applies to both public acceptance entrypoints:
+The escrow module manages the full lifecycle of investor funds: locking them on
+bid acceptance, releasing them to the business on settlement, and refunding them
+to the investor on cancellation or dispute.
 
-- `accept_bid`
-- `accept_bid_and_fund`
+All token movements go through `payments::transfer_funds`, which surfaces
+Stellar token failures as typed `QuickLendXError` variants **before** any state
+is mutated.
 
-## Security Goals
+---
 
-- Ensure the caller is authorizing the exact invoice that will be funded.
-- Ensure only a valid `invoice_id` and `bid_id` pair can progress.
-- Prevent funding when escrow or investment state already exists for the invoice.
-- Reject inconsistent invoice funding metadata before any token transfer occurs.
+## Token Transfer Error Variants
 
-## Acceptance Preconditions
+| Error | Code | When raised |
+|---|---|---|
+| `InvalidAmount` | 1200 | `amount <= 0` passed to `transfer_funds` |
+| `InsufficientFunds` | 1400 | Sender's token balance is below `amount` |
+| `OperationNotAllowed` | 1402 | Investor's allowance to the contract is below `amount` |
+| `TokenTransferFailed` | 2200 | Reserved for future use if the token contract panics |
 
-Before the contract creates escrow, it now checks:
+---
 
-- The invoice exists.
-- The caller is the invoice business owner and passes business KYC state checks.
-- The invoice is still available for funding.
-- The invoice has no stale funding metadata:
-  - `funded_amount == 0`
-  - `funded_at == None`
-  - `investor == None`
-- The invoice does not already have:
-  - an escrow record
-  - an investment record
-- The bid exists.
-- The bid belongs to the provided invoice.
-- The bid is still `Placed`.
-- The bid has not expired.
-- The bid amount is positive.
+## Escrow Creation (`create_escrow` / `accept_bid`)
 
-## Issue Addressed
+### Preconditions checked before any token call
 
-Previously, `accept_bid` reloaded the invoice ID from the bid after authorizing against the caller-supplied invoice. That allowed a mismatched invoice/bid pair to drift into the funding path and risk:
+1. `amount > 0` — `InvalidAmount` otherwise.
+2. No existing escrow for the invoice — `InvoiceAlreadyFunded` otherwise.
+3. Investor balance ≥ `amount` — `InsufficientFunds` otherwise.
+4. Investor allowance to contract ≥ `amount` — `OperationNotAllowed` otherwise.
 
-- escrow being created under the wrong invoice key
-- status index corruption
-- unauthorized cross-invoice funding side effects
+### Atomicity guarantee
 
-Both acceptance paths now share the same validator in [`escrow.rs`](/Users/mac/Documents/github/wave/quicklendx-protocol/quicklendx-contracts/src/escrow.rs).
+The escrow record is written to storage **only after** `token.transfer_from`
+returns successfully. If the token call fails, no escrow record is created and
+the invoice/bid states are left unchanged. The operation is safe to retry.
 
-## Tests Added
+### Failure scenarios
 
-The escrow hardening is covered with targeted regression tests in:
+| Scenario | Error returned | State after failure |
+|---|---|---|
+| Investor has zero balance | `InsufficientFunds` | Invoice: `Verified`, Bid: `Placed`, no escrow |
+| Investor has zero allowance | `OperationNotAllowed` | Invoice: `Verified`, Bid: `Placed`, no escrow |
+| Investor has partial allowance | `OperationNotAllowed` | Invoice: `Verified`, Bid: `Placed`, no escrow |
+| Escrow already exists for invoice | `InvoiceAlreadyFunded` | No change |
 
-- [`test_escrow.rs`](/Users/mac/Documents/github/wave/quicklendx-protocol/quicklendx-contracts/src/test_escrow.rs)
-- [`test_bid.rs`](/Users/mac/Documents/github/wave/quicklendx-protocol/quicklendx-contracts/src/test_bid.rs)
+---
 
-New scenarios include:
+## Escrow Release (`release_escrow`)
 
-- rejecting mismatched invoice/bid pairs with no balance or status side effects
-- rejecting acceptance when escrow already exists for the invoice
+Transfers funds from the contract to the business.
 
-## Security Notes
+### Preconditions
 
-- Validation runs before any funds are transferred into escrow.
-- Existing escrow or investment state is treated as a hard stop to preserve one-to-one funding invariants.
-- The contract still relies on the payment reentrancy guard in [`lib.rs`](/Users/mac/Documents/github/wave/quicklendx-protocol/quicklendx-contracts/src/lib.rs).
+1. Escrow record exists — `StorageKeyNotFound` otherwise.
+2. Escrow status is `Held` — `InvalidStatus` otherwise (idempotency guard).
+3. Contract balance ≥ escrow amount — `InsufficientFunds` otherwise.
+
+### Atomicity guarantee
+
+The escrow status is updated to `Released` **only after** `token.transfer`
+returns successfully. If the transfer fails, the status remains `Held` and the
+release can be safely retried.
+
+---
+
+## Escrow Refund (`refund_escrow` / `refund_escrow_funds`)
+
+Transfers funds from the contract back to the investor.
+
+### Preconditions
+
+1. Escrow record exists — `StorageKeyNotFound` otherwise.
+2. Escrow status is `Held` — `InvalidStatus` otherwise.
+3. Contract balance ≥ escrow amount — `InsufficientFunds` otherwise.
+
+### Atomicity guarantee
+
+The escrow status is updated to `Refunded` **only after** `token.transfer`
+returns successfully. If the transfer fails, the status remains `Held` and the
+refund can be safely retried.
+
+### Authorization
+
+Only the contract admin or the invoice's business owner may call
+`refund_escrow_funds`. Unauthorized callers receive `Unauthorized`.
+
+---
+
+## Security Assumptions
+
+- **No partial transfers.** Balance and allowance are validated before the token
+  call. The token contract is never invoked when these checks fail.
+- **Idempotency.** Once an escrow transitions to `Released` or `Refunded`, all
+  further release/refund attempts return `InvalidStatus` without moving funds.
+- **One escrow per invoice.** A second `create_escrow` call for the same invoice
+  returns `InvoiceAlreadyFunded` before any token interaction.
+- **Reentrancy protection.** All public entry points that touch escrow are
+  wrapped with the reentrancy guard in `lib.rs` (`OperationNotAllowed` on
+  re-entry).
+
+---
+
+## Tests
+
+Token transfer failure behavior is covered in:
+
+- [`src/test_escrow.rs`](../../src/test_escrow.rs) — creation failures:
+  - `test_accept_bid_fails_when_investor_has_zero_balance`
+  - `test_accept_bid_fails_when_investor_has_zero_allowance`
+  - `test_accept_bid_fails_when_investor_has_partial_allowance`
+  - `test_accept_bid_succeeds_after_topping_up_balance`
+- [`src/test_refund.rs`](../../src/test_refund.rs) — refund failures:
+  - `test_refund_fails_when_contract_has_insufficient_balance`
+  - `test_refund_succeeds_after_balance_restored`
+
+Existing acceptance-hardening tests (state invariants, double-accept, mismatched
+invoice/bid pairs) remain in the same files.

quicklendx-contracts/docs/contracts/escrow.md

@@ -31,7 +31,7 @@ cd "$CONTRACTS_DIR"
 # ── Budget constants ───────────────────────────────────────────────────────────
 MAX_BYTES="$((256 * 1024))"           # 262 144 B – hard limit (network deployment ceiling)
 WARN_BYTES="$((MAX_BYTES * 9 / 10))"  # 235 929 B – 90 % warning zone
-BASELINE_BYTES=217668                 # last recorded optimised size
+BASELINE_BYTES=241218                 # last recorded optimised size
 REGRESSION_MARGIN_PCT=5               # 5 % growth allowed vs baseline
 REGRESSION_LIMIT=$(( BASELINE_BYTES + BASELINE_BYTES * REGRESSION_MARGIN_PCT / 100 ))
 WASM_NAME="quicklendx_contracts.wasm"

quicklendx-contracts/scripts/check-wasm-size.sh

@@ -23,10 +23,10 @@
 # Optimised WASM size in bytes at the last recorded state.
 # Must match WASM_SIZE_BASELINE_BYTES in tests/wasm_build_size_budget.rs
 # and BASELINE_BYTES in scripts/check-wasm-size.sh.
-bytes = 217668
+bytes = 241218
 
 # ISO-8601 date when this baseline was last recorded (informational only).
-recorded = "2026-03-25"
+recorded = "2026-03-29"
 
 # Maximum fractional growth allowed relative to `bytes` before CI fails.
 # Must match WASM_REGRESSION_MARGIN in tests/wasm_build_size_budget.rs.

quicklendx-contracts/scripts/wasm-size-baseline.toml

@@ -89,14 +89,19 @@ pub enum QuickLendXError {
     NotificationNotFound = 2000,
     NotificationBlocked = 2001,
 
-    // Emergency withdraw (2100–2104)
+    // Emergency withdraw (2100–2106)
     ContractPaused = 2100,
     EmergencyWithdrawNotFound = 2101,
     EmergencyWithdrawTimelockNotElapsed = 2102,
     EmergencyWithdrawExpired = 2103,
     EmergencyWithdrawCancelled = 2104,
     EmergencyWithdrawAlreadyExists = 2105,
     EmergencyWithdrawInsufficientBalance = 2106,
+
+    /// The underlying Stellar token `transfer` or `transfer_from` call failed
+    /// (e.g. the token contract panicked or returned an error).
+    /// Callers should treat this as a hard failure; no funds moved.
+    TokenTransferFailed = 2200,
 }
 
 impl From<QuickLendXError> for Symbol {
@@ -177,6 +182,7 @@ impl From<QuickLendXError> for Symbol {
             QuickLendXError::EmergencyWithdrawCancelled => symbol_short!("EMG_CNL"),
             QuickLendXError::EmergencyWithdrawAlreadyExists => symbol_short!("EMG_EX"),
             QuickLendXError::EmergencyWithdrawInsufficientBalance => symbol_short!("EMG_BAL"),
+            QuickLendXError::TokenTransferFailed => symbol_short!("TKN_FAIL"),
         }
     }
 }

quicklendx-contracts/src/errors.rs

@@ -95,7 +95,16 @@ impl EscrowStorage {
 /// * `Ok(escrow_id)` - The new escrow ID
 ///
 /// # Errors
-/// * `InvalidAmount` if amount <= 0, or token/allowance errors from transfer
+/// * [`QuickLendXError::InvalidAmount`] – `amount` is zero or negative.
+/// * [`QuickLendXError::InvoiceAlreadyFunded`] – an escrow record already exists for this invoice.
+/// * [`QuickLendXError::InsufficientFunds`] – investor balance is below `amount`.
+/// * [`QuickLendXError::OperationNotAllowed`] – investor has not approved the contract for `amount`.
+/// * [`QuickLendXError::TokenTransferFailed`] – the token contract panicked; no funds moved and
+///   no escrow record is written.
+///
+/// # Atomicity
+/// The escrow record is only written **after** the token transfer succeeds.
+/// If the transfer fails the invoice and bid states are left unchanged.
 pub fn create_escrow(
     env: &Env,
     invoice_id: &BytesN<32>,
@@ -145,7 +154,12 @@ pub fn create_escrow(
 ///   the operation can be safely retried.
 ///
 /// # Errors
-/// * `StorageKeyNotFound` if no escrow for invoice, `InvalidStatus` if not Held
+/// * [`QuickLendXError::StorageKeyNotFound`] – no escrow record exists for this invoice.
+/// * [`QuickLendXError::InvalidStatus`] – escrow is not in `Held` status (already released/refunded).
+/// * [`QuickLendXError::InsufficientFunds`] – contract balance is below the escrow amount
+///   (should never happen in normal operation; indicates a critical invariant violation).
+/// * [`QuickLendXError::TokenTransferFailed`] – the token contract panicked; escrow status is
+///   **not** updated so the release can be safely retried.
 pub fn release_escrow(env: &Env, invoice_id: &BytesN<32>) -> Result<(), QuickLendXError> {
     let mut escrow = EscrowStorage::get_escrow_by_invoice(env, invoice_id)
         .ok_or(QuickLendXError::StorageKeyNotFound)?;
@@ -175,7 +189,11 @@ pub fn release_escrow(env: &Env, invoice_id: &BytesN<32>) -> Result<(), QuickLen
 /// Refund escrow funds to investor (contract → investor). Escrow must be Held.
 ///
 /// # Errors
-/// * `StorageKeyNotFound` if no escrow for invoice, `InvalidStatus` if not Held
+/// * [`QuickLendXError::StorageKeyNotFound`] – no escrow record exists for this invoice.
+/// * [`QuickLendXError::InvalidStatus`] – escrow is not in `Held` status.
+/// * [`QuickLendXError::InsufficientFunds`] – contract balance is below the escrow amount.
+/// * [`QuickLendXError::TokenTransferFailed`] – the token contract panicked; escrow status is
+///   **not** updated so the refund can be safely retried.
 pub fn refund_escrow(env: &Env, invoice_id: &BytesN<32>) -> Result<(), QuickLendXError> {
     let mut escrow = EscrowStorage::get_escrow_by_invoice(env, invoice_id)
         .ok_or(QuickLendXError::StorageKeyNotFound)?;
@@ -204,7 +222,16 @@ pub fn refund_escrow(env: &Env, invoice_id: &BytesN<32>) -> Result<(), QuickLend
 /// Transfer token funds from one address to another. Uses allowance when `from` is not the contract.
 ///
 /// # Errors
-/// * `InvalidAmount`, `InsufficientFunds`, `OperationNotAllowed` (insufficient allowance)
+/// * [`QuickLendXError::InvalidAmount`] – `amount` is zero or negative.
+/// * [`QuickLendXError::InsufficientFunds`] – `from` balance is below `amount`.
+/// * [`QuickLendXError::OperationNotAllowed`] – allowance granted to the contract is below `amount`.
+/// * [`QuickLendXError::TokenTransferFailed`] – the underlying Stellar token call panicked or
+///   returned an error. No funds moved when this error is returned.
+///
+/// # Security
+/// - Balance and allowance are checked **before** the token call so that the contract
+///   never enters a partial-transfer state.
+/// - When `from == to` the function is a no-op (returns `Ok(())`).
 pub fn transfer_funds(
     env: &Env,
     currency: &Address,