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

Dex Aggregator Migration - #1 - Lifi #4054

Merged
merged 12 commits into from
Aug 21, 2023

Conversation

henrystats
Copy link
Contributor

Thank you for contributing to Spellbook!

Please refer to the top of the readme in the root of Spellbook to learn how to contribute to Spellbook on DuneSQL.

@dune-eng
Copy link

Workflow run id 5838376322 approved.

@dune-eng
Copy link

Workflow run id 5838376381 approved.

@dune-eng
Copy link

Workflow run id 5838462152 approved.

@dune-eng
Copy link

Workflow run id 5838462302 approved.

@dune-eng
Copy link

Workflow run id 5838510283 approved.

@dune-eng
Copy link

Workflow run id 5838510387 approved.

@dune-eng
Copy link

Workflow run id 5840578360 approved.

@dune-eng
Copy link

Workflow run id 5840578413 approved.

@dune-eng
Copy link

Workflow run id 5840604879 approved.

@dune-eng
Copy link

Workflow run id 5840604948 approved.

@dune-eng
Copy link

Workflow run id 5840662723 approved.

@dune-eng
Copy link

Workflow run id 5840662743 approved.

@dune-eng
Copy link

Workflow run id 5843829939 approved.

@dune-eng
Copy link

Workflow run id 5843829992 approved.

@henrystats henrystats changed the title Dex Aggregator Migration - #1 - Lifi Dex Aggregator Migration - #1 - Lifi - Ready for review Aug 12, 2023
@dune-eng
Copy link

Workflow run id 5843855123 approved.

@dune-eng
Copy link

Workflow run id 5843855120 approved.

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.

LGTM.✅
Thank you @henrystats

  • Partition by block_month ✅
  • Tests and seeds migrated ✅

Sample lifi_trades

models/lifi/fantom/lifi_v2_fantom_trades.sql Show resolved Hide resolved
@antonio-mendes antonio-mendes added the ready-for-review this PR development is complete, please review label Aug 17, 2023
@antonio-mendes antonio-mendes changed the title Dex Aggregator Migration - #1 - Lifi - Ready for review Dex Aggregator Migration - #1 - Lifi Aug 17, 2023
@jeff-dude jeff-dude self-assigned this Aug 18, 2023
@jeff-dude jeff-dude added in review Assignee is currently reviewing the PR and removed ready-for-review this PR development is complete, please review ready-for-final-review labels Aug 18, 2023
Comment on lines 72 to 73
CAST(dexs.token_bought_amount_raw AS UINT256) AS token_bought_amount_raw,
CAST(dexs.token_sold_amount_raw AS UINT256) AS token_sold_amount_raw,
Copy link
Member

Choose a reason for hiding this comment

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

these are both already uint256 at the source, we can remove the cast

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix, there are a couple of other changes @antonio-mendes mentioned in other models as well... If you're reviewing/merging this evening I can submit the fix for all the models since it won't take a lot of time.. actually lemme get to it now.. I was trying to complete something else but It should be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleaned up will fix other models now @jeff-dude

@dune-eng
Copy link

Workflow run id 5906206862 approved.

@dune-eng
Copy link

Workflow run id 5906206789 approved.

@dune-eng
Copy link

Workflow run id 5906209873 approved.

@dune-eng
Copy link

Workflow run id 5906209943 approved.

@dune-eng
Copy link

Workflow run id 5906310872 approved.

@dune-eng
Copy link

Workflow run id 5906310886 approved.

@jeff-dude jeff-dude added ready-for-merging and removed in review Assignee is currently reviewing the PR labels Aug 21, 2023
@jeff-dude jeff-dude merged commit ab34874 into duneanalytics:main Aug 21, 2023
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2023
@henrystats henrystats deleted the migrate-lifi-fantom branch August 23, 2023 03:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants