Limit order fees #690
Limit order fees #690
Conversation
sistemd
force-pushed
the
limit_order_fees
branch
2 times, most recently
from
321744b
to
31c587d
Compare
| let gas_price_estimator = Arc::new(InstrumentedGasEstimator::new( | ||
| shared::gas_price_estimation::create_priority_estimator( | ||
| &http_factory, |
There was a problem hiding this comment.
nit: The block instantiating the quoter could be moved right before the solvable_orders_cache. Then the solvable_orders_cache wouldn't need to be moved at all which shrinks the PR diff a bit.
| /// Quotes all limit orders and sets the fee_amount for each one to the fee returned by the | ||
| /// quoting process. If quoting fails, the corresponding order is filtered out. | ||
| async fn set_limit_order_fees(&self, orders: Vec<Order>) -> Vec<Order> { |
There was a problem hiding this comment.
I think even when caching gets introduced in a follow up PR time boxing the fee calculation might be needed.
There was a problem hiding this comment.
Good point. In the same module we also have a time limit on native prices. Should do something similar for getting as many quotes as possible and run both tasks at the same time.
There was a problem hiding this comment.
Thanks for letting me know @vkgnosis, that's good to know.
sistemd
force-pushed
the
limit_order_fees
branch
2 times, most recently
from
afc44e3
to
3a62da0
Compare
sistemd
force-pushed
the
limit_order_fees
branch
from
888f027
to
1188e25
Compare
sistemd
force-pushed
the
limit_order_fees
branch
from
1188e25
to
5d9996a
Compare
| self.optimal_quoter | ||
| .store_quote(quote) | ||
| .await | ||
| .map_err(CalculateQuoteError::Other)? |
There was a problem hiding this comment.
Hoisting this up to the API level was a good choice.
Progress on #643.
Quotes limit orders whenever an auction is cut. The next PR will start caching the calculated fees to reduce the frequency of quoting, since quoting isn't cheap.
Note that the fee does not get stored anywhere in out database yet.
Refactored some interfaces to make things more explicit and to try to illustrate business logic a tiny bit more clearly.
Test Plan
Currently the change is not covered by tests. I will add an E2E test once it makes sense.
Release notes
Adds a new field to the
Ordertype. This is a backwards-compatible change.