diff --git a/commands/economy_extra_commands.py b/commands/economy_extra_commands.py index 3f0f4d9..6b8356f 100644 --- a/commands/economy_extra_commands.py +++ b/commands/economy_extra_commands.py @@ -396,12 +396,17 @@ def register_economy_extra_commands( def __init__(self, user_id: int): super().__init__(timeout=60) self.user_id = user_id + self._paying = False @discord.ui.button(label=S.JAILBREAK_UI["bail_btn"], style=discord.ButtonStyle.danger) async def pay_bail(self, interaction: discord.Interaction, _: discord.ui.Button): if interaction.user.id != self.user_id: await interaction.response.send_message(S.ERR["not_your_game"], ephemeral=True) return + if self._paying or self.is_finished(): + await interaction.response.defer() + return + self._paying = True res = await economy.do_bail(self.user_id) self.clear_items() self.stop() @@ -414,6 +419,15 @@ def register_economy_extra_commands( ), color=0xED4245, ) + elif not res["ok"]: + # Already released (stale view or double-click) - no charge applied. + embed = discord.Embed( + title=S.TITLE["jailbreak_bail"], + description=S.JAILBREAK_UI["bail_already_free"].format( + balance=coin(res["balance"]), + ), + color=0x57F287, + ) else: embed = discord.Embed( title=S.TITLE["jailbreak_bail"], diff --git a/commands/economy_games_commands.py b/commands/economy_games_commands.py index 5952f65..e798420 100644 --- a/commands/economy_games_commands.py +++ b/commands/economy_games_commands.py @@ -792,6 +792,11 @@ def register_economy_games_commands( self._doubled_hands: set[int] = set() self._split_aces: bool = False self.message: discord.Message | None = None + # _busy: a callback is mid-flight (prevents overlapping button tasks, + # since discord.py dispatches each click as its own task). + # _resolved: the game has paid out (makes settlement idempotent). + self._busy = False + self._resolved = False self._refresh_buttons() @property @@ -864,6 +869,9 @@ def register_economy_games_commands( return discord.Embed(title=S.TITLE["blackjack"], description=desc, color=0x5865F2) async def _resolve_all(self, interaction: discord.Interaction) -> None: + if self._resolved: + return + self._resolved = True active_games.discard(self.user_id) self.clear_items() self.stop() @@ -949,80 +957,111 @@ def register_economy_games_commands( if interaction.user.id != self.user_id: await interaction.response.send_message(S.ERR["not_your_game"], ephemeral=True) return - await interaction.response.defer() - self._cur_hand.append(self.deck.pop()) - val = _bj_value(self._cur_hand) - if val > 21: - await self.message.edit(embed=self._cur_embed(), view=None) - await asyncio.sleep(_BJ_DEAL_DELAY) - if len(self.hands) > 1: - await self._advance_or_finish(interaction) + if self._busy or self.is_finished(): + await interaction.response.defer() + return + self._busy = True + try: + await interaction.response.defer() + self._cur_hand.append(self.deck.pop()) + val = _bj_value(self._cur_hand) + if val > 21: + await self.message.edit(embed=self._cur_embed(), view=None) + await asyncio.sleep(_BJ_DEAL_DELAY) + if len(self.hands) > 1: + await self._advance_or_finish(interaction) + else: + await self._resolve_all(interaction) + elif val == 21: + await self.message.edit(embed=self._cur_embed(), view=None) + await asyncio.sleep(_BJ_DEAL_DELAY * 0.5) + if len(self.hands) > 1: + await self._advance_or_finish(interaction) + else: + await self._do_dealer_reveal(interaction) else: - await self._resolve_all(interaction) - elif val == 21: - await self.message.edit(embed=self._cur_embed(), view=None) - await asyncio.sleep(_BJ_DEAL_DELAY * 0.5) - if len(self.hands) > 1: - await self._advance_or_finish(interaction) - else: - await self._do_dealer_reveal(interaction) - else: - self._refresh_buttons() - await self.message.edit(embed=self._cur_embed(), view=self) + self._refresh_buttons() + await self.message.edit(embed=self._cur_embed(), view=self) + finally: + self._busy = False async def _stand(self, interaction: discord.Interaction) -> None: if interaction.user.id != self.user_id: await interaction.response.send_message(S.ERR["not_your_game"], ephemeral=True) return - await interaction.response.defer() - if len(self.hands) > 1: - await self._advance_or_finish(interaction) - else: - await self._do_dealer_reveal(interaction) + if self._busy or self.is_finished(): + await interaction.response.defer() + return + self._busy = True + try: + await interaction.response.defer() + if len(self.hands) > 1: + await self._advance_or_finish(interaction) + else: + await self._do_dealer_reveal(interaction) + finally: + self._busy = False async def _double(self, interaction: discord.Interaction) -> None: if interaction.user.id != self.user_id: await interaction.response.send_message(S.ERR["not_your_game"], ephemeral=True) return - res = await economy.do_blackjack_bet(self.user_id, self.bet) - if not res["ok"]: - await interaction.response.send_message( - S.ERR["broke"].format(bal=coin(res.get("balance", 0))), ephemeral=True - ) + if self._busy or self.is_finished(): + await interaction.response.defer() return - await interaction.response.defer() - self._doubled_hands.add(0) - self.bets[0] *= 2 - self._cur_hand.append(self.deck.pop()) - await self.message.edit(embed=self._cur_embed(), view=None) - await asyncio.sleep(_BJ_DEAL_DELAY) - await self._do_dealer_reveal(interaction) + self._busy = True + try: + res = await economy.do_blackjack_bet(self.user_id, self.bet) + if not res["ok"]: + await interaction.response.send_message( + S.ERR["broke"].format(bal=coin(res.get("balance", 0))), ephemeral=True + ) + return + await interaction.response.defer() + self._doubled_hands.add(0) + self.bets[0] *= 2 + self._cur_hand.append(self.deck.pop()) + await self.message.edit(embed=self._cur_embed(), view=None) + await asyncio.sleep(_BJ_DEAL_DELAY) + await self._do_dealer_reveal(interaction) + finally: + self._busy = False async def _split_hand(self, interaction: discord.Interaction) -> None: if interaction.user.id != self.user_id: await interaction.response.send_message(S.ERR["not_your_game"], ephemeral=True) return - res = await economy.do_blackjack_bet(self.user_id, self.bet) - if not res["ok"]: - await interaction.response.send_message( - S.ERR["broke"].format(bal=coin(res.get("balance", 0))), ephemeral=True - ) + if self._busy or self.is_finished(): + await interaction.response.defer() return - await interaction.response.defer() - card1, card2 = self._cur_hand[0], self._cur_hand[1] - self._split_aces = card1[0] == "A" - self.hands = [[card1, self.deck.pop()], [card2, self.deck.pop()]] - self.bets = [self.bet, self.bet] - self.hand_idx = 0 - await self.message.edit(embed=self._cur_embed(), view=None) - await asyncio.sleep(_BJ_DEAL_DELAY) - if self._split_aces: - await self._do_dealer_reveal(interaction) - else: - self._refresh_buttons() - await self.message.edit(embed=self._cur_embed(), view=self) + self._busy = True + try: + res = await economy.do_blackjack_bet(self.user_id, self.bet) + if not res["ok"]: + await interaction.response.send_message( + S.ERR["broke"].format(bal=coin(res.get("balance", 0))), ephemeral=True + ) + return + await interaction.response.defer() + card1, card2 = self._cur_hand[0], self._cur_hand[1] + self._split_aces = card1[0] == "A" + self.hands = [[card1, self.deck.pop()], [card2, self.deck.pop()]] + self.bets = [self.bet, self.bet] + self.hand_idx = 0 + await self.message.edit(embed=self._cur_embed(), view=None) + await asyncio.sleep(_BJ_DEAL_DELAY) + if self._split_aces: + await self._do_dealer_reveal(interaction) + else: + self._refresh_buttons() + await self.message.edit(embed=self._cur_embed(), view=self) + finally: + self._busy = False async def on_timeout(self) -> None: + if self._resolved: + return + self._resolved = True active_games.discard(self.user_id) try: await economy.do_blackjack_payout(self.user_id, 0, sum(self.bets)) diff --git a/core/economy/heist.py b/core/economy/heist.py index 0bf498e..ff0f75e 100644 --- a/core/economy/heist.py +++ b/core/economy/heist.py @@ -7,7 +7,7 @@ import random from .. import pb_client from ..pb_client import DatabaseError from . import house -from .store import HEIST_JAIL, _commit, _now, _txn, _user_lock, get_user +from .store import HEIST_JAIL, _commit, _is_jailed, _now, _txn, _user_lock, get_user from .house import _credit_house, _refund_house_safe, _refund_user_safe @@ -39,19 +39,25 @@ async def do_heist_resolve(user_ids: list[int], success: bool) -> dict: failed_users: list[int] = [] if success and house.HOUSE_ID is not None: + # NB: use a distinct local name - assigning to `house` here would shadow + # the imported module for the whole function and break `house.HOUSE_ID`. try: - house = await get_user(house.HOUSE_ID) + house_rec = await get_user(house.HOUSE_ID) pct = random.uniform(0.20, 0.55) - total = max(300, int(house["balance"] * pct)) - payout_each = total // len(user_ids) - # Atomic decrement (capped at the balance we read) instead of a full - # record commit, so concurrent _credit_house increments aren't lost. - debit = min(total, house["balance"]) + # Never promise more than the house actually holds: the desired pot + # is capped at the current balance, and each share is floored, so the + # amount debited equals the amount paid out (no minting, no leak). + pot = min(max(300, int(house_rec["balance"] * pct)), house_rec["balance"]) + pot = max(0, pot) + payout_each = pot // len(user_ids) + debit = payout_each * len(user_ids) + # Atomic decrement instead of a full record commit, so concurrent + # _credit_house increments aren't lost. if debit > 0: - await pb_client.update_record(house["_pb_id"], {"balance-": debit}) # type: ignore[typeddict-item] + await pb_client.update_record(house_rec["_pb_id"], {"balance-": debit}) # type: ignore[typeddict-item] except DatabaseError: return {"ok": False, "reason": "db_error"} - _txn("HEIST_HOUSE", change=f"-{debit}", house_bal=house["balance"] - debit) + _txn("HEIST_HOUSE", change=f"-{debit}", house_bal=house_rec["balance"] - debit) for uid in user_ids: async with _user_lock(uid): diff --git a/core/economy/jail.py b/core/economy/jail.py index 84ce0c0..0819d7e 100644 --- a/core/economy/jail.py +++ b/core/economy/jail.py @@ -50,6 +50,12 @@ async def do_bail(user_id: int) -> dict: """Charge bail fine after exhausting jailbreak rolls and free the user. Fine = 20-30% of current balance, floored at 350. If balance < 350, stay jailed.""" user = await get_user(user_id) + # Idempotency guard: only an actively-jailed user can be charged bail. The + # first successful call clears jailed_until, so a rapid second click or a + # stale BailView from a re-run /jailbreak becomes a no-op instead of a + # second fine (bail is a pure sink - a double charge destroys coins). + if not _is_jailed(user): + return {"ok": False, "reason": "not_jailed", "balance": user["balance"]} if user["balance"] < MIN_BAIL: return {"ok": False, "reason": "broke", "balance": user["balance"]} pct = random.uniform(0.20, 0.30) diff --git a/strings.py b/strings.py index 9033543..5ef005e 100644 --- a/strings.py +++ b/strings.py @@ -1167,6 +1167,7 @@ JAILBREAK_UI: dict[str, str] = { "bail_btn": "💸 Maksa kautsjon", "bail_broke_desc": "❌ Sul pole piisavalt raha (min {min}).\nJääd vanglasse kuni aja lõpuni! Saldo: {balance}", "bail_paid_desc": "✅ Kautsjon makstud: **{fine}**\nOled vaba! Saldo: {balance}", + "bail_already_free": "✅ Oled juba vaba - kautsjonit ei võetud. Saldo: {balance}", "fail_broke_desc": "{d1} {d2} - ei olnud duubel\n\n❌ Sul pole kautsjoni maksmiseks piisavalt raha (min 350 ⬡).\nJääd vanglasse kuni aja lõpuni! Saldo: {balance}", "fail_bail_desc": "{d1} {d2} - ei olnud duubel\n\nKautsjon makstud: **{fine}**\nSaldo: {balance}", "fail_bail_offer": "{d1} {d2} - ei olnud duubel\n\n💰 **Kautsjon: {min} - {max}** (20-30% saldost)\nSaldo: {bal}\n\nSaad maksta kautsjoni või jääda vanglasse kuni aja lõpuni.", diff --git a/tests/test_money_safety_fixes.py b/tests/test_money_safety_fixes.py new file mode 100644 index 0000000..39ccdfd --- /dev/null +++ b/tests/test_money_safety_fixes.py @@ -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