Skip to content
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

Create uni_v2_amm_lp_and_tvl_4047194 #16

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Conversation

olgafetisova
Copy link
Contributor

Uni v2 pool is found based on the cow amm pool composition (the user inputs cow amm pool and the query finds an equivalent for uni pool).

Lp token supply is calculated based on the in and out transfer amounts on the token (pool address).

Then the query calculates token reserves and finds the latest daily price to calculate TVL.

Uni v2 pool is found based on the cow amm pool composition (the user inputs cow amm pool and the query finds an equivalent for uni pool).

Lp token supply is calculated based on the in and out transfer amounts on the token (pool address).

Then the query calculates token reserves and finds the latest daily price to calculate TVL.
limit 1
)

, get_lp_balance as (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe rename this a bit. If i understand correctly, this computes how many LP tokens were minted and burned on a given day, right? So maybe something very literal as lp_token_mint_burn_per_day

SELECT *, MAX(sync_count_in_day) OVER (PARTITION BY date_trunc('day', evt_block_time)) AS max_sync_count_in_day
FROM pair_sync_with_counts
)
,v2_pair_data AS (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh it is not clear to me what info we are trying to recover in lines 41-51 that we don't already have.

r.contract_address
, r.token0
, r.token1
, month as day
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, this also looks a bit odd.

UNION ALL
SELECT token1 AS token_address FROM v2_pair_data
)
,v2_reserves_by_day AS (
Copy link
Contributor

@harisang harisang Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to compute this, we could also do something similar to what you do for LP tokens, i.e., track the transfers in and out of the pool, right?

I wonder also whether there are cases of random transfers to the pool that are not proper liquidity injections (not sure why anyone would do that though), in which case probably there is no Sync() event emitted.

, r.token0
, r.token1
, month as day
, total_lp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah the names here, total_lp and lp_supply are a bit confusing to me.

Copy link
Contributor

@fhenneke fhenneke Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: Just saw the comment by Felix on the other PR. Could you rename to have an ".sql" at the end? There is no syntax highlighting and not linting for the current file name.

WITH
cow_amm_pool as (
select blockchain, address, token_1_address, token_2_address
from query_3959044 q
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That query should also be added to the repo at some point.

Copy link
Contributor

@fleupold fleupold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, please provide good comments on how and what is being done. I think this query can be simplified quite a bit and should have the same output format as the cow one

cow_amm_pool as (
select blockchain, address, token_1_address, token_2_address
from query_3959044 q
where address={{cow_amm_pool}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is the right input for the comparison? This way they can only be used for CoW AMM pools (where in theory profitability could be computed for any AMM regardless whether there exists a CoW AMM)\

Maybe we should have all queries be parametered on token1, token2 and then put the search for CoW AMMs in the top query that calls the subqueries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought of adding token_1 and token_2 parameters, the issue see here is how to know which token is 1 and which one is 2? I think there is an alphabetical order but does it always stand? Should we allow token symbol inputs or raw tokens (more accurate)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Def. addresses and not symbols. As to ordering, if we cannot make it ordering agnostic you can also follow @fhenneke's suggestion of using the uniswap pool as an argument. Note, that this query is not designed to be used by itself but instead will be embedded in a simple visualisation query which can make sure the input parameter is well defined (e.g. by choosing it from a hard coded list or the like).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this more, I believe the input should be two tokens. This way each "$10k growth" chart is independent of cow amms (e.g. we could still compare uni, balancer and a counterfactual rebalancing portfolio even if no CoW AMM existed).

AND contract_address IN (SELECT token_address FROM token_address_list)
)

SELECT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to have the same columns as the cow table so they can be put together in the top level query.

ORDER BY evt_block_time DESC
)
,pair_sync_last_in_day AS (
SELECT *, MAX(sync_count_in_day) OVER (PARTITION BY date_trunc('day', evt_block_time)) AS max_sync_count_in_day
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you simply order DESC when partition above and then use rank 1?

, pair_sync_with_counts AS (
SELECT *
,ROW_NUMBER() OVER (PARTITION BY date_trunc('day', evt_block_time) ORDER BY evt_block_time ASC ) AS sync_count_in_day
FROM uniswap_v2_ethereum.Pair_evt_Sync
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good idea to use this event!


,prices AS (
SELECT day, contract_address,symbol, price_close as price
FROM prices.usd_daily
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can get the decimals from here.

s.contract_address
,pc.token0
,pc.token1
,date_trunc('day',s.evt_block_time) AS month
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very misleading naming, calling a day month

cow_amm_pool as (
select blockchain, address, token_1_address, token_2_address
from query_3959044 q
where address={{cow_amm_pool}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected the query to take a univ2 pool address as a parameter instead.

The midterm goal is to have a query per amm type with a standardized result. The result can be used for comparison with a cow amm but need not be.

There can be a "UI" query where you choose a cow amm and it automatically finds the univ2 pool for a comparison. But for this base query I would go for simplicity.

)

, total_lp as (
select day, lp_supply, sum(lp_supply) over (order by day) as total_lp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens here if a day passes without any mint or burn? We would have a gap in the data even though trading may have happened?


, get_lp_balance as (
select date(evt_block_time) as day,
sum(case when "from" = 0x0000000000000000000000000000000000000000 THEN (value/1e18) ELSE -(value/1e18) END) as lp_supply
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dividing by 1e18 here leads to float arithmetic rounding errors downstream, which make it hard to verify the output data is exactly correct (by checking with on chain values)

cow_amm_pool as (
select blockchain, address, token_1_address, token_2_address
from query_3959044 q
where address={{cow_amm_pool}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this more, I believe the input should be two tokens. This way each "$10k growth" chart is independent of cow amms (e.g. we could still compare uni, balancer and a counterfactual rebalancing portfolio even if no CoW AMM existed).

@fleupold
Copy link
Contributor

fleupold commented Sep 8, 2024

@olgafetisova can you please take a look at the changes I made and incorporate those into #17?

@harisang harisang requested a review from fhenneke September 9, 2024 07:24
Comment on lines +53 to +70
lp_total_supply as (
select
day,
total_lp
from (
-- join full date range with potentially incomplete data. This results in many rows per day (all total supplies on or before that day)
-- rank() is then used to order join candidates by recency (rank = 1 is the latest lp supply)
select
date_range.day,
total_lp,
rank() over (partition by (date_range.day) order by lp.day desc) as latest
from date_range
inner join lp_total_supply_incomplete as lp
on date_range.day >= lp.day
-- performance optimisation: this assumes one week prior to start there was at least one lp supply change event
and lp.day >= (timestamp '{{start}}' - interval '7' day)
)
where latest = 1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure it works as intended, is it supposed to take the previous available value if the data is missing for a certain date?

Something like that ->

SELECT
        date_range.day,
        COALESCE(v.total_lp, LAG(v.total_lp) OVER (ORDER BY date_range.day)) AS total_lp
    FROM
        date_range
    LEFT JOIN
        total_lp v ON date_range.day = v.day

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it supposed to take the previous available value if the data is missing for a certain date?

Yes exactly. Can you elaborate why you think this may not be working?

Wouldn't we in your suggestion still miss a record if there are missing values for two days in a row (which is the case for less frequently traded pairs)? Cf minimal example query: https://dune.com/queries/4055768

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous approach did not work for me initially, but after rerunning it again it seems to be fine.
Initially, if I selected the date with the missing values, it would not find any corresponding previous values, but I cannot replicate it anymore, so all fine from my size now. Adjusted my query as well.

@olgafetisova olgafetisova merged commit 2e01f9d into main Sep 10, 2024
1 check passed
@olgafetisova olgafetisova deleted the olgafetisova-patch-1 branch September 10, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants