search

tokamak-network / tokyo

tokyo monorepo
Apache License 2.0
4 stars 5 forks source link

claimTokens func added to BaseCrowdsale.sol #11

Closed ggs134 closed 6 years ago

ggs134 commented 6 years ago

Tests are not implememted. This pull request related to #11

shingonu commented 6 years ago

This line is dangerous, because tokenOwner can be anyone. https://github.com/ggs134/tokyo/blob/e95ce7d87c3a5b0b2ca54b3bd5cfecb5b8498c77/packages/tokyo-reusable-crowdsale/contracts/crowdsale/BaseCrowdsale.sol#L125

I think, It seems better to send the claimed ERC20 to the owner like this. https://github.com/OpenZeppelin/zeppelin-solidity/blob/a7e91856f3e275668b4a4c55cbd14864aa61b100/contracts/ownership/CanReclaimToken.sol

function claimTokens(ERC20Basic token) external onlyOwner {
    require(isFinalized);
    uint256 balance = token.balanceOf(this);
    token.transfer(owner, balance);
}
4000D commented 6 years ago

you may check Contributing guide

4000D commented 6 years ago

Because only owner can claimTokens, it is assumed that token owner is known address for the contract owner.

But we need to consider which interface should we use for claiming.

4000D commented 6 years ago

replaced with #12