#101 — / View Functions Panic on Missing IDs: Inconsistent Semantics

[Topmatrixmor2014]

Labels: Official Campaign | FWC26 GrantFox OSS Maybe Rewarded contract soroban api robustness bug

This is a smart-contract issue for the GrantFox FWC26 campaign. Return ContractError::NotFound from the stream/escrow getters so off-chain clients can distinguish "missing" from "contract error".

Requirements and Context

contracts/finchippay-contract/src/streams.rs:

• get_stream returns Result<Stream, ContractError> with ContractError::NotFound — good.
• get_claimable (≈ line 460) uses .expect("stream not found") — panics.
• list_streams_by_payer uses .unwrap() on p_streams.get(i) and .unwrap() on the Stream lookup — panics on a stale index.

contracts/finchippay-contract/src/escrow.rs:

• get_escrow returns Result<Escrow, ContractError> — good.
• get_user_escrows silently returns an empty Vec for a missing key — acceptable but inconsistent.

The mix of Result, panic!, and silent-empty semantics across the read API forces SDK/indexer authors to special-case each call and makes a genuine NotFound indistinguishable from a contract bug. A stale StreamByPayer index entry (possible with the current silent-index-drop behavior, see batch-3 #62) would permanently panic list_streams_by_payer for that payer.

Objectives

1. Convert get_claimable to return Result<i128, ContractError> (or 0 on missing — pick one and document).
2. Make list_streams_by_payer skip (or return an error for) stale index entries instead of unwrap()-panicking.
3. Add tests for missing IDs and stale indexes.

Suggested Execution

1. Fork and branch: git checkout -b fix/view-semantics.
2. Patch streams.rs getters; align escrow getters' semantics.
3. Add tests; run cargo fmt --check && cargo clippy -- -D warnings && cargo test.

Acceptance Criteria

• Every public getter returns a documented, non-panicking result for missing input.
• Stale index entries are handled without panicking.
• ≥4 tests; cargo test + wasm32v1-none build pass.

Guidelines

• Keep SDK-facing names stable; document the chosen semantics in rustdoc.

Timeframe: 24 hours

[github-actions]

🤖 Greptile AI Context

Greptile has indexed this repository and can provide AI-assisted code review
and context for this issue.

How to use Greptile on this issue:

• Comment @greptileai to ask about relevant code, files, or implementation approaches
• Reference specific files or error messages for targeted analysis
• Use @greptileai explain <file> to get an explanation of any file

Greptile is configured with project-specific rules in .greptile/config.json.

[safal207]

Hi @Topmatrixmor2014 — I’d like to work on #882.

For get_claimable, I would return ContractError::NotFound rather than 0: an existing stream can legitimately have zero currently claimable value, so using zero for a missing stream would make two different states indistinguishable.

My approach:

1. Convert get_claimable to an explicit Result<i128, ContractError> path and remove the panic-based lookup.
2. Handle stale StreamByPayer entries without unwrap() while preserving the existing SDK-facing function names and documenting the chosen list semantics.
3. Add at least four regression tests covering a missing stream, a valid zero-claimable stream, a stale payer index, and a normal multi-entry listing.
4. Run formatting, Clippy, the complete test suite, and the wasm32v1-none build.

I have recent Soroban/Rust experience with structured contract errors, failure-path atomicity, and state/index consistency.

I can open the first draft within 24 hours after assignment and will wait for maintainer confirmation before starting.

[dollarop]

I'd like to take this on if it is still available. I will keep the change limited to non-panicking getter semantics, stale-index handling, rustdoc, and the required regression tests. I will wait for maintainer/GrantFox assignment before opening a PR to avoid duplicating the existing applicant's work.