Skip to content
This repository has been archived by the owner on Apr 28, 2022. It is now read-only.

Postgres: Appropriate Handling of Multiple Settlements in Single Block #1214

Closed
bh2smith opened this issue Oct 7, 2021 · 3 comments
Closed
Labels

Comments

@bh2smith
Copy link
Contributor

bh2smith commented Oct 7, 2021

At this moment some of our queries are fragile to the fact that, technically, multiple settlements could happen in a single block. For example, the recent query for fetching orders by transaction hash (introduced in #1194) whose raw SQL query assumes block number is unique per settlement.

We have included a todo with a rough sketch of how to handle this which I will also paste here.

with target_block_number as (
    SELECT block_number from settlements where tx_hash = $1
),

with next_log_index as (
    SELECT log_index from settlements
    WHERE block_number > target_block_number
    ORDER BY block_number asc
    LIMIT 1
)

"SELECT ", ORDERS_SELECT,
"FROM ", ORDERS_FROM,
"JOIN trades t \
    ON t.order_uid = o.uid \
 JOIN settlements s \
    ON s.block_number = t.block_number \
 WHERE s.tx_hash = $1 "
 AND t.log_index BETWEEN s.log_index AND next_log_index,

Up until this moment, we do not yet have a single instance of multiple settlements in a single block (cf Settlements Table) so this issue is not of immediate concern. However, with the introduction of multiple solver accounts (#1010) we may see this sooner than expected.

@nlordell
Copy link
Contributor

nlordell commented Mar 5, 2022

Looks like this is actually happening in the while and found by our FE team.

Specifically these two transactions:

Are on the same block and return the same list of orders.

An alternative to the solution proposed above would be to:

  • Add a tx_hash field to our trades table
  • Add indexes on tx_hash on both our settlements and trades tables
  • Modify the query to s.tx_hash = t.tx_hash

@bh2smith
Copy link
Contributor Author

bh2smith commented Mar 6, 2022

Seems like a reasonable alternative. The only reason we we didn't include it before is because it was considered redundant (which is no longer the case). I'm surprised the same order were included in two separate settlements... were they partially fillable orders?

It will be a bit of a task to populate all the existing records with their corresponding transaction hash, but at least we already have the values!

@nlordell
Copy link
Contributor

nlordell commented Mar 7, 2022

I'm surprised the same order were included in two separate settlements... were they partially fillable orders?

Orders weren't included in different settlements, there were just two settlements on the same block which caused the query to return incorrect data (i.e. the same list of orders for two separate transactions).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants