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

zeroex - v2 macro updates #7460

Merged
merged 132 commits into from
Feb 20, 2025
Merged

zeroex - v2 macro updates #7460

merged 132 commits into from
Feb 20, 2025

Conversation

RantumBits
Copy link
Contributor

@RantumBits RantumBits commented Jan 13, 2025

Description:

  • zeroex v2 macro updates

quick links for more information:

@github-actions github-actions bot added WIP work in progress dbt: dex covers the DEX dbt subproject labels Jan 13, 2025
@RantumBits
Copy link
Contributor Author

this is finally ready @Hosuke - runs relatively quickly and properly picks up bundled txs. also adds some new chains. lmk if you have questions

@RantumBits
Copy link
Contributor Author

@Hosuke I'm adding additional chains via the same macro. There was a database connection error on last run

@RantumBits
Copy link
Contributor Author

all set again @Hosuke, now with all chains moved to this macro

@jeff-dude
Copy link
Member

all set again @Hosuke, now with all chains moved to this macro

thank you for getting these back into a final reviewable state 🙏

we should take time to query the CI tables in-depth, to ensure we're happy this time around.
@RantumBits can you query each chain-level v2 CI table and compare vs. the old settler tables, ensure you are happy with output?
@Hosuke can we query the amount_usd fields, to ensure applied properly? last time we merged to prod, due to some bugs, the amount was inflated to trigger alerts. if we need to add more downstream models into this PR to get CI to run, we can do that too.

@RantumBits
Copy link
Contributor Author

Yes I am happy with the output. There are some transactions with value mismatches compared to old settler model. This is purposeful and corrects a previous issue with some trades having wrong maker values. These now match internal data. Comparison here: https://dune.com/queries/4742039?sidebar=none

I took a look at the top trades by USD Volume and the output looks good there as well. These can be viewed here: https://dune.com/queries/4742159

all set again @Hosuke, now with all chains moved to this macro

thank you for getting these back into a final reviewable state 🙏

we should take time to query the CI tables in-depth, to ensure we're happy this time around. @RantumBits can you query each chain-level v2 CI table and compare vs. the old settler tables, ensure you are happy with output? @Hosuke can we query the amount_usd fields, to ensure applied properly? last time we merged to prod, due to some bugs, the amount was inflated to trigger alerts. if we need to add more downstream models into this PR to get CI to run, we can do that too.

@jeff-dude
Copy link
Member

thank you for the additional context and validation. i pushed a few minor commits to filter a bit more on incremental runs & then run tables downstream to ensure pipeline is all good still. let me know if any of the incremental filters break anything here 🙏

@jeff-dude jeff-dude self-assigned this Feb 19, 2025
Copy link
Member

@jeff-dude jeff-dude left a comment

Choose a reason for hiding this comment

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

data in dex_aggregator.trades looks good to me after running here in CI. i'm good with the code now. thanks again for taking the time to build this and test thoroughly

@jeff-dude jeff-dude added ready-for-merging and removed in review Assignee is currently reviewing the PR labels Feb 19, 2025
@jeff-dude jeff-dude merged commit ef96274 into duneanalytics:main Feb 20, 2025
4 checks passed
Copy link

gitpoap-bot bot commented Feb 20, 2025

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2025 Dune Contributor:

GitPOAP: 2025 Dune Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dbt: dex covers the DEX dbt subproject ready-for-merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants