diff --git a/BUGS.md b/BUGS.md index e4ea343..838d033 100644 --- a/BUGS.md +++ b/BUGS.md @@ -40,18 +40,6 @@ remains the last prerequisite for running unattended. ## Low — documentation and consistency drift -### B-70 — the flowchart's WITHDRAW precondition is not implemented as written - -`flowchart/platform-overview.mmd:39` (node E1) states a withdrawal cannot happen -together with a bet in progress. The code only serializes the *builds* through the -per-user lock (`app/tx/locks.py`): a withdrawal is accepted while a bet is still -unconfirmed, as long as confirmed UTXOs cover it. - -CLAUDE.md declares every node and edge label of the diagrams a behaviour that must -be implemented as described, so one of the two has to move — most likely the -diagram, since the lock already prevents the actual double-spend hazard, but that -is a decision, not a cleanup. - ### B-71 — `.env` points `MASTER_KEY_PATH` at a second copy of the master key CLAUDE.md's deployment section prescribes pointing `MASTER_KEY_PATH` at the diff --git a/CLAUDE.md b/CLAUDE.md index 96fb66d..2d3b364 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ The user communicates in Italian in chat — reply to them in Italian. Everythin ## Project status -All 10 stages of the original build order are code-complete and unit-tested — 349 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below. +All 10 stages of the original build order are code-complete and unit-tested — 351 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below. Verified on mainnet with real money: registration + address derivation, deposit crediting (1-conf), a real 10 PLM bet (broadcast → confirmed → change credited back), and one full round cycle (close → draw on a real block hash → 70/30 payout with sat math checked against the broadcast tx → confirmation → close → next round auto-opened). **Withdrawal and the RBF bump path have never been exercised against a live broadcast** — unit-tested only. @@ -33,7 +33,7 @@ PYTHONPATH=. python scripts/decrypt_master_key.py # ops recovery: decrypt+pr PYTHONPATH=. python scripts/encrypt_master_key.py # ops bootstrap: import an externally-generated xprv (--overwrite to replace) PYTHONPATH=. python scripts/electrum_smoke_test.py # manual check: connect, handshake, subscribe to headers, print the tip -python -m pytest # all 349 tests +python -m pytest # all 351 tests python -m pytest tests/unit/test_hd.py # one file python -m pytest tests/unit/test_hd.py::test_derivation_is_deterministic # one test ``` @@ -166,7 +166,7 @@ Diagrams: [platform-overview.mmd](flowchart/platform-overview.mmd), [round-lifec **WITHDRAW** — the only way out to an external address: PSBT user-address → external + change back to the user, fee deducted from the withdrawn amount, same RBF pattern. A full-balance withdrawal moves `balance - DUST_LIMIT_SATS` so the change output (and with it the ability to fee-bump) always exists — `Withdrawal.amount_requested_sats` vs `amount_sent_sats` is what records the difference (B-62). -PLAY and WITHDRAW share a **per-user lock** (`tx/locks.py`): a bet-build and a withdrawal-build can never be in flight at once, since both spend the same UTXO set. +PLAY and WITHDRAW share a **per-user lock** (`tx/locks.py`): a bet-build and a withdrawal-build can never be in flight at once, since both spend the same UTXO set. That is the *only* mutual exclusion between them (B-70): a withdrawal is accepted while a bet is still unconfirmed, as long as confirmed, unspent UTXOs cover it — the lock plus `select_utxos` skipping anything already marked `spent_txid` is what prevents the two from picking the same input, so freezing the rest of the balance for a block on top of that would restrict the user without protecting anything. **Three separate on-chain confirmations sit between the timer hitting zero and the payout landing** — a common point of confusion: 1. **Last bet's confirmation** — the round doesn't even flip to `"closing"` until every broadcast bet has 1 conf (`_tick`'s `pending_count` check). May already have happened before the deadline. diff --git a/README.md b/README.md index 78486e3..55963c8 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ python -m pytest # all tests python -m pytest tests/unit/test_hd.py # one file ``` -349 unit tests cover HD derivation, PSBT building, the Electrum client, bets, +351 unit tests cover HD derivation, PSBT building, the Electrum client, bets, deposits, withdrawals, the round/draw engine, RBF fee-bumping, admin config, the pending-inclusive balance calculation, and the SSE push channel. No automated integration tests against a live Electrum connection — mainnet diff --git a/flowchart/platform-overview.mmd b/flowchart/platform-overview.mmd index 51d5452..41dbf88 100644 --- a/flowchart/platform-overview.mmd +++ b/flowchart/platform-overview.mmd @@ -36,7 +36,7 @@ flowchart LR subgraph WITHDRAW["FASE 5 - Prelievo"] direction TB - E1["L'utente richiede un prelievo:\nindirizzo esterno + importo\n(non puo' avvenire insieme\na una scommessa in corso)"] --> E2["Si prepara e firma la transazione:\ndal suo indirizzo verso\nl'indirizzo esterno indicato\n(con resto che torna a lui)"] + E1["L'utente richiede un prelievo:\nindirizzo esterno + importo\n(spende solo saldo confermato\ne non ancora impegnato: una scommessa\nin attesa di conferma non lo blocca,\nma le due operazioni non vengono\nmai preparate nello stesso momento)"] --> E2["Si prepara e firma la transazione:\ndal suo indirizzo verso\nl'indirizzo esterno indicato\n(con resto che torna a lui)"] E2 --> E3["Transazione inviata\nalla rete"] E3 --> E4{"Confermata?"} E4 -- "No, troppo tempo" --> E5["Si aumenta la commissione\ne si reinvia"] diff --git a/tests/unit/test_withdrawals.py b/tests/unit/test_withdrawals.py index 6dd59b1..b00d764 100644 --- a/tests/unit/test_withdrawals.py +++ b/tests/unit/test_withdrawals.py @@ -301,3 +301,57 @@ async def test_a_bet_with_a_dust_limit_of_headroom_is_accepted(session_factory): participant = await place_bet(session, client, user) assert participant.status == "broadcast" + + +async def test_withdrawal_is_allowed_while_a_bet_is_still_unconfirmed(session_factory): + """B-70: the flowchart's WITHDRAW node used to state that a withdrawal cannot + happen together with a bet in progress. It can, and should: the hazard is the two + picking the *same* UTXO, which is already excluded twice over — the per-user lock + (app/tx/locks.py) keeps the two builds from ever being in flight at once, and + select_utxos skips anything already marked spent_txid. What is left is untouched, + confirmed money, and freezing it for a block just because a bet is in flight would + be a restriction with no safety behind it. The diagram was corrected to match.""" + user_id = await _make_funded_user(session_factory, 43, 2_000_000_000) + async with session_factory() as session: + # A second confirmed UTXO the bet won't touch (select_utxos is largest-first). + session.add( + UtxoEvent(user_id=user_id, txid="ab" * 32, vout=1, amount_sats=1_500_000_000, confirmed_height=100) + ) + await session.commit() + + bet_client = FakeElectrumClient() + async with session_factory() as session: + user = await session.get(User, user_id) + participant = await place_bet(session, bet_client, user) + assert participant.status == "broadcast" # broadcast, not yet confirmed + + withdraw_client = FakeElectrumClient() + async with session_factory() as session: + user = await session.get(User, user_id) + withdrawal = await request_withdrawal(session, withdraw_client, user, EXTERNAL_ADDRESS, BET_AMOUNT_SATS) + + assert withdrawal.status == "broadcast" + assert withdraw_client.broadcasted + + async with session_factory() as session: + utxos = (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).all() + spenders = {u.amount_sats: u.spent_txid for u in utxos} + # Each transaction took its own input; neither is spending the other's. + assert spenders[2_000_000_000] != spenders[1_500_000_000] + assert all(txid is not None for txid in spenders.values()) + pending_kinds = { + p.kind for p in (await session.scalars(select(PendingTransaction))).all() + } + assert pending_kinds == {"bet", "withdrawal"} + + +def test_the_flowchart_no_longer_claims_bets_and_withdrawals_are_exclusive(): + from pathlib import Path + + diagram = ( + Path(__file__).resolve().parents[2] / "flowchart" / "platform-overview.mmd" + ).read_text(encoding="utf-8") + node = [line for line in diagram.splitlines() if line.strip().startswith("E1[")] + assert len(node) == 1 + assert "non puo' avvenire insieme" not in node[0] + assert "saldo confermato" in node[0]