-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
lido liquidity updates #6033
lido liquidity updates #6033
Conversation
Workflow run id 9299895516 approved. |
Workflow run id 9299895747 approved. |
Workflow run id 9300083408 approved. |
Workflow run id 9300083604 approved. |
Workflow run id 9302723579 approved. |
Workflow run id 9302723858 approved. |
Workflow run id 9303700350 approved. |
Workflow run id 9303700450 approved. |
…he/spellbook into lido-new-liquidity-pools
Workflow run id 9304014045 approved. |
Workflow run id 9304014509 approved. |
Workflow run id 9304558124 approved. |
Workflow run id 9304557874 approved. |
Workflow run id 9305697366 approved. |
Workflow run id 9305697616 approved. |
Workflow run id 9306048671 approved. |
Workflow run id 9306048835 approved. |
Workflow run id 9315992299 approved. |
Workflow run id 9315992662 approved. |
Workflow run id 9316220259 approved. |
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.
Hi @ppclunghe,
we can apply the new format for post_hooks in this PR.
Also I think we may use ||
to replace concat
to shorten the long line.
post_hook='{{ expose_spells(\'["base"]\', | ||
"project", | ||
"lido_liquidity", | ||
\'["pipistrella"]\') }}' |
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.
post_hook='{{ expose_spells(\'["base"]\', | |
"project", | |
"lido_liquidity", | |
\'["pipistrella"]\') }}' | |
post_hook='{{ expose_spells(blockchains = \'["base"]\', | |
spell_type = "project", | |
spell_name = "lido_liquidity", | |
contributors = \'["pipistrella"]\') }}' |
{% if not is_incremental() %} | ||
WHERE DATE_TRUNC('day', s.evt_block_time) >= DATE '{{ project_start_date }}' | ||
{% else %} | ||
WHERE DATE_TRUNC('day', s.evt_block_time) >= DATE_TRUNC('day', NOW() - INTERVAL '1' day) |
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 can use incremental predicate for incremental filters:
WHERE DATE_TRUNC('day', s.evt_block_time) >= DATE_TRUNC('day', NOW() - INTERVAL '1' day) | |
WHERE {{ incremental_predicate('s.evt_block_time') }} |
) | ||
|
||
|
||
select CONCAT(CONCAT(CONCAT(CONCAT(CONCAT(blockchain,CONCAT(' ', project)) ,' '), coalesce(paired_token_symbol,'unknown')),':') , main_token_symbol, ' ', pool_type, ' ', format('%,.3f',round(coalesce(fee,0),4))) as pool_name, |
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 am not sure concat
can be replaced by ||
here.
post_hook='{{ expose_spells(\'["scroll"]\', | ||
"project", | ||
"lido_liquidity", | ||
\'["pipistrella"]\') }}' |
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.
Same as above post_hook.
post_hook='{{ expose_spells(\'["scroll"]\', | ||
"project", | ||
"lido_liquidity", | ||
\'["pipistrella"]\') }}' |
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.
Same as above.
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.
Thanks a lot for your review, @Hosuke
I have updated code according to your comments
Workflow run id 9387920600 approved. |
Workflow run id 9387920945 approved. |
Workflow run id 9389266383 approved. |
Workflow run id 9389266254 approved. |
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.
LGTM.✅
Thank you @ppclunghe
Workflow run id 9447607654 approved. |
Workflow run id 9447607884 approved. |
I have added a all fix for SyncSwap wstETH pools on zksync, sorry for it, all comments from @Hosuke were taken into account for this fix |
Workflow run id 9448574961 approved. |
Workflow run id 9448575259 approved. |
models/lido/liquidity/zksync/lido_liquidity_zksync_syncswap_v2_pools.sql
Outdated
Show resolved
Hide resolved
@@ -63,7 +63,7 @@ with | |||
{% if not is_incremental() %} | |||
WHERE DATE_TRUNC('day', p.minute) >= DATE '{{ project_start_date }}' | |||
{% else %} | |||
WHERE DATE_TRUNC('day', p.minute) >= DATE_TRUNC('day', NOW() - INTERVAL '1' day) | |||
WHERE {{ incremental_predicate('p.minute') }} |
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.
similar to the changes made here to use incremental predicate on source reads, we also leverage them for filtering the target data. this allows the generated merge statement to filter both to same time period and speed up processing. i won't consider a blocker for now, but it'd be nice to see a follow up PR soon that implements on all lido models. looks like this:
https://github.com/duneanalytics/spellbook/blob/main/models/_sector/dex/trades/arbitrum/platforms/airswap_arbitrum_base_trades.sql#L9
be sure the column name passed in matches target spell column name, sometimes it changes from source to target
Adding new wstETH pools to Lido liquidity spell, update for Uniswap wstETH pools spell on Base