Skip to content

Conversation

@liquid-8
Copy link
Member

@liquid-8 liquid-8 commented Jul 4, 2021

Closes #40 #130

@liquid-8
Copy link
Member Author

liquid-8 commented Jul 4, 2021

Comments kinda trivial but whatever.
Router contract has according methods for exact input only so there is no need to rework make_trade_output() as it wraps exact output methods.
If we need these calls for swap then usage gonna be like
make_trade(token0, token1, qty, ... fee_on_transfer=True)
If we don't then just omit fee_on_transfer param.
Also, please note that univ3 doesn't and won't support this kind of tokens.

Copy link
Member

@ErikBjare ErikBjare left a comment

Choose a reason for hiding this comment

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

Nice work! A few comments.

And also: Huge thanks for helping with replying to issues, much appreciated!

@ErikBjare
Copy link
Member

Do the tests pass for you locally? (CI is not working right for PRs... #88)

liquid-8 and others added 5 commits July 5, 2021 14:38
Co-authored-by: Erik Bjäreholt <erik.bjareholt@gmail.com>
Co-authored-by: Erik Bjäreholt <erik.bjareholt@gmail.com>
Co-authored-by: Erik Bjäreholt <erik.bjareholt@gmail.com>
@liquid-8
Copy link
Member Author

liquid-8 commented Jul 5, 2021

I like your suggestions about method selection but can't commit them as they are marked "outdated" dunno why. Tests are OK.

Co-authored-by: Erik Bjäreholt <erik.bjareholt@gmail.com>
@ErikBjare
Copy link
Member

ErikBjare commented Jul 5, 2021

Nice! Just one more thing: run black on the files to apply correct formatting.

After that I'll merge 🙂

Copy link
Member

@ErikBjare ErikBjare left a comment

Choose a reason for hiding this comment

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

Not sure what happened, but some lines were duplicated.

@liquid-8
Copy link
Member Author

liquid-8 commented Jul 5, 2021

Nice! Just one more thing: run black on the files to apply correct formatting.

After that I'll merge 🙂

Bruh I swear I didn't got ya🤷‍♀️

@liquid-8
Copy link
Member Author

liquid-8 commented Jul 7, 2021

Nice! Just one more thing: run black on the files to apply correct formatting.

After that I'll merge 🙂

+++
Seems OK

@debaenenicolas
Copy link

Hello,
First thanks for the great repo you built.

I tried to buy reflection tokens with make_trade(token0, token1, qty, ... fee_on_transfer=True) but I still get an error:
"reason : transaction failed"

Do you have an idea why?

Thanks

@liquid-8
Copy link
Member Author

liquid-8 commented Jul 7, 2021

still get an error:
"reason : transaction failed"
Do you have an idea why?

Well, this PR hasn't been merged yet.

@ErikBjare
Copy link
Member

LGTM, merging.

Thanks for your contributions @liquid-8!

@ErikBjare ErikBjare changed the title enhancement: added swapExact...SupportingFeeOnTransferTokens() support feat: added swapExact...SupportingFeeOnTransferTokens() support Jul 7, 2021
@ErikBjare ErikBjare merged commit c2cc36c into uniswap-python:master Jul 7, 2021
@liquid-8
Copy link
Member Author

liquid-8 commented Jul 8, 2021

Hello,
First thanks for the great repo you built.

I tried to buy reflection tokens with make_trade(token0, token1, qty, ... fee_on_transfer=True) but I still get an error:
"reason : transaction failed"

Do you have an idea why?

Thanks

@debaenenicolas if problem still persists please create thread in Discussions.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for swapExactTokensForTokensSupportingFeeOnTransferTokens

3 participants