Mint re-entrancy guard with optimizations. #131
Merged
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.
Ok, so as we know, the
_safeMint
function is NOT safe from re-entrancy. #92Ideally, most contracts should use
_mint
.Although we can warn about it in the comments / docs, many devs won't read it and will happily use
_safeMint
as it is.For security reasons, I won't cite any examples here.
Furthermore, the README example shows
_safeMint
being used in an unprotected manner.From a usability point of view, I suggest that we make the
_mint
function re-entrancy safe regardless, to be foolproof.This will also reduce the cognitive dissonance of
_safeMint
being unsafe, giving a cleaner API design.The overhead is only 100 gas for an extra warm SLOAD.
Assuming a function using
_safeMint
costs ~60K gas, this is only 0.2% overhead that can protect against catastrophic consequences. For users who use_mint
, the gas overhead is as good as none.To ease the concerns for the increase in gas for
_safeMint
, the function is optimized even further, such that we can save more gas to cover for the overhead.In practice, implementations that need to use
_safeMint
for some reason can save 2k gas (cold SLOAD) with this functionality, as they don't need to use OpenZepplin's Reentrancy guard.Before:
After re-entrancy guard + gas optimizations:
@cygaar @ahbanavi