fix(control-panel): consume deploy quota before spending cycles - #652
Draft
MRmarioruci wants to merge 1 commit into
Draft
fix(control-panel): consume deploy quota before spending cycles#652MRmarioruci wants to merge 1 commit into
MRmarioruci wants to merge 1 commit into
Conversation
deploy_station checked the per-user and global rate limiters before doing any work, but only advanced them inside add_deployed_station, which runs on the fully successful path. Creating and funding the station happens well before that and is irreversible, so a deployment that failed after those steps had already spent real cycles while leaving both counters untouched. The pre-flight check therefore always passed and the spend was unbounded. The failure was also trivially forced rather than incidental: station init rejects an empty user list, so passing no admins fails deterministically after the station has been created and funded. Charge the quota immediately before the first irreversible spend, and reject an empty admin list up front so that path costs nothing at all. Recording the station and registering it for monitoring stay on the success path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
deploy_stationchecks the per-user quota (user.can_deploy_station()) and the controller checks the global quota before any work happens, but both counters are advanced only insideadd_deployed_station, which runs on the fully successful path.Creating and funding the station happens in between and is irreversible. A deployment that fails after those calls has spent cycles while leaving both counters untouched, so the check that gates the next attempt sees no change.
The quota is now charged immediately before the first irreversible spend. Recording the station id and registering it with the fund manager stay on the success path, where the canister id exists. Failures before the charge point — unknown user, uninitialised config, quota exceeded — still consume nothing.
adminsis also validated up front. Station init requires at least one user, so an empty list cannot succeed, and checking it before any spend costs nothing.Tests
consume_deploy_quota_charges_the_user_without_recording_a_station— exhausting the per-user quota yieldsQuotaExceededand no station is recorded, confirming the charge is independent of success.consume_deploy_quota_advances_the_global_limiter.cargo test -p control-panel --libpasses (209). Clippy andcargo fmtclean.Note
This changes observable behaviour: a deployment that fails after canister creation now consumes a day's quota. That is deliberate, since the cycles were spent regardless, but it means a user hitting a genuine infrastructure error loses quota. Releasing the charge on failure cannot distinguish a no-spend failure from a post-spend one once the creation call has returned.