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

chore: consolidate cowswap utils #8331

Merged
merged 6 commits into from
Dec 11, 2024

Conversation

woodenfurniture
Copy link
Member

@woodenfurniture woodenfurniture commented Dec 11, 2024

Description

Moves shared utils and constants for cowswap into @shapeshiftoss/swapper under cowswap-utils.

Issue (if applicable)

closes #8214

Risk

High Risk PRs Require 2 approvals

Very low risk of broken cowswap trades - utils were only moved without changes to logic.

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Cowswap trades.

Testing

Sanity check cowswap trades are working correctly.

Engineering

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

Cowswap trade after chagnes:

https://jam.dev/c/916e56d9-37b0-4f1c-9e26-e5eb40cc1319

@woodenfurniture woodenfurniture requested a review from a team as a code owner December 11, 2024 01:14
@gomesalexandre gomesalexandre self-requested a review December 11, 2024 03:44
Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Hell yeah to keeping the house tidy 🧹

Spot check looking gucci.

Spot: https://jam.dev/c/ce6219e0-7d96-405b-9f82-cb71580272e2
Limit: https://jam.dev/c/38dfc269-f15b-4131-846e-a69a06047cab

@woodenfurniture noting execution is sad on the Jam above for limit, though confirmed this is isolated to limit and spot isn't affected: https://jam.dev/c/cda1470a-ef58-4250-81cb-310a90301efe

Also confirmed this isn't a regression introduced by this PR by testing on upstream develop: https://jam.dev/c/4cf981c4-8d84-4e03-83c8-761b10aec9c0

Authored #8333 for it :fridaydog:

@gomesalexandre gomesalexandre enabled auto-merge (squash) December 11, 2024 04:38
@gomesalexandre gomesalexandre merged commit 46e597a into develop Dec 11, 2024
3 checks passed
@gomesalexandre gomesalexandre deleted the limit-orders-cowswap-consolidation branch December 11, 2024 04:45
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.

Consolitate cowswap helpers
2 participants