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

Support CoW AMM stats on all chains #46

Merged
merged 5 commits into from
Sep 27, 2024
Merged

Conversation

fleupold
Copy link
Contributor

@fleupold fleupold commented Sep 20, 2024

This PR makes it so that statistics for CoW AMMs are chain agnostic. Unfortunately I wasn't able to get the Uniswap graph working our of the box, as it uses different event tables on different chains (and dunes variable syntax isn't flexible enough to choose a target table based on a condition).

Will ask them on tg, but for now we will have to live with CoW AMM performance vs. rebalancing portfolio.

Test Plan

@fhenneke
Copy link
Contributor

Unfortunately I wasn't able to get the Uniswap graph working our of the box, as it uses different even tables on different chains (and dunes variable syntax isn't flexible enough to choose a target table based on a condition).

Would the approach in

select * from special_balance_changes_{{blockchain}}
help here?

It is a bit ugly but one can create different tables for different chains and then select the correct one based on a parameter.

@fleupold
Copy link
Contributor Author

This could work. I'm also considering reconstructing the events based on raw logs (which is available in a chain uniform way). However, I think it's ok to have a separate PR for this.

Copy link
Contributor

@harisang harisang left a comment

Choose a reason for hiding this comment

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

Looks good. Would only update the description at the top of 3959044 and also fix the typo in the description of 4059700

@fleupold
Copy link
Contributor Author

fleupold commented Sep 27, 2024

and also fix the typo in the description of 4059700

It's usually easier to comment directly on the line that has the typo so that I don't have to guess which one you mean (although it's a good test for my orthographic reading skills). You can even suggest the fix there :-)

@fleupold fleupold merged commit 942a667 into main Sep 27, 2024
1 check passed
@fleupold fleupold deleted the cowamm/multichain_support branch September 27, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants