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

Add complex crowdsale example #331 #342

Merged
merged 3 commits into from
Jul 31, 2017

Conversation

jakub-wojciechowski
Copy link
Contributor

Resolves #331

Submitted together with SampleCrowdsaleToken to illustrate which components are needed to launch a crowdsale. Contains tests to demonstrate basic features of a crowdsale and to assure that combining different subclasses maintains their functionalities.

Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

This is great! Thanks @jakub-wojciechowski. A couple of comments.

Let's put both contracts (crowdsale and token) in the same SampleCrowdsale.sol file. I envision each file in examples/ to be like a self-contained tutorial which you can read from top to bottom to learn about a part of OpenZeppelin.

Keep in mind this will be where newcomers will come to learn, so the examples should be thoroughly documented and explain what each line does. You don't need to include that in this PR, though go ahead if you want to!

The test is great too. It'll be useful as an integration test of all crowdsale features.


string public constant name = "Sample Crowdsale Token";
string public constant symbol = "SCT";
uint256 public constant decimals = 18;
Copy link
Contributor

Choose a reason for hiding this comment

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

decimals should be uint8 according to ERC20. (I've just realized this is wrong in the SimpleToken example too.)

(await this.crowdsale.startBlock()).should.be.bignumber.equal(this.startBlock);
(await this.crowdsale.endBlock()).should.be.bignumber.equal(this.endBlock);
(await this.crowdsale.rate()).should.be.bignumber.equal(RATE);
(await this.crowdsale.wallet()).should.be.bignumber.equal(wallet);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd have to confirm but I'm pretty sure Truffle returns address typed values as strings, not BigNumber instances. The test works because addresses are numbers anyway and they get converted, but it would be best to compare them as strings.

@jakub-wojciechowski
Copy link
Contributor Author

Thx for the comments @frangio. I'm glad that you liked integration tests :)

@frangio frangio merged commit 115e7af into OpenZeppelin:master Jul 31, 2017
ProphetDaniel pushed a commit to classicdelta/Smart-Contracts that referenced this pull request Mar 9, 2018
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 this pull request may close these issues.

2 participants