fix: advance(batch_size=0) must not brick the queue (#199)

[BigDella]

Summary

advance() had no guard against batch_size == 0. With batch_size = 0 the for _ in 0..batch_size loop body never executes — nothing advances — but the function still fell through to write the idx storage key, wasting Soroban fee budget for zero work.

The original issue also described this as bricking the queue permanently in AdvancementActive with no way to advance. That specific consequence is already avoided in the current code — this contract gates the AdvancementActive transition on !advanced.is_empty() — so a zero-result advance() no longer commits the status change. But the contract still silently accepted a call that can never do anything useful, and the issue's acceptance criteria explicitly wants the operator-facing panic plus a status-preserving guarantee proven by test, so:

Changes

• Added if batch_size == 0 { panic!("batch_size_must_be_positive") } at the very top of advance(), before any storage read or write (before config/admin are even loaded) — a caller that passes 0 finds out immediately, and there's nothing left to accidentally mutate.
• Added test_advance_zero_batch_size_panics (#[should_panic(expected = "batch_size_must_be_positive")]).
• Added test_advance_zero_batch_size_does_not_change_queue_status, which catches the panic via std::panic::catch_unwind (the same pattern already used elsewhere in test.rs), asserts the queue is still EnrollmentClosed afterward, and then proves the queue is still advanceable with a real batch size — i.e. advance(0) can't brick it even implicitly.

The contributor note's open question

Whether an all-cancelled batch (loop runs, advanced stays empty, only Skipped events fire) should transition to AdvancementActive: left as-is — it does not, per the existing !advanced.is_empty() guard. A queue where every position in the sampled range was pre-cancelled hasn't actually progressed, so treating it as "still closed, nothing done" rather than silently committing to AdvancementActive on cancellations alone seemed like the safer default; happy to discuss if the intended behavior is different.

Acceptance Criteria

• advance(env, admin, 0) panics with "batch_size_must_be_positive"
• Queue status is NOT changed to AdvancementActive when advance() panics on validation
• #[should_panic(expected = "batch_size_must_be_positive")] test added
• Status transition to AdvancementActive only occurs when loop produces ≥1 result (pre-existing, verified untouched)
• All existing advance tests still pass (unmodified — new guard only affects the batch_size == 0 path)
• cargo test -p lineproof-queue passes with zero failures — could not run locally, see verification note

Verification

No local Rust toolchain is available in this environment (link.exe fails compiling proc-macro2/quote build scripts — unrelated to this change). Verified instead by close manual review: the added guard is a single early-return panic with no interaction with any other code path, the rest of the existing advance() test suite is untouched and calls advance() with batch sizes ≥ 1 throughout, and the new catch_unwind test reuses an already-proven-working pattern from elsewhere in the same file. Would appreciate CI/a reviewer confirming cargo test -p lineproof-queue locally.

Closes #199

[vercel]

@Obiajulu-gif is attempting to deploy a commit to the Deejah Team on Vercel.

A member of the Team first needs to authorize it.