Move token creation outside of crowdsale contract #690
Conversation
|
We should also remove |
spalladino
force-pushed
the
feature/remove_token_from_crowdsale
branch
from
86fb1f4 to
70fd5e5
Compare
|
Ouch, missed it in the PR, my bad. Fixed now! |
| * as they arrive. | ||
| * as they arrive. The contract requires a MintableToken that will be | ||
| * minted as contributions arrive, note that the crowdsale contract | ||
| * must be owner of the token in order to be able to mint it. |
There was a problem hiding this comment.
We could enforce this in the Crowdsale constructor as:
require(_token.owner() == this);
On the other hand, we have had a lot of requests to be able to use the crowdsale with a non-mintable token, for example in a model where the crowdsale can transfer pre-minted tokens. #554 proposed to solve this problem by having a "Mintable" interface with a possible implementation where mint doesn't create tokens but transfers from a pre-minted pool. In this case the crowdsale doesn't need to be the token owner.
Just throwing this out there because we might have to keep it in mind in the near future, to move forward towards something like #554 (which I personally want us to do).
There was a problem hiding this comment.
Hmm agree to not enforce it, to acommodate for non-ownable tokens in the future as you say.
There was a problem hiding this comment.
Are we comfortable with the owner being able to mint tokens arbitrarily before the crowdsale starts?
There was a problem hiding this comment.
I'd say yes, since it can be verified by anyone by looking at the total supply right before the sale starts. Besides, it's perfectly possible to do that now, simply by extending the crowdsale contract and spawning a mintable token that allows more than one owner to mint.
|
We forgot to merge this for 1.6.0. 😢 |
@frangio any idea when will be the next release with this included? |
|
@medied Next release scheduled for Feb 20th, should include this! |
Fixes #358
npm run lint:all:fix) and fixed any issues.Fixes #358
🚀 Description
Remove the creation of the token contract from within the Crowdsale contract to save gas upon deployment. Note that crowdsale contracts compiled without optimisations were dangerously close to the block gas limit. See this comment for an analysis on gas cost savings via this PR.
This is a breaking change, since all usages of Crowdsale now require an extra parameter, and a manual setup of the token contract. I've noted this in the crowdsale contract documentation.