Skip to content
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

Swap without a query #5

Open
halo3mic opened this issue Jun 17, 2021 · 17 comments
Open

Swap without a query #5

halo3mic opened this issue Jun 17, 2021 · 17 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@halo3mic
Copy link
Member

The current version of YakRouter swap queries all adapters for their amounts before swapping. This ensures the correct amounts are used for swaps and saves user gas if the final amount-out is below the minimum amount-out specified by the user.

The queries however can be quite gas-intensive, for example:

  • Querying Gondola DAI-ZDAI pool through adapter costs 84340 gas
  • Querying Snob S3 pool through the adapter to swap DAI for USDT costs 99130 gas
  • Querying Unilike pool through the adapter to swap DAI to WAVAX costs 13612 gas

To avoid such high gas costs queries could be removed and the swap amount for the next step could be derived from the token balance in the adapter. ER20.balanceOf costs 23k, so the gas reduction would be substantial.

The big downside of this approach is that if price moved on the user to the point that the trade would fail. Now the trade would fail with relatively low gas cost after all the queries are completed, but with this approach, trade would fail in the last step costing the user substantially more gas.

@snow-farmer
Copy link
Contributor

Recently on the router, about 4% of trades fail. We can now do the math for identifying which is better for users.

https://cchain.explorer.avax.network/address/0x187Cd11549a20ACdABd43992d01bfcF2Bfc3E18d/transactions

@snow-farmer snow-farmer added the question Further information is requested label Jul 16, 2021
@snow-farmer
Copy link
Contributor

Adding a low slippage (0.2%) the failure rate has become closer to zero.

@snow-farmer snow-farmer added enhancement New feature or request good first issue Good for newcomers labels Sep 3, 2021
@snow-farmer
Copy link
Contributor

We could include both ways and let front-ends solve. For example, low slippage trades (higher chance to fail) should use the method which consumes less gas in revert scenarios and high slippage trades (lower chance to fail) should use the method which consumes less gas in success scenarios.

@halo3mic
Copy link
Member Author

halo3mic commented Sep 6, 2021

We could include both ways and let front-ends solve. For example, low slippage trades (higher chance to fail) should use the method which consumes less gas in revert scenarios and high slippage trades (lower chance to fail) should use the method which consumes less gas in success scenarios.

That is a really good idea!

@snow-farmer snow-farmer removed the question Further information is requested label Oct 3, 2022
@RedWilly
Copy link

RedWilly commented Mar 8, 2024

Hey i have a question how do i get the amount out of when using findBestPath it provides the the path, amountin and not out

@halo3mic
Copy link
Member Author

halo3mic commented Mar 8, 2024

Hey i have a question how do i get the amount out of when using findBestPath it provides the the path, amountin and not out

Method findBestPath returns object of type FormattedOffer. There is parameter amounts in this object, an array that contains input amount, all intermediate amounts (in case of >1 hops) and output amount. Output amount is the last element of the amounts array.

@RicqCodes
Copy link

RicqCodes commented Apr 16, 2024

@halo3mic, I'm really struggling here and could use your help.

  1. I've been using the findBestPathWithGas function, and it correctly returns the FormattedOffer object needed for swapNoSplit. However, I'm facing a recurring issue: when I set it to 3 steps, it sometimes uses multiple adapters, and I frequently encounter an insufficient amount-out error from the pair contract. But if I reduce the steps to 2, the swap transaction seems to go through without any issues. It's quite perplexing and frustrating.

  2. Also, when dealing with tokens that have known fees, I keep getting the same insufficient amount-out error, no matter how I adjust the amountOut, even setting it to 0. It just doesn’t seem to work at all.

The router is deployed on base blockchain at 0xC448CCD732085891c3B2E819FED09ad3C4C39509, and the token I’m trying to swap is called frenpet, at 0xFF0C532FDB8Cd566Ae169C1CB157ff2Bdc83E105. Interestingly, using BaseSwap for the same token swap works with 5% slippage which is where liquidity was created. I have baseswap v2, uniswap v2 & v3, sushiswap v2, pancakeswap v2 & v3 deployed as adapters.

Any advice or pointers you could provide would be greatly appreciated. I'm really lost.

@RedWilly
Copy link

@halo3mic, I'm really struggling here and could use your help.

  1. I've been using the findBestPathWithGas function, and it correctly returns the FormattedOffer object needed for swapNoSplit. However, I'm facing a recurring issue: when I set it to 3 steps, it sometimes uses multiple adapters, and I frequently encounter an insufficient amount-out error from the pair contract. But if I reduce the steps to 2, the swap transaction seems to go through without any issues. It's quite perplexing and frustrating.
  2. Also, when dealing with tokens that have known fees, I keep getting the same insufficient amount-out error, no matter how I adjust the amountOut, even setting it to 0. It just doesn’t seem to work at all.

The router is deployed on base blockchain at 0xC448CCD732085891c3B2E819FED09ad3C4C39509, and the token I’m trying to swap is called frenpet, at 0xFF0C532FDB8Cd566Ae169C1CB157ff2Bdc83E105. Interestingly, using BaseSwap for the same token swap works with 5% slippage which is where liquidity was created. I have baseswap v2, uniswap v2 & v3, sushiswap v2, pancakeswap v2 & v3 deployed as adapters.

Any advice or pointers you could provide would be greatly appreciated. I'm really lost.

I also did get the exact same issue. I posted it in the telegram dev community but got no helpful response on how to deal with that. Was even hoping to take a look at how the handle it on the front end part but nothing.

Anyways if you somehow ever found an issue please tag me.

@RicqCodes
Copy link

@halo3mic, I'm really struggling here and could use your help.

  1. I've been using the findBestPathWithGas function, and it correctly returns the FormattedOffer object needed for swapNoSplit. However, I'm facing a recurring issue: when I set it to 3 steps, it sometimes uses multiple adapters, and I frequently encounter an insufficient amount-out error from the pair contract. But if I reduce the steps to 2, the swap transaction seems to go through without any issues. It's quite perplexing and frustrating.
  2. Also, when dealing with tokens that have known fees, I keep getting the same insufficient amount-out error, no matter how I adjust the amountOut, even setting it to 0. It just doesn’t seem to work at all.

The router is deployed on base blockchain at 0xC448CCD732085891c3B2E819FED09ad3C4C39509, and the token I’m trying to swap is called frenpet, at 0xFF0C532FDB8Cd566Ae169C1CB157ff2Bdc83E105. Interestingly, using BaseSwap for the same token swap works with 5% slippage which is where liquidity was created. I have baseswap v2, uniswap v2 & v3, sushiswap v2, pancakeswap v2 & v3 deployed as adapters.
Any advice or pointers you could provide would be greatly appreciated. I'm really lost.

I also did get the exact same issue. I posted it in the telegram dev community but got no helpful response on how to deal with that. Was even hoping to take a look at how the handle it on the front end part but nothing.

Anyways if you somehow ever found an issue please tag me.

I'm hoping to find solution, if you could share the telegram channel i'd appreciate that.

@RicqCodes
Copy link

@RedWilly I think I found out why, took a while to debug!

@RedWilly
Copy link

Okay can you share your finding

@halo3mic
Copy link
Member Author

halo3mic commented Apr 17, 2024

2. Also, when dealing with tokens that have known fees, I keep getting the same insufficient amount-out error, no matter how I adjust the amountOut, even setting it to 0. It just doesn’t seem to work at all.

Hey, @RicqCodes thanks for reaching out! Sorry to hear you had no help on tg, personally not active there very much.

Tokens with fee on transfer are not supported with this version of the router due to the way swap execution works. Swap execution runs the query before each swap which doesn't account for the fee on transfer - the query calculated amount will be higher than the actual amount.

This has been discussed for some time now and I agree that it would be more practical to change the swap logic in favour of checking the balance after each swap instead of quoting. Would you be interested in implementing this?

@halo3mic
Copy link
Member Author

@halo3mic, I'm really struggling here and could use your help.

  1. I've been using the findBestPathWithGas function, and it correctly returns the FormattedOffer object needed for swapNoSplit. However, I'm facing a recurring issue: when I set it to 3 steps, it sometimes uses multiple adapters, and I frequently encounter an insufficient amount-out error from the pair contract. But if I reduce the steps to 2, the swap transaction seems to go through without any issues. It's quite perplexing and frustrating.
  2. Also, when dealing with tokens that have known fees, I keep getting the same insufficient amount-out error, no matter how I adjust the amountOut, even setting it to 0. It just doesn’t seem to work at all.

The router is deployed on base blockchain at 0xC448CCD732085891c3B2E819FED09ad3C4C39509, and the token I’m trying to swap is called frenpet, at 0xFF0C532FDB8Cd566Ae169C1CB157ff2Bdc83E105. Interestingly, using BaseSwap for the same token swap works with 5% slippage which is where liquidity was created. I have baseswap v2, uniswap v2 & v3, sushiswap v2, pancakeswap v2 & v3 deployed as adapters.

Any advice or pointers you could provide would be greatly appreciated. I'm really lost.

Was the first issue related to fee on transfer? If not, it could relate to the same pool being used twice (can happen with pool with multiple assets)

@RicqCodes
Copy link

  1. Also, when dealing with tokens that have known fees, I keep getting the same insufficient amount-out error, no matter how I adjust the amountOut, even setting it to 0. It just doesn’t seem to work at all.

Hey, @RicqCodes thanks for reaching out! Sorry to hear you had no help on tg, personally not active there very much.

Tokens with fee on transfer are not supported with this version of the router due to the way swap execution works. Swap execution runs the query before each swap which doesn't account for the fee on transfer - the query calculated amount will be higher than the actual amount.

This has been discussed for some time now and I agree that it would be more practical to change the swap logic in favour of checking the balance after each swap instead of quoting. Would you be interested in implementing this?

Certainly! Here's a more professional rewrite suitable for a GitHub comment:


Hello @halo3mic,

Thank you for your quick reply. I've found a partial workaround for this issue. It effectively handles swaps from ETH to tokens with fees, but unfortunately, it doesn’t work for swapping in the reverse direction.

Based on the results from findBestPathWithGas, we already have the final amountOut, which allows us to calculate slippage directly. I've adapted the swap function in the base adapter to accept the amountOut specified in Trade enums, which seems to work well given that we ensure the amountOut is adequate.

However, when attempting to swap back to ETH, we encounter the k error, as described in the Uniswap documentation. The documentation recommends using “SupportingFeeOnTransfer” from their router, which could pose integration challenges with our yakrouter interacting with the pair contract.

I would appreciate any insights or suggestions you might have on this matter, especially regarding implementing the recommended Uniswap approach.

@RicqCodes
Copy link

Also instead of using the pair address to swap, we could use the dex routers, what do you think?

@midnight-commit
Copy link
Contributor

However, when attempting to swap back to ETH, we encounter the k error, as described in the Uniswap documentation. The documentation recommends using “SupportingFeeOnTransfer” from their router, which could pose integration challenges with our yakrouter interacting with the pair contract.

Hi @RicqCodes,

Could it be that you are not taking into account that there are two transfers involved when you are swapping from FeeToken to X?

It goes User -> Adapter -> Pool, so it gets taxed twice I guess.

@halo3mic
Copy link
Member Author

@RicqCodes

The router that you referenced is a fork of YakSwap, so I would consult Artemis for help as they have their own version of the router and have possibly modified the adapters...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants