Changeset 9577c79 for docs/P4-Prototype/UseCase0005Implementation.md
- Timestamp:
- 09/16/26 23:37:15 (13 days ago)
- Branches:
- main
- Children:
- 8b447ef
- Parents:
- df05838
- File:
-
- 1 edited
-
docs/P4-Prototype/UseCase0005Implementation.md (modified) (4 diffs)
Legend:
- Unmodified
- Added
- Removed
-
docs/P4-Prototype/UseCase0005Implementation.md
rdf05838 r9577c79 2 2 3 3 **Initiating actor:** Trader. **Source file:** `server/trade.go`, function `PlaceOrder(s, "sell")`. 4 5 ## The bug this closes 6 7 Before this change, `holdings` had `quantity` and `avg_price` only. The sell 8 path checked `held < qty` straight against `quantity`, which cannot tell 9 "owned" apart from "owned, but already committed to another order that has 10 not settled." `holdings.reserved_quantity` fixes that: the crypto being sold 11 is reserved before it is removed from the position, and the check is against 12 `quantity - reserved_quantity`. 4 13 5 14 ## Scenario (implemented) … … 13 22 BEGIN; 14 23 24 -- (a) record the order as 'open' — no trade has happened yet 15 25 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) 17 27 VALUES 18 ($1, $2, 'sell', 'market', ' executed', $3, $4, now())28 ($1, $2, 'sell', 'market', 'open', $3, $4) 19 29 RETURNING id; 20 30 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 22 33 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 25 38 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() 27 48 WHERE user_id = $1 AND crypto_id = $c; 28 49 … … 43 64 ($2, now(), $price, $qty, 'sell', 'user'); 44 65 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 45 69 COMMIT; 46 70 ``` … … 52 76 ## Failure path — insufficient holding 53 77 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 ``` 78 If 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 ``` 81 Insufficient 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 86 Run 2026-09-16 against PostgreSQL 16 (`bp_database` on `localhost:5433`). 87 Alice's ETH/BTC holdings were seeded, then her BTC holding was set to exactly 88 the 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 ``` 115 Order 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 126 designed. 127 128 ## Verified run — reserve and settle as two distinct, observable steps 129 130 The 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 132 hand in one `psql` session (one transaction, so the session sees its own 133 uncommitted writes) to show the intermediate state that step (c) alone would 134 leave, before step (d) runs: 135 136 ```sql 137 BEGIN; 138 139 -- before: Alice owns 2 BTC, none reserved 140 SELECT 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 147 UPDATE holdings SET reserved_quantity = reserved_quantity + 0.5, updated_at = now() 148 WHERE user_id = '<alice>' AND crypto_id = '<btc>'; 149 150 SELECT 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 157 UPDATE 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 160 SELECT 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 166 COMMIT; 167 ``` 168 169 This is the row that would stay visible to every other connection for as long 170 as the order stayed `open` — i.e. for as long as it took a matcher to fill 171 it, once limit orders exist. 172 173 ## Verified run — two concurrent sells, which is the bug itself 174 175 The scenario the design review described: a user should not be able to place 176 two sell orders whose combined quantity exceeds what they actually hold. With 177 Alice's BTC holding at 1.5 BTC (0 reserved), two independent CLI processes 178 were started at the same instant, each selling `1.0 BTC` — together 2.0 BTC, 179 more 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 === 187 Insufficient holding: trying to sell 1.0000, available 0.5000 (of 0.5000 held, 0.0000 reserved) 188 === B === 189 Order 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 197 One 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 199 1.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 201 there 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 207 UPDATE holdings SET reserved_quantity = quantity + 1 WHERE user_id = '<alice>' AND crypto_id = '<btc>'; 208 209 ERROR: 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 214 database level, independent of `trade.go`.
Note:
See TracChangeset
for help on using the changeset viewer.
