maintenance-pool::get_deposit returns misleading Error::PoolNotFound for a missing deposit index, not just a missing pool

[chonilius]

Overview

maintenance-pool::get_deposit returns the wrong Error variant for a missing record:

pub fn get_deposit(env: Env, pool_id: u64, index: u32) -> Result<Deposit, Error> {
    env.storage()
        .persistent()
        .get(&DataKey::Deposit(pool_id, index))
        .ok_or(Error::PoolNotFound)
}

(contracts/maintenance-pool/src/lib.rs:166-171). When the lookup misses, it returns Error::PoolNotFound — but a miss here can mean two semantically different things: (a) pool_id itself was never created (genuinely "pool not found"), or (b) pool_id exists and has deposits, but index is out of range (e.g. index >= pool.deposit_count, or an index that was never valid to begin with) — which is not "the pool wasn't found" at all, it's "this deposit wasn't found." PoolNotFound conflates both into one error code, with no way for a caller to distinguish them.

This is a real, isolated mistake, not a codebase-wide pattern: escrow::get_escrow correctly returns Error::EscrowNotFound (contracts/escrow/src/lib.rs:218-223), milestones::get_milestone correctly returns Error::MilestoneNotFound (contracts/milestones/src/lib.rs:216-221), and milestones::get_issue_status correctly returns Error::IssueNotAllocated for its own distinct "not found" case (contracts/milestones/src/lib.rs:223-232) rather than reusing MilestoneNotFound. get_deposit is the one get_* function across all three contracts that reuses a different record type's not-found error instead of having (or reusing) its own — confirmed via grep -n "ok_or(Error::" contracts/*/src/lib.rs across all three contracts.

This directly undermines the design #10 ("Design a bounded/paginated access pattern for maintenance-pool deposit history") will need: any reasonable pagination strategy built on top of get_deposit (e.g. "call get_deposit(pool_id, i) for i = 0, 1, 2, ... until you hit a not-found error, then stop") needs to distinguish "you've reached the end of this pool's deposit list" from "the pool_id itself is wrong" — with the current single conflated error, a backend integration can't tell a legitimate end-of-list condition from a caller bug that passed the wrong pool_id entirely, silently masking bugs in whatever pagination logic #10 eventually produces.

Requirements

• Add a DepositNotFound (or equivalently-named) Error variant and return it from get_deposit when the pool exists but the specific (pool_id, index) deposit record doesn't.
• Decide whether get_deposit should first check pool existence (returning PoolNotFound if the pool itself was never created) before checking the deposit index specifically (returning the new DepositNotFound otherwise), so both failure modes remain distinguishable and correctly attributed.

Acceptance Criteria

• New DepositNotFound Error variant added to contracts/maintenance-pool/src/error.rs
• get_deposit returns PoolNotFound only when pool_id itself doesn't exist, and DepositNotFound when the pool exists but the index doesn't
• test_get_deposit_distinguishes_missing_pool_from_missing_index added, asserting both distinct error variants for their respective scenarios
• cargo test --workspace passes

Additional Notes

• Precise reference: contracts/maintenance-pool/src/lib.rs:166-171 (the bug), contrasted with the correctly-scoped get_escrow/get_milestone/get_issue_status implementations cited above as the pattern this should follow.
• Test sketch: test_get_deposit_distinguishes_missing_pool_from_missing_index — call get_deposit(pool_id=999, index=0) on a pool_id that was never created via deposit(), assert Error::PoolNotFound; separately, deposit() once into pool_id=1 (creating it with deposit_count=1, valid index 0), then call get_deposit(pool_id=1, index=5), assert the new Error::DepositNotFound rather than the current PoolNotFound.
• Cross-references: Design a bounded/paginated access pattern for maintenance-pool deposit history #10 (bounded/paginated deposit-history access pattern — this issue's fix is a direct, small prerequisite for building correct pagination logic on top of get_deposit, since paginating code needs to distinguish "end of list" from "wrong pool" to behave correctly); this is scoped as a narrow, standalone correctness fix rather than folded into Design a bounded/paginated access pattern for maintenance-pool deposit history #10 itself since it's valuable and testable independent of whether/when Design a bounded/paginated access pattern for maintenance-pool deposit history #10's larger pagination design lands.