Seven `unwrap()`/`panic!` sites in contract source bypass the typed error enum and trap with opaque host errors

[Jagadeeshftw]

Priority: Medium  ·  Area: Soroban contract / error handling  ·  Est. effort: 5–8 h

📌 Problem

The crate has a dedicated src/error.rs, so typed contract errors are the established pattern. Despite that, 7 unwrap()/expect()/panic! sites remain in non-test contract source — lib.rs, storage.rs, events.rs, types.rs, error.rs.

(The crate has 234 such sites in total, but 227 are inside src/test.rs, where they are entirely appropriate. This issue is only the 7 in contract source — do not touch the test ones.)

When one of these fires, the contract does not return a typed error. It traps, and the client receives an undecodable host error with no indication of which precondition failed — "not authorised", "record does not exist" and "never initialised" become indistinguishable.

Seven is a small, tractable number. That is what makes this a good candidate for a complete fix rather than a partial one.

🎯 Design decision required

A blanket replacement with ? will be rejected. Classify each of the 7 and put the table in the PR:

1. Recoverable precondition — a missing or invalid stored value a caller can act on. Must become a typed error from src/error.rs.
2. Genuine invariant — a state the contract's own logic makes unreachable. May remain a panic, but must carry a comment naming the invariant and why it holds.
3. Misplaced test helper — code that belongs behind #[cfg(test)]. Say so and move it.

You may need to add variants to the error enum. That is an ABI-visible change: state whether you appended (safe) or renumbered (breaking), and why.

🧩 Requirements and context

• New error variants must be appended with fresh discriminants — off-chain code decodes these by number. Do not renumber existing ones.
• Every new variant needs a test provoking it and asserting the exact error via a try_* client method.
• Soroban caps type and function identifiers at 30 characters — check any new variant name.
• Report the wasm byte delta; the contract's size is currently unmeasured (tracked separately).
• Consider adding a CI guard — a grep, or a lint — so new unguarded unwraps cannot enter contract source. Say whether you did and why.

🛠️ Suggested execution

1. List all 7 with file and line, and classify each. Put it in the PR before writing code.
2. Add any needed error variants.
3. Convert the recoverable sites; annotate the invariant ones.
4. Add a test per new variant.
5. Add the regression guard if you argued for one.

✅ Acceptance criteria

• The PR contains a table of all 7 sites with file, line and classification.
• Every recoverable site returns a typed error.
• Every surviving panic carries a comment naming its invariant.
• Each new error variant has a test asserting that exact variant.
• Existing error discriminants are unchanged — shown by an enum diff.
• No test-module unwrap() was touched.
• cargo test passes (305 tests); wasm byte delta reported.

🚫 Out of scope

• The 227 unwrap()s in src/test.rs — appropriate as they are.
• Splitting test.rs or lib.rs — separate issues.
• Adding clippy — separate issue.

🧪 Verification

cargo test
grep -n '\.unwrap()\|\.expect(\|panic!' src/lib.rs src/storage.rs src/events.rs src/types.rs src/error.rs
cargo build --target wasm32-unknown-unknown --release
cargo fmt --all -- --check

📤 What your PR must include

1. The 7-site classification table.
2. Your ABI-compatibility statement.
3. Before/after wasm size.
4. Whether you added a regression guard, and why.
5. Closes #<n>.

🔒 Security notes

Traps and typed errors are not equivalent failure modes. An integrator cannot distinguish an authorisation failure from a missing record from an uninitialised contract, which pushes clients toward blind retries against a contract whose state they cannot determine. On settlement and liquidity paths, an opaque failure is indistinguishable from a transient fault.

📋 Guidelines

• Minimum 95% test coverage on changed lines
• Clear documentation
• Timeframe: 96 hours from assignment
• One logical change per commit; no merge commits

---

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

[madisonsc52-del]

Hi maintainer i can work on this, I'll make sure to work on the issue asap

[Jagadeeshftw]

Assignment update — typed contract failure paths

Good luck, @madisonsc52-del — I am assigning you the contract error-path audit. Please read the full issue description and cover every unwrap, panic, and opaque failure site and the typed-error expectations it identifies. The PR should be comprehensive and high quality, with focused tests proving the new behavior, clear handling of edge cases, and verification that the contract no longer traps with opaque host errors.

Please raise the PR within 48 hours of assignment. If a PR is not opened by then, the issue may be unassigned and reassigned. Please join the Telegram group before you begin so you can coordinate with the project team: https://t.me/+u5qmu35nZ7I0OTU1

[grantfox-oss]

🦊 GrantFox — @madisonsc52-del has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #263)
2. Your PR will be reviewed by the AnchorNet-Org maintainers

Good luck! Track your progress on GrantFox.