Airdrop status has no enforced state machine — cancel() and update() succeed regardless of current status

[prodbycorne]

Overview

src/services/airdrops.js's cancel() only special-cases the airdrop already being cancelled:

async function cancel(id) {
  const airdrop = await get(id);
  if (!airdrop) return null;
  if (airdrop.status === 'cancelled') {
    return airdrop;
  }
  const updated = { ...airdrop, status: 'cancelled', updated_at: new Date().toISOString() };
  await cache.set(airdropKey(id), updated);
  return updated;
}

There is no check against any other status. update() similarly applies name/description/expiry_ledger changes to an airdrop regardless of its current status. The status field itself only ever transitions via create() (initialized to 'draft') and cancel() (to 'cancelled') anywhere in this codebase — there is no code path that ever sets it to 'executing', 'completed', or 'failed', despite the README's webhook events list explicitly documenting airdrop.executing, airdrop.completed, and airdrop.failed as lifecycle events this system is meant to model. This means today, calling POST /airdrops/:id/cancel on an airdrop is accepted unconditionally regardless of whatever real-world state it may eventually represent once execution is wired up (e.g. cancelling an airdrop that has already begun distributing funds on-chain must not be silently accepted the same way as cancelling a still-draft one), and PATCH /airdrops/:id allows editing expiry_ledger/name/description on an airdrop in any status with no restriction (e.g. renaming or extending the expiry of an airdrop that should, per its documented lifecycle, already be immutable once it starts executing).

Requirements

• Introduce an explicit, enforced status state machine for airdrops covering at least draft → executing → completed, draft → executing → failed, and draft|executing → cancelled, matching the lifecycle already implied by the webhook events documented in the README.
• cancel() must reject (with a clear AppError, not a silent no-op or unconditional success) attempts to cancel an airdrop whose status is not in an allowed "cancellable" set (e.g. draft, executing — not completed, not already-terminal failed).
• update() must reject edits to fields that should be immutable once an airdrop leaves draft status (at minimum expiry_ledger, since extending/shortening an in-flight distribution window after execution has started is a correctness hazard).
• Add the missing status-transition functions (even if the actual on-chain execution trigger is out of scope for this issue, the service-layer transition functions and their guards should exist and be tested, since nothing currently prevents a future caller from doing an unguarded cache.set with an arbitrary status string bypassing validation entirely).
• Document the full state machine (a simple table or diagram) in the README's airdrop section.

Acceptance Criteria

• POST /airdrops/:id/cancel returns a 409-style AppError (not success) when called against an airdrop whose status is not cancellable, with a message naming the current status.
• PATCH /airdrops/:id rejects attempts to modify expiry_ledger (at minimum) once an airdrop is no longer draft.
• New service-layer functions exist for transitioning to executing, completed, and failed, each validating the current status permits that transition.
• Invalid transitions (e.g. completed → executing) are rejected at the service layer, not just the route layer, so no future caller can bypass the guard.
• README documents the full airdrop status state machine.
• New tests cover every valid and at least one invalid transition per status.

Additional Notes

More precise references

• src/services/airdrops.js:107-123 (cancel): only special-cases airdrop.status === 'cancelled' (returns as-is, no error) — every other status (including a hypothetical future 'completed') falls through to the unconditional cache.set(airdropKey(id), { ...airdrop, status: 'cancelled', ... }).
• src/services/airdrops.js:79-94 (update): applies name/description/expiry_ledger unconditionally regardless of airdrop.status; no status check exists anywhere in this function.
• src/services/airdrops.js:28-54 (create): confirmed status: 'draft' is the only place a status is ever initialized.
• Confirmed via grep-level read of airdrops.js in full: no other function ever writes status — cancel is the only status-mutating function that exists today, so 'executing'/'completed'/'failed' are indeed entirely unreachable states in current code, exactly as the issue states.
• src/routes/airdrops.js:134-151 (PATCH /airdrops/:id): validateAirdropUpdate (line 67-73) only validates expiry_ledger's relationship to currentLedger, never checks the airdrop's current status before calling airdropsService.update.

Additional edge cases

• remove() (src/services/airdrops.js:96-105) has the identical gap — nothing stops DELETE /airdrops/:id from removing an airdrop mid-executing (once that status exists), silently discarding a distribution that may be in-flight on-chain with no record left behind for reconciliation. The issue's requirements list doesn't explicitly mention remove(); worth deciding whether hard-delete should be disallowed entirely once an airdrop leaves draft (softer "cancelled" should probably be the only terminal-from-non-draft path, with remove() restricted to draft/already-terminal statuses).
• addRecipients (src/routes/airdrops.js:179-222, src/services/airdrops.js:125-128) similarly performs no status check — recipients can be added to an airdrop that's already executing/completed once those states exist, which is a correctness hazard symmetric to the update() gap called out in the issue (changing the recipient set after execution has started/finished is arguably worse than changing expiry_ledger, since it directly affects fund distribution amounts).
• Since create()/cancel() are the only status writers today, a "bypass" via direct cache.set isn't just theoretical — any future PR wiring up on-chain execution will need to add new status-transition code, and if that code doesn't reuse a shared guarded transition function, it's trivially easy to bypass validation by writing status: 'executing' directly, exactly as the issue's requirements flag.

Implementation sketch

1. Define the state machine as a plain data structure, e.g. in src/services/airdrops.js:

   const TRANSITIONS = {
     draft: ['executing', 'cancelled'],
     executing: ['completed', 'failed', 'cancelled'],
     completed: [],
     failed: [],
     cancelled: [],
   };
   function assertTransition(current, next) {
     if (!TRANSITIONS[current]?.includes(next)) {
       throw new AppError('INVALID_STATE_TRANSITION', `cannot transition airdrop from ${current} to ${next}`, 409);
     }
   }

2. Add transitionTo(id, nextStatus) as the single internal choke point; rewrite cancel() to call transitionTo(id, 'cancelled') (which naturally rejects completed → cancelled per the table); add markExecuting(id), markCompleted(id), markFailed(id) as thin wrappers.
3. In update(), add if (airdrop.status !== 'draft' && data.expiry_ledger !== undefined) throw new AppError('VALIDATION_ERROR', 'expiry_ledger is immutable once an airdrop leaves draft', 409); before applying the patch.
4. Propagate the AppError (409) up through src/routes/airdrops.js's existing try/catch → next(err) — no route-layer changes needed since errorHandler.js presumably already serializes AppError correctly (confirm during implementation).
5. README: add a table draft → executing → {completed|failed}, draft|executing → cancelled, with a note that remove()/addRecipients() are restricted to draft (if that's the decision made per the edge case above).

Test/reproduction plan

• Unit tests for assertTransition: every valid pair in TRANSITIONS succeeds; at least one invalid pair per starting state (completed → executing, cancelled → executing, failed → completed) throws 409.
• cancel() against a completed-status fixture (manually seeded in cache, since no code path produces completed yet) → assert 409, not silent success.
• update() with expiry_ledger against an executing-status fixture → assert 409; against a draft fixture → assert success (regression check that the common case still works).
• Route-level supertest: POST /airdrops/:id/cancel against a manually-seeded completed airdrop → assert HTTP 409 body names the current status per the acceptance criteria's explicit wording requirement.

Cross-references

• Airdrops never auto-expire against expiry_ledger — no reconciliation job checks current chain state #88 (auto-expiry reconciliation job) is the primary future consumer of the executing/failed transitions this issue introduces — that job will call whatever markFailed/markExpired function this issue creates, so the exact function names and signatures chosen here should anticipate Airdrops never auto-expire against expiry_ledger — no reconciliation job checks current chain state #88's needs (e.g. accepting a reason string for the webhook payload).
• Repository create() functions perform non-atomic two-step Redis writes, risking orphaned or dangling records on crash #84 (non-atomic Redis writes) — once transitionTo centralizes all status writes into one function, that function becomes the natural single place to apply the MULTI/EXEC atomicity fix from Repository create() functions perform non-atomic two-step Redis writes, risking orphaned or dangling records on crash #84 if a status transition ever needs to update more than one key (e.g. an index of "airdrops by status" if one is added later).
• multer() is configured with no file size limit — unbounded CSV upload enables memory-exhaustion DoS #85 (CSV upload DoS) — per the edge case above, addRecipients has no status check; if Airdrop status has no enforced state machine — cancel() and update() succeed regardless of current status #87's status guard is extended to addRecipients as suggested, that change touches the same route handler #85 is also modifying (POST /airdrops/:id/recipients), so sequence carefully to avoid merge conflicts.

[OluRemiFour]

I'm interested in contributing to this issue because enforcing a well-defined state machine is essential for maintaining consistency and preventing invalid airdrop operations.

My approach is to introduce explicit service-layer state transition rules for the complete airdrop lifecycle, ensuring every transition is validated regardless of the caller. I'll update cancel() and update() to reject invalid operations with clear AppError responses, preventing cancellation of terminal states and modification of immutable fields once an airdrop leaves the draft stage.

I'll also add dedicated transition functions for executing, completed, and failed, document the lifecycle in the README, and provide comprehensive tests covering valid transitions, invalid transitions, and route behavior to ensure the state machine is enforced consistently throughout the application.

I have experience building reliable backend services with TypeScript/Node.js and implementing state-driven business logic with comprehensive test coverage, and I'm confident I can deliver a robust, maintainable solution.

[grantfox-oss]

🦊 GrantFox — @OluRemiFour has been assigned to this issue as part of the Official Campaign | FWC26 campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #87)
2. Your PR will be reviewed by the SmartDropLabs maintainers

Good luck! Track your progress on GrantFox.