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.

Sign up for GitHub

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

Jump to bottom

Pika Protocol v4 added for optimism #7427

Merged
merged 24 commits into from
Jan 29, 2025
Merged

Pika Protocol v4 added for optimism #7427

merged 24 commits into from
Jan 29, 2025

Conversation

PatelPrinci
Copy link
Contributor

Thank you for contributing to Spellbook 🪄

Please open the PR in draft and mark as ready when you want to request a review.

Description:

[...]

quick links for more information:

@github-actions github-actions bot marked this pull request as draft January 7, 2025 06:21
@github-actions github-actions bot added WIP work in progress dbt: hourly covers the hourly dbt subproject labels Jan 7, 2025
@PatelPrinci PatelPrinci marked this pull request as ready for review January 7, 2025 06:31
@github-actions github-actions bot added ready-for-review this PR development is complete, please review and removed WIP work in progress labels Jan 7, 2025
@0xRobin 0xRobin requested review from Hosuke and 0xRobin January 7, 2025 08:32
0xRobin
0xRobin reviewed Jan 7, 2025
View reviewed changes
...llbook/models/_sector/perpetual/projects/pika/optimism/pika_v4_optimism_perpetual_trades.sql Outdated
Comment on lines 71 to 93
WHEN productId = UINT256 '1' OR productId = UINT256 '16' THEN 'ETH'
WHEN productId = UINT256 '2' OR productId = UINT256 '17' THEN 'BTC'
WHEN productId = UINT256 '3' OR productId = UINT256 '18' THEN 'LINK'
WHEN productId = UINT256 '4' OR productId = UINT256 '19' THEN 'SNX'
WHEN productId = UINT256 '5' OR productId = UINT256 '20' THEN 'SOL'
WHEN productId = UINT256 '6' OR productId = UINT256 '21' THEN 'AVAX'
WHEN productId = UINT256 '7' OR productId = UINT256 '22' THEN 'MATIC'
WHEN productId = UINT256 '8' THEN 'LUNA'
WHEN productId = UINT256 '9' OR productId = UINT256 '23' THEN 'AAVE'
WHEN productId = UINT256 '10' OR productId = UINT256 '24' THEN 'APE'
WHEN productId = UINT256 '11' OR productId = UINT256 '25' THEN 'AXS'
WHEN productId = UINT256 '12' OR productId = UINT256 '26' THEN 'UNI'
ELSE CONCAT ('product_id_', CAST(productId as VARCHAR))
END AS virtual_asset

,CASE
WHEN productId = UINT256 '1' OR productId = UINT256 '16' THEN 'ETH'
WHEN productId = UINT256 '2' OR productId = UINT256 '17' THEN 'BTC'
WHEN productId = UINT256 '3' OR productId = UINT256 '18' THEN 'LINK'
WHEN productId = UINT256 '4' OR productId = UINT256 '19' THEN 'SNX'
WHEN productId = UINT256 '5' OR productId = UINT256 '20' THEN 'SOL'
WHEN productId = UINT256 '6' OR productId = UINT256 '21' THEN 'AVAX'
WHEN productId = UINT256 '7' OR productId = UINT256 '22' THEN 'MATIC'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there no way of getting these productId <> token symbols from onchain data?
If not I would make a separate model containing the static mapping and then join it in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there no way of getting these productId <> token symbols from onchain data? If not I would make a separate model containing the static mapping and then join it in here.

No, the productId and token symbols are not coming from on-chain data. They are part of a static mapping defined within the protocol's architecture, which cannot be queried directly on-chain.

I agree with your suggestion—it makes sense to create a separate model containing this static mapping and join it here for better maintainability and clarity. This will also simplify any future updates if new mappings are introduced.

Hosuke
Hosuke requested changes Jan 7, 2025
View reviewed changes
Copy link
Collaborator

@Hosuke Hosuke left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We may need to add some seed for testing pika_v4_optimism_perpetual_trades, you can add the seed here:
https://github.com/duneanalytics/spellbook/blob/d09fa392c8e07e6443cf26c4e59c97f1c7e41e8d/dbt_subprojects/hourly_spellbook/seeds/_sector/perpetual/trades/perpetual_trades_seed.csv

...llbook/models/_sector/perpetual/projects/pika/optimism/pika_v4_optimism_perpetual_trades.sql Outdated Show resolved Hide resolved
...llbook/models/_sector/perpetual/projects/pika/optimism/pika_v4_optimism_perpetual_trades.sql Outdated Show resolved Hide resolved
...llbook/models/_sector/perpetual/projects/pika/optimism/pika_v4_optimism_perpetual_trades.sql Outdated Show resolved Hide resolved
...llbook/models/_sector/perpetual/projects/pika/optimism/pika_v4_optimism_perpetual_trades.sql Outdated Show resolved Hide resolved
@Hosuke Hosuke added in review Assignee is currently reviewing the PR and removed ready-for-review this PR development is complete, please review labels Jan 7, 2025
@Hosuke
Copy link
Collaborator

Hosuke commented Jan 10, 2025

Please apply the incremental_predicate in incremental filters.

Hosuke
Hosuke approved these changes Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@Hosuke Hosuke left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.✅
Thank you @PatelPrinci

Sample pika_v4_optimism_perpetual_trades

...llbook/models/_sector/perpetual/projects/pika/optimism/pika_v4_optimism_perpetual_trades.sql Outdated
materialized = 'incremental',
file_format = 'delta',
incremental_strategy = 'merge',
unique_key = ['block_date', 'blockchain', 'project', 'version', 'tx_hash', 'evt_index'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may simplify the unique_key to:

Suggested change
unique_key = ['block_date', 'blockchain', 'project', 'version', 'tx_hash', 'evt_index'],
unique_key = ['tx_hash', 'evt_index'],

...ts/hourly_spellbook/models/_sector/perpetual/projects/pika/optimism/pika_optimism_schema.yml
Comment on lines +188 to +195
- dbt_utils.unique_combination_of_columns:
combination_of_columns:
- block_date
- blockchain
- project
- version
- tx_hash
- evt_index
Copy link
Collaborator

Choose a reason for hiding this comment

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

To match the simplified unique_key:

Suggested change
- dbt_utils.unique_combination_of_columns:
combination_of_columns:
- block_date
- blockchain
- project
- version
- tx_hash
- evt_index
- dbt_utils.unique_combination_of_columns:
combination_of_columns:
- tx_hash
- evt_index

@Hosuke Hosuke added ready-for-final-review and removed in review Assignee is currently reviewing the PR labels Jan 22, 2025
@jeff-dude jeff-dude merged commit b5008b6 into duneanalytics:main Jan 29, 2025
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@0xRobin 0xRobin 0xRobin left review comments

@Hosuke Hosuke Hosuke approved these changes

@jeff-dude jeff-dude jeff-dude approved these changes

Assignees

@Hosuke Hosuke

@0xRobin 0xRobin

Labels
dbt: hourly covers the hourly dbt subproject ready-for-merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@PatelPrinci @Hosuke @0xRobin @jeff-dude