(Medium) ORG-E3.1 reputation error-model

[EmeditWeb]

Summary

ReputationError is the workspace's smallest error enum — 11 variants (contracts/reputation-contract/src/errors.rs:7-19) — and still has no documentation, so the contract's semantics are all implicit. Four families are entangled: score-range (OutOfBounds = 3, raised at contracts/reputation-contract/src/lib.rs:96 and asserted by multiple tests, e.g. tests.rs:252,447,468), authorization/state (NotAdmin = 1, NotUpdater = 2 at access.rs:18), arithmetic (Overflow = 4, Underflow = 5), and the setup/upgrade set (NotInitialized = 6 at storage.rs:17, AlreadyInitialized = 11 at lib.rs:193, ReentrancyDetected = 7, UpgradeNotProposed = 8, UpgradeTimelockNotMet = 9, UpgradeHashMismatch = 10). The suite pins bare numeric codes (tests.rs:233 expects #2, :252 #3, :332 #8) which are ABI-load-bearing and undocumented. This issue is the reputation slice of the error-model program and must align with the storage-error migration that already routes NotInitialized through storage.rs.

Why these together: both workstreams are faces of one property — every score, authorization, and lifecycle failure in the reputation contract is a documented code, and its test suite asserts named variants rather than bare integers, so the ABI stays truthful through the rebanding. Documentation without symbolic tests leaves the numbers free to drift silently; symbolic tests without docs leave a caller guessing what 3 means.

Labels

area: contracts type: documentation priority: medium

---

Workstream 1 — Document every ReputationError variant

Objective

Give each of the 11 variants a doc comment stating family, recoverability, trigger, and fix, with OutOfBounds (the hot-path score error) called out precisely.

Problem

contracts/reputation-contract/src/errors.rs:7-19 declares 11 variants with no docs. OutOfBounds = 3 is the central semantic error — it means a score update would leave the 0-100 range (lib.rs:96) — yet nothing in the enum defines the range or says whether the caller should clamp and retry or abandon. NotUpdater = 2 (access.rs:18) is an authorization failure distinct from NotAdmin = 1; a caller receiving 2 versus 1 must know whether to call set_updater or set_admin. The upgrade variants (8,9,10) mirror every other contract's but are undocumented as a set.

Scope

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

Implementation

1. Add /// <family>. <recoverable?>. <when>. <fix>. to each variant. For OutOfBounds, state the 0-100 range (per the canonical tier table referenced by the PRD) and the fix (retry with an in-range delta / check current score). For NotUpdater vs NotAdmin, state the distinct remediation (set_updater vs set_admin).
2. Mark Overflow/Underflow as arithmetic guards on the score math; ReentrancyDetected as terminal; the upgrade trio as lifecycle with the timelock semantics.
3. Keep codes unchanged (comments only).
4. Ensure the generated docs/ERROR_CODES.md consumes these as its Recoverable/When/Fix columns.

Acceptance Criteria

• All 11 variants carry a family/recoverability/trigger/fix doc comment.
• OutOfBounds documents the 0-100 bound; NotUpdater and NotAdmin document distinct fixes.
• cargo doc -p reputation-contract --no-deps renders the docs with no missing-docs warnings; no code value changes.

Testing

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

Workstream 2 — Align with the migrated storage errors and de-magic test assertions

Objective

Confirm the storage-layer errors (NotInitialized, AlreadyInitialized) flow consistently, and convert the suite's bare numeric assertions to name the variants.

Problem

The storage layer already returns typed errors — storage.rs:17 returns ReputationError::NotInitialized, and lib.rs:193 returns AlreadyInitialized — and some tests use the named form (tests.rs:35,914 expect NotInitialized, :945 expects AlreadyInitialized), but many still pin bare integers: tests.rs:233 expects #2, :252 #3, :332 #8, :347 #9, :363 #10, :447,468 #4, :510 #5. This inconsistency means the same enum is asserted two ways, and the integer form will break or mis-pass when the codes are rebanded.

Scope

• contracts/reputation-contract/src/tests.rs (edit — assert named variants consistently)
• contracts/reputation-contract/src/lib.rs, storage.rs, access.rs (edit — only if a raw panic is found; otherwise no-op)

Implementation

1. Convert the #[should_panic(expected = "Error(Contract, #n)")] assertions to derive the expected code from the named variant, matching the already-symbolic tests at tests.rs:35,914,945.
2. Add a PR-checklist grep gate: no raw panic!(" or .unwrap() on runtime paths in lib.rs (none found today; keep it so).
3. Document NotInitialized/AlreadyInitialized as the storage lifecycle errors, aligned with the migration already reflected in storage.rs:17.

Acceptance Criteria

• No #[should_panic] in tests.rs hard-codes a bare #n without tying it to a named variant.
• grep -n 'panic!("' contracts/reputation-contract/src/lib.rs returns nothing.
• The full reputation suite (60 tests) passes.

Testing

• Unit: each converted assertion raises its named variant; existing named assertions still hold.
• Integration: try_* returns Err codes matching named variants.

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 (reputation suite is 60 tests).
• No code value changes here (values are namespaced by the separate reband work).
• Variants documented and present in the generated docs/ERROR_CODES.md.
• No secrets; no changes outside the reputation crate.

Security & compatibility considerations

• Score errors must not disclose another user's score; docs must keep the range public but not expose per-account values.
• Updater/admin errors must not reveal which address holds each role.
• No code values move; the numeric literals in tests are made symbolic but keep current meaning until the reband lands.

References

• contracts/reputation-contract/src/errors.rs:7-19 — the 11-variant ReputationError to document.
• contracts/reputation-contract/src/lib.rs:96 — OutOfBounds raise site.
• contracts/reputation-contract/src/lib.rs:193 — AlreadyInitialized return.
• contracts/reputation-contract/src/access.rs:15-18 — NotUpdater raise site.
• contracts/reputation-contract/src/storage.rs:17 — NotInitialized return (migrated storage error).
• contracts/reputation-contract/src/tests.rs:35,914,945 — already-symbolic assertions to match.
• contracts/reputation-contract/src/tests.rs:233,252,332,347,363,447,468,510 — numeric-code assertions to de-magic.
• docs/standards/error-handling.md:3-26 — stale reputation table (documents only 5 of 11 variants).
• Feeds the generated error reference; aligns codes with the namespacing work.

If you're solving this with AI

In scope: contracts/reputation-contract/src/errors.rs, contracts/reputation-contract/src/tests.rs, and lib.rs/storage.rs/access.rs only if a raw panic is found — otherwise those three are untouched.
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 de-magic the should_panic assertions; 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.

[shobhamerabacha-star]

Hello team,

I would like to take on and implement this issue: (Medium) ORG-E3.1 reputation 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.

Looking forward to delivering this robust implementation. Please assign whenever convenient!