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

Bonding Curve aka Automatic Market Maker Contract #819

Closed
5 of 6 tasks
balasan opened this issue Mar 15, 2018 · 8 comments
Closed
5 of 6 tasks

Bonding Curve aka Automatic Market Maker Contract #819

balasan opened this issue Mar 15, 2018 · 8 comments

Comments

@balasan
Copy link

balasan commented Mar 15, 2018

🎉 Description

I'm working on a PR to add a simple bonding curve contract (aka automatic market maker) based on Bancor formula. Here is the reference contract: https://github.com/relevant-community/contracts/blob/master/contracts/BondingCurve.sol

I have a few questions about structure and dependencies:

  • Folder structure —where should this contract go? /token? a separate /bondingcurve folder?

Planning on adding a curation-markets folder width BancorFormula.sol and BondingCurve.sol. Adding Power.sol function to the math folder.

Extracting the power function into a separate Power.sol (licensed under Apache 2.0) file in the /math folder.

Renaming this to Power.test.js. The file is very lengthy — its iterating through all the constants generated via python — do we want to keep it?

  • license - Bancor code is licensed under Apache License v2.0 and OpenZeppelin is MIT, is there a conflict here? do we need some special notice when we use Bancor code?

Adding a an Apache 2.0 License and modification notice to the BancorFormula.sol and Power.sol files with links to the Bancor repo.


  • 🐛 This is a bug report.
  • 📈 This is a feature request.

👍 Other Information

Some background reading about bonding curves:
https://hackernoon.com/how-to-make-bonding-curves-for-continuous-token-models-3784653f8b17
https://medium.com/@simondlr/tokens-2-0-curved-token-bonding-in-curation-markets-1764a2e0bee5
and Bancor formula:
https://drive.google.com/file/d/0B3HPNP-GDn7aRkVaV3dkVl9NS2M/view

@shrugs
Copy link
Contributor

shrugs commented Mar 15, 2018

  • looks like we've been putting random stuff like that at the top level (see Bounty, DayLimit, ECRecovery). so I'm cool with putting it there until we have a reason to give it its own folder
  • wew that's a lot of extra stuff. My opinion is to link it out and simply have the tested/audited stuff in OZ, but that's not a strong opinion. it also means we'd have to train maintainers on how to maintain that code as well, which doesn't seem exciting.
  • yeah we'd want the tests as well
  • I don't know enough about the licensing to comment
  • thanks for the extra bonding curve links!

cc @maraoz for the rest of these questions?

@balasan
Copy link
Author

balasan commented Mar 15, 2018

thanks @shrugs!
One advantage of a separate folder is to keep BondingCurve.sol and BancorFormula.sol close together

One more note:
I could move the exponentiation power function from the BancorFormula into a separate contract in the math folder. This will make BancorFormula contract very simple (could even merge it into a single BondingCurve.sol) and all the complex python constants will live in power. This will also make power usable in other contracts—for example continuous inflation.

@shrugs
Copy link
Contributor

shrugs commented Mar 15, 2018

I hadn't actually looked at those files, so now that makes a lot of sense. I like the Power.sol + BondingCurve.sol approach. Only downside is that now it's not 1:1 with the existing bancor stuff. If we want to keep it as BancorFormula and BondingCurve, I'd support another folder called something like curation-markets or similar, where we can put generic TCRs as well.

@balasan
Copy link
Author

balasan commented Mar 15, 2018

Cool, yeah I like that too. Especially since BancorFormula.sol it won't be 1:1 anyway if we want to use Zeppelin's math library instead Bancor's Utils.sol.

Will get started with this approach unless anyone else objects.

@balasan
Copy link
Author

balasan commented Mar 15, 2018

RE: license, I'm looking at this: https://softwareengineering.stackexchange.com/questions/297217/using-code-under-apache-2-0-license-in-my-program-using-mit-license

Seems like the best thing to do is keep stuff from Bancor separate from the Zeppelin stuff. Seems like the best way to do it would be separate Power.sol, BancorFunction.sol files with a license note at the top and separate BondingCurve.sol that would fall under the default Zeppelin license. (Unless we just make a single BondingCurve.sol licensed under Apache... 🤔

@shrugs
Copy link
Contributor

shrugs commented Mar 16, 2018

I can't really comment on the validity of any licensing approaches, but I like the idea of putting Power and Bancor stuff into apache compliance with a header and keeping BondingCurve under the Zeppelin license (MIT).

Can you update the post at the top to describe the changes we've figured out (namely folder structure, which files are added, licensing)

@balasan balasan mentioned this issue Mar 20, 2018
4 tasks
@balasan
Copy link
Author

balasan commented Mar 20, 2018

PR is in!
In case the Power.sol function is too much of a maintenance/trust risk for OpenZeppelin, I extracted the bonding curve code to a separate repo: https://github.com/relevant-community/bonding-curve

Also note: there is a TODO to add a minReturn parameter to the buy and sell functions — this is to prevent front-running by miners. This may be another reason to hold off merging into Zeppelin repo and work on this in the dedicated one.

@nventuro
Copy link
Contributor

Closing due to staleness. Also, @balasan has proposed an alternative repository where this can be found.

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 a pull request may close this issue.

3 participants