Ignore:
Timestamp:
09/16/26 23:37:15 (13 days ago)
Author:
Stefan <trsunovstefan@…>
Branches:
main
Children:
8b447ef
Parents:
df05838
Message:

add reserved_quantity and modify the phases, add v_03.png and v_03.xml for P1

File:
1 edited

Legend:

Unmodified
Added
Removed
  • docs/P4-Prototype/UseCase0005Implementation.md

    rdf05838 r9577c79  
    22
    33**Initiating actor:** Trader. **Source file:** `server/trade.go`, function `PlaceOrder(s, "sell")`.
     4
     5## The bug this closes
     6
     7Before this change, `holdings` had `quantity` and `avg_price` only. The sell
     8path checked `held < qty` straight against `quantity`, which cannot tell
     9"owned" apart from "owned, but already committed to another order that has
     10not settled." `holdings.reserved_quantity` fixes that: the crypto being sold
     11is reserved before it is removed from the position, and the check is against
     12`quantity - reserved_quantity`.
    413
    514## Scenario (implemented)
    … …  
    1322   BEGIN;
    1423
     24   -- (a) record the order as 'open' — no trade has happened yet
    1525   INSERT INTO orders
    16        (user_id, market_id, side, type, status, quantity, price, executed_at)
     26       (user_id, market_id, side, type, status, quantity, price)
    1727   VALUES
    18        ($1, $2, 'sell', 'market', 'executed', $3, $4, now())
     28       ($1, $2, 'sell', 'market', 'open', $3, $4)
    1929   RETURNING id;
    2030
    21    SELECT quantity, avg_price FROM holdings
     31   -- (b) lock the holding and check what is actually free to sell
     32   SELECT quantity, reserved_quantity, avg_price FROM holdings
    2233    WHERE user_id = $1 AND crypto_id = $c FOR UPDATE;
    23    -- abort if missing or insufficient
    24 
     34   -- available := quantity - reserved_quantity
     35   -- abort if missing or available < $qty
     36
     37   -- (c) reserve: committed to this order, not yet removed from the position
    2538   UPDATE holdings
    26       SET quantity = quantity - $qty, updated_at = now()
     39      SET reserved_quantity = reserved_quantity + $qty, updated_at = now()
     40    WHERE user_id = $1 AND crypto_id = $c;
     41
     42   -- (d) settle: a market order fills immediately, so release the
     43   --     reservation and remove the asset in the same step
     44   UPDATE holdings
     45      SET quantity = quantity - $qty,
     46          reserved_quantity = reserved_quantity - $qty,
     47          updated_at = now()
    2748    WHERE user_id = $1 AND crypto_id = $c;
    2849
    … …  
    4364       ($2, now(), $price, $qty, 'sell', 'user');
    4465
     66   -- (e) settle the order itself — it has now actually been filled
     67   UPDATE orders SET status = 'executed', executed_at = now() WHERE id = $orderId;
     68
    4569   COMMIT;
    4670   ```
    … …  
    5276## Failure path — insufficient holding
    5377
    54 If the holding does not exist or `quantity < requested`, the `defer tx.Rollback()` in `server/trade.go` reverts every statement above and the user sees:
    55 
    56 ```
    57 Insufficient holding: trying to sell X, hold Y
    58 ```
     78If the holding does not exist, or `quantity - reserved_quantity < requested`, the `defer tx.Rollback()` in `server/trade.go` reverts every statement above — including the `open` order, which was never committed — and the user sees:
     79
     80```
     81Insufficient holding: trying to sell X, available Y (of Z held, W reserved)
     82```
     83
     84## Verified run — the exact scenario from the design review
     85
     86Run 2026-09-16 against PostgreSQL 16 (`bp_database` on `localhost:5433`).
     87Alice's ETH/BTC holdings were seeded, then her BTC holding was set to exactly
     88the scenario that motivated this fix: 2 BTC owned, nothing reserved.
     89
     90```
     91$ psql ... -c "SELECT symbol, quantity, reserved_quantity, avg_price
     92               FROM holdings h JOIN crypto c ON c.id = h.crypto_id
     93               WHERE user_id = '<alice>';"
     94
     95 symbol | quantity | reserved_quantity |  avg_price
     96--------+----------+--------------------+-------------
     97 BTC    |   2.0000 |             0.0000 | 65000.000000
     98 ETH    |   0.5000 |             0.0000 |  3500.000000
     99```
     100
     101**Step 1 — portfolio before the sell** (`[6] View portfolio`):
     102
     103```
     104  Symbol        Quantity      Reserved     Available         Avg buy         Current           Value  Unrealised P/L
     105  ------------------------------------------------------------------------------------------------------------------
     106  BTC             2.0000        0.0000        2.0000    65000.000000    67140.000000     134280.0000      +4280.0000
     107  ETH             0.5000        0.0000        0.5000     3500.000000     3520.000000       1760.0000        +10.0000
     108  ------------------------------------------------------------------------------------------------------------------
     109  TOTAL                                                                                  136040.0000      +4290.0000
     110```
     111
     112**Step 2 — `[5] Place market SELL order` → `BTC` → `0.5`:**
     113
     114```
     115Order executed: sell 0.5000 BTC @ 67140.000000 (notional 33570.0000 USD)
     116```
     117
     118**Step 3 — portfolio after the sell:**
     119
     120```
     121  BTC             1.5000        0.0000        1.5000    65000.000000    67140.000000     100710.0000      +3210.0000
     122```
     123
     124`quantity` dropped from 2.0 to 1.5 and `reserved_quantity` is back to 0.0000
     125— reserve and settle both happened, inside the one commit, exactly as
     126designed.
     127
     128## Verified run — reserve and settle as two distinct, observable steps
     129
     130The CLI settles a market order in the same transaction it reserves in, so
     131`reserved_quantity` is never visibly nonzero *outside* a transaction. Run by
     132hand in one `psql` session (one transaction, so the session sees its own
     133uncommitted writes) to show the intermediate state that step (c) alone would
     134leave, before step (d) runs:
     135
     136```sql
     137BEGIN;
     138
     139-- before: Alice owns 2 BTC, none reserved
     140SELECT quantity, reserved_quantity, quantity - reserved_quantity AS available
     141  FROM holdings WHERE user_id = '<alice>' AND crypto_id = '<btc>';
     142--  quantity | reserved_quantity | available
     143-- ----------+--------------------+-----------
     144--    2.0000 |             0.0000 |    2.0000
     145
     146-- step (c): order placed, 0.5 BTC reserved — no trade has happened yet
     147UPDATE holdings SET reserved_quantity = reserved_quantity + 0.5, updated_at = now()
     148 WHERE user_id = '<alice>' AND crypto_id = '<btc>';
     149
     150SELECT quantity, reserved_quantity, quantity - reserved_quantity AS available
     151  FROM holdings WHERE user_id = '<alice>' AND crypto_id = '<btc>';
     152--  quantity | reserved_quantity | available
     153-- ----------+--------------------+-----------
     154--    2.0000 |             0.5000 |    1.5000
     155
     156-- step (d): market order settles immediately, reservation released
     157UPDATE holdings SET quantity = quantity - 0.5, reserved_quantity = reserved_quantity - 0.5, updated_at = now()
     158 WHERE user_id = '<alice>' AND crypto_id = '<btc>';
     159
     160SELECT quantity, reserved_quantity, quantity - reserved_quantity AS available
     161  FROM holdings WHERE user_id = '<alice>' AND crypto_id = '<btc>';
     162--  quantity | reserved_quantity | available
     163-- ----------+--------------------+-----------
     164--    1.5000 |             0.0000 |    1.5000
     165
     166COMMIT;
     167```
     168
     169This is the row that would stay visible to every other connection for as long
     170as the order stayed `open` — i.e. for as long as it took a matcher to fill
     171it, once limit orders exist.
     172
     173## Verified run — two concurrent sells, which is the bug itself
     174
     175The scenario the design review described: a user should not be able to place
     176two sell orders whose combined quantity exceeds what they actually hold. With
     177Alice's BTC holding at 1.5 BTC (0 reserved), two independent CLI processes
     178were started at the same instant, each selling `1.0 BTC` — together 2.0 BTC,
     179more than she has:
     180
     181```
     182$ ( eduberza-sell-1.0-BTC ) &   # process A
     183$ ( eduberza-sell-1.0-BTC ) &   # process B
     184$ wait
     185
     186=== A ===
     187Insufficient holding: trying to sell 1.0000, available 0.5000 (of 0.5000 held, 0.0000 reserved)
     188=== B ===
     189Order executed: sell 1.0000 BTC @ 67140.000000 (notional 67140.0000 USD)
     190
     191=== final holding ===
     192 quantity | reserved_quantity
     193----------+--------------------
     194   0.5000 |             0.0000
     195```
     196
     197One order settled, one was correctly rejected, and the final `quantity`
     198(0.5) is consistent with exactly one 1.0 BTC sell having happened against the
     1991.5 BTC available — not both, and not neither. This is enforced by the
     200`SELECT ... FOR UPDATE` lock on the holdings row: whichever transaction gets
     201there second blocks until the first commits, then re-reads the now-current
     202`quantity`/`reserved_quantity` before deciding.
     203
     204## Verified — the constraint holds even if application code did not
     205
     206```sql
     207UPDATE holdings SET reserved_quantity = quantity + 1 WHERE user_id = '<alice>' AND crypto_id = '<btc>';
     208
     209ERROR:  new row for relation "holdings" violates check constraint "holdings_check"
     210```
     211
     212`CHECK (reserved_quantity >= 0 AND reserved_quantity <= quantity)` in
     213`schema_creation.sql` makes an inconsistent reservation impossible at the
     214database level, independent of `trade.go`.
Note: See TracChangeset for help on using the changeset viewer.