forked from sass/tipibot
Fix money-safety bugs in economy (heist, blackjack, bail)
Multi-agent audit of the money-moving economy modules surfaced three confirmed correctness bugs. All three are fixed here with regression tests. heist (core/economy/heist.py): - `house = await get_user(house.HOUSE_ID)` shadowed the imported `house` module for the whole function, so `house.HOUSE_ID` raised UnboundLocalError on every successful heist (win payout was entirely dead) and on the fail-path compensation branch. Rename the local to `house_rec`. - Un-shadowing exposed a latent mint: the pot was floored at 300 but the house was debited only min(total, balance), so a poor house paid out more than it lost. Cap the pot at the balance and debit exactly what is paid (house debit == sum of payouts). No mint, no leak. - Add the missing `_is_jailed` import (do_heist_check referenced it unimported). blackjack (commands/economy_games_commands.py): - Button callbacks had no reentrancy guard; discord.py dispatches each click as its own task, so double-clicking Stand within the dealer-reveal window paid out twice (mint), and double-clicking Double/Split deducted the extra bet twice. Add a synchronous `_busy` guard (matching the existing RpsGame idiom) on all four callbacks plus a `_resolved` idempotency flag on settlement, so a game can only pay out once. bail (core/economy/jail.py, commands/economy_extra_commands.py): - do_bail only checked balance, never jail state; a double-click or a stale BailView from a re-run /jailbreak charged bail twice, destroying coins (bail is a pure sink). Make do_bail a no-op when the user is not jailed, and add a UI reentrancy guard + "already free" message. Tests: 47 passed (4 new regression tests covering heist coin-conservation and bail idempotency). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
91
tests/test_money_safety_fixes.py
Normal file
91
tests/test_money_safety_fixes.py
Normal file
@@ -0,0 +1,91 @@
|
||||
"""Regression tests for the money-safety audit fixes.
|
||||
|
||||
Covers the two pure-logic fixes:
|
||||
- heist payout conserves coins (no minting when the house is poor) and the win
|
||||
path no longer crashes on the shadowed `house` local.
|
||||
- do_bail is idempotent: a jailed user can only be charged once per sentence, so
|
||||
a double-click or a stale BailView cannot destroy coins with a second fine.
|
||||
"""
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from core import economy
|
||||
|
||||
from conftest import run
|
||||
|
||||
UID = 111
|
||||
OTHER = 222
|
||||
HOUSE = 999
|
||||
|
||||
|
||||
def _fixed_now(monkeypatch, dt: datetime):
|
||||
monkeypatch.setattr(economy.store, "_clock", lambda: dt)
|
||||
return dt
|
||||
|
||||
|
||||
class TestHeistConservation:
|
||||
def _setup_house(self, fake_pb, monkeypatch, balance: int):
|
||||
monkeypatch.setattr(economy.house, "HOUSE_ID", HOUSE)
|
||||
monkeypatch.setattr(economy.house, "_house_pb_id", None)
|
||||
run(economy.get_user(HOUSE))
|
||||
fake_pb.record_for(HOUSE)["balance"] = balance
|
||||
|
||||
def test_poor_house_win_does_not_mint(self, fake_pb, monkeypatch):
|
||||
# Poor house is the case the pre-fix code minted from: the desired pot
|
||||
# (floored at 300) exceeded the balance, so payouts outran the debit.
|
||||
self._setup_house(fake_pb, monkeypatch, balance=100)
|
||||
run(economy.get_user(UID))
|
||||
run(economy.get_user(OTHER))
|
||||
|
||||
house_before = fake_pb.record_for(HOUSE)["balance"]
|
||||
res = run(economy.do_heist_resolve([UID, OTHER], True)) # must not raise
|
||||
assert res["ok"] and res["success"]
|
||||
|
||||
house_after = fake_pb.record_for(HOUSE)["balance"]
|
||||
gains = fake_pb.record_for(UID)["balance"] + fake_pb.record_for(OTHER)["balance"]
|
||||
# Coin conservation: the house loses exactly what the players gain.
|
||||
assert house_before - house_after == gains
|
||||
assert house_after >= 0
|
||||
assert res["payout_each"] * 2 == gains
|
||||
|
||||
def test_rich_house_win_pays_out_and_conserves(self, fake_pb, monkeypatch):
|
||||
self._setup_house(fake_pb, monkeypatch, balance=100_000)
|
||||
run(economy.get_user(UID))
|
||||
run(economy.get_user(OTHER))
|
||||
|
||||
house_before = fake_pb.record_for(HOUSE)["balance"]
|
||||
res = run(economy.do_heist_resolve([UID, OTHER], True))
|
||||
assert res["ok"] and res["payout_each"] > 0
|
||||
|
||||
house_after = fake_pb.record_for(HOUSE)["balance"]
|
||||
gains = fake_pb.record_for(UID)["balance"] + fake_pb.record_for(OTHER)["balance"]
|
||||
assert house_before - house_after == gains
|
||||
|
||||
|
||||
class TestBailIdempotency:
|
||||
def _jail(self, fake_pb, now, balance: int):
|
||||
run(economy.get_user(UID))
|
||||
rec = fake_pb.record_for(UID)
|
||||
rec["balance"] = balance
|
||||
rec["jailed_until"] = (now + timedelta(minutes=30)).isoformat()
|
||||
|
||||
def test_second_bail_when_free_is_noop(self, fake_pb, monkeypatch):
|
||||
now = _fixed_now(monkeypatch, datetime(2026, 7, 25, 12, tzinfo=timezone.utc))
|
||||
self._jail(fake_pb, now, balance=1000)
|
||||
|
||||
first = run(economy.do_bail(UID))
|
||||
assert first["ok"]
|
||||
bal_after_first = fake_pb.record_for(UID)["balance"]
|
||||
assert bal_after_first < 1000 # a fine was charged
|
||||
assert fake_pb.record_for(UID)["jailed_until"] is None # freed
|
||||
|
||||
# A double-click / stale view fires do_bail again while already free.
|
||||
second = run(economy.do_bail(UID))
|
||||
assert not second["ok"] and second["reason"] == "not_jailed"
|
||||
assert fake_pb.record_for(UID)["balance"] == bal_after_first # no second charge
|
||||
|
||||
def test_bail_charges_once_for_a_real_sentence(self, fake_pb, monkeypatch):
|
||||
now = _fixed_now(monkeypatch, datetime(2026, 7, 25, 12, tzinfo=timezone.utc))
|
||||
self._jail(fake_pb, now, balance=1000)
|
||||
res = run(economy.do_bail(UID))
|
||||
assert res["ok"] and res["fine"] >= economy.MIN_BAIL
|
||||
Reference in New Issue
Block a user