Adding more tests

Author: brickpop

test/core/dao/DAO.t.sol

@@ -150,6 +150,53 @@ contract DAOInitializeTest is DAOTestBase {
     }
 }
 
+/// @notice `initializeFrom` (the post-upgrade reinitializer). The live v1.3→v1.4
+/// path can't reach its body (F9: v1.3.0 already set `_initialized == 3`), so we
+/// exercise it directly on a fresh proxy: deployed WITHOUT `initialize`,
+/// `_initialized == 0 < 3`, so `reinitializer(3)` runs. Covers the major-version
+/// gate and the version-conditional side effects.
+contract DAOInitializeFromTest is DAOTestBase {
+    /// A proxy pointing at a DAO impl but never `initialize`d (`_initialized == 0`).
+    function _freshProxy() internal returns (DAO) {
+        DAO impl = new DAO();
+        return DAO(payable(address(new ERC1967Proxy(address(impl), ""))));
+    }
+
+    /// Upgrading across a major release is rejected.
+    function test_initializeFrom_revertsIfMajorVersionNotOne() public {
+        DAO d = _freshProxy();
+        uint8[3] memory prev = [uint8(2), 0, 0];
+        vm.expectRevert(abi.encodeWithSelector(DAO.ProtocolVersionUpgradeNotSupported.selector, prev));
+        d.initializeFrom(prev, "");
+    }
+
+    /// From a pre-1.3.0 source, the body arms the reentrancy guard and registers
+    /// the interfaces added in v1.3.0 (`IProtocolVersion`) and v1.4.0 (`IDAO`,
+    /// `IExecutor`), and bumps the OZ initialized slot to 3.
+    function test_initializeFrom_fromPre130_initsReentrancyGuardAndInterfaces() public {
+        DAO d = _freshProxy();
+        d.initializeFrom([uint8(1), 0, 0], "");
+
+        assertEq(uint8(uint256(vm.load(address(d), bytes32(uint256(0))))), 3, "_initialized == 3");
+        assertEq(uint256(vm.load(address(d), bytes32(uint256(304)))), 1, "_reentrancyStatus == _NOT_ENTERED");
+        assertTrue(d.supportsInterface(type(IProtocolVersion).interfaceId), "IProtocolVersion registered");
+        assertTrue(d.supportsInterface(type(IDAO).interfaceId), "IDAO registered");
+        assertTrue(d.supportsInterface(type(IExecutor).interfaceId), "IExecutor registered");
+    }
+
+    /// Boundary check: `[1,3,0]` is NOT `lt [1,3,0]`, so the reentrancy /
+    /// `IProtocolVersion` branch is skipped — proving the version gate is strict.
+    function test_initializeFrom_from130_skipsReentrancyBranch() public {
+        DAO d = _freshProxy();
+        d.initializeFrom([uint8(1), 3, 0], "");
+
+        assertEq(uint256(vm.load(address(d), bytes32(uint256(304)))), 0, "reentrancy branch skipped");
+        assertFalse(d.supportsInterface(type(IProtocolVersion).interfaceId), "IProtocolVersion NOT registered");
+        // The `< [1,4,0]` branch still ran.
+        assertTrue(d.supportsInterface(type(IDAO).interfaceId), "IDAO registered");
+    }
+}
+
 /// @notice setTrustedForwarder + setMetadata.
 contract DAOMetadataTest is DAOTestBase {
     function test_setTrustedForwarder_revertsIfCallerLacksPermission() public {

test/framework/plugin/setup/PSP.Installation.t.sol

@@ -122,6 +122,25 @@ contract PSPPrepareInstallationTest is PSPBaseTest {
         assertTrue(found, "InstallationPrepared not emitted");
     }
 
+    /// Preparing an installation for a plugin that is already installed reverts.
+    /// Build 5 (Bad) returns a fixed plugin address, so re-preparing after an
+    /// apply lands in the SAME installation slot (non-zero `currentAppliedSetupId`).
+    function test_prepareInstallation_revertsIfPluginAlreadyInstalled() public {
+        (address plugin, IPluginSetup.PreparedSetupData memory data) =
+            psp.prepareInstallation(address(dao), _prepareInstallParams(5, ""));
+        _grantApplyInstallation(owner);
+        _grantPspRoot();
+        psp.applyInstallation(
+            address(dao),
+            PluginSetupProcessor.ApplyInstallationParams({
+                pluginSetupRef: _ref(5), plugin: plugin, permissions: data.permissions, helpersHash: hashHelpers(data.helpers)
+            })
+        );
+
+        vm.expectRevert(PluginSetupProcessor.PluginAlreadyInstalled.selector);
+        psp.prepareInstallation(address(dao), _prepareInstallParams(5, ""));
+    }
+
     function test_prepareInstallation_revertsIfSamePrepIdPending() public {
         // The V1 mock deploys a NEW proxy each call (different `plugin` →
         // different installationId), so re-using V1 lands in a fresh state
@@ -208,6 +227,24 @@ contract PSPApplyInstallationTest is PSPBaseTest {
         assertEq(currentAppliedId, expectedAppliedId);
     }
 
+    /// Applying an installation for an already-installed plugin reverts. The
+    /// `currentAppliedSetupId != 0` guard is checked BEFORE `validatePreparedSetupId`,
+    /// so re-applying the same params surfaces `PluginAlreadyInstalled` (not
+    /// `SetupNotApplicable`). Build 5 (Bad) gives the fixed plugin address needed.
+    function test_applyInstallation_revertsIfPluginAlreadyInstalled() public {
+        (address plugin, IPluginSetup.PreparedSetupData memory data) =
+            psp.prepareInstallation(address(dao), _prepareInstallParams(5, ""));
+        PluginSetupProcessor.ApplyInstallationParams memory p = PluginSetupProcessor.ApplyInstallationParams({
+            pluginSetupRef: _ref(5), plugin: plugin, permissions: data.permissions, helpersHash: hashHelpers(data.helpers)
+        });
+        _grantApplyInstallation(owner);
+        _grantPspRoot();
+        psp.applyInstallation(address(dao), p);
+
+        vm.expectRevert(PluginSetupProcessor.PluginAlreadyInstalled.selector);
+        psp.applyInstallation(address(dao), p);
+    }
+
     /// **F32 closer**: `msg.sender == _dao` bypasses APPLY_INSTALLATION_PERMISSION.
     function test_applyInstallation_daoAsSelfBypassesPermissionCheck() public {
         (address plugin, PluginSetupProcessor.ApplyInstallationParams memory p) = _prepare();
@@ -432,6 +469,13 @@ contract PSPConstructorAndConstantsTest is PSPBaseTest {
         assertEq(address(psp.repoRegistry()), address(pluginRepoRegistry));
     }
 
+    function test_protocolVersion_returnsCurrent() public view {
+        uint8[3] memory v = psp.protocolVersion();
+        assertEq(v[0], 1);
+        assertEq(v[1], 4);
+        assertEq(v[2], 0);
+    }
+
     function test_permissionId_applyInstallationMatchesKeccak() public view {
         assertEq(psp.APPLY_INSTALLATION_PERMISSION_ID(), keccak256("APPLY_INSTALLATION_PERMISSION"));
     }

test/framework/plugin/setup/PSP.Update.t.sol

@@ -244,6 +244,52 @@ contract PSPApplyUpdateTest is PSPUpdateFixture {
         assertEq(currentAppliedId, expectedAppliedId);
     }
 
+    /// Twin of `PSP.Uninstallation.t.sol::test_applyUninstallation_otherPendingPrepsBecomeInapplicable`:
+    /// two distinct update preparations for the same plugin, applying one bumps
+    /// `pluginState.blockNumber`, invalidating the other (its `preparedBlock` is
+    /// now stale) → `SetupNotApplicable`.
+    function test_applyUpdate_otherPendingPrepsBecomeInapplicable() public {
+        (address plugin, address[] memory helpers) = _installV1();
+
+        // Prep A: default permission range → setup id A.
+        setupV2.mockPermissionIndexes(1, 2);
+        (bytes memory initA, IPluginSetup.PreparedSetupData memory dataA) =
+            psp.prepareUpdate(address(dao), _prepareUpdateParams(1, 2, plugin, helpers));
+
+        // Prep B: different permission range → distinct setup id B.
+        setupV2.mockPermissionIndexes(3, 4);
+        (bytes memory initB, IPluginSetup.PreparedSetupData memory dataB) =
+            psp.prepareUpdate(address(dao), _prepareUpdateParams(1, 2, plugin, helpers));
+
+        _grantApplyUpdate(owner);
+        _grantPspRoot();
+
+        // Apply A.
+        psp.applyUpdate(
+            address(dao),
+            PluginSetupProcessor.ApplyUpdateParams({
+                plugin: plugin,
+                pluginSetupRef: _ref(2),
+                initData: initA,
+                permissions: dataA.permissions,
+                helpersHash: hashHelpers(dataA.helpers)
+            })
+        );
+
+        // B is now inapplicable — its prepared block predates A's applied bump.
+        vm.expectRevert(); // SetupNotApplicable
+        psp.applyUpdate(
+            address(dao),
+            PluginSetupProcessor.ApplyUpdateParams({
+                plugin: plugin,
+                pluginSetupRef: _ref(2),
+                initData: initB,
+                permissions: dataB.permissions,
+                helpersHash: hashHelpers(dataB.helpers)
+            })
+        );
+    }
+
     /// V2's prepareUpdate returns `_mockHelpers(2)` — same length as V1's helpers
     /// (both 2 entries: address(0), address(1)).
     function _helpersV2() internal pure returns (address[] memory h) {