search

duneanalytics / spellbook

SQL views for Dune
Other
1.19k stars 1.13k forks source link

[ENHANCEMENT] Make nft.mints support every chain (native mints) #4578

Open MSilb7 opened 1 year ago

MSilb7 commented 1 year ago

Description

NFT Mints only has data for L1 Ethereuma nd L2 Optimism. This should support every EVM Chain (for native mints):(https://github.com/duneanalytics/spellbook/blob/64a15ffad5c5724d02751cc03f334af9be0ba034/models/_sector/nft/mints/nft_mints.sql#L18)

How can we best enable this? New folders for every chain? A Macro for identifying mints + a macro for every chain name (so that it can natively support new chains)? evms view? Something else?

0xRobin commented 1 year ago

A Macro for identifying mints + a macro for every chain name (so that it can natively support new chains)?

I've not yet found a way to make the 2 level macro setup work.. :(

Designing a native_mints() macro and then one model using the macro for each chain is the way to go. No need for new folders, these models can all be placed in models/_sector/nft/mints/native

MSilb7 commented 1 year ago

I've not yet found a way to make the 2 level macro setup work.. :(

By this you mean?:

Yeah, i had builds fail in runtime with all of the union alls. If there was a way to better "iterate" in dbt, this could maybe be feasible.

One concern is the overhead to "remember" every spell that needs a new chain integrated once it is added to Dune (i.e. how do we avoid a situation where all but a few chains are included).

0xRobin commented 1 year ago

how do we avoid a situation where all but a few chains are included

There are a couple small improvements we could do to better monitor/track this:

  1. A sector spell overview dashboard so you can quickly see which chains are integrated in which sector spells and can spot missing ones.
  2. For spells that you would always want to have data from all chains, we could add a generic test that checks the blockchain column of these spells. Then when a new chain is added you can open a PR to add the chain to a reference list in this generic test and the CI will fail on all the models that are still missing that chain.
jeff-dude commented 1 year ago

are we taking any action on this one?

0xRobin commented 1 year ago

It is definitely on my backlog to take a closer look at nft.mints, but it's not "planned in" yet.