Refactor slippage query #31

fhenneke · 2024-09-16T12:13:03Z

This PR refactors the slippage query. It separates different parts of the slippage computation, simplifies debugging unusual slippage, and prepares for moving some of the slippage computation offline.

Query layout

The general layout of the query is as follows (dashed lines and rectangles correspond to queries which do not yet exist):

There are three mostly independent queries on raw token imbalances, fees, and prices. Those correspond to tables we plan to have available offline.
- Raw imbalances are computed from all balance changes. They are observable on chain and can be computed on Dune for all chains we support.
- Fees are computed from offchain information we upload. Fees are classified into 'protocol_fee' and 'network_fee'. Protocol fees could in future PRs be separated into protocol fees and partner fees, and, combined with other prices, could give totals for protocol and parner fees.
- Prices are hourly averages. If Dune prices are available, those prices are used. If Dune prices are not available, prices are reconstructed from trades involving that token with another token which does have a Dune price. Only if that is not possible, the price is null.
The results of those three queries are combined into a raw slippage query. It has raw imbalances and explicit entries for fees, as well as aggregated numbers per transaction. This table is useful for detecting wrong prices and wrong imbalances, as well as having an unfiltered view of slippage per transaction.
The result of the previous query is filtered using excluded batches and aggregated pe solver. This query can be used to detect cases of unusual slippage and also gives totals used for payments. Solvers can use that query and filter on their solver address to detect large slippage settlements per solver.
Since the format of the result is not exactly what the solver-rewards script expects, there is another query, an updated old slippage query, which adds some meta information and renames columns. The naming convention in solver rewards should be changed and then this query can be removed.

The changes to the main slippage query can be observed here.

Except for the empty fee table, all of the queries should work on other chains as well. Missing fee data currently means that network and protocol fees are treated as slippage, which is fine for network fees but incorrect for protocol fees.

Differences to old slippage query

This is mostly but not quite a replacement of the old slippage query. To exceptions:

The old query computed prices from exchange rates of the auction. It did so, however, in a way which broke on a lot of cases due to missing data on decimals. The new query works with prices per atom which do not require decimals and can thus recover more token prices from trades.
The old query for slippage per transaction (but not the one in the repo at the moment, due to a bug) had a parameter for filtering on solver address. This is removed for simplicity. Instead one can filter the final table in the web interface.

Fix of balances changes query

Working on slippage revealed an issue with the balance changes query: unlike on mainnet, it is not required on Gnosis to have special treatment for sDAI. All balance changes are accompanied by transfer events. Looking at deposit and withdraw events is not required.

~~The change is also independently addresses in #41.~~

(Updated with modified design.)

this file should have all information required for debugging unusual silppage

for compatibility with the old query

cowprotocol/accounting/fees/classified_fees_4058574.sql

cowprotocol/accounting/slippage/slippage_breakdown_4059683.sql

cowprotocol/accounting/fees/classified_fees_4058574.sql

cowprotocol/accounting/slippage/slippage_breakdown_4059683.sql

cowprotocol/accounting/slippage/slippage_per_solver_4070065.sql

cowprotocol/accounting/slippage/slippage_per_transaction_4070059.sql

harisang · 2024-09-18T17:38:58Z

cowprotocol/accounting/slippage/slippage_per_transaction_4070059.sql

+-- Results of the query are filtered to not include batches from excluded_batches.
+-- Batches are also excluded if there is a non-zero imbalance and no value (in native atomes).
+
+with excluded_batches as (


I think this should be moved to the slippage_per_solver query.

One goal was to have a query per transaction which is exactly what we use for total slippage to base an unusual slippage query on.

If you think that is not a requirement and having a discrepancy there if fine, I can move excluded batches somewhere else.

Maybe the better distinction is to merge the three queries slippage breakdown, slippage per transaction, slippage per solver into

raw slippage (including breakdown and aggregation per transaction)

slippage (with filtering, per transaction and per solver)

The first query would then be for debugging and could also leave out solver names, the second would be compatible with actual payment.

harisang · 2024-09-18T17:51:12Z

cowprotocol/accounting/slippage/slippage_per_transaction_4070059.sql

+select
+    s.block_time,
+    s.tx_hash,
+    solver_address,


i guess solver_address could be skipped here as it is not really relevant to the main functionality of the query. And also causes this inner join with the batches table which looked a bit confusing at first.

It was useful for me to have a query with results per transaction and where I could filter on solver address. A cleaner way might be what I proposed in the response above.

harisang · 2024-09-18T17:51:42Z

cowprotocol/accounting/slippage/slippage_per_transaction_4070059.sql

+    on s.tx_hash = b.tx_hash
+where s.tx_hash not in (select tx_hash from excluded_batches)
+group by 1, 2, 3
+having bool_and(slippage_native_atom is not null or slippage_atoms = 0)


Can you explain how this bool_and works here? And why is the second condition in the parenthesis needed? Sorry for my ignorance

I also did not know this before I saw it being used in the old slippage query.
bool_and is an aggregating expression, similar to sum. it checks that all of the expressions (iterating over rows within a group, same as with sum), i.e. the logical and of all the entries.

Here it is used to check that for each entry in the slippage breakdown slippage_wei is not null, i.e., we have a value for slippage in wei due to a price being available, or slippage_atoms = 0, i.e., there is no slippage so we also do not need a price. This excludes all settlements where a price is missing for a token for which we need a price.

cowprotocol/accounting/slippage/slippage_prices_4064601.sql

- rename transfer_type to type - rename fee_data to raw_fee_data - reorder columns for consistency - fix some comments - rename price to price_unit - rename slippage_native_atom to slippage_wei

- rename type to fee_type or slippage_type - allow more intuitive ordering of columns

…59.sql Co-authored-by: Haris Angelidakis <64154020+harisang@users.noreply.github.com>

fhenneke · 2024-09-19T11:09:16Z

I joined the new and old slippage query to check differences, https://dune.com/queries/4083184.

In cases where both queries report a value, they are almost identical (~1e-10).
The new query returns a slippage value for more auctions. Looking at the largest and smallest slippage values indicates that the new query is more accurate.

positive slippage:
- https://etherscan.io/tx/0xb003b9cf3d448bfbbb6161519504cd9f761fde1cef0ca4e544ef7daa44d5a944
  $90 of slippage, price missing for 0xd39de8fcecb7f2e0c3deb198d8e33663afa0e977, which can be reconstructed from the auction
- https://etherscan.io/tx/0xdce056e27585d11304ed70282b677bf59f7c7648ee4c951bbcb7de70709b8d5b
  $30 of slippage, price missing for 0xb6f4d11d6c274f8f0a55d101b65223a45a3d1128, which can be reconstructed from the auction
negative slippage:
- https://etherscan.io/tx/0x4d38fe407beb4cef9651913f73adab65529df7daf0d968256e48fe453667d262
  -$30 of slippage, price missing for 0x88ce174c655b6d11210a069b2c106632dabdb068, which can be reconstructed from the auction
  amounts cancel almost exactly, but not quite (due to rounding in protocol fees?)
- https://etherscan.io/tx/0x7ca4094e1ae5a7d68c571ceecb247efcd66f1953cf7c81c533465dd847410f80
  -$30 of slippage, price missing for 0x777be1c6075c20184c4fd76344b7b0b7c858fe6b, which can be reconstructed from the auction. the price of $2.62 closely corresponded to the coingecko price $2.63 at the time of the auction

cowprotocol/accounting/fees/classified_fees_4058574.sql

harisang · 2024-09-20T09:01:06Z

cowprotocol/accounting/fees/classified_fees_4058574.sql

+    where block_time >= cast('{{start_time}}' as timestamp) and block_time < cast('{{end_time}}' as timestamp)
+),
+
+protocol_fee_balance_changes as (


The table name, just looking at this query, is a bit strange. Same for the next one

harisang · 2024-09-20T09:12:38Z

cowprotocol/accounting/slippage/raw_slippage_4059683.sql

+    select * from "query_4064601(blockchain='{{blockchain}}',start_time='{{start_time}}',end_time='{{end_time}}')"
+),
+
+raw_slippage_breakdown as (


what do we mean by "raw" here?

I want to differentiate between slippage values without much processing ("raw_slippage", for debugging) and slippage values after processing (just "slippage", what is really used later on).

Maybe "unfiltered" would be better. Or just slippage_breakdown and slippage_per_transaction and name the query "slippage_breakdown"?

Do you have a different naming suggestion?

harisang · 2024-09-20T13:51:36Z

Looks good!

Working on slippage revealed an issue with the balance changes query: unlike on mainnet, it is not required on Gnosis to have special treatment for sDAI. All balance changes are accompanied by transfer events. Looking at deposit and withdraw events is not required. The change is also independently addresses in #41.

Shouldn't with then remove that change from here?

fhenneke · 2024-09-20T13:54:28Z

Shouldn't` with then remove that change from here?

I will remove them now. They were useful for testing on Gnosis since otherwise wrong slippage would have dominated everything.

This reverts commit 9961053.

fhenneke added 11 commits September 16, 2024 10:41

remove sdai handling on gnosis

9961053

add query for classified fees

411b4d9

add query for slippage prices

fc0c21d

add slippage breakdown

23a2cfd

this file should have all information required for debugging unusual silppage

move slippage to its own folder

2d77815

add slippage per transaction and slippage per solver

3248e34

fix linting

6b4423d

fix small bug in grouping

e6eadee

change slippage to atoms of native token

aec07c1

for compatibility with the old query

add new slippage query, remove the old one

462c045

fix linting

cdf2b57

fhenneke requested a review from harisang September 16, 2024 12:13

harisang added the solver accounting label Sep 16, 2024

exclude settlements with missing prices

cf754bf