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

block steps functions that create price raise #241

Open
ilanDoron opened this issue Jun 26, 2018 · 1 comment
Open

block steps functions that create price raise #241

ilanDoron opened this issue Jun 26, 2018 · 1 comment

Comments

@ilanDoron
Copy link
Contributor

ilanDoron commented Jun 26, 2018

Too high rate due to cached rate: In case an exchange exists a reserve which has a (partially) increasing
conversion rate in relation to the exchanged source quantity, then the user can reach a better rate using
maxDestAmount. Consider the conversion rate of an exchange as shown below:

can't paste graph example :(

In case of a trade request for 2 ETH → KNC, maxDestAmount = 500 KNCs, the chosen rate will be 1250,
as the red dot is the chosen point on the curve. Afterwards, only 500 KNCs will be emitted resulting in a
price of 0.4 ETH.
In the optimal case, the green point would have been chosen, as it suffices to exchange 500 KNCs. In
that case, the fair rate would have been 750 and hence the price would have been 0.66 ETH.
Hence, the user is able to enforce a higher rate than expected for reserves with a (partially) increasing
conversion rate by making a trade request with a large source amount.
Likelihood: Medium
Impact: Medium
1.8 Failing transaction due to rounding issue M
During the audit, the following code was modified:
125
126 require(reportedDestAmount == tradeOutcome.userDeltaDestAmount);
127 require(tradeOutcome.actualRate >= minConversionRate);
128
129 ExecuteTrade(msg.sender, src, dest, tradeOutcome.userDeltaSrcAmount, tradeOutcome.userDeltaDestAmount);

KyberNetworkProxy.sol

This new code can lead to a failure of a valid transaction. To explain, we provide the following example:
• One exchange offers an ETH-KNC rate of 1.1
• A user tries to exchange 300 wei into KNC with a minimum conversion rate of 1.1 and a maximum
destination amount of 218 KNC twei.
• Due to the maximum destination amount, only 199 wei will be exchanged.
• Hence, the actualRate computed for the line above will be 218

199 and therefore smaller than the minimum

conversion rate of 1.1.
• The proxy reverts the transaction.

As the example shows, a satisfiable transaction can fail, due to rounding issues. The likelihood of occurrence depends on the frequency with which the maximum destination amount is set and the conversion rates

of the reserves.
Likelihood:

@manhlx3006
Copy link
Contributor

Addressed in #422

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

No branches or pull requests

2 participants