From 7222a31d548695998a475c9661fa159ef45a0e88 Mon Sep 17 00:00:00 2001
From: Prince Allwin <princeallwin199@gmail.com>
Date: Thu, 27 Jul 2023 03:27:50 +0530
Subject: [PATCH] Add internal functions inside modifiers (#4472)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Ernesto García <ernestognw@gmail.com>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco <fg@frang.io>
---
 .changeset/swift-numbers-cry.md           |  5 +++
 contracts/governance/Governor.sol         | 25 ++++++++++-----
 contracts/proxy/utils/Initializable.sol   |  9 +++++-
 contracts/proxy/utils/UUPSUpgradeable.sol | 37 +++++++++++++++++------
 4 files changed, 57 insertions(+), 19 deletions(-)
 create mode 100644 .changeset/swift-numbers-cry.md

diff --git a/.changeset/swift-numbers-cry.md b/.changeset/swift-numbers-cry.md
new file mode 100644
index 00000000000..48afbd24503
--- /dev/null
+++ b/.changeset/swift-numbers-cry.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Governor`, `Initializable`, and `UUPSUpgradeable`: Use internal functions in modifiers to optimize bytecode size.
diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
index 94f29e95f37..a94b4b305e6 100644
--- a/contracts/governance/Governor.sol
+++ b/contracts/governance/Governor.sol
@@ -66,14 +66,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
      * governance protocol (since v4.6).
      */
     modifier onlyGovernance() {
-        if (_executor() != _msgSender()) {
-            revert GovernorOnlyExecutor(_msgSender());
-        }
-        if (_executor() != address(this)) {
-            bytes32 msgDataHash = keccak256(_msgData());
-            // loop until popping the expected operation - throw if deque is empty (operation not authorized)
-            while (_governanceCall.popFront() != msgDataHash) {}
-        }
+        _checkGovernance();
         _;
     }
 
@@ -227,6 +220,22 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         return _proposals[proposalId].proposer;
     }
 
+    /**
+     * @dev Reverts if the `msg.sender` is not the executor. In case the executor is not this contract
+     * itself, the function reverts if `msg.data` is not whitelisted as a result of an {execute}
+     * operation. See {onlyGovernance}.
+     */
+    function _checkGovernance() internal virtual {
+        if (_executor() != _msgSender()) {
+            revert GovernorOnlyExecutor(_msgSender());
+        }
+        if (_executor() != address(this)) {
+            bytes32 msgDataHash = keccak256(_msgData());
+            // loop until popping the expected operation - throw if deque is empty (operation not authorized)
+            while (_governanceCall.popFront() != msgDataHash) {}
+        }
+    }
+
     /**
      * @dev Amount of votes already cast passes the threshold limit.
      */
diff --git a/contracts/proxy/utils/Initializable.sol b/contracts/proxy/utils/Initializable.sol
index 08171015d3c..43f82feca3c 100644
--- a/contracts/proxy/utils/Initializable.sol
+++ b/contracts/proxy/utils/Initializable.sol
@@ -138,10 +138,17 @@ abstract contract Initializable {
      * {initializer} and {reinitializer} modifiers, directly or indirectly.
      */
     modifier onlyInitializing() {
+        _checkInitializing();
+        _;
+    }
+
+    /**
+     * @dev Reverts if the contract is not in an initializing state. See {onlyInitializing}.
+     */
+    function _checkInitializing() internal view virtual {
         if (!_initializing) {
             revert NotInitializing();
         }
-        _;
     }
 
     /**
diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol
index 982e1e58f98..fd41e37ec40 100644
--- a/contracts/proxy/utils/UUPSUpgradeable.sol
+++ b/contracts/proxy/utils/UUPSUpgradeable.sol
@@ -48,12 +48,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
      * fail.
      */
     modifier onlyProxy() {
-        if (
-            address(this) == __self || // Must be called through delegatecall
-            ERC1967Utils.getImplementation() != __self // Must be called through an active proxy
-        ) {
-            revert UUPSUnauthorizedCallContext();
-        }
+        _checkProxy();
         _;
     }
 
@@ -62,10 +57,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
      * callable on the implementing contract but not through proxies.
      */
     modifier notDelegated() {
-        if (address(this) != __self) {
-            // Must not be called through delegatecall
-            revert UUPSUnauthorizedCallContext();
-        }
+        _checkNotDelegated();
         _;
     }
 
@@ -96,6 +88,31 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
         _upgradeToAndCallUUPS(newImplementation, data);
     }
 
+    /**
+     * @dev Reverts if the execution is not performed via delegatecall or the execution
+     * context is not of a proxy with an ERC1967-compliant implementation pointing to self.
+     * See {_onlyProxy}.
+     */
+    function _checkProxy() internal view virtual {
+        if (
+            address(this) == __self || // Must be called through delegatecall
+            ERC1967Utils.getImplementation() != __self // Must be called through an active proxy
+        ) {
+            revert UUPSUnauthorizedCallContext();
+        }
+    }
+
+    /**
+     * @dev Reverts if the execution is performed via delegatecall.
+     * See {notDelegated}.
+     */
+    function _checkNotDelegated() internal view virtual {
+        if (address(this) != __self) {
+            // Must not be called through delegatecall
+            revert UUPSUnauthorizedCallContext();
+        }
+    }
+
     /**
      * @dev Function that should revert when `msg.sender` is not authorized to upgrade the contract. Called by
      * {upgradeToAndCall}.