Skip to content

Contracts: advance() with batch_size=0 still transitions queue to AdvancementActive and wastes ledger resources #199

Description

@k-deejah

Difficulty: Advanced

Problem

1. advance(env, admin, 0) transitions the queue to AdvancementActive with zero work done
contracts/lineproof-queue/src/lib.rs lines 120–160: the function immediately sets config.status = QueueStatus::AdvancementActive and writes to storage before entering the loop. With batch_size = 0, the loop body never executes, but the status transition and config storage write still occur. The queue permanently transitions to AdvancementActive with nothing advanced.

2. After advance(batch_size: 0), subsequent calls panic with "enrollment must be closed before advancing"
The status is now AdvancementActive (not EnrollmentClosed), so the guard if !matches!(config.status, QueueStatus::EnrollmentClosed) { panic!("enrollment must be closed") } will trigger on all future calls. The queue is stuck in AdvancementActive with all positions still Pending and no way to advance them.

3. The event emission loop also runs for zero iterations — no events emitted, but storage is still modified
The storage write for idx (line 155) and the config write (line 127) still execute even with batch_size = 0. This consumes Soroban fee budget for storage modifications that accomplish nothing.

Impact: An operator who accidentally calls advance(batch_size: 0) permanently bricks the queue — all positions remain Pending forever with no way to advance them. This is a one-way destructive operation with no error.

Proposed Solution

  • Add a guard at the start of advance(): if batch_size == 0 { panic!("batch_size_must_be_positive") }.
  • Move the config.status = AdvancementActive write to after at least one position is successfully advanced (or conditionally, only if the loop produces at least one result).
  • Add a test: #[should_panic(expected = "batch_size_must_be_positive")] for advance(0).

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
  • All existing advance tests still pass
  • cargo test -p lineproof-queue passes with zero failures

Contributor Note

If assigned, your PR must also consider whether advance() that produces zero results (all-cancelled batch) should transition to AdvancementActive or remain in EnrollmentClosed, and document the decision.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third CampaignbugSomething isn't workingcontractssoroban

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions