diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index a9974f5..58c8ab9 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -97,6 +97,10 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function createPolicy(address admin, PolicyType policyType) external returns (uint64 newPolicyId) { + // ZeroAddress precedes IncompatiblePolicyType (see interface natspec). `_create` + // re-checks zero-admin, but the hoisted copy pins the precedence. + if (admin == address(0)) revert ZeroAddress(); + if (_isCompositeType(policyType)) revert IncompatiblePolicyType(); newPolicyId = _create(admin, policyType); } @@ -106,15 +110,17 @@ contract MockPolicyRegistry is IPolicyRegistry { returns (uint64 newPolicyId) { // Match the Rust precompile's check precedence: - // validate_create_policy_inputs (zero-admin) → require_account_batch_size → + // validate_create_policy_inputs (zero-admin → composite-type) → require_account_batch_size → // create_policy_inner → write members - // Both checks are duplicated downstream (`_create` re-checks zero-admin for - // direct `createPolicy` callers, `_batchSetMembers` re-checks batch size for - // `updateAllowlist` / `updateBlocklist` callers). The hoisted entry-point - // copies ensure we revert before any `_create` mutation on the failing path - // AND pin the same revert-selector precedence Rust enforces (see Rust test - // `create_policy_with_accounts_zero_admin_precedes_batch_size_revert`). + // Both zero-admin and batch-size are duplicated downstream (`_create` re-checks + // zero-admin for direct `createPolicy` callers, `_batchSetMembers` re-checks batch + // size for `updateAllowlist` / `updateBlocklist` callers). The hoisted entry-point + // copies ensure we revert before any `_create` mutation on the failing path AND pin + // the same revert-selector precedence Rust enforces (see Rust test + // `create_policy_with_accounts_zero_admin_precedes_batch_size_revert`). A composite + // `policyType` is rejected here — this is a simple-policy constructor. if (admin == address(0)) revert ZeroAddress(); + if (_isCompositeType(policyType)) revert IncompatiblePolicyType(); if (accounts.length > MAX_BATCH_SIZE) revert BatchSizeTooLarge(MAX_BATCH_SIZE); newPolicyId = _create(admin, policyType); _batchSetMembers({policyId: newPolicyId, policyType: policyType, value: true, accounts: accounts}); @@ -126,7 +132,7 @@ contract MockPolicyRegistry is IPolicyRegistry { returns (uint64 newPolicyId) { if (admin == address(0)) revert ZeroAddress(); - if (policyType != PolicyType.UNION && policyType != PolicyType.INTERSECT) revert IncompatiblePolicyType(); + if (!_isCompositeType(policyType)) revert IncompatiblePolicyType(); if (childPolicyIds.length < MIN_CHILD_POLICIES || childPolicyIds.length > MAX_CHILD_POLICIES) { revert ChildPoliciesOutsideOfRange(MIN_CHILD_POLICIES, MAX_CHILD_POLICIES); } @@ -401,11 +407,15 @@ contract MockPolicyRegistry is IPolicyRegistry { } } + /// @dev True iff `policyType` is a composite gate (UNION or INTERSECT). + function _isCompositeType(PolicyType policyType) internal pure returns (bool) { + return policyType == PolicyType.UNION || policyType == PolicyType.INTERSECT; + } + /// @dev True iff `policyId`'s top byte encodes a composite gate (UNION or INTERSECT). /// Assumes a well-formed ID; composite children come from stored/created policies. function _isComposite(uint64 policyId) internal pure returns (bool) { - PolicyType policyType = _typeOf(policyId); - return policyType == PolicyType.UNION || policyType == PolicyType.INTERSECT; + return _isCompositeType(_typeOf(policyId)); } /// @dev True iff `policyId` is a built-in (ALWAYS_ALLOW / ALWAYS_BLOCK). diff --git a/test/unit/PolicyRegistry/createPolicy.t.sol b/test/unit/PolicyRegistry/createPolicy.t.sol index 45552cc..b9848ac 100644 --- a/test/unit/PolicyRegistry/createPolicy.t.sol +++ b/test/unit/PolicyRegistry/createPolicy.t.sol @@ -17,6 +17,18 @@ contract PolicyRegistryCreatePolicyTest is PolicyRegistryTest { policyRegistry.createPolicy(address(0), pt); } + /// @notice Verifies createPolicy reverts when given a composite policy type + /// @dev createPolicy is a simple-policy constructor; a UNION/INTERSECT gate reverts with + /// IncompatiblePolicyType() (composites are created via createCompositePolicy). + function test_createPolicy_revert_incompatiblePolicyType(address caller, address admin_, uint8 typeIdx) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); // UNION or INTERSECT + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + vm.prank(caller); + policyRegistry.createPolicy(admin_, pt); + } + /// @notice Verifies createPolicy assigns a fresh allowlist policy id /// @dev Paired slot: admin lane matches, exists bit set, ID top byte = ALLOWLIST. function test_createPolicy_success_allowlist(address caller, address admin_) public { diff --git a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol index da194cf..50cf583 100644 --- a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol +++ b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol @@ -17,6 +17,24 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { policyRegistry.createPolicyWithAccounts(address(0), IPolicyRegistry.PolicyType.ALLOWLIST, accounts); } + /// @notice Verifies createPolicyWithAccounts reverts when given a composite policy type + /// @dev createPolicyWithAccounts is a simple-policy constructor; a UNION/INTERSECT gate reverts + /// with IncompatiblePolicyType(). Fires after the zero-admin check and before the batch-size check. + function test_createPolicyWithAccounts_revert_incompatiblePolicyType( + address caller, + address admin_, + uint8 typeIdx, + address[] memory accounts + ) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + accounts = _boundAccounts(accounts); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); // UNION or INTERSECT + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + vm.prank(caller); + policyRegistry.createPolicyWithAccounts(admin_, pt, accounts); + } + /// @notice Verifies createPolicyWithAccounts seeds an allowlist policy with the provided members /// @dev Post-creation isAuthorized returns true for each seeded account. /// Paired slot assertion: each `members[id][account]` slot diff --git a/test/unit/PolicyRegistry/createPolicyWithAccounts_revertOrder.t.sol b/test/unit/PolicyRegistry/createPolicyWithAccounts_revertOrder.t.sol index b6a281f..9f29904 100644 --- a/test/unit/PolicyRegistry/createPolicyWithAccounts_revertOrder.t.sol +++ b/test/unit/PolicyRegistry/createPolicyWithAccounts_revertOrder.t.sol @@ -9,7 +9,8 @@ import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; /// /// @notice **Canonical order:** /// 1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress` -/// 2. BATCH-SIZE (`accounts.length > MAX_BATCH_SIZE`) → `BatchSizeTooLarge` +/// 2. INCOMPATIBLE-TYPE (`policyType` is a composite gate) → `IncompatiblePolicyType` +/// 3. BATCH-SIZE (`accounts.length > MAX_BATCH_SIZE`) → `BatchSizeTooLarge` /// /// Walks from all conditions broken to success, fixing one per step. contract PolicyRegistryCreatePolicyWithAccountsRevertOrderTest is PolicyRegistryTest { @@ -24,14 +25,22 @@ contract PolicyRegistryCreatePolicyWithAccountsRevertOrderTest is PolicyRegistry address[] memory tooMany = _makeAccounts(n); address[] memory valid = new address[](0); - // 1. ZERO-ADMIN: admin==address(0) AND batch oversized → ZeroAddress fires first. + // 1. ZERO-ADMIN: admin==address(0) AND composite type AND batch oversized → ZeroAddress fires first. vm.prank(caller); vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); - policyRegistry.createPolicyWithAccounts(address(0), pt, tooMany); + policyRegistry.createPolicyWithAccounts(address(0), _creatableCompositeType(typeIdx), tooMany); // Fix: use a non-zero admin. - // 2. BATCH-SIZE: valid admin, but accounts.length > MAX_BATCH_SIZE → BatchSizeTooLarge. + // 2. INCOMPATIBLE-TYPE: valid admin, but a composite gate AND batch oversized → + // IncompatiblePolicyType fires before the batch-size check. + vm.prank(caller); + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + policyRegistry.createPolicyWithAccounts(admin_, _creatableCompositeType(typeIdx), tooMany); + + // Fix: use a simple policy type. + + // 3. BATCH-SIZE: valid admin and simple type, 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); diff --git a/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol b/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol index 659b5c4..1d2ed4f 100644 --- a/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol +++ b/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol @@ -10,7 +10,8 @@ import {MockPolicyRegistryStorage} from "base-std-test/lib/mocks/MockPolicyRegis /// /// @notice **Canonical order:** /// 1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress` -/// 2. COUNTER-OVERFLOW (nextCounter == type(uint56).max) → `Panic(0x11)` +/// 2. INCOMPATIBLE-TYPE (`policyType` is a composite gate) → `IncompatiblePolicyType` +/// 3. COUNTER-OVERFLOW (nextCounter == type(uint56).max) → `Panic(0x11)` /// /// Walks from the first failing condition to success. /// @@ -35,7 +36,15 @@ contract PolicyRegistryCreatePolicyRevertOrderTest is PolicyRegistryTest { // Fix: use a non-zero admin. - // 2. COUNTER-OVERFLOW: nextCounter == type(uint56).max → Panic(0x11) + // 2. INCOMPATIBLE-TYPE: valid admin, but a composite gate → IncompatiblePolicyType + // (fires before any counter use, so it beats the seeded overflow). + vm.prank(caller); + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + policyRegistry.createPolicy(admin_, _creatableCompositeType(typeIdx)); + + // Fix: use a simple policy type. + + // 3. COUNTER-OVERFLOW: nextCounter == type(uint56).max → Panic(0x11) vm.prank(caller); vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11)); policyRegistry.createPolicy(admin_, pt);