feat(market): FIFA 5% transfer fee policy, host settle_sale capability, isolated staging harness
Core gains the generic settlement (gitlink 31ab4a6); the FIFA-specific parts live here. FEE (openfut-adapter-fifa17/src/fut/economy_policy.rs), beside pack_price and match_reward_total because 5% is a game policy constant and Core must stay game-neutral — Core only validates 0 <= fee <= gross and never computes a rate: TRANSFER_MARKET_FEE_PERCENT = 5 transfer_market_fee(gross) = floor(gross * 5 / 100), i128 intermediate seller_proceeds(gross) = gross - fee Integer only. Floating point is never used for coin settlement: 0.05 is not representable in binary and a f64 round trip can create or destroy a coin at large prices. Widening to i128 makes overflow unreachable for any i64 price, so no price ceiling has to be assumed. ROUNDING IS A CHOICE AND IT IS NOT CONFIRMED. The fee is floored, so the seller keeps the fractional coin, chosen because it makes fee + proceeds == gross hold exactly at every input — the property the accounting invariant rests on. The discriminating case against flooring the seller's 95% instead is a gross of 150: this rule pays 143, the alternative 142. Nothing in the corpus or the client binary settles which the real server did (the client is only ever told the gross; no tax/netPrice/sellerProceeds wire field exists). Pinned at 0/1/19/20/21/39/40/100/ 150/200/1_000/15_000/15_000_000/i64::MAX plus a fee+proceeds==gross sweep. HOST: CoreEconomy gains settle_sale + EconomySale/EconomySaleReceipt, implemented on HttpCoreClient as POST /economy/settle-sale. Request field names were checked against Core's actual SettleSaleRequest/SaleReceipt rather than assumed. Absent club ids are OMITTED from the body (not null), which is what Core's Outside/active-club defaults depend on, so a unit test pins that body shape. handle_market_buy is deliberately untouched: the synthetic buy path has no counterparty, so minting there is correct. HARNESS: scripts/settlement-staging.py, stdlib only, drives a REAL Core over real HTTP on an ephemeral port against a throwaway DB (production 8099/8199/18080 in a hard deny-list checked in three places), seeds the canonical two-party fixture, prints BEFORE/PURCHASE/AFTER with PASS-FAIL lines, cleans up in a finally. 31/31 pass. It found the rejection-precedence bug fixed in Core, and that Core's content preflight aborts startup on an owned card whose CardDefinitionId no pack defines. Gates: Core 194, adapter 217, host 127, harness 31/31, clippy clean, new code fmt-clean. Nothing deployed; no production process, port or database was touched.
This commit is contained in:
funman300
@@ -0,0 +1,352 @@
|
||||
# Transfer-market sold settlement + transaction fee
|
||||
|
||||
How a completed market sale moves coins and ownership. Written while the
|
||||
`forSale` expiry lifecycle test was running, so **nothing here has been deployed
|
||||
or validated against a FIFA client** — see the confidence table.
|
||||
|
||||
Every claim carries one of:
|
||||
|
||||
| Tag | Meaning |
|
||||
|---|---|
|
||||
| **CODE** | Read directly out of the current implementation |
|
||||
| **TESTED** | Pinned by a unit or integration test that fails if it regresses |
|
||||
| **FIFA17-HISTORICAL** | Supported by contemporaneous FIFA 17 knowledge, not by our client binary |
|
||||
| **NEEDS-LIVE** | Requires a real FIFA 17 client to confirm; must not be treated as settled |
|
||||
|
||||
---
|
||||
|
||||
## 1. What the Buy Now path did before this work
|
||||
|
||||
Traced end to end, **CODE**:
|
||||
|
||||
```text
|
||||
POST/PUT /ut/game/fifa17/trade/<id>
|
||||
-> openfut-utas-host/src/lib.rs route classification (EconomyRoute::MarketBuy)
|
||||
-> market.rs::handle_market_buy
|
||||
store.get_listing(id) market_store.rs
|
||||
bid < buy_now -> echo raised bid, NO coin movement, return
|
||||
store.reserve_listing(id) CAS active -> reserved
|
||||
econ.balance() pre-check, 461 if short
|
||||
econ.purchase_item(price, "market-buy:<id>", listing.card_id)
|
||||
-> POST {core}/economy/purchase-item
|
||||
-> openfut-core services/economy.rs::purchase_item
|
||||
BEGIN IMMEDIATE; debit(club, cost); add_item(club, item, card); COMMIT
|
||||
store.complete_sale(id) CAS reserved -> sold
|
||||
-> auctionInfo record with tradeState "closed", bidState "highest",
|
||||
currentBid = price, itemData.itemState "free"
|
||||
```
|
||||
|
||||
The module header at `openfut-utas-host/src/market.rs:12-16` states it plainly:
|
||||
a buy-now *"debits the buyer and MINTS the won card into their club … there is no
|
||||
real counterparty and no seller credit"*.
|
||||
|
||||
### The ten questions, answered from code
|
||||
|
||||
| # | Question | Answer (**CODE**) |
|
||||
|---|---|---|
|
||||
| 1 | Where are buyer coins debited? | `services/economy.rs::debit` inside `purchase_item`, one `BEGIN IMMEDIATE` transaction. Host pre-checks with `econ.balance()` only to distinguish 461 from 503. |
|
||||
| 2 | Does Core know the seller? | **No.** Every `/economy/*` route resolves ONE club from the `X-OpenFUT-Game` active profile (`routes/economy.rs:23-28`); no request shape carries a counterparty. |
|
||||
| 3 | Does the auction persist seller-owned item identity? | **Yes** — `listings.core_item_id` and `wire_item_id` (`market_store.rs:199-203`). The settlement inputs already existed; the buy path just ignored them. |
|
||||
| 4 | Transfer or mint? | **Mint.** `purchase_item` INSERTs a new `owned_cards` row keyed `market-buy:<tradeId>`. The seller's row is untouched, so a sold card existed **twice**. |
|
||||
| 5 | Is the seller credited? | **No.** Nowhere. |
|
||||
| 6 | Any tax calculation? | **No.** Zero occurrences of `*5/100`, `0.95`, `fee`, `tax` in Core, host, adapter *or* the Python oracle. It existed only as prose in `FIFA17_HUB_BEHAVIOR_SPEC.md:884` and `FIFA17_TRANSFER_MARKET_WIRE.md:127-131`, both labelled unimplemented. |
|
||||
| 7 | Auction state sequence? | `active -> reserved -> sold`, host-side (`market_store.rs`). On any Core failure `reserved -> active`. |
|
||||
| 8 | Is there a `reserved` state? | **Yes**, and it is the concurrency guard, not decoration — `reserve_listing` is a `BEGIN IMMEDIATE` compare-and-swap, so of two concurrent buyers exactly one proceeds to a debit. |
|
||||
| 9 | When does the item leave the seller's `/tradePile`? | When the listing stops being `active`: the pile projection emits only real active auctions (the Fix A invariant). `sold`/`cancelled` rows vanish from the pile the moment the CAS lands. |
|
||||
| 10 | What makes a retry safe? | The same CAS. A second Buy Now finds the listing non-`active`, so `reserve_listing` returns `false` and the handler returns an empty `auctionInfo` — the client reads "auction gone", and no economy call is made. |
|
||||
|
||||
### Deficiencies
|
||||
|
||||
1. **Ownership duplication.** A sold card ends up owned by both parties. Structural, not a race.
|
||||
2. **No seller credit** — selling was economically pointless.
|
||||
3. **No fee**, so no sink; every synthetic buy was pure coin destruction and every sale would have been pure creation.
|
||||
4. **Core could not express a counterparty at all** — the single-club resolver had no seller/buyer distinction to make.
|
||||
5. `UPDATE owned_cards SET club_id` did not exist anywhere in Core: there was **no ownership-transfer primitive** to call.
|
||||
|
||||
---
|
||||
|
||||
## 2. The settlement transaction
|
||||
|
||||
`openfut-core/src/services/economy.rs`, alongside the existing tx-scoped
|
||||
primitives so it reuses the module's proven atomicity (`pool.acquire()` +
|
||||
`BEGIN IMMEDIATE` + `finish()`), **CODE**:
|
||||
|
||||
```rust
|
||||
pub enum SaleBuyer<'a> { Club(&'a str), Outside }
|
||||
pub struct SaleTerms { pub gross: i64, pub fee: i64 }
|
||||
|
||||
pub async fn settle_sale(
|
||||
pool: &Pool, item_id: &str, seller_club_id: &str,
|
||||
buyer: SaleBuyer<'_>, terms: SaleTerms,
|
||||
) -> AppResult<SaleReceipt>
|
||||
```
|
||||
|
||||
One transaction: debit buyer `gross` → evict the item from every squad →
|
||||
transfer the existing row → credit seller `gross - fee`. The fee is simply never
|
||||
credited anywhere, which is what destroys it.
|
||||
|
||||
Exposed as `POST /economy/settle-sale`. It is the **only** economy route that
|
||||
names clubs explicitly, because a sale has two sides and the active-profile
|
||||
resolver can only describe one. Both are optional: an omitted `seller_club_id`
|
||||
means the active club (the production shape — the player listed the item), and an
|
||||
omitted `buyer_club_id` means `Outside`, *not* the active club, so the route can
|
||||
never accidentally settle a club against itself.
|
||||
|
||||
### Ownership transfer, not duplication
|
||||
|
||||
```sql
|
||||
UPDATE owned_cards SET club_id = ? WHERE id = ? AND club_id = ?
|
||||
```
|
||||
|
||||
No INSERT and no DELETE on the two-party path, so the instance id, its
|
||||
`chemistry_style`/`position_override`/`training_bonus` and its `acquired_at` all
|
||||
survive and the inventory row count cannot change. Duplication is ruled out
|
||||
structurally rather than by an assertion. `acquired_at` is deliberately preserved:
|
||||
this is the same instance under new ownership, not a new acquisition.
|
||||
|
||||
**TESTED** — `settle_sale_transfers_ownership_and_splits_coins` asserts both that
|
||||
the item's owner changed and that `SELECT COUNT(*) FROM owned_cards` is still 1.
|
||||
|
||||
### The seller is pinned, not inferred — and this was a real bug
|
||||
|
||||
The first implementation derived the seller from *current* ownership, which reads
|
||||
naturally ("the seller is whoever owns it") and is wrong. The concurrency test
|
||||
caught it immediately: **both** racing buyers succeeded, because after the first
|
||||
sale the item was owned by buyer B, so the second call read B as the seller and
|
||||
cheerfully chain-sold it B → C. Core has no listing concept, so it had no way to
|
||||
notice that this was a replay of one listing rather than a new sale.
|
||||
|
||||
The fix is that `seller_club_id` is the club the caller **believes** owns the item,
|
||||
and every ownership statement is predicated on it. That turns the `UPDATE` into an
|
||||
ownership compare-and-swap which is authoritative about "this sale already
|
||||
happened", independently of any caller-side listing state. Safety is not weakened:
|
||||
a caller still cannot credit a club that never owned the item, because the credit
|
||||
only follows a matched CAS.
|
||||
|
||||
**TESTED** — `two_buyers_racing_one_item_settle_once` (exactly one winner, loser's
|
||||
balance untouched, one instance, seller paid once) and
|
||||
`settling_the_same_sale_twice_pays_once`.
|
||||
|
||||
### Squad eviction is mandatory
|
||||
|
||||
`squad_players.owned_card_id` is a foreign key onto `owned_cards(id)` and the pool
|
||||
enables `foreign_keys`. Two consequences, both **CODE**:
|
||||
|
||||
* an `Outside` sale of a squadded card would fail outright on the FK;
|
||||
* a two-party transfer preserves the row id, so a surviving `squad_players` row
|
||||
would leave the **previous** owner fielding a card they no longer own.
|
||||
|
||||
So both paths evict first, and the receipt reports `squad_slots_freed`.
|
||||
**TESTED** — `sale_evicts_the_item_from_the_sellers_squad`.
|
||||
|
||||
> Noticed while reading, NOT fixed here (out of scope, no behaviour changed):
|
||||
> `routes/cards.rs:187-192` (quick sell) and `services/market.rs` both DELETE an
|
||||
> owned card without clearing `squad_players` and without a transaction, so
|
||||
> quick-selling a squadded card should fail on the same FK. Worth a follow-up.
|
||||
|
||||
---
|
||||
|
||||
## 3. The fee arithmetic
|
||||
|
||||
Lives in `openfut-adapter-fifa17/src/fut/economy_policy.rs`, beside `pack_price`
|
||||
and `match_reward_total`, because 5% is a **FIFA policy constant** and Core must
|
||||
stay game-neutral. Core validates `0 <= fee <= gross` and never computes a rate.
|
||||
|
||||
```rust
|
||||
pub const TRANSFER_MARKET_FEE_PERCENT: i64 = 5;
|
||||
pub fn transfer_market_fee(gross: i64) -> i64 { /* floor, i128 intermediate */ }
|
||||
pub fn seller_proceeds(gross: i64) -> i64 { gross.max(0) - transfer_market_fee(gross) }
|
||||
```
|
||||
|
||||
Integer only. Floating point is never used for coin settlement: `0.05` is not
|
||||
representable in binary, and at large prices a `f64` round trip can create or
|
||||
destroy a coin. The multiply widens to `i128`, so overflow is unreachable for any
|
||||
`i64` price and no price ceiling has to be assumed.
|
||||
|
||||
**Rounding is a CHOICE, and it is NEEDS-LIVE.** The fee is floored, so the seller
|
||||
keeps the fractional coin. That was chosen because it makes
|
||||
|
||||
```text
|
||||
fee + proceeds == gross
|
||||
```
|
||||
|
||||
hold exactly at every input, which is what the accounting invariant rests on. The
|
||||
discriminating case against the alternative (flooring the seller's 95% instead) is
|
||||
a gross of **150**: this rule pays **143**, the alternative pays **142**. Nothing
|
||||
in the corpus or the client binary settles which the real server did — the client
|
||||
is only ever told the gross, and no `tax`/`netPrice`/`sellerProceeds` wire field
|
||||
exists. **A future live test should list something at 150 coins and read the
|
||||
credited amount.**
|
||||
|
||||
**TESTED** at `0, 1, 19, 20, 21, 39, 40, 100, 150, 200, 1_000, 15_000,
|
||||
15_000_000, i64::MAX`, plus `fee + proceeds == gross` swept over 0..2000 and the
|
||||
extremes, plus negative-gross rejection.
|
||||
|
||||
| gross | fee | proceeds | why it is in the table |
|
||||
|---|---|---|---|
|
||||
| 19 | 0 | 19 | last fee-free price |
|
||||
| 20 | 1 | 19 | first price that pays |
|
||||
| 150 | 7 | 143 | **the NEEDS-LIVE discriminator** (alternative: 8 / 142) |
|
||||
| 15,000 | 750 | 14,250 | canonical fixture |
|
||||
| i64::MAX | MAX/20 | remainder | proves no overflow |
|
||||
|
||||
---
|
||||
|
||||
## 4. Accounting invariant
|
||||
|
||||
For `SaleBuyer::Club`, **TESTED** by `sale_conserves_coins_minus_the_fee`:
|
||||
|
||||
```text
|
||||
buyer_debit == seller_credit + fee
|
||||
total modelled coins shrink by EXACTLY the fee
|
||||
21,000 -> 20,250 (difference 750)
|
||||
```
|
||||
|
||||
For `SaleBuyer::Outside` the identity deliberately differs and the doc comment
|
||||
says so: the counterparty owns no `clubs` row, so nothing is debited and the
|
||||
seller's proceeds **enter** the economy from outside. **TESTED** by
|
||||
`settle_sale_to_outside_retires_the_item_and_pays_net`, which asserts total coins
|
||||
*rise* by the proceeds. Conflating the two would look like a conservation bug.
|
||||
|
||||
---
|
||||
|
||||
## 5. Rejected sales
|
||||
|
||||
All **TESTED** by `invalid_sales_change_nothing`, which re-seeds a fresh fixture
|
||||
per case and asserts both balances and the item's owner afterwards:
|
||||
|
||||
| Case | Outcome |
|
||||
|---|---|
|
||||
| buyer cannot afford `gross` | rejected, nothing moves |
|
||||
| buyer == seller | rejected at the Core domain boundary — not a market path, and it would otherwise debit and credit one club while destroying the fee |
|
||||
| item does not exist | rejected |
|
||||
| item not owned by the named seller (**includes every replay**) | rejected |
|
||||
| buyer club does not exist | rejected |
|
||||
| `fee > gross` | rejected |
|
||||
| negative fee / negative gross | rejected |
|
||||
| ownership changed mid-flight | rejected by the CAS |
|
||||
|
||||
A zero-price sale is **legal** (a free transfer, no fee) — **TESTED** by
|
||||
`zero_price_sale_is_a_free_transfer`.
|
||||
|
||||
Auction-level states (`expired`, `cancelled`, already-`sold`) are **not** Core's
|
||||
business: Core has no listing concept, and the host's `active -> reserved` CAS
|
||||
already refuses those. Duplicating that check in Core would create a second
|
||||
authority on auction lifecycle.
|
||||
|
||||
---
|
||||
|
||||
## 6. Atomicity
|
||||
|
||||
`services::economy` is the **only** module in Core that is genuinely atomic; it
|
||||
uses `pool.acquire()` + raw `BEGIN IMMEDIATE` + `finish()` deliberately, because a
|
||||
DEFERRED `pool.begin()` upgrades to a write at first write, where SQLite returns
|
||||
`SQLITE_BUSY` immediately instead of honouring `busy_timeout`
|
||||
(`services/economy.rs:218-223`). Settlement is built inside that module for
|
||||
exactly this reason, so it inherits the guarantee rather than re-deriving it.
|
||||
|
||||
Handlers receive a pooled clone, never `&mut SqliteConnection`, so the transaction
|
||||
boundary is inside the service function. **CODE.** No externally observable state
|
||||
exists where the buyer is debited without the item moving, or the seller credited
|
||||
without the buyer debited — every step shares one connection inside one
|
||||
transaction, and `finish()` rolls back the whole thing on any error.
|
||||
|
||||
### Rejection precedence, and a test that passed for the wrong reason
|
||||
|
||||
Inside one transaction the order of the steps cannot change the final state — any
|
||||
error rolls everything back. It does change **which reason** a rejection carries,
|
||||
and that turned out to matter.
|
||||
|
||||
The first implementation debited the buyer before touching ownership, on the
|
||||
reasonable-sounding grounds that an unaffordable sale should fail before ownership
|
||||
moves. The isolated staging harness then showed that a replayed settlement was
|
||||
refused with `insufficient balance: have 5000, need 15000` — because the buyer had
|
||||
already spent the coins on the sale that succeeded. The affordability guard fired
|
||||
first, so **the ownership CAS was never consulted**, and the unit test named
|
||||
`settling_the_same_sale_twice_pays_once` was passing without ever exercising the
|
||||
replay guard it claimed to test.
|
||||
|
||||
Two fixes, both **TESTED**:
|
||||
|
||||
* ownership is now judged before affordability, so a replay is reported as
|
||||
`item not owned by club` (404) — the true cause;
|
||||
* the replay test now asserts the error is specifically `AppError::NotFound`, and
|
||||
adds a deliberately CHEAP replay (gross 100 out of the buyer's remaining 5,000)
|
||||
that only ownership can possibly refuse.
|
||||
|
||||
The lesson generalises: a guard that is merely *shadowed* by an earlier guard is
|
||||
untested, and a green assertion on "the second call failed" says nothing about
|
||||
which mechanism failed it.
|
||||
|
||||
---
|
||||
|
||||
## 6b. Isolated staging harness
|
||||
|
||||
`scripts/settlement-staging.py` — stdlib-only, exercises the settlement against a
|
||||
**real Core over real HTTP** with zero production contact. It picks an ephemeral
|
||||
loopback port (production 8099/8199/18080 sit in a hard deny-list checked in three
|
||||
places), runs Core once to apply its own migrations, seeds the canonical two-party
|
||||
fixture by direct SQL, relaunches Core, then prints BEFORE / PURCHASE / AFTER with
|
||||
PASS-FAIL lines and cleans up in a `finally`.
|
||||
|
||||
Result: **31/31 checks pass**, exit 0.
|
||||
|
||||
```text
|
||||
BEFORE seller 1,000 buyer 20,000 owner club-seller rows 1 total 21,000
|
||||
POST /economy/settle-sale {item_id, gross 15000, fee 750, seller, buyer} -> 200
|
||||
AFTER seller 15,250 buyer 5,000 owner club-buyer rows 1 total 20,250
|
||||
fee destroyed 750 duplicates 1 coins destroyed 750
|
||||
RETRY (identical) -> 404 item not owned by club: item-x nothing moved
|
||||
REPLAY GUARD (gross 100) -> 404 item not owned by club: item-x nothing moved
|
||||
```
|
||||
|
||||
Two things it found that the unit tests had not:
|
||||
|
||||
1. the rejection-precedence bug above;
|
||||
2. Core's **content preflight** aborts startup when an owned card references a
|
||||
`CardDefinitionId` that no loaded pack defines (`content preflight failed: 1
|
||||
owned card(s) reference CardDefinitionId(s) not loaded (e.g. ["def-x"])`), so
|
||||
the harness writes a one-entry content pack for its synthetic card rather than
|
||||
quietly substituting a stock card id.
|
||||
|
||||
---
|
||||
|
||||
## 7. What is NOT done, and why
|
||||
|
||||
* **No production wire change, no deployment.** The host gained the *capability*
|
||||
(`CoreEconomy::settle_sale`) but `handle_market_buy` is untouched: the synthetic
|
||||
buy path (`core_item_id == None`) has no counterparty, so minting there is
|
||||
correct and unchanged.
|
||||
* **Nothing decides that a player's listing has sold.** That trigger is the real
|
||||
remaining feature, and it needs the seller-facing sold wire representation,
|
||||
which is deliberately deferred — `tradeState`/`itemState` for a sold row are
|
||||
**NEEDS-LIVE** and must not be guessed. `docs/FIFA17_TRANSFER_MARKET_WIRE.md`
|
||||
records that no `sold` token exists in FIFA 17's `tradeState` table
|
||||
(`active=1 inactive=2 expired=3 closed=4`).
|
||||
* **`/tradePile/counts` semantics unchanged.** Its `sold` field stays as-is; the
|
||||
Core facts a correct `sold` count would need now exist, but the route output
|
||||
must not change without client evidence.
|
||||
* **The 5% rate itself is FIFA17-HISTORICAL**, and the rounding rule is
|
||||
**NEEDS-LIVE** (the 150-coin discriminator above).
|
||||
|
||||
## 8. Confidence summary
|
||||
|
||||
| Claim | Status |
|
||||
|---|---|
|
||||
| The old buy path minted a duplicate and never credited a seller | **CODE** |
|
||||
| No fee arithmetic existed anywhere in the project | **CODE** |
|
||||
| Ownership now transfers as one row, with no duplicate possible | **TESTED** |
|
||||
| Buyer debited, seller credited net, fee destroyed, in one transaction | **TESTED** |
|
||||
| `buyer_debit == seller_credit + fee` | **TESTED** |
|
||||
| Canonical 15,000 fixture: 20,000→5,000 / 1,000→15,250 / fee 750 | **TESTED** |
|
||||
| A replayed settlement pays once | **TESTED** |
|
||||
| Two racing buyers settle exactly once | **TESTED** |
|
||||
| A sold card leaves the seller's lineup | **TESTED** |
|
||||
| Invalid sales leave the economy unchanged | **TESTED** |
|
||||
| The fee rate is 5% of gross | **FIFA17-HISTORICAL** |
|
||||
| The fee is floored (150 → 143, not 142) | **NEEDS-LIVE** |
|
||||
| The seller-facing sold wire state | **NEEDS-LIVE** — not implemented, not guessed |
|
||||
| The client renders a settled sale correctly | **NEEDS-LIVE** — never exercised |
|
||||
|
||||
No Core test here establishes anything about the FIFA 17 wire contract. They
|
||||
establish the domain transaction only.
|
||||
Reference in New Issue
Block a user