fix(gas_manager): set_limits must pause when tightened limit is below current spend#120
Conversation
…ow current spend GasManager._update_pause_state_locked early-returned unless the wallet was already paused, so it could only ever *clear* a pause, never set one. Lowering a wallet's limit below its recorded spend via set_limits therefore left is_paused() reporting False while can_spend() (correctly) returned False — the two disagree. This breaks the documented drop-in parity with GasTracker, whose _update_pause_state recomputes the flag in both directions (_is_paused = not (can_proceed_hourly and can_proceed_daily)). A consumer that gates dispatch on the GasTracker-compatible is_paused() after tightening a budget would keep sending while the budget is actually exhausted. Fix: recompute the pause flag in both directions after evicting aged events (set_limits previously also skipped eviction, so the sums it read could be stale). +2 regression tests covering the per-hour and per-day tighten paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🤖 Audit verdict: Straightforward bug fix correcting an inconsistency where pause state and spend checks disagreed; adds appropriate test coverage with no malicious patterns, credential leaks, or supply-chain risks. Audited by the kcolbchain PR pipeline. See pipeline docs. |
The pause-direction regression tests assert that tightening a limit below current spend pauses the wallet, but the _update_pause_state_locked docstring contracts BOTH directions. Add the mirror case: a wallet paused by a too-low limit must unpause (and is_paused() must agree with can_spend()) when the limit is raised back above spend. Guards against a future regression restoring only the pause half.
|
Pushed a follow-up: added the mirror unpause regression test ( Heads-up on the earlier red CI: it wasn't this change — the |
|
Merged — thanks, @kite-builds. That's 7 merged PRs to kcolbchain now. You're a Fellow — when paid research, partner-org work, or grant milestones come up, you're in the first invite pool. Next-up issues across the org: https://kcolbchain.com/invitations/ |
Summary
GasManageris documented as a drop-in replacement for bothGasBudgetTrackerandGasTracker"without behavioural surprises" (issue #97). It isn't, for one path: tightening a wallet's limit below its current spend viaset_limits()leavesis_paused()andcan_spend()disagreeing._update_pause_state_lockedearly-returned unless the wallet was already paused:So lowering a limit below the recorded spend never flips
pausedtoTrue. The referenceGasTracker._update_pause_staterecomputes the flag in both directions (_is_paused = not (can_proceed_hourly and can_proceed_daily)).Impact
A consumer migrating from
GasTrackerthat gates dispatch on the compatibility aliasis_paused()after tightening a wallet budget will believe the wallet is healthy and keep sending, whilecan_spend()(correctly) rejects. Budget-enforcement state becomes internally inconsistent.Reproduction (before fix)
The reference
GasTrackerreportsis_paused() == Trueafter the equivalent operation.Fix
Recompute the pause flag in both directions after evicting aged events (
set_limitspreviously also skipped eviction, so the sums it read could be stale). Loosening a limit still unpauses; tightening below current spend now pauses.Tests
test_set_limits_below_spend_pauses_and_stays_consistent(per-hour)test_set_limits_per_day_below_spend_pauses(per-day)Both fail on
main(is_paused()returnsFalse), pass with the fix. Existingtest_pause_on_hit_release_on_set_limits/test_pause_stays_if_new_limits_still_exceededstill pass. Full Python suite: 220 passed, 62 skipped (1 pre-existing unrelatedflaskimport failure intest_x402_server),gas_manager.pyruff-clean.🤖 Generated with Claude Code