Crowdsale.buyTokens is now nonReentrant. #1438
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nventuro
added
contracts
|
The nonReentrant modifier has the following comment: We need to figure out first if that comment about not supported if it's not external makes sense. |
|
It has been sitting there since the original PR, where the implementation was very similar to today's: #171 |
come-maiz
approved these changes
Oct 19, 2018
come-maiz
pushed a commit
that referenced
this pull request
Oct 21, 2018
(cherry picked from commit 6d415c5)
vittominacori
added a commit
to vittominacori/zeppelin-solidity
that referenced
this pull request
Nov 21, 2018
…lidity into feature/token-recover * 'master' of https://github.com/vittominacori/zeppelin-solidity: (98 commits) Renamed roles private variables to adhere to code style. (OpenZeppelin#1507) Remove extraneous quantity check, fixes OpenZeppelin#1454 (OpenZeppelin#1455) Remove redundant require statements (OpenZeppelin#1409) Add the step to delete the build dir to the RELEASE notes (OpenZeppelin#1467) add an address typecast to this per issue OpenZeppelin#1457 (OpenZeppelin#1471) add improvement in simpletoken example OpenZeppelin#1458 (OpenZeppelin#1473) SafeMath Test Coverage Improved (OpenZeppelin#1477) The beneficiary parameter of claimRefund is replaced with refundee (OpenZeppelin#1481) fix ERC20.sol#L174 and ERC20.sol#L187 should be casted to an address type. (OpenZeppelin#1470) Fix/add comment erc721 burnable OpenZeppelin#1464 (OpenZeppelin#1469) Release v2.0.0 Release candidate v2.0.0-rc.4 Improved some ERC721 internal shenanigans (OpenZeppelin#1450) Add warning about trading tokens before refundable crowdsale goal is met (OpenZeppelin#1452) Crowdsale.buyTokens is now nonReentrant. (OpenZeppelin#1438) InitialRate must be strictly larger than finalRate. (OpenZeppelin#1441) Fixed how allowance crowdsale checks remaining tokens. (OpenZeppelin#1449) Deleted unnecessary import. (OpenZeppelin#1437) Made SampleCrowdsale a bit clearer. (OpenZeppelin#1448) Now setting the finalized flag before doing finalization to prevent possbile reentrancy issues. (OpenZeppelin#1447) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This doesn't affect anything right now, but potentially could, with standards such as ERC677, and is hard to think about. Forbidding reentrancy altogether disallows all kinds of nasty behaviors.
Thanks @cwhinfrey and the Level K team for spotting this!