Support custom route methods && uniswap v3 multi hop #45

liu-zhipeng · 2022-06-28T03:17:05Z

liu-zhipeng · 2022-06-29T17:37:56Z

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 · 2022-06-29T17:58:48Z

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 · 2022-06-29T18:09:09Z

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.

.gitignore

joshstevens19 · 2022-06-30T16:44:59Z

README.md

+  // For multihop swap
+  // Sequence of Fees 
+  // ex: USDC>USDT>WETH>WBTC => [(USDC-USDT fee), (USDT-WETH fee), (WETH-WBTC fee)]
+  liquidityProviderFee: string[];


doesn't seem correct here as in v2 it would still be string

in v2 liquidityProviderFee should be array in this version

@liu-zhipeng am i correct?

joshstevens19 · 2022-06-30T16:45:03Z

README.md

@@ -437,7 +447,7 @@ export interface TradeContext {
  // aka 0.05% = 0.0005
  // 0.3% = 0.003
  // 1% = 0.01
-  liquidityProviderFeePercent: number;
+  liquidityProviderFeePercent: number[];


doesn't seem correct here as in v2 it would still be string

joshstevens19 · 2022-06-30T16:45:09Z

README.md

@@ -464,7 +474,7 @@ export interface TradeContext {
    routeText: string;
    routePathArray: string[];
    uniswapVersion: UniswapVersion;
-    liquidityProviderFee: number;
+    liquidityProviderFee: number[];


doesn't seem correct here as in v2 it would still be string

joshstevens19 · 2022-06-30T16:46:32Z

README.md

-  liquidityProviderFee: '0.030000000000000000',
-  liquidityProviderFeePercent: 0.003,
+  liquidityProviderFee: ['0.030000000000000000','0.050000000000000000','0.030000000000000000'],
+  liquidityProviderFeePercent: [0.003, 0.005, 0.003],


from my memory the liquidity provider fee percent on this level is just 1 percent and if you got many routes with different providers fees it would create repeated objects with the single fee. Then order by the best.

joshstevens19 · 2022-06-30T16:50:07Z

src/factories/pair/models/trade-context.ts

-  liquidityProviderFee: string;
-  liquidityProviderFeePercent: number;
+  liquidityProviderFee: string[];
+  liquidityProviderFeePercent: number[];


again arrays here v2 wont be an array

Same as above breaking change in the trade context ..

joshstevens19 · 2022-06-30T16:50:57Z

src/factories/pair/uniswap-pair.factory.ts

@@ -333,14 +333,13 @@ export class UniswapPairFactory {
          ? null
          : bestRouteQuote.expectedConvertQuoteOrTokenAmountInMaxWithSlippage,
      expectedConvertQuote: bestRouteQuote.expectedConvertQuote,
-      liquidityProviderFee:
+      liquidityProviderFee: bestRouteQuote.liquidityProviderFee.map((f) =>


breaking changes for v2.. if many liquidity provider fees the expected convert quote will be different depending on that no? would not expect a loop here i dont think...

joshstevens19 · 2022-06-30T16:51:14Z

src/factories/pair/uniswap-pair.factory.ts

@@ -404,14 +403,13 @@ export class UniswapPairFactory {
          ? null
          : bestRouteQuote.expectedConvertQuoteOrTokenAmountInMaxWithSlippage,
      expectedConvertQuote: bestRouteQuote.expectedConvertQuote,
-      liquidityProviderFee:
+      liquidityProviderFee: bestRouteQuote.liquidityProviderFee.map((f) =>


same as above

joshstevens19 · 2022-06-30T16:51:23Z

src/factories/pair/uniswap-pair.factory.ts

@@ -470,14 +468,13 @@ export class UniswapPairFactory {
          ? null
          : bestRouteQuote.expectedConvertQuoteOrTokenAmountInMaxWithSlippage,
      expectedConvertQuote: bestRouteQuote.expectedConvertQuote,
-      liquidityProviderFee:
+      liquidityProviderFee: bestRouteQuote.liquidityProviderFee.map((f) =>


same as above

src/factories/router/models/token-routes.ts

mburger81 · 2022-07-11T08:41:58Z

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

liu-zhipeng · 2022-07-11T08:44:49Z

@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 · 2022-07-11T08:47:47Z

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 · 2022-07-11T08:52:27Z

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.

joshstevens19 · 2022-07-14T12:16:40Z

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

mburger81 · 2022-07-27T04:59:09Z

@liu-zhipeng why did you close this unmerged?

ghost · 2022-08-01T14:58:19Z

when will you release this? @liu-zhipeng

mburger81 · 2022-08-01T17:21:04Z

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

liu-zhipeng · 2022-08-01T18:20:39Z

@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 · 2022-08-01T19:57:54Z

@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 · 2022-08-01T20:08:22Z

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 · 2022-08-01T20:09:28Z

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

mburger81 · 2022-08-01T21:58:23Z

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 · 2022-08-01T22:05:39Z

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!

joshstevens19 · 2022-08-01T22:06:52Z

src/factories/pair/models/current-trade-context.ts

@@ -8,7 +8,7 @@ export interface CurrentTradeContext {
  quoteDirection: TradeDirection;
  fromToken: Token;
  toToken: Token;
-  liquidityProviderFee: string;
+  liquidityProviderFee: string[];


This here! This needs to be a new field if it has to be an array so it doesn’t do a breaking change to the interface

joshstevens19 · 2022-08-01T22:07:17Z

src/factories/pair/models/trade-context.ts

-  liquidityProviderFee: string;
-  liquidityProviderFeePercent: number;
+  liquidityProviderFee: string[];
+  liquidityProviderFeePercent: number[];


Same as above breaking change in the trade context ..

mburger81 · 2022-08-02T06:03:17Z

@liu-zhipeng nice!

joshstevens19 · 2022-08-02T08:50:46Z

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 · 2022-08-02T11:42:51Z

I have already updated readme.

liu-zhipeng · 2022-08-02T11:44:03Z

7a178a7

joshstevens19

need to run 1 manual test later if good will merge and deploy.. thanks for doing this and sorry it ended up taking longer then we wanted. @liu-zhipeng your a legend!

liu-zhipeng · 2022-08-03T13:07:33Z

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

joshstevens19 · 2022-08-03T23:58:12Z

@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 · 2022-08-04T00:54:53Z

@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 · 2022-08-04T01:03:33Z

amazing! Will deploy tomorrow @liu-zhipeng thanks again

liu-zhipeng · 2022-08-04T01:11:21Z

Awesome!

joshstevens19 · 2022-08-04T09:04:48Z

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

liu-zhipeng added 7 commits June 28, 2022 11:11

Fix token decimals for usdc, usdt

6c908b5

add customized route abi to the settings

fe19516

add customized router methods

fdedbc1

add isSameAddress utils func

4df4ffe

update FUN token totalSupply

0a0cb0b

update models for uniswap v3

699125d

uniswap v3 quote works

6097a0f

liu-zhipeng marked this pull request as ready for review June 29, 2022 15:05

liu-zhipeng closed this Jun 29, 2022

liu-zhipeng reopened this Jun 29, 2022

liu-zhipeng added 2 commits June 30, 2022 02:19

formatting

5ce261c

Update README.md

50b8098

savasadar approved these changes Jun 30, 2022

View reviewed changes

joshstevens19 requested changes Jun 30, 2022

View reviewed changes

liu-zhipeng added 2 commits July 1, 2022 01:42

fix uniswap v2 provider fee

98b9ecd

fix test description

ebc3fa1

liu-zhipeng requested a review from joshstevens19 June 30, 2022 18:08

liu-zhipeng added 2 commits July 1, 2022 02:30

fix percent examples

594fcb7

EthToErc20, Erc20ToEth working for v3

1e6cedc

liu-zhipeng mentioned this pull request Jul 5, 2022

feat: AVALANCHE support #46

Closed

liu-zhipeng added 2 commits July 6, 2022 01:36

fix generateTradeDataForV3Input remove eth

db9ce96

fixed customized router methods

26aa246

bug fix on hexlify

e0432b1

liu-zhipeng closed this Jul 27, 2022

joshstevens19 reopened this Jul 27, 2022

joshstevens19 requested changes Aug 1, 2022

View reviewed changes

liu-zhipeng added 3 commits August 2, 2022 09:15

add new fields for uniswap v3 liquidity fee

1c166b9

fix router & factory

501e5d6

update Readme

7a178a7

liu-zhipeng requested a review from joshstevens19 August 2, 2022 11:44

joshstevens19 approved these changes Aug 3, 2022

View reviewed changes

empty liquidityProviderFeesV3 for v2

6a9aab4

joshstevens19 merged commit 75b9f9c into joshstevens19:master Aug 4, 2022

Support custom route methods && uniswap v3 multi hop #45

Support custom route methods && uniswap v3 multi hop #45

Conversation

liu-zhipeng commented Jun 28, 2022 • edited Loading

liu-zhipeng commented Jun 29, 2022

joshstevens19 commented Jun 29, 2022

liu-zhipeng commented Jun 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshstevens19 Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mburger81 commented Jul 11, 2022

liu-zhipeng commented Jul 11, 2022

mburger81 commented Jul 11, 2022

liu-zhipeng commented Jul 11, 2022 • edited Loading

joshstevens19 commented Jul 14, 2022

mburger81 commented Jul 27, 2022

ghost commented Aug 1, 2022

mburger81 commented Aug 1, 2022

liu-zhipeng commented Aug 1, 2022

mburger81 commented Aug 1, 2022

joshstevens19 commented Aug 1, 2022

joshstevens19 commented Aug 1, 2022

mburger81 commented Aug 1, 2022

joshstevens19 commented Aug 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mburger81 commented Aug 2, 2022

joshstevens19 commented Aug 2, 2022

liu-zhipeng commented Aug 2, 2022 • edited Loading

liu-zhipeng commented Aug 2, 2022

joshstevens19 left a comment

Choose a reason for hiding this comment

liu-zhipeng commented Aug 3, 2022

joshstevens19 commented Aug 3, 2022 • edited Loading

liu-zhipeng commented Aug 4, 2022

joshstevens19 commented Aug 4, 2022

liu-zhipeng commented Aug 4, 2022

joshstevens19 commented Aug 4, 2022

liu-zhipeng commented Jun 28, 2022 •

edited

Loading

liu-zhipeng commented Jun 29, 2022 •

edited

Loading

joshstevens19 Jun 30, 2022 •

edited

Loading

liu-zhipeng commented Jul 11, 2022 •

edited

Loading

liu-zhipeng commented Aug 2, 2022 •

edited

Loading

joshstevens19 commented Aug 3, 2022 •

edited

Loading