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

upgrade to new KyberNetworkProxy contract, fixes #12 #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mohamedalichelbi
Copy link

No description provided.

@mohamedalichelbi mohamedalichelbi changed the title upgrade to new KyberNetworkProxy contract upgrade to new KyberNetworkProxy contract, fixes #12 Jan 7, 2021
@jordanmcdougall
Copy link

This PR fixed #13 for me - thanks @mohamedalichelbi!

Copy link

@RAPHAELSTZ RAPHAELSTZ left a comment

Choose a reason for hiding this comment

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

Good job !

@AngelLozan
Copy link

Excellent thank you! This was a great fix @mohamedalichelbi

I appreciate you putting this out there. How did you discover this? I'd love to know your thought process.

@ccoop1
Copy link

ccoop1 commented Jul 30, 2021

Just wanted to confirm this also fixed my issue. There were so many others from the start I was able to walk myself through but this one was tricky. Several places to edit. Thanks again! Working like a charm after about 15 edits haha!!

@KingEriic
Copy link

Hi, Devs!!

In order to complement the solution, it is also necessary to update the ABI, so as not to receive the error "Error: Invalid number of parameters for "getExpectedRate". Got 3 expected 4!". Then go to etherscan, search for the updated contract, go to the contract tab and find the "Contract ABI" below and update the code.

@Asthay97
Copy link

Asthay97 commented Nov 1, 2021

I was getting error : execution reverted for all tokens except MKR. This commit has resolved the error. Thanks :)

@anibaljasin
Copy link

This also solve the issue for me

Copy link

@Ab-hishek Ab-hishek left a comment

Choose a reason for hiding this comment

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

Thanks for this fix.

@loekTheDreamer
Copy link

Thanks a million dollars!

@krdiamond
Copy link

thanks so much for putting in the time to fix this!!


console.table([{
'Input Token': inputTokenSymbol,
'Output Token': outputTokenSymbol,
'Input Amount': web3.utils.fromWei(inputAmount, 'Ether'),
'Uniswap Return': web3.utils.fromWei(uniswapResult, 'Ether'),
'Kyber Expected Rate': web3.utils.fromWei(kyberResult.expectedRate, 'Ether'),
'Kyber Min Return': web3.utils.fromWei(kyberResult.slippageRate, 'Ether'),
'Kyber Min Return': web3.utils.fromWei(kyberResult.worstRate, 'Ether'),
Copy link

@Byusa Byusa Apr 10, 2022

Choose a reason for hiding this comment

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

Does mean that it will just pick the worst price instead of slippageRate as before? Is there a better way of doing this?
Otherwise, thanks for the good solution

Choose a reason for hiding this comment

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

oh wow that is very interesting, i didn't notice this...
do we have answer to your question yet?

@ccoop1
Copy link

ccoop1 commented Oct 11, 2022 via email

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.