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

using SafeERC20 to implement safeTransfer in Crowdsale #1006

Conversation

dougiebuckets
Copy link
Contributor

Fixes #985

🚀 Description

Introduced safeTransfer to Crowdsale. Opened new pull request b/c I accidently closed this one: #985

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.

LGTM save for one unnecessary input. Thank you @dougiebuckets!

@@ -2,6 +2,8 @@ pragma solidity ^0.4.24;

import "../token/ERC20/ERC20.sol";
import "../math/SafeMath.sol";
import "../token/ERC20/ERC20Basic.sol";
Copy link
Contributor

Choose a reason for hiding this comment

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

This new import is not necessary.

Copy link
Contributor

@come-maiz come-maiz left a comment

Choose a reason for hiding this comment

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

Thank you! Leaving my +1 too, but we'll wait for you to remove the import before merging.

@dougiebuckets dougiebuckets force-pushed the chore/introduced-safeTransfer-to-Crowdsale-#945 branch from c3f1c73 to bd6241a Compare June 15, 2018 13:51
@dougiebuckets dougiebuckets force-pushed the chore/introduced-safeTransfer-to-Crowdsale-#945 branch from bd6241a to 6c4ef3d Compare June 15, 2018 13:59
@come-maiz
Copy link
Contributor

One more thing @dougiebuckets. There is a file .node-xmlhttprequest-sync-7601, can you please remove it?

@dougiebuckets
Copy link
Contributor Author

Good catch, @ElOpio. Thank you. Update made.

@frangio frangio merged commit 74a62a1 into OpenZeppelin:master Jun 16, 2018
@frangio
Copy link
Contributor

frangio commented Jun 16, 2018

Thanks @dougiebuckets!

nventuro pushed a commit to nventuro/openzeppelin-contracts that referenced this pull request Jun 18, 2018
…1006)

* introduced safeTransfer to Crowdsale

* Removed .node-xmlhttprequest-sync-7601
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.

3 participants