FillModel.LimitIfTouchedFill checks the wrong side of the quote (sell vs ask, buy vs bid), so LIT orders fill across the spread

Abierto
  • ¿Algo no está bien?
#10,025 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
55/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
csharp
Área
backend, testing

Línea de trabajo

Start with Common/Orders/Fills/FillModel.cs and compare LimitIfTouchedFill with the equity model behavior described in the issue; the intended rule needs confirmation because the linked documentation disagrees. Read Tests/Common/Orders/Fills/LimitIfTouchedSpreadTests.cs and the existing ImmediateFillModelTests.PerformsLimitIfTouchedFillBuy/Sell cases. Done means the intended quote side is established and tests cover a spread where bid and ask differ.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Expected Behavior

After the trigger is touched, a LIT order should behave like a limit order: a buy fills only when the ask is <= limit, and a sell fills only when the bid is >= limit. That is what EquityFillModel.LimitIfTouchedFill does (ask for buys, bid for sells). The base FillModel already prices this way in LimitFill, StopLimitFill and GetPrices.

Actual Behavior

Common/Orders/Fills/FillModel.cs, LimitIfTouchedFill uses the opposite side of the quote:

// line 659, Sell
if (GetAskPrice(asset, out pricesEndTime) >= order.LimitPrice)
// line 675, Buy
if (GetBidPrice(asset, out pricesEndTime) <= order.LimitPrice)

The order then fills at order.LimitPrice. A buy LIT fills as soon as the bid reaches the limit, even when no seller is offering at that price. A sell LIT fills as soon as the ask reaches the limit. Either way the order fills at its limit price on the far side of the spread.

Example: an option quotes bid 1.00 / ask 1.20. A buy LIT with limit 1.05 fills at 1.05 once triggered, but nobody is offering at 1.05.

This affects every security that uses the base implementation: ImmediateFillModel, FutureFillModel and FutureOptionFillModel, which covers options, futures, future options, forex, crypto, CFDs and indices. Equities are not affected because EquityFillModel overrides the method.

The code has been this way since LIT orders were added in 643e8754 (#5164). The existing tests ImmediateFillModelTests.PerformsLimitIfTouchedFillBuy/Sell use quote bars where bid == ask (101/101/100.5/101 and 100/100/99/99), so they cannot catch which side is used.

Docs: which behaviour is intended?

The documentation does not agree with itself, so I'd like to confirm the intended rule before opening a PR:

If the Immediate model page describes the intended behaviour, this is a docs and consistency question rather than a bug. If the Equity model rule is the intended one, I'd propose the fix below.

Potential Solution

Swap the two price getters in FillModel.LimitIfTouchedFill:

-                        if (GetAskPrice(asset, out pricesEndTime) >= order.LimitPrice)
+                        if (GetBidPrice(asset, out pricesEndTime) >= order.LimitPrice)
...
-                        if (GetBidPrice(asset, out pricesEndTime) <= order.LimitPrice)
+                        if (GetAskPrice(asset, out pricesEndTime) <= order.LimitPrice)

I tested this locally. The two new tests below pass, and all 35 LIT-related unit tests (including those two) pass. This issue does not cover whether the fill price should also follow the Equity model (min(ask, limit) / max(bid, limit)) rather than always using the limit price. I can include that change if you want it.

Regression algorithms that may need their statistics updated:

  • ContinuousFutureLimitIfTouchedOrderRegressionAlgorithm (C#), which uses FutureFillModel. This is the only LIT regression algorithm I found on a non-equity security.
  • LimitIfTouchedRegressionAlgorithm / LimitIfTouchedAsyncRegressionAlgorithm (C# and Python) and SplitEquityRegressionAlgorithm trade equities through EquityFillModel. They should not change, but I would re-run them to check.

If you confirm the intended behaviour, I can open a PR with the code change, the tests and any regression algorithm updates. I can also send a matching fix to QuantConnect/Documentation for whichever pages are wrong.

Reproducing the Problem

Unit test Tests/Common/Orders/Fills/LimitIfTouchedSpreadTests.cs uses ImmediateFillModel with SPY minute TradeBar and QuoteBar subscriptions. The spread is set wider than the limit, so neither order should fill:

  • Buy LIT, quantity 100, trigger 101.5, limit 100. TradeBar low 99.8 touches the trigger. Quote: bid 99.90, ask 100.30. The ask is above the limit, so the order should not fill. Current code compares the bid (99.90 <= 100) and fills 100 @ 100.
  • Sell LIT, quantity -100, trigger 101.5, limit 103. TradeBar high 103.1 touches the trigger. Quote: bid 102.70, ask 103.10. The bid is below the limit, so the order should not fill. Current code compares the ask (103.10 >= 103) and fills -100 @ 103.

Both cases assert OrderStatus.None and fail on master. Both pass with the swap above.

System Information

Linux (kernel 7.0), .NET SDK 10.0.401, Lean master at 9ecfed9.

Checklist
  • I have completely filled out this template
  • I have confirmed that this issue exists on the current master branch
  • I have confirmed that this is not a duplicate issue by searching issues
  • I have provided detailed steps to reproduce the issue
Lenguaje dominante
C#
Estrellas
22.5k
Forks
5.4k
Merge medio
22 h 6 min
PR fusionados (30 d)
81

Preparar el entorno

Abrir en Codespaces

Inicia el contenedor de desarrollo del proyecto en tu navegador, con tu propia cuenta de GitHub.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de QuantConnect/Lean

Todos los issues de QuantConnect/Lean

Issues similares

Más issues de C#

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.