From a06b35260556899c6e5e01fe0e827b96449ef75e Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 19:46:54 -0600
Subject: [PATCH 01/26] Fix M-05

---
 contracts/metatx/ERC2771Forwarder.sol | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index 93e3346220f..255567dcfa3 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -26,6 +26,12 @@ import {Address} from "../utils/Address.sol";
  * transactions in the mempool. In these cases the recommendation is to distribute the load among
  * multiple accounts.
  *
+ * NOTE: Batching requests includes an optional refund for unused `msg.value` that is achieved by
+ * performing a call with empty calldata. While this is within the bounds of ERC-2771 compliance,
+ * if the refund receiver happens to consider the forwarder a trusted forwarder, it MUST properly
+ * handle `msg.data.length == 0`. `ERC2771Context` in OpenZeppelin Contracts versions prior to 4.9.3
+ * do not handle this properly.
+ *
  * ==== Security Considerations
  *
  * If a relayer submits a forward request, it should be willing to pay up to 100% of the gas amount

From 316c30ce3c889eca5eae059df6dcd37dbcc11620 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 20:12:44 -0600
Subject: [PATCH 02/26] Fix M-07

---
 .changeset/fifty-owls-retire.md           |   5 +
 contracts/mocks/AddressFnPointersMock.sol |  50 ----------
 contracts/utils/Address.sol               | 114 ++++------------------
 test/utils/Address.test.js                |  44 +--------
 4 files changed, 25 insertions(+), 188 deletions(-)
 create mode 100644 .changeset/fifty-owls-retire.md
 delete mode 100644 contracts/mocks/AddressFnPointersMock.sol

diff --git a/.changeset/fifty-owls-retire.md b/.changeset/fifty-owls-retire.md
new file mode 100644
index 00000000000..118fad4216a
--- /dev/null
+++ b/.changeset/fifty-owls-retire.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`Address`: Removed the ability to customize error messages. A common custom error is always used if the underlying revert reason cannot be bubbled up.
diff --git a/contracts/mocks/AddressFnPointersMock.sol b/contracts/mocks/AddressFnPointersMock.sol
deleted file mode 100644
index dc4edc075a4..00000000000
--- a/contracts/mocks/AddressFnPointersMock.sol
+++ /dev/null
@@ -1,50 +0,0 @@
-// SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.20;
-
-import {Address} from "../utils/Address.sol";
-
-/**
- * @dev A mock to expose `Address`'s functions with function pointers.
- */
-contract AddressFnPointerMock {
-    error CustomRevert();
-
-    function functionCall(address target, bytes memory data) external returns (bytes memory) {
-        return Address.functionCall(target, data, _customRevert);
-    }
-
-    function functionCallWithValue(address target, bytes memory data, uint256 value) external returns (bytes memory) {
-        return Address.functionCallWithValue(target, data, value, _customRevert);
-    }
-
-    function functionStaticCall(address target, bytes memory data) external view returns (bytes memory) {
-        return Address.functionStaticCall(target, data, _customRevert);
-    }
-
-    function functionDelegateCall(address target, bytes memory data) external returns (bytes memory) {
-        return Address.functionDelegateCall(target, data, _customRevert);
-    }
-
-    function verifyCallResultFromTarget(
-        address target,
-        bool success,
-        bytes memory returndata
-    ) external view returns (bytes memory) {
-        return Address.verifyCallResultFromTarget(target, success, returndata, _customRevert);
-    }
-
-    function verifyCallResult(bool success, bytes memory returndata) external view returns (bytes memory) {
-        return Address.verifyCallResult(success, returndata, _customRevert);
-    }
-
-    function verifyCallResultVoid(bool success, bytes memory returndata) external view returns (bytes memory) {
-        return Address.verifyCallResult(success, returndata, _customRevertVoid);
-    }
-
-    function _customRevert() internal pure {
-        revert CustomRevert();
-    }
-
-    function _customRevertVoid() internal pure {}
-}
diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol
index e3a71313c56..a2614787ffc 100644
--- a/contracts/utils/Address.sol
+++ b/contracts/utils/Address.sol
@@ -54,8 +54,10 @@ library Address {
      * plain `call` is an unsafe replacement for a function call: use this
      * function instead.
      *
-     * If `target` reverts with a revert reason, it is bubbled up by this
-     * function (like regular Solidity function calls).
+     * If `target` reverts with a revert reason or custom error, it is bubbled
+     * up by this function (like regular Solidity function calls). However, if
+     * the call reverted with no returned reason, this function reverts with a
+     * {FailedInnerCall} error.
      *
      * Returns the raw returned data. To convert to the expected return value,
      * use https://solidity.readthedocs.io/en/latest/units-and-global-variables.html?highlight=abi.decode#abi-encoding-and-decoding-functions[`abi.decode`].
@@ -66,23 +68,7 @@ library Address {
      * - calling `target` with `data` must not revert.
      */
     function functionCall(address target, bytes memory data) internal returns (bytes memory) {
-        return functionCallWithValue(target, data, 0, defaultRevert);
-    }
-
-    /**
-     * @dev Same as {xref-Address-functionCall-address-bytes-}[`functionCall`], but with a
-     * `customRevert` function as a fallback when `target` reverts.
-     *
-     * Requirements:
-     *
-     * - `customRevert` must be a reverting function.
-     */
-    function functionCall(
-        address target,
-        bytes memory data,
-        function() internal view customRevert
-    ) internal returns (bytes memory) {
-        return functionCallWithValue(target, data, 0, customRevert);
+        return functionCallWithValue(target, data, 0);
     }
 
     /**
@@ -95,28 +81,11 @@ library Address {
      * - the called Solidity function must be `payable`.
      */
     function functionCallWithValue(address target, bytes memory data, uint256 value) internal returns (bytes memory) {
-        return functionCallWithValue(target, data, value, defaultRevert);
-    }
-
-    /**
-     * @dev Same as {xref-Address-functionCallWithValue-address-bytes-uint256-}[`functionCallWithValue`], but
-     * with a `customRevert` function as a fallback revert reason when `target` reverts.
-     *
-     * Requirements:
-     *
-     * - `customRevert` must be a reverting function.
-     */
-    function functionCallWithValue(
-        address target,
-        bytes memory data,
-        uint256 value,
-        function() internal view customRevert
-    ) internal returns (bytes memory) {
         if (address(this).balance < value) {
             revert AddressInsufficientBalance(address(this));
         }
         (bool success, bytes memory returndata) = target.call{value: value}(data);
-        return verifyCallResultFromTarget(target, success, returndata, customRevert);
+        return verifyCallResultFromTarget(target, success, returndata);
     }
 
     /**
@@ -124,20 +93,8 @@ library Address {
      * but performing a static call.
      */
     function functionStaticCall(address target, bytes memory data) internal view returns (bytes memory) {
-        return functionStaticCall(target, data, defaultRevert);
-    }
-
-    /**
-     * @dev Same as {xref-Address-functionCall-address-bytes-string-}[`functionCall`],
-     * but performing a static call.
-     */
-    function functionStaticCall(
-        address target,
-        bytes memory data,
-        function() internal view customRevert
-    ) internal view returns (bytes memory) {
         (bool success, bytes memory returndata) = target.staticcall(data);
-        return verifyCallResultFromTarget(target, success, returndata, customRevert);
+        return verifyCallResultFromTarget(target, success, returndata);
     }
 
     /**
@@ -145,31 +102,19 @@ library Address {
      * but performing a delegate call.
      */
     function functionDelegateCall(address target, bytes memory data) internal returns (bytes memory) {
-        return functionDelegateCall(target, data, defaultRevert);
-    }
-
-    /**
-     * @dev Same as {xref-Address-functionCall-address-bytes-string-}[`functionCall`],
-     * but performing a delegate call.
-     */
-    function functionDelegateCall(
-        address target,
-        bytes memory data,
-        function() internal view customRevert
-    ) internal returns (bytes memory) {
         (bool success, bytes memory returndata) = target.delegatecall(data);
-        return verifyCallResultFromTarget(target, success, returndata, customRevert);
+        return verifyCallResultFromTarget(target, success, returndata);
     }
 
     /**
-     * @dev Tool to verify that a low level call to smart-contract was successful, and revert (either by bubbling
-     * the revert reason or using the provided `customRevert`) in case of unsuccessful call or if target was not a contract.
+     * @dev Tool to verify that a low level call to smart-contract was successful, and reverts if the target
+     * was not a contract or bubbling up the revert reason (falling back to {FailedInnerCall}) in case of an
+     * unsuccessful call.
      */
     function verifyCallResultFromTarget(
         address target,
         bool success,
-        bytes memory returndata,
-        function() internal view customRevert
+        bytes memory returndata
     ) internal view returns (bytes memory) {
         if (success) {
             if (returndata.length == 0) {
@@ -181,46 +126,26 @@ library Address {
             }
             return returndata;
         } else {
-            _revert(returndata, customRevert);
+            _revert(returndata);
         }
     }
 
     /**
-     * @dev Tool to verify that a low level call was successful, and revert if it wasn't, either by bubbling the
-     * revert reason or with a default revert error.
-     */
-    function verifyCallResult(bool success, bytes memory returndata) internal view returns (bytes memory) {
-        return verifyCallResult(success, returndata, defaultRevert);
-    }
-
-    /**
-     * @dev Same as {xref-Address-verifyCallResult-bool-bytes-}[`verifyCallResult`], but with a
-     * `customRevert` function as a fallback when `success` is `false`.
-     *
-     * Requirements:
-     *
-     * - `customRevert` must be a reverting function.
+     * @dev Tool to verify that a low level call was successful, and reverts if it wasn't, either by bubbling the
+     * revert reason or with a default {FailedInnerCall} error.
      */
-    function verifyCallResult(
-        bool success,
-        bytes memory returndata,
-        function() internal view customRevert
-    ) internal view returns (bytes memory) {
+    function verifyCallResult(bool success, bytes memory returndata) internal pure returns (bytes memory) {
         if (success) {
             return returndata;
         } else {
-            _revert(returndata, customRevert);
+            _revert(returndata);
         }
     }
 
     /**
-     * @dev Default reverting function when no `customRevert` is provided in a function call.
+     * @dev Reverts with returndata if present. Otherwise reverts with {FailedInnerCall}.
      */
-    function defaultRevert() internal pure {
-        revert FailedInnerCall();
-    }
-
-    function _revert(bytes memory returndata, function() internal view customRevert) private view {
+    function _revert(bytes memory returndata) private pure {
         // Look for revert reason and bubble it up if present
         if (returndata.length > 0) {
             // The easiest way to bubble the revert reason is using memory via assembly
@@ -230,7 +155,6 @@ library Address {
                 revert(add(32, returndata), returndata_size)
             }
         } else {
-            customRevert();
             revert FailedInnerCall();
         }
     }
diff --git a/test/utils/Address.test.js b/test/utils/Address.test.js
index beded18e1d4..57453abd580 100644
--- a/test/utils/Address.test.js
+++ b/test/utils/Address.test.js
@@ -3,7 +3,6 @@ const { expect } = require('chai');
 const { expectRevertCustomError } = require('../helpers/customError');
 
 const Address = artifacts.require('$Address');
-const AddressFnPointerMock = artifacts.require('$AddressFnPointerMock');
 const EtherReceiver = artifacts.require('EtherReceiverMock');
 const CallReceiverMock = artifacts.require('CallReceiverMock');
 
@@ -12,7 +11,6 @@ contract('Address', function (accounts) {
 
   beforeEach(async function () {
     this.mock = await Address.new();
-    this.mockFnPointer = await AddressFnPointerMock.new();
   });
 
   describe('sendValue', function () {
@@ -141,14 +139,6 @@ contract('Address', function (accounts) {
         await expectRevert.unspecified(this.mock.$functionCall(this.target.address, abiEncodedCall));
       });
 
-      it('bubbles up error if specified', async function () {
-        await expectRevertCustomError(
-          this.mockFnPointer.functionCall(this.target.address, '0x12345678'),
-          'CustomRevert',
-          [],
-        );
-      });
-
       it('reverts when function does not exist', async function () {
         const abiEncodedCall = web3.eth.abi.encodeFunctionCall(
           {
@@ -254,14 +244,6 @@ contract('Address', function (accounts) {
           [],
         );
       });
-
-      it('bubbles up error if specified', async function () {
-        await expectRevertCustomError(
-          this.mockFnPointer.functionCallWithValue(this.target.address, '0x12345678', 0),
-          'CustomRevert',
-          [],
-        );
-      });
     });
   });
 
@@ -305,14 +287,6 @@ contract('Address', function (accounts) {
         recipient,
       ]);
     });
-
-    it('bubbles up error if specified', async function () {
-      await expectRevertCustomError(
-        this.mockFnPointer.functionCallWithValue(this.target.address, '0x12345678', 0),
-        'CustomRevert',
-        [],
-      );
-    });
   });
 
   describe('functionDelegateCall', function () {
@@ -355,28 +329,12 @@ contract('Address', function (accounts) {
         recipient,
       ]);
     });
-
-    it('bubbles up error if specified', async function () {
-      await expectRevertCustomError(
-        this.mockFnPointer.functionCallWithValue(this.target.address, '0x12345678', 0),
-        'CustomRevert',
-        [],
-      );
-    });
   });
 
   describe('verifyCallResult', function () {
     it('returns returndata on success', async function () {
       const returndata = '0x123abc';
-      expect(await this.mockFnPointer.verifyCallResult(true, returndata)).to.equal(returndata);
-    });
-
-    it('reverts with return data and error', async function () {
-      await expectRevertCustomError(this.mockFnPointer.verifyCallResult(false, '0x'), 'CustomRevert', []);
-    });
-
-    it('reverts expecting error if provided onRevert is a non-reverting function', async function () {
-      await expectRevertCustomError(this.mockFnPointer.verifyCallResultVoid(false, '0x'), 'FailedInnerCall', []);
+      expect(await this.mock.$verifyCallResult(true, returndata)).to.equal(returndata);
     });
   });
 });

From 351565f273c75be2601a4ccd27b5b6f69b1594b0 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 20:18:42 -0600
Subject: [PATCH 03/26] Fix L-02

---
 contracts/proxy/transparent/TransparentUpgradeableProxy.sol | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index b51d2973e9b..413f46ecf7d 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -45,6 +45,9 @@ interface ITransparentUpgradeableProxy is IERC1967 {
  * implement transparency without decoding reverts caused by selector clashes between the proxy and the
  * implementation.
  *
+ * NOTE: This proxy does not inherit from {Context} deliberately. The {ProxyAdmin} of this contract won't send a
+ * meta-transaction in any way, and any other meta-transaction setup should be made in the implementation contract.
+ *
  * IMPORTANT: This contract avoids unnecessary storage reads by setting the admin only during construction as an immutable variable,
  * preventing any changes thereafter. However, the admin slot defined in ERC-1967 can still be overwritten by the implementation
  * logic pointed to by this proxy. In such cases, the contract may end up in an undesirable state where the admin slot is different

From 1bd35440a9eae540fa6db1fad0a87058c55db0bc Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 20:33:17 -0600
Subject: [PATCH 04/26] Fix L-03

---
 contracts/metatx/ERC2771Forwarder.sol | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index 255567dcfa3..2c26aa64163 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -26,6 +26,10 @@ import {Address} from "../utils/Address.sol";
  * transactions in the mempool. In these cases the recommendation is to distribute the load among
  * multiple accounts.
  *
+ * WARNING: Do not approve this contract to spend tokens. Anyone can use this forwarder
+ * to execute calls with an arbitrary calldata to any address. Any form of approval may
+ * result in a loss of funds for the approving party.
+ *
  * NOTE: Batching requests includes an optional refund for unused `msg.value` that is achieved by
  * performing a call with empty calldata. While this is within the bounds of ERC-2771 compliance,
  * if the refund receiver happens to consider the forwarder a trusted forwarder, it MUST properly

From 53b4febe4267abcf11dfcbfa69c942cd477c79d4 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 20:51:08 -0600
Subject: [PATCH 05/26] Fix L-04

---
 contracts/metatx/ERC2771Forwarder.sol | 43 +++++++++++++++------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index 2c26aa64163..d648244eefd 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -197,30 +197,35 @@ contract ERC2771Forwarder is EIP712, Nonces {
     function _validate(
         ForwardRequestData calldata request
     ) internal view virtual returns (bool alive, bool signerMatch, address signer) {
-        signer = _recoverForwardRequestSigner(request);
-        return (request.deadline >= block.timestamp, signer == request.from, signer);
+        (bool isValid, address recovered) = _recoverForwardRequestSigner(request);
+        return (request.deadline >= block.timestamp, isValid && recovered == request.from, recovered);
     }
 
     /**
-     * @dev Recovers the signer of an EIP712 message hash for a forward `request` and its corresponding `signature`.
-     * See {ECDSA-recover}.
+     * @dev Returns a tuple with the recovered the signer of an EIP712 forward request message hash
+     * and a boolean indicating if the signature is valid.
+     *
+     * NOTE: The signature is considered valid if {ECDSA-tryRecover} indicates no recover error for it.
      */
-    function _recoverForwardRequestSigner(ForwardRequestData calldata request) internal view virtual returns (address) {
-        return
-            _hashTypedDataV4(
-                keccak256(
-                    abi.encode(
-                        _FORWARD_REQUEST_TYPEHASH,
-                        request.from,
-                        request.to,
-                        request.value,
-                        request.gas,
-                        nonces(request.from),
-                        request.deadline,
-                        keccak256(request.data)
-                    )
+    function _recoverForwardRequestSigner(
+        ForwardRequestData calldata request
+    ) internal view virtual returns (bool, address) {
+        (address recovered, ECDSA.RecoverError err, ) = _hashTypedDataV4(
+            keccak256(
+                abi.encode(
+                    _FORWARD_REQUEST_TYPEHASH,
+                    request.from,
+                    request.to,
+                    request.value,
+                    request.gas,
+                    nonces(request.from),
+                    request.deadline,
+                    keccak256(request.data)
                 )
-            ).recover(request.signature);
+            )
+        ).tryRecover(request.signature);
+
+        return (err == ECDSA.RecoverError.NoError, recovered);
     }
 
     /**

From a94adcd82c8c58e02af6778e08d22a6221a89546 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 22:00:29 -0600
Subject: [PATCH 06/26] Fix L-08

---
 contracts/mocks/UpgradeableBeaconMock.sol  | 27 ++++++++++++++++++++++
 contracts/mocks/UpgreadeableBeaconMock.sol | 12 ----------
 contracts/proxy/ERC1967/ERC1967Utils.sol   |  4 ++--
 test/proxy/ERC1967/ERC1967Utils.test.js    | 12 ++++++++++
 4 files changed, 41 insertions(+), 14 deletions(-)
 create mode 100644 contracts/mocks/UpgradeableBeaconMock.sol
 delete mode 100644 contracts/mocks/UpgreadeableBeaconMock.sol

diff --git a/contracts/mocks/UpgradeableBeaconMock.sol b/contracts/mocks/UpgradeableBeaconMock.sol
new file mode 100644
index 00000000000..354ac02f0d7
--- /dev/null
+++ b/contracts/mocks/UpgradeableBeaconMock.sol
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IBeacon} from "../proxy/beacon/IBeacon.sol";
+
+contract UpgradeableBeaconMock is IBeacon {
+    address public implementation;
+
+    constructor(address impl) {
+        implementation = impl;
+    }
+}
+
+interface IProxyExposed {
+    // solhint-disable-next-line func-name-mixedcase
+    function $getBeacon() external view returns (address);
+}
+
+contract UpgradeableBeaconReentrantMock is IBeacon {
+    error BeaconProxyBeaconSlotAddress(address beacon);
+
+    function implementation() external view override returns (address) {
+        // Revert with the beacon seen in the proxy at the moment of calling to check if it's
+        // set before the call.
+        revert BeaconProxyBeaconSlotAddress(IProxyExposed(msg.sender).$getBeacon());
+    }
+}
diff --git a/contracts/mocks/UpgreadeableBeaconMock.sol b/contracts/mocks/UpgreadeableBeaconMock.sol
deleted file mode 100644
index 4bee5c0f292..00000000000
--- a/contracts/mocks/UpgreadeableBeaconMock.sol
+++ /dev/null
@@ -1,12 +0,0 @@
-// SPDX-License-Identifier: MIT
-pragma solidity ^0.8.20;
-
-import {IBeacon} from "../proxy/beacon/IBeacon.sol";
-
-contract UpgradeableBeaconMock is IBeacon {
-    address public implementation;
-
-    constructor(address impl) {
-        implementation = impl;
-    }
-}
diff --git a/contracts/proxy/ERC1967/ERC1967Utils.sol b/contracts/proxy/ERC1967/ERC1967Utils.sol
index bd2108096da..bb822bf3cd7 100644
--- a/contracts/proxy/ERC1967/ERC1967Utils.sol
+++ b/contracts/proxy/ERC1967/ERC1967Utils.sol
@@ -153,12 +153,12 @@ library ERC1967Utils {
             revert ERC1967InvalidBeacon(newBeacon);
         }
 
+        StorageSlot.getAddressSlot(BEACON_SLOT).value = newBeacon;
+
         address beaconImplementation = IBeacon(newBeacon).implementation();
         if (beaconImplementation.code.length == 0) {
             revert ERC1967InvalidImplementation(beaconImplementation);
         }
-
-        StorageSlot.getAddressSlot(BEACON_SLOT).value = newBeacon;
     }
 
     /**
diff --git a/test/proxy/ERC1967/ERC1967Utils.test.js b/test/proxy/ERC1967/ERC1967Utils.test.js
index cce874cd93c..975b08d81a8 100644
--- a/test/proxy/ERC1967/ERC1967Utils.test.js
+++ b/test/proxy/ERC1967/ERC1967Utils.test.js
@@ -9,6 +9,7 @@ const ERC1967Utils = artifacts.require('$ERC1967Utils');
 const V1 = artifacts.require('DummyImplementation');
 const V2 = artifacts.require('CallReceiverMock');
 const UpgradeableBeaconMock = artifacts.require('UpgradeableBeaconMock');
+const UpgradeableBeaconReentrantMock = artifacts.require('UpgradeableBeaconReentrantMock');
 
 contract('ERC1967Utils', function (accounts) {
   const [, admin, anotherAccount] = accounts;
@@ -155,6 +156,17 @@ contract('ERC1967Utils', function (accounts) {
           await expectEvent.inTransaction(receipt.tx, await V2.at(this.utils.address), 'MockFunctionCalled');
         });
       });
+
+      describe('reentrant beacon implementation() call', function () {
+        it('sees the new beacon implementation', async function () {
+          const newBeacon = await UpgradeableBeaconReentrantMock.new();
+          await expectRevertCustomError(
+            this.utils.$upgradeBeaconToAndCall(newBeacon.address, '0x'),
+            'BeaconProxyBeaconSlotAddress',
+            [newBeacon.address],
+          );
+        });
+      });
     });
   });
 });

From 9afa6c9e50d6d63f6fa4e97262d9c38a7ce692b5 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 22:02:00 -0600
Subject: [PATCH 07/26] Fix L-09

---
 contracts/finance/VestingWallet.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol
index 840837d27e0..cd1a2406448 100644
--- a/contracts/finance/VestingWallet.sol
+++ b/contracts/finance/VestingWallet.sol
@@ -154,7 +154,7 @@ contract VestingWallet is Context {
     function _vestingSchedule(uint256 totalAllocation, uint64 timestamp) internal view virtual returns (uint256) {
         if (timestamp < start()) {
             return 0;
-        } else if (timestamp > end()) {
+        } else if (timestamp >= end()) {
             return totalAllocation;
         } else {
             return (totalAllocation * (timestamp - start())) / duration();

From 5a0e133df93f19e33d840891cc49d88f5327590a Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 22:24:48 -0600
Subject: [PATCH 08/26] Fix CR-01

---
 contracts/proxy/Proxy.sol | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/contracts/proxy/Proxy.sol b/contracts/proxy/Proxy.sol
index 3644197fab5..005a876ac4b 100644
--- a/contracts/proxy/Proxy.sol
+++ b/contracts/proxy/Proxy.sol
@@ -56,7 +56,6 @@ abstract contract Proxy {
      * This function does not return to its internal call site, it will return directly to the external caller.
      */
     function _fallback() internal virtual {
-        _beforeFallback();
         _delegate(_implementation());
     }
 
@@ -67,12 +66,4 @@ abstract contract Proxy {
     fallback() external payable virtual {
         _fallback();
     }
-
-    /**
-     * @dev Hook that is called before falling back to the implementation. Can happen as part of a manual `_fallback`
-     * call, or as part of the Solidity `fallback` or `receive` functions.
-     *
-     * If overridden should call `super._beforeFallback()`.
-     */
-    function _beforeFallback() internal virtual {}
 }

From 58546a17c4e78aedf55aa3ae49e81d421bdca4a6 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 22:27:28 -0600
Subject: [PATCH 09/26] Fix N-22

---
 contracts/proxy/ERC1967/ERC1967Proxy.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/proxy/ERC1967/ERC1967Proxy.sol b/contracts/proxy/ERC1967/ERC1967Proxy.sol
index d4104ccfbb8..5e3f4dc78be 100644
--- a/contracts/proxy/ERC1967/ERC1967Proxy.sol
+++ b/contracts/proxy/ERC1967/ERC1967Proxy.sol
@@ -34,7 +34,7 @@ contract ERC1967Proxy is Proxy {
      * https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call.
      * `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc`
      */
-    function _implementation() internal view virtual override returns (address impl) {
+    function _implementation() internal view virtual override returns (address) {
         return ERC1967Utils.getImplementation();
     }
 }

From 1164c5157d477d35ebfb6231819d9624bf401436 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 22:35:08 -0600
Subject: [PATCH 10/26] Fix N-21

---
 contracts/metatx/ERC2771Context.sol | 32 ++++++++++++++++++++++++++---
 test/metatx/ERC2771Context.test.js  |  6 +++++-
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/contracts/metatx/ERC2771Context.sol b/contracts/metatx/ERC2771Context.sol
index 4a2df6e3df9..1084afb1561 100644
--- a/contracts/metatx/ERC2771Context.sol
+++ b/contracts/metatx/ERC2771Context.sol
@@ -18,15 +18,36 @@ abstract contract ERC2771Context is Context {
     /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address private immutable _trustedForwarder;
 
+    /**
+     * @dev Initializes the contract with a trusted forwarder, which will be able to
+     * invoke functions on this contract on behalf of other accounts.
+     *
+     * NOTE: The trusted forwarder can be replaced by overriding {trustedForwarder}.
+     */
     /// @custom:oz-upgrades-unsafe-allow constructor
-    constructor(address trustedForwarder) {
-        _trustedForwarder = trustedForwarder;
+    constructor(address trustedForwarder_) {
+        _trustedForwarder = trustedForwarder_;
     }
 
+    /**
+     * @dev Returns the address of the trusted forwarder.
+     */
+    function trustedForwarder() public view virtual returns (address) {
+        return _trustedForwarder;
+    }
+
+    /**
+     * @dev Indicates whether any particular address is the trusted forwarder.
+     */
     function isTrustedForwarder(address forwarder) public view virtual returns (bool) {
-        return forwarder == _trustedForwarder;
+        return forwarder == trustedForwarder();
     }
 
+    /**
+     * @dev Override for `msg.sender`. Defaults to the original `msg.sender` whenever
+     * a call is not performed by the trusted forwarder or the calldata length is less than
+     * 20 bytes (an address length).
+     */
     function _msgSender() internal view virtual override returns (address sender) {
         if (isTrustedForwarder(msg.sender) && msg.data.length >= 20) {
             // The assembly code is more direct than the Solidity version using `abi.decode`.
@@ -39,6 +60,11 @@ abstract contract ERC2771Context is Context {
         }
     }
 
+    /**
+     * @dev Override for `msg.data`. Defaults to the original `msg.data` whenever
+     * a call is not performed by the trusted forwarder or the calldata length is less than
+     * 20 bytes (an address length).
+     */
     function _msgData() internal view virtual override returns (bytes calldata) {
         if (isTrustedForwarder(msg.sender) && msg.data.length >= 20) {
             return msg.data[:msg.data.length - 20];
diff --git a/test/metatx/ERC2771Context.test.js b/test/metatx/ERC2771Context.test.js
index 0ec8d98dde6..522b05cd1cf 100644
--- a/test/metatx/ERC2771Context.test.js
+++ b/test/metatx/ERC2771Context.test.js
@@ -36,7 +36,11 @@ contract('ERC2771Context', function (accounts) {
   });
 
   it('recognize trusted forwarder', async function () {
-    expect(await this.recipient.isTrustedForwarder(this.forwarder.address));
+    expect(await this.recipient.isTrustedForwarder(this.forwarder.address)).to.equal(true);
+  });
+
+  it('returns the trusted forwarder', async function () {
+    expect(await this.recipient.trustedForwarder()).to.equal(this.forwarder.address);
   });
 
   context('when called directly', function () {

From dee9963a03f587e9080a872063ba02039f97b99c Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 1 Aug 2023 22:46:54 -0600
Subject: [PATCH 11/26] Fix N-20

---
 contracts/metatx/ERC2771Forwarder.sol    | 12 ++++++------
 contracts/proxy/ERC1967/ERC1967Proxy.sol |  8 ++++----
 contracts/utils/Strings.sol              |  6 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index d648244eefd..54780605a5c 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -107,8 +107,8 @@ contract ERC2771Forwarder is EIP712, Nonces {
      * receiver is provided.
      */
     function verify(ForwardRequestData calldata request) public view virtual returns (bool) {
-        (bool alive, bool signerMatch, ) = _validate(request);
-        return alive && signerMatch;
+        (bool active, bool signerMatch, ) = _validate(request);
+        return active && signerMatch;
     }
 
     /**
@@ -196,7 +196,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
      */
     function _validate(
         ForwardRequestData calldata request
-    ) internal view virtual returns (bool alive, bool signerMatch, address signer) {
+    ) internal view virtual returns (bool active, bool signerMatch, address signer) {
         (bool isValid, address recovered) = _recoverForwardRequestSigner(request);
         return (request.deadline >= block.timestamp, isValid && recovered == request.from, recovered);
     }
@@ -247,12 +247,12 @@ contract ERC2771Forwarder is EIP712, Nonces {
         ForwardRequestData calldata request,
         bool requireValidRequest
     ) internal virtual returns (bool success) {
-        (bool alive, bool signerMatch, address signer) = _validate(request);
+        (bool active, bool signerMatch, address signer) = _validate(request);
 
         // Need to explicitly specify if a revert is required since non-reverting is default for
         // batches and reversion is opt-in since it could be useful in some scenarios
         if (requireValidRequest) {
-            if (!alive) {
+            if (!active) {
                 revert ERC2771ForwarderExpiredRequest(request.deadline);
             }
 
@@ -262,7 +262,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         }
 
         // Ignore an invalid request because requireValidRequest = false
-        if (signerMatch && alive) {
+        if (signerMatch && active) {
             // Nonce should be used before the call to prevent reusing by reentrancy
             uint256 currentNonce = _useNonce(signer);
 
diff --git a/contracts/proxy/ERC1967/ERC1967Proxy.sol b/contracts/proxy/ERC1967/ERC1967Proxy.sol
index 5e3f4dc78be..df3a6136919 100644
--- a/contracts/proxy/ERC1967/ERC1967Proxy.sol
+++ b/contracts/proxy/ERC1967/ERC1967Proxy.sol
@@ -14,17 +14,17 @@ import {ERC1967Utils} from "./ERC1967Utils.sol";
  */
 contract ERC1967Proxy is Proxy {
     /**
-     * @dev Initializes the upgradeable proxy with an initial implementation specified by `_logic`.
+     * @dev Initializes the upgradeable proxy with an initial implementation specified by `implementation`.
      *
-     * If `_data` is nonempty, it's used as data in a delegate call to `_logic`. This will typically be an encoded
+     * If `_data` is nonempty, it's used as data in a delegate call to `implementation`. This will typically be an encoded
      * function call, and allows initializing the storage of the proxy like a Solidity constructor.
      *
      * Requirements:
      *
      * - If `data` is empty, `msg.value` must be zero.
      */
-    constructor(address _logic, bytes memory _data) payable {
-        ERC1967Utils.upgradeToAndCall(_logic, _data);
+    constructor(address implementation, bytes memory _data) payable {
+        ERC1967Utils.upgradeToAndCall(implementation, _data);
     }
 
     /**
diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol
index 0037eee1b59..24c22d53d75 100644
--- a/contracts/utils/Strings.sol
+++ b/contracts/utils/Strings.sol
@@ -10,7 +10,7 @@ import {SignedMath} from "./math/SignedMath.sol";
  * @dev String operations.
  */
 library Strings {
-    bytes16 private constant _SYMBOLS = "0123456789abcdef";
+    bytes16 private constant _HEX_DIGITS = "0123456789abcdef";
     uint8 private constant _ADDRESS_LENGTH = 20;
 
     /**
@@ -34,7 +34,7 @@ library Strings {
                 ptr--;
                 /// @solidity memory-safe-assembly
                 assembly {
-                    mstore8(ptr, byte(mod(value, 10), _SYMBOLS))
+                    mstore8(ptr, byte(mod(value, 10), _HEX_DIGITS))
                 }
                 value /= 10;
                 if (value == 0) break;
@@ -68,7 +68,7 @@ library Strings {
         buffer[0] = "0";
         buffer[1] = "x";
         for (uint256 i = 2 * length + 1; i > 1; --i) {
-            buffer[i] = _SYMBOLS[localValue & 0xf];
+            buffer[i] = _HEX_DIGITS[localValue & 0xf];
             localValue >>= 4;
         }
         if (localValue != 0) {

From 687b80515b16138d11e79eb2d9503fb1ae8defec Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Wed, 2 Aug 2023 00:20:45 -0600
Subject: [PATCH 12/26] Fix N-17

---
 contracts/proxy/beacon/UpgradeableBeacon.sol | 2 +-
 test/proxy/beacon/UpgradeableBeacon.test.js  | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/contracts/proxy/beacon/UpgradeableBeacon.sol b/contracts/proxy/beacon/UpgradeableBeacon.sol
index 81ce5090218..a7816a1e6d0 100644
--- a/contracts/proxy/beacon/UpgradeableBeacon.sol
+++ b/contracts/proxy/beacon/UpgradeableBeacon.sol
@@ -51,7 +51,6 @@ contract UpgradeableBeacon is IBeacon, Ownable {
      */
     function upgradeTo(address newImplementation) public virtual onlyOwner {
         _setImplementation(newImplementation);
-        emit Upgraded(newImplementation);
     }
 
     /**
@@ -66,5 +65,6 @@ contract UpgradeableBeacon is IBeacon, Ownable {
             revert BeaconInvalidImplementation(newImplementation);
         }
         _implementation = newImplementation;
+        emit Upgraded(newImplementation);
     }
 }
diff --git a/test/proxy/beacon/UpgradeableBeacon.test.js b/test/proxy/beacon/UpgradeableBeacon.test.js
index 4c58f1740b6..0737f6fdfe6 100644
--- a/test/proxy/beacon/UpgradeableBeacon.test.js
+++ b/test/proxy/beacon/UpgradeableBeacon.test.js
@@ -20,6 +20,13 @@ contract('UpgradeableBeacon', function (accounts) {
       this.beacon = await UpgradeableBeacon.new(this.v1.address, owner);
     });
 
+    it('emits Upgraded event to the first implementation', async function () {
+      const beacon = await UpgradeableBeacon.new(this.v1.address, owner);
+      await expectEvent.inTransaction(beacon.contract.transactionHash, beacon, 'Upgraded', {
+        implementation: this.v1.address,
+      });
+    });
+
     it('returns implementation', async function () {
       expect(await this.beacon.implementation()).to.equal(this.v1.address);
     });

From 60329ea7fe656a330db783cc7ba046acaaa0e045 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Wed, 2 Aug 2023 00:44:55 -0600
Subject: [PATCH 13/26] Fix N-19

---
 contracts/proxy/ERC1967/ERC1967Utils.sol   | 11 +++++------
 contracts/proxy/beacon/IBeacon.sol         |  2 +-
 contracts/proxy/transparent/ProxyAdmin.sol |  2 +-
 contracts/proxy/utils/UUPSUpgradeable.sol  |  7 +++++--
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/contracts/proxy/ERC1967/ERC1967Utils.sol b/contracts/proxy/ERC1967/ERC1967Utils.sol
index bb822bf3cd7..b6f705e54c1 100644
--- a/contracts/proxy/ERC1967/ERC1967Utils.sol
+++ b/contracts/proxy/ERC1967/ERC1967Utils.sol
@@ -31,8 +31,7 @@ library ERC1967Utils {
 
     /**
      * @dev Storage slot with the address of the current implementation.
-     * This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is
-     * validated in the constructor.
+     * This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1.
      */
     // solhint-disable-next-line private-vars-leading-underscore
     bytes32 internal constant IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
@@ -94,8 +93,7 @@ library ERC1967Utils {
 
     /**
      * @dev Storage slot with the admin of the contract.
-     * This is the keccak-256 hash of "eip1967.proxy.admin" subtracted by 1, and is
-     * validated in the constructor.
+     * This is the keccak-256 hash of "eip1967.proxy.admin" subtracted by 1.
      */
     // solhint-disable-next-line private-vars-leading-underscore
     bytes32 internal constant ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;
@@ -133,7 +131,7 @@ library ERC1967Utils {
 
     /**
      * @dev The storage slot of the UpgradeableBeacon contract which defines the implementation for this proxy.
-     * This is bytes32(uint256(keccak256('eip1967.proxy.beacon')) - 1) and is validated in the constructor.
+     * This is the keccak-256 hash of "eip1967.proxy.beacon" subtracted by 1.
      */
     // solhint-disable-next-line private-vars-leading-underscore
     bytes32 internal constant BEACON_SLOT = 0xa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d50;
@@ -184,7 +182,8 @@ library ERC1967Utils {
     }
 
     /**
-     * @dev Reverts if `msg.value` is not zero.
+     * @dev Reverts if `msg.value` is not zero. It can be used to avoid `msg.value` stuck in the contract
+     * if an upgrade doesn't perform an initialization call.
      */
     function _checkNonPayable() private {
         if (msg.value > 0) {
diff --git a/contracts/proxy/beacon/IBeacon.sol b/contracts/proxy/beacon/IBeacon.sol
index f5e9f79811f..56477c92a27 100644
--- a/contracts/proxy/beacon/IBeacon.sol
+++ b/contracts/proxy/beacon/IBeacon.sol
@@ -10,7 +10,7 @@ interface IBeacon {
     /**
      * @dev Must return an address that can be used as a delegate call target.
      *
-     * {BeaconProxy} will check that this address is a contract.
+     * {UpgradeableBeacon} will check that this address is a contract.
      */
     function implementation() external view returns (address);
 }
diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol
index c0e6780f41e..cd03fb939e4 100644
--- a/contracts/proxy/transparent/ProxyAdmin.sol
+++ b/contracts/proxy/transparent/ProxyAdmin.sol
@@ -16,7 +16,7 @@ contract ProxyAdmin is Ownable {
      * and `upgradeAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,
      * while `upgradeAndCall` will invoke the `receive` function if the second argument is the empty byte string.
      * If the getter returns `"5.0.0"`, only `upgradeAndCall(address,bytes)` is present, and the second argument must
-     * be the empty byte string if no function should be called, being impossible to invoke the `receive` function
+     * be the empty byte string if no function should be called, making it impossible to invoke the `receive` function
      * during an upgrade.
      */
     string public constant UPGRADE_INTERFACE_VERSION = "5.0.0";
diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol
index 7ad0c9abcb9..c861078400b 100644
--- a/contracts/proxy/utils/UUPSUpgradeable.sol
+++ b/contracts/proxy/utils/UUPSUpgradeable.sol
@@ -25,7 +25,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
      * and `upgradeToAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,
      * while `upgradeToAndCall` will invoke the `receive` function if the second argument is the empty byte string.
      * If the getter returns `"5.0.0"`, only `upgradeToAndCall(address,bytes)` is present, and the second argument must
-     * be the empty byte string if no function should be called, being impossible to invoke the `receive` function
+     * be the empty byte string if no function should be called, making it impossible to invoke the `receive` function
      * during an upgrade.
      */
     string public constant UPGRADE_INTERFACE_VERSION = "5.0.0";
@@ -126,7 +126,10 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
     function _authorizeUpgrade(address newImplementation) internal virtual;
 
     /**
-     * @dev Perform implementation upgrade with security checks for UUPS proxies, and additional setup call.
+     * @dev Performs ab implementation upgrade with a security check for UUPS proxies, and additional setup call.
+     *
+     * As a security check, {proxiableUUID} is invoked in the new implementation, and the return value
+     * is expected to be the implementation slot in ERC1967.
      *
      * Emits an {IERC1967-Upgraded} event.
      */

From ffd95209fde29a491e33f2319d0cfd68aa92420e Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Wed, 2 Aug 2023 10:52:15 +0200
Subject: [PATCH 14/26] Update contracts/proxy/utils/UUPSUpgradeable.sol

---
 contracts/proxy/utils/UUPSUpgradeable.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol
index c861078400b..f08e61c1e8b 100644
--- a/contracts/proxy/utils/UUPSUpgradeable.sol
+++ b/contracts/proxy/utils/UUPSUpgradeable.sol
@@ -126,7 +126,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
     function _authorizeUpgrade(address newImplementation) internal virtual;
 
     /**
-     * @dev Performs ab implementation upgrade with a security check for UUPS proxies, and additional setup call.
+     * @dev Performs an implementation upgrade with a security check for UUPS proxies, and additional setup call.
      *
      * As a security check, {proxiableUUID} is invoked in the new implementation, and the return value
      * is expected to be the implementation slot in ERC1967.

From c2e835edcc97c3506ad4604ee321f69b9cc198a7 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Wed, 2 Aug 2023 13:32:33 -0600
Subject: [PATCH 15/26] Fix R-04

---
 .../proxy/transparent/TransparentUpgradeableProxy.sol | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index 413f46ecf7d..b64365c35f4 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -78,14 +78,21 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
     constructor(address _logic, address initialOwner, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
         _admin = address(new ProxyAdmin(initialOwner));
         // Set the storage value and emit an event for ERC-1967 compatibility
-        ERC1967Utils.changeAdmin(_admin);
+        ERC1967Utils.changeAdmin(_proxyAdmin());
+    }
+
+    /**
+     * @dev Returns the admin of this proxy.
+     */
+    function _proxyAdmin() internal virtual returns (address) {
+        return _admin;
     }
 
     /**
      * @dev If caller is the admin process the call internally, otherwise transparently fallback to the proxy behavior.
      */
     function _fallback() internal virtual override {
-        if (msg.sender == _admin) {
+        if (msg.sender == _proxyAdmin()) {
             if (msg.sig == ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
                 _dispatchUpgradeToAndCall();
             } else {

From 72635240430ea1a1a17b90bc68de882963b97008 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 3 Aug 2023 16:14:59 -0600
Subject: [PATCH 16/26] Fix N-14

---
 contracts/proxy/transparent/TransparentUpgradeableProxy.sol | 6 +++---
 contracts/utils/Address.sol                                 | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
index b64365c35f4..77ed5fe730c 100644
--- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
+++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol
@@ -93,10 +93,10 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
      */
     function _fallback() internal virtual override {
         if (msg.sender == _proxyAdmin()) {
-            if (msg.sig == ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
-                _dispatchUpgradeToAndCall();
-            } else {
+            if (msg.sig != ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
                 revert ProxyDeniedAdminAccess();
+            } else {
+                _dispatchUpgradeToAndCall();
             }
         } else {
             super._fallback();
diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol
index a2614787ffc..74ea4bc7a41 100644
--- a/contracts/utils/Address.sol
+++ b/contracts/utils/Address.sol
@@ -116,7 +116,9 @@ library Address {
         bool success,
         bytes memory returndata
     ) internal view returns (bytes memory) {
-        if (success) {
+        if (!success) {
+            _revert(returndata);
+        } else {
             if (returndata.length == 0) {
                 // only check if target is a contract if the call was successful and the return data is empty
                 // otherwise we already know that it was a contract
@@ -125,8 +127,6 @@ library Address {
                 }
             }
             return returndata;
-        } else {
-            _revert(returndata);
         }
     }
 

From 6689060500fd53c4577ff441693962725b39fe36 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 3 Aug 2023 16:30:10 -0600
Subject: [PATCH 17/26] Fix N-16

---
 contracts/finance/VestingWallet.sol | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol
index cd1a2406448..2e8fdab8255 100644
--- a/contracts/finance/VestingWallet.sol
+++ b/contracts/finance/VestingWallet.sol
@@ -19,6 +19,9 @@ import {Context} from "../utils/Context.sol";
  *
  * By setting the duration to 0, one can configure this contract to behave like an asset timelock that hold tokens for
  * a beneficiary until a specified time.
+ *
+ * NOTE: When using this contract with any token whose balance is adjusted automatically (i.e. a rebase token), make sure
+ * to account the supply/balance adjustment in the vesting schedule to ensure the vested amount is as intended.
  */
 contract VestingWallet is Context {
     event EtherReleased(uint256 amount);

From 11213f59912149f2df68632a0b115d67092288c3 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 3 Aug 2023 21:51:52 -0600
Subject: [PATCH 18/26] Fix L-03-2

---
 contracts/metatx/ERC2771Forwarder.sol | 82 ++++++++++++++++++++++++---
 contracts/mocks/CallReceiverMock.sol  | 12 ++++
 test/metatx/ERC2771Forwarder.t.sol    |  6 +-
 test/metatx/ERC2771Forwarder.test.js  | 29 +++++++++-
 4 files changed, 115 insertions(+), 14 deletions(-)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index 54780605a5c..4b1e0b53714 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -3,6 +3,7 @@
 
 pragma solidity ^0.8.20;
 
+import {ERC2771Context} from "./ERC2771Context.sol";
 import {ECDSA} from "../utils/cryptography/ECDSA.sol";
 import {EIP712} from "../utils/cryptography/EIP712.sol";
 import {Nonces} from "../utils/Nonces.sol";
@@ -92,6 +93,11 @@ contract ERC2771Forwarder is EIP712, Nonces {
      */
     error ERC2771ForwarderExpiredRequest(uint48 deadline);
 
+    /**
+     * @dev The request targert doesn't trust the `forwarder`.
+     */
+    error ERC2771UntrustfulTarget(address target, address forwarder);
+
     /**
      * @dev See {EIP712-constructor}.
      */
@@ -100,15 +106,15 @@ contract ERC2771Forwarder is EIP712, Nonces {
     /**
      * @dev Returns `true` if a request is valid for a provided `signature` at the current block timestamp.
      *
-     * A transaction is considered valid when it hasn't expired (deadline is not met), and the signer
-     * matches the `from` parameter of the signed request.
+     * A transaction is considered valid when the target trusts this forwarder, the request hasn't expired
+     * (deadline is not met), and the signer matches the `from` parameter of the signed request.
      *
      * NOTE: A request may return false here but it won't cause {executeBatch} to revert if a refund
      * receiver is provided.
      */
     function verify(ForwardRequestData calldata request) public view virtual returns (bool) {
-        (bool active, bool signerMatch, ) = _validate(request);
-        return active && signerMatch;
+        (bool isTrustedForwarder, bool active, bool signerMatch, ) = _validate(request);
+        return isTrustedForwarder && active && signerMatch;
     }
 
     /**
@@ -196,9 +202,15 @@ contract ERC2771Forwarder is EIP712, Nonces {
      */
     function _validate(
         ForwardRequestData calldata request
-    ) internal view virtual returns (bool active, bool signerMatch, address signer) {
+    ) internal view virtual returns (bool isTrustedForwarder, bool active, bool signerMatch, address signer) {
         (bool isValid, address recovered) = _recoverForwardRequestSigner(request);
-        return (request.deadline >= block.timestamp, isValid && recovered == request.from, recovered);
+
+        return (
+            _isTrustedByTarget(request.to),
+            request.deadline >= block.timestamp,
+            isValid && recovered == request.from,
+            recovered
+        );
     }
 
     /**
@@ -247,11 +259,15 @@ contract ERC2771Forwarder is EIP712, Nonces {
         ForwardRequestData calldata request,
         bool requireValidRequest
     ) internal virtual returns (bool success) {
-        (bool active, bool signerMatch, address signer) = _validate(request);
+        (bool isTrustedForwarder, bool active, bool signerMatch, address signer) = _validate(request);
 
         // Need to explicitly specify if a revert is required since non-reverting is default for
         // batches and reversion is opt-in since it could be useful in some scenarios
         if (requireValidRequest) {
+            if (!isTrustedForwarder) {
+                revert ERC2771UntrustfulTarget(request.to, address(this));
+            }
+
             if (!active) {
                 revert ERC2771ForwarderExpiredRequest(request.deadline);
             }
@@ -262,7 +278,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         }
 
         // Ignore an invalid request because requireValidRequest = false
-        if (signerMatch && active) {
+        if (isTrustedForwarder && signerMatch && active) {
             // Nonce should be used before the call to prevent reusing by reentrancy
             uint256 currentNonce = _useNonce(signer);
 
@@ -284,6 +300,56 @@ contract ERC2771Forwarder is EIP712, Nonces {
         }
     }
 
+    /**
+     * @dev Returns whether the target trusts this forwarder.
+     *
+     * This function performs a static call to the target contract calling the
+     * {ERC2771Context-isTrustedForwarder} function.
+     */
+    function _isTrustedByTarget(address target) private view returns (bool) {
+        bytes4 selector = ERC2771Context.isTrustedForwarder.selector;
+
+        // Calldata is always the same, and we can guarantee its length is 36 bytes
+        // - 4 bytes from the selector
+        // - 32 bytes from the abi encoded target address (padded to 32)
+        uint8 calldataLength = 36;
+
+        bool success;
+        bool returnValue;
+        /// @solidity memory-safe-assembly
+        assembly {
+            // Because the return value is a boolean (requires 1 byte copied to memory) and the
+            // calldata length is 24 bytes, we can safely reuse the scratch space in memory (0x00 - 0x3F).
+            // This avoids memory leakage of allocating the encoded calldata in memory during each
+            // `isTrustedForwarder` call.
+            let ptr := 0x00
+
+            // | Location  | Content  | Content (Hex)                                                      |
+            // |-----------|----------|--------------------------------------------------------------------|
+            // |           |          |                                                  selector ↓        |
+            // | 0x00:0x1F | selector | 0x00000000000000000000000000000000000000000000000000000000AAAAAAAA |
+            // |           |          |   ↓ PADDING        target ↓                                        |
+            // | 0x20:0x3F | target   | 0x000000000000000000000000BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB |
+            mstore(ptr, shr(224, selector))
+            mstore(add(ptr, 0x20), address())
+
+            // Perform the staticcal and save the result in the scratch space.
+            // | Location  | Content  | Content (Hex)                                                      |
+            // |-----------|----------|--------------------------------------------------------------------|
+            // |           |          |                                                           result ↓ |
+            // | 0x00:0x1F | selector | 0x0000000000000000000000000000000000000000000000000000000000000001 |
+            success := staticcall(gas(), target, add(ptr, 0x1c), calldataLength, 0, 0x20)
+
+            // Avoid strange returndatasizes (eg. an EOA)
+            if eq(returndatasize(), 0x20) {
+                // Copy from memory and clean the `returnValue` as a boolean.
+                returnValue := shr(255, shl(255, mload(0)))
+            }
+        }
+
+        return success && returnValue;
+    }
+
     /**
      * @dev Checks if the requested gas was correctly forwarded to the callee.
      *
diff --git a/contracts/mocks/CallReceiverMock.sol b/contracts/mocks/CallReceiverMock.sol
index 281afacfe04..e371c7db800 100644
--- a/contracts/mocks/CallReceiverMock.sol
+++ b/contracts/mocks/CallReceiverMock.sol
@@ -59,3 +59,15 @@ contract CallReceiverMock {
         return "0x1234";
     }
 }
+
+contract CallReceiverMockTrustingForwarder is CallReceiverMock {
+    address private _trustedForwarder;
+
+    constructor(address trustedForwarder_) {
+        _trustedForwarder = trustedForwarder_;
+    }
+
+    function isTrustedForwarder(address forwarder) public view virtual returns (bool) {
+        return forwarder == _trustedForwarder;
+    }
+}
diff --git a/test/metatx/ERC2771Forwarder.t.sol b/test/metatx/ERC2771Forwarder.t.sol
index 189ed6ac5ac..3256289ea68 100644
--- a/test/metatx/ERC2771Forwarder.t.sol
+++ b/test/metatx/ERC2771Forwarder.t.sol
@@ -4,7 +4,7 @@ pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
 import {ERC2771Forwarder} from "contracts/metatx/ERC2771Forwarder.sol";
-import {CallReceiverMock} from "contracts/mocks/CallReceiverMock.sol";
+import {CallReceiverMockTrustingForwarder, CallReceiverMock} from "contracts/mocks/CallReceiverMock.sol";
 
 struct ForwardRequest {
     address from;
@@ -40,7 +40,7 @@ contract ERC2771ForwarderMock is ERC2771Forwarder {
 
 contract ERC2771ForwarderTest is Test {
     ERC2771ForwarderMock internal _erc2771Forwarder;
-    CallReceiverMock internal _receiver;
+    CallReceiverMockTrustingForwarder internal _receiver;
 
     uint256 internal _signerPrivateKey;
     uint256 internal _relayerPrivateKey;
@@ -52,7 +52,7 @@ contract ERC2771ForwarderTest is Test {
 
     function setUp() public {
         _erc2771Forwarder = new ERC2771ForwarderMock("ERC2771Forwarder");
-        _receiver = new CallReceiverMock();
+        _receiver = new CallReceiverMockTrustingForwarder(address(_erc2771Forwarder));
 
         _signerPrivateKey = 0xA11CE;
         _relayerPrivateKey = 0xB0B;
diff --git a/test/metatx/ERC2771Forwarder.test.js b/test/metatx/ERC2771Forwarder.test.js
index 26726e8df4c..209c84b2f36 100644
--- a/test/metatx/ERC2771Forwarder.test.js
+++ b/test/metatx/ERC2771Forwarder.test.js
@@ -7,14 +7,13 @@ const { constants, expectRevert, expectEvent, time } = require('@openzeppelin/te
 const { expect } = require('chai');
 
 const ERC2771Forwarder = artifacts.require('ERC2771Forwarder');
-const CallReceiverMock = artifacts.require('CallReceiverMock');
+const CallReceiverMockTrustingForwarder = artifacts.require('CallReceiverMockTrustingForwarder');
 
 contract('ERC2771Forwarder', function (accounts) {
   const [, refundReceiver, another] = accounts;
 
   const tamperedValues = {
     from: another,
-    to: another,
     value: web3.utils.toWei('0.5'),
     data: '0x1742',
     deadline: 0xdeadbeef,
@@ -41,7 +40,7 @@ contract('ERC2771Forwarder', function (accounts) {
     this.alice.address = web3.utils.toChecksumAddress(this.alice.getAddressString());
 
     this.timestamp = await time.latest();
-    this.receiver = await CallReceiverMock.new();
+    this.receiver = await CallReceiverMockTrustingForwarder.new(this.forwarder.address);
     this.request = {
       from: this.alice.address,
       to: this.receiver.address,
@@ -97,6 +96,10 @@ contract('ERC2771Forwarder', function (accounts) {
         });
       }
 
+      it('returns false with an untrustful to', async function () {
+        expect(await this.forwarder.verify(this.forgeData({ to: another }).message)).to.be.equal(false);
+      });
+
       it('returns false with tampered signature', async function () {
         const tamperedsign = web3.utils.hexToBytes(this.requestData.signature);
         tamperedsign[42] ^= 0xff;
@@ -169,6 +172,14 @@ contract('ERC2771Forwarder', function (accounts) {
         });
       }
 
+      it('reverts with an untrustful to', async function () {
+        const data = this.forgeData({ to: another });
+        await expectRevertCustomError(this.forwarder.execute(data.message), 'ERC2771UntrustfulTarget', [
+          data.message.to,
+          this.forwarder.address,
+        ]);
+      });
+
       it('reverts with tampered signature', async function () {
         const tamperedSig = web3.utils.hexToBytes(this.requestData.signature);
         tamperedSig[42] ^= 0xff;
@@ -360,6 +371,18 @@ contract('ERC2771Forwarder', function (accounts) {
           });
         }
 
+        it('reverts with at least one untrustful to', async function () {
+          const data = this.forgeData({ ...this.requestDatas[this.idx], to: another });
+
+          this.requestDatas[this.idx] = data.message;
+
+          await expectRevertCustomError(
+            this.forwarder.executeBatch(this.requestDatas, this.refundReceiver, { value: this.msgValue }),
+            'ERC2771UntrustfulTarget',
+            [this.requestDatas[this.idx].to, this.forwarder.address],
+          );
+        });
+
         it('reverts with at least one tampered request signature', async function () {
           const tamperedSig = web3.utils.hexToBytes(this.requestDatas[this.idx].signature);
           tamperedSig[42] ^= 0xff;

From 29fd3ea944ee95bd4a77eba45bafe06458442cf1 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Thu, 3 Aug 2023 22:19:20 -0600
Subject: [PATCH 19/26] Fix L-10

---
 contracts/proxy/beacon/UpgradeableBeacon.sol | 4 ++--
 contracts/proxy/transparent/ProxyAdmin.sol   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contracts/proxy/beacon/UpgradeableBeacon.sol b/contracts/proxy/beacon/UpgradeableBeacon.sol
index a7816a1e6d0..ca692ea68e5 100644
--- a/contracts/proxy/beacon/UpgradeableBeacon.sol
+++ b/contracts/proxy/beacon/UpgradeableBeacon.sol
@@ -4,7 +4,7 @@
 pragma solidity ^0.8.20;
 
 import {IBeacon} from "./IBeacon.sol";
-import {Ownable} from "../../access/Ownable.sol";
+import {Ownable2Step, Ownable} from "../../access/Ownable2Step.sol";
 
 /**
  * @dev This contract is used in conjunction with one or more instances of {BeaconProxy} to determine their
@@ -12,7 +12,7 @@ import {Ownable} from "../../access/Ownable.sol";
  *
  * An owner is able to change the implementation the beacon points to, thus upgrading the proxies that use this beacon.
  */
-contract UpgradeableBeacon is IBeacon, Ownable {
+contract UpgradeableBeacon is IBeacon, Ownable2Step {
     address private _implementation;
 
     /**
diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol
index cd03fb939e4..d5d1151dd00 100644
--- a/contracts/proxy/transparent/ProxyAdmin.sol
+++ b/contracts/proxy/transparent/ProxyAdmin.sol
@@ -4,13 +4,13 @@
 pragma solidity ^0.8.20;
 
 import {ITransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol";
-import {Ownable} from "../../access/Ownable.sol";
+import {Ownable2Step, Ownable} from "../../access/Ownable2Step.sol";
 
 /**
  * @dev This is an auxiliary contract meant to be assigned as the admin of a {TransparentUpgradeableProxy}. For an
  * explanation of why you would want to use this see the documentation for {TransparentUpgradeableProxy}.
  */
-contract ProxyAdmin is Ownable {
+contract ProxyAdmin is Ownable2Step {
     /**
      * @dev The version of the upgrade interface of the contract. If this getter is missing, both `upgrade(address)`
      * and `upgradeAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,

From 889c11271f704e25b6d5685e9b0b8c36e4695fd3 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 10:50:15 -0300
Subject: [PATCH 20/26] minimize assembly in _isTrustedByTraget

---
 contracts/metatx/ERC2771Forwarder.sol | 34 ++++++---------------------
 1 file changed, 7 insertions(+), 27 deletions(-)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index 4b1e0b53714..daffba1bdad 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -307,47 +307,27 @@ contract ERC2771Forwarder is EIP712, Nonces {
      * {ERC2771Context-isTrustedForwarder} function.
      */
     function _isTrustedByTarget(address target) private view returns (bool) {
-        bytes4 selector = ERC2771Context.isTrustedForwarder.selector;
-
-        // Calldata is always the same, and we can guarantee its length is 36 bytes
-        // - 4 bytes from the selector
-        // - 32 bytes from the abi encoded target address (padded to 32)
-        uint8 calldataLength = 36;
+        bytes memory encodedParams = abi.encodeCall(ERC2771Context.isTrustedForwarder, (address(this)));
 
         bool success;
-        bool returnValue;
+        uint256 returnSize;
+        uint256 returnValue;
         /// @solidity memory-safe-assembly
         assembly {
             // Because the return value is a boolean (requires 1 byte copied to memory) and the
             // calldata length is 24 bytes, we can safely reuse the scratch space in memory (0x00 - 0x3F).
-            // This avoids memory leakage of allocating the encoded calldata in memory during each
-            // `isTrustedForwarder` call.
-            let ptr := 0x00
-
-            // | Location  | Content  | Content (Hex)                                                      |
-            // |-----------|----------|--------------------------------------------------------------------|
-            // |           |          |                                                  selector ↓        |
-            // | 0x00:0x1F | selector | 0x00000000000000000000000000000000000000000000000000000000AAAAAAAA |
-            // |           |          |   ↓ PADDING        target ↓                                        |
-            // | 0x20:0x3F | target   | 0x000000000000000000000000BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB |
-            mstore(ptr, shr(224, selector))
-            mstore(add(ptr, 0x20), address())
 
             // Perform the staticcal and save the result in the scratch space.
             // | Location  | Content  | Content (Hex)                                                      |
             // |-----------|----------|--------------------------------------------------------------------|
             // |           |          |                                                           result ↓ |
             // | 0x00:0x1F | selector | 0x0000000000000000000000000000000000000000000000000000000000000001 |
-            success := staticcall(gas(), target, add(ptr, 0x1c), calldataLength, 0, 0x20)
-
-            // Avoid strange returndatasizes (eg. an EOA)
-            if eq(returndatasize(), 0x20) {
-                // Copy from memory and clean the `returnValue` as a boolean.
-                returnValue := shr(255, shl(255, mload(0)))
-            }
+            success := staticcall(gas(), target, add(encodedParams, 0x20), mload(encodedParams), 0, 0x20)
+            returnSize := returndatasize()
+            returnValue := mload(0)
         }
 
-        return success && returnValue;
+        return success && returnSize >= 0x20 && returnValue > 0;
     }
 
     /**

From 13cdb42d91e10d5ac45313bc39b686b174f9a699 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 10:54:18 -0300
Subject: [PATCH 21/26] add changeset for VestingWallet

---
 .changeset/hip-goats-fail.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/hip-goats-fail.md

diff --git a/.changeset/hip-goats-fail.md b/.changeset/hip-goats-fail.md
new file mode 100644
index 00000000000..5cfe2ef79df
--- /dev/null
+++ b/.changeset/hip-goats-fail.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`VestingWallet`: Fix revert during 1 second time window when duration is 0.

From 745b14d6aad24f75a538eaa410f876e2a94fede2 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 10:59:24 -0300
Subject: [PATCH 22/26] merge if statements

---
 contracts/utils/Address.sol | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol
index 74ea4bc7a41..035ad7dc98a 100644
--- a/contracts/utils/Address.sol
+++ b/contracts/utils/Address.sol
@@ -119,12 +119,10 @@ library Address {
         if (!success) {
             _revert(returndata);
         } else {
-            if (returndata.length == 0) {
-                // only check if target is a contract if the call was successful and the return data is empty
-                // otherwise we already know that it was a contract
-                if (target.code.length == 0) {
-                    revert AddressEmptyCode(target);
-                }
+            // only check if target is a contract if the call was successful and the return data is empty
+            // otherwise we already know that it was a contract
+            if (returndata.length == 0 && target.code.length == 0) {
+                revert AddressEmptyCode(target);
             }
             return returndata;
         }

From 3d8babad048f820b1f8cb314ffde842a9603d494 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 10:59:51 -0300
Subject: [PATCH 23/26] swap if statements for consistency

---
 contracts/utils/Address.sol | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol
index 035ad7dc98a..fd22b05ab7b 100644
--- a/contracts/utils/Address.sol
+++ b/contracts/utils/Address.sol
@@ -133,10 +133,10 @@ library Address {
      * revert reason or with a default {FailedInnerCall} error.
      */
     function verifyCallResult(bool success, bytes memory returndata) internal pure returns (bytes memory) {
-        if (success) {
-            return returndata;
-        } else {
+        if (!success) {
             _revert(returndata);
+        } else {
+            return returndata;
         }
     }
 

From ceb19fdd4b4d20b3f1ed50be7716ebd71560b661 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <fg@frang.io>
Date: Fri, 4 Aug 2023 13:05:16 -0300
Subject: [PATCH 24/26] Revert "Fix L-10"

This reverts commit 29fd3ea944ee95bd4a77eba45bafe06458442cf1.
---
 contracts/proxy/beacon/UpgradeableBeacon.sol | 4 ++--
 contracts/proxy/transparent/ProxyAdmin.sol   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contracts/proxy/beacon/UpgradeableBeacon.sol b/contracts/proxy/beacon/UpgradeableBeacon.sol
index ca692ea68e5..a7816a1e6d0 100644
--- a/contracts/proxy/beacon/UpgradeableBeacon.sol
+++ b/contracts/proxy/beacon/UpgradeableBeacon.sol
@@ -4,7 +4,7 @@
 pragma solidity ^0.8.20;
 
 import {IBeacon} from "./IBeacon.sol";
-import {Ownable2Step, Ownable} from "../../access/Ownable2Step.sol";
+import {Ownable} from "../../access/Ownable.sol";
 
 /**
  * @dev This contract is used in conjunction with one or more instances of {BeaconProxy} to determine their
@@ -12,7 +12,7 @@ import {Ownable2Step, Ownable} from "../../access/Ownable2Step.sol";
  *
  * An owner is able to change the implementation the beacon points to, thus upgrading the proxies that use this beacon.
  */
-contract UpgradeableBeacon is IBeacon, Ownable2Step {
+contract UpgradeableBeacon is IBeacon, Ownable {
     address private _implementation;
 
     /**
diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol
index d5d1151dd00..cd03fb939e4 100644
--- a/contracts/proxy/transparent/ProxyAdmin.sol
+++ b/contracts/proxy/transparent/ProxyAdmin.sol
@@ -4,13 +4,13 @@
 pragma solidity ^0.8.20;
 
 import {ITransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol";
-import {Ownable2Step, Ownable} from "../../access/Ownable2Step.sol";
+import {Ownable} from "../../access/Ownable.sol";
 
 /**
  * @dev This is an auxiliary contract meant to be assigned as the admin of a {TransparentUpgradeableProxy}. For an
  * explanation of why you would want to use this see the documentation for {TransparentUpgradeableProxy}.
  */
-contract ProxyAdmin is Ownable2Step {
+contract ProxyAdmin is Ownable {
     /**
      * @dev The version of the upgrade interface of the contract. If this getter is missing, both `upgrade(address)`
      * and `upgradeAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,

From f9904fa46b05a9793f91057a6904763898ab191a Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Fri, 4 Aug 2023 14:04:01 -0300
Subject: [PATCH 25/26] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Ernesto García <ernestognw@gmail.com>
---
 contracts/metatx/ERC2771Forwarder.sol | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index daffba1bdad..858c1e2a287 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -94,7 +94,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
     error ERC2771ForwarderExpiredRequest(uint48 deadline);
 
     /**
-     * @dev The request targert doesn't trust the `forwarder`.
+     * @dev The request target doesn't trust the `forwarder`.
      */
     error ERC2771UntrustfulTarget(address target, address forwarder);
 
@@ -314,8 +314,6 @@ contract ERC2771Forwarder is EIP712, Nonces {
         uint256 returnValue;
         /// @solidity memory-safe-assembly
         assembly {
-            // Because the return value is a boolean (requires 1 byte copied to memory) and the
-            // calldata length is 24 bytes, we can safely reuse the scratch space in memory (0x00 - 0x3F).
 
             // Perform the staticcal and save the result in the scratch space.
             // | Location  | Content  | Content (Hex)                                                      |

From 8cb5200797e2ff322d3632be6c9df80e72dd8fed Mon Sep 17 00:00:00 2001
From: Francisco <fg@frang.io>
Date: Fri, 4 Aug 2023 14:04:22 -0300
Subject: [PATCH 26/26] remove whitespace

---
 contracts/metatx/ERC2771Forwarder.sol | 1 -
 1 file changed, 1 deletion(-)

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
index 858c1e2a287..6f8e000d217 100644
--- a/contracts/metatx/ERC2771Forwarder.sol
+++ b/contracts/metatx/ERC2771Forwarder.sol
@@ -314,7 +314,6 @@ contract ERC2771Forwarder is EIP712, Nonces {
         uint256 returnValue;
         /// @solidity memory-safe-assembly
         assembly {
-
             // Perform the staticcal and save the result in the scratch space.
             // | Location  | Content  | Content (Hex)                                                      |
             // |-----------|----------|--------------------------------------------------------------------|