fix(contracts): keep enrollment_count and get_total_held in sync with state transitions

[nanoulloa]

Closes #10

What this fixes

Issue #10 flags three accounting bugs. Since the issue was filed, main gained inline increment/decrement blocks for the enrollment counter, so this PR completes the enrollment side (helpers, a remaining double-count bug, saturation) and fixes the still-live escrow bug:

1. lineproof-escrow: get_total_held() never decreased (live bug)

deposit() accumulated into escrow_total, but release(), refund(), and expire() never subtracted, so the reported held balance only ever grew. Extracted two private helpers and wired them into every transition:

• add_held(env, queue_id, amount) — saturating add, used by deposit().
• subtract_held(env, queue_id, amount) — saturating sub clamped at zero, used by release(), refund(), and expire().

2. lineproof-enrollment: counter helpers + a promotion double-count

• Extracted increment_count / decrement_count (both saturating; decrement clamps at 0 instead of the previous if count > 0 guard) and replaced the three duplicated inline blocks in enroll(), cancel(), and promote_from_waitlist().
• Fixed a double-count found while writing tests: promote_from_waitlist() unconditionally incremented the counter, but a waitlisted identity by construction already holds a live enrollment record (the waitlist only accepts duplicate enrollers), so promotion overwrites the record yet inflated the count. Promotion now increments only when the identity has no live record — the counter is exactly "number of live enrollment records per queue".
• Removed a duplicated extend_ttl call in enroll().

3. Both test suites did not compile on main

cargo test -p lineproof-escrow and -p lineproof-enrollment failed with 16 and 9 compile errors respectively (calls to a nonexistent Address::new, direct trait calls outside a contract context, Val compared with ==, moved Env). The escrow suite was migrated to the generated-client pattern (env.register + EscrowImplClient + mock_all_auths), preserving every existing test's intent; the enrollment suite had its leftover direct-call fragments removed.

Acceptance criteria → tests

Criterion	Test
enrollment_count correct after multiple enroll()	test_enrollment_count_increments_per_enroll
decreases by 1 after cancel()	test_enrollment_count_decrements_on_cancel
never goes below 0 (saturating)	test_enrollment_count_never_underflows (forces drifted storage via env.as_contract, then cancels)
get_total_held = 0 after all deposits released/refunded/expired	test_total_held_zero_after_all_transitions
correctly reflects partial releases	test_total_held_decreases_on_release (2 deposits, 1 release)
never goes below 0	test_total_held_never_goes_negative (drifted storage, release clamps at 0)
per-transition coverage	test_total_held_decreases_on_refund / _on_expire, test_total_held_is_isolated_per_queue
waitlist/promotion/override don't corrupt the count	test_enrollment_count_ignores_waitlist_and_survives_promotion, test_enrollment_count_unchanged_by_override_expired
no unwrap() without fallback on first-access keys	all counter/total reads use unwrap_or(0) inside the helpers

Test output

$ cargo test -p lineproof-enrollment
running 24 tests
...
test result: ok. 24 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.14s

$ cargo test -p lineproof-escrow
running 16 tests
...
test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.15s

Full per-test output

running 24 tests
test test::test_enrollment_count_starts_at_zero ... ok
test test::test_cancel_panics_when_not_enrolled - should panic ... ok
test test::test_enrollment_record_returns_none_when_missing ... ok
test test::test_enrollment_count_never_underflows ... ok
test test::test_cancel_removes_enrollment ... ok
test test::test_cancel_emits_original_hash ... ok
test test::test_enroll_creates_record ... ok
test test::test_enroll_rejects_duplicate_by_default - should panic ... ok
test test::test_enrollment_count_decrements_on_cancel ... ok
test test::test_is_enrolled_returns_correct_state ... ok
test test::test_finalize_enrollment ... ok
test test::test_finalize_twice_panics - should panic ... ok
test test::test_enrollment_count_unchanged_by_override_expired ... ok
test test::test_override_expired_rejection - should panic ... ok
test test::test_enrollment_count_increments_per_enroll ... ok
test test::test_multiple_users_same_queue ... ok
test test::test_set_duplicate_behavior ... ok
test test::test_enrollment_count_ignores_waitlist_and_survives_promotion ... ok
test test::test_set_duplicate_behavior_emits_event ... ok
test test::test_proof_hash_is_distinct_for_different_inputs ... ok
test test::test_override_expired_success ... ok
test test::test_waitlist_addition_and_queries ... ok
test test::test_waitlist_prevent_duplicate_waitlist_entry - should panic ... ok
test test::test_waitlist_promotion ... ok

test result: ok. 24 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.14s
running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

running 16 tests
test test::test_set_and_get_config ... ok
test test::test_deposit_rejects_above_max - should panic ... ok
test test::test_deposit_rejects_non_positive_amount - should panic ... ok
test test::test_deposit_creates_record ... ok
test test::test_expire_updates_status ... ok
test test::test_release_already_released_panics - should panic ... ok
test test::test_deposit_rejects_duplicate_for_same_user_and_queue - should panic ... ok
test test::test_refund_changes_status ... ok
test test::test_release_changes_status ... ok
test test::test_get_total_held_accumulates ... ok
test test::test_total_held_decreases_on_expire ... ok
test test::test_total_held_decreases_on_refund ... ok
test test::test_total_held_is_isolated_per_queue ... ok
test test::test_total_held_never_goes_negative ... ok
test test::test_total_held_decreases_on_release ... ok
test test::test_total_held_zero_after_all_transitions ... ok

test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.15s
running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Contributor note

Was the missing counter intentional? Almost certainly not. The contract shipped count_key, a public enrollment_count() getter, and (in escrow) an event trail designed for auditors — all the read-side machinery exists, only the write side was missing. That pattern (getter + storage key with no writer) reads as scaffold code where the wiring was deferred and forgotten, not a design decision. The same asymmetry appearing independently in both crates (accumulate-without-reverse in escrow, read-without-write in enrollment) supports "incomplete scaffold" over intent.

In-storage counter vs. deriving from the index at query time:

• In-storage counter (chosen): O(1) read, O(1) extra write per transition. The cost is discipline — every path that creates or removes a record must touch the counter symmetrically, which is exactly the bug class this issue is about. Centralizing the writes in two private helpers (and defining the invariant as "count == live records") is the mitigation: new transitions have one obvious call to make, and saturation bounds the damage if drift ever happens.
• Derive at query time: structurally bug-proof (single source of truth), but requires an enumerable per-queue index. Enrollment records are keyed (queue_id, identity), so counting would require maintaining a Vec<Address> per queue — which reintroduces the same write-symmetry burden on a more expensive structure, plus unbounded read cost and instruction-budget risk on large queues. For escrow, summing all records is worse still (i128 amounts across unbounded records).
• Given Soroban's per-entry storage pricing and instruction metering, the O(1) counter is the right trade here; the helpers + tests supply the discipline it demands.

🤖 Generated with Claude Code

[vercel]

@nanoulloa is attempting to deploy a commit to the Deejah Team on Vercel.

A member of the Team first needs to authorize it.

[k-deejah]

Welldone, please resolve conflicts