Review #2
Review #2
Conversation
|
|
||
| // disable transfer | ||
| function transfer(address _to, uint _value) returns (bool) { | ||
| require(1==0); |
There was a problem hiding this comment.
have you decided to disable transfer?
In this case please use throw
| if (now > deadline || totalSupply > hardcap) { | ||
| active = false; | ||
| if (totalSupply < softcap) { | ||
| for (uint i = 0; i < balancesForReturn.length; ++i) { |
There was a problem hiding this comment.
Please, don't use for if you can avoid it.
Let's let everyone to refund ether by themselves.
In this case, you can use mapping instead of the array.
The length of the balancesForReturn can be so high, that amount of gas needed for the cycle can be more than block gas limit.
In this case, all gathered ether will be locked forever
|
|
||
| function buyTokens(address beneficiary) canBuy payable { | ||
|
|
||
| finishIfNeed(); |
There was a problem hiding this comment.
Please, remove the finishIfNeed call here, you can always invoke it manually.
In case when finishIfNeed will throw an exception, it will block every request to buyTokens function
| if (totalSupply < softcap) { | ||
| for (uint i = 0; i < balancesForReturn.length; ++i) { | ||
| balancesForReturn[i].addr.transfer(balancesForReturn[i].amount); | ||
| } |
There was a problem hiding this comment.
Refund and buy logic must be thoroughly covered by unit tests
You can use our git as an example
https://github.com/rootprojectco/backend/tree/master/test
| } | ||
|
|
||
| function finishIfNeed() { | ||
| if (now > deadline || totalSupply > hardcap) { |
There was a problem hiding this comment.
You are not guaranteed that totalSupply will be <= hardcap by this check
No description provided.