(Medium) ORG-E3.1 liquidity-pool error-model

[EmeditWeb]

Summary

LiquidityPoolError has 19 variants (contracts/liquidity-pool-contract/src/errors.rs:6-26) and not one doc comment, so the contract's two distinct failure families are indistinguishable to a reader: single-contract failures (NotAdmin = 1, AlreadyInitialized = 2, NotInitialized = 3, InvalidAmount = 4, InsufficientShares = 5, InsufficientLiquidity = 6, Overflow = 7, Underflow = 8, ZeroTotalShares = 10, ReentrancyDetected = 11, ContractPaused = 15) and the risk-cap / cross-contract family that gates lending (NotCreditLine = 9, OutflowCapExceeded = 16, MerchantExposureCapExceeded = 17, VendorNotActive = 18, InvalidCap = 19), plus the three upgrade variants (UpgradeNotProposed = 12, UpgradeTimelockNotMet = 13, UpgradeHashMismatch = 14). The cap variants are raised mid-settlement (contracts/liquidity-pool-contract/src/lib.rs:418 raises OutflowCapExceeded, :434 raises MerchantExposureCapExceeded, :385 returns VendorNotActive) and nothing tells a caller whether exceeding a cap is retryable (lower the amount) or terminal. The share-issuance path documents its own math in prose (lib.rs:209-212) but has no error doc for the arithmetic failures it can raise.

Why these together: both workstreams are faces of one property — every failure the pool can raise is a documented, decodable code that says whether a caller can retry, and no path escapes as an undescribed panic. Documenting variants while leaving the panic surface undescribed lets a caller still hit an opaque trap; describing only the caps leaves the deposit/withdraw arithmetic failures undocumented.

Labels

area: contracts type: documentation priority: medium

---

Workstream 1 — Document every LiquidityPoolError variant

Objective

Give each of the 19 variants a doc comment stating recoverability, trigger, and fix, with the cap/cross-contract family clearly distinguished from the local-validation family.

Problem

contracts/liquidity-pool-contract/src/errors.rs:6-26 declares 19 variants with no documentation. The risk-gate variants carry real policy: OutflowCapExceeded = 16 (lib.rs:418) and MerchantExposureCapExceeded = 17 (lib.rs:434) mean a settlement was refused for breaching a configured cap and the caller can retry with a smaller amount, while NotCreditLine = 9 (lib.rs:879,881) means the caller is not the authorised creditline contract and no retry helps. InvalidCap = 19 (lib.rs:70,82) is an admin-setup failure (set_caps validation) that is not a runtime settlement error at all. Nothing in the file conveys these distinctions.

Scope

• contracts/liquidity-pool-contract/src/errors.rs (edit — add /// docs to every variant)

Implementation

1. Add a /// <recoverable?>. <when>. <fix>. comment to each variant; classify OutflowCapExceeded/MerchantExposureCapExceeded and InvalidAmount/InsufficientLiquidity/InsufficientShares as recoverable-with-adjustment, NotCreditLine/AlreadyInitialized/NotInitialized/ReentrancyDetected as terminal, and name the remote contract for VendorNotActive (vendor-registry).
2. For OutflowCapExceeded and MerchantExposureCapExceeded, name the relevant cap in the "when" (pool outflow cap; per-vendor exposure cap) and the fix (retry below the cap, or admin raises it via set_caps).
3. Keep all codes unchanged; comments only.
4. Ensure the generated docs/ERROR_CODES.md uses these as the Recoverable/When/Fix columns.

Acceptance Criteria

• All 19 variants carry a recoverability/trigger/fix doc comment.
• The cap variants' docs state they are retryable-with-smaller-amount; NotCreditLine states terminal.
• cargo doc -p liquidity-pool-contract --no-deps renders the variant docs with no missing-docs warnings; no code value changes.

Testing

• Unit: a doc-coverage test asserting every Variant = n line in errors.rs is preceded by a ///.
• Integration: the pool section of docs/ERROR_CODES.md shows no TBD.

Workstream 2 — Close untyped-panic gaps

Objective

Confirm and, where present, eliminate any non-typed panic in the pool's runtime paths so settlement failures always return a code.

Problem

The pool's runtime paths are largely typed — every raise uses panic_with_error! or Err(...) (lib.rs:70,82,385,418,434,879,881, and :228 re-raises a storage error) — but the deposit/withdraw share math (lib.rs:209-212 documents shares = (amount × PRECISION) / share_price) has no documented error for the zero/overflow cases it can reach, and ZeroTotalShares = 10 exists without a stated trigger. A caller cannot tell what condition produces ZeroTotalShares versus InvalidAmount. A grep over contracts/liquidity-pool-contract/src/lib.rs finds no raw panic!/unwrap() today — this workstream keeps that true and locks it with a gate.

Scope

• contracts/liquidity-pool-contract/src/errors.rs (edit — document ZeroTotalShares trigger)
• contracts/liquidity-pool-contract/src/lib.rs (edit — only if a raw panic is found; otherwise no-op)

Implementation

1. Document ZeroTotalShares precisely (when total shares are zero and a division would be undefined) and confirm the code paths that raise it.
2. Add a CI-adjacent grep gate in the PR checklist: no panic!(" or .unwrap() in contracts/liquidity-pool-contract/src/lib.rs outside tests.
3. If any raw panic/unwrap is found on a runtime path, replace it with the appropriate LiquidityPoolError variant.

Acceptance Criteria

• grep -n 'panic!("' contracts/liquidity-pool-contract/src/lib.rs returns nothing.
• ZeroTotalShares has a documented trigger and at least one test that raises it.
• No raw unwrap() on a runtime (non-test) path in lib.rs.

Testing

• Unit: a test that drives the zero-total-shares condition and asserts the trap code equals ZeroTotalShares.
• Integration: try_* on a cap-breaching settlement returns Err with code OutflowCapExceeded / MerchantExposureCapExceeded.

Shared acceptance criteria (whole epic)

• cargo fmt --all --check, cargo clippy --workspace --all-targets --locked -- -D warnings, cargo build --locked --target wasm32-unknown-unknown --release, cargo test --locked all exit 0; test count does not drop (pool suite is 125 tests).
• No code value changes; documentation and gap-fill only.
• Variants documented and present in the generated docs/ERROR_CODES.md.
• No secrets; no changes outside the liquidity-pool crate.

Security & compatibility considerations

• Cap errors reveal only that a configured cap was hit, not the cap's value or the holder's exposure — docs must not instruct callers to compute internal exposure from the error.
• No code values move; existing clients and the creditline caller are unaffected.
• Doc comments must not expose admin-only storage keys or cap internals beyond what the code publishes.

References

• contracts/liquidity-pool-contract/src/errors.rs:6-26 — the 19-variant LiquidityPoolError to document.
• contracts/liquidity-pool-contract/src/lib.rs:70,82 — InvalidCap raise sites.
• contracts/liquidity-pool-contract/src/lib.rs:385 — VendorNotActive.
• contracts/liquidity-pool-contract/src/lib.rs:418 — OutflowCapExceeded.
• contracts/liquidity-pool-contract/src/lib.rs:434 — MerchantExposureCapExceeded.
• contracts/liquidity-pool-contract/src/lib.rs:879,881 — NotCreditLine.
• contracts/liquidity-pool-contract/src/lib.rs:209-212,225-232 — deposit share math and first-deposit seeding (ZeroTotalShares context).
• contracts/liquidity-pool-contract/src/tests.rs — suite to extend (125 tests).
• docs/standards/error-handling.md:53-63 — stale pool table this work supersedes.
• Feeds the generated error reference; aligns codes with the namespacing work.

If you're solving this with AI

In scope: contracts/liquidity-pool-contract/src/errors.rs, contracts/liquidity-pool-contract/src/lib.rs, contracts/liquidity-pool-contract/src/tests.rs — only.
Out of scope: other contracts; other repos; renaming variants or functions; renumbering codes; new dependencies; reformatting untouched files.
Must: follow the repo PR template exactly; keep CI green for real (build + fmt + clippy + tests); add the doc-coverage test and a ZeroTotalShares regression test; reference this issue (Closes #<n>); no secrets.

Contribution requirements: Follow the repo PR template exactly — https://github.com/StepFi-app/StepFi-Contracts/blob/main/.github/pull_request_template.md. Reference this issue in your PR. CI must pass green (build + fmt + clippy + tests). No secrets; no out-of-scope changes.

[ranjeet150]

Hi team,

I would like to take on and implement this issue: (Medium) ORG-E3.1 liquidity-pool error-model.

Technical Implementation Roadmap:

1. Formal Soroban Invariant Design: Audit storage layout (Instance, Persistent, Temporary) with strict TTL management and explicit require_auth guards.
2. Core Rust WASM Implementation: Implement deterministic state transition handlers eliminating arithmetic overflow risks using checked/saturating math.
3. Hermetic Testing Suite: Build comprehensive test cases in tests/ asserting authorized vs unauthorized invocations, mock auth environments, and edge value bounds.
4. Gas & Footprint Optimization: Profile CPU/memory instruction budgets under Soroban host environment to ensure optimal runtime efficiency.

I am ready to begin implementation immediately upon assignment and will deliver a high-quality, fully-tested PR. Please assign this issue to me!