Merge pull request 'Fix money-safety bugs in economy (heist, blackjack, bail)' (#3) from fix/economy-money-safety into master
Reviewed-on: renkar/tipibot#3
This commit is contained in:
@@ -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"],
|
||||
|
||||
@@ -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,6 +957,11 @@ 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
|
||||
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)
|
||||
@@ -969,21 +982,35 @@ def register_economy_games_commands(
|
||||
else:
|
||||
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
|
||||
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
|
||||
if self._busy or self.is_finished():
|
||||
await interaction.response.defer()
|
||||
return
|
||||
self._busy = True
|
||||
try:
|
||||
res = await economy.do_blackjack_bet(self.user_id, self.bet)
|
||||
if not res["ok"]:
|
||||
await interaction.response.send_message(
|
||||
@@ -997,11 +1024,18 @@ def register_economy_games_commands(
|
||||
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
|
||||
if self._busy or self.is_finished():
|
||||
await interaction.response.defer()
|
||||
return
|
||||
self._busy = True
|
||||
try:
|
||||
res = await economy.do_blackjack_bet(self.user_id, self.bet)
|
||||
if not res["ok"]:
|
||||
await interaction.response.send_message(
|
||||
@@ -1021,8 +1055,13 @@ def register_economy_games_commands(
|
||||
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))
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.",
|
||||
|
||||
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