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

[Error] Bancor Protocol #3032

Closed
hyunwoongJi opened this issue May 14, 2018 · 6 comments

Comments

@hyunwoongJi
Copy link

@hyunwoongJi hyunwoongJi commented May 14, 2018

According to Bancor Protocol, balance seems to be set as a wrong value

real_type C(c.balance.amount+in.amount); -> real_type C(c.balance.amount);

real_type C(c.balance.amount+in.amount);

Also, supply seems to be set as a wrong value.

real_type R(supply.amount - in.amount); -> real_type R(supply.amount);

real_type R(supply.amount - in.amount);

if connector weight was intended to be 50%, the number "1000.0" should be "1.0" (since c.weight = .5 in exchange_state.hpp)

real_type F(c.weight/1000.0);

real_type F(1000.0/c.weight);

@bytemaster

This comment has been minimized.

Copy link
Contributor

@bytemaster bytemaster commented May 14, 2018

Connector weight was not intended to be 50%, but .1%.

The Bancor Protocol math is wrong in their paper, absent the "+ in.amount" and "- in.amount" the protocol can be gamed. Extensive tests have verified the math cannot be gamed and always sees error approaching 0 when higher precision numbers are used.

@bytemaster bytemaster closed this May 14, 2018
@hyunwoongJi

This comment has been minimized.

Copy link
Author

@hyunwoongJi hyunwoongJi commented May 15, 2018

@bytemaster

Yes the Bancor Protocol math is wrong in their "white paper"

If you wanted to fix it, you had to fix the following three lines:

real_type C(c.balance.amount+in.amount);

real_type R(supply.amount - in.amount);

real_type T = C * (std::pow( ONE + E/R, F) - ONE);

It should be as follows:

real_type C(c.balance.amount); 
real_type R(supply.amount); 
real_type T = -1 * C * (std::pow( ONE - E/R, F) - ONE);

It will solve the problem

To see why this could be urgent, I append the following spread sheet. spread sheet

To apply Bancor protocol, initial total supply of ram and connected balance (in this case EOS) should be configured. This could be another governance issue to block producers.

@sergioyuhjtman

This comment has been minimized.

Copy link

@sergioyuhjtman sergioyuhjtman commented May 15, 2018

I am reviewing carefully Bancor papers. When saying that there is an error, please, refer clearly to what formula from what document you are talking about. So far I didn't find any errors in Meni Rosenfeld's article https://drive.google.com/file/d/0B3HPNP-GDn7aRkVaV3dkVl9NS2M/view
(though it could have been done simpler without integrating).

@hyunwoongJi

This comment has been minimized.

Copy link
Author

@hyunwoongJi hyunwoongJi commented May 16, 2018

@sergioyuhjtman
I am sorry for the unclearness. And the paper you have linked has no error. It was "white paper" https://storage.googleapis.com/website-bancor/2018/04/01ba8253-bancor_protocol_whitepaper_en.pdf (second equation of page 11) that I referred to. (Now I can see that whether the equation is wrong or not depends on convention). The term "connected tokens paid out" in the LHS itself has negative sign therefore it should be multiplied by "-1". The same is true for the term "tokens destroyed" in the RHS. Thank you for reviewing this issue 👍

@sergioyuhjtman

This comment has been minimized.

Copy link

@sergioyuhjtman sergioyuhjtman commented May 17, 2018

@hyunwoongJi Thanks. Please read also here https://eosio.stackexchange.com/questions/317/is-there-a-math-error-in-bancor-paper to understand better what Dan Larimer has in mind. He is confused because he thinks that Rosenfeld's formulas are wrong. Since Dan's formulas make a difference in favor of the exchange, maybe they are good enough. But still I think it has to be reviewed carefully.

@hyunwoongJi

This comment has been minimized.

Copy link
Author

@hyunwoongJi hyunwoongJi commented May 17, 2018

@sergioyuhjtman Thank you for sharing. Now I can understand it. And thank you again for taking this issue carefully :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.