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

Feature: Ability to use crowdsale contract with pre minted tokens #554

Closed
wants to merge 11 commits into from
Closed

Feature: Ability to use crowdsale contract with pre minted tokens #554

wants to merge 11 commits into from

Conversation

MADANA-HQ
Copy link

@MADANA-HQ MADANA-HQ commented Nov 11, 2017

Legal issues and perhaps other reasons such as pre sales require the need to use the crowdsale contract with already minted tokens. Currently the zeppelin-solidity repo does not offer a solution for this problem. I have developed a small contract which can be assigned tokens with the StandardTokens approve function and therefore gains pseudo minting abilities. This contract can be passed on to the Crowdsale contract, allowing it to sell the already minted tokens.
Because of the various important steps needed to be done outside of the crowdsale contract, I opted to create an example including a vault contract in which the that outside logic is shown.
Further I created a Mintable interface, which replaces the MintableToken inside the Crowdsale contract. This is not necessary and possibly creates backwards compatibility issues, however I chose to add an interface for better code structure and to prevent misconception.

Fixes #351.

@frangio
Copy link
Contributor

frangio commented Nov 22, 2017

Actually the Mintable interface is a great idea! There have been several approaches to this problem before and this idea ha never come up (see #528 for example) but it's obvious now that it should be this way.

We're making a release today so I can't look at it in detail no but we'll do it soon.

Thanks @MADANA-HQ!

@shrugs
Copy link
Contributor

shrugs commented Nov 27, 2017

I like this; would simplify a lot of usecases that currently require copy-pasting Crowdsale and changing the token interface. Does composite crowdsale also solve this issue, though?

@fnerdman
Copy link

@shrugs As far as I understand, the CompositeCrowdsale does not solve this issue. I believe though that they would fit together by combining the CompositeCrowdsale with a DistributionStrategy which then pseudo mints tokens via the PseudoMinter contract.

@theethernaut theethernaut added review good first issue Low hanging fruit for new contributors to get involved! feature New contracts, functions, or helpers. and removed review good first issue Low hanging fruit for new contributors to get involved! labels Jan 2, 2018
@theethernaut theethernaut added backlog and removed next labels Jan 16, 2018
@fnerdman
Copy link

fnerdman commented Feb 1, 2018

@frangio I've updated this PR to include the changes form #690.

@fnerdman
Copy link

fnerdman commented Feb 1, 2018

@frangio Since #690 implemented a breaking change, I'd suggest to merge this PR quickly since it implements another breaking change at the same location, changing the MintableToken in the Crowdsale constructor to the Mintable interface.

@theethernaut
Copy link
Contributor

@MADANA-HQ
Thanks for your suggestion. We've taken your idea and incorporated it in a general refactor of crowdsales.
If you're interested in following progress, please see #744.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New contracts, functions, or helpers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants