test(PolicyRegistry): add revertOrder tests for all mutating functions

test/unit/PolicyRegistry/createPolicyWithAccounts_revertOrder.t.sol

@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol";
+
+import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol";
+
+/// @title Sequential revert-order test for `createPolicyWithAccounts`.
+///
+/// @notice **Canonical order:**
+///         1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress`
+///         2. BATCH-SIZE (`accounts.length > MAX_BATCH_SIZE`) → `BatchSizeTooLarge`
+///
+///         Walks from all conditions broken to success, fixing one per step.
+contract PolicyRegistryCreatePolicyWithAccountsRevertOrderTest is PolicyRegistryTest {
+    /// @notice Walks through every revert in canonical order, fixing one per step, ending at success.
+    function test_createPolicyWithAccounts_revertOrder(address caller, address admin_, uint8 typeIdx, uint8 overflow)
+        public
+    {
+        _assumeValidCaller(caller);
+        vm.assume(admin_ != address(0));
+        IPolicyRegistry.PolicyType pt = _creatablePolicyType(typeIdx);
+        uint256 n = MAX_BATCH_SIZE + 1 + (uint256(overflow) % 16);
+        address[] memory tooMany = _makeAccounts(n);
+        address[] memory valid = new address[](0);
+
+        // 1. ZERO-ADMIN: admin==address(0) AND batch oversized → ZeroAddress fires first.
+        vm.prank(caller);
+        vm.expectRevert(IPolicyRegistry.ZeroAddress.selector);
+        policyRegistry.createPolicyWithAccounts(address(0), pt, tooMany);
+
+        // Fix: use a non-zero admin.
+
+        // 2. BATCH-SIZE: valid admin, but accounts.length > MAX_BATCH_SIZE → BatchSizeTooLarge.
+        vm.prank(caller);
+        vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_BATCH_SIZE));
+        policyRegistry.createPolicyWithAccounts(admin_, pt, tooMany);
+
+        // Fix: use an empty accounts array.
+
+        // Success
+        vm.prank(caller);
+        policyRegistry.createPolicyWithAccounts(admin_, pt, valid);
+    }
+}

test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol";
+
+import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol";
+
+/// @title Sequential revert-order test for `createPolicy`.
+///
+/// @notice **Canonical order:**
+///         1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress`
+///
+///         Single revert condition; walks from that condition to success.
+contract PolicyRegistryCreatePolicyRevertOrderTest is PolicyRegistryTest {
+    /// @notice Walks through every revert in canonical order, fixing one per step, ending at success.
+    function test_createPolicy_revertOrder(address caller, address admin_, uint8 typeIdx) public {
+        _assumeValidCaller(caller);
+        vm.assume(admin_ != address(0));
+        IPolicyRegistry.PolicyType pt = _creatablePolicyType(typeIdx);
+
+        // 1. ZERO-ADMIN: admin == address(0) → ZeroAddress
+        vm.prank(caller);
+        vm.expectRevert(IPolicyRegistry.ZeroAddress.selector);
+        policyRegistry.createPolicy(address(0), pt);
+
+        // Fix: use a non-zero admin.
+
+        // Success
+        vm.prank(caller);
+        policyRegistry.createPolicy(admin_, pt);
+    }
+}

test/unit/PolicyRegistry/finalizeUpdateAdmin_revertOrder.t.sol

@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol";
+
+import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol";
+
+/// @title Sequential revert-order test for `finalizeUpdateAdmin`.
+///
+/// @notice **Canonical order:**
+///         1. POLICY-NOT-FOUND (`policies[policyId] == 0`) → `PolicyNotFound`
+///         2. NO-PENDING-ADMIN (`pendingAdmins[policyId] == address(0)`) → `NoPendingAdmin`
+///         3. UNAUTHORIZED (`pendingAdmins[policyId] != msg.sender`) → `Unauthorized`
+///
+///         Walks from all conditions broken to success, fixing one per step.
+contract PolicyRegistryFinalizeUpdateAdminRevertOrderTest is PolicyRegistryTest {
+    /// @notice Walks through every revert in canonical order, fixing one per step, ending at success.
+    function test_finalizeUpdateAdmin_revertOrder() public {
+        // ghostId is a well-formed policyId that has never been created.
+        uint64 ghostId = _wellFormedUncreatedPolicyId(type(uint64).max);
+
+        // 1. POLICY-NOT-FOUND: policyId has never been created AND no pending admin is
+        //    staged (NoPendingAdmin and Unauthorized would also apply once a policy exists).
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector);
+        policyRegistry.finalizeUpdateAdmin(ghostId);
+
+        // Fix: create an allowlist policy with alice as admin.
+        uint64 policyId = policyRegistry.createPolicy(alice, IPolicyRegistry.PolicyType.ALLOWLIST);
+
+        // 2. NO-PENDING-ADMIN: policy exists but no pending admin has been staged.
+        //    (Unauthorized would also fire if NoPendingAdmin were absent, since
+        //    pendingAdmins[policyId] == address(0) != attacker.)
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.NoPendingAdmin.selector);
+        policyRegistry.finalizeUpdateAdmin(policyId);
+
+        // Fix: stage bob as the pending admin.
+        vm.prank(alice);
+        policyRegistry.stageUpdateAdmin(policyId, bob);
+
+        // 3. UNAUTHORIZED: bob is the staged pending admin, but attacker is calling.
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.Unauthorized.selector);
+        policyRegistry.finalizeUpdateAdmin(policyId);
+
+        // Fix: call as bob (the staged pending admin).
+
+        // Success
+        vm.prank(bob);
+        policyRegistry.finalizeUpdateAdmin(policyId);
+    }
+}

test/unit/PolicyRegistry/renounceAdmin_revertOrder.t.sol

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol";
+
+import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol";
+
+/// @title Sequential revert-order test for `renounceAdmin`.
+///
+/// @notice **Canonical order:**
+///         1. POLICY-NOT-FOUND (`policies[policyId] == 0`) → `PolicyNotFound`
+///         2. UNAUTHORIZED (`_decodeAdmin(packed) != msg.sender`) → `Unauthorized`
+///
+///         Walks from all conditions broken to success, fixing one per step.
+contract PolicyRegistryRenounceAdminRevertOrderTest is PolicyRegistryTest {
+    /// @notice Walks through every revert in canonical order, fixing one per step, ending at success.
+    function test_renounceAdmin_revertOrder() public {
+        // ghostId is a well-formed policyId that has never been created.
+        uint64 ghostId = _wellFormedUncreatedPolicyId(type(uint64).max);
+
+        // 1. POLICY-NOT-FOUND: policyId has never been created AND attacker is not the
+        //    policy admin (Unauthorized would also apply if the policy existed).
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector);
+        policyRegistry.renounceAdmin(ghostId);
+
+        // Fix: create an allowlist policy with alice as admin.
+        uint64 policyId = policyRegistry.createPolicy(alice, IPolicyRegistry.PolicyType.ALLOWLIST);
+
+        // 2. UNAUTHORIZED: attacker is not the policy admin (alice).
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.Unauthorized.selector);
+        policyRegistry.renounceAdmin(policyId);
+
+        // Fix: call as alice (the policy admin).
+
+        // Success
+        vm.prank(alice);
+        policyRegistry.renounceAdmin(policyId);
+    }
+}

test/unit/PolicyRegistry/stageUpdateAdmin_revertOrder.t.sol

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol";
+
+import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol";
+
+/// @title Sequential revert-order test for `stageUpdateAdmin`.
+///
+/// @notice **Canonical order:**
+///         1. POLICY-NOT-FOUND (`policies[policyId] == 0`) → `PolicyNotFound`
+///         2. UNAUTHORIZED (`_decodeAdmin(packed) != msg.sender`) → `Unauthorized`
+///
+///         Walks from all conditions broken to success, fixing one per step.
+contract PolicyRegistryStageUpdateAdminRevertOrderTest is PolicyRegistryTest {
+    /// @notice Walks through every revert in canonical order, fixing one per step, ending at success.
+    function test_stageUpdateAdmin_revertOrder(address newAdmin) public {
+        // ghostId is a well-formed policyId that has never been created.
+        uint64 ghostId = _wellFormedUncreatedPolicyId(type(uint64).max);
+
+        // 1. POLICY-NOT-FOUND: policyId has never been created AND attacker is not the
+        //    policy admin (Unauthorized would also apply if the policy existed).
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector);
+        policyRegistry.stageUpdateAdmin(ghostId, newAdmin);
+
+        // Fix: create an allowlist policy with alice as admin.
+        uint64 policyId = policyRegistry.createPolicy(alice, IPolicyRegistry.PolicyType.ALLOWLIST);
+
+        // 2. UNAUTHORIZED: attacker is not the policy admin (alice).
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.Unauthorized.selector);
+        policyRegistry.stageUpdateAdmin(policyId, newAdmin);
+
+        // Fix: call as alice (the policy admin).
+
+        // Success
+        vm.prank(alice);
+        policyRegistry.stageUpdateAdmin(policyId, newAdmin);
+    }
+}

test/unit/PolicyRegistry/updateAllowlist_revertOrder.t.sol

@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol";
+
+import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol";
+
+/// @title Sequential revert-order test for `updateAllowlist`.
+///
+/// @notice **Canonical order:**
+///         1. POLICY-NOT-FOUND (`policies[policyId] == 0`) → `PolicyNotFound`
+///         2. INCOMPATIBLE-TYPE (`_typeOf(policyId) != ALLOWLIST`) → `IncompatiblePolicyType`
+///         3. UNAUTHORIZED (`_decodeAdmin(packed) != msg.sender`) → `Unauthorized`
+///         4. BATCH-SIZE (inside `_batchSetMembers`) → `BatchSizeTooLarge`
+///
+///         Walks from all conditions broken to success, fixing one per step.
+contract PolicyRegistryUpdateAllowlistRevertOrderTest is PolicyRegistryTest {
+    /// @notice Walks through every revert in canonical order, fixing one per step, ending at success.
+    function test_updateAllowlist_revertOrder(uint8 overflow) public {
+        uint256 n = MAX_BATCH_SIZE + 1 + (uint256(overflow) % 16);
+        address[] memory tooMany = _makeAccounts(n);
+        address[] memory empty = new address[](0);
+        // ghostId is a well-formed policyId that has never been created.
+        uint64 ghostId = _wellFormedUncreatedPolicyId(type(uint64).max);
+
+        // 1. POLICY-NOT-FOUND: policyId has never been created AND all later conditions
+        //    would also apply (wrong type, unauthorized caller, oversized batch).
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector);
+        policyRegistry.updateAllowlist(ghostId, true, tooMany);
+
+        // Fix: create a BLOCKLIST policy with alice as admin (exists, but wrong type for updateAllowlist).
+        uint64 blocklistId = _createBlocklist(admin, alice);
+
+        // 2. INCOMPATIBLE-TYPE: policy exists as BLOCKLIST; updateAllowlist requires ALLOWLIST.
+        //    (Unauthorized and BatchSizeTooLarge would also apply.)
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector);
+        policyRegistry.updateAllowlist(blocklistId, true, tooMany);
+
+        // Fix: create an ALLOWLIST policy with alice as admin.
+        uint64 policyId = _createAllowlist(admin, alice);
+
+        // 3. UNAUTHORIZED: policy is ALLOWLIST but attacker is not the policy admin (alice).
+        //    (BatchSizeTooLarge would also apply.)
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.Unauthorized.selector);
+        policyRegistry.updateAllowlist(policyId, true, tooMany);
+
+        // Fix: call as alice (the policy admin).
+
+        // 4. BATCH-SIZE: alice is the admin, but accounts.length > MAX_BATCH_SIZE.
+        vm.prank(alice);
+        vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_BATCH_SIZE));
+        policyRegistry.updateAllowlist(policyId, true, tooMany);
+
+        // Fix: use an empty accounts array.
+
+        // Success
+        vm.prank(alice);
+        policyRegistry.updateAllowlist(policyId, true, empty);
+    }
+}

test/unit/PolicyRegistry/updateBlocklist_revertOrder.t.sol

@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol";
+
+import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol";
+
+/// @title Sequential revert-order test for `updateBlocklist`.
+///
+/// @notice **Canonical order:**
+///         1. POLICY-NOT-FOUND (`policies[policyId] == 0`) → `PolicyNotFound`
+///         2. INCOMPATIBLE-TYPE (`_typeOf(policyId) != BLOCKLIST`) → `IncompatiblePolicyType`
+///         3. UNAUTHORIZED (`_decodeAdmin(packed) != msg.sender`) → `Unauthorized`
+///         4. BATCH-SIZE (inside `_batchSetMembers`) → `BatchSizeTooLarge`
+///
+///         Walks from all conditions broken to success, fixing one per step.
+///         Mirror of `updateAllowlist_revertOrder.t.sol` with ALLOWLIST and BLOCKLIST roles swapped.
+contract PolicyRegistryUpdateBlocklistRevertOrderTest is PolicyRegistryTest {
+    /// @notice Walks through every revert in canonical order, fixing one per step, ending at success.
+    function test_updateBlocklist_revertOrder(uint8 overflow) public {
+        uint256 n = MAX_BATCH_SIZE + 1 + (uint256(overflow) % 16);
+        address[] memory tooMany = _makeAccounts(n);
+        address[] memory empty = new address[](0);
+        // ghostId is a well-formed policyId that has never been created.
+        uint64 ghostId = _wellFormedUncreatedPolicyId(type(uint64).max);
+
+        // 1. POLICY-NOT-FOUND: policyId has never been created AND all later conditions
+        //    would also apply (wrong type, unauthorized caller, oversized batch).
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector);
+        policyRegistry.updateBlocklist(ghostId, true, tooMany);
+
+        // Fix: create an ALLOWLIST policy with alice as admin (exists, but wrong type for updateBlocklist).
+        uint64 allowlistId = _createAllowlist(admin, alice);
+
+        // 2. INCOMPATIBLE-TYPE: policy exists as ALLOWLIST; updateBlocklist requires BLOCKLIST.
+        //    (Unauthorized and BatchSizeTooLarge would also apply.)
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector);
+        policyRegistry.updateBlocklist(allowlistId, true, tooMany);
+
+        // Fix: create a BLOCKLIST policy with alice as admin.
+        uint64 policyId = _createBlocklist(admin, alice);
+
+        // 3. UNAUTHORIZED: policy is BLOCKLIST but attacker is not the policy admin (alice).
+        //    (BatchSizeTooLarge would also apply.)
+        vm.prank(attacker);
+        vm.expectRevert(IPolicyRegistry.Unauthorized.selector);
+        policyRegistry.updateBlocklist(policyId, true, tooMany);
+
+        // Fix: call as alice (the policy admin).
+
+        // 4. BATCH-SIZE: alice is the admin, but accounts.length > MAX_BATCH_SIZE.
+        vm.prank(alice);
+        vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_BATCH_SIZE));
+        policyRegistry.updateBlocklist(policyId, true, tooMany);
+
+        // Fix: use an empty accounts array.
+
+        // Success
+        vm.prank(alice);
+        policyRegistry.updateBlocklist(policyId, true, empty);
+    }
+}