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.

Sign up for GitHub

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

Move token creation outside of crowdsale contract #690

Merged
merged 2 commits into from
Jan 24, 2018
+39 −34
Merged

Move token creation outside of crowdsale contract #690

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70fd5e5
Move token creation outside of crowdsale contract
spalladino Jan 18, 2018
0fb1ffc
Merge branch 'master' into feature/remove_token_from_crowdsale
frangio Jan 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions contracts/crowdsale/Crowdsale.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import "../math/SafeMath.sol";
* Crowdsales have a start and end timestamps, where investors can make
* token purchases and the crowdsale will assign them tokens based
* on a token per ETH rate. Funds collected are forwarded to a wallet
* 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm agree to not enforce it, to acommodate for non-ownable tokens in the future as you say.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we comfortable with the owner being able to mint tokens arbitrarily before the crowdsale starts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

*/
contract Crowdsale {
using SafeMath for uint256;
Expand Down Expand Up @@ -41,17 +43,18 @@ contract Crowdsale {
event TokenPurchase(address indexed purchaser, address indexed beneficiary, uint256 value, uint256 amount);


function Crowdsale(uint256 _startTime, uint256 _endTime, uint256 _rate, address _wallet) public {
function Crowdsale(uint256 _startTime, uint256 _endTime, uint256 _rate, address _wallet, MintableToken _token) public {
require(_startTime >= now);
require(_endTime >= _startTime);
require(_rate > 0);
require(_wallet != address(0));
require(_token != address(0));

token = createTokenContract();
startTime = _startTime;
endTime = _endTime;
rate = _rate;
wallet = _wallet;
token = _token;
}

// fallback function can be used to buy tokens
Expand Down Expand Up @@ -83,12 +86,6 @@ contract Crowdsale {
return now > endTime;
}

// creates the token to be sold.
// override this method to have crowdsale of a specific mintable token.
function createTokenContract() internal returns (MintableToken) {
return new MintableToken();
}

// Override this method to have a way to add business logic to your crowdsale when buying
function getTokenAmount(uint256 weiAmount) internal view returns(uint256) {
return weiAmount.mul(rate);
Expand Down
9 changes: 2 additions & 7 deletions contracts/examples/SampleCrowdsale.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,14 @@ contract SampleCrowdsaleToken is MintableToken {
*/
contract SampleCrowdsale is CappedCrowdsale, RefundableCrowdsale {

function SampleCrowdsale(uint256 _startTime, uint256 _endTime, uint256 _rate, uint256 _goal, uint256 _cap, address _wallet) public
function SampleCrowdsale(uint256 _startTime, uint256 _endTime, uint256 _rate, uint256 _goal, uint256 _cap, address _wallet, MintableToken _token) public
CappedCrowdsale(_cap)
FinalizableCrowdsale()
RefundableCrowdsale(_goal)
Crowdsale(_startTime, _endTime, _rate, _wallet)
Crowdsale(_startTime, _endTime, _rate, _wallet, _token)
{
//As goal needs to be met for a successful crowdsale
//the value needs to less or equal than a cap which is limit for accepted funds
require(_goal <= _cap);
}

function createTokenContract() internal returns (MintableToken) {
return new SampleCrowdsaleToken();
}

}
5 changes: 3 additions & 2 deletions contracts/mocks/CappedCrowdsaleImpl.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ contract CappedCrowdsaleImpl is CappedCrowdsale {
uint256 _endTime,
uint256 _rate,
address _wallet,
uint256 _cap
uint256 _cap,
MintableToken _token
) public
Crowdsale(_startTime, _endTime, _rate, _wallet)
Crowdsale(_startTime, _endTime, _rate, _wallet, _token)
CappedCrowdsale(_cap)
{
}
Expand Down
5 changes: 3 additions & 2 deletions contracts/mocks/FinalizableCrowdsaleImpl.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ contract FinalizableCrowdsaleImpl is FinalizableCrowdsale {
uint256 _startTime,
uint256 _endTime,
uint256 _rate,
address _wallet
address _wallet,
MintableToken _token
) public
Crowdsale(_startTime, _endTime, _rate, _wallet)
Crowdsale(_startTime, _endTime, _rate, _wallet, _token)
{
}

Expand Down
5 changes: 3 additions & 2 deletions contracts/mocks/RefundableCrowdsaleImpl.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ contract RefundableCrowdsaleImpl is RefundableCrowdsale {
uint256 _endTime,
uint256 _rate,
address _wallet,
uint256 _goal
uint256 _goal,
MintableToken _token
) public
Crowdsale(_startTime, _endTime, _rate, _wallet)
Crowdsale(_startTime, _endTime, _rate, _wallet, _token)
RefundableCrowdsale(_goal)
{
}
Expand Down
6 changes: 3 additions & 3 deletions test/crowdsale/CappedCrowdsale.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ contract('CappedCrowdsale', function ([_, wallet]) {
this.startTime = latestTime() + duration.weeks(1);
this.endTime = this.startTime + duration.weeks(1);

this.crowdsale = await CappedCrowdsale.new(this.startTime, this.endTime, rate, wallet, cap);

this.token = MintableToken.at(await this.crowdsale.token());
this.token = await MintableToken.new();
this.crowdsale = await CappedCrowdsale.new(this.startTime, this.endTime, rate, wallet, cap, this.token.address);
await this.token.transferOwnership(this.crowdsale.address);
});

describe('creating a valid crowdsale', function () {
Expand Down
6 changes: 3 additions & 3 deletions test/crowdsale/Crowdsale.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) {
this.endTime = this.startTime + duration.weeks(1);
this.afterEndTime = this.endTime + duration.seconds(1);

this.crowdsale = await Crowdsale.new(this.startTime, this.endTime, rate, wallet);

this.token = MintableToken.at(await this.crowdsale.token());
this.token = await MintableToken.new();
this.crowdsale = await Crowdsale.new(this.startTime, this.endTime, rate, wallet, this.token.address);
await this.token.transferOwnership(this.crowdsale.address);
});

it('should be token owner', async function () {
Expand Down
8 changes: 5 additions & 3 deletions test/crowdsale/FinalizableCrowdsale.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ contract('FinalizableCrowdsale', function ([_, owner, wallet, thirdparty]) {
this.endTime = this.startTime + duration.weeks(1);
this.afterEndTime = this.endTime + duration.seconds(1);

this.crowdsale = await FinalizableCrowdsale.new(this.startTime, this.endTime, rate, wallet, { from: owner });

this.token = MintableToken.at(await this.crowdsale.token());
this.token = await MintableToken.new();
this.crowdsale = await FinalizableCrowdsale.new(
this.startTime, this.endTime, rate, wallet, this.token.address, { from: owner }
);
await this.token.transferOwnership(this.crowdsale.address);
});

it('cannot be finalized before ending', async function () {
Expand Down
7 changes: 6 additions & 1 deletion test/crowdsale/RefundableCrowdsale.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require('chai')
.should();

const RefundableCrowdsale = artifacts.require('mocks/RefundableCrowdsaleImpl.sol');
const MintableToken = artifacts.require('MintableToken');

contract('RefundableCrowdsale', function ([_, owner, wallet, investor]) {
const rate = new BigNumber(1000);
Expand All @@ -28,7 +29,11 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor]) {
this.endTime = this.startTime + duration.weeks(1);
this.afterEndTime = this.endTime + duration.seconds(1);

this.crowdsale = await RefundableCrowdsale.new(this.startTime, this.endTime, rate, wallet, goal, { from: owner });
this.token = await MintableToken.new();
this.crowdsale = await RefundableCrowdsale.new(
this.startTime, this.endTime, rate, wallet, goal, this.token.address, { from: owner }
);
await this.token.transferOwnership(this.crowdsale.address);
});

describe('creating a valid crowdsale', function () {
Expand Down
7 changes: 5 additions & 2 deletions test/examples/SampleCrowdsale.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ contract('SampleCrowdsale', function ([owner, wallet, investor]) {
this.endTime = this.startTime + duration.weeks(1);
this.afterEndTime = this.endTime + duration.seconds(1);

this.crowdsale = await SampleCrowdsale.new(this.startTime, this.endTime, RATE, GOAL, CAP, wallet);
this.token = SampleCrowdsaleToken.at(await this.crowdsale.token());
this.token = await SampleCrowdsaleToken.new();
this.crowdsale = await SampleCrowdsale.new(
this.startTime, this.endTime, RATE, GOAL, CAP, wallet, this.token.address
);
await this.token.transferOwnership(this.crowdsale.address);
});

it('should create crowdsale with correct parameters', async function () {
Expand Down