Non-reentrant DAO executor

packages/contracts/src/core/dao/DAO.sol

@@ -67,16 +67,19 @@ contract DAO is
     uint256 private constant _ENTERED = 2;
 
     /// @notice The [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature validator contract.
+    /// @dev Added in v1.0.0.
     IERC1271 public signatureValidator;
 
     /// @notice The address of the trusted forwarder verifying meta transactions.
+    /// @dev Added in v1.0.0.
     address private trustedForwarder;
 
     /// @notice The [EIP-4824](https://eips.ethereum.org/EIPS/eip-4824) DAO URI.
+    /// @dev Added in v1.0.0.
     string private _daoURI;
 
     /// @notice The state variable for the reentrancy guard of the `execute` function.
-    /// @dev The variable can of value `_NOT_ENTERED = 1` or `_ENTERED = 2` in usage and shoud be initialized with `_NOT_ENTERED`.
+    /// @dev Added in v1.3.0. The variable can be of value `_NOT_ENTERED = 1` or `_ENTERED = 2` in usage and is initialized with `_NOT_ENTERED`.
     uint256 private _reentrancyStatus;
 
     /// @notice Thrown if a call is reentrant.
@@ -141,7 +144,7 @@ contract DAO is
         address _trustedForwarder,
         string calldata daoURI_
     ) external reinitializer(2) {
-        _reentrancyStatus = _NOT_ENTERED;
+        _reentrancyStatus = _NOT_ENTERED; // added in v1.3.0
 
         _registerInterface(type(IDAO).interfaceId);
         _registerInterface(type(IERC1271).interfaceId);
@@ -168,7 +171,7 @@ contract DAO is
             revert ProtocolVersionUpgradeNotSupported(_previousProtocolVersion);
         }
 
-        // Initialize `_reentrancyStatus` that was added in v1.3.0
+        // Initialize `_reentrancyStatus` that was added in v1.3.0.
         if (_previousProtocolVersion[1] <= 2) {
             _reentrancyStatus = _NOT_ENTERED;
         }

packages/contracts/test/core/dao/dao.ts

@@ -239,22 +239,56 @@ describe('DAO', function () {
         .withArgs([0, 1, 0]);
     });
 
-    it('reverts if trying to upgrade to the same version', async () => {
+    it('initializes `_reentrancyStatus` for versions < 1.3.0', async () => {
+      // Create an unitialized DAO.
       const uninitializedDao = await deployWithProxy<DAO>(DAO);
 
-      await expect(uninitializedDao.initializeFrom([1, 3, 0], EMPTY_DATA))
-        .to.be.revertedWithCustomError(
-          dao,
-          'ProtocolVersionUpgradeNotSupported'
-        )
-        .withArgs([1, 3, 0]);
+      // Expect the contract to be uninitialized  with `_initialized = 0` and `_reentrancyStatus = 0`.
+      expect(
+        ethers.BigNumber.from(
+          await ethers.provider.getStorageAt(
+            uninitializedDao.address,
+            OZ_INITIALIZED_SLOT_POSITION
+          )
+        ).toNumber()
+      ).to.equal(0);
+      expect(
+        ethers.BigNumber.from(
+          await ethers.provider.getStorageAt(
+            uninitializedDao.address,
+            REENTRANCY_STATUS_SLOT_POSITION
+          )
+        ).toNumber()
+      ).to.equal(0);
+
+      // Call `initializeFrom` with version 1.2.0.
+      await expect(uninitializedDao.initializeFrom([1, 2, 0], EMPTY_DATA)).to
+        .not.be.reverted;
+
+      // Expect the contract to be initialized with `_initialized = 2` and  `_reentrancyStatus = 1`.
+      expect(
+        ethers.BigNumber.from(
+          await ethers.provider.getStorageAt(
+            uninitializedDao.address,
+            OZ_INITIALIZED_SLOT_POSITION
+          )
+        ).toNumber()
+      ).to.equal(2);
+      expect(
+        ethers.BigNumber.from(
+          await ethers.provider.getStorageAt(
+            uninitializedDao.address,
+            REENTRANCY_STATUS_SLOT_POSITION
+          )
+        ).toNumber()
+      ).to.equal(1);
     });
 
-    it('initializes upgrades for versions < 1.3.0', async () => {
+    it('does not initialize `_reentrancyStatus` for versions >= 1.3.0', async () => {
       // Create an unitialized DAO.
       const uninitializedDao = await deployWithProxy<DAO>(DAO);
 
-      // Expect the contract to be uninitialized  with `_initialized = 0`.
+      // Expect the contract to be uninitialized  with `_initialized = 0` and `_reentrancyStatus = 0`.
       expect(
         ethers.BigNumber.from(
           await ethers.provider.getStorageAt(
@@ -263,20 +297,20 @@ describe('DAO', function () {
           )
         ).toNumber()
       ).to.equal(0);
+      expect(
+        ethers.BigNumber.from(
+          await ethers.provider.getStorageAt(
+            uninitializedDao.address,
+            REENTRANCY_STATUS_SLOT_POSITION
+          )
+        ).toNumber()
+      ).to.equal(0);
 
-      // Call `initializeFrom` with the previous version 1.3.0 which is not supported.
-      await expect(uninitializedDao.initializeFrom([1, 3, 0], EMPTY_DATA))
-        .to.be.revertedWithCustomError(
-          dao,
-          'ProtocolVersionUpgradeNotSupported'
-        )
-        .withArgs([1, 3, 0]);
-
-      // Call `initializeFrom` with the previous version 1.2.0.
-      await expect(uninitializedDao.initializeFrom([1, 2, 0], EMPTY_DATA)).to
+      // Call `initializeFrom` with version 1.3.0.
+      await expect(uninitializedDao.initializeFrom([1, 3, 0], EMPTY_DATA)).to
         .not.be.reverted;
 
-      // Expect the contract to be initialized with `_initialized = 2`
+      // Expect the contract to be initialized with `_initialized = 2` but `_reentrancyStatus` to remain unchanged.
       expect(
         ethers.BigNumber.from(
           await ethers.provider.getStorageAt(
@@ -285,6 +319,14 @@ describe('DAO', function () {
           )
         ).toNumber()
       ).to.equal(2);
+      expect(
+        ethers.BigNumber.from(
+          await ethers.provider.getStorageAt(
+            uninitializedDao.address,
+            REENTRANCY_STATUS_SLOT_POSITION
+          )
+        ).toNumber()
+      ).to.equal(0);
     });
   });