From fe01a150f086f8c0fcf5b76357f68b9a3d60fea7 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Sun, 1 Sep 2019 15:54:16 -0700
Subject: [PATCH] Use internal function in onlyOwner, onlyAuthorized, and
 nonReentrant modifiers

---
 .../utils/contracts/src/Authorizable.sol      | 14 ++++++++++---
 contracts/utils/contracts/src/Ownable.sol     | 19 ++++++++++++------
 .../utils/contracts/src/ReentrancyGuard.sol   | 20 +++++++++++++------
 3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/contracts/utils/contracts/src/Authorizable.sol b/contracts/utils/contracts/src/Authorizable.sol
index e14832d48b..e5132219b8 100644
--- a/contracts/utils/contracts/src/Authorizable.sol
+++ b/contracts/utils/contracts/src/Authorizable.sol
@@ -30,9 +30,7 @@ contract Authorizable is
 {
     /// @dev Only authorized addresses can invoke functions with this modifier.
     modifier onlyAuthorized {
-        if (!authorized[msg.sender]) {
-            LibRichErrors.rrevert(LibAuthorizableRichErrors.SenderNotAuthorizedError(msg.sender));
-        }
+        _assertSenderIsAuthorized();
         _;
     }
 
@@ -122,4 +120,14 @@ contract Authorizable is
     {
         return authorities;
     }
+
+    /// @dev Reverts if msg.sender is not authorized.
+    function _assertSenderIsAuthorized()
+        internal
+        view
+    {
+        if (!authorized[msg.sender]) {
+            LibRichErrors.rrevert(LibAuthorizableRichErrors.SenderNotAuthorizedError(msg.sender));
+        }
+    }
 }
diff --git a/contracts/utils/contracts/src/Ownable.sol b/contracts/utils/contracts/src/Ownable.sol
index 05aed815a4..c03fac3b24 100644
--- a/contracts/utils/contracts/src/Ownable.sol
+++ b/contracts/utils/contracts/src/Ownable.sol
@@ -17,12 +17,7 @@ contract Ownable is
     }
 
     modifier onlyOwner() {
-        if (msg.sender != owner) {
-            LibRichErrors.rrevert(LibOwnableRichErrors.OnlyOwnerError(
-                msg.sender,
-                owner
-            ));
-        }
+        _assertSenderIsOwner();
         _;
     }
 
@@ -36,4 +31,16 @@ contract Ownable is
             owner = newOwner;
         }
     }
+
+    function _assertSenderIsOwner()
+        internal
+        view
+    {
+        if (msg.sender != owner) {
+            LibRichErrors.rrevert(LibOwnableRichErrors.OnlyOwnerError(
+                msg.sender,
+                owner
+            ));
+        }
+    }
 }
diff --git a/contracts/utils/contracts/src/ReentrancyGuard.sol b/contracts/utils/contracts/src/ReentrancyGuard.sol
index 550e94ac90..78dc68e0ea 100644
--- a/contracts/utils/contracts/src/ReentrancyGuard.sol
+++ b/contracts/utils/contracts/src/ReentrancyGuard.sol
@@ -30,20 +30,28 @@ contract ReentrancyGuard {
     /// @dev Functions with this modifer cannot be reentered. The mutex will be locked
     ///      before function execution and unlocked after.
     modifier nonReentrant() {
+        _lockMutexOrThrowIfAlreadyLocked();
+        _;
+        _unlockMutex();
+    }
+
+    function _lockMutexOrThrowIfAlreadyLocked()
+        internal
+    {
         // Ensure mutex is unlocked.
         if (locked) {
             LibRichErrors.rrevert(
                 LibReentrancyGuardRichErrors.IllegalReentrancyError()
             );
         }
-
-        // Lock mutex before function call
+        // Lock mutex.
         locked = true;
+    }
 
-        // Perform function call
-        _;
-
-        // Unlock mutex after function call
+    function _unlockMutex()
+        internal
+    {
+        // Unlock mutex.
         locked = false;
     }
 }