-
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
Refactor amount_usd calculation in paraswap.trades #6216
Conversation
Currently I keep avalanche_c implementation untouched. |
-- USDC.e AND USDT.e price are missing before 2022-10 | ||
price_missed_previous AS ( | ||
WITH usdc_price AS ( | ||
SELECT minute, contract_address, decimals, symbol, price | ||
FROM {{ source('prices', 'usd') }} | ||
WHERE contract_address = 0xa7d7079b0fead91f3e65f86e8915cb59c1a4c664 -- USDC.e |
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.
Removed these parts after testing:
https://dune.com/queries/3848201?category=abstraction&namespace=paraswap&id=paraswap.trades
Should we keep the |
post_hook='{{ expose_spells(blockchains = \'["arbitrum"]\', | ||
spell_type = "project", | ||
spell_name = "paraswap_v5", | ||
contributors = \'["springzhang"]\') }}' |
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 @jeff-dude,
should I remove the post_hook
in blockchain-version-level spells?
@@ -19,39 +19,72 @@ ref('paraswap_avalanche_c_trades') | |||
,ref('paraswap_base_trades') | |||
] %} | |||
|
|||
WITH basic_trades AS ( |
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 @jeff-dude,
should I use basic_trades
or simply trades
here for the CTE naming?
As I may add similar CTE across the spells, so I think we may have an unified naming standard.
Updated. |
this is last remaining "leftover" PR that will eventually get replaced with new one. i'm going to close this, it'll be there in closed for historical purposes. |
Linked issue: #6215
Thank you for contributing to Spellbook 🪄
Contribution type
Please check the type of contribution this pull request is for:
Note: You can safely discard any section below which doesn't apply based on selection above
For new spell(s)
If you are building new spell(s), please provide the following information:
For adding to existing spell lineage
If you are adding to an existing spell lineage, please provide the following information:
For bug fixes
If you are fixing a bug, please provide the following information:
Additional information
Please provide any additional information that might help us review your pull request:
Thank you for your contribution!