164 lines
11 KiB
Markdown
164 lines
11 KiB
Markdown
# Umsetzungsplan: Fable-Code-Review-Fixes (Copytrading)
|
||
|
||
> Basis: Fable-5-Review nach Umsetzung des Rentabilitätsplans (Stand 2026-07-08).
|
||
> Ausgangslage: 207 Tests grün, Build/Smoke grün. Die Logic/-Klassen sind laut Review
|
||
> sauber; die Lücken liegen im **Zusammenspiel** von SELL-Leiter, Engine und den
|
||
> Hintergrund-Services (TraderMonitorService).
|
||
|
||
## Fortschritt
|
||
- ✅ **Slice 0** – IClobClient-Seam + FakeClobClient (verhaltensneutral).
|
||
- ✅ **Slice 1** – K1/H2/H1: atomarer Claim, Cleanup+Engine schonen Leitern, Floor-Robustheit. 8 Tests.
|
||
- ✅ **Slice 2** – K2: Startup-Reconciliation (GetOpenOrders ohne assetId = alle). 3 Tests.
|
||
- ✅ **Slice 3** – K3 (System-SELL vom Ownership-Check ausgenommen + Resolved-Cache) + M5 (Demo-Score-Anzeige, stündl. Auto-Pause). 5 Tests.
|
||
- ✅ **Slice 4** – H4 (RoundToTick + Dust-Abbruch), M1 (GlobalPnl im Guard), M2 (TokenId), M3-min (serverseitiges Max + lauter Fehlschlag), M4 (Parser 9999), M6 (Fees in Orders), Doku. 10 Tests.
|
||
- ✅ **Slice 5** – H3: BUY-Skip während ExitPending (Entscheidung A).
|
||
- ✅ **Slice 6** – SnapshotService entfernt, Demo-Balance/PnL-Reconciliation, Settings-Validierung (IsLadderConfigInverted + Load-Warnung). 3 Tests.
|
||
|
||
**Stand: 244 Tests grün, Build/Smoke grün.**
|
||
|
||
### Nachgelagerte Testabdeckung (nach dem K3-Fund)
|
||
- **Engine-Integrationstests** (`CopyTradingEngineTests`, gemockter CLOB): H3 BUY-Skip, Doppel-SELL-Guard, K3 System-Close, Fremd-Trader-Reject, H2 Cleanup-schont-Leiter (+Kontrast). Engine `_clob`→`IClobClient`, `ProcessAccountOrderAsync` internal.
|
||
- **⚠️ K3-Korrektur:** Der Slice-3-Fix sass am falschen Ort (downstream ~Z.643). Der echte Ownership-Check ist der frühe `inPortfolio`-Lookup (~Z.437, `p.SourceTraderId == signal.TraderId`), der System-Signale schon vorher mit early return abwies. Jetzt am richtigen Ort via `IsAuthorizedSell` – **vom Engine-Test aufgedeckt**.
|
||
- **K1a-Test** (`TraderMonitorServiceTests`): Cleanup cancelt Leiter-Order nicht (aktive Leiter) bzw. cancelt sie ohne Leiter. `_clob`→`IClobClient`, `CleanupStaleOpenOrdersAsync` internal.
|
||
|
||
**Alle 3 kritischen + 4 hohen Bugs sind jetzt durch Tests abgesichert** (K1a/K1b/K2/K3/H1/H2/H3/H4).
|
||
|
||
### Bewusst aufgeschobene Follow-ups (Live-Verifikation/Risiko)
|
||
- **M3 Autoincrement-Migration**: `TradeId` auf DB-Autoincrement umstellen – Schema-Änderung an der Trade-Persistenz, erst im Zielland live verifizieren. (M3-Minimum ist umgesetzt.)
|
||
- **PersistenceService-Dedup-Zeitfenster**: `Exists(AccountId,TokenId)` blockt legit Re-Entries; robuster Fix (z.B. OpenedAt-basiert) braucht Live-Daten – Duplikat-Schutz nicht unverifiziert brechen.
|
||
- **Perf**: `UpsertLive`-Dirty-Check (Schreib-Amplifikation) und Leiter-Parallelität – laut Fable bei aktueller Größe unkritisch.
|
||
- **M6/K2**: fee-signierte Orders bzw. `/data/orders` ohne asset_id sind API-gated → im Zielland verifizieren.
|
||
|
||
## Arbeitsgrundsätze (für jeden Slice)
|
||
|
||
1. **`.agents/rules/clob.md`:** vor jedem CLOB-nahen Slice ein Commit als Rollback-Punkt;
|
||
Preis-/Zustandslogik pur in `SellLogic`/`CopyTradingRisk` + neue Tests; Service-Interaktionen
|
||
(Cleanup überspringt Leiter etc.) mit kleinem **Integrationstest über gemockten CLOB-Client**.
|
||
2. Nach jedem Slice: `dotnet build` + `dotnet test` (alle grün) + `--smoke-ui` grün, dann commit+push.
|
||
3. Ein Slice = eine kohärente Einheit = ein Commit. Reihenfolge unten folgt Fables Empfehlung.
|
||
|
||
---
|
||
|
||
## Slice 0 (Prereq): Testbarkeit — `IClobClient`-Interface
|
||
**Warum zuerst:** K1/H2/K2 brauchen Integrationstests mit gemocktem CLOB. `PolymarketClobClient`
|
||
ist heute eine konkrete Klasse ohne Interface → nicht mockbar.
|
||
|
||
- Interface `IClobClient` (Core) mit den von Leiter/Reconciliation genutzten Methoden:
|
||
`PlaceOrderAsync`, `CancelOrderAsync`, `GetOpenOrdersAsync`, `CancelConflictingOrdersAsync`.
|
||
- `PolymarketClobClient : IClobClient`. DI zusätzlich `IClobClient → PolymarketClobClient`.
|
||
- `SellLadderService`/Reconciliation gegen `IClobClient` typisieren (Engine kann vorerst konkret bleiben).
|
||
- **Verhaltensneutral, keine Logikänderung.** Ermöglicht `FakeClobClient` im Testprojekt.
|
||
- Tests: keine neuen fachlichen; Build grün genügt.
|
||
|
||
---
|
||
|
||
## Slice 1: „Wer darf Leiter-Orders anfassen" (H1 + K1 + H2) 🔴🟠
|
||
Kernthema: Leiter-Order darf nur von der Leiter angefasst/gecancelt werden.
|
||
|
||
- **H1 — Atomarer Claim:** In `SellLadderService.StartLadderAsync` als ERSTES
|
||
`if (!_copyState.ExitLadders.TryAdd(key, placeholder)) return false;` → macht ALLE Aufrufer
|
||
(Engine-SELL + ProfitTarget) idempotent. Bei Fehlschlag der Order den Key wieder entfernen.
|
||
- **K1 — Cleanup überspringt Leitern:** In `TraderMonitorService.CleanupStaleOpenOrdersAsync`
|
||
Keys mit `_copyState.ExitLadders.ContainsKey(key)` überspringen (`continue`).
|
||
- **K1 — Floor-Robustheit:** In `SellLadderService.ProcessLadderAsync` am Floor NICHT dauerhaft
|
||
früh zurückkehren, sondern periodisch via `GetOpenOrdersAsync` prüfen, ob die Floor-Order noch
|
||
ruht; wenn nicht → am Floor neu platzieren (+ `PendingOrderTimestamps` refreshen).
|
||
- **H2 — Engine-Cancel schont Leiter:** Den Pre-Signal-`CancelConflictingOrdersAsync`-Aufruf der
|
||
Engine überspringen, wenn `_copyState.ExitLadders.ContainsKey(key)` (oder hinter den
|
||
ExitPending-Check verschieben).
|
||
- Tests: Integrationstest (FakeClob) — Cleanup cancelt KEINE Leiter-Order; zwei parallele
|
||
StartLadder-Aufrufe → nur eine Leiter; Floor-Order weg → Leiter platziert neu. Pure: ggf.
|
||
Floor-Recheck-Entscheidung.
|
||
|
||
---
|
||
|
||
## Slice 2: Neustart-Reconciliation (K2) 🔴
|
||
Ruhende GTC-Leiter-/Maker-Orders überleben Neustarts, der Verwaltungszustand nicht.
|
||
|
||
- Beim Modul-Start je **Live-Account** alle offenen CLOB-Orders via `GetOpenOrdersAsync` abrufen und
|
||
pauschal canceln (deterministisch; die Engine entscheidet danach sauber neu). Kein Leiter-Rebuild.
|
||
- Ort: eigener Startup-Schritt im Modul (z. B. in `TraderMonitorService`-Warmup oder als kurzer
|
||
`IHostedService`), NACH der State-Hydration, VOR dem ersten Signal-Processing.
|
||
- Umfangreiches Logging (welche Orders gecancelt).
|
||
- Tests: Integrationstest (FakeClob) — für jeden offenen Order-Eintrag wird Cancel gerufen.
|
||
|
||
---
|
||
|
||
## Slice 3: Demo-Resolution + Demo-Score (K3 + M5) 🔴🟡
|
||
Sonst ist die Demo-Validierungsphase (auf der die Zielland-Strategie beruht) wertlos.
|
||
|
||
- **K3 — System-Signale (TraderId==0) vom Ownership-Check ausnehmen:** In der Engine SELL-Pre-Flight
|
||
(`p.SourceTraderId == signal.TraderId`) den Fall `signal.TraderId == 0` zulassen (System-Close bei
|
||
Marktauflösung). Zusätzlich „bereits als resolved erkannt"-Cache, damit ein Markt nur einmal
|
||
verarbeitet wird (verhindert 30-s-Loop-Spam + API-Last).
|
||
- **M5 — Demo-Score & schnellerer Auto-Pause:** Copy-Score getrennt für Demo (Anzeige/Validierung)
|
||
und Live (Pausieren) berechnen; der Kill-Switch filtert weiterhin `!IsDemo`, aber die Demo-Kennzahlen
|
||
füllen die Spalten. Zusätzlich stündlicher Light-Check nur für die Pause-Regel (statt nur alle 12 h).
|
||
- Tests: Ownership-Ausnahme (Engine), Resolved-Cache (pure). Demo/Live-Score-Trennung ist Job-Logik.
|
||
|
||
---
|
||
|
||
## Slice 4: Kleine, klar umrissene Fixes (H4 + M1 + M2 + M3 + M4 + M6 + Doku) 🟠🟡🟢
|
||
Jeweils klein und abgegrenzt — in einem oder zwei Commits.
|
||
|
||
- **H4 — Dust-Reject-Schleife:** (a) Leiter-Preis vor der USDC-Berechnung auf Tick runden
|
||
(`Math.Round(next, 3)`, zentral in `SellLogic`); (b) Abbruch in `ProcessLadderAsync`:
|
||
`pos.Size < CopyTradingRisk.MinShares` → Leiter beenden, `ExitPending=false`, Dust loggen.
|
||
Pure Tests für Rundung + Abbruch.
|
||
- **M1 — GlobalPnl-Doppelzählung:** In `TraderMonitorService` (~Z.910 und ~Z.961) das
|
||
`GlobalPnl += realizedPnl` INNERHALB des `_processedClosures`-Guards buchen (wie in
|
||
`PollClosedAccountsAsync` bereits korrekt).
|
||
- **M2 — TokenId in Live-Close-Records:** In beiden Live-Close-Records (~Z.922-940 und ~Z.972-990)
|
||
`TokenId = removedPos.TokenId` setzen (der 0.4-Fix erwischte nur den Demo-Pfad).
|
||
- **M3 — TradeId robust:** `ClosedTrade.TradeId` auf DB-Autoincrement (`ValueGeneratedOnAdd`)
|
||
umstellen + Code-Vergabe (`GetNextTradeId`) entfernen + Migration. Eliminiert die stille
|
||
PK-Kollisions-Fehlerklasse und den teuren Full-Table-`Max()`-Startup in `Program.cs`.
|
||
(Alternative/Minimum: serverseitiges `Max()` + lauter Fehlschlag statt `catch {}`.)
|
||
- **M4 — Parser-Default:** `MongoExportParser` ProfitTarget-Fallback `50m → 9999m` (sonst schaltet
|
||
ein erneuter `--migrate-json`-Lauf Take-Profit unbeabsichtigt scharf). Pure Test.
|
||
- **M6 — Fee in signierte Orders:** An den Callsites (Engine-BUY, Leiter, PreRedeem)
|
||
`actualFeeBps` aus `FeeModel`/`MarketData.TakerFeeBps` an `PlaceOrderAsync` durchreichen.
|
||
Verifikation im Zielland, aber die Verdrahtung jetzt.
|
||
- **Doku — Stale [Description]:** `SellFloorPct` ist verdrahtet (nicht „Phase 0.1 offen");
|
||
`ProfitTarget`-Text nicht mehr „folgt in Phase 0.3". Texte aktualisieren (Richard verlässt sich drauf).
|
||
|
||
---
|
||
|
||
## Slice 5: H3 — BUY während ExitPending 🟠
|
||
Re-buyt der Master, während unsere Leiter verkauft, kauft die Engine normal zu → die Leiter verkauft
|
||
danach `pos.Size` inkl. neuer Shares zum alten Floor.
|
||
|
||
**ENTSCHEIDUNG (Richard, 2026-07-08): Variante A — BUYs skippen, solange `ExitPending`.**
|
||
Während des Ausstiegs keine Zukäufe; die Leiter verkauft die Position sauber zu Ende.
|
||
|
||
- Umsetzung: In der Engine BUY-Pre-Flight früh prüfen —
|
||
`if (account.OpenPositions.TryGetValue(signal.TokenId, out var p) && p.ExitPending) { log + return; }`.
|
||
(Spiegelt die bestehende Double-Sell-Guard-Logik, nur für den BUY-Pfad.)
|
||
- Umfangreiches Logging (verworfener BUY während aktivem Exit inkl. TokenId/TraderId).
|
||
- Tests: Engine-BUY-Pfad überspringt, solange `ExitPending`; nach Leiter-Ende (ExitPending=false)
|
||
wird ein neuer BUY wieder normal ausgeführt.
|
||
|
||
---
|
||
|
||
## Slice 6: Rest nach Gelegenheit (🟢 Perf/Doku)
|
||
- **PersistenceService-Dedup:** `Exists(AccountId, TokenId)` blockt legitime Re-Entries (HF-Alltag) →
|
||
Dedup-Schlüssel um Zeitfenster ergänzen. (Copy-Score untererfasst sonst.)
|
||
- **Demo-Balance vs. PnL:** Balance sollte `exitUsd − ExitFee` gutschreiben (und der BUY die Entry-Fee
|
||
abziehen), damit Σ(Balance-Änderungen) = Σ(PnL). Aktuell driftet es um die Fees.
|
||
- **Settings-Validierung:** `MaxPriceDifference% > SellFloorPct` → Leiter startet unter dem Floor
|
||
(sofortige „Floor erreicht"-Notification). UI-Warnung/Validierung.
|
||
- **Perf — Schreib-Amplifikation:** `PollLiveAccountsAsync` `UpsertLive` je Position alle 30 s →
|
||
Dirty-Check (nur bei Änderung) oder Batch.
|
||
- **Perf — Leitern seriell:** `ProcessLadderAsync` pro Tick seriell → begrenzte Parallelität + Timeout.
|
||
- **Totcode:** `services/SnapshotService.cs` entfernen (nirgends registriert) oder bewusst reaktivieren.
|
||
|
||
---
|
||
|
||
## Empfohlene Reihenfolge (Fable)
|
||
`Slice 0` (Test-Infra) → `Slice 1` (K1+H2+H1) → `Slice 2` (K2) → `Slice 3` (K3+M5) →
|
||
`Slice 4` (H4/M1/M2/M3/M4/M6/Doku) → `Slice 5` (H3, nach Entscheidung) → `Slice 6` (Rest).
|
||
|
||
**Als korrekt bestätigt (nicht anfassen):** Logic/-Klassen sauber/verhaltenstreu; ExitPending
|
||
EF-ignoriert; TotalFees gemappt; closed_trades-Indizes vorhanden; SellLadderService als
|
||
Singleton+Hosted (eine Instanz); Copy-Score-Find serverseitig; SELL-Spam-Blockade seitensensitiv.
|