-
Notifications
You must be signed in to change notification settings - Fork 134
Description
The core data types for the payment escrow contract are well-documented and reasonably structured, but several design gaps exist around type safety, storage efficiency, and future extensibility that could cause subtle bugs, unnecessary ledger costs, and breaking changes as the contract evolves.
Problem
1. amount uses i128 instead of u128
Escrow amounts are inherently non-negative values. Using a signed integer (i128) allows negative values to pass type checks silently, pushing validation burden entirely to runtime guards. A negative amount should be unrepresentable at the type level.
2. No canonical max length enforced on String fields
Both id and description are unbounded soroban_sdk::String fields. Without enforced length limits, callers can store arbitrarily large strings, inflating ledger entry sizes and increasing storage fees unpredictably. There is no guidance in the type definition or docs on acceptable lengths.
3. EscrowStatus has no terminal state distinction
Released and Refunded are both terminal states, but Disputed is a transitional one. There is no structural or documented distinction between these, making it easy for contract logic to inadvertently attempt state transitions from terminal states without an explicit guard at the type level.
4. Timestamps are plain u64 — no semantic distinction
created_at, release_after, dispute_window, and dispute_raised_at all use raw u64. It's unclear from the type alone whether release_after is an absolute timestamp or an offset, and whether dispute_window is a duration or a deadline. This ambiguity is currently only resolved by reading the doc comments, which is fragile.
5. No resolution_reason or audit field on resolved escrows
When an admin resolves a disputed escrow, there is no field to record the resolution rationale. This limits auditability and makes it harder to reconstruct the decision history on-chain.
6. dispute_raised_at and resolved_at use Option<u64> which inflates storage
Optional fields in Soroban contract types still occupy storage slots. For a high-volume escrow contract, this adds up. Consider whether sentinel values (e.g. 0) or a separate dispute record type would be more storage-efficient.
Expected Behavior
- Negative amounts should be unrepresentable without an explicit cast
- String fields should document (and ideally enforce) maximum byte lengths
- Terminal vs. transitional states should be clearly distinguishable
- Timestamp semantics (absolute vs. relative) should be unambiguous from the type or its docs
- Resolved escrows should carry enough context for basic on-chain auditability
Acceptance Criteria
-
amountfield is non-negative by type or enforced consistently at all entry points -
idanddescriptionhave documented (and validated) length constraints - All timestamp fields have clear semantic documentation (absolute vs. relative)
- Terminal and transitional
EscrowStatusvariants are clearly distinguished in docs or structure - Any new fields are covered by existing or new unit tests
- No breaking changes to existing escrow serialisation without a migration note
Context
- Contract:
contracts/payment_escrow - File:
src/types.rs