reentrancy mutex gas optimization #1155
Conversation
nventuro
added
feature
There was a problem hiding this comment.
Thanks @yaronvel, I love this implementation! Extremely simple yet effective and cheap.
The gas costs during a revert may be higher when compared to the original implementation, since this will revert after the fact, as opposed to checking before the reentrancy occurs. I think this is a non-issue though, since we should only care about gas costs for the regular case.
contracts/ReentrancyGuard.sol
Outdated
| */ | ||
| uint private reentrancyLock = REENTRANCY_GUARD_FREE; | ||
| /// @dev counter to allow mutex lock with only one SSTORE operation | ||
| uint private guardCounter = 1; |
There was a problem hiding this comment.
Could you make the actual type explicit with uint256?
contracts/ReentrancyGuard.sol
Outdated
| @@ -30,10 +21,9 @@ contract ReentrancyGuard { | |||
| * wrapper marked as `nonReentrant`. | |||
| */ | |||
| modifier nonReentrant() { | |||
| require(reentrancyLock == REENTRANCY_GUARD_FREE); | |||
| reentrancyLock = REENTRANCY_GUARD_LOCKED; | |||
| uint localCounter = ++guardCounter; | |||
There was a problem hiding this comment.
I think this would read better if we split the increment and the assignment, the pre-increment syntax might be a bit too magic for something as sensitive as this.
guardCounter += 1;
uint256 localCounter = guardCounter;
|
@nventuro did the changes you asked for. |
|
Awesome work @yaronvel, thanks a lot! |
Implementation of reentrancy mutex that is using only one SSTORE operation.
Expected to reduce the original mutex implementation by half. From roughly 10k gas to 5k gas.