Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions test/lib/mocks/MockPolicyRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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});
Expand All @@ -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);
}
Expand Down Expand Up @@ -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).
Expand Down
12 changes: 12 additions & 0 deletions test/unit/PolicyRegistry/createPolicy.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down
13 changes: 11 additions & 2 deletions test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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);
Expand Down
Loading