-
Notifications
You must be signed in to change notification settings - Fork 152
Introduce confirmed_valid_to column in orders table #4047
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
base: main
Are you sure you want to change the base?
Conversation
Ethflow orders might have different validity than what user has signed submitted, resulting in the need of additional table to store this information. The need for joins slows down our queries getting all live orders. The new column `confirmed_valid_to` allows to store all needed data in the same table, treating normal and ethflow orders uniformly.
|
Reminder: Please update the DB Readme and comment whether migrations are reversible (include rollback scripts if applicable).
Caused by: |
database/sql/V096__create_index.sql
Outdated
| @@ -0,0 +1,4 @@ | |||
| CREATE INDEX orders_owner_live_limit | |||
| ON orders USING btree (owner, confirmed_valid_to) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the index only has to have confirmed_valid_to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need two indexes then? Because the user_orders_with_quote:
WITH live_orders AS (
SELECT o.*
FROM orders o
LEFT JOIN ethflow_orders e ON e.uid = o.uid
WHERE o.cancellation_timestamp IS NULL
AND o.valid_to >= $1
AND (e.valid_to IS NULL OR e.valid_to >= $1)
AND NOT EXISTS (SELECT 1 FROM invalidations i WHERE i.order_uid = o.uid)
AND NOT EXISTS (SELECT 1 FROM onchain_order_invalidations oi WHERE oi.uid = o.uid)
AND NOT EXISTS (SELECT 1 FROM onchain_placed_orders op WHERE op.uid = o.uid
AND op.placement_error IS NOT NULL)
AND o.owner = $2
AND o.class = 'limit'
),
trades_agg AS (
SELECT t.order_uid,
SUM(t.buy_amount) AS sum_buy,
SUM(t.sell_amount) AS sum_sell,
SUM(t.fee_amount) AS sum_fee
FROM trades t
JOIN live_orders lo ON lo.uid = t.order_uid
GROUP BY t.order_uid
)
SELECT
o_quotes.sell_amount as quote_sell_amount,
lo.sell_amount as order_sell_amount,
o_quotes.buy_amount as quote_buy_amount,
lo.buy_amount as order_buy_amount,
lo.kind as order_kind,
o_quotes.gas_amount as quote_gas_amount,
o_quotes.gas_price as quote_gas_price,
o_quotes.sell_token_price as quote_sell_token_price
FROM live_orders lo
LEFT JOIN trades_agg ta ON ta.order_uid = lo.uid
INNER JOIN order_quotes o_quotes ON lo.uid = o_quotes.order_uid
WHERE ((lo.kind = 'sell' AND COALESCE(ta.sum_sell,0) < lo.sell_amount) OR
(lo.kind = 'buy' AND COALESCE(ta.sum_buy ,0) < lo.buy_amount))still has to select the owner of the order, although with limited amount of live orders (valid_to >= timestamp) it can still be pretty cheap.
On the contrary, the solvable orders query does not need owner at all
WITH live_orders AS (
SELECT o.*
FROM orders o
LEFT JOIN ethflow_orders e ON e.uid = o.uid
WHERE o.cancellation_timestamp IS NULL
AND o.valid_to >= $1
AND (e.valid_to IS NULL OR e.valid_to >= $1)
AND NOT EXISTS (SELECT 1 FROM invalidations i WHERE i.order_uid = o.uid)
AND NOT EXISTS (SELECT 1 FROM onchain_order_invalidations oi WHERE oi.uid = o.uid)
AND NOT EXISTS (SELECT 1 FROM onchain_placed_orders op WHERE op.uid = o.uid
AND op.placement_error IS NOT NULL)
),
trades_agg AS (
SELECT t.order_uid,
SUM(t.buy_amount) AS sum_buy,
SUM(t.sell_amount) AS sum_sell,
SUM(t.fee_amount) AS sum_fee
FROM trades t
JOIN live_orders lo ON lo.uid = t.order_uid
GROUP BY t.order_uid
)
SELECT
lo.uid,
lo.owner,
lo.creation_timestamp,
lo.sell_token,
lo.buy_token,
lo.sell_amount,
lo.buy_amount,
lo.valid_to,
lo.app_data,
lo.fee_amount,
lo.kind,
lo.partially_fillable,
lo.signature,
lo.receiver,
lo.signing_scheme,
lo.settlement_contract,
lo.sell_token_balance,
lo.buy_token_balance,
lo.class,
lo.confirmed_valid_to,
COALESCE(ta.sum_buy, 0) AS sum_buy,
COALESCE(ta.sum_sell, 0) AS sum_sell,
COALESCE(ta.sum_fee, 0) AS sum_fee,
false AS invalidated,
(lo.signing_scheme = 'presign' AND COALESCE(pe.unsigned, TRUE)) AS presignature_pending,
ARRAY(
SELECT (p.target, p.value, p.data)
FROM interactions p
WHERE p.order_uid = lo.uid AND p.execution = 'pre'
ORDER BY p.index
) AS pre_interactions,
ARRAY(
SELECT (p.target, p.value, p.data)
FROM interactions p
WHERE p.order_uid = lo.uid AND p.execution = 'post'
ORDER BY p.index
) AS post_interactions,
ed.ethflow_data,
opo.onchain_user,
NULL AS onchain_placement_error,
COALESCE(fee_agg.executed_fee,0) AS executed_fee,
COALESCE(fee_agg.executed_fee_token, lo.sell_token) AS executed_fee_token,
ad.full_app_data
FROM live_orders lo
LEFT JOIN LATERAL (
SELECT NOT signed AS unsigned
FROM presignature_events
WHERE order_uid = lo.uid
ORDER BY block_number DESC, log_index DESC
LIMIT 1
) pe ON TRUE
LEFT JOIN LATERAL (
SELECT sender AS onchain_user
FROM onchain_placed_orders
WHERE uid = lo.uid
ORDER BY block_number DESC
LIMIT 1
) opo ON TRUE
LEFT JOIN LATERAL (
SELECT ROW(tx_hash, eo.valid_to) AS ethflow_data
FROM ethflow_orders eo
LEFT JOIN ethflow_refunds r ON r.order_uid = eo.uid
WHERE eo.uid = lo.uid
) ed ON TRUE
LEFT JOIN LATERAL (
SELECT SUM(executed_fee) AS executed_fee,
(ARRAY_AGG(executed_fee_token))[1] AS executed_fee_token
FROM order_execution
WHERE order_uid = lo.uid
) fee_agg ON TRUE
LEFT JOIN app_data ad ON ad.contract_app_data = lo.app_data
LEFT JOIN trades_agg ta ON ta.order_uid = lo.uid
WHERE ((lo.kind = 'sell' AND COALESCE(ta.sum_sell,0) < lo.sell_amount) OR
(lo.kind = 'buy' AND COALESCE(ta.sum_buy ,0) < lo.buy_amount))
database/README.md
Outdated
| buy\_token\_balance | [enum](#buytokendestination) | not null | defined how buy\_tokens need to be transferred back to the user | ||
| class | [enum](#orderclass) | not null | determines which special trade semantics will apply to the execution of this order | ||
|
|
||
| confirmed_valid_to | timestamptz | not null | order validity as returned by the settlement contract. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smart contract does not return this timestamp. It's more like this is the timestamp at which this order is no longer executable. Probably makes sense to mention the ethflow implementation details because those caused the whole mess.
| protocol_fees, | ||
| created: u32::try_from(order.metadata.creation_date.timestamp()).unwrap_or(u32::MIN), | ||
| valid_to: order.data.valid_to, | ||
| confirmed_valid_to: order.data.confirmed_valid_to, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need the new field inside the rust structs. This column only exists to speed up filtering but we should still return the same data from the DB queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we still need to populate it when parsing the Settlement contract's events right?
|
Thanks @MartinquaXD for the input. I will address the comments. |
Only the database interfacing confirmed_valid_to is preserved.
Description
Ethflow orders might have different validity than what user has signed submitted, resulting in the need of additional table to store this information. The need for joins slows down our queries getting all live orders.
The new column
confirmed_valid_toallows to store all needed data in the same table, treating normal and ethflow orders uniformly.Changes
confirmed_valid_tocolumn toorderstablebtree(confirmed_valid_to, owner)index on orders tableconfirmed_valid_toparsed from settlement contract's event datavalidTouniformlyAs this PR introduces new column and makes use of it, it will be split into 2 for backwards-compatible deployment:
confirmed_valid_toas the event-parsedvalidTo. Old autopilot in the meantime would still not populate those values.How to test
TBD
Testing will happen on a custom db spun up by @MartinquaXD, to compare the relative performance of the old query and the new one against the new column
Related Issues
Aims to resolve high latency on create_orders POST request and solvable_orders query.