From 793e338dd3ebd84c2de9fbade3c63bc0c1ac39dc Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 25 Aug 2019 17:06:52 -0700 Subject: [PATCH] Revert to old ReentrancyGuard implementation --- .../utils/contracts/src/ReentrancyGuard.sol | 26 +++++++++++-------- .../contracts/test/TestReentrancyGuard.sol | 11 ++------ 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/contracts/utils/contracts/src/ReentrancyGuard.sol b/contracts/utils/contracts/src/ReentrancyGuard.sol index c0b7e4750a..550e94ac90 100644 --- a/contracts/utils/contracts/src/ReentrancyGuard.sol +++ b/contracts/utils/contracts/src/ReentrancyGuard.sol @@ -24,22 +24,26 @@ import "./LibRichErrors.sol"; contract ReentrancyGuard { - // Mutex counter. - // Starts at 1 and increases whenever a nonReentrant function is called. - uint256 private reentrancyGuardCounter = 1; + // Locked state of mutex. + bool private locked = false; - /// @dev Functions with this modifer cannot be reentered. + /// @dev Functions with this modifer cannot be reentered. The mutex will be locked + /// before function execution and unlocked after. modifier nonReentrant() { - // Increment and remember the current counter value. - uint256 localCounter = ++reentrancyGuardCounter; - // Call the function. - _; - // If the counter value is different from what we remember, the function - // was called more than once and an illegal reentrancy occured. - if (localCounter != reentrancyGuardCounter) { + // Ensure mutex is unlocked. + if (locked) { LibRichErrors.rrevert( LibReentrancyGuardRichErrors.IllegalReentrancyError() ); } + + // Lock mutex before function call + locked = true; + + // Perform function call + _; + + // Unlock mutex after function call + locked = false; } } diff --git a/contracts/utils/contracts/test/TestReentrancyGuard.sol b/contracts/utils/contracts/test/TestReentrancyGuard.sol index 7a878fd072..3bf6d3d274 100644 --- a/contracts/utils/contracts/test/TestReentrancyGuard.sol +++ b/contracts/utils/contracts/test/TestReentrancyGuard.sol @@ -24,8 +24,6 @@ import "../src/ReentrancyGuard.sol"; contract TestReentrancyGuard is ReentrancyGuard { - uint256 internal counter = 2; - /// @dev Exposes the nonReentrant modifier publicly. /// @param shouldBeAttacked True if the contract should be attacked. /// @return True if successful. @@ -47,19 +45,14 @@ contract TestReentrancyGuard is external returns (bool) { - if (counter > 0) { - counter--; - this.guarded(true); - } else { - counter = 2; - return true; - } + return this.guarded(true); } /// @dev This is a function that will not reenter the current contract. /// @return True if successful. function nonExploitive() external + pure returns (bool) { return true;