-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
balances upgrade + daily aggregation with forward fill #5464
Conversation
0xRobin
commented
Feb 28, 2024
•
edited
Loading
edited
- rename some of the columns in the balances schema
- addition of daily aggregation with forward fill
example query: https://dune.com/queries/3475840/5841282 |
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.
Looks great! had a couple of comments. Can you also update the description with a summary of the changes and PR name for posterity
-- TODO: should not be hardcoded | ||
WHEN type = 'native' THEN erc20_tokens.contract_address = 0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee | ||
WHEN token_standard = 'native' THEN erc20_tokens.contract_address = 0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee |
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.
This will not work for Polygon I think, we should pass this in instead.
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 have only changed the column name here. 🤔
What exactly should we pass in?
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.
We use this for polygon https://github.com/duneanalytics/spellbook/blob/main/tokens/models/tokens/polygon/tokens_polygon_erc20.sql#L14
Most other L2s use ETH, we could actually also just pass in the the symbol and the decimal (it seems to be 8 for BNB?)
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.
ah ok I'll add it to the schema fix macro, ideally we would fill these values already in the raw table (instead of the current null values)