-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat(uniswapx-sdk): Implement Dutch V3 #84
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking great so far!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Left some small comments.
Graphite Automations"Request reviewers once CI passes on sdks monorepo" took an action on this PR • (09/19/24)1 reviewer was added to this PR based on 's automation. |
…as Adjustment Feature With E2E Tests (#106)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job!
Description
We are adding support for the new Dutch v3 orders in the UniswapX SDK
How Has This Been Tested?
Unit testing for all files so far, will include integration after contracts are completely finalized
Are there any breaking changes?
None. V3 was intentionally written in a way that guarantees V2 doesn't break. In the future, we should consider creating a CosignedDutch class that parents classes such as V2 and V3. This would require significant refactoring but would be cleaner.