From d45faea7c5f9d3b29dbd93104e38c70bd86238d5 Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Fri, 24 Jul 2026 12:27:29 -0400 Subject: [PATCH 1/5] feat: create composite polocies --- test/lib/PolicyRegistryTest.sol | 62 +++- .../createCompositePolicy.t.sol | 293 ++++++++++++++++++ .../createCompositePolicy_revertOrder.t.sol | 82 +++++ test/unit/PolicyRegistry/isAuthorized.t.sol | 92 ++++++ .../unit/PolicyRegistry/updateComposite.t.sol | 170 ++++++++++ .../updateComposite_revertOrder.t.sol | 98 ++++++ 6 files changed, 795 insertions(+), 2 deletions(-) create mode 100644 test/unit/PolicyRegistry/createCompositePolicy.t.sol create mode 100644 test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol create mode 100644 test/unit/PolicyRegistry/updateComposite.t.sol create mode 100644 test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol diff --git a/test/lib/PolicyRegistryTest.sol b/test/lib/PolicyRegistryTest.sol index fe0fa048..da900204 100644 --- a/test/lib/PolicyRegistryTest.sol +++ b/test/lib/PolicyRegistryTest.sol @@ -43,14 +43,20 @@ contract PolicyRegistryTest is BaseTest { // ============================================================ // POLICY-TYPE FUZZ HELPERS // ============================================================ - // The `PolicyType` enum has two values (BLOCKLIST = 0, ALLOWLIST = 1), - // both creatable. The helper picks one from a fuzz seed. + // The `PolicyType` enum has two SIMPLE values (BLOCKLIST = 0, + // ALLOWLIST = 1) and two COMPOSITE gates (UNION = 2, INTERSECT = 3). + // The helpers below pick one from a fuzz seed. /// @notice Maps a fuzz seed to ALLOWLIST or BLOCKLIST. function _creatablePolicyType(uint8 idx) internal pure returns (IPolicyRegistry.PolicyType) { return idx % 2 == 0 ? IPolicyRegistry.PolicyType.ALLOWLIST : IPolicyRegistry.PolicyType.BLOCKLIST; } + /// @notice Maps a fuzz seed to a composite gate: UNION or INTERSECT. + function _creatableCompositeType(uint8 idx) internal pure returns (IPolicyRegistry.PolicyType) { + return idx % 2 == 0 ? IPolicyRegistry.PolicyType.UNION : IPolicyRegistry.PolicyType.INTERSECT; + } + /// @notice Predict the ID the next `createPolicy(_, policyType)` would assign. /// @dev Reads `nextCounter` directly via `vm.load`. When the registry /// has not yet been initialized (counter == 0), the next @@ -107,4 +113,56 @@ contract PolicyRegistryTest is BaseTest { accounts[i] = address(uint160(0x1000 + i)); } } + + // ============================================================ + // COMPOSITE-POLICY HELPERS + // ============================================================ + + /// @notice Maximum number of child policies a composite may reference. + /// @dev Mirrors the registry's composite child-policy cap. Distinct + /// from `MAX_BATCH_SIZE` (64), which caps account membership + /// batches; a composite created or updated with more than + /// `MAX_CHILD_POLICIES` children reverts with + /// `BatchSizeTooLarge(MAX_CHILD_POLICIES)`. Kept as a + /// test-side literal so fork tests against the real precompile + /// use the same compile-time constant. + uint256 internal constant MAX_CHILD_POLICIES = 4; + + /// @notice Create `n` simple ALLOWLIST policies (admin = `admin`) and + /// return their IDs. Used to seed valid child sets for + /// composite creation / update tests. + function _makeSimpleChildren(uint256 n) internal returns (uint64[] memory childPolicyIds) { + childPolicyIds = new uint64[](n); + for (uint256 i = 0; i < n; ++i) { + childPolicyIds[i] = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + } + } + + /// @notice Create a composite policy with explicit caller, admin, gate + /// type, and child set. + function _createComposite( + address caller, + address policyAdmin, + IPolicyRegistry.PolicyType policyType, + uint64[] memory childPolicyIds + ) internal returns (uint64 policyId) { + vm.prank(caller); + policyId = policyRegistry.createCompositePolicy(policyAdmin, policyType, childPolicyIds); + } + + /// @notice Create a composite policy as the default admin over a fresh + /// set of `n` simple ALLOWLIST children. Returns the composite + /// ID; use `_makeSimpleChildren` directly when the child IDs + /// are needed at the call site. + function _createComposite(IPolicyRegistry.PolicyType policyType, uint256 n) internal returns (uint64 policyId) { + uint64[] memory childPolicyIds = _makeSimpleChildren(n); + policyId = policyRegistry.createCompositePolicy(admin, policyType, childPolicyIds); + } + + /// @notice Pack two policy IDs into a length-2 `uint64[]`. + function _childIds(uint64 a, uint64 b) internal pure returns (uint64[] memory ids) { + ids = new uint64[](2); + ids[0] = a; + ids[1] = b; + } } diff --git a/test/unit/PolicyRegistry/createCompositePolicy.t.sol b/test/unit/PolicyRegistry/createCompositePolicy.t.sol new file mode 100644 index 00000000..82e31201 --- /dev/null +++ b/test/unit/PolicyRegistry/createCompositePolicy.t.sol @@ -0,0 +1,293 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IPolicyRegistry} from "base-std/interfaces/IPolicyRegistry.sol"; + +import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; +import {MockPolicyRegistryStorage} from "base-std-test/lib/mocks/MockPolicyRegistryStorage.sol"; + +contract PolicyRegistryCreateCompositePolicyTest is PolicyRegistryTest { + // ============================================================ + // REVERTS + // ============================================================ + + /// @notice Verifies createCompositePolicy reverts when admin is the zero address + /// @dev Required-field guard; checks ZeroAddress() error. Takes precedence over every later check. + function test_createCompositePolicy_revert_zeroAdmin(address caller, uint8 typeIdx) public { + _assumeValidCaller(caller); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + uint64[] memory children = _makeSimpleChildren(2); + vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); + vm.prank(caller); + policyRegistry.createCompositePolicy(address(0), pt, children); + } + + /// @notice Verifies createCompositePolicy reverts when policyType is a simple gate + /// @dev Type guard: composites must be UNION or INTERSECT; a simple ALLOWLIST/BLOCKLIST + /// type reverts with IncompatiblePolicyType(). + function test_createCompositePolicy_revert_incompatiblePolicyType(address caller, address admin_, uint8 typeIdx) + public + { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatablePolicyType(typeIdx); // ALLOWLIST or BLOCKLIST + uint64[] memory children = _makeSimpleChildren(2); + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, children); + } + + /// @notice Verifies createCompositePolicy reverts when given fewer than two children + /// @dev A composite must reference at least two simple policies; checks TooFewChildPolicies(). + /// Exercises both the empty (0) and single-child (1) cases. + function test_createCompositePolicy_revert_tooFewChildPolicies(address caller, address admin_, uint8 typeIdx) + public + { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + + // 0 children. + uint64[] memory none = new uint64[](0); + vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, none); + + // 1 child (a real, existing simple policy) — still below the minimum. + uint64[] memory one = _makeSimpleChildren(1); + vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, one); + } + + /// @notice Verifies createCompositePolicy reverts when given more than the child cap + /// @dev Composite child-policy cap is MAX_CHILD_POLICIES (4), distinct from the 64-account + /// membership limit; checks BatchSizeTooLarge(MAX_CHILD_POLICIES). All children are valid + /// simple policies, so only the size condition is broken. + function test_createCompositePolicy_revert_batchSizeTooLarge( + address caller, + address admin_, + uint8 typeIdx, + uint8 overflow + ) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + uint256 n = MAX_CHILD_POLICIES + 1 + (uint256(overflow) % 4); // 5..8 + uint64[] memory tooMany = _makeSimpleChildren(n); + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, tooMany); + } + + /// @notice Verifies createCompositePolicy reverts when a child policy does not exist + /// @dev Checks PolicyNotFound(). Both children are well-formed but never-created IDs, so the + /// existence check fails before the simple-vs-composite check. + function test_createCompositePolicy_revert_policyNotFound( + address caller, + address admin_, + uint8 typeIdx, + uint64 seed + ) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + uint64[] memory children = new uint64[](2); + children[0] = _wellFormedUncreatedPolicyId(seed); + children[1] = _wellFormedUncreatedPolicyId(seed ^ 0xdead); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, children); + } + + /// @notice Verifies createCompositePolicy reverts when a child is itself a composite + /// @dev Child policies must be simple; a composite child reverts with + /// InvalidChildPolicy(childPolicyId) carrying the offending ID. + function test_createCompositePolicy_revert_invalidChildPolicy(address caller, address admin_, uint8 typeIdx) + public + { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + + // A composite policy to use as an (invalid) child. + uint64 compositeChild = _createComposite(IPolicyRegistry.PolicyType.UNION, 2); + // A valid simple sibling so the child set has the required size and the composite child + // is the sole offending entry. + uint64 simpleChild = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + + uint64[] memory children = _childIds(simpleChild, compositeChild); + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, compositeChild)); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, children); + } + + /// @notice Verifies createCompositePolicy panics with arithmetic overflow at the counter max + /// @dev Slot-writes nextCounter to type(uint56).max after seeding children to avoid iterating + /// 2^56 times. Mock-only: vm.store cannot write to native precompile addresses. + /// Matches the Rust precompile which reverts with Panic(UnderOverflow) = Panic(0x11). + function test_createCompositePolicy_revert_counterOverflow(address caller, address admin_, uint8 typeIdx) public { + vm.skip(livePrecompiles); + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + + // Seed valid children first (advances the counter past the built-ins), then force overflow. + uint64[] memory children = _makeSimpleChildren(2); + vm.store( + address(policyRegistry), MockPolicyRegistryStorage.nextCounterSlot(), bytes32(uint256(type(uint56).max)) + ); + + vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11)); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, children); + } + + // ============================================================ + // SUCCESS + // ============================================================ + + /// @notice Verifies createCompositePolicy assigns a fresh UNION policy id + /// @dev Paired slot: admin lane matches, exists bit set, ID top byte = UNION. + function test_createCompositePolicy_success_union(address caller, address admin_) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + uint64[] memory children = _makeSimpleChildren(2); + + uint64 predicted = _predictNextPolicyId(IPolicyRegistry.PolicyType.UNION); + vm.prank(caller); + uint64 policyId = policyRegistry.createCompositePolicy(admin_, IPolicyRegistry.PolicyType.UNION, children); + assertEq(policyId, predicted); + assertTrue(policyRegistry.policyExists(policyId)); + assertEq(policyRegistry.policyAdmin(policyId), admin_); + + uint256 packed = uint256(vm.load(address(policyRegistry), MockPolicyRegistryStorage.policySlot(policyId))); + assertEq( + MockPolicyRegistryStorage.policyAdminFromPacked(packed), + admin_, + "policies[id] slot admin must reflect the composite admin" + ); + assertTrue(MockPolicyRegistryStorage.policyExistsFromPacked(packed), "policies[id] slot exists bit must be set"); + assertEq( + MockPolicyRegistryStorage.policyTypeFromId(policyId), + uint8(IPolicyRegistry.PolicyType.UNION), + "policy ID high byte must encode UNION" + ); + } + + /// @notice Verifies createCompositePolicy assigns a fresh INTERSECT policy id + /// @dev Paired slot: admin lane matches, exists bit set, ID top byte = INTERSECT. + function test_createCompositePolicy_success_intersect(address caller, address admin_) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + uint64[] memory children = _makeSimpleChildren(2); + + uint64 predicted = _predictNextPolicyId(IPolicyRegistry.PolicyType.INTERSECT); + vm.prank(caller); + uint64 policyId = policyRegistry.createCompositePolicy(admin_, IPolicyRegistry.PolicyType.INTERSECT, children); + assertEq(policyId, predicted); + assertTrue(policyRegistry.policyExists(policyId)); + assertEq(policyRegistry.policyAdmin(policyId), admin_); + + uint256 packed = uint256(vm.load(address(policyRegistry), MockPolicyRegistryStorage.policySlot(policyId))); + assertEq( + MockPolicyRegistryStorage.policyAdminFromPacked(packed), + admin_, + "policies[id] slot admin must reflect the composite admin" + ); + assertTrue(MockPolicyRegistryStorage.policyExistsFromPacked(packed), "policies[id] slot exists bit must be set"); + assertEq( + MockPolicyRegistryStorage.policyTypeFromId(policyId), + uint8(IPolicyRegistry.PolicyType.INTERSECT), + "policy ID high byte must encode INTERSECT" + ); + } + + /// @notice Verifies createCompositePolicy accepts a child set exactly at the cap + /// @dev Boundary check: MAX_CHILD_POLICIES (4) children is inclusive. + function test_createCompositePolicy_success_atMaxChildren(address caller, address admin_, uint8 typeIdx) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + uint64[] memory children = _makeSimpleChildren(MAX_CHILD_POLICIES); + vm.prank(caller); + uint64 policyId = policyRegistry.createCompositePolicy(admin_, pt, children); + assertTrue(policyRegistry.policyExists(policyId)); + } + + /// @notice Verifies sequential composite creates each consume a fresh id from the global counter + /// @dev Each create's ID matches the prediction taken from the live counter immediately before it. + function test_createCompositePolicy_success_advancesNextPolicyId(address admin_, uint8 typeIdxA, uint8 typeIdxB) + public + { + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType ptA = _creatableCompositeType(typeIdxA); + IPolicyRegistry.PolicyType ptB = _creatableCompositeType(typeIdxB); + + uint64[] memory childrenA = _makeSimpleChildren(2); + uint64 predictedA = _predictNextPolicyId(ptA); + uint64 idA = policyRegistry.createCompositePolicy(admin_, ptA, childrenA); + assertEq(idA, predictedA); + + uint64[] memory childrenB = _makeSimpleChildren(2); + uint64 predictedB = _predictNextPolicyId(ptB); + uint64 idB = policyRegistry.createCompositePolicy(admin_, ptB, childrenB); + assertEq(idB, predictedB); + + assertTrue(idA != idB); + } + + /// @notice Verifies createCompositePolicy emits PolicyCreated with the correct args + function test_createCompositePolicy_success_emitsPolicyCreated(address caller, address admin_, uint8 typeIdx) + public + { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + uint64[] memory children = _makeSimpleChildren(2); + + uint64 expectedId = _predictNextPolicyId(pt); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.PolicyCreated(expectedId, caller, pt); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, children); + } + + /// @notice Verifies createCompositePolicy emits PolicyAdminUpdated(previousAdmin = 0) on initial assignment + function test_createCompositePolicy_success_emitsInitialPolicyAdminUpdated( + address caller, + address admin_, + uint8 typeIdx + ) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + uint64[] memory children = _makeSimpleChildren(2); + + uint64 expectedId = _predictNextPolicyId(pt); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.PolicyAdminUpdated(expectedId, address(0), admin_); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, children); + } + + /// @notice Verifies createCompositePolicy emits CompositePolicyUpdated carrying the full child set + /// @dev The event is emitted on creation as well as on every subsequent update; the payload is the + /// complete post-create child set. + function test_createCompositePolicy_success_emitsCompositePolicyUpdated( + address caller, + address admin_, + uint8 typeIdx + ) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + uint64[] memory children = _makeSimpleChildren(2); + + uint64 expectedId = _predictNextPolicyId(pt); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.CompositePolicyUpdated(expectedId, caller, children); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, children); + } +} diff --git a/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol b/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol new file mode 100644 index 00000000..c2183d46 --- /dev/null +++ b/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IPolicyRegistry} from "base-std/interfaces/IPolicyRegistry.sol"; + +import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; + +/// @title Sequential revert-order test for `createCompositePolicy`. +/// +/// @notice **Canonical order:** +/// 1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress` +/// 2. INCOMPATIBLE-TYPE (`policyType` not UNION/INTERSECT) → `IncompatiblePolicyType` +/// 3. TOO-FEW-CHILDREN (`childPolicyIds.length < 2`) → `TooFewChildPolicies` +/// 4. BATCH-SIZE (`childPolicyIds.length > MAX_CHILD_POLICIES`) → `BatchSizeTooLarge(4)` +/// 5. CHILD-NOT-FOUND (a child does not exist) → `PolicyNotFound` +/// 6. INVALID-CHILD (a child is itself a composite) → `InvalidChildPolicy` +/// +/// Walks from all conditions broken to success, fixing one per step. ZeroAddress-first +/// precedence mirrors `createPolicy` / `createPolicyWithAccounts`. +contract PolicyRegistryCreateCompositePolicyRevertOrderTest is PolicyRegistryTest { + /// @notice Walks through every revert in canonical order, fixing one per step, ending at success. + function test_createCompositePolicy_revertOrder(uint8 typeIdx) public { + IPolicyRegistry.PolicyType composite = _creatableCompositeType(typeIdx); + + // Fixtures. + uint64 validSimpleA = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 validSimpleB = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + uint64 compositeChild = _createComposite(IPolicyRegistry.PolicyType.UNION, 2); + + // Ghost (well-formed, never-created) child IDs at counters far above anything created. + uint64 neverCreatedWellFormedPolicy = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(2_000_000); + uint64[] memory tooManyNeverCreatedWellFormedPolicyList = new uint64[](MAX_CHILD_POLICIES + 1); // 5, all nonexistent + for (uint256 i = 0; i < tooManyNeverCreatedWellFormedPolicyList.length; ++i) { + tooManyNeverCreatedWellFormedPolicyList[i] = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(1_000_000 + i); + } + + // 1. ZERO-ADMIN: admin==0 AND type simple AND oversized child set → ZeroAddress fires first. + vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); + policyRegistry.createCompositePolicy(address(0), IPolicyRegistry.PolicyType.ALLOWLIST, tooManyNeverCreatedWellFormedPolicyList); + + // Fix: use a non-zero admin. + + // 2. INCOMPATIBLE-TYPE: valid admin, but simple type (ALLOWLIST) AND oversized child set + // → IncompatiblePolicyType fires before the count/size checks. + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST, tooManyNeverCreatedWellFormedPolicyList); + + // Fix: use a composite gate. + + // 3. TOO-FEW-CHILDREN: composite type, but a single (nonexistent) child → TooFewChildPolicies + // fires before the existence check. + uint64[] memory one = new uint64[](1); + one[0] = neverCreatedWellFormedPolicy; + vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + policyRegistry.createCompositePolicy(admin, composite, one); + + // Fix: supply at least two children. + + // 4. BATCH-SIZE: composite type, but > MAX_CHILD_POLICIES (all nonexistent) → BatchSizeTooLarge + // fires before the existence check. + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + policyRegistry.createCompositePolicy(admin, composite, tooManyNeverCreatedWellFormedPolicyList); + + // Fix: bring the child set within the cap. + + // 5. CHILD-NOT-FOUND: in-range child set, but one child never existed → PolicyNotFound + // fires before the simple-vs-composite check. + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + policyRegistry.createCompositePolicy(admin, composite, _childIds(tooManyNeverCreatedWellFormedPolicyList[0], tooManyNeverCreatedWellFormedPolicyList[1])); + + // Fix: replace the ghost with an existing simple policy. + + // 6. INVALID-CHILD: all children exist, but one is itself a composite → InvalidChildPolicy. + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, compositeChild)); + policyRegistry.createCompositePolicy(admin, composite, _childIds(validSimpleA, compositeChild)); + + // Fix: replace the composite child with a simple policy. + + // Success. + policyRegistry.createCompositePolicy(admin, composite, _childIds(validSimpleA, validSimpleB)); + } +} diff --git a/test/unit/PolicyRegistry/isAuthorized.t.sol b/test/unit/PolicyRegistry/isAuthorized.t.sol index 5f475556..0d037624 100644 --- a/test/unit/PolicyRegistry/isAuthorized.t.sol +++ b/test/unit/PolicyRegistry/isAuthorized.t.sol @@ -79,4 +79,96 @@ contract PolicyRegistryIsAuthorizedTest is PolicyRegistryTest { uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); assertTrue(policyRegistry.isAuthorized(policyId, account)); } + + // ============================================================ + // COMPOSITE POLICY SEMANTICS + // ============================================================ + // UNION is OR: authorized if ANY child authorizes. + // INTERSECT is AND: authorized only if EVERY child authorizes. + // Composites reference their children live — evaluation reads current + // child membership, not a snapshot taken at composite-creation time. + + /// @notice Sets membership of a single `account` on a simple policy, as `admin`. + function _setAllowlistMember(uint64 policyId, address account, bool member) private { + address[] memory accounts = new address[](1); + accounts[0] = account; + vm.prank(admin); + policyRegistry.updateAllowlist(policyId, member, accounts); + } + + /// @notice Verifies UNION authorizes an account that is a member of any single child + /// @dev OR semantics: `account` is on allowlist A only, yet the UNION authorizes it. + function test_isAuthorized_success_union_anyChildAuthorizes(address account) public { + uint64 childA = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 childB = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 composite = + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.UNION, _childIds(childA, childB)); + _setAllowlistMember(childA, account, true); + assertTrue(policyRegistry.isAuthorized(composite, account)); + } + + /// @notice Verifies UNION denies an account that no child authorizes + /// @dev OR semantics: `account` is on neither empty allowlist, so the UNION denies it. + function test_isAuthorized_success_union_noChildAuthorizes(address account) public { + uint64 childA = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 childB = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 composite = + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.UNION, _childIds(childA, childB)); + assertFalse(policyRegistry.isAuthorized(composite, account)); + } + + /// @notice Verifies INTERSECT authorizes an account only when every child authorizes it + /// @dev AND semantics: `account` is on both allowlists, so the INTERSECT authorizes it. + function test_isAuthorized_success_intersect_allChildrenAuthorize(address account) public { + uint64 childA = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 childB = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 composite = policyRegistry.createCompositePolicy( + admin, IPolicyRegistry.PolicyType.INTERSECT, _childIds(childA, childB) + ); + _setAllowlistMember(childA, account, true); + _setAllowlistMember(childB, account, true); + assertTrue(policyRegistry.isAuthorized(composite, account)); + } + + /// @notice Verifies INTERSECT denies an account when even one child denies it + /// @dev AND semantics: `account` is on allowlist A only, so the INTERSECT denies it. + function test_isAuthorized_success_intersect_oneChildDenies(address account) public { + uint64 childA = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 childB = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 composite = policyRegistry.createCompositePolicy( + admin, IPolicyRegistry.PolicyType.INTERSECT, _childIds(childA, childB) + ); + _setAllowlistMember(childA, account, true); + assertFalse(policyRegistry.isAuthorized(composite, account)); + } + + /// @notice Verifies composite gates combine mixed ALLOWLIST + BLOCKLIST children correctly + /// @dev With `account` absent from both children, the allowlist denies and the blocklist allows: + /// UNION → true (OR), INTERSECT → false (AND). Exercises both simple semantics under a gate. + function test_isAuthorized_success_composite_mixedChildTypes(address account) public { + uint64 allow = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 block_ = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + uint64 union = + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.UNION, _childIds(allow, block_)); + uint64 intersect = + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.INTERSECT, _childIds(allow, block_)); + + // account absent: allowlist denies, blocklist allows. + assertTrue(policyRegistry.isAuthorized(union, account), "UNION(deny, allow) must be true"); + assertFalse(policyRegistry.isAuthorized(intersect, account), "INTERSECT(deny, allow) must be false"); + } + + /// @notice Verifies composite evaluation tracks live child membership changes + /// @dev A UNION over two empty allowlists denies `account`; adding `account` to a child + /// flips the composite to authorize — composites reference children, not snapshots. + function test_isAuthorized_success_composite_reflectsChildMembershipChange(address account) public { + uint64 childA = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 childB = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 composite = + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.UNION, _childIds(childA, childB)); + + assertFalse(policyRegistry.isAuthorized(composite, account)); + _setAllowlistMember(childB, account, true); + assertTrue(policyRegistry.isAuthorized(composite, account)); + } } diff --git a/test/unit/PolicyRegistry/updateComposite.t.sol b/test/unit/PolicyRegistry/updateComposite.t.sol new file mode 100644 index 00000000..9c27dec6 --- /dev/null +++ b/test/unit/PolicyRegistry/updateComposite.t.sol @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IPolicyRegistry} from "base-std/interfaces/IPolicyRegistry.sol"; + +import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; + +contract PolicyRegistryUpdateCompositeTest is PolicyRegistryTest { + // ============================================================ + // REVERTS + // ============================================================ + + /// @notice Verifies updateComposite reverts for an unknown composite id + /// @dev The target policy must exist; checks PolicyNotFound(). Fires before any child validation. + function test_updateComposite_revert_policyNotFound(address caller, uint64 seed) public { + _assumeValidCaller(caller); + uint64 ghostComposite = _wellFormedUncreatedPolicyId(seed); + uint64[] memory children = + _childIds(_wellFormedUncreatedPolicyId(seed ^ 1), _wellFormedUncreatedPolicyId(seed ^ 2)); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(caller); + policyRegistry.updateComposite(ghostComposite, children); + } + + /// @notice Verifies updateComposite reverts when the target is a simple policy + /// @dev Type guard: updateComposite only operates on UNION/INTERSECT policies; a simple + /// ALLOWLIST/BLOCKLIST target reverts with IncompatiblePolicyType(). + function test_updateComposite_revert_incompatiblePolicyType(uint8 typeIdx) public { + IPolicyRegistry.PolicyType simple = _creatablePolicyType(typeIdx); + uint64 simpleId = policyRegistry.createPolicy(admin, simple); + uint64[] memory children = _makeSimpleChildren(2); + // Call as the policy admin so the type guard, not the auth guard, is under test. + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + vm.prank(admin); + policyRegistry.updateComposite(simpleId, children); + } + + /// @notice Verifies updateComposite reverts when called by a non-admin + /// @dev Access control: only the current admin may replace the child set; checks Unauthorized(). + function test_updateComposite_revert_unauthorized(address caller, uint8 typeIdx) public { + _assumeValidCaller(caller); + vm.assume(caller != admin); + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + uint64[] memory children = _makeSimpleChildren(2); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(caller); + policyRegistry.updateComposite(composite, children); + } + + /// @notice Verifies a renounced composite can never be updated + /// @dev After renounceAdmin the admin lane is zero, so every updateComposite reverts with + /// Unauthorized() — the policy is frozen but still observable. + function test_updateComposite_revert_renouncedComposite(uint8 typeIdx) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + vm.prank(admin); + policyRegistry.renounceAdmin(composite); + + uint64[] memory children = _makeSimpleChildren(2); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(admin); // former admin — no longer authorized + policyRegistry.updateComposite(composite, children); + } + + /// @notice Verifies updateComposite reverts when given fewer than two children + /// @dev A composite must reference at least two simple policies; checks TooFewChildPolicies(). + /// There is no clear-the-list path. Exercises both the empty (0) and single-child (1) cases. + function test_updateComposite_revert_tooFewChildPolicies(uint8 typeIdx) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + + uint64[] memory none = new uint64[](0); + vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.prank(admin); + policyRegistry.updateComposite(composite, none); + + uint64[] memory one = _makeSimpleChildren(1); + vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.prank(admin); + policyRegistry.updateComposite(composite, one); + } + + /// @notice Verifies updateComposite reverts when given more than the child cap + /// @dev Composite child-policy cap is MAX_CHILD_POLICIES (4), distinct from the 64-account + /// membership limit; checks BatchSizeTooLarge(MAX_CHILD_POLICIES). + function test_updateComposite_revert_batchSizeTooLarge(uint8 typeIdx, uint8 overflow) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + uint256 n = MAX_CHILD_POLICIES + 1 + (uint256(overflow) % 4); // 5..8 + uint64[] memory tooMany = _makeSimpleChildren(n); + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + vm.prank(admin); + policyRegistry.updateComposite(composite, tooMany); + } + + /// @notice Verifies updateComposite reverts when a new child policy does not exist + /// @dev Checks PolicyNotFound() for a well-formed but never-created child. + function test_updateComposite_revert_policyNotFoundChild(uint8 typeIdx, uint64 seed) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + uint64[] memory children = + _childIds(_wellFormedUncreatedPolicyId(seed), _wellFormedUncreatedPolicyId(seed ^ 0xbeef)); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(admin); + policyRegistry.updateComposite(composite, children); + } + + /// @notice Verifies updateComposite reverts when a new child is itself a composite + /// @dev Child policies must be simple; a composite child reverts with + /// InvalidChildPolicy(childPolicyId) carrying the offending ID. + function test_updateComposite_revert_invalidChildPolicy(uint8 typeIdx) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + uint64 otherComposite = _createComposite(IPolicyRegistry.PolicyType.INTERSECT, 2); + uint64 simpleChild = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + + uint64[] memory children = _childIds(simpleChild, otherComposite); + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, otherComposite)); + vm.prank(admin); + policyRegistry.updateComposite(composite, children); + } + + // ============================================================ + // EVENT + REPLACEMENT + // ============================================================ + + /// @notice Verifies updateComposite emits CompositePolicyUpdated with the full new child set + /// @dev One event per call carrying the complete post-update set; topic args match policyId / updater. + function test_updateComposite_success_emitsCompositePolicyUpdated(uint8 typeIdx) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + uint64[] memory newChildren = _makeSimpleChildren(2); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.CompositePolicyUpdated(composite, admin, newChildren); + vm.prank(admin); + policyRegistry.updateComposite(composite, newChildren); + } + + /// @notice Verifies updateComposite accepts a new child set exactly at the cap + /// @dev Boundary check: MAX_CHILD_POLICIES (4) children is inclusive. + function test_updateComposite_success_atMaxChildren(uint8 typeIdx) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + uint64[] memory newChildren = _makeSimpleChildren(MAX_CHILD_POLICIES); + vm.prank(admin); + policyRegistry.updateComposite(composite, newChildren); + assertTrue(policyRegistry.policyExists(composite)); + } + + /// @notice Verifies updateComposite replaces the child set in full rather than merging + /// @dev Behavioral proof via isAuthorized: an account authorized under the OLD child set is no + /// longer authorized once the set is swapped for children it does not belong to. + function test_updateComposite_success_replacesChildSet(address account) public { + // UNION over two allowlists; `account` is a member of the first only. + uint64 childA = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 childB = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 composite = + policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.UNION, _childIds(childA, childB)); + + address[] memory one = new address[](1); + one[0] = account; + vm.prank(admin); + policyRegistry.updateAllowlist(childA, true, one); + assertTrue(policyRegistry.isAuthorized(composite, account), "union should authorize a member of child A"); + + // Swap in a fresh child set that `account` does not belong to. + uint64 childC = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 childD = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(admin); + policyRegistry.updateComposite(composite, _childIds(childC, childD)); + + assertFalse( + policyRegistry.isAuthorized(composite, account), + "old child set must no longer govern after full replacement" + ); + } +} diff --git a/test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol b/test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol new file mode 100644 index 00000000..425496cc --- /dev/null +++ b/test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IPolicyRegistry} from "base-std/interfaces/IPolicyRegistry.sol"; + +import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; + +/// @title Sequential revert-order test for `updateComposite`. +/// +/// @notice **Canonical order:** +/// 1. POLICY-NOT-FOUND (composite `policyId` does not exist) → `PolicyNotFound` +/// 2. INCOMPATIBLE-TYPE (`policyId` is a simple policy) → `IncompatiblePolicyType` +/// 3. UNAUTHORIZED (caller is not the admin) → `Unauthorized` +/// 4. TOO-FEW-CHILDREN (`childPolicyIds.length < 2`) → `TooFewChildPolicies` +/// 5. BATCH-SIZE (`childPolicyIds.length > MAX_CHILD_POLICIES`) → `BatchSizeTooLarge(4)` +/// 6. CHILD-NOT-FOUND (a child does not exist) → `PolicyNotFound` +/// 7. INVALID-CHILD (a child is itself a composite) → `InvalidChildPolicy` +/// +/// Walks from all conditions broken to success, fixing one per step. `PolicyNotFound` +/// appears twice: for the composite itself (step 1) and for a child (step 6). +contract PolicyRegistryUpdateCompositeRevertOrderTest is PolicyRegistryTest { + /// @notice Walks through every revert in canonical order, fixing one per step, ending at success. + function test_updateComposite_revertOrder(uint8 typeIdx) public { + IPolicyRegistry.PolicyType gate = _creatableCompositeType(typeIdx); + + // Fixtures. + // Two existing simple policies double as the composite's seed children and the valid + // replacement children used at the end of the walk. + uint64[] memory validSimple = _makeSimpleChildren(2); + uint64 composite = _createComposite(bob, alice, gate, validSimple); // admin = alice + uint64 simpleId = policyRegistry.createPolicy(alice, IPolicyRegistry.PolicyType.ALLOWLIST); // exists, simple + uint64 otherComposite = _createComposite(IPolicyRegistry.PolicyType.INTERSECT, 2); // composite child + + // Ghost (well-formed, never-created) IDs at counters far above anything created. + uint64 ghostComposite = (uint64(uint8(IPolicyRegistry.PolicyType.UNION)) << 56) | uint64(3_000_000); + uint64 ghostChild = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(2_000_000); + uint64[] memory tooManyGhosts = new uint64[](MAX_CHILD_POLICIES + 1); // 5, all nonexistent + for (uint256 i = 0; i < tooManyGhosts.length; ++i) { + tooManyGhosts[i] = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(1_000_000 + i); + } + + // 1. POLICY-NOT-FOUND (self): composite never created, called by a non-admin, oversized child set. + vm.prank(attacker); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + policyRegistry.updateComposite(ghostComposite, tooManyGhosts); + + // Fix: target an existing policy — but a simple one (wrong type). + + // 2. INCOMPATIBLE-TYPE: target exists but is simple; fires before the auth check. + vm.prank(attacker); + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + policyRegistry.updateComposite(simpleId, tooManyGhosts); + + // Fix: target the composite (correct type); attacker is still not its admin. + + // 3. UNAUTHORIZED: composite, but caller is not the admin (alice); fires before the count/size checks. + vm.prank(attacker); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + policyRegistry.updateComposite(composite, tooManyGhosts); + + // Fix: call as the admin (alice). + + // 4. TOO-FEW-CHILDREN: admin, but a single (nonexistent) child; fires before the existence check. + uint64[] memory one = new uint64[](1); + one[0] = ghostChild; + vm.prank(alice); + vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + policyRegistry.updateComposite(composite, one); + + // Fix: supply at least two children. + + // 5. BATCH-SIZE: admin, but > MAX_CHILD_POLICIES (all nonexistent); fires before the existence check. + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + policyRegistry.updateComposite(composite, tooManyGhosts); + + // Fix: bring the child set within the cap. + + // 6. CHILD-NOT-FOUND: in-range child set, but one child never existed; fires before the + // simple-vs-composite check. + vm.prank(alice); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + policyRegistry.updateComposite(composite, _childIds(ghostChild, otherComposite)); + + // Fix: replace the ghost with an existing simple policy. + + // 7. INVALID-CHILD: all children exist, but one is itself a composite → InvalidChildPolicy. + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, otherComposite)); + policyRegistry.updateComposite(composite, _childIds(validSimple[0], otherComposite)); + + // Fix: replace the composite child with a simple policy. + + // Success. + vm.prank(alice); + policyRegistry.updateComposite(composite, _childIds(validSimple[0], validSimple[1])); + } +} From ae96444855080531668070492f9096395c8810bd Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Fri, 24 Jul 2026 12:59:06 -0400 Subject: [PATCH 2/5] feat: create mock polocy --- test/lib/mocks/MockPolicyRegistry.sol | 137 +++++++++++++++++-- test/lib/mocks/MockPolicyRegistryStorage.sol | 15 ++ 2 files changed, 140 insertions(+), 12 deletions(-) diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index 03fbfcc2..dfebae07 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -77,6 +77,13 @@ contract MockPolicyRegistry is IPolicyRegistry { /// precompile. uint256 internal constant MAX_BATCH_SIZE = 64; + /// @notice Maximum number of child policies a composite may reference. + /// `createCompositePolicy` and `updateComposite` revert with + /// `BatchSizeTooLarge(MAX_CHILD_POLICIES)` when `childPolicyIds.length` + /// exceeds this value. Distinct from `MAX_BATCH_SIZE` (64), which caps + /// account-membership batches. Mirrors the Rust PolicyRegistry precompile. + uint256 internal constant MAX_CHILD_POLICIES = 4; + // ============================================================ // POLICY CREATION // ============================================================ @@ -106,6 +113,22 @@ contract MockPolicyRegistry is IPolicyRegistry { _batchSetMembers({policyId: newPolicyId, policyType: policyType, value: true, accounts: accounts}); } + /// @inheritdoc IPolicyRegistry + function createCompositePolicy(address admin, PolicyType policyType, uint64[] calldata childPolicyIds) + external + returns (uint64 newPolicyId) + { + if (admin == address(0)) revert ZeroAddress(); + if (policyType != PolicyType.UNION && policyType != PolicyType.INTERSECT) revert IncompatiblePolicyType(); + if (childPolicyIds.length < 2) revert TooFewChildPolicies(); + if (childPolicyIds.length > MAX_CHILD_POLICIES) revert BatchSizeTooLarge(MAX_CHILD_POLICIES); + _requireCreatedSimplePolicies(childPolicyIds); + + newPolicyId = _create(admin, policyType); + MockPolicyRegistryStorage.layout().children[newPolicyId] = childPolicyIds; + emit CompositePolicyUpdated(newPolicyId, msg.sender, childPolicyIds); + } + // ============================================================ // POLICY ADMINISTRATION // ============================================================ @@ -163,24 +186,33 @@ contract MockPolicyRegistry is IPolicyRegistry { _batchSetMembers({policyId: policyId, policyType: PolicyType.BLOCKLIST, value: blocked, accounts: accounts}); } + /// @inheritdoc IPolicyRegistry + function updateComposite(uint64 policyId, uint64[] calldata childPolicyIds) external { + // Canonical check precedence (see updateComposite_revertOrder test): + // self-not-found → incompatible-type → unauthorized → too-few → batch-size → + // child-not-found → invalid-child. The type guard precedes the auth guard, and + // a renounced composite (admin zero) fails the auth guard for every caller. + uint256 packed = _requireCustom(policyId); + if (!_isComposite(policyId)) revert IncompatiblePolicyType(); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + if (childPolicyIds.length < 2) revert TooFewChildPolicies(); + if (childPolicyIds.length > MAX_CHILD_POLICIES) revert BatchSizeTooLarge(MAX_CHILD_POLICIES); + _requireCreatedSimplePolicies(childPolicyIds); + + // Full replacement: assigning a memory array to the storage array resets its + // length and overwrites elements. Child sets are ≤ MAX_CHILD_POLICIES, so there + // is no stale-tail concern. + MockPolicyRegistryStorage.layout().children[policyId] = childPolicyIds; + emit CompositePolicyUpdated(policyId, msg.sender, childPolicyIds); + } + // ============================================================ // AUTHORIZATION QUERIES // ============================================================ /// @inheritdoc IPolicyRegistry function isAuthorized(uint64 policyId, address account) external view returns (bool) { - // Built-in short-circuits precede any SLOAD; sentinels have no - // storage entry. - if (policyId == ALWAYS_ALLOW_ID) return true; - if (policyId == ALWAYS_BLOCK_ID) return false; - // Short-circuit malformed IDs so the `_typeOf` enum cast can't panic. - if (!_isWellFormed(policyId)) return false; - // Hot path: one SLOAD (the membership bit). No existence check — - // callers pre-validate via `policyExists` at write time. For - // non-existent IDs the result collapses to empty-member-set - // semantics (ALLOWLIST → false, BLOCKLIST → true). - bool member = MockPolicyRegistryStorage.layout().members[policyId][account]; - return _typeOf(policyId) == PolicyType.ALLOWLIST ? member : !member; + return _isAuthorized(policyId, account); } // ============================================================ @@ -292,6 +324,87 @@ contract MockPolicyRegistry is IPolicyRegistry { if (packed == 0) revert PolicyNotFound(); } + /// @dev Core authorization logic shared by the external view and composite + /// child evaluation. Never reverts. + /// + /// Composites evaluate their children LIVE (reading each child's current + /// membership, not a snapshot). Children are validated to be simple at + /// write time, so the recursion terminates at depth 1: a composite calls + /// `_isAuthorized` per child, each of which resolves via the simple path + /// (or a built-in short-circuit). + function _isAuthorized(uint64 policyId, address account) internal view returns (bool) { + // Built-in short-circuits precede any SLOAD; sentinels have no + // storage entry. + if (policyId == ALWAYS_ALLOW_ID) return true; + if (policyId == ALWAYS_BLOCK_ID) return false; + // Short-circuit malformed IDs so the `_typeOf` enum cast can't panic. + if (!_isWellFormed(policyId)) return false; + + PolicyType policyType = _typeOf(policyId); + if (policyType == PolicyType.UNION) return _isAuthorizedUnion(policyId, account); + if (policyType == PolicyType.INTERSECT) return _isAuthorizedIntersect(policyId, account); + + // Simple hot path: one SLOAD (the membership bit). No existence check — + // callers pre-validate via `policyExists` at write time. For non-existent + // IDs the result collapses to empty-member-set semantics (ALLOWLIST → + // false, BLOCKLIST → true). + bool member = MockPolicyRegistryStorage.layout().members[policyId][account]; + return policyType == PolicyType.ALLOWLIST ? member : !member; + } + + /// @dev Evaluates a UNION (OR) composite over its LIVE child set: authorized if ANY + /// child authorizes; + function _isAuthorizedUnion(uint64 policyId, address account) internal view returns (bool) { + uint64[] storage childPolicyIds = MockPolicyRegistryStorage.layout().children[policyId]; + uint256 childCount = childPolicyIds.length; + for (uint256 i = 0; i < childCount; ++i) { + if (_isAuthorized(childPolicyIds[i], account)) return true; + } + return false; + } + + /// @dev Evaluates an INTERSECT (AND) composite over its LIVE child set: authorized only + /// if EVERY child authorizes; + function _isAuthorizedIntersect(uint64 policyId, address account) internal view returns (bool) { + uint64[] storage childPolicyIds = MockPolicyRegistryStorage.layout().children[policyId]; + uint256 childCount = childPolicyIds.length; + for (uint256 i = 0; i < childCount; ++i) { + if (!_isAuthorized(childPolicyIds[i], account)) return false; + } + return true; + } + + /// @dev Requires every composite child to be a created, custom, SIMPLE policy: + /// it must exist, must not be a built-in sentinel (ALWAYS_ALLOW / ALWAYS_BLOCK), + /// and must not itself be a composite. Two passes so `PolicyNotFound` takes + /// precedence over `InvalidChildPolicy` across the whole set (matches the + /// canonical revert order the Rust precompile mirrors). + function _requireCreatedSimplePolicies(uint64[] calldata childPolicyIds) internal view { + MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout(); + // Pass 1: existence. + for (uint256 i = 0; i < childPolicyIds.length; ++i) { + if ($.policies[childPolicyIds[i]] == 0) revert PolicyNotFound(); + } + // Pass 2: must be a simple policy + for (uint256 i = 0; i < childPolicyIds.length; ++i) { + uint64 child = childPolicyIds[i]; + if (_isBuiltin(child) || _isComposite(child)) revert InvalidChildPolicy(child); + } + } + + /// @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; + } + + /// @dev True iff `policyId` is a built-in (ALWAYS_ALLOW / ALWAYS_BLOCK). + /// Sentinels are reserved and may not be used as composite children. + function _isBuiltin(uint64 policyId) internal pure returns (bool) { + return policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID; + } + function _makeId(PolicyType policyType, uint56 counter) internal pure returns (uint64) { return (uint64(uint8(policyType)) << POLICY_ID_TYPE_SHIFT) | uint64(counter); } diff --git a/test/lib/mocks/MockPolicyRegistryStorage.sol b/test/lib/mocks/MockPolicyRegistryStorage.sol index 7c260ee3..01f5d01b 100644 --- a/test/lib/mocks/MockPolicyRegistryStorage.sol +++ b/test/lib/mocks/MockPolicyRegistryStorage.sol @@ -40,6 +40,10 @@ library MockPolicyRegistryStorage { // into counters 0 and 1 before consuming counter 2 for the new // custom policy. uint56 nextCounter; + // Child policy IDs of a composite (UNION/INTERSECT); empty for simple policies. + // The full set is replaced on every `createCompositePolicy` / `updateComposite`; + // capped at MAX_CHILD_POLICIES entries. + mapping(uint64 policyId => uint64[] childPolicyIds) children; } // keccak256(abi.encode(uint256(keccak256("base.policy_registry")) - 1)) & ~bytes32(uint256(0xff)) @@ -58,6 +62,7 @@ library MockPolicyRegistryStorage { uint256 internal constant MEMBERS_OFFSET = 1; uint256 internal constant PENDING_ADMINS_OFFSET = 2; uint256 internal constant NEXT_COUNTER_OFFSET = 3; + uint256 internal constant CHILDREN_OFFSET = 4; /// @notice Absolute slot for a top-level field of `Layout`. function slotOf(uint256 offset) internal pure returns (bytes32) { @@ -88,6 +93,7 @@ library MockPolicyRegistryStorage { function membersBaseSlot() internal pure returns (bytes32) { return slotOf(MEMBERS_OFFSET); } function pendingAdminsBaseSlot() internal pure returns (bytes32) { return slotOf(PENDING_ADMINS_OFFSET); } function nextCounterSlot() internal pure returns (bytes32) { return slotOf(NEXT_COUNTER_OFFSET); } + function childrenBaseSlot() internal pure returns (bytes32) { return slotOf(CHILDREN_OFFSET); } // forgefmt: disable-end @@ -116,6 +122,15 @@ library MockPolicyRegistryStorage { return keccak256(abi.encode(policyId, pendingAdminsBaseSlot())); } + /// @notice Slot of the length word of the dynamic array `children[policyId]`. + /// @dev The `childPolicyIds` elements are packed contiguously starting at + /// `keccak256(childrenSlot(policyId))`, 4 `uint64` entries per 32-byte + /// slot (Solidity dynamic-array element layout). The Rust impl mirrors + /// this: length here, elements at the hashed base. + function childrenSlot(uint64 policyId) internal pure returns (bytes32) { + return keccak256(abi.encode(policyId, childrenBaseSlot())); + } + // ============================================================ // PACKED-SLOT CODECS // ============================================================ From f5c259ddcfb93ab51703383f6f3fbd4819e84554 Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Fri, 24 Jul 2026 13:04:14 -0400 Subject: [PATCH 3/5] test: reject built-in sentinels as composite children Add tests asserting createCompositePolicy and updateComposite revert InvalidChildPolicy when a child is a built-in sentinel (ALWAYS_ALLOW_ID / ALWAYS_BLOCK_ID), and align the IPolicyRegistry natspec accordingly. Generated with Claude Code Co-Authored-By: Claude --- src/interfaces/IPolicyRegistry.sol | 2 +- .../createCompositePolicy.t.sol | 30 +++++++++++++++++++ .../unit/PolicyRegistry/updateComposite.t.sol | 23 ++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/interfaces/IPolicyRegistry.sol b/src/interfaces/IPolicyRegistry.sol index 85493a51..dd8f9758 100644 --- a/src/interfaces/IPolicyRegistry.sol +++ b/src/interfaces/IPolicyRegistry.sol @@ -130,7 +130,7 @@ interface IPolicyRegistry { /// @dev Reverts with `TooFewChildPolicies` when `childPolicyIds.length < 2`. /// @dev Reverts with `BatchSizeTooLarge(4)` when `childPolicyIds.length > 4` /// @dev Reverts with `PolicyNotFound` when any child policy does not exist. - /// @dev Reverts with `InvalidChildPolicy` when any child policy is not a simple policy. + /// @dev Reverts with `InvalidChildPolicy` when any child policy is not a simple policy or a built-in policy. /// @dev Panics with arithmetic overflow (Panic 0x11) when the policy counter has reached its maximum value. /// /// @param admin Initial admin authorized to update child policies and transfer or renounce diff --git a/test/unit/PolicyRegistry/createCompositePolicy.t.sol b/test/unit/PolicyRegistry/createCompositePolicy.t.sol index 82e31201..6df2c60d 100644 --- a/test/unit/PolicyRegistry/createCompositePolicy.t.sol +++ b/test/unit/PolicyRegistry/createCompositePolicy.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {IPolicyRegistry} from "base-std/interfaces/IPolicyRegistry.sol"; import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; +import {PolicyRegistryConstants} from "base-std-test/lib/mocks/MockPolicyRegistry.sol"; import {MockPolicyRegistryStorage} from "base-std-test/lib/mocks/MockPolicyRegistryStorage.sol"; contract PolicyRegistryCreateCompositePolicyTest is PolicyRegistryTest { @@ -122,6 +123,35 @@ contract PolicyRegistryCreateCompositePolicyTest is PolicyRegistryTest { policyRegistry.createCompositePolicy(admin_, pt, children); } + /// @notice Verifies createCompositePolicy reverts when a child is a built-in sentinel + /// @dev Built-in sentinels (ALWAYS_ALLOW_ID / ALWAYS_BLOCK_ID) are reserved and may not be + /// composed; each reverts with InvalidChildPolicy(childPolicyId) carrying the offending ID. + function test_createCompositePolicy_revert_builtinChild(address caller, address admin_, uint8 typeIdx) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + + // A valid simple sibling; creating it also lazily initializes the built-in sentinels so + // they pass the existence check and the built-in guard is the sole offending entry. + uint64 simpleChild = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + + // ALWAYS_ALLOW_ID as a child. + uint64[] memory withAllow = _childIds(simpleChild, PolicyRegistryConstants.ALWAYS_ALLOW_ID); + vm.expectRevert( + abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, PolicyRegistryConstants.ALWAYS_ALLOW_ID) + ); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, withAllow); + + // ALWAYS_BLOCK_ID as a child. + uint64[] memory withBlock = _childIds(simpleChild, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + vm.expectRevert( + abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, PolicyRegistryConstants.ALWAYS_BLOCK_ID) + ); + vm.prank(caller); + policyRegistry.createCompositePolicy(admin_, pt, withBlock); + } + /// @notice Verifies createCompositePolicy panics with arithmetic overflow at the counter max /// @dev Slot-writes nextCounter to type(uint56).max after seeding children to avoid iterating /// 2^56 times. Mock-only: vm.store cannot write to native precompile addresses. diff --git a/test/unit/PolicyRegistry/updateComposite.t.sol b/test/unit/PolicyRegistry/updateComposite.t.sol index 9c27dec6..7d532815 100644 --- a/test/unit/PolicyRegistry/updateComposite.t.sol +++ b/test/unit/PolicyRegistry/updateComposite.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {IPolicyRegistry} from "base-std/interfaces/IPolicyRegistry.sol"; import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; +import {PolicyRegistryConstants} from "base-std-test/lib/mocks/MockPolicyRegistry.sol"; contract PolicyRegistryUpdateCompositeTest is PolicyRegistryTest { // ============================================================ @@ -115,6 +116,28 @@ contract PolicyRegistryUpdateCompositeTest is PolicyRegistryTest { policyRegistry.updateComposite(composite, children); } + /// @notice Verifies updateComposite reverts when a new child is a built-in sentinel + /// @dev Built-in sentinels (ALWAYS_ALLOW_ID / ALWAYS_BLOCK_ID) are reserved and may not be + /// composed; each reverts with InvalidChildPolicy(childPolicyId) carrying the offending ID. + function test_updateComposite_revert_builtinChild(uint8 typeIdx) public { + uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + uint64 simpleChild = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + + uint64[] memory withAllow = _childIds(simpleChild, PolicyRegistryConstants.ALWAYS_ALLOW_ID); + vm.expectRevert( + abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, PolicyRegistryConstants.ALWAYS_ALLOW_ID) + ); + vm.prank(admin); + policyRegistry.updateComposite(composite, withAllow); + + uint64[] memory withBlock = _childIds(simpleChild, PolicyRegistryConstants.ALWAYS_BLOCK_ID); + vm.expectRevert( + abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, PolicyRegistryConstants.ALWAYS_BLOCK_ID) + ); + vm.prank(admin); + policyRegistry.updateComposite(composite, withBlock); + } + // ============================================================ // EVENT + REPLACEMENT // ============================================================ From c79057c20e69cf1ddd4758a67700d430ec7ae011 Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Fri, 24 Jul 2026 13:41:00 -0400 Subject: [PATCH 4/5] style: forge fmt Generated with Claude Code Co-Authored-By: Claude --- src/interfaces/IPolicyRegistry.sol | 2 +- test/lib/mocks/MockPolicyRegistry.sol | 2 +- .../createCompositePolicy_revertOrder.t.sol | 22 ++++++++++++++----- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/interfaces/IPolicyRegistry.sol b/src/interfaces/IPolicyRegistry.sol index dd8f9758..a2522085 100644 --- a/src/interfaces/IPolicyRegistry.sol +++ b/src/interfaces/IPolicyRegistry.sol @@ -126,7 +126,7 @@ interface IPolicyRegistry { /// @dev Child policies must be simple policies (ALLOWLIST or BLOCKLIST), never another composite. /// The child-policy set is capped at 4. /// @dev Reverts with `IncompatiblePolicyType` when `policyType` is not UNION or INTERSECT. - /// @dev Reverts with `ZeroAddress` when `admin` is `address(0)`. + /// @dev Reverts with `ZeroAddress` when `admin` is `address(0)`. /// @dev Reverts with `TooFewChildPolicies` when `childPolicyIds.length < 2`. /// @dev Reverts with `BatchSizeTooLarge(4)` when `childPolicyIds.length > 4` /// @dev Reverts with `PolicyNotFound` when any child policy does not exist. diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index dfebae07..a15f394d 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -385,7 +385,7 @@ contract MockPolicyRegistry is IPolicyRegistry { for (uint256 i = 0; i < childPolicyIds.length; ++i) { if ($.policies[childPolicyIds[i]] == 0) revert PolicyNotFound(); } - // Pass 2: must be a simple policy + // Pass 2: must be a simple policy for (uint256 i = 0; i < childPolicyIds.length; ++i) { uint64 child = childPolicyIds[i]; if (_isBuiltin(child) || _isComposite(child)) revert InvalidChildPolicy(child); diff --git a/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol b/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol index c2183d46..f7efc326 100644 --- a/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol +++ b/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol @@ -28,22 +28,28 @@ contract PolicyRegistryCreateCompositePolicyRevertOrderTest is PolicyRegistryTes uint64 compositeChild = _createComposite(IPolicyRegistry.PolicyType.UNION, 2); // Ghost (well-formed, never-created) child IDs at counters far above anything created. - uint64 neverCreatedWellFormedPolicy = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(2_000_000); + uint64 neverCreatedWellFormedPolicy = + (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(2_000_000); uint64[] memory tooManyNeverCreatedWellFormedPolicyList = new uint64[](MAX_CHILD_POLICIES + 1); // 5, all nonexistent for (uint256 i = 0; i < tooManyNeverCreatedWellFormedPolicyList.length; ++i) { - tooManyNeverCreatedWellFormedPolicyList[i] = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(1_000_000 + i); + tooManyNeverCreatedWellFormedPolicyList[i] = + (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << 56) | uint64(1_000_000 + i); } // 1. ZERO-ADMIN: admin==0 AND type simple AND oversized child set → ZeroAddress fires first. vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); - policyRegistry.createCompositePolicy(address(0), IPolicyRegistry.PolicyType.ALLOWLIST, tooManyNeverCreatedWellFormedPolicyList); + policyRegistry.createCompositePolicy( + address(0), IPolicyRegistry.PolicyType.ALLOWLIST, tooManyNeverCreatedWellFormedPolicyList + ); // Fix: use a non-zero admin. - + // 2. INCOMPATIBLE-TYPE: valid admin, but simple type (ALLOWLIST) AND oversized child set // → IncompatiblePolicyType fires before the count/size checks. vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); - policyRegistry.createCompositePolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST, tooManyNeverCreatedWellFormedPolicyList); + policyRegistry.createCompositePolicy( + admin, IPolicyRegistry.PolicyType.ALLOWLIST, tooManyNeverCreatedWellFormedPolicyList + ); // Fix: use a composite gate. @@ -66,7 +72,11 @@ contract PolicyRegistryCreateCompositePolicyRevertOrderTest is PolicyRegistryTes // 5. CHILD-NOT-FOUND: in-range child set, but one child never existed → PolicyNotFound // fires before the simple-vs-composite check. vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); - policyRegistry.createCompositePolicy(admin, composite, _childIds(tooManyNeverCreatedWellFormedPolicyList[0], tooManyNeverCreatedWellFormedPolicyList[1])); + policyRegistry.createCompositePolicy( + admin, + composite, + _childIds(tooManyNeverCreatedWellFormedPolicyList[0], tooManyNeverCreatedWellFormedPolicyList[1]) + ); // Fix: replace the ghost with an existing simple policy. From 3ea405f568a351e4ae0c0c41d41be3eebbf53358 Mon Sep 17 00:00:00 2001 From: Rayyan Alam Date: Fri, 24 Jul 2026 15:29:40 -0400 Subject: [PATCH 5/5] refactor: replace composite count errors with ChildPoliciesOutsideOfRange Collapse the TooFewChildPolicies (< 2) and BatchSizeTooLarge(4) (> 4) composite child-count checks into a single ChildPoliciesOutsideOfRange(min, max) error reverting with (2, 4). Remove the now-unused TooFewChildPolicies error; BatchSizeTooLarge is retained for account-membership batches. Add MIN_CHILD_POLICIES and update the composite tests + revert-order walks accordingly. Generated with Claude Code Co-Authored-By: Claude --- src/interfaces/IPolicyRegistry.sol | 17 +++---- test/lib/PolicyRegistryTest.sol | 16 ++++--- test/lib/mocks/MockPolicyRegistry.sol | 23 +++++++--- .../createCompositePolicy.t.sol | 45 +++++++++---------- .../createCompositePolicy_revertOrder.t.sol | 29 ++++++------ .../unit/PolicyRegistry/updateComposite.t.sol | 28 ++++++------ .../updateComposite_revertOrder.t.sol | 29 ++++++------ 7 files changed, 100 insertions(+), 87 deletions(-) diff --git a/src/interfaces/IPolicyRegistry.sol b/src/interfaces/IPolicyRegistry.sol index a2522085..84003de4 100644 --- a/src/interfaces/IPolicyRegistry.sol +++ b/src/interfaces/IPolicyRegistry.sol @@ -56,9 +56,12 @@ interface IPolicyRegistry { /// @notice `finalizeUpdateAdmin` was called with no pending admin staged. error NoPendingAdmin(); - /// @notice A composite policy was created or updated with fewer than the minimum number of - /// child policies. A composite must reference at least two simple policies. - error TooFewChildPolicies(); + /// @notice A composite policy was created or updated with a child-policy count outside the + /// permitted range. A composite must reference between `min` and `max` simple policies, + /// inclusive. + /// @param min Minimum number of child policies permitted. + /// @param max Maximum number of child policies permitted. + error ChildPoliciesOutsideOfRange(uint256 min, uint256 max); /// @notice Composite policies are not simple policies. Child policies must be existing /// ALLOWLIST or BLOCKLIST policies; @@ -127,8 +130,7 @@ interface IPolicyRegistry { /// The child-policy set is capped at 4. /// @dev Reverts with `IncompatiblePolicyType` when `policyType` is not UNION or INTERSECT. /// @dev Reverts with `ZeroAddress` when `admin` is `address(0)`. - /// @dev Reverts with `TooFewChildPolicies` when `childPolicyIds.length < 2`. - /// @dev Reverts with `BatchSizeTooLarge(4)` when `childPolicyIds.length > 4` + /// @dev Reverts with `ChildPoliciesOutsideOfRange(2, 4)` when `childPolicyIds.length` is not in `[2, 4]`. /// @dev Reverts with `PolicyNotFound` when any child policy does not exist. /// @dev Reverts with `InvalidChildPolicy` when any child policy is not a simple policy or a built-in policy. /// @dev Panics with arithmetic overflow (Panic 0x11) when the policy counter has reached its maximum value. @@ -204,9 +206,8 @@ interface IPolicyRegistry { /// @dev Reverts with `IncompatiblePolicyType` when `policyId` is not a composite (UNION or INTERSECT). /// @dev Reverts with `Unauthorized` when the caller is not the current admin. A renounced composite /// (admin `address(0)`) can never be updated. - /// @dev Reverts with `TooFewChildPolicies` when `childPolicyIds.length < 2`; there is no clear-the-list path. - /// @dev Reverts with `BatchSizeTooLarge(4)` when `childPolicyIds.length > 4` - /// (composite child-policy cap; not the account membership batch limit of 64). + /// @dev Reverts with `ChildPoliciesOutsideOfRange(2, 4)` when `childPolicyIds.length` is not in `[2, 4]`; + /// there is no clear-the-list path (the composite child-policy range, not the 64-account batch limit). /// @dev Reverts with `PolicyNotFound` when any child policy does not exist. /// @dev Reverts with `InvalidChildPolicy` when any child policy is itself a composite /// (not a simple policy). diff --git a/test/lib/PolicyRegistryTest.sol b/test/lib/PolicyRegistryTest.sol index da900204..a4631370 100644 --- a/test/lib/PolicyRegistryTest.sol +++ b/test/lib/PolicyRegistryTest.sol @@ -118,14 +118,20 @@ contract PolicyRegistryTest is BaseTest { // COMPOSITE-POLICY HELPERS // ============================================================ + /// @notice Minimum number of child policies a composite must reference. + /// @dev Mirrors the registry's composite child-policy floor. Kept as a + /// test-side literal so fork tests against the real precompile use + /// the same compile-time constant. + uint256 internal constant MIN_CHILD_POLICIES = 2; + /// @notice Maximum number of child policies a composite may reference. /// @dev Mirrors the registry's composite child-policy cap. Distinct /// from `MAX_BATCH_SIZE` (64), which caps account membership - /// batches; a composite created or updated with more than - /// `MAX_CHILD_POLICIES` children reverts with - /// `BatchSizeTooLarge(MAX_CHILD_POLICIES)`. Kept as a - /// test-side literal so fork tests against the real precompile - /// use the same compile-time constant. + /// batches; a composite created or updated with a child count + /// outside `[MIN_CHILD_POLICIES, MAX_CHILD_POLICIES]` reverts with + /// `ChildPoliciesOutsideOfRange(MIN_CHILD_POLICIES, MAX_CHILD_POLICIES)`. + /// Kept as a test-side literal so fork tests against the real + /// precompile use the same compile-time constant. uint256 internal constant MAX_CHILD_POLICIES = 4; /// @notice Create `n` simple ALLOWLIST policies (admin = `admin`) and diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index a15f394d..a9974f5c 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -77,11 +77,18 @@ contract MockPolicyRegistry is IPolicyRegistry { /// precompile. uint256 internal constant MAX_BATCH_SIZE = 64; + /// @notice Minimum number of child policies a composite must reference. + /// `createCompositePolicy` and `updateComposite` revert with + /// `ChildPoliciesOutsideOfRange(MIN_CHILD_POLICIES, MAX_CHILD_POLICIES)` + /// when `childPolicyIds.length` is below this value. + uint256 internal constant MIN_CHILD_POLICIES = 2; + /// @notice Maximum number of child policies a composite may reference. /// `createCompositePolicy` and `updateComposite` revert with - /// `BatchSizeTooLarge(MAX_CHILD_POLICIES)` when `childPolicyIds.length` - /// exceeds this value. Distinct from `MAX_BATCH_SIZE` (64), which caps - /// account-membership batches. Mirrors the Rust PolicyRegistry precompile. + /// `ChildPoliciesOutsideOfRange(MIN_CHILD_POLICIES, MAX_CHILD_POLICIES)` + /// when `childPolicyIds.length` is outside `[MIN_CHILD_POLICIES, MAX_CHILD_POLICIES]`. + /// Distinct from `MAX_BATCH_SIZE` (64), which caps account-membership batches. + /// Mirrors the Rust PolicyRegistry precompile. uint256 internal constant MAX_CHILD_POLICIES = 4; // ============================================================ @@ -120,8 +127,9 @@ contract MockPolicyRegistry is IPolicyRegistry { { if (admin == address(0)) revert ZeroAddress(); if (policyType != PolicyType.UNION && policyType != PolicyType.INTERSECT) revert IncompatiblePolicyType(); - if (childPolicyIds.length < 2) revert TooFewChildPolicies(); - if (childPolicyIds.length > MAX_CHILD_POLICIES) revert BatchSizeTooLarge(MAX_CHILD_POLICIES); + if (childPolicyIds.length < MIN_CHILD_POLICIES || childPolicyIds.length > MAX_CHILD_POLICIES) { + revert ChildPoliciesOutsideOfRange(MIN_CHILD_POLICIES, MAX_CHILD_POLICIES); + } _requireCreatedSimplePolicies(childPolicyIds); newPolicyId = _create(admin, policyType); @@ -195,8 +203,9 @@ contract MockPolicyRegistry is IPolicyRegistry { uint256 packed = _requireCustom(policyId); if (!_isComposite(policyId)) revert IncompatiblePolicyType(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - if (childPolicyIds.length < 2) revert TooFewChildPolicies(); - if (childPolicyIds.length > MAX_CHILD_POLICIES) revert BatchSizeTooLarge(MAX_CHILD_POLICIES); + if (childPolicyIds.length < MIN_CHILD_POLICIES || childPolicyIds.length > MAX_CHILD_POLICIES) { + revert ChildPoliciesOutsideOfRange(MIN_CHILD_POLICIES, MAX_CHILD_POLICIES); + } _requireCreatedSimplePolicies(childPolicyIds); // Full replacement: assigning a memory array to the storage array resets its diff --git a/test/unit/PolicyRegistry/createCompositePolicy.t.sol b/test/unit/PolicyRegistry/createCompositePolicy.t.sol index 6df2c60d..a63f7afb 100644 --- a/test/unit/PolicyRegistry/createCompositePolicy.t.sol +++ b/test/unit/PolicyRegistry/createCompositePolicy.t.sol @@ -38,45 +38,40 @@ contract PolicyRegistryCreateCompositePolicyTest is PolicyRegistryTest { policyRegistry.createCompositePolicy(admin_, pt, children); } - /// @notice Verifies createCompositePolicy reverts when given fewer than two children - /// @dev A composite must reference at least two simple policies; checks TooFewChildPolicies(). - /// Exercises both the empty (0) and single-child (1) cases. - function test_createCompositePolicy_revert_tooFewChildPolicies(address caller, address admin_, uint8 typeIdx) - public - { + /// @notice Verifies createCompositePolicy reverts when the child count is outside [2, 4] + /// @dev A composite must reference between MIN_CHILD_POLICIES (2) and MAX_CHILD_POLICIES (4) + /// simple policies, inclusive; checks ChildPoliciesOutsideOfRange(2, 4). Exercises both + /// under-range (0 and 1 children) and over-range (5..8 children) cases. The composite + /// child-policy range is distinct from the 64-account membership limit. + function test_createCompositePolicy_revert_childPoliciesOutsideOfRange( + address caller, + address admin_, + uint8 typeIdx, + uint8 overflow + ) public { _assumeValidCaller(caller); vm.assume(admin_ != address(0)); IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + bytes memory expectedRevert = abi.encodeWithSelector( + IPolicyRegistry.ChildPoliciesOutsideOfRange.selector, MIN_CHILD_POLICIES, MAX_CHILD_POLICIES + ); - // 0 children. + // Under range: 0 children. uint64[] memory none = new uint64[](0); - vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.expectRevert(expectedRevert); vm.prank(caller); policyRegistry.createCompositePolicy(admin_, pt, none); - // 1 child (a real, existing simple policy) — still below the minimum. + // Under range: 1 child (a real, existing simple policy) — still below the minimum. uint64[] memory one = _makeSimpleChildren(1); - vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.expectRevert(expectedRevert); vm.prank(caller); policyRegistry.createCompositePolicy(admin_, pt, one); - } - /// @notice Verifies createCompositePolicy reverts when given more than the child cap - /// @dev Composite child-policy cap is MAX_CHILD_POLICIES (4), distinct from the 64-account - /// membership limit; checks BatchSizeTooLarge(MAX_CHILD_POLICIES). All children are valid - /// simple policies, so only the size condition is broken. - function test_createCompositePolicy_revert_batchSizeTooLarge( - address caller, - address admin_, - uint8 typeIdx, - uint8 overflow - ) public { - _assumeValidCaller(caller); - vm.assume(admin_ != address(0)); - IPolicyRegistry.PolicyType pt = _creatableCompositeType(typeIdx); + // Over range: 5..8 valid simple children — only the count condition is broken. uint256 n = MAX_CHILD_POLICIES + 1 + (uint256(overflow) % 4); // 5..8 uint64[] memory tooMany = _makeSimpleChildren(n); - vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + vm.expectRevert(expectedRevert); vm.prank(caller); policyRegistry.createCompositePolicy(admin_, pt, tooMany); } diff --git a/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol b/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol index f7efc326..03f861e6 100644 --- a/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol +++ b/test/unit/PolicyRegistry/createCompositePolicy_revertOrder.t.sol @@ -10,10 +10,9 @@ import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; /// @notice **Canonical order:** /// 1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress` /// 2. INCOMPATIBLE-TYPE (`policyType` not UNION/INTERSECT) → `IncompatiblePolicyType` -/// 3. TOO-FEW-CHILDREN (`childPolicyIds.length < 2`) → `TooFewChildPolicies` -/// 4. BATCH-SIZE (`childPolicyIds.length > MAX_CHILD_POLICIES`) → `BatchSizeTooLarge(4)` -/// 5. CHILD-NOT-FOUND (a child does not exist) → `PolicyNotFound` -/// 6. INVALID-CHILD (a child is itself a composite) → `InvalidChildPolicy` +/// 3. COUNT-OUTSIDE-RANGE (`childPolicyIds.length` not in `[2, 4]`) → `ChildPoliciesOutsideOfRange(2, 4)` +/// 4. CHILD-NOT-FOUND (a child does not exist) → `PolicyNotFound` +/// 5. INVALID-CHILD (a child is itself a composite) → `InvalidChildPolicy` /// /// Walks from all conditions broken to success, fixing one per step. ZeroAddress-first /// precedence mirrors `createPolicy` / `createPolicyWithAccounts`. @@ -53,23 +52,23 @@ contract PolicyRegistryCreateCompositePolicyRevertOrderTest is PolicyRegistryTes // Fix: use a composite gate. - // 3. TOO-FEW-CHILDREN: composite type, but a single (nonexistent) child → TooFewChildPolicies - // fires before the existence check. + // 3. COUNT-OUTSIDE-RANGE: composite type, but a child count outside [2, 4] (all nonexistent) + // → ChildPoliciesOutsideOfRange fires before the existence check. Exercises both the + // under-range (1 child) and over-range (> MAX_CHILD_POLICIES) ends. + bytes memory outsideRange = abi.encodeWithSelector( + IPolicyRegistry.ChildPoliciesOutsideOfRange.selector, MIN_CHILD_POLICIES, MAX_CHILD_POLICIES + ); uint64[] memory one = new uint64[](1); one[0] = neverCreatedWellFormedPolicy; - vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.expectRevert(outsideRange); policyRegistry.createCompositePolicy(admin, composite, one); - // Fix: supply at least two children. - - // 4. BATCH-SIZE: composite type, but > MAX_CHILD_POLICIES (all nonexistent) → BatchSizeTooLarge - // fires before the existence check. - vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + vm.expectRevert(outsideRange); policyRegistry.createCompositePolicy(admin, composite, tooManyNeverCreatedWellFormedPolicyList); - // Fix: bring the child set within the cap. + // Fix: bring the child count within [2, 4]. - // 5. CHILD-NOT-FOUND: in-range child set, but one child never existed → PolicyNotFound + // 4. CHILD-NOT-FOUND: in-range child set, but one child never existed → PolicyNotFound // fires before the simple-vs-composite check. vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); policyRegistry.createCompositePolicy( @@ -80,7 +79,7 @@ contract PolicyRegistryCreateCompositePolicyRevertOrderTest is PolicyRegistryTes // Fix: replace the ghost with an existing simple policy. - // 6. INVALID-CHILD: all children exist, but one is itself a composite → InvalidChildPolicy. + // 5. INVALID-CHILD: all children exist, but one is itself a composite → InvalidChildPolicy. vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, compositeChild)); policyRegistry.createCompositePolicy(admin, composite, _childIds(validSimpleA, compositeChild)); diff --git a/test/unit/PolicyRegistry/updateComposite.t.sol b/test/unit/PolicyRegistry/updateComposite.t.sol index 7d532815..eb82a6f5 100644 --- a/test/unit/PolicyRegistry/updateComposite.t.sol +++ b/test/unit/PolicyRegistry/updateComposite.t.sol @@ -62,31 +62,33 @@ contract PolicyRegistryUpdateCompositeTest is PolicyRegistryTest { policyRegistry.updateComposite(composite, children); } - /// @notice Verifies updateComposite reverts when given fewer than two children - /// @dev A composite must reference at least two simple policies; checks TooFewChildPolicies(). - /// There is no clear-the-list path. Exercises both the empty (0) and single-child (1) cases. - function test_updateComposite_revert_tooFewChildPolicies(uint8 typeIdx) public { + /// @notice Verifies updateComposite reverts when the new child count is outside [2, 4] + /// @dev A composite must reference between MIN_CHILD_POLICIES (2) and MAX_CHILD_POLICIES (4) + /// simple policies, inclusive; checks ChildPoliciesOutsideOfRange(2, 4). There is no + /// clear-the-list path. Exercises both under-range (0 and 1 children) and over-range + /// (5..8 children) cases. + function test_updateComposite_revert_childPoliciesOutsideOfRange(uint8 typeIdx, uint8 overflow) public { uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + bytes memory expectedRevert = abi.encodeWithSelector( + IPolicyRegistry.ChildPoliciesOutsideOfRange.selector, MIN_CHILD_POLICIES, MAX_CHILD_POLICIES + ); + // Under range: 0 children. uint64[] memory none = new uint64[](0); - vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.expectRevert(expectedRevert); vm.prank(admin); policyRegistry.updateComposite(composite, none); + // Under range: 1 child. uint64[] memory one = _makeSimpleChildren(1); - vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.expectRevert(expectedRevert); vm.prank(admin); policyRegistry.updateComposite(composite, one); - } - /// @notice Verifies updateComposite reverts when given more than the child cap - /// @dev Composite child-policy cap is MAX_CHILD_POLICIES (4), distinct from the 64-account - /// membership limit; checks BatchSizeTooLarge(MAX_CHILD_POLICIES). - function test_updateComposite_revert_batchSizeTooLarge(uint8 typeIdx, uint8 overflow) public { - uint64 composite = _createComposite(_creatableCompositeType(typeIdx), 2); + // Over range: 5..8 valid simple children. uint256 n = MAX_CHILD_POLICIES + 1 + (uint256(overflow) % 4); // 5..8 uint64[] memory tooMany = _makeSimpleChildren(n); - vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + vm.expectRevert(expectedRevert); vm.prank(admin); policyRegistry.updateComposite(composite, tooMany); } diff --git a/test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol b/test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol index 425496cc..ba5f22a0 100644 --- a/test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol +++ b/test/unit/PolicyRegistry/updateComposite_revertOrder.t.sol @@ -11,13 +11,12 @@ import {PolicyRegistryTest} from "base-std-test/lib/PolicyRegistryTest.sol"; /// 1. POLICY-NOT-FOUND (composite `policyId` does not exist) → `PolicyNotFound` /// 2. INCOMPATIBLE-TYPE (`policyId` is a simple policy) → `IncompatiblePolicyType` /// 3. UNAUTHORIZED (caller is not the admin) → `Unauthorized` -/// 4. TOO-FEW-CHILDREN (`childPolicyIds.length < 2`) → `TooFewChildPolicies` -/// 5. BATCH-SIZE (`childPolicyIds.length > MAX_CHILD_POLICIES`) → `BatchSizeTooLarge(4)` -/// 6. CHILD-NOT-FOUND (a child does not exist) → `PolicyNotFound` -/// 7. INVALID-CHILD (a child is itself a composite) → `InvalidChildPolicy` +/// 4. COUNT-OUTSIDE-RANGE (`childPolicyIds.length` not in `[2, 4]`) → `ChildPoliciesOutsideOfRange(2, 4)` +/// 5. CHILD-NOT-FOUND (a child does not exist) → `PolicyNotFound` +/// 6. INVALID-CHILD (a child is itself a composite) → `InvalidChildPolicy` /// /// Walks from all conditions broken to success, fixing one per step. `PolicyNotFound` -/// appears twice: for the composite itself (step 1) and for a child (step 6). +/// appears twice: for the composite itself (step 1) and for a child (step 5). contract PolicyRegistryUpdateCompositeRevertOrderTest is PolicyRegistryTest { /// @notice Walks through every revert in canonical order, fixing one per step, ending at success. function test_updateComposite_revertOrder(uint8 typeIdx) public { @@ -60,23 +59,25 @@ contract PolicyRegistryUpdateCompositeRevertOrderTest is PolicyRegistryTest { // Fix: call as the admin (alice). - // 4. TOO-FEW-CHILDREN: admin, but a single (nonexistent) child; fires before the existence check. + // 4. COUNT-OUTSIDE-RANGE: admin, but a child count outside [2, 4] (all nonexistent); fires + // before the existence check. Exercises both the under-range (1 child) and over-range + // (> MAX_CHILD_POLICIES) ends. + bytes memory outsideRange = abi.encodeWithSelector( + IPolicyRegistry.ChildPoliciesOutsideOfRange.selector, MIN_CHILD_POLICIES, MAX_CHILD_POLICIES + ); uint64[] memory one = new uint64[](1); one[0] = ghostChild; vm.prank(alice); - vm.expectRevert(IPolicyRegistry.TooFewChildPolicies.selector); + vm.expectRevert(outsideRange); policyRegistry.updateComposite(composite, one); - // Fix: supply at least two children. - - // 5. BATCH-SIZE: admin, but > MAX_CHILD_POLICIES (all nonexistent); fires before the existence check. vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.BatchSizeTooLarge.selector, MAX_CHILD_POLICIES)); + vm.expectRevert(outsideRange); policyRegistry.updateComposite(composite, tooManyGhosts); - // Fix: bring the child set within the cap. + // Fix: bring the child count within [2, 4]. - // 6. CHILD-NOT-FOUND: in-range child set, but one child never existed; fires before the + // 5. CHILD-NOT-FOUND: in-range child set, but one child never existed; fires before the // simple-vs-composite check. vm.prank(alice); vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); @@ -84,7 +85,7 @@ contract PolicyRegistryUpdateCompositeRevertOrderTest is PolicyRegistryTest { // Fix: replace the ghost with an existing simple policy. - // 7. INVALID-CHILD: all children exist, but one is itself a composite → InvalidChildPolicy. + // 6. INVALID-CHILD: all children exist, but one is itself a composite → InvalidChildPolicy. vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(IPolicyRegistry.InvalidChildPolicy.selector, otherComposite)); policyRegistry.updateComposite(composite, _childIds(validSimple[0], otherComposite));