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

adding uni v4 to dex.trades #7543

Merged
merged 11 commits into from
Jan 24, 2025
Merged

Conversation

agaperste
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:

Adding uniswap v4 to dex.trades on base


quick links for more information:

@github-actions github-actions bot added WIP work in progress dbt: dex covers the DEX dbt subproject labels Jan 24, 2025
@jeff-dude jeff-dude self-assigned this Jan 24, 2025
@jeff-dude jeff-dude added the in review Assignee is currently reviewing the PR label Jan 24, 2025
@jeff-dude jeff-dude requested a review from Hosuke January 24, 2025 17:59
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.

One possible way of implementation

@agaperste agaperste marked this pull request as ready for review January 24, 2025 19:28
@github-actions github-actions bot added ready-for-review this PR development is complete, please review and removed WIP work in progress labels Jan 24, 2025
@agaperste
Copy link
Contributor Author

Adding a record here for chains that are missing evt_tx_from in the Swap() event table, so we can address them later (either remove what I added for taker or add for taker for the other chains)

❌Arbitrum
❌Avalanche_c
✅Base
✅Blast
❌BNB
✅Optimism
❌Polygon
❌Ethereum
✅Zora
✅Worldchain

@jeff-dude jeff-dude changed the title adding uni v4 to base dex.trades adding uni v4 to dex.trades Jan 24, 2025
@jeff-dude
Copy link
Member

jeff-dude commented Jan 24, 2025

Adding a record here for chains that are missing evt_tx_from in the Swap() event table, so we can address them later (either remove what I added for taker or add for taker for the other chains)

❌Arbitrum ❌Avalanche_c ✅Base ✅Blast ❌BNB ✅Optimism ❌Polygon ❌Ethereum ✅Zora ✅Worldchain

fwiw, i think you should leave the logic and just pass in where applicable. you can also change null default in macro to be the column name if it ends up being consistent across all

edit: to be clear, that's for follow up PRs when decoding is cleaned up (if it is at all)

@jeff-dude jeff-dude removed the ready-for-review this PR development is complete, please review label Jan 24, 2025
@jeff-dude jeff-dude removed the in review Assignee is currently reviewing the PR label Jan 24, 2025
@jeff-dude jeff-dude merged commit fddbb5d into duneanalytics:main Jan 24, 2025
2 of 3 checks passed
Copy link

gitpoap-bot bot commented Jan 24, 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 Jan 24, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants