ReentrancyGuard gas usage optimization

[Eenae]

🎉 Description

ReentrancyGuard is very useful contract to prevent reentrancy attack and is actively used. But there is associated performance (gas usage) penalty: about 25k gas per invocation. This could be optimized to 10k gas pre invocation: 2.5x times. A PR will follow.

• 📈 This is a feature request.

💻 Environment

• OpenZeppelin v1.10.0
• Deploying to Ganache v6.1.0
• Deploying with truffle v4.1.11

📝 Details

The problem with the current implementation is that it uses boolean flag and boolean value false is stored in contract storage as 0. Changing storage cell content from 0 to non-zero costs 20k gas and it's happening on each invocation of nonReentrant modifier, because at the end of each invocation cell content is changed back to zero. As a result, each call to nonReentrant costs 25k gas: 20k for writing to zeroed storage cell and 5k for writing to non-zero storage cell.

Proposed solution is to never return to zero. Each call to nonReentrant will use non-zero integer constants to represent state of the guard.

🔢 Code To Reproduce Issue

It could be seen in Remix that execution cost of each call to Test.test() is 25929 gas.

pragma solidity ^0.4.24;


/**
 * @title Helps contracts guard agains reentrancy attacks.
 * @author Remco Bloemen <remco@2π.com>
 * @notice If you mark a function `nonReentrant`, you should also
 * mark it `external`.
 */
contract ReentrancyGuard {

  /**
   * @dev We use a single lock for the whole contract.
   */
  bool private reentrancyLock = false;

  /**
   * @dev Prevents a contract from calling itself, directly or indirectly.
   * @notice If you mark a function `nonReentrant`, you should also
   * mark it `external`. Calling one nonReentrant function from
   * another is not supported. Instead, you can implement a
   * `private` function doing the actual work, and a `external`
   * wrapper marked as `nonReentrant`.
   */
  modifier nonReentrant() {
    require(!reentrancyLock);
    reentrancyLock = true;
    _;
    reentrancyLock = false;
  }

}

contract Test is ReentrancyGuard {
    function test() external nonReentrant {
    }
}

👍 Other Information

PR will follow.

[BrendanChou]

It is also important to note that this only works for small transactions. The write back to zero is 5k gas with a 15k gas refund, which means that for large transactions, the cost for the boolean implementation is still 10k gas in total.

The reason the integer implementation saves gas is because gas refunds can only refund up to a certain % of the total transaction, so for small transactions, the refund is not actually completely refunded.