Skip to content

reconstruct orderId and validate for cowswap #144

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.

Sign up for GitHub

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

Jump to bottom
Open
to-sh-1 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

reconstruct orderId and validate for cowswap #144

to-sh-1 wants to merge 3 commits into from

Conversation

@to-sh-1
Copy link
Contributor

@to-sh-1 to-sh-1 commented Aug 1, 2022 •
edited
Loading

@to-sh-1 to-sh-1 changed the title reconstruct orderId and validated for cowswap reconstruct orderId and validate for cowswap Aug 2, 2022
0xean
0xean approved these changes Aug 8, 2022
View reviewed changes
Copy link
Collaborator

@0xean 0xean left a comment

Choose a reason for hiding this comment

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

This one is a little hard for me to review with so little understanding of cowswap.

Have you been able to test that it works end to end for a trade (not just the pre-signature)?

@to-sh-1
Copy link
Contributor Author

to-sh-1 commented Aug 8, 2022

This one is a little hard for me to review with so little understanding of cowswap.

Have you been able to test that it works end to end for a trade (not just the pre-signature)?

Yes, I was successfully able to do a trade on forked mainnet. This one does get hairy as I'm not an assembly expert, but I've spent some time looking at the code and seeing what it does and it's basically just taking info from the quote data from cowswap and creating the orderId from that data and matching with the orderId we are presigning. If you check the reference the guy who made the issue had an example and it's mostly copied from that. We can get on a call and walk through it if you would like

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0xean 0xean 0xean approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@to-sh-1 @0xean