search

betcode-org / flumine

flūmine - Betting trading framework
MIT License
172 stars 60 forks source link

Simulated fill or kill order broken on master #706

Closed mzaja closed 6 months ago

mzaja commented 11 months ago

This commit breaks the simulation if FOK orders are used: c4359009907f374844885bae58ac128cde8e4ffc. I'll open a PR for the fix, but clearly there is a testing gap which needs to be closed.

liampauling commented 10 months ago

Hi @mzaja I have been doing some testing with FILL_OR_KILL and noticed what I think is a bug, we currently default minFillSize to 0 if not provided but reading the docs I believe it should actually default to stake.

Current:

min_fill_size = instruction["limitOrder"].get("minFillSize") or 0

Docs

Exchange will only match the order if at least the specified minFillSize can be matched (if passed) or the whole order matched (if not)

Proposed:

min_fill_size = instruction["limitOrder"].get("minFillSize") or size
mzaja commented 10 months ago

That is very interesting... I have checked my records and I am getting FoK bets matched for less than the full stake when not specifying minFillSize. Could it be that Flumine/bflw is inserting 0 when the minFillSize value is not specified, or is this an error in Betfair's documentation?

Can you test this to clear any doubt? Find a market with £1 available to back/lay at some price, then place a £2 (or larger) bet without minFillSize specified and see if it matches. I am not able to do it myself at the moment unfortunately.

liampauling commented 10 months ago

Testing matches the betfair docs, can confirm that bflw/flumine are sending None when None so slightly confused.

mzaja commented 9 months ago

Interesting indeed... I will need to check in more detail what is going on with this. But, since you tested it and it confirms what is mentioned in the official documentation, please do update the code.

mzaja commented 7 months ago

@liampauling I have tested this today and indeed, FoK orders work the way it is described on Betfair's website. Therefore, that fix you proposed should be applied.

mzaja commented 6 months ago

The issue which prompted the reopening of this ticket has been fixed and merged now.