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

Combined swap and add/remove liquidity contract #26

Closed
wants to merge 16 commits into from

Conversation

moodysalem
Copy link
Contributor

@moodysalem moodysalem commented Apr 28, 2020

closes #22

@haydenadams haydenadams marked this pull request as ready for review April 29, 2020 15:45
@moodysalem
Copy link
Contributor Author

I realize this could be a lot more gas efficient, either by going to the pair directly or using the router ETH methods, but this unoptimized version works for now

@olivdb
Copy link

olivdb commented Jun 22, 2020

Some tokens like USDT do not allow approving an amount M > 0 when an existing amount N > 0 is already approved. This is to protect from an ERC20 attack vector described here https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit#heading=h.b32yfk54vyg9.

We are encountering some issues adding/removing liquidity with ExampleCombinedSwapAddRemoveLiquidity using such tokens. ExampleCombinedSwapAddRemoveLiquidity approves UniswapV2Router to pull some tokens from itself but UniswapV2Router does not always use its full allowance and sometimes leaves some residual allowance after the transaction. As a result, the next time ExampleCombinedSwapAddRemoveLiquidity tries to approve UniswapV2Router, the residual allowance makes the approval fail for tokens such as USDT.

A simple fix would probably be to just clear any residual allowance at the end of a call.

cc @elenadimitrova

@elenadimitrova
Copy link

Any progress on the above issue reported? It is blocking a lot of transactions atm and there's also the KNC token suffering the same symptoms on token.approve as USDT.

…ovals with the router (#38)

* Using MAX_UINT approvals with the router to avoid issues with tokens that don't support changing a non-zero approval

* Clearing the existing router allowance in the (unlikely) case the allowance has shrank to less than the required amount

* Trying to fix linter issue
@stale
Copy link

stale bot commented Jul 14, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Jul 14, 2022
@stale stale bot closed this Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swap and add liquidity
3 participants