Track trading times for user and liquidity orders separately

[MartinquaXD]

The introduction of more liquidity orders made the "Average Trade Time" panel less meaningful since we don't really care about taking forever to match liquidity orders.
Now we only update those metrics with settled user orders to better gauge what the actual UX is like.

Test Plan

CI

Edit after merge: I confused order matching quota with time to trade. 🤦‍♂️
This PR actually only updated the matching quota. Time to trade gets covered in this PR
I left the incorrect original title of this PR to stay consistent with the title of the commit referencing this PR.

[sunce86]

Makes sense.

[nlordell]

Ah, OK. So the solution is easy.

[nlordell]

Nice! Happy there is a simple solution.

That being said, I would request a change where we have metrics for both user orders and liquidity orders (so we can keep track of how many those trade as well). Since we pass in the full order to order_settled we can just differentiate on is_liquidity_order.

[MartinquaXD]

Since we pass in the full order to order_settled we can just differentiate on is_liquidity_order.

It's not as easy as that. Only LimitOrder has the field is_liquidity_order but we are passing in Orders. Since you already suggested to store is_liquidity_order in the OrderMetadata maybe we should start with that and put this PR on hold until then? But that change would also affect the database.
Otherwise I can easily get the information whether an order is a liquidity order elsewhere and track their metrics separately.

[nlordell]

maybe we should start with that and put this PR

This would be my suggestion. The other advantages of this are:

• You know which are liquidity orders (which have different semantics for objective value computation than regular orders) when retrieving the orderbook.
• You don't need to duplicate the --liquidity-order-owner configuration as we do now.
• The "liquidity order status" of an order correctly survives across restarts.

Not sure how others feel about this though.

[vkgnosis]

It makes sense to me. If we do this we should come up with a good textual description of what it means to be a liquidity order since that will be part of the public api.

[MartinquaXD]

I'm all for it. 👍

[MartinquaXD]

Since OrderMetadata now holds is_liquidity_order it's way easier to track metrics for user and liquidity orders separately.
The only unfortunate thing is that it's still not trivial to track partially fillable orders. A partially fillable order gets created but can be involved in many trades so we need to be careful to not over count them.
The new implementation only considers a partially order as "matched" if it has been filled completely.

[codecov-commenter]

Codecov Report

Merging #172 (a17ae91) into main (c346f3a) will decrease coverage by 0.28%.
The diff coverage is 18.75%.

@@            Coverage Diff             @@
##             main     #172      +/-   ##
==========================================
- Coverage   64.71%   64.42%   -0.29%     
==========================================
  Files         190      189       -1     
  Lines       39045    38772     -273     
==========================================
- Hits        25268    24980     -288     
- Misses      13777    13792      +15     

[nlordell]

LGTM.

I think there is one tiny edge case. Approving as either I am mistaken about the edge case or assume it will be resolved.