Skip to content
This repository has been archived by the owner on Apr 11, 2023. It is now read-only.

feat: cowswap executeTrade and getTradeTxs methods #868

Merged
merged 59 commits into from
Jul 22, 2022
Merged

feat: cowswap executeTrade and getTradeTxs methods #868

merged 59 commits into from
Jul 22, 2022

Conversation

chxash
Copy link
Contributor

@chxash chxash commented Jul 12, 2022 •
edited
Loading

This implements both methods for CowSwapper.
This has been tested in web to successfully perform a swap with native wallet, metamask and metamask + ledger

Steps to test :

Swap with native wallet :

native1

native2

Linked TX : https://etherscan.io/tx/0xf7d251e527c19db3ef68063f99c5cd98cb4d86959382e8043303743d23cfb9cf

Swap with metamask :

trade_cow_product

Linked TX : https://etherscan.io/tx/0xcec287df80d98ce1d42b26f97260d9aea89e7daa551f388622bb4ca02978a049

chxash added 30 commits June 2, 2022 10:08
@chxash
feat: implements cowswapper skeleton
b6765a6
@chxash
feat: implements getUsdRate method
62cd50f
@chxash
Merge branch 'main' into cowswap-cowservice-getusdrate
0dd0930
@chxash
fix: review comments
3886894
@chxash
Merge branch 'main' into cowswap-cowservice-getusdrate
617b21a
@chxash
fix: review comments
477edeb
@chxash
feat: start implement filterBuyAssetsBySellAssetId and filterAssetIds…
dfed0c7 
…BySellable
@chxash
Merge branch 'main' into cowswap-cowservice-getusdrate
5af860a
@chxash
fix: issues after merge main
6d538af
@chxash
fix: review comment
41add66
@chxash
fix: review comments
a7335af
@chxash
Merge branch 'cowswap-cowservice-getusdrate' into cowswap-getminmax-f…
231551b 
…ilters
@chxash
feat: polish filtering functions and implements getCowSwapMinMax
6bf03b2
@chxash
Merge branch 'main' into cowswap-getminmax-filters
cca8954
@chxash
fix: lint
0ed0504
@chxash
fix: brackets
1cbf125
@chxash
feat: add comment
2f7af05
@chxash
feat: wip cow swap trade quote
e140874
@chxash
Merge branch 'main' into cowswap-getTradeQuote
6901162
@chxash
feat: implements cow swap quote trade
6205308
@chxash
feat: wip gas price and tests
5df3033
@chxash
feat: fee estimate for trade quote
413d84c
@chxash
Merge branch 'main' into cowswap-getTradeQuote
9ddd16c
@chxash
feat: unit tests
bedfa04
@chxash
Merge branch 'main' into cowswap-getTradeQuote
e9db525
@chxash
Merge branch 'main' into cowswap-getTradeQuote
a6debc4
@chxash
feat: implements buildTrade for cowswap
37cbc2f
@chxash
fix: integration issues when used in web
84428ce
@chxash
fix: unit test
e1808b1
@chxash
Merge branch 'main' into cowswap-getTradeQuote
5a7f96e
@chxash chxash linked an issue Jul 12, 2022 that may be closed by this pull request
CowSwap : Implements executeTrade / getTradeTxs methods in CowSwapper #726
Closed
gomesalexandre
gomesalexandre reviewed Jul 13, 2022
View reviewed changes
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.

A few early comments, will give another pass to this after shapeshift/hdwallet#546 is published

@chxash chxash marked this pull request as ready for review July 15, 2022 19:59
@chxash chxash requested review from a team and kaladinlight as code owners July 15, 2022 19:59
gomesalexandre
gomesalexandre previously approved these changes Jul 15, 2022
View reviewed changes
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.

Tested out locally with MM and managed to succesfully trade ERC-20s using the CowSwap swapper 🎉 . Tx is properly populated in Transaction History components on your web branch.

Note that on this branch, accounts without enough ETH for ZRX swaps will error because of hasEnoughBalanceForGas being false, and I had to use an account with enough ETH for fees, despite Cowswap Txs being gasless.

Irrelevant since the web PR is just for demo purposes, but when making the web PR for this, we'll have to make sure not to get regular ZRX fees estimation / not to handle them in <TradeInput /> in the case of moo swaps

🥛

@chxash
Copy link
Contributor Author

chxash commented Jul 17, 2022

Note that on this branch, accounts without enough ETH for ZRX swaps will error because of hasEnoughBalanceForGas being false, and I had to use an account with enough ETH for fees, despite Cowswap Txs being gasless.

@gomesalexandre interesting finding, thank you !

@gomesalexandre gomesalexandre mentioned this pull request Jul 18, 2022
Merged
5 tasks
@chxash
Copy link
Contributor Author

chxash commented Jul 20, 2022

@gomesalexandre your finding led me to fix an issue related to ETH fees in getCowSwapTradeQuote.ts.
Basically I replicated the fix I already applied in cowBuildTrade.ts.
I tested with an account without any ETH and was able to perform a trade with CowSwap.
Also merged main into branch

@chxash chxash mentioned this pull request Jul 20, 2022
Merged
5 tasks
0xdef1cafe
0xdef1cafe approved these changes Jul 20, 2022
View reviewed changes
Copy link
Collaborator

@0xdef1cafe 0xdef1cafe left a comment

Choose a reason for hiding this comment

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

after we have cowswap, osmo, thorchain, zrx across eth and avalanche we can revisit and tidy abstractions, but this is good for now

gomesalexandre
gomesalexandre approved these changes Jul 20, 2022
View reviewed changes
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.

LGTM

Tested again with your latest changes and I was able to make a moo swap with your latest changes and I was able to get cowswaps working both with an account that has enough ETH for gas, and an account that doesn't have enough ETH for regular swaps

🐮

@gomesalexandre gomesalexandre merged commit b9ce707 into main Jul 22, 2022
@gomesalexandre gomesalexandre deleted the cowswap-executeTrade-PR branch July 22, 2022 15:34
shapeshift-ci-bot pushed a commit that referenced this pull request Jul 22, 2022
@semantic-release-bot
chore(release): 7.10.0 [skip ci]
be47963 
# [@shapeshiftoss/chain-adapters-v7.10.0](https://github.com/shapeshift/lib/compare/@shapeshiftoss/chain-adapters-v7.9.0...@shapeshiftoss/chain-adapters-v7.10.0) (2022-07-22)

### Features

* cowswap executeTrade and getTradeTxs methods ([#868](#868)) ([b9ce707](b9ce707))
@shapeshift-ci-bot
Copy link
Member

🎉 This PR is included in version @shapeshiftoss/chain-adapters-v7.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

shapeshift-ci-bot pushed a commit that referenced this pull request Jul 22, 2022
@semantic-release-bot
chore(release): 8.6.0 [skip ci]
7b4e756 
# [@shapeshiftoss/swapper-v8.6.0](https://github.com/shapeshift/lib/compare/@shapeshiftoss/swapper-v8.5.3...@shapeshiftoss/swapper-v8.6.0) (2022-07-22)

### Features

* cowswap executeTrade and getTradeTxs methods ([#868](#868)) ([b9ce707](b9ce707))
@shapeshift-ci-bot
Copy link
Member

🎉 This PR is included in version @shapeshiftoss/swapper-v8.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@gomesalexandre gomesalexandre gomesalexandre approved these changes

@0xdef1cafe 0xdef1cafe 0xdef1cafe approved these changes

@kaladinlight kaladinlight Awaiting requested review from kaladinlight

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CowSwap : Implements executeTrade / getTradeTxs methods in CowSwapper
4 participants
@chxash @shapeshift-ci-bot @gomesalexandre @0xdef1cafe