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
correct hasSaleEnded #11
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing your fix helped me to better understand the issue. Thanks!
I did not "Approve" because I have two suggested changes, but I am not OK with requiring them, so I'm also not marking "Request changes".
contracts/TokenSale.sol
Outdated
// and endtime has past, then sale has ended | ||
} else if (pausedTime == 0 && currentTime() >= endTime) { | ||
return true; | ||
// otherwise it not past and not paused; or paused |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest adding "is": "... otherwise it is not past and not paused ..."
contracts/TokenSale.sol
Outdated
// otherwise it not past and not paused; or paused | ||
// and as such not ended | ||
} else { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that hasSaleEnded
will return false if the sale is paused and we have past the original endTime
, updateWhitelist
should probably stet require(!hasSaleEnded())
, because otherwise the whitelist can be updated after the sale ends but before it is finalized.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great point; will make said changes.
…WhiteList to not-hasSaleEnded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
correct
hasSaleEnded
to correct for paused, but not yet ended sale condition