Support custom route methods && uniswap v3 multi hop

[liu-zhipeng]

• [x] support custom router abi (For Pangolin and Trader Joe swap on avalanche)
• [x] support custom router method names
• [x] support uniswap v3 multihop
• [x] fix some issues
• [x] Unit tests
• [x] readme

[liu-zhipeng]

Hi. I am so excited to contribute to such a great project. I found some small problems while using uniswap sdk. For example, some uniswap forked dexs like pangolin swap changed router functions. especially ETH to AVAX. So i added some changes for developers to use custom router abi and router methods. Also I added some code to support uniswap v3 multihop. I would like you to review my changes if you have time. Thank you again for a great sdk!

[joshstevens19]

Hey @liu-zhipeng firstly thanks so much for this!! Help from others is needed and this looks incredible already.

Only scanned on phone but 1 thing which popped out any new things or featured you added to this please add to the readme doc, for example you done some liquidity work and now supporting multihop adding to the docs is the last part missing I think!

I will review this tomorrow be great to review with the docs update as well thanks a lot hope this will be your first of many PRs 💪

[liu-zhipeng]

Hey @liu-zhipeng firstly thanks so much for this!! Help from others is needed and this looks incredible already.

Only scanned on phone but 1 thing which popped out any new things or featured you added to this please add to the readme doc, for example you done some liquidity work and now supporting multihop adding to the docs is the last part missing I think!

I will review this tomorrow be great to review with the docs update as well thanks a lot hope this will be your first of many PRs 💪

Yep. i will do! Also i am going to add unit tests for new features as well.

[mburger81]

@joshstevens19 @liu-zhipeng when do you think you can find a solution for the changes requested and release this?

[liu-zhipeng]

@joshstevens19 @liu-zhipeng when do you think you can find a solution for the changes requested and release this?

Once @joshstevens19 confirms this, we can release this, cus all features are working properly now.

[mburger81]

So you resolved the backward compatibility? There was a problem about in some points your returned now an array []

could you maybe release in meantime your version on npm?

[liu-zhipeng]

That is no problem at all to return liquidityProviderFee as array. If there are some issues on the apps which are using old version. they MUST update to support uniswap v3. Because each uniswap V3 pairs have 3 different fee tiers. I am waiting for @joshstevens19 's reivew yet.

[liu-zhipeng]

This PR should be closed? @joshstevens19

[joshstevens19]

No PR is good sorry been away I’m going to get to this soon!

[mburger81]

@liu-zhipeng why did you close this unmerged?

[ghost]

when will you release this? @liu-zhipeng

[mburger81]

@helios8090 the problem is, @liu-zhipeng can not do the merge and the release @joshstevens19 sorry dude but its really annoying :(

[liu-zhipeng]

@helios8090 the problem is, @liu-zhipeng can not do the merge and the release @joshstevens19 sorry dude but its really annoying :(

I have no permission to merge.

[mburger81]

@liu-zhipeng I know, for that reason I asked you to fork, merge and release your own repository, just temporally until @joshstevens19 will merge it

we are trying to make webcomponents for the community using simple-uniswap to deliver a well working, free, fast, working everywhere solution but we are stuck on waiting this merge since e 5 weeks

[joshstevens19]

Guys super sorry I’ve been away and it’s super hard for me to see these sometimes. The main problem with the PR is it’s a breaking interface change to the liquidity interface which means if we release it everyones integration will break who use to think that was a string. I’m going to set a note for tomorrow AM before I do anything else to do another PR review on this. I did do a PR review but the things I asked still is missing. We can’t do breaking changes in the interface we can ofc add a new field and depreciate the old one for v1 and v2 use only but if you could make it so v2 interface is NONE breaking then I’m happy to get this in asap @liu-zhipeng

[joshstevens19]

Remind me set.. tomorrow AM 9:45 BST! will do another review of it and highlight the main issues. Sorry for the delay!

[mburger81]

Yeah that was what I'm saying, there is this breaking change you already told us for that reason I was wondering why you wrote later, anything is fine, because it isn't

I Just ask you to maybe integrate this changes also on the swap ui you provide

THX a lot!

[joshstevens19]

Well it is - you just expose the field in a new property in the interface and write docs around it why, it’s a very easy fix in this PR!

[mburger81]

@liu-zhipeng nice!

[joshstevens19]

this looks better @liu-zhipeng can you just update the readme docs to show the new field and explain it so people understand?

[liu-zhipeng]

I have already updated readme.

[liu-zhipeng]

https://github.com/joshstevens19/simple-uniswap-sdk/pull/45/commits/7a178a7e9618ef32258b2e6daaa8d076189f183d

[liu-zhipeng]

Thank you for your approval. @joshstevens19 And I am super happy if my work can be helpful for others.

[joshstevens19]

@liu-zhipeng so done manually tests everything works well, just one thing I spotted is when on a v2 trade the liquidityProviderFeesV3 is populated it should be an empty array [] could you do that then we can merge and deploy!

[liu-zhipeng]

@liu-zhipeng so done manually tests everything works well, just one thing I spotted is when on a v2 trade the liquidityProviderFeesV3 is populated it should be an empty array [] could you do that then we can merge and deploy!

I thought that was more consistent. anyway updated finally!

[joshstevens19]

amazing! Will deploy tomorrow @liu-zhipeng thanks again

[liu-zhipeng]

Awesome!

[joshstevens19]

https://github.com/joshstevens19/simple-uniswap-sdk/releases/tag/3.7.0 - its now deployed