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

Update/create stablecoin token sets. #6186

Closed
wants to merge 22 commits into from

Conversation

thetroyharris
Copy link
Contributor

Thank you for contributing to Spellbook 🪄

Contribution type

Please check the type of contribution this pull request is for:

  • New spell(s)
  • Adding to existing spell lineage
  • Bug fix

Note: You can safely discard any section below which doesn't apply based on selection above


Adding stablecoins to existing chains and creating the necessary files for chains that do not currently have them.

@dune-eng
Copy link

Workflow run id 9539966168 approved.

@dune-eng
Copy link

Workflow run id 9539966255 approved.

@dune-eng
Copy link

Workflow run id 9540079571 approved.

@dune-eng
Copy link

Workflow run id 9540079664 approved.

@dune-eng
Copy link

Workflow run id 9540106635 approved.

@dune-eng
Copy link

Workflow run id 9540106704 approved.

@dune-eng
Copy link

Workflow run id 9540155140 approved.

@dune-eng
Copy link

Workflow run id 9540155251 approved.

@thetroyharris
Copy link
Contributor Author

Ready for review @jeff-dude @Hosuke

Copy link
Collaborator

@Hosuke Hosuke left a comment

Choose a reason for hiding this comment

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

Do you want to add a roll-up spell tokens.erc20_stablecoins view?

@Hosuke Hosuke self-assigned this Jun 17, 2024
@dune-eng
Copy link

Workflow run id 9548623561 approved.

@dune-eng
Copy link

Workflow run id 9548623719 approved.

@dune-eng
Copy link

Workflow run id 9548927496 approved.

@dune-eng
Copy link

Workflow run id 9548927852 approved.

@dune-eng
Copy link

Workflow run id 9548947784 approved.

@dune-eng
Copy link

Workflow run id 9548947962 approved.

@dune-eng
Copy link

Workflow run id 9549083108 approved.

@dune-eng
Copy link

Workflow run id 9549083328 approved.

@thetroyharris
Copy link
Contributor Author

thetroyharris commented Jun 17, 2024

Do you want to add a roll-up spell tokens.erc20_stablecoins view?

Sure. Just created it!
If this PR could please get merged today that would be lovely as my next PR depends on these spells existing! :D

@thetroyharris thetroyharris requested a review from Hosuke June 17, 2024 16:30
@dune-eng
Copy link

Workflow run id 9568957371 approved.

@dune-eng
Copy link

Workflow run id 9569018154 approved.

@dune-eng
Copy link

Workflow run id 9569018478 approved.

@dune-eng
Copy link

Workflow run id 9569106194 approved.

@dune-eng
Copy link

Workflow run id 9569106422 approved.

@dune-eng
Copy link

Workflow run id 9569174894 approved.

@dune-eng
Copy link

Workflow run id 9569175384 approved.

@dune-eng
Copy link

Workflow run id 9569260988 approved.

@dune-eng
Copy link

Workflow run id 9569261071 approved.

@dune-eng
Copy link

Workflow run id 9569309033 approved.

@dune-eng
Copy link

Workflow run id 9569309225 approved.

@dune-eng
Copy link

Workflow run id 9569461879 approved.

@dune-eng
Copy link

Workflow run id 9569462151 approved.

@thetroyharris
Copy link
Contributor Author

@jeff-dude

I have changed this into a view. I removed the stablecoins tables from Fantom and Ethereum as they did not contain much more information than the erc20 table. Conversely, Optimism's stablecoin table contains much more information so that remains as is. Eventually, I would like my stablecoin view to contain that information but that task is not a priority for me at the moment.

This view is just laying the groundwork for the next PR.

@thetroyharris
Copy link
Contributor Author

Also, what do I have to do to run the dbt-run and dbt-test tests locally? Could you give me example of how to do it for this PR?

@Hosuke
Copy link
Collaborator

Hosuke commented Jun 19, 2024

Also, what do I have to do to run the dbt-run and dbt-test tests locally? Could you give me example of how to do it for this PR?

Currently you can only run dbt compile locally to check syntax/compilation error.

@thetroyharris
Copy link
Contributor Author

Currently you can only run dbt compile locally to check syntax/compilation error.

Ok ya I do that, it just doesn't catch the same errors as the tests here.

@@ -79,4 +79,9 @@ FROM (VALUES
, (0x323665443cef804a3b5206103304bd4872ea4253, 'USDV', 6)
, (0x966570a84709d693463cdd69dcadb0121b2c9d26, 'taoUSD', 18)
, (0x993614e1c8c9c5abe49462ce5702431978fd767f, 'S*ETH', 18)
, (0xaf88d065e77c8cc2239327c5edb3a432268e5831, 'USDC', 6) -- Native
Copy link
Member

Choose a reason for hiding this comment

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

i'm still a bit confused on this approach here. these stablecoins already exist in tokens.erc20:

select
  *
from
  tokens.erc20
where
  contract_address = 0xaf88d065e77c8cc2239327c5edb3a432268e5831

keep in mind that erc20 token metadata now combines an automated source + these static files to handle outlier, long tail tokens missing. i would validate all of these already exist in the table, then remove from this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

keep in mind that erc20 token metadata now combines an automated source

Wow did I ever jump the gun on this. This hurts my soul.

Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure we should delete these, as they may have downstream dependencies or get used by a few users/dashboards on dune?

Copy link
Member

Choose a reason for hiding this comment

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

similar to all other chains, i would check these already exist, but since this is a net new file it's worth a close look. we can remove the file altogether if they exist

)
}}

SELECT contract_address, symbol, decimals, 'arbitrum' AS blockchain
Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure we need to take the hardcoded values approach again. the ideal scenario is to read from tokens.erc20 and filter on specific tokens. maybe that's a jinja list at top of model of address/blockchain, then filter the main tokens table on the list via a for loop?

Copy link
Contributor Author

@thetroyharris thetroyharris Jun 19, 2024

Choose a reason for hiding this comment

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

What is the best approach for me to create a spell that I can use to filter token transfers for contracts that are stablecoins?

The filtered table will then itself be a spell. Trying to do all of this in a query takes to long.

One way or another I have to manually keep a list of contract address that are stablecoins.

Copy link
Member

Choose a reason for hiding this comment

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

yes, agree to build an intermediate spell that contains only stablecoins. my ask is that we read from the source tokens.erc20 and filter that down to stablecoins in this new spell, rather than start from scratch on a hardcoded list. that way we at least keep consistent on symbol/decimals.

you can get fancy with a jinja list variable and loop through that for the filter on the source table, essentially saying:

where
    (blockchain = blockchain and address = address)
    OR (blockchain = blockchain and address = address)
    ...

Copy link
Member

Choose a reason for hiding this comment

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

that would be preferred approach. if the variable isn't working, you can also do the union all approach and filter on a blockchain/addresses per union'd query

Copy link
Member

Choose a reason for hiding this comment

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

i would expect the files modified in this PR to be much fewer to get this built

@jeff-dude jeff-dude added the WIP work in progress label Jun 19, 2024
@thetroyharris thetroyharris marked this pull request as draft June 20, 2024 01:55
@thetroyharris thetroyharris changed the title EASY Update/create stablecoin token sets. Update/create stablecoin token sets. Jun 20, 2024
@jeff-dude
Copy link
Member

closing due to inactivity. feel free to reopen and describe what needs to happen next, if necessary. thank you.

@jeff-dude jeff-dude closed this Aug 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in review Assignee is currently reviewing the PR WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants