fix(market): /trade/status must resolve the unlisted ids /tradePile advertises
Explains and fixes the Phase C partial failure WITHOUT changing a single wire field. The operator saw a difference between the one-item probe (Time Remaining "-") and the generalized rows (Time Remaining "Expired"). Cause: route coverage, not encoding. /tradePile advertised the unlisted tradeIds while ISVIEWTRADE (GET .../trade/status) resolved ids from the market store only -- and an unlisted pile member has no listing row, so the poll returned an empty auctionInfo. Observed live as `route=market-status requested=1 returned=0` repeating for the row the operator had selected, while that same id was present in /tradePile. The client polls status for the row it displays and degrades it when the answer is empty, which is also why no actions were offered. The probe showed "-" only because the client had not yet polled that id (logs of the time show only tradeIds=1000000097). So expires, tradeState, itemData.itemState and pile were all innocent. Nothing was guessed and no field changed: both routes now share one pile enumeration (Server::resolve_trade_pile), so an id advertised by /tradePile always resolves on /trade/status. The corpus predicted exactly this -- tradeId must resolve across /transfermarket, /tradePile, /watchList AND /trade/status; we had stability but not coverage. Same defect class as the original empty-trade/status bug. Status still answers only the ids actually asked about, and a real auction always wins over an inactive row for the same tradeId. Regression test covers all four cases. Records the downgraded conclusion: "inactive" is a CONFIRMED section/lifecycle discriminator; whether the full actionable contract is now complete is the operator's next test. itemState/pile recovery was queued on the assumption the encoding was incomplete -- neither was touched, and both remain the next candidates if actions are still absent. 342 tests pass, 0 failed, clippy clean. Verified live: the six inactive ids went from returned=0 to returned=6.
This commit is contained in:
@@ -531,3 +531,60 @@ So `count` currently tracks AUCTION entries, not total Transfer List membership.
|
||||
not change this until the full state set (empty / unlisted / active / expired / sold /
|
||||
mixed) has been observed — it remains an open question whether FIFA 17 expects
|
||||
`count` to include non-active pile members.
|
||||
|
||||
---
|
||||
|
||||
## Q2 status downgrade + the "Expired" differential EXPLAINED (2026-08-17)
|
||||
|
||||
Phase C was a PARTIAL pass. Corrected conclusions:
|
||||
|
||||
```text
|
||||
tradeState "inactive" = CONFIRMED section/lifecycle discriminator
|
||||
(rows persist across a fresh session and land under
|
||||
TRANSFER LIST, not LISTED ITEMS)
|
||||
|
||||
complete ACTIONABLE unlisted representation = was UNKNOWN; see below
|
||||
```
|
||||
|
||||
### The visual differential, and its cause
|
||||
|
||||
| | one-item probe | generalized rows |
|
||||
|---|---|---|
|
||||
| Start Price | 0 | 0 |
|
||||
| Buy Now / Current Bid | `-` | `-` |
|
||||
| Time Remaining | `-` | **`Expired`** |
|
||||
|
||||
Cause: **route coverage, not a field.** `/tradePile` advertised the unlisted
|
||||
tradeIds, but `GET …/trade/status` (ISVIEWTRADE) answered them from the market store
|
||||
ONLY — and an unlisted pile member has no listing row, so the poll returned an empty
|
||||
`auctionInfo`. Observed live as `route=market-status requested=1 returned=0` repeating
|
||||
for the row the operator had selected (`tradeIds=1000000122`), while the same id was
|
||||
present in `/tradePile`.
|
||||
|
||||
The client polls `/trade/status` for the row it is displaying, and an empty answer
|
||||
degrades it: Time Remaining renders `Expired` and no actions are offered. The earlier
|
||||
probe showed `-` simply because the client had not yet polled that id — the logs at
|
||||
the time show only `tradeIds=1000000097`. So `expires`, `tradeState`,
|
||||
`itemData.itemState` and `pile` were all innocent, and NO field was changed.
|
||||
|
||||
This is the same defect class as the original `trade/status` bug: a route the client
|
||||
polls being answered with an empty set. The corpus predicted it — `tradeId` must
|
||||
resolve across `/transfermarket`, `/tradePile`, `/watchList` AND `/trade/status`. We
|
||||
had stability but not coverage.
|
||||
|
||||
Fix: both routes now share ONE pile enumeration (`Server::resolve_trade_pile`), so an
|
||||
id advertised by `/tradePile` always resolves on `/trade/status`. Verified live: the
|
||||
six inactive ids went from `returned=0` to `returned=6`. `/trade/status` still answers
|
||||
only the ids actually asked about, and a real auction always wins over an inactive row
|
||||
for the same id.
|
||||
|
||||
Regression test: `trade_status_resolves_the_unlisted_ids_tradepile_advertises`.
|
||||
|
||||
### Investigations NOT needed as a result
|
||||
|
||||
`itemData.itemState` and numeric `itemData.pile` were queued for PE recovery on the
|
||||
assumption the encoding was incomplete. The cause was route coverage, so neither was
|
||||
touched and neither is implicated. If the actions still do not appear, those remain
|
||||
the next candidates — and `itemState`'s vocabulary should be dumped statically with
|
||||
`fifa17-recon/tools/vocab_dump.py` (already validated against the tradeState table)
|
||||
rather than guessed.
|
||||
|
||||
@@ -2208,6 +2208,39 @@ impl Server {
|
||||
self
|
||||
}
|
||||
|
||||
/// Resolve every Core instance in the `trade` pile to a shaped unlisted
|
||||
/// candidate, for the routes that must agree about those ids.
|
||||
///
|
||||
/// Three phases in this order for a reason: the pile read is async, the
|
||||
/// identity/Core resolvers are NOT `Send` so they cannot cross an await, and the
|
||||
/// caller's response build is async again.
|
||||
///
|
||||
/// Both `/tradePile` and `/trade/status` use this. They MUST: the client polls
|
||||
/// `/trade/status` for the row it is showing, and answering there with an empty
|
||||
/// body while `/tradePile` advertises the id makes the client degrade the row to
|
||||
/// "Expired" with no actions. A tradeId has to resolve on every route that can be
|
||||
/// asked about it.
|
||||
fn resolve_trade_pile(&self, svc: &EconomyServices) -> Vec<crate::market::UnlistedCandidate> {
|
||||
let piles = svc.piles.clone();
|
||||
let trade_ids = svc
|
||||
.bridge
|
||||
.block_on(async move { piles.list_by_pile("trade").await })
|
||||
.unwrap_or_default();
|
||||
if trade_ids.is_empty() {
|
||||
return Vec::new();
|
||||
}
|
||||
let lookup = crate::economy_store::CoreItemLookup {
|
||||
core: self.core.as_ref(),
|
||||
};
|
||||
crate::market::resolve_unlisted_pile(
|
||||
&trade_ids,
|
||||
|core_id| self.resolver.wire_for_owned_id(core_id),
|
||||
self.resolver.as_ref(),
|
||||
&lookup,
|
||||
self.entities.as_ref(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Dispatch a FIFA17 economy route to its Rust handler, or `None` if the path
|
||||
/// is not an economy route (or no economy services are wired). This is the
|
||||
/// handler-wiring entry point exercised by the integration harness; it is
|
||||
@@ -2362,38 +2395,9 @@ impl Server {
|
||||
})
|
||||
}
|
||||
EconomyRoute::MarketQuery => {
|
||||
// Unlisted trade-pile members are exposed as `tradeState: "inactive"`
|
||||
// rows (LIVE-CONFIRMED; see docs/FIFA17_TRANSFER_MARKET_WIRE.md).
|
||||
//
|
||||
// Three steps, in this order for a reason: the pile read is async, the
|
||||
// identity/Core resolvers are NOT `Send` so they cannot cross an await,
|
||||
// and the response build is async again.
|
||||
let (bridge, market, econ, piles) = (
|
||||
svc.bridge.clone(),
|
||||
svc.market.clone(),
|
||||
svc.econ.clone(),
|
||||
svc.piles.clone(),
|
||||
);
|
||||
let trade_ids = {
|
||||
let p = piles.clone();
|
||||
bridge
|
||||
.block_on(async move { p.list_by_pile("trade").await })
|
||||
.unwrap_or_default()
|
||||
};
|
||||
let unlisted = if trade_ids.is_empty() {
|
||||
Vec::new()
|
||||
} else {
|
||||
let lookup = CoreItemLookup {
|
||||
core: self.core.as_ref(),
|
||||
};
|
||||
crate::market::resolve_unlisted_pile(
|
||||
&trade_ids,
|
||||
|core_id| self.resolver.wire_for_owned_id(core_id),
|
||||
self.resolver.as_ref(),
|
||||
&lookup,
|
||||
self.entities.as_ref(),
|
||||
)
|
||||
};
|
||||
let (bridge, market, econ) =
|
||||
(svc.bridge.clone(), svc.market.clone(), svc.econ.clone());
|
||||
let unlisted = self.resolve_trade_pile(svc);
|
||||
bridge.block_on(async move {
|
||||
crate::market::handle_market_query(
|
||||
"active",
|
||||
@@ -2414,11 +2418,15 @@ impl Server {
|
||||
(svc.bridge.clone(), svc.market.clone(), svc.econ.clone());
|
||||
// The raw query carries `tradeIds`; `path` is already stripped.
|
||||
let q = target.split_once('?').map(|(_, q)| q.to_string());
|
||||
// ISViewTrade is asked about the same ids /tradePile advertises, so it
|
||||
// needs the same pile enumeration or an unlisted row degrades.
|
||||
let unlisted = self.resolve_trade_pile(svc);
|
||||
bridge.block_on(async move {
|
||||
crate::market::handle_market_status(
|
||||
q.as_deref(),
|
||||
econ.as_ref(),
|
||||
market.as_ref(),
|
||||
&unlisted,
|
||||
)
|
||||
.await
|
||||
})
|
||||
|
||||
@@ -587,6 +587,7 @@ pub async fn handle_market_status(
|
||||
query: Option<&str>,
|
||||
econ: &dyn CoreEconomy,
|
||||
store: &MarketStore,
|
||||
unlisted: &[UnlistedCandidate],
|
||||
) -> WireResponse {
|
||||
let ids = trade_ids_from_query(query);
|
||||
let listings = if ids.is_empty() {
|
||||
@@ -603,10 +604,33 @@ pub async fn handle_market_status(
|
||||
}
|
||||
found
|
||||
};
|
||||
let auctions: Vec<Value> = listings
|
||||
let mut auctions: Vec<Value> = listings
|
||||
.iter()
|
||||
.map(|l| auction_record_as(l, "listFS"))
|
||||
.collect();
|
||||
|
||||
// ISViewTrade MUST resolve the SAME tradeIds `/tradePile` advertises. The client
|
||||
// polls the row it is displaying and, for an unlisted pile member, there is no
|
||||
// listing row to find — so answering from the market store alone returned an empty
|
||||
// body and the client DEGRADED the row (Time Remaining rendered "Expired", and no
|
||||
// actions were offered). Observed live: `requested=1 returned=0` on repeat for the
|
||||
// selected row. A tradeId must resolve on every route that can be asked about it.
|
||||
let blocked = store.blocking_core_items().await.unwrap_or_default();
|
||||
let wanted: Option<Vec<i64>> = if ids.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(ids.iter().filter_map(|s| s.parse::<i64>().ok()).collect())
|
||||
};
|
||||
for c in unlisted {
|
||||
if blocked.contains(&c.core_id) {
|
||||
continue;
|
||||
}
|
||||
let trade_id = TRADE_ID_BASE + c.item_id;
|
||||
let asked = wanted.as_ref().is_none_or(|w| w.contains(&trade_id));
|
||||
if asked && !auctions.iter().any(|a| a["tradeId"] == json!(trade_id)) {
|
||||
auctions.push(unlisted_record(c));
|
||||
}
|
||||
}
|
||||
eprintln!(
|
||||
"utas-host owner=RUST route=market-status requested={} returned={} query={}",
|
||||
ids.len(),
|
||||
@@ -1429,7 +1453,7 @@ mod tests {
|
||||
seed_listing(&store, "900000031", 2500).await;
|
||||
|
||||
// No filter: answer with the player's own active pile.
|
||||
let all = parse(&handle_market_status(None, &econ, &store).await);
|
||||
let all = parse(&handle_market_status(None, &econ, &store, &[]).await);
|
||||
assert_eq!(
|
||||
all["auctionInfo"].as_array().unwrap().len(),
|
||||
1,
|
||||
@@ -1443,14 +1467,14 @@ mod tests {
|
||||
|
||||
// Explicit tradeIds filter returns exactly the requested auction.
|
||||
let one = parse(
|
||||
&handle_market_status(Some("tradeIds=900000031"), &econ, &store).await,
|
||||
&handle_market_status(Some("tradeIds=900000031"), &econ, &store, &[]).await,
|
||||
);
|
||||
assert_eq!(one["auctionInfo"].as_array().unwrap().len(), 1);
|
||||
assert_eq!(one["auctionInfo"][0]["tradeId"], 900_000_031i64);
|
||||
|
||||
// An unknown id is absent, not an error: the poll must never fail closed.
|
||||
let miss =
|
||||
parse(&handle_market_status(Some("tradeIds=900000099"), &econ, &store).await);
|
||||
parse(&handle_market_status(Some("tradeIds=900000099"), &econ, &store, &[]).await);
|
||||
assert_eq!(miss["auctionInfo"].as_array().unwrap().len(), 0);
|
||||
|
||||
// Garbage is skipped rather than poisoning the whole poll.
|
||||
@@ -1460,6 +1484,75 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn trade_status_resolves_the_unlisted_ids_tradepile_advertises() {
|
||||
// A tradeId must resolve on EVERY route that can be asked about it. The client
|
||||
// polls /trade/status for the row it is displaying; when /tradePile advertised
|
||||
// an unlisted id but /trade/status answered with an empty body, the client
|
||||
// DEGRADED the row — Time Remaining rendered "Expired" and no actions were
|
||||
// offered. Observed live as `requested=1 returned=0` on repeat for the selected
|
||||
// row, while the same id was present in /tradePile.
|
||||
let (store, _d) = store_at("statusunlisted").await;
|
||||
let econ = CountingEconomy::with_balance(10_000);
|
||||
let c = UnlistedCandidate {
|
||||
item_id: 100_000_122,
|
||||
core_id: "core-pile".into(),
|
||||
item_json: Some(r#"{"rating":90,"preferredPosition":"CM"}"#.into()),
|
||||
};
|
||||
let cands = std::slice::from_ref(&c);
|
||||
let trade_id = TRADE_ID_BASE + 100_000_122;
|
||||
|
||||
// Asked for explicitly: it must come back, not be silently absent.
|
||||
let one = parse(
|
||||
&handle_market_status(Some(&format!("tradeIds={trade_id}")), &econ, &store, cands)
|
||||
.await,
|
||||
);
|
||||
let recs = one["auctionInfo"].as_array().unwrap();
|
||||
assert_eq!(recs.len(), 1, "the polled unlisted id must resolve");
|
||||
assert_eq!(recs[0]["tradeId"], trade_id);
|
||||
assert_eq!(recs[0]["tradeState"], "inactive");
|
||||
assert_eq!(recs[0]["expires"], 0);
|
||||
|
||||
// Unfiltered poll: still present.
|
||||
let all = parse(&handle_market_status(None, &econ, &store, cands).await);
|
||||
assert_eq!(all["auctionInfo"].as_array().unwrap().len(), 1);
|
||||
|
||||
// A DIFFERENT id was asked for: the unlisted row must not be volunteered.
|
||||
let other = parse(
|
||||
&handle_market_status(Some("tradeIds=900000999"), &econ, &store, cands).await,
|
||||
);
|
||||
assert_eq!(
|
||||
other["auctionInfo"].as_array().unwrap().len(),
|
||||
0,
|
||||
"only the ids actually asked about are answered"
|
||||
);
|
||||
|
||||
// Once the item owns a real auction, the auction wins and the id is not
|
||||
// duplicated by an inactive row.
|
||||
store
|
||||
.create_listing(
|
||||
&trade_id.to_string(),
|
||||
"169193",
|
||||
Some("core-pile"),
|
||||
Some(100_000_122),
|
||||
Some(169193),
|
||||
150,
|
||||
2500,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let listed = parse(
|
||||
&handle_market_status(Some(&format!("tradeIds={trade_id}")), &econ, &store, cands)
|
||||
.await,
|
||||
);
|
||||
let recs = listed["auctionInfo"].as_array().unwrap();
|
||||
assert_eq!(recs.len(), 1, "exactly one record for one tradeId");
|
||||
assert_eq!(recs[0]["tradeState"], "active");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn buy_now_debits_mints_and_closes() {
|
||||
let (store, _d) = store_at("buy").await;
|
||||
|
||||
Reference in New Issue
Block a user