Skip to content

Introduce trigger model config for OTO contingencies #2623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hope2see opened this issue May 12, 2025 · 16 comments
Open

Introduce trigger model config for OTO contingencies #2623

hope2see opened this issue May 12, 2025 · 16 comments
Assignees
Labels
enhancement New feature or request

Comments

@hope2see
Copy link
Contributor

hope2see commented May 12, 2025

Bug Report

According to OTO doc, a parent order only trigger child orders only when fully filled. However, it actually triggers in partially filled status, resulting in some derivative problems.

Confirmation

Before opening a bug report, please confirm:

  • [v] I’ve re-read the relevant sections of the documentation.
  • [v] I’ve searched existing issues and discussions to avoid duplicates.
  • [v] I’ve reviewed or skimmed the source code (or examples) to confirm the behavior isn’t by design.
  • [v] I’ve confirmed the issue is reproducible with the latest version of nautilus_trader.

Expected Behavior

A parent order should not trigger OTO child orders before it is fully filled.

Actual Behavior

It triggers OTO child orders when it is filled, regardless of whether it is fully or partially filled.

Steps to Reproduce the Problem

It can be checked using the pytest below.
The current implementation fails these tests.

Code Snippets or Logs

I tested these by adding to test_exchange_contingences.py.


    def test_submit_bracket_limit_entry_buy_paritally_fills_and_not_trigger_sl_and_tp(self):
        # Arrange: Prepare market
        tick = QuoteTick(
            instrument_id=ETHUSDT_PERP_BINANCE.id,
            bid_price=ETHUSDT_PERP_BINANCE.make_price(3090.2),
            ask_price=ETHUSDT_PERP_BINANCE.make_price(3090.5),
            bid_size=ETHUSDT_PERP_BINANCE.make_qty(15.100),
            ask_size=ETHUSDT_PERP_BINANCE.make_qty(5.100), # <-- Less than order qunatity
            ts_event=0,
            ts_init=0,
        )

        self.data_engine.process(tick)
        self.exchange.process_quote_tick(tick)

        bracket = self.strategy.order_factory.bracket(
            instrument_id=ETHUSDT_PERP_BINANCE.id,
            order_side=OrderSide.BUY,
            quantity=ETHUSDT_PERP_BINANCE.make_qty(10.000),
            entry_price=ETHUSDT_PERP_BINANCE.make_price(3090.5), # <-- Limit price is equal to top-of-book
            sl_trigger_price=ETHUSDT_PERP_BINANCE.make_price(3050.0),
            tp_price=ETHUSDT_PERP_BINANCE.make_price(3150.0),
            entry_order_type=OrderType.LIMIT,
        )

        # Act
        self.strategy.submit_order_list(bracket)
        self.exchange.process(0)

        # Only after the entry order is fully filled, the child orders should be triggered.
        assert bracket.orders[0].status == OrderStatus.PARTIALLY_FILLED
        assert bracket.orders[1].status == OrderStatus.SUBMITTED # Should not be ACCEPTED
        assert bracket.orders[2].status == OrderStatus.SUBMITTED # Should not be ACCEPTED
        assert len(self.exchange.get_open_orders()) == 1
        assert bracket.orders[0] in self.exchange.get_open_orders()

    def test_submit_bracket_limit_entry_sell_paritally_fills_and_not_trigger_sl_and_tp(self):
        # Arrange: Prepare market
        tick = QuoteTick(
            instrument_id=ETHUSDT_PERP_BINANCE.id,
            bid_price=ETHUSDT_PERP_BINANCE.make_price(3090.2),
            ask_price=ETHUSDT_PERP_BINANCE.make_price(3090.5),
            bid_size=ETHUSDT_PERP_BINANCE.make_qty(5.100), # <-- less than order qunatity
            ask_size=ETHUSDT_PERP_BINANCE.make_qty(15.100),
            ts_event=0,
            ts_init=0,
        )

        self.data_engine.process(tick)
        self.exchange.process_quote_tick(tick)

        bracket = self.strategy.order_factory.bracket(
            instrument_id=ETHUSDT_PERP_BINANCE.id,
            order_side=OrderSide.SELL,
            quantity=ETHUSDT_PERP_BINANCE.make_qty(10.000),
            entry_price=ETHUSDT_PERP_BINANCE.make_price(3090.2), # <-- Limit price is equal to top-of-book
            sl_trigger_price=ETHUSDT_PERP_BINANCE.make_price(3150.0),
            tp_price=ETHUSDT_PERP_BINANCE.make_price(3000.0),
            entry_order_type=OrderType.LIMIT,
        )

        # Act
        self.strategy.submit_order_list(bracket)
        self.exchange.process(0)

        # Only after the entry order is fully filled, the child orders should be triggered.
        assert bracket.orders[0].status == OrderStatus.PARTIALLY_FILLED
        assert bracket.orders[1].status == OrderStatus.SUBMITTED # Should not be ACCEPTED
        assert bracket.orders[2].status == OrderStatus.SUBMITTED # Should not be ACCEPTED
        assert len(self.exchange.get_open_orders()) == 1
        assert bracket.orders[0] in self.exchange.get_open_orders()

Specifications

  • OS platform: Mac
  • Python version: 3.12.9
  • nautilus_trader version: branch (commit 4b29b43)
@hope2see hope2see added the bug Something isn't working label May 12, 2025
@hope2see
Copy link
Contributor Author

@cjdsellers
If my analysis is correct, the parent order's fill status should be checked in OrderMatchingEngine.fill_order() method. However it is not checked there.

@cjdsellers
Copy link
Member

Hi @hope2see

Thanks for the report.

It seems the docs are misaligned with the code. Potentially the docs were lifted from a FIX description of that contingency type, however this might not be how contingent orders are actually executed in all cases.

This requires some research to find venue specs, but I think it's desirable for child orders to be triggered on a partial fill of the parent. For instance, if you filled half of the entry quantity, you'd want the equivalent quantity working in the market as SL and TP, rather than awaiting a complete fill of the parent (which isn't guaranteed).

What are your thoughts? (I'm leaning towards a docs update here.)

@hope2see
Copy link
Contributor Author

Right, venue specs need to be researched.

BTW, this issue is not just limited to bracket order, but related to general OTO contingency.

If a partially filled order is allowed to trigger child orders, the quantities of them should be automatically adjusted, whenever the parent order is filled. However, this is not addressed in the current implementation.

@hope2see
Copy link
Contributor Author

In binance, only an fully filled filled order triggers child orders.
binance

If the limit order is only partially filled, the TP/SL will not be triggered.

@cjdsellers
Copy link
Member

cjdsellers commented May 12, 2025

BTW, this issue is not just limited to bracket order, but related to general OTO contingency.

Indeed, all of the below relates to the OTO contingency.

If a partially filled order is allowed to trigger child orders, the quantities of them should be automatically adjusted, whenever the
parent order is filled. However, this is not addressed in the current implementation.

That is almost the current implementation, quantities are automatically adjusted for reduce_only orders:
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/backtest/matching_engine.pyx#L2289

Also to be considered is behavior for the order emulator:
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/execution/emulator.pyx#L221

For the order manager (looks like quantities are modified here on partial fills):
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/execution/manager.pyx#L422

Note the order manager is used for live trading also.

So if we'd like to align with other venues such as Binance, this is going to require another venue configuration option oto_triggers_on_partial_fill (or similarly named, and true by default to maintain current behavior).

@cjdsellers
Copy link
Member

If the limit order is only partially filled, the TP/SL will not be triggered.

Understood, seems like an interesting default which results in uncovered exposure in partially filled entry scenarios.

@hope2see
Copy link
Contributor Author

@cjdsellers
I made a tiny patch for this issue and passed all the pytests including those I added to verifying this issue.

If you are okay with the policy of not allowing partial fill triggering, then I will open a PR.

@cjdsellers
Copy link
Member

Hi @hope2see

A patch would be most welcome to introduce the possibility of being able to align the behavior more accurately with Binance.

That said, I don't think Binance should be the canonical standard to which we align behavior.

Here is an except from some research I did earlier:

Full trigger model

Many traditional stock brokers (TD Ameritrade, Schwab, Fidelity, E*TRADE) favor the full execution trigger model for OTO orders​. This means the contingent orders are held off-market until the primary order completely fills. This approach simplifies the logic (you either have the position or you don’t) but leaves a potential gap in coverage during partial fills.

Partial trigger model

On the other hand, brokers/platforms oriented towards active trading and high-volume markets (Interactive Brokers, Kraken, futures platforms, etc.) use the partial trigger model​. This method actively adjusts or sends child orders as each fill comes in, ensuring no portion of a filled order is left without its OCO protection. Crypto exchanges vary: some like Kraken follow the partial model, while others like Binance and Coinbase use the full-fill model on their spot exchanges​.

Solution

So I think we need to introduce a venue configuration option to retrain the partial trigger model behavior by default (so we don't introduce a breaking change). This will then allow us to more accurately simulate a venue such as Binance, which will meet your use case.

Introducing configuration is often a trade-off between increasing complexity and increasing value on some axis. In this case, I think it's worth it - and will be very useful when we introduce "venue templates", which will give you say a Binance Futures venue out-of-box as closely as the platform can simulate.

What are your thoughts?

References

fidelity.com

help.streetsmart.schwab.com

reddit.com

help.streetsmart.schwab.com

binance.com

coinbase.com

support.kraken.com

@hope2see
Copy link
Contributor Author

hope2see commented May 12, 2025

What a quick and great research!
Given that there’s no unified policy across venues, using venue-specific configuration sounds like a solution.

That said, if we decide to adopt the partial trigger model as a default, we’ll also need to implement (or fix) the logic for auto-adjusting the quantities of child orders. At the moment, that functionality doesn’t work.

@hope2see
Copy link
Contributor Author

Just to add one more thing, as of now, the current implementation can be easily fixed to work correctly under the full trigger model.

@cjdsellers
Copy link
Member

Hi @hope2see

Current state

That said, if we decide to adopt the partial trigger model as a default, we’ll also need to implement (or fix) the logic for auto-adjusting the quantities of child orders. At the moment, that functionality doesn’t work.

For the venue managed contingency case, I think the functionality works for reduce_only orders:
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/backtest/matching_engine.pyx#L2297

Otherwise, on a trigger the order is just processed as if it was just submitted (I think this is the correct behavior for partial trigger model).
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/backtest/matching_engine.pyx#L2264

If you believe it doesn't, then a test asserting this would be helpful (then I can fix the behavior).

For the strategy managed contingency case, the quantity is updated here:
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/execution/manager.pyx#L421

Moving forward

Just to add one more thing, as of now, the current implementation can be easily fixed to work correctly under the full trigger model.

How about we add a venue configuration option use_oto_partial_trigger which is true by default.
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/backtest/config.py#L151

Then, you could add the behavior for the full trigger model which works when this is set to false.
Also include tests for the new branches.

Then after the above, if any of the default behavior does not have test cases - I'll add those as a follow-up.

What do you think?

@hope2see
Copy link
Contributor Author

For the venue managed contingency case, I think the functionality works for reduce_only orders:
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/backtest/matching_engine.pyx#L2297

Actually, when fill_order() is invoked due to a partial fill, this part of the code is not executed because the position is None at that moment—it returns before reaching this section.

@hope2see
Copy link
Contributor Author

How about we add a venue configuration option use_oto_partial_trigger which is true by default.
https://github.com/nautechsystems/nautilus_trader/blob/develop/nautilus_trader/backtest/config.py#L151

Yes, it would be great! :)

@cjdsellers
Copy link
Member

cjdsellers commented May 15, 2025

Hi @hope2see,

I've updated the orders concept guide to more accurately describe the behavior we're targeting.

I didn't add anything about the config yet as it's still TBD.

@hope2see
Copy link
Contributor Author

Oh, It is very comprehensive!! 👍🏻

@cjdsellers cjdsellers self-assigned this May 28, 2025
@cjdsellers cjdsellers added enhancement New feature or request and removed bug Something isn't working labels May 28, 2025
@cjdsellers
Copy link
Member

Hi @hope2see

I've converted this ticket to an enhancement request to add a trigger model config.

@cjdsellers cjdsellers changed the title Partially filled order should not trigger child orders with OTO contingency Introduce trigger model config for OTO contingencies May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants