From 3e5565566fcd2c8abf42dbd30cd5078fae06d155 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 09:03:22 -0400 Subject: [PATCH 01/17] Add PolicyRegistry implementation Implements IPolicyRegistry following the simplified interface on main: ALLOWLIST/BLOCKLIST only (no COMPOUND), batch membership updates, spec naming (updateAllowlist/Blocklist, stageUpdateAdmin, finalizeUpdateAdmin, renounceAdmin), and built-in IDs 0 (always-allow) and type(uint64).max (always-reject). Storage layout: each policy is a single packed uint256 with the admin address in bits [167:8] and the PolicyType discriminator in bits [7:0]. Existence is checked by _policies[id] == 0 (safe because createPolicy requires a non-zero admin). --- src/impls/PolicyRegistry.sol | 215 +++++++++++++++++++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 src/impls/PolicyRegistry.sol diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol new file mode 100644 index 0000000..66ba75c --- /dev/null +++ b/src/impls/PolicyRegistry.sol @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.30; + +import {IPolicyRegistry} from "../interfaces/IPolicyRegistry.sol"; + +/// @title PolicyRegistry +/// @author Coinbase +/// @notice Reference implementation of IPolicyRegistry. +/// +/// @dev Storage layout: +/// +/// _policies[id]: packed uint256 +/// [255:168] unused +/// [167:8] admin address (160 bits) +/// [7:0] PolicyType (ALLOWLIST = 0, BLOCKLIST = 1) +/// +/// Existence sentinel: _policies[id] == 0 means the policy was +/// never created. This is safe because createPolicy requires a +/// non-zero admin, so a valid packed value is always non-zero. +/// +/// Built-in IDs (0 and type(uint64).max) are never stored in +/// _policies; their behavior is handled via constants. +contract PolicyRegistry is IPolicyRegistry { + /*////////////////////////////////////////////////////////////// + CONSTANTS + //////////////////////////////////////////////////////////////*/ + + uint64 private constant ALWAYS_ALLOW_ID = 0; + uint64 private constant ALWAYS_REJECT_ID = type(uint64).max; + + uint256 private constant TYPE_MASK = 0xFF; + uint256 private constant ADMIN_SHIFT = 8; + + /*////////////////////////////////////////////////////////////// + STORAGE + //////////////////////////////////////////////////////////////*/ + + mapping(uint64 policyId => uint256 packed) private _policies; + + // ALLOWLIST: member == true means the address is authorized. + // BLOCKLIST: member == true means the address is blocked. + mapping(uint64 policyId => mapping(address account => bool)) private _members; + + mapping(uint64 policyId => address pendingAdmin) private _pendingAdmins; + + uint64 private _nextId = 1; + + /*////////////////////////////////////////////////////////////// + POLICY CREATION + //////////////////////////////////////////////////////////////*/ + + /// @inheritdoc IPolicyRegistry + function createPolicy(address admin, PolicyType pt) external returns (uint64 newPolicyId) { + newPolicyId = _create(admin, pt); + } + + /// @inheritdoc IPolicyRegistry + function createPolicyWithAccounts(address admin, PolicyType pt, address[] calldata accounts) + external + returns (uint64 newPolicyId) + { + newPolicyId = _create(admin, pt); + _batchSetMembers(newPolicyId, pt, true, accounts); + } + + /*////////////////////////////////////////////////////////////// + POLICY ADMINISTRATION + //////////////////////////////////////////////////////////////*/ + + /// @inheritdoc IPolicyRegistry + function stageUpdateAdmin(uint64 policyId, address newAdmin) external { + uint256 packed = _requireCustom(policyId); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _pendingAdmins[policyId] = newAdmin; + emit PolicyAdminStaged(policyId, msg.sender, newAdmin); + } + + /// @inheritdoc IPolicyRegistry + function finalizeUpdateAdmin(uint64 policyId) external { + uint256 packed = _requireCustom(policyId); + address pending = _pendingAdmins[policyId]; + if (pending == address(0)) revert NoPendingAdmin(); + if (pending != msg.sender) revert Unauthorized(); + address previousAdmin = _decodeAdmin(packed); + _policies[policyId] = _encode(_decodeType(packed), msg.sender); + delete _pendingAdmins[policyId]; + emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); + } + + /// @inheritdoc IPolicyRegistry + function renounceAdmin(uint64 policyId) external { + uint256 packed = _requireCustom(policyId); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _policies[policyId] = _encode(_decodeType(packed), address(0)); + if (_pendingAdmins[policyId] != address(0)) delete _pendingAdmins[policyId]; + emit PolicyAdminUpdated(policyId, msg.sender, address(0)); + } + + /// @inheritdoc IPolicyRegistry + function updateAllowlist(uint64 policyId, bool allowed, address[] calldata accounts) external { + uint256 packed = _requireCustom(policyId); + if (_decodeType(packed) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _batchSetMembers(policyId, PolicyType.ALLOWLIST, allowed, accounts); + } + + /// @inheritdoc IPolicyRegistry + function updateBlocklist(uint64 policyId, bool blocked, address[] calldata accounts) external { + uint256 packed = _requireCustom(policyId); + if (_decodeType(packed) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _batchSetMembers(policyId, PolicyType.BLOCKLIST, blocked, accounts); + } + + /*////////////////////////////////////////////////////////////// + AUTHORIZATION QUERIES + //////////////////////////////////////////////////////////////*/ + + /// @inheritdoc IPolicyRegistry + function isAuthorized(uint64 policyId, address account) external view returns (bool) { + if (policyId == ALWAYS_ALLOW_ID) return true; + if (policyId == ALWAYS_REJECT_ID) return false; + uint256 packed = _policies[policyId]; + if (packed == 0) revert PolicyNotFound(); + bool member = _members[policyId][account]; + return _decodeType(packed) == PolicyType.ALLOWLIST ? member : !member; + } + + /*////////////////////////////////////////////////////////////// + POLICY QUERIES + //////////////////////////////////////////////////////////////*/ + + /// @inheritdoc IPolicyRegistry + function nextPolicyId() external view returns (uint64) { + return _nextId; + } + + /// @inheritdoc IPolicyRegistry + function policyExists(uint64 policyId) external view returns (bool) { + return policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID || _policies[policyId] != 0; + } + + /// @inheritdoc IPolicyRegistry + function policyType(uint64 policyId) external view returns (PolicyType) { + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID) return PolicyType.ALLOWLIST; + uint256 packed = _policies[policyId]; + if (packed == 0) revert PolicyNotFound(); + return _decodeType(packed); + } + + /// @inheritdoc IPolicyRegistry + function policyAdmin(uint64 policyId) external view returns (address) { + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID) return address(0); + uint256 packed = _policies[policyId]; + if (packed == 0) revert PolicyNotFound(); + return _decodeAdmin(packed); + } + + /// @inheritdoc IPolicyRegistry + function pendingPolicyAdmin(uint64 policyId) external view returns (address) { + return _pendingAdmins[policyId]; + } + + /*////////////////////////////////////////////////////////////// + INTERNAL HELPERS + //////////////////////////////////////////////////////////////*/ + + function _create(address admin, PolicyType pt) internal returns (uint64 newPolicyId) { + if (pt != PolicyType.ALLOWLIST && pt != PolicyType.BLOCKLIST) revert InvalidPolicyType(); + if (admin == address(0)) revert ZeroAddress(); + newPolicyId = _nextId; + // Overflow is structurally impossible before heat death: uint64.max is the + // ALWAYS_REJECT sentinel and is never reached by the monotonic counter. + unchecked { + ++_nextId; + } + _policies[newPolicyId] = _encode(pt, admin); + emit PolicyCreated(newPolicyId, msg.sender, pt); + emit PolicyAdminUpdated(newPolicyId, address(0), admin); + } + + function _batchSetMembers(uint64 policyId, PolicyType pt, bool value, address[] calldata accounts) internal { + mapping(address => bool) storage members = _members[policyId]; + if (pt == PolicyType.ALLOWLIST) { + emit AllowlistUpdated(policyId, msg.sender, value, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + members[accounts[i]] = value; + } + } else { + emit BlocklistUpdated(policyId, msg.sender, value, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + members[accounts[i]] = value; + } + } + } + + function _requireCustom(uint64 policyId) internal view returns (uint256 packed) { + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID) revert PolicyNotFound(); + packed = _policies[policyId]; + if (packed == 0) revert PolicyNotFound(); + } + + function _encode(PolicyType pt, address admin) internal pure returns (uint256) { + return uint256(pt) | (uint256(uint160(admin)) << ADMIN_SHIFT); + } + + function _decodeType(uint256 packed) internal pure returns (PolicyType) { + return PolicyType(packed & TYPE_MASK); + } + + function _decodeAdmin(uint256 packed) internal pure returns (address) { + // forge-lint: disable-next-line(unsafe-typecast) + return address(uint160(packed >> ADMIN_SHIFT)); + } +} From a3b1ad9ac949deb3a4b1263971e2ac4fcfcdf61d Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 09:05:34 -0400 Subject: [PATCH 02/17] Apply harness style: named args, forge fmt --- src/impls/PolicyRegistry.sol | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol index 66ba75c..e9413d0 100644 --- a/src/impls/PolicyRegistry.sol +++ b/src/impls/PolicyRegistry.sol @@ -50,17 +50,17 @@ contract PolicyRegistry is IPolicyRegistry { //////////////////////////////////////////////////////////////*/ /// @inheritdoc IPolicyRegistry - function createPolicy(address admin, PolicyType pt) external returns (uint64 newPolicyId) { - newPolicyId = _create(admin, pt); + function createPolicy(address admin, PolicyType policyType) external returns (uint64 newPolicyId) { + newPolicyId = _create(admin, policyType); } /// @inheritdoc IPolicyRegistry - function createPolicyWithAccounts(address admin, PolicyType pt, address[] calldata accounts) + function createPolicyWithAccounts(address admin, PolicyType policyType, address[] calldata accounts) external returns (uint64 newPolicyId) { - newPolicyId = _create(admin, pt); - _batchSetMembers(newPolicyId, pt, true, accounts); + newPolicyId = _create(admin, policyType); + _batchSetMembers({policyId: newPolicyId, policyType: policyType, value: true, accounts: accounts}); } /*////////////////////////////////////////////////////////////// @@ -82,7 +82,7 @@ contract PolicyRegistry is IPolicyRegistry { if (pending == address(0)) revert NoPendingAdmin(); if (pending != msg.sender) revert Unauthorized(); address previousAdmin = _decodeAdmin(packed); - _policies[policyId] = _encode(_decodeType(packed), msg.sender); + _policies[policyId] = _encode({policyType: _decodeType(packed), admin: msg.sender}); delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); } @@ -91,7 +91,7 @@ contract PolicyRegistry is IPolicyRegistry { function renounceAdmin(uint64 policyId) external { uint256 packed = _requireCustom(policyId); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _policies[policyId] = _encode(_decodeType(packed), address(0)); + _policies[policyId] = _encode({policyType: _decodeType(packed), admin: address(0)}); if (_pendingAdmins[policyId] != address(0)) delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, msg.sender, address(0)); } @@ -101,7 +101,7 @@ contract PolicyRegistry is IPolicyRegistry { uint256 packed = _requireCustom(policyId); if (_decodeType(packed) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _batchSetMembers(policyId, PolicyType.ALLOWLIST, allowed, accounts); + _batchSetMembers({policyId: policyId, policyType: PolicyType.ALLOWLIST, value: allowed, accounts: accounts}); } /// @inheritdoc IPolicyRegistry @@ -109,7 +109,7 @@ contract PolicyRegistry is IPolicyRegistry { uint256 packed = _requireCustom(policyId); if (_decodeType(packed) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _batchSetMembers(policyId, PolicyType.BLOCKLIST, blocked, accounts); + _batchSetMembers({policyId: policyId, policyType: PolicyType.BLOCKLIST, value: blocked, accounts: accounts}); } /*////////////////////////////////////////////////////////////// @@ -165,8 +165,8 @@ contract PolicyRegistry is IPolicyRegistry { INTERNAL HELPERS //////////////////////////////////////////////////////////////*/ - function _create(address admin, PolicyType pt) internal returns (uint64 newPolicyId) { - if (pt != PolicyType.ALLOWLIST && pt != PolicyType.BLOCKLIST) revert InvalidPolicyType(); + function _create(address admin, PolicyType policyType) internal returns (uint64 newPolicyId) { + if (policyType != PolicyType.ALLOWLIST && policyType != PolicyType.BLOCKLIST) revert InvalidPolicyType(); if (admin == address(0)) revert ZeroAddress(); newPolicyId = _nextId; // Overflow is structurally impossible before heat death: uint64.max is the @@ -174,14 +174,16 @@ contract PolicyRegistry is IPolicyRegistry { unchecked { ++_nextId; } - _policies[newPolicyId] = _encode(pt, admin); - emit PolicyCreated(newPolicyId, msg.sender, pt); + _policies[newPolicyId] = _encode({policyType: policyType, admin: admin}); + emit PolicyCreated(newPolicyId, msg.sender, policyType); emit PolicyAdminUpdated(newPolicyId, address(0), admin); } - function _batchSetMembers(uint64 policyId, PolicyType pt, bool value, address[] calldata accounts) internal { + function _batchSetMembers(uint64 policyId, PolicyType policyType, bool value, address[] calldata accounts) + internal + { mapping(address => bool) storage members = _members[policyId]; - if (pt == PolicyType.ALLOWLIST) { + if (policyType == PolicyType.ALLOWLIST) { emit AllowlistUpdated(policyId, msg.sender, value, accounts); for (uint256 i = 0; i < accounts.length; ++i) { members[accounts[i]] = value; @@ -200,8 +202,8 @@ contract PolicyRegistry is IPolicyRegistry { if (packed == 0) revert PolicyNotFound(); } - function _encode(PolicyType pt, address admin) internal pure returns (uint256) { - return uint256(pt) | (uint256(uint160(admin)) << ADMIN_SHIFT); + function _encode(PolicyType policyType, address admin) internal pure returns (uint256) { + return uint256(policyType) | (uint256(uint160(admin)) << ADMIN_SHIFT); } function _decodeType(uint256 packed) internal pure returns (PolicyType) { From ed234e7a74c81c5086a4526957555c2093f7dc8a Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 12:19:47 -0400 Subject: [PATCH 03/17] Revert policyType for built-in IDs with AlwaysAllowPolicy / AlwaysRejectPolicy --- src/impls/PolicyRegistry.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol index e9413d0..0cd5aa2 100644 --- a/src/impls/PolicyRegistry.sol +++ b/src/impls/PolicyRegistry.sol @@ -142,7 +142,8 @@ contract PolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function policyType(uint64 policyId) external view returns (PolicyType) { - if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID) return PolicyType.ALLOWLIST; + if (policyId == ALWAYS_ALLOW_ID) revert AlwaysAllowPolicy(); + if (policyId == ALWAYS_REJECT_ID) revert AlwaysRejectPolicy(); uint256 packed = _policies[policyId]; if (packed == 0) revert PolicyNotFound(); return _decodeType(packed); From 33a6e9b28e74e8897b2a1112a1d74df4154676e5 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 12:26:40 -0400 Subject: [PATCH 04/17] Emit batch membership events after state writes, unify loop --- src/impls/PolicyRegistry.sol | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol index 0cd5aa2..41407af 100644 --- a/src/impls/PolicyRegistry.sol +++ b/src/impls/PolicyRegistry.sol @@ -184,16 +184,13 @@ contract PolicyRegistry is IPolicyRegistry { internal { mapping(address => bool) storage members = _members[policyId]; + for (uint256 i = 0; i < accounts.length; ++i) { + members[accounts[i]] = value; + } if (policyType == PolicyType.ALLOWLIST) { emit AllowlistUpdated(policyId, msg.sender, value, accounts); - for (uint256 i = 0; i < accounts.length; ++i) { - members[accounts[i]] = value; - } } else { emit BlocklistUpdated(policyId, msg.sender, value, accounts); - for (uint256 i = 0; i < accounts.length; ++i) { - members[accounts[i]] = value; - } } } From 0a1879d3c5a96cfe6c22f07449f2499a647457a0 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 12:41:46 -0400 Subject: [PATCH 05/17] Fix renounceAdmin ALLOWLIST bug via CREATED_BIT, specific errors for built-in IDs, overflow comment --- src/impls/PolicyRegistry.sol | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol index 41407af..5cd84f7 100644 --- a/src/impls/PolicyRegistry.sol +++ b/src/impls/PolicyRegistry.sol @@ -10,13 +10,15 @@ import {IPolicyRegistry} from "../interfaces/IPolicyRegistry.sol"; /// @dev Storage layout: /// /// _policies[id]: packed uint256 -/// [255:168] unused -/// [167:8] admin address (160 bits) +/// [255:169] unused +/// [168] created flag (always 1 for existing policies, never cleared) +/// [167:8] admin address (160 bits, 0 after renounceAdmin) /// [7:0] PolicyType (ALLOWLIST = 0, BLOCKLIST = 1) /// -/// Existence sentinel: _policies[id] == 0 means the policy was -/// never created. This is safe because createPolicy requires a -/// non-zero admin, so a valid packed value is always non-zero. +/// Existence sentinel: bit 168 (CREATED_BIT). A zero admin after +/// renounceAdmin would make the packed slot zero for ALLOWLIST +/// policies (type = 0), so we cannot use packed == 0 as the +/// sentinel. CREATED_BIT is set at creation and never cleared. /// /// Built-in IDs (0 and type(uint64).max) are never stored in /// _policies; their behavior is handled via constants. @@ -30,6 +32,7 @@ contract PolicyRegistry is IPolicyRegistry { uint256 private constant TYPE_MASK = 0xFF; uint256 private constant ADMIN_SHIFT = 8; + uint256 private constant CREATED_BIT = uint256(1) << 168; /*////////////////////////////////////////////////////////////// STORAGE @@ -121,7 +124,7 @@ contract PolicyRegistry is IPolicyRegistry { if (policyId == ALWAYS_ALLOW_ID) return true; if (policyId == ALWAYS_REJECT_ID) return false; uint256 packed = _policies[policyId]; - if (packed == 0) revert PolicyNotFound(); + if (packed & CREATED_BIT == 0) revert PolicyNotFound(); bool member = _members[policyId][account]; return _decodeType(packed) == PolicyType.ALLOWLIST ? member : !member; } @@ -137,7 +140,7 @@ contract PolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function policyExists(uint64 policyId) external view returns (bool) { - return policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID || _policies[policyId] != 0; + return policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID || _policies[policyId] & CREATED_BIT != 0; } /// @inheritdoc IPolicyRegistry @@ -145,7 +148,7 @@ contract PolicyRegistry is IPolicyRegistry { if (policyId == ALWAYS_ALLOW_ID) revert AlwaysAllowPolicy(); if (policyId == ALWAYS_REJECT_ID) revert AlwaysRejectPolicy(); uint256 packed = _policies[policyId]; - if (packed == 0) revert PolicyNotFound(); + if (packed & CREATED_BIT == 0) revert PolicyNotFound(); return _decodeType(packed); } @@ -153,7 +156,7 @@ contract PolicyRegistry is IPolicyRegistry { function policyAdmin(uint64 policyId) external view returns (address) { if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID) return address(0); uint256 packed = _policies[policyId]; - if (packed == 0) revert PolicyNotFound(); + if (packed & CREATED_BIT == 0) revert PolicyNotFound(); return _decodeAdmin(packed); } @@ -170,8 +173,9 @@ contract PolicyRegistry is IPolicyRegistry { if (policyType != PolicyType.ALLOWLIST && policyType != PolicyType.BLOCKLIST) revert InvalidPolicyType(); if (admin == address(0)) revert ZeroAddress(); newPolicyId = _nextId; - // Overflow is structurally impossible before heat death: uint64.max is the - // ALWAYS_REJECT sentinel and is never reached by the monotonic counter. + // No overflow guard: at one policy per 2-second block, reaching uint64.max + // takes ~1.2 trillion years. An explicit revert would cost gas on every + // createPolicy call to protect against an unreachable condition. unchecked { ++_nextId; } @@ -195,13 +199,14 @@ contract PolicyRegistry is IPolicyRegistry { } function _requireCustom(uint64 policyId) internal view returns (uint256 packed) { - if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID) revert PolicyNotFound(); + if (policyId == ALWAYS_ALLOW_ID) revert AlwaysAllowPolicy(); + if (policyId == ALWAYS_REJECT_ID) revert AlwaysRejectPolicy(); packed = _policies[policyId]; - if (packed == 0) revert PolicyNotFound(); + if (packed & CREATED_BIT == 0) revert PolicyNotFound(); } function _encode(PolicyType policyType, address admin) internal pure returns (uint256) { - return uint256(policyType) | (uint256(uint160(admin)) << ADMIN_SHIFT); + return CREATED_BIT | uint256(policyType) | (uint256(uint160(admin)) << ADMIN_SHIFT); } function _decodeType(uint256 packed) internal pure returns (PolicyType) { From a43a18ba0c4dfd259c6042c1c0d528921dd1e735 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 12:50:34 -0400 Subject: [PATCH 06/17] Add invariant comment to isAuthorized built-in short-circuits --- src/impls/PolicyRegistry.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol index 5cd84f7..d1bba3e 100644 --- a/src/impls/PolicyRegistry.sol +++ b/src/impls/PolicyRegistry.sol @@ -121,6 +121,11 @@ contract PolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function isAuthorized(uint64 policyId, address account) external view returns (bool) { + // Built-in short-circuits MUST remain before any storage read. A renounced + // ALLOWLIST policy has admin == address(0) in its packed slot, which looks + // identical to the always-allow built-in's admin — but they are distinguished + // by ID, not by data. If these checks are ever moved or removed, a renounced + // policy at ID 0 would become indistinguishable from always-allow. if (policyId == ALWAYS_ALLOW_ID) return true; if (policyId == ALWAYS_REJECT_ID) return false; uint256 packed = _policies[policyId]; From 1786caccc396ea1b0638380eae6b16025222cf5c Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 12:53:23 -0400 Subject: [PATCH 07/17] Tighten isAuthorized built-in comment --- src/impls/PolicyRegistry.sol | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol index d1bba3e..d27fd7b 100644 --- a/src/impls/PolicyRegistry.sol +++ b/src/impls/PolicyRegistry.sol @@ -121,11 +121,8 @@ contract PolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function isAuthorized(uint64 policyId, address account) external view returns (bool) { - // Built-in short-circuits MUST remain before any storage read. A renounced - // ALLOWLIST policy has admin == address(0) in its packed slot, which looks - // identical to the always-allow built-in's admin — but they are distinguished - // by ID, not by data. If these checks are ever moved or removed, a renounced - // policy at ID 0 would become indistinguishable from always-allow. + // Built-in short-circuits MUST remain before any storage read: built-in IDs + // have no entry in _policies and must never reach the storage path. if (policyId == ALWAYS_ALLOW_ID) return true; if (policyId == ALWAYS_REJECT_ID) return false; uint256 packed = _policies[policyId]; From 6fd0607baec3bba0fd4d32be91a0f4a2c4fff314 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 22:53:13 -0400 Subject: [PATCH 08/17] Update PolicyRegistry for new IPolicyRegistry interface - ALWAYS_BLOCK_ID changes from type(uint64).max to 1 - Global counter (_nextCounter) starts at 2, typed uint56 - nextPolicyId(PolicyType) returns full top-byte-encoded ID - Type is derived from ID top byte via _typeFromId; not stored in packed slot - policyType() returns ALWAYS_ALLOW/ALWAYS_BLOCK for built-ins instead of reverting - Remove AlwaysAllowPolicy/AlwaysRejectPolicy (not in interface) - _requireCustom simplified: just checks CREATED_BIT, built-ins fall through to PolicyNotFound --- src/impls/PolicyRegistry.sol | 86 ++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol index d27fd7b..386e59d 100644 --- a/src/impls/PolicyRegistry.sol +++ b/src/impls/PolicyRegistry.sol @@ -13,26 +13,31 @@ import {IPolicyRegistry} from "../interfaces/IPolicyRegistry.sol"; /// [255:169] unused /// [168] created flag (always 1 for existing policies, never cleared) /// [167:8] admin address (160 bits, 0 after renounceAdmin) -/// [7:0] PolicyType (ALLOWLIST = 0, BLOCKLIST = 1) +/// [7:0] unused /// -/// Existence sentinel: bit 168 (CREATED_BIT). A zero admin after -/// renounceAdmin would make the packed slot zero for ALLOWLIST -/// policies (type = 0), so we cannot use packed == 0 as the -/// sentinel. CREATED_BIT is set at creation and never cleared. +/// Existence sentinel: bit 168 (CREATED_BIT). A renounced policy has +/// admin = address(0), making bits [167:8] zero; CREATED_BIT prevents +/// that from being confused with a never-created slot. /// -/// Built-in IDs (0 and type(uint64).max) are never stored in -/// _policies; their behavior is handled via constants. +/// Policy type is NOT stored in the packed slot. It is encoded in the +/// top byte of the policy ID itself per the IPolicyRegistry encoding +/// scheme, and recovered via _typeFromId() with no storage read. +/// +/// Built-in IDs (0 and 1) are never stored in _policies; their +/// behavior is handled via constants. contract PolicyRegistry is IPolicyRegistry { /*////////////////////////////////////////////////////////////// CONSTANTS //////////////////////////////////////////////////////////////*/ uint64 private constant ALWAYS_ALLOW_ID = 0; - uint64 private constant ALWAYS_REJECT_ID = type(uint64).max; + uint64 private constant ALWAYS_BLOCK_ID = 1; + + // Policy ID encoding: [63:56] = uint8(PolicyType), [55:0] = global counter. + uint256 private constant TYPE_SHIFT = 56; - uint256 private constant TYPE_MASK = 0xFF; - uint256 private constant ADMIN_SHIFT = 8; uint256 private constant CREATED_BIT = uint256(1) << 168; + uint256 private constant ADMIN_SHIFT = 8; /*////////////////////////////////////////////////////////////// STORAGE @@ -46,7 +51,9 @@ contract PolicyRegistry is IPolicyRegistry { mapping(uint64 policyId => address pendingAdmin) private _pendingAdmins; - uint64 private _nextId = 1; + // Global monotonic counter for the low 56 bits of custom policy IDs. + // Starts at 2: IDs 0 and 1 are reserved for the built-ins. + uint56 private _nextCounter = 2; /*////////////////////////////////////////////////////////////// POLICY CREATION @@ -85,7 +92,7 @@ contract PolicyRegistry is IPolicyRegistry { if (pending == address(0)) revert NoPendingAdmin(); if (pending != msg.sender) revert Unauthorized(); address previousAdmin = _decodeAdmin(packed); - _policies[policyId] = _encode({policyType: _decodeType(packed), admin: msg.sender}); + _policies[policyId] = _encode(msg.sender); delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); } @@ -94,7 +101,7 @@ contract PolicyRegistry is IPolicyRegistry { function renounceAdmin(uint64 policyId) external { uint256 packed = _requireCustom(policyId); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _policies[policyId] = _encode({policyType: _decodeType(packed), admin: address(0)}); + _policies[policyId] = _encode(address(0)); if (_pendingAdmins[policyId] != address(0)) delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, msg.sender, address(0)); } @@ -102,7 +109,7 @@ contract PolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function updateAllowlist(uint64 policyId, bool allowed, address[] calldata accounts) external { uint256 packed = _requireCustom(policyId); - if (_decodeType(packed) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); + if (_typeFromId(policyId) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); _batchSetMembers({policyId: policyId, policyType: PolicyType.ALLOWLIST, value: allowed, accounts: accounts}); } @@ -110,7 +117,7 @@ contract PolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function updateBlocklist(uint64 policyId, bool blocked, address[] calldata accounts) external { uint256 packed = _requireCustom(policyId); - if (_decodeType(packed) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); + if (_typeFromId(policyId) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); _batchSetMembers({policyId: policyId, policyType: PolicyType.BLOCKLIST, value: blocked, accounts: accounts}); } @@ -124,11 +131,11 @@ contract PolicyRegistry is IPolicyRegistry { // Built-in short-circuits MUST remain before any storage read: built-in IDs // have no entry in _policies and must never reach the storage path. if (policyId == ALWAYS_ALLOW_ID) return true; - if (policyId == ALWAYS_REJECT_ID) return false; + if (policyId == ALWAYS_BLOCK_ID) return false; uint256 packed = _policies[policyId]; if (packed & CREATED_BIT == 0) revert PolicyNotFound(); bool member = _members[policyId][account]; - return _decodeType(packed) == PolicyType.ALLOWLIST ? member : !member; + return _typeFromId(policyId) == PolicyType.ALLOWLIST ? member : !member; } /*////////////////////////////////////////////////////////////// @@ -136,27 +143,27 @@ contract PolicyRegistry is IPolicyRegistry { //////////////////////////////////////////////////////////////*/ /// @inheritdoc IPolicyRegistry - function nextPolicyId() external view returns (uint64) { - return _nextId; + function nextPolicyId(PolicyType policyType) external view returns (uint64) { + return _makeId(policyType, _nextCounter); } /// @inheritdoc IPolicyRegistry function policyExists(uint64 policyId) external view returns (bool) { - return policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID || _policies[policyId] & CREATED_BIT != 0; + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return true; + return _policies[policyId] & CREATED_BIT != 0; } /// @inheritdoc IPolicyRegistry function policyType(uint64 policyId) external view returns (PolicyType) { - if (policyId == ALWAYS_ALLOW_ID) revert AlwaysAllowPolicy(); - if (policyId == ALWAYS_REJECT_ID) revert AlwaysRejectPolicy(); - uint256 packed = _policies[policyId]; - if (packed & CREATED_BIT == 0) revert PolicyNotFound(); - return _decodeType(packed); + if (policyId == ALWAYS_ALLOW_ID) return PolicyType.ALWAYS_ALLOW; + if (policyId == ALWAYS_BLOCK_ID) return PolicyType.ALWAYS_BLOCK; + if (_policies[policyId] & CREATED_BIT == 0) revert PolicyNotFound(); + return _typeFromId(policyId); } /// @inheritdoc IPolicyRegistry function policyAdmin(uint64 policyId) external view returns (address) { - if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_REJECT_ID) return address(0); + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return address(0); uint256 packed = _policies[policyId]; if (packed & CREATED_BIT == 0) revert PolicyNotFound(); return _decodeAdmin(packed); @@ -174,14 +181,14 @@ contract PolicyRegistry is IPolicyRegistry { function _create(address admin, PolicyType policyType) internal returns (uint64 newPolicyId) { if (policyType != PolicyType.ALLOWLIST && policyType != PolicyType.BLOCKLIST) revert InvalidPolicyType(); if (admin == address(0)) revert ZeroAddress(); - newPolicyId = _nextId; - // No overflow guard: at one policy per 2-second block, reaching uint64.max - // takes ~1.2 trillion years. An explicit revert would cost gas on every - // createPolicy call to protect against an unreachable condition. + uint56 counter = _nextCounter; + // No overflow guard: at one policy per 2-second block, exhausting the 56-bit + // counter space (~7.2e16 IDs) takes ~4.6 billion years. unchecked { - ++_nextId; + _nextCounter = counter + 1; } - _policies[newPolicyId] = _encode({policyType: policyType, admin: admin}); + newPolicyId = _makeId(policyType, counter); + _policies[newPolicyId] = _encode(admin); emit PolicyCreated(newPolicyId, msg.sender, policyType); emit PolicyAdminUpdated(newPolicyId, address(0), admin); } @@ -201,18 +208,21 @@ contract PolicyRegistry is IPolicyRegistry { } function _requireCustom(uint64 policyId) internal view returns (uint256 packed) { - if (policyId == ALWAYS_ALLOW_ID) revert AlwaysAllowPolicy(); - if (policyId == ALWAYS_REJECT_ID) revert AlwaysRejectPolicy(); packed = _policies[policyId]; if (packed & CREATED_BIT == 0) revert PolicyNotFound(); } - function _encode(PolicyType policyType, address admin) internal pure returns (uint256) { - return CREATED_BIT | uint256(policyType) | (uint256(uint160(admin)) << ADMIN_SHIFT); + function _makeId(PolicyType policyType, uint56 counter) internal pure returns (uint64) { + return (uint64(uint8(policyType)) << TYPE_SHIFT) | uint64(counter); + } + + function _typeFromId(uint64 policyId) internal pure returns (PolicyType) { + // forge-lint: disable-next-line(unsafe-typecast) + return PolicyType(uint8(policyId >> TYPE_SHIFT)); } - function _decodeType(uint256 packed) internal pure returns (PolicyType) { - return PolicyType(packed & TYPE_MASK); + function _encode(address admin) internal pure returns (uint256) { + return CREATED_BIT | (uint256(uint160(admin)) << ADMIN_SHIFT); } function _decodeAdmin(uint256 packed) internal pure returns (address) { From 5cee5021473d9877888d557a52c7b10f9c100b0b Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 23:09:11 -0400 Subject: [PATCH 09/17] Port PolicyRegistry to MockPolicyRegistry; fix stale test stubs Replaces the skeleton MockPolicyRegistry with the full reference implementation following the MockB20/MockActivationRegistry pattern: written as Solidity-as-if-Rust, etched at the precompile address via vm.etch. Removes src/impls/PolicyRegistry.sol. Also fixes stale test stubs and BaseTest comment that still referenced type(uint64).max as the always-reject sentinel (now built-in ID 1), and rewrites nextPolicyId stubs to reflect the global counter design (shared counter, both types advance together). --- src/impls/PolicyRegistry.sol | 232 ------------------ test/lib/BaseTest.sol | 18 +- test/lib/mocks/MockPolicyRegistry.sol | 253 ++++++++++++++++---- test/unit/PolicyRegistry/isAuthorized.t.sol | 6 +- test/unit/PolicyRegistry/nextPolicyId.t.sol | 45 ++-- test/unit/PolicyRegistry/policyAdmin.t.sol | 2 +- test/unit/PolicyRegistry/policyExists.t.sol | 6 +- 7 files changed, 247 insertions(+), 315 deletions(-) delete mode 100644 src/impls/PolicyRegistry.sol diff --git a/src/impls/PolicyRegistry.sol b/src/impls/PolicyRegistry.sol deleted file mode 100644 index 386e59d..0000000 --- a/src/impls/PolicyRegistry.sol +++ /dev/null @@ -1,232 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.30; - -import {IPolicyRegistry} from "../interfaces/IPolicyRegistry.sol"; - -/// @title PolicyRegistry -/// @author Coinbase -/// @notice Reference implementation of IPolicyRegistry. -/// -/// @dev Storage layout: -/// -/// _policies[id]: packed uint256 -/// [255:169] unused -/// [168] created flag (always 1 for existing policies, never cleared) -/// [167:8] admin address (160 bits, 0 after renounceAdmin) -/// [7:0] unused -/// -/// Existence sentinel: bit 168 (CREATED_BIT). A renounced policy has -/// admin = address(0), making bits [167:8] zero; CREATED_BIT prevents -/// that from being confused with a never-created slot. -/// -/// Policy type is NOT stored in the packed slot. It is encoded in the -/// top byte of the policy ID itself per the IPolicyRegistry encoding -/// scheme, and recovered via _typeFromId() with no storage read. -/// -/// Built-in IDs (0 and 1) are never stored in _policies; their -/// behavior is handled via constants. -contract PolicyRegistry is IPolicyRegistry { - /*////////////////////////////////////////////////////////////// - CONSTANTS - //////////////////////////////////////////////////////////////*/ - - uint64 private constant ALWAYS_ALLOW_ID = 0; - uint64 private constant ALWAYS_BLOCK_ID = 1; - - // Policy ID encoding: [63:56] = uint8(PolicyType), [55:0] = global counter. - uint256 private constant TYPE_SHIFT = 56; - - uint256 private constant CREATED_BIT = uint256(1) << 168; - uint256 private constant ADMIN_SHIFT = 8; - - /*////////////////////////////////////////////////////////////// - STORAGE - //////////////////////////////////////////////////////////////*/ - - mapping(uint64 policyId => uint256 packed) private _policies; - - // ALLOWLIST: member == true means the address is authorized. - // BLOCKLIST: member == true means the address is blocked. - mapping(uint64 policyId => mapping(address account => bool)) private _members; - - mapping(uint64 policyId => address pendingAdmin) private _pendingAdmins; - - // Global monotonic counter for the low 56 bits of custom policy IDs. - // Starts at 2: IDs 0 and 1 are reserved for the built-ins. - uint56 private _nextCounter = 2; - - /*////////////////////////////////////////////////////////////// - POLICY CREATION - //////////////////////////////////////////////////////////////*/ - - /// @inheritdoc IPolicyRegistry - function createPolicy(address admin, PolicyType policyType) external returns (uint64 newPolicyId) { - newPolicyId = _create(admin, policyType); - } - - /// @inheritdoc IPolicyRegistry - function createPolicyWithAccounts(address admin, PolicyType policyType, address[] calldata accounts) - external - returns (uint64 newPolicyId) - { - newPolicyId = _create(admin, policyType); - _batchSetMembers({policyId: newPolicyId, policyType: policyType, value: true, accounts: accounts}); - } - - /*////////////////////////////////////////////////////////////// - POLICY ADMINISTRATION - //////////////////////////////////////////////////////////////*/ - - /// @inheritdoc IPolicyRegistry - function stageUpdateAdmin(uint64 policyId, address newAdmin) external { - uint256 packed = _requireCustom(policyId); - if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _pendingAdmins[policyId] = newAdmin; - emit PolicyAdminStaged(policyId, msg.sender, newAdmin); - } - - /// @inheritdoc IPolicyRegistry - function finalizeUpdateAdmin(uint64 policyId) external { - uint256 packed = _requireCustom(policyId); - address pending = _pendingAdmins[policyId]; - if (pending == address(0)) revert NoPendingAdmin(); - if (pending != msg.sender) revert Unauthorized(); - address previousAdmin = _decodeAdmin(packed); - _policies[policyId] = _encode(msg.sender); - delete _pendingAdmins[policyId]; - emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); - } - - /// @inheritdoc IPolicyRegistry - function renounceAdmin(uint64 policyId) external { - uint256 packed = _requireCustom(policyId); - if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _policies[policyId] = _encode(address(0)); - if (_pendingAdmins[policyId] != address(0)) delete _pendingAdmins[policyId]; - emit PolicyAdminUpdated(policyId, msg.sender, address(0)); - } - - /// @inheritdoc IPolicyRegistry - function updateAllowlist(uint64 policyId, bool allowed, address[] calldata accounts) external { - uint256 packed = _requireCustom(policyId); - if (_typeFromId(policyId) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); - if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _batchSetMembers({policyId: policyId, policyType: PolicyType.ALLOWLIST, value: allowed, accounts: accounts}); - } - - /// @inheritdoc IPolicyRegistry - function updateBlocklist(uint64 policyId, bool blocked, address[] calldata accounts) external { - uint256 packed = _requireCustom(policyId); - if (_typeFromId(policyId) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); - if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _batchSetMembers({policyId: policyId, policyType: PolicyType.BLOCKLIST, value: blocked, accounts: accounts}); - } - - /*////////////////////////////////////////////////////////////// - AUTHORIZATION QUERIES - //////////////////////////////////////////////////////////////*/ - - /// @inheritdoc IPolicyRegistry - function isAuthorized(uint64 policyId, address account) external view returns (bool) { - // Built-in short-circuits MUST remain before any storage read: built-in IDs - // have no entry in _policies and must never reach the storage path. - if (policyId == ALWAYS_ALLOW_ID) return true; - if (policyId == ALWAYS_BLOCK_ID) return false; - uint256 packed = _policies[policyId]; - if (packed & CREATED_BIT == 0) revert PolicyNotFound(); - bool member = _members[policyId][account]; - return _typeFromId(policyId) == PolicyType.ALLOWLIST ? member : !member; - } - - /*////////////////////////////////////////////////////////////// - POLICY QUERIES - //////////////////////////////////////////////////////////////*/ - - /// @inheritdoc IPolicyRegistry - function nextPolicyId(PolicyType policyType) external view returns (uint64) { - return _makeId(policyType, _nextCounter); - } - - /// @inheritdoc IPolicyRegistry - function policyExists(uint64 policyId) external view returns (bool) { - if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return true; - return _policies[policyId] & CREATED_BIT != 0; - } - - /// @inheritdoc IPolicyRegistry - function policyType(uint64 policyId) external view returns (PolicyType) { - if (policyId == ALWAYS_ALLOW_ID) return PolicyType.ALWAYS_ALLOW; - if (policyId == ALWAYS_BLOCK_ID) return PolicyType.ALWAYS_BLOCK; - if (_policies[policyId] & CREATED_BIT == 0) revert PolicyNotFound(); - return _typeFromId(policyId); - } - - /// @inheritdoc IPolicyRegistry - function policyAdmin(uint64 policyId) external view returns (address) { - if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return address(0); - uint256 packed = _policies[policyId]; - if (packed & CREATED_BIT == 0) revert PolicyNotFound(); - return _decodeAdmin(packed); - } - - /// @inheritdoc IPolicyRegistry - function pendingPolicyAdmin(uint64 policyId) external view returns (address) { - return _pendingAdmins[policyId]; - } - - /*////////////////////////////////////////////////////////////// - INTERNAL HELPERS - //////////////////////////////////////////////////////////////*/ - - function _create(address admin, PolicyType policyType) internal returns (uint64 newPolicyId) { - if (policyType != PolicyType.ALLOWLIST && policyType != PolicyType.BLOCKLIST) revert InvalidPolicyType(); - if (admin == address(0)) revert ZeroAddress(); - uint56 counter = _nextCounter; - // No overflow guard: at one policy per 2-second block, exhausting the 56-bit - // counter space (~7.2e16 IDs) takes ~4.6 billion years. - unchecked { - _nextCounter = counter + 1; - } - newPolicyId = _makeId(policyType, counter); - _policies[newPolicyId] = _encode(admin); - emit PolicyCreated(newPolicyId, msg.sender, policyType); - emit PolicyAdminUpdated(newPolicyId, address(0), admin); - } - - function _batchSetMembers(uint64 policyId, PolicyType policyType, bool value, address[] calldata accounts) - internal - { - mapping(address => bool) storage members = _members[policyId]; - for (uint256 i = 0; i < accounts.length; ++i) { - members[accounts[i]] = value; - } - if (policyType == PolicyType.ALLOWLIST) { - emit AllowlistUpdated(policyId, msg.sender, value, accounts); - } else { - emit BlocklistUpdated(policyId, msg.sender, value, accounts); - } - } - - function _requireCustom(uint64 policyId) internal view returns (uint256 packed) { - packed = _policies[policyId]; - if (packed & CREATED_BIT == 0) revert PolicyNotFound(); - } - - function _makeId(PolicyType policyType, uint56 counter) internal pure returns (uint64) { - return (uint64(uint8(policyType)) << TYPE_SHIFT) | uint64(counter); - } - - function _typeFromId(uint64 policyId) internal pure returns (PolicyType) { - // forge-lint: disable-next-line(unsafe-typecast) - return PolicyType(uint8(policyId >> TYPE_SHIFT)); - } - - function _encode(address admin) internal pure returns (uint256) { - return CREATED_BIT | (uint256(uint160(admin)) << ADMIN_SHIFT); - } - - function _decodeAdmin(uint256 packed) internal pure returns (address) { - // forge-lint: disable-next-line(unsafe-typecast) - return address(uint160(packed >> ADMIN_SHIFT)); - } -} diff --git a/test/lib/BaseTest.sol b/test/lib/BaseTest.sol index a00b8b0..ca1aaf3 100644 --- a/test/lib/BaseTest.sol +++ b/test/lib/BaseTest.sol @@ -28,9 +28,9 @@ import {StdPrecompiles} from "src/StdPrecompiles.sol"; /// deploy a token, and need the policy-registry mock so the token's /// cross-precompile `isAuthorized` calls don't hit empty code and /// revert at the EVM level (the most common B20 policy tests use the -/// built-in sentinel IDs `0` / `type(uint64).max` to exercise both -/// authorize and forbid paths without any custom registry state). -/// Centralizing the etch here means concrete bases don't need to +/// built-in sentinel IDs `0` (ALWAYS_ALLOW) and `1` (ALWAYS_BLOCK) to +/// exercise both authorize and forbid paths without any custom registry +/// state). Centralizing the etch here means concrete bases don't need to /// reason about which precompiles they "depend on" — they're all just /// available, the way the EVM has `SLOAD`. /// @@ -45,13 +45,11 @@ import {StdPrecompiles} from "src/StdPrecompiles.sol"; /// surface function is live, with the bootstrap-window auth bypass /// for factory-originated calls and the standard role / policy / /// pause / supply-cap checks otherwise. -/// - `MockPolicyRegistry` is a SKELETON: implements only the two -/// built-in sentinel IDs (`0` → always-allow, -/// `type(uint64).max` → always-reject) so the most common B-20 tests -/// can exercise both authorize and forbid paths without any custom -/// policy state. Tests that need custom policies (create, update -/// membership, rotate admin, etc.) will fail until the full mock -/// lands in a follow-up PR. +/// - `MockPolicyRegistry` is fully implemented: every `IPolicyRegistry` +/// surface function is live. Custom policy creation, membership +/// mutation, and admin rotation all work. Built-in IDs `0` +/// (ALWAYS_ALLOW) and `1` (ALWAYS_BLOCK) are short-circuited before +/// any storage read. /// - `MockActivationRegistry` is a SKELETON: implements only `admin()` /// to return the hardcoded test admin. abstract contract BaseTest is Test { diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index d5452e4..02ae6c1 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -3,80 +3,249 @@ pragma solidity ^0.8.20; import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; -/// @notice Placeholder mock for the `IPolicyRegistry` precompile. +/// @title MockPolicyRegistry +/// @notice Reference implementation of the `IPolicyRegistry` precompile. +/// Etched at the canonical policy-registry address via `vm.etch` +/// from `BaseTest.setUp`. /// -/// Implements only the built-in sentinel semantics that B20 tests rely -/// on to exercise policy gating without configuring custom policies: -/// - `isAuthorized(0, _)` → true (ALWAYS_ALLOW, built-in ID 0) -/// - `isAuthorized(1, _)` → false (ALWAYS_BLOCK, built-in ID 1) -/// - `policyExists` returns true for those two built-ins -/// - `policyType` returns the matching PolicyType enum for those two +/// @dev Written as Solidity-as-if-Rust: unambiguous spec-correspondence +/// with the production Rust precompile is the goal, not gas +/// optimisation or Solidity idiom adherence. /// -/// Built-in ID assignments updated per IPolicyRegistry's -/// ALWAYS_ALLOW = 0, ALWAYS_BLOCK = 1 design (PR #24): the built-in -/// IDs are now the small numeric values that match the PolicyType -/// ordinals rather than the previous (0, type(uint64).max) convention. +/// **Storage layout** (Rust impl mirrors these fields in order): /// -/// Every other method reverts pending the full mock implementation in -/// a follow-up PR. Custom policy creation, admin rotation, and -/// membership mutation are out of scope until then. +/// `_policies[id]` — packed uint256: +/// [255:169] unused +/// [168] CREATED_BIT — set at creation, never cleared. +/// Required because a renounced policy has admin = +/// address(0), making bits [167:8] zero; without this +/// sentinel a renounced ALLOWLIST or BLOCKLIST policy +/// would be indistinguishable from an un-created slot. +/// [167:8] admin address (160 bits). Zero after renounceAdmin. +/// [7:0] unused (reserved for future policy flags) +/// +/// `_members[policyId][account]` — bool: +/// ALLOWLIST: true → account IS authorized. +/// BLOCKLIST: true → account IS blocked (NOT authorized). +/// +/// `_pendingAdmins[policyId]` — address staged by stageUpdateAdmin. +/// +/// `_nextCounter` — uint56 global counter for the low 56 bits of +/// custom policy IDs. Starts at 2; values 0 and 1 are defensively +/// skipped to avoid any numeric overlap with the built-in IDs. +/// +/// **Policy ID encoding:** +/// [63:56] uint8(PolicyType) discriminator +/// [55:0] _nextCounter value at creation time +/// _create rejects ALWAYS_ALLOW and ALWAYS_BLOCK types, so no +/// custom ID ever carries discriminator 0x00 or 0x01. +/// +/// **Built-in IDs** (short-circuited before any storage read): +/// 0 — ALWAYS_ALLOW: isAuthorized always returns true. +/// 1 — ALWAYS_BLOCK: isAuthorized always returns false. contract MockPolicyRegistry is IPolicyRegistry { + // ============================================================ + // CONSTANTS + // ============================================================ + uint64 internal constant ALWAYS_ALLOW_ID = 0; uint64 internal constant ALWAYS_BLOCK_ID = 1; - function isAuthorized(uint64 policyId, address /*account*/ ) external pure returns (bool) { - if (policyId == ALWAYS_ALLOW_ID) return true; - if (policyId == ALWAYS_BLOCK_ID) return false; - revert PolicyNotFound(); + // Policy ID encoding: top byte = uint8(PolicyType), low 56 bits = counter. + uint256 internal constant TYPE_SHIFT = 56; + + // Bit 168 of _policies[id]. Sits one bit above the 160-bit admin field + // at [167:8] so the two fields never overlap regardless of admin value. + uint256 internal constant CREATED_BIT = uint256(1) << 168; + + // Admin address occupies bits [167:8]; bits [7:0] are reserved. + uint256 internal constant ADMIN_SHIFT = 8; + + // ============================================================ + // STORAGE + // ============================================================ + + mapping(uint64 policyId => uint256 packed) private _policies; + mapping(uint64 policyId => mapping(address account => bool)) private _members; + mapping(uint64 policyId => address pendingAdmin) private _pendingAdmins; + + // Global monotonic counter for the low 56 bits of custom policy IDs. + // Starts at 2 to defensively skip the numeric values of the built-in IDs. + uint56 private _nextCounter = 2; + + // ============================================================ + // POLICY CREATION + // ============================================================ + + /// @inheritdoc IPolicyRegistry + function createPolicy(address admin, PolicyType policyType) external returns (uint64 newPolicyId) { + newPolicyId = _create(admin, policyType); } - function policyExists(uint64 policyId) external pure returns (bool) { - return policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID; + /// @inheritdoc IPolicyRegistry + function createPolicyWithAccounts(address admin, PolicyType policyType, address[] calldata accounts) + external + returns (uint64 newPolicyId) + { + newPolicyId = _create(admin, policyType); + _batchSetMembers({policyId: newPolicyId, policyType: policyType, value: true, accounts: accounts}); } - function createPolicy(address, PolicyType) external pure returns (uint64) { - revert("MockPolicyRegistry: not implemented"); + // ============================================================ + // POLICY ADMINISTRATION + // ============================================================ + + /// @inheritdoc IPolicyRegistry + function stageUpdateAdmin(uint64 policyId, address newAdmin) external { + uint256 packed = _requireCustom(policyId); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _pendingAdmins[policyId] = newAdmin; + emit PolicyAdminStaged(policyId, msg.sender, newAdmin); } - function createPolicyWithAccounts(address, PolicyType, address[] calldata) external pure returns (uint64) { - revert("MockPolicyRegistry: not implemented"); + /// @inheritdoc IPolicyRegistry + function finalizeUpdateAdmin(uint64 policyId) external { + uint256 packed = _requireCustom(policyId); + address pending = _pendingAdmins[policyId]; + if (pending == address(0)) revert NoPendingAdmin(); + if (pending != msg.sender) revert Unauthorized(); + address previousAdmin = _decodeAdmin(packed); + _policies[policyId] = _encode(msg.sender); + delete _pendingAdmins[policyId]; + emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); } - function stageUpdateAdmin(uint64, address) external pure { - revert("MockPolicyRegistry: not implemented"); + /// @inheritdoc IPolicyRegistry + function renounceAdmin(uint64 policyId) external { + uint256 packed = _requireCustom(policyId); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _policies[policyId] = _encode(address(0)); + delete _pendingAdmins[policyId]; + emit PolicyAdminUpdated(policyId, msg.sender, address(0)); } - function finalizeUpdateAdmin(uint64) external pure { - revert("MockPolicyRegistry: not implemented"); + /// @inheritdoc IPolicyRegistry + function updateAllowlist(uint64 policyId, bool allowed, address[] calldata accounts) external { + uint256 packed = _requireCustom(policyId); + if (_typeFromId(policyId) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _batchSetMembers({policyId: policyId, policyType: PolicyType.ALLOWLIST, value: allowed, accounts: accounts}); } - function renounceAdmin(uint64) external pure { - revert("MockPolicyRegistry: not implemented"); + /// @inheritdoc IPolicyRegistry + function updateBlocklist(uint64 policyId, bool blocked, address[] calldata accounts) external { + uint256 packed = _requireCustom(policyId); + if (_typeFromId(policyId) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); + if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); + _batchSetMembers({policyId: policyId, policyType: PolicyType.BLOCKLIST, value: blocked, accounts: accounts}); } - function updateAllowlist(uint64, bool, address[] calldata) external pure { - revert("MockPolicyRegistry: not implemented"); + // ============================================================ + // AUTHORIZATION QUERIES + // ============================================================ + + /// @inheritdoc IPolicyRegistry + function isAuthorized(uint64 policyId, address account) external view returns (bool) { + // Built-in short-circuits MUST precede any storage read: IDs 0 and 1 + // have no entry in _policies and must never reach the storage path. + if (policyId == ALWAYS_ALLOW_ID) return true; + if (policyId == ALWAYS_BLOCK_ID) return false; + uint256 packed = _policies[policyId]; + if ((packed & CREATED_BIT) == 0) revert PolicyNotFound(); + bool member = _members[policyId][account]; + return _typeFromId(policyId) == PolicyType.ALLOWLIST ? member : !member; } - function updateBlocklist(uint64, bool, address[] calldata) external pure { - revert("MockPolicyRegistry: not implemented"); + // ============================================================ + // POLICY QUERIES + // ============================================================ + + /// @inheritdoc IPolicyRegistry + function nextPolicyId(PolicyType policyType) external view returns (uint64) { + return _makeId(policyType, _nextCounter); } - function nextPolicyId(PolicyType) external pure returns (uint64) { - revert("MockPolicyRegistry: not implemented"); + /// @inheritdoc IPolicyRegistry + function policyExists(uint64 policyId) external view returns (bool) { + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return true; + return (_policies[policyId] & CREATED_BIT) != 0; } - function policyType(uint64 policyId) external pure returns (PolicyType) { + /// @inheritdoc IPolicyRegistry + function policyType(uint64 policyId) external view returns (PolicyType) { if (policyId == ALWAYS_ALLOW_ID) return PolicyType.ALWAYS_ALLOW; if (policyId == ALWAYS_BLOCK_ID) return PolicyType.ALWAYS_BLOCK; - revert("MockPolicyRegistry: not implemented"); + if ((_policies[policyId] & CREATED_BIT) == 0) revert PolicyNotFound(); + return _typeFromId(policyId); + } + + /// @inheritdoc IPolicyRegistry + function policyAdmin(uint64 policyId) external view returns (address) { + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return address(0); + uint256 packed = _policies[policyId]; + if ((packed & CREATED_BIT) == 0) revert PolicyNotFound(); + return _decodeAdmin(packed); + } + + /// @inheritdoc IPolicyRegistry + function pendingPolicyAdmin(uint64 policyId) external view returns (address) { + if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return address(0); + return _pendingAdmins[policyId]; + } + + // ============================================================ + // INTERNAL HELPERS + // ============================================================ + + function _create(address admin, PolicyType policyType) internal returns (uint64 newPolicyId) { + if (policyType != PolicyType.ALLOWLIST && policyType != PolicyType.BLOCKLIST) revert InvalidPolicyType(); + if (admin == address(0)) revert ZeroAddress(); + uint56 counter = _nextCounter; + // No overflow guard: at one policy per 2-second block, exhausting the + // 56-bit counter space (~7.2e16 values) takes ~4.6 billion years. + unchecked { + _nextCounter = counter + 1; + } + newPolicyId = _makeId(policyType, counter); + _policies[newPolicyId] = _encode(admin); + emit PolicyCreated(newPolicyId, msg.sender, policyType); + emit PolicyAdminUpdated(newPolicyId, address(0), admin); + } + + function _batchSetMembers(uint64 policyId, PolicyType policyType, bool value, address[] calldata accounts) + internal + { + mapping(address => bool) storage members = _members[policyId]; + for (uint256 i = 0; i < accounts.length; ++i) { + members[accounts[i]] = value; + } + if (policyType == PolicyType.ALLOWLIST) { + emit AllowlistUpdated(policyId, msg.sender, value, accounts); + } else { + emit BlocklistUpdated(policyId, msg.sender, value, accounts); + } + } + + function _requireCustom(uint64 policyId) internal view returns (uint256 packed) { + packed = _policies[policyId]; + if ((packed & CREATED_BIT) == 0) revert PolicyNotFound(); + } + + function _makeId(PolicyType policyType, uint56 counter) internal pure returns (uint64) { + return (uint64(uint8(policyType)) << TYPE_SHIFT) | uint64(counter); + } + + function _typeFromId(uint64 policyId) internal pure returns (PolicyType) { + // forge-lint: disable-next-line(unsafe-typecast) + return PolicyType(uint8(policyId >> TYPE_SHIFT)); } - function policyAdmin(uint64) external pure returns (address) { - revert("MockPolicyRegistry: not implemented"); + function _encode(address admin) internal pure returns (uint256) { + return CREATED_BIT | (uint256(uint160(admin)) << ADMIN_SHIFT); } - function pendingPolicyAdmin(uint64) external pure returns (address) { - revert("MockPolicyRegistry: not implemented"); + function _decodeAdmin(uint256 packed) internal pure returns (address) { + // forge-lint: disable-next-line(unsafe-typecast) + return address(uint160(packed >> ADMIN_SHIFT)); } } diff --git a/test/unit/PolicyRegistry/isAuthorized.t.sol b/test/unit/PolicyRegistry/isAuthorized.t.sol index e0c4950..1e60e24 100644 --- a/test/unit/PolicyRegistry/isAuthorized.t.sol +++ b/test/unit/PolicyRegistry/isAuthorized.t.sol @@ -16,9 +16,9 @@ contract PolicyRegistryIsAuthorizedTest is PolicyRegistryTest { // unimplemented } - /// @notice Verifies isAuthorized returns false for any account under built-in id type(uint64).max - /// @dev Built-in sentinel semantics: id uint64.max returns false unconditionally - function test_isAuthorized_success_alwaysRejectBuiltin(address account) public { + /// @notice Verifies isAuthorized returns false for any account under built-in id 1 (always-block) + /// @dev Built-in sentinel semantics: id 1 returns false unconditionally + function test_isAuthorized_success_alwaysBlockBuiltin(address account) public { // unimplemented } diff --git a/test/unit/PolicyRegistry/nextPolicyId.t.sol b/test/unit/PolicyRegistry/nextPolicyId.t.sol index d8c624f..2b1d601 100644 --- a/test/unit/PolicyRegistry/nextPolicyId.t.sol +++ b/test/unit/PolicyRegistry/nextPolicyId.t.sol @@ -3,43 +3,40 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryNextPolicyIdTest is PolicyRegistryTest { - /// @notice Verifies nextPolicyId(ALLOWLIST) returns the first ALLOWLIST-encoded id - /// the next createPolicy(_, ALLOWLIST) would assign - /// @dev Per the policy ID encoding scheme (top byte = type discriminator, - /// ALLOWLIST counter skips local-id 0 to avoid colliding with the - /// always-allow built-in at encoded id 0), the first allowlist id is - /// `(uint64(uint8(PolicyType.ALLOWLIST)) << 56) | 1` + /// @notice Verifies nextPolicyId(ALLOWLIST) returns the correct initial encoded id + /// @dev Global counter starts at 2. The first ALLOWLIST id is + /// `(uint64(uint8(PolicyType.ALLOWLIST)) << 56) | 2` + /// i.e. discriminator 0x02 in the top byte, counter value 2 in the low 56 bits. function test_nextPolicyId_success_allowlistInitialEncoded() public { // unimplemented } - /// @notice Verifies nextPolicyId(BLOCKLIST) returns the first BLOCKLIST-encoded id - /// the next createPolicy(_, BLOCKLIST) would assign - /// @dev Per the policy ID encoding scheme, the first blocklist id is - /// `(uint64(uint8(PolicyType.BLOCKLIST)) << 56) | 0` — no skip on - /// non-ALLOWLIST counters (only ALLOWLIST collides with built-in 0) + /// @notice Verifies nextPolicyId(BLOCKLIST) returns the correct initial encoded id + /// @dev Global counter starts at 2. The first BLOCKLIST id is + /// `(uint64(uint8(PolicyType.BLOCKLIST)) << 56) | 2` + /// i.e. discriminator 0x03 in the top byte, counter value 2 in the low 56 bits. function test_nextPolicyId_success_blocklistInitialEncoded() public { // unimplemented } - /// @notice Verifies nextPolicyId(type) advances by one (local-id) per - /// successful createPolicy(_, type) call - /// @dev Per-type monotonic counter; check returned id equals the prior - /// nextPolicyId(type) value and that the top-byte discriminator stays - /// stable across the sequence. policyTypeRaw is bounded inside the - /// body to `< 2` (the count of PolicyType enum values) via vm.assume - /// before being cast — direct enum-typed parameters cause the fuzzer - /// to revert at function entry on out-of-range uint8 inputs. + /// @notice Verifies nextPolicyId advances by one per createPolicy call regardless of type + /// @dev Single global counter: each createPolicy call increments it once. + /// The top-byte discriminator reflects the type passed to nextPolicyId; + /// the low 56 bits advance monotonically regardless of which type was created. + /// policyTypeRaw is bounded to ALLOWLIST (2) or BLOCKLIST (3) via vm.assume. function test_nextPolicyId_success_advancesPerCreate(uint8 policyTypeRaw, uint8 count) public { // unimplemented } - /// @notice Verifies nextPolicyId(ALLOWLIST) and nextPolicyId(BLOCKLIST) - /// advance independently - /// @dev Per-type counters do not share state; creating an allowlist must - /// not affect the next-blocklist-id value and vice versa - function test_nextPolicyId_success_perTypeCountersIndependent(uint8 allowCount, uint8 blockCount) public { + /// @notice Verifies creating one type advances nextPolicyId for the other type + /// @dev The global counter is shared: creating an ALLOWLIST policy increments the + /// counter that BLOCKLIST uses, and vice versa. nextPolicyId(ALLOWLIST) and + /// nextPolicyId(BLOCKLIST) always differ only in their top byte — their low + /// 56 bits are identical at any given point in time. + function test_nextPolicyId_success_globalCounterSharedAcrossTypes(uint8 allowCount, uint8 blockCount) public { // unimplemented } } diff --git a/test/unit/PolicyRegistry/policyAdmin.t.sol b/test/unit/PolicyRegistry/policyAdmin.t.sol index f6188a0..b37eda4 100644 --- a/test/unit/PolicyRegistry/policyAdmin.t.sol +++ b/test/unit/PolicyRegistry/policyAdmin.t.sol @@ -11,7 +11,7 @@ contract PolicyRegistryPolicyAdminTest is PolicyRegistryTest { } /// @notice Verifies policyAdmin returns address(0) for built-in policies - /// @dev Built-ins have no admin; both id 0 and id type(uint64).max return zero + /// @dev Built-ins have no admin; both id 0 and id 1 return zero function test_policyAdmin_success_zeroForBuiltins() public { // unimplemented } diff --git a/test/unit/PolicyRegistry/policyExists.t.sol b/test/unit/PolicyRegistry/policyExists.t.sol index 9c02f6f..d94c9b1 100644 --- a/test/unit/PolicyRegistry/policyExists.t.sol +++ b/test/unit/PolicyRegistry/policyExists.t.sol @@ -10,9 +10,9 @@ contract PolicyRegistryPolicyExistsTest is PolicyRegistryTest { // unimplemented } - /// @notice Verifies policyExists returns true for built-in id type(uint64).max - /// @dev Always-reject built-in is always present - function test_policyExists_success_builtinMax() public { + /// @notice Verifies policyExists returns true for built-in id 1 (always-block) + /// @dev Always-block built-in is always present + function test_policyExists_success_builtinOne() public { // unimplemented } From cc57c28b6d94ec85a0e880351924239419dfb4c7 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 19 May 2026 23:31:48 -0400 Subject: [PATCH 10/17] Implement PolicyRegistry unit tests (75 tests, all passing) Fills in all 13 test stubs across createPolicy, createPolicyWithAccounts, stageUpdateAdmin, finalizeUpdateAdmin, renounceAdmin, updateAllowlist, updateBlocklist, isAuthorized, nextPolicyId, policyExists, policyType, policyAdmin, and pendingPolicyAdmin. Also fixes _nextCounter initialization: vm.etch does not run the constructor so storage state variable initializers are not applied. Counter starts at 0; the discriminator byte (0x02/0x03) ensures custom IDs can never equal built-in IDs 0 or 1. --- test/lib/mocks/MockPolicyRegistry.sol | 6 +- test/unit/PolicyRegistry/createPolicy.t.sol | 78 +++++++++++++++++-- .../createPolicyWithAccounts.t.sol | 72 ++++++++++++++--- .../PolicyRegistry/finalizeUpdateAdmin.t.sol | 52 +++++++++++-- test/unit/PolicyRegistry/isAuthorized.t.sol | 34 +++++--- test/unit/PolicyRegistry/nextPolicyId.t.sol | 62 ++++++++++----- .../PolicyRegistry/pendingPolicyAdmin.t.sol | 32 ++++++-- test/unit/PolicyRegistry/policyAdmin.t.sol | 21 +++-- test/unit/PolicyRegistry/policyExists.t.sol | 28 ++++--- test/unit/PolicyRegistry/policyType.t.sol | 20 ++++- test/unit/PolicyRegistry/renounceAdmin.t.sol | 55 +++++++++++-- .../PolicyRegistry/stageUpdateAdmin.t.sol | 47 +++++++++-- .../unit/PolicyRegistry/updateAllowlist.t.sol | 68 +++++++++++++--- .../unit/PolicyRegistry/updateBlocklist.t.sol | 68 +++++++++++++--- 14 files changed, 527 insertions(+), 116 deletions(-) diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index 02ae6c1..027bc85 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -70,8 +70,10 @@ contract MockPolicyRegistry is IPolicyRegistry { mapping(uint64 policyId => address pendingAdmin) private _pendingAdmins; // Global monotonic counter for the low 56 bits of custom policy IDs. - // Starts at 2 to defensively skip the numeric values of the built-in IDs. - uint56 private _nextCounter = 2; + // Starts at 0. The discriminator byte in bits [63:56] ensures no custom ID + // can equal built-in 0 or 1 regardless of counter value (ALLOWLIST = 0x02, + // BLOCKLIST = 0x03, so the minimum custom ID is 0x0200000000000000). + uint56 private _nextCounter; // ============================================================ // POLICY CREATION diff --git a/test/unit/PolicyRegistry/createPolicy.t.sol b/test/unit/PolicyRegistry/createPolicy.t.sol index 2627e3a..97b2f07 100644 --- a/test/unit/PolicyRegistry/createPolicy.t.sol +++ b/test/unit/PolicyRegistry/createPolicy.t.sol @@ -3,29 +3,55 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryCreatePolicyTest is PolicyRegistryTest { /// @notice Verifies createPolicy reverts when admin is the zero address /// @dev Required-field guard; checks ZeroAddress() error function test_createPolicy_revert_zeroAdmin(address caller, uint8 policyTypeInt) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(policyTypeInt == 2 || policyTypeInt == 3); + IPolicyRegistry.PolicyType pt = IPolicyRegistry.PolicyType(policyTypeInt); + vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); + vm.prank(caller); + policyRegistry.createPolicy(address(0), pt); } /// @notice Verifies createPolicy reverts for any policyType value outside the enum /// @dev Fuzz confirms only ALLOWLIST / BLOCKLIST are accepted; checks InvalidPolicyType() error function test_createPolicy_revert_invalidPolicyType(address caller, address admin_, uint8 policyTypeInt) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(policyTypeInt != 2 && policyTypeInt != 3); + vm.assume(policyTypeInt < 4); // stay within valid enum cast range + IPolicyRegistry.PolicyType invalidType = IPolicyRegistry.PolicyType(policyTypeInt); + vm.expectRevert(IPolicyRegistry.InvalidPolicyType.selector); + vm.prank(caller); + policyRegistry.createPolicy(admin_, invalidType); } /// @notice Verifies createPolicy assigns a fresh allowlist policy id /// @dev Type, admin, and existence all readable post-creation function test_createPolicy_success_allowlist(address caller, address admin_) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.prank(caller); + uint64 policyId = policyRegistry.createPolicy(admin_, IPolicyRegistry.PolicyType.ALLOWLIST); + assertTrue(policyRegistry.policyExists(policyId)); + assertEq(uint8(policyRegistry.policyType(policyId)), uint8(IPolicyRegistry.PolicyType.ALLOWLIST)); + assertEq(policyRegistry.policyAdmin(policyId), admin_); } /// @notice Verifies createPolicy assigns a fresh blocklist policy id /// @dev Type, admin, and existence all readable post-creation function test_createPolicy_success_blocklist(address caller, address admin_) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.prank(caller); + uint64 policyId = policyRegistry.createPolicy(admin_, IPolicyRegistry.PolicyType.BLOCKLIST); + assertTrue(policyRegistry.policyExists(policyId)); + assertEq(uint8(policyRegistry.policyType(policyId)), uint8(IPolicyRegistry.PolicyType.BLOCKLIST)); + assertEq(policyRegistry.policyAdmin(policyId), admin_); } /// @notice Verifies the returned policy id advances nextPolicyId monotonically @@ -33,13 +59,42 @@ contract PolicyRegistryCreatePolicyTest is PolicyRegistryTest { function test_createPolicy_success_advancesNextPolicyId(address caller, address admin_, uint8 typeA, uint8 typeB) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(typeA == 2 || typeA == 3); + vm.assume(typeB == 2 || typeB == 3); + IPolicyRegistry.PolicyType ptA = IPolicyRegistry.PolicyType(typeA); + IPolicyRegistry.PolicyType ptB = IPolicyRegistry.PolicyType(typeB); + + uint64 predictedA = policyRegistry.nextPolicyId(ptA); + vm.prank(caller); + uint64 idA = policyRegistry.createPolicy(admin_, ptA); + assertEq(idA, predictedA); + + uint64 predictedB = policyRegistry.nextPolicyId(ptB); + vm.prank(caller); + uint64 idB = policyRegistry.createPolicy(admin_, ptB); + assertEq(idB, predictedB); + + assertTrue(idA != idB); + // Low 56 bits advance by exactly 1 between any two consecutive creates. + uint64 counterMask = (uint64(1) << 56) - 1; + assertEq((idA & counterMask) + 1, idB & counterMask); } /// @notice Verifies createPolicy emits PolicyCreated with the correct args /// @dev Event integrity: policyId, creator, policyType match the call function test_createPolicy_success_emitsPolicyCreated(address caller, address admin_, uint8 policyTypeInt) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(policyTypeInt == 2 || policyTypeInt == 3); + IPolicyRegistry.PolicyType pt = IPolicyRegistry.PolicyType(policyTypeInt); + + uint64 expectedId = policyRegistry.nextPolicyId(pt); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.PolicyCreated(expectedId, caller, pt); + vm.prank(caller); + policyRegistry.createPolicy(admin_, pt); } /// @notice Verifies createPolicy emits PolicyAdminUpdated(previousAdmin = 0) on initial assignment @@ -49,6 +104,15 @@ contract PolicyRegistryCreatePolicyTest is PolicyRegistryTest { address admin_, uint8 policyTypeInt ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(policyTypeInt == 2 || policyTypeInt == 3); + IPolicyRegistry.PolicyType pt = IPolicyRegistry.PolicyType(policyTypeInt); + + uint64 expectedId = policyRegistry.nextPolicyId(pt); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.PolicyAdminUpdated(expectedId, address(0), admin_); + vm.prank(caller); + policyRegistry.createPolicy(admin_, pt); } } diff --git a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol index f703a60..4c355ee 100644 --- a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol +++ b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol @@ -3,15 +3,18 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { /// @notice Verifies createPolicyWithAccounts reverts when admin is the zero address /// @dev Required-field guard; checks ZeroAddress() error - function test_createPolicyWithAccounts_revert_zeroAdmin( - address caller, - uint8 policyTypeInt, - address[] memory accounts - ) public { - // unimplemented + function test_createPolicyWithAccounts_revert_zeroAdmin(address caller, address[] memory accounts) public { + _assumeValidCaller(caller); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } + vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); + vm.prank(caller); + policyRegistry.createPolicyWithAccounts(address(0), IPolicyRegistry.PolicyType.ALLOWLIST, accounts); } /// @notice Verifies createPolicyWithAccounts reverts for any policyType outside the enum @@ -22,7 +25,15 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { uint8 policyTypeInt, address[] memory accounts ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(policyTypeInt != 2 && policyTypeInt != 3); + vm.assume(policyTypeInt < 4); + vm.assume(accounts.length <= 5); + IPolicyRegistry.PolicyType invalidType = IPolicyRegistry.PolicyType(policyTypeInt); + vm.expectRevert(IPolicyRegistry.InvalidPolicyType.selector); + vm.prank(caller); + policyRegistry.createPolicyWithAccounts(admin_, invalidType, accounts); } /// @notice Verifies createPolicyWithAccounts seeds an allowlist policy with the provided members @@ -32,7 +43,15 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { address admin_, address[] memory accounts ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(accounts.length <= 5); + vm.prank(caller); + uint64 policyId = + policyRegistry.createPolicyWithAccounts(admin_, IPolicyRegistry.PolicyType.ALLOWLIST, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + assertTrue(policyRegistry.isAuthorized(policyId, accounts[i])); + } } /// @notice Verifies createPolicyWithAccounts seeds a blocklist policy with the provided members @@ -42,7 +61,15 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { address admin_, address[] memory accounts ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(accounts.length <= 5); + vm.prank(caller); + uint64 policyId = + policyRegistry.createPolicyWithAccounts(admin_, IPolicyRegistry.PolicyType.BLOCKLIST, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + assertFalse(policyRegistry.isAuthorized(policyId, accounts[i])); + } } /// @notice Verifies the seeding step on an allowlist policy emits AllowlistUpdated with the full batch @@ -52,7 +79,14 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { address admin_, address[] memory accounts ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(accounts.length <= 5); + uint64 expectedId = policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.AllowlistUpdated(expectedId, caller, true, accounts); + vm.prank(caller); + policyRegistry.createPolicyWithAccounts(admin_, IPolicyRegistry.PolicyType.ALLOWLIST, accounts); } /// @notice Verifies the seeding step on a blocklist policy emits BlocklistUpdated with the full batch @@ -62,7 +96,14 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { address admin_, address[] memory accounts ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(accounts.length <= 5); + uint64 expectedId = policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.BLOCKLIST); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.BlocklistUpdated(expectedId, caller, true, accounts); + vm.prank(caller); + policyRegistry.createPolicyWithAccounts(admin_, IPolicyRegistry.PolicyType.BLOCKLIST, accounts); } /// @notice Verifies createPolicyWithAccounts succeeds with an empty accounts array @@ -70,6 +111,13 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { function test_createPolicyWithAccounts_success_emptyAccounts(address caller, address admin_, uint8 policyTypeInt) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + vm.assume(policyTypeInt == 2 || policyTypeInt == 3); + IPolicyRegistry.PolicyType pt = IPolicyRegistry.PolicyType(policyTypeInt); + address[] memory empty = new address[](0); + vm.prank(caller); + uint64 policyId = policyRegistry.createPolicyWithAccounts(admin_, pt, empty); + assertTrue(policyRegistry.policyExists(policyId)); } } diff --git a/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol b/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol index 8838613..7e6d597 100644 --- a/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol +++ b/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol @@ -3,41 +3,81 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryFinalizeUpdateAdminTest is PolicyRegistryTest { /// @notice Verifies finalizeUpdateAdmin reverts when no admin transfer is in flight /// @dev Pending-slot precondition; checks NoPendingAdmin() error function test_finalizeUpdateAdmin_revert_noPendingAdmin(address caller) public { - // unimplemented + _assumeValidCaller(caller); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectRevert(IPolicyRegistry.NoPendingAdmin.selector); + vm.prank(caller); + policyRegistry.finalizeUpdateAdmin(policyId); } /// @notice Verifies finalizeUpdateAdmin reverts for callers other than the staged pending admin /// @dev Access control: only the pending admin can claim; checks Unauthorized() error function test_finalizeUpdateAdmin_revert_unauthorized(address pending, address caller) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(pending != address(0)); + vm.assume(caller != pending); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(admin); + policyRegistry.stageUpdateAdmin(policyId, pending); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(caller); + policyRegistry.finalizeUpdateAdmin(policyId); } /// @notice Verifies finalizeUpdateAdmin reverts for an unknown policy id /// @dev Built-ins and unknown ids are not administrable; checks PolicyNotFound() error function test_finalizeUpdateAdmin_revert_policyNotFound(address caller, uint64 policyId) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(policyId > 1); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(caller); + policyRegistry.finalizeUpdateAdmin(policyId); } /// @notice Verifies finalizeUpdateAdmin promotes the pending admin to current admin /// @dev policyAdmin returns the previously-staged address after this call function test_finalizeUpdateAdmin_success_promotesPending(address currentAdmin, address newAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(newAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); + vm.prank(newAdmin); + policyRegistry.finalizeUpdateAdmin(policyId); + assertEq(policyRegistry.policyAdmin(policyId), newAdmin); } /// @notice Verifies finalizeUpdateAdmin clears the pending slot /// @dev pendingPolicyAdmin returns address(0) after the transfer completes function test_finalizeUpdateAdmin_success_clearsPending(address currentAdmin, address newAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(newAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); + vm.prank(newAdmin); + policyRegistry.finalizeUpdateAdmin(policyId); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } /// @notice Verifies finalizeUpdateAdmin emits PolicyAdminUpdated with previousAdmin and newAdmin /// @dev Canonical PolicyAdminUpdated event test; other emission paths (create / renounce) are /// tested in their own files because their args differ (previousAdmin = 0 / newAdmin = 0) function test_finalizeUpdateAdmin_success_emitsPolicyAdminUpdated(address currentAdmin, address newAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(newAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.PolicyAdminUpdated(policyId, currentAdmin, newAdmin); + vm.prank(newAdmin); + policyRegistry.finalizeUpdateAdmin(policyId); } } diff --git a/test/unit/PolicyRegistry/isAuthorized.t.sol b/test/unit/PolicyRegistry/isAuthorized.t.sol index 1e60e24..46a74d8 100644 --- a/test/unit/PolicyRegistry/isAuthorized.t.sol +++ b/test/unit/PolicyRegistry/isAuthorized.t.sol @@ -3,46 +3,62 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryIsAuthorizedTest is PolicyRegistryTest { /// @notice Verifies isAuthorized reverts for an unknown policy id /// @dev Lookup guard for non-existent ids; checks PolicyNotFound() error function test_isAuthorized_revert_policyNotFound(uint64 policyId, address account) public { - // unimplemented + vm.assume(policyId > 1); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + policyRegistry.isAuthorized(policyId, account); } /// @notice Verifies isAuthorized returns true for any account under built-in id 0 (always-allow) /// @dev Built-in sentinel semantics: id 0 returns true unconditionally - function test_isAuthorized_success_alwaysAllowBuiltin(address account) public { - // unimplemented + function test_isAuthorized_success_alwaysAllowBuiltin(address account) public view { + assertTrue(policyRegistry.isAuthorized(0, account)); } /// @notice Verifies isAuthorized returns false for any account under built-in id 1 (always-block) /// @dev Built-in sentinel semantics: id 1 returns false unconditionally - function test_isAuthorized_success_alwaysBlockBuiltin(address account) public { - // unimplemented + function test_isAuthorized_success_alwaysBlockBuiltin(address account) public view { + assertFalse(policyRegistry.isAuthorized(1, account)); } /// @notice Verifies isAuthorized returns true for an allowlist member /// @dev Allowlist semantics: membership grants authorization function test_isAuthorized_success_allowlistMember(address account) public { - // unimplemented + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + address[] memory accounts = new address[](1); + accounts[0] = account; + vm.prank(admin); + policyRegistry.updateAllowlist(policyId, true, accounts); + assertTrue(policyRegistry.isAuthorized(policyId, account)); } /// @notice Verifies isAuthorized returns false for a non-member of an allowlist /// @dev Allowlist semantics: absence denies authorization function test_isAuthorized_success_allowlistNonMember(address account) public { - // unimplemented + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + assertFalse(policyRegistry.isAuthorized(policyId, account)); } /// @notice Verifies isAuthorized returns false for a blocklist member /// @dev Blocklist semantics: membership denies authorization function test_isAuthorized_success_blocklistMember(address account) public { - // unimplemented + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + address[] memory accounts = new address[](1); + accounts[0] = account; + vm.prank(admin); + policyRegistry.updateBlocklist(policyId, true, accounts); + assertFalse(policyRegistry.isAuthorized(policyId, account)); } /// @notice Verifies isAuthorized returns true for a non-member of a blocklist /// @dev Blocklist semantics: absence grants authorization function test_isAuthorized_success_blocklistNonMember(address account) public { - // unimplemented + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + assertTrue(policyRegistry.isAuthorized(policyId, account)); } } diff --git a/test/unit/PolicyRegistry/nextPolicyId.t.sol b/test/unit/PolicyRegistry/nextPolicyId.t.sol index 2b1d601..fa20540 100644 --- a/test/unit/PolicyRegistry/nextPolicyId.t.sol +++ b/test/unit/PolicyRegistry/nextPolicyId.t.sol @@ -6,37 +6,63 @@ import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; contract PolicyRegistryNextPolicyIdTest is PolicyRegistryTest { + uint256 private constant TYPE_SHIFT = 56; + /// @notice Verifies nextPolicyId(ALLOWLIST) returns the correct initial encoded id - /// @dev Global counter starts at 2. The first ALLOWLIST id is - /// `(uint64(uint8(PolicyType.ALLOWLIST)) << 56) | 2` - /// i.e. discriminator 0x02 in the top byte, counter value 2 in the low 56 bits. - function test_nextPolicyId_success_allowlistInitialEncoded() public { - // unimplemented + /// @dev Global counter starts at 0. The first ALLOWLIST id is + /// `(uint64(uint8(PolicyType.ALLOWLIST)) << 56) | 0` + /// The discriminator byte (0x02) ensures this never equals built-in IDs 0 or 1. + function test_nextPolicyId_success_allowlistInitialEncoded() public view { + uint64 expected = (uint64(uint8(IPolicyRegistry.PolicyType.ALLOWLIST)) << TYPE_SHIFT) | 0; + assertEq(policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.ALLOWLIST), expected); } /// @notice Verifies nextPolicyId(BLOCKLIST) returns the correct initial encoded id - /// @dev Global counter starts at 2. The first BLOCKLIST id is - /// `(uint64(uint8(PolicyType.BLOCKLIST)) << 56) | 2` - /// i.e. discriminator 0x03 in the top byte, counter value 2 in the low 56 bits. - function test_nextPolicyId_success_blocklistInitialEncoded() public { - // unimplemented + /// @dev Global counter starts at 0. The first BLOCKLIST id is + /// `(uint64(uint8(PolicyType.BLOCKLIST)) << 56) | 0` + function test_nextPolicyId_success_blocklistInitialEncoded() public view { + uint64 expected = (uint64(uint8(IPolicyRegistry.PolicyType.BLOCKLIST)) << TYPE_SHIFT) | 0; + assertEq(policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.BLOCKLIST), expected); } /// @notice Verifies nextPolicyId advances by one per createPolicy call regardless of type /// @dev Single global counter: each createPolicy call increments it once. - /// The top-byte discriminator reflects the type passed to nextPolicyId; - /// the low 56 bits advance monotonically regardless of which type was created. /// policyTypeRaw is bounded to ALLOWLIST (2) or BLOCKLIST (3) via vm.assume. function test_nextPolicyId_success_advancesPerCreate(uint8 policyTypeRaw, uint8 count) public { - // unimplemented + vm.assume(policyTypeRaw == 2 || policyTypeRaw == 3); + count = uint8(bound(count, 0, 10)); + IPolicyRegistry.PolicyType pt = IPolicyRegistry.PolicyType(policyTypeRaw); + + for (uint256 i = 0; i < count; ++i) { + uint64 predicted = policyRegistry.nextPolicyId(pt); + uint64 assigned = policyRegistry.createPolicy(admin, pt); + assertEq(assigned, predicted); + } } /// @notice Verifies creating one type advances nextPolicyId for the other type - /// @dev The global counter is shared: creating an ALLOWLIST policy increments the - /// counter that BLOCKLIST uses, and vice versa. nextPolicyId(ALLOWLIST) and - /// nextPolicyId(BLOCKLIST) always differ only in their top byte — their low - /// 56 bits are identical at any given point in time. + /// @dev The global counter is shared: nextPolicyId(ALLOWLIST) and nextPolicyId(BLOCKLIST) + /// always differ only in their top byte — their low 56 bits are identical. function test_nextPolicyId_success_globalCounterSharedAcrossTypes(uint8 allowCount, uint8 blockCount) public { - // unimplemented + allowCount = uint8(bound(allowCount, 0, 5)); + blockCount = uint8(bound(blockCount, 0, 5)); + + for (uint256 i = 0; i < allowCount; ++i) { + policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + } + for (uint256 i = 0; i < blockCount; ++i) { + policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + } + + uint64 nextAllow = policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.ALLOWLIST); + uint64 nextBlock = policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.BLOCKLIST); + + // Low 56 bits are identical — both types share the same global counter. + uint64 counterMask = (uint64(1) << 56) - 1; + assertEq(nextAllow & counterMask, nextBlock & counterMask); + + // Top bytes differ by type discriminator. + assertEq(uint8(nextAllow >> 56), uint8(IPolicyRegistry.PolicyType.ALLOWLIST)); + assertEq(uint8(nextBlock >> 56), uint8(IPolicyRegistry.PolicyType.BLOCKLIST)); } } diff --git a/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol b/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol index 3e5c32d..fb1e551 100644 --- a/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol +++ b/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol @@ -3,34 +3,54 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryPendingPolicyAdminTest is PolicyRegistryTest { /// @notice Verifies pendingPolicyAdmin returns address(0) before any transfer is staged /// @dev Default state for a freshly-created policy function test_pendingPolicyAdmin_success_defaultZero() public { - // unimplemented + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } /// @notice Verifies pendingPolicyAdmin returns the address most recently staged /// @dev Read-after-write for stageUpdateAdmin function test_pendingPolicyAdmin_success_returnsStaged(address newAdmin) public { - // unimplemented + vm.assume(newAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(admin); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), newAdmin); } /// @notice Verifies pendingPolicyAdmin returns address(0) after finalizeUpdateAdmin /// @dev Pending slot is cleared once the transfer completes function test_pendingPolicyAdmin_success_zeroAfterFinalize(address newAdmin) public { - // unimplemented + vm.assume(newAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(admin); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); + vm.prank(newAdmin); + policyRegistry.finalizeUpdateAdmin(policyId); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } /// @notice Verifies pendingPolicyAdmin returns address(0) after renounceAdmin /// @dev In-flight transfers are invalidated as a side effect of renouncement function test_pendingPolicyAdmin_success_zeroAfterRenounce(address pending) public { - // unimplemented + vm.assume(pending != address(0)); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(admin); + policyRegistry.stageUpdateAdmin(policyId, pending); + vm.prank(admin); + policyRegistry.renounceAdmin(policyId); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } /// @notice Verifies pendingPolicyAdmin returns address(0) for built-in policies /// @dev Built-ins have no admin and therefore no pending admin - function test_pendingPolicyAdmin_success_zeroForBuiltins() public { - // unimplemented + function test_pendingPolicyAdmin_success_zeroForBuiltins() public view { + assertEq(policyRegistry.pendingPolicyAdmin(0), address(0)); + assertEq(policyRegistry.pendingPolicyAdmin(1), address(0)); } } diff --git a/test/unit/PolicyRegistry/policyAdmin.t.sol b/test/unit/PolicyRegistry/policyAdmin.t.sol index b37eda4..0e85144 100644 --- a/test/unit/PolicyRegistry/policyAdmin.t.sol +++ b/test/unit/PolicyRegistry/policyAdmin.t.sol @@ -3,28 +3,39 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryPolicyAdminTest is PolicyRegistryTest { /// @notice Verifies policyAdmin reverts for an unknown policy id /// @dev Lookup guard for non-existent ids; checks PolicyNotFound() error function test_policyAdmin_revert_policyNotFound(uint64 policyId) public { - // unimplemented + vm.assume(policyId > 1); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + policyRegistry.policyAdmin(policyId); } /// @notice Verifies policyAdmin returns address(0) for built-in policies /// @dev Built-ins have no admin; both id 0 and id 1 return zero - function test_policyAdmin_success_zeroForBuiltins() public { - // unimplemented + function test_policyAdmin_success_zeroForBuiltins() public view { + assertEq(policyRegistry.policyAdmin(0), address(0)); + assertEq(policyRegistry.policyAdmin(1), address(0)); } /// @notice Verifies policyAdmin returns the admin nominated at creation time /// @dev Initial-admin readback function test_policyAdmin_success_returnsAssigned(address admin_) public { - // unimplemented + vm.assume(admin_ != address(0)); + uint64 policyId = policyRegistry.createPolicy(admin_, IPolicyRegistry.PolicyType.ALLOWLIST); + assertEq(policyRegistry.policyAdmin(policyId), admin_); } /// @notice Verifies policyAdmin returns address(0) after renounceAdmin /// @dev Post-renounce: admin slot is permanently cleared function test_policyAdmin_success_zeroAfterRenounce(address admin_) public { - // unimplemented + vm.assume(admin_ != address(0)); + uint64 policyId = policyRegistry.createPolicy(admin_, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(admin_); + policyRegistry.renounceAdmin(policyId); + assertEq(policyRegistry.policyAdmin(policyId), address(0)); } } diff --git a/test/unit/PolicyRegistry/policyExists.t.sol b/test/unit/PolicyRegistry/policyExists.t.sol index d94c9b1..22d0a2c 100644 --- a/test/unit/PolicyRegistry/policyExists.t.sol +++ b/test/unit/PolicyRegistry/policyExists.t.sol @@ -3,28 +3,26 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryPolicyExistsTest is PolicyRegistryTest { - /// @notice Verifies policyExists returns true for built-in id 0 - /// @dev Always-allow built-in is always present - function test_policyExists_success_builtinZero() public { - // unimplemented + function test_policyExists_success_builtinZero() public view { + assertTrue(policyRegistry.policyExists(0)); } - /// @notice Verifies policyExists returns true for built-in id 1 (always-block) - /// @dev Always-block built-in is always present - function test_policyExists_success_builtinOne() public { - // unimplemented + function test_policyExists_success_builtinOne() public view { + assertTrue(policyRegistry.policyExists(1)); } - /// @notice Verifies policyExists returns false for any id that has not been created - /// @dev Fuzz across the custom id space outside the issued range - function test_policyExists_success_falseForUncreated(uint64 policyId) public { - // unimplemented + function test_policyExists_success_falseForUncreated(uint64 policyId) public view { + vm.assume(policyId > 1); + assertFalse(policyRegistry.policyExists(policyId)); } - /// @notice Verifies policyExists returns true for a freshly-created policy id - /// @dev Existence flips immediately on createPolicy function test_policyExists_success_trueAfterCreate(uint8 policyTypeInt) public { - // unimplemented + vm.assume(policyTypeInt == 2 || policyTypeInt == 3); + IPolicyRegistry.PolicyType pt = IPolicyRegistry.PolicyType(policyTypeInt); + uint64 policyId = policyRegistry.createPolicy(admin, pt); + assertTrue(policyRegistry.policyExists(policyId)); } } diff --git a/test/unit/PolicyRegistry/policyType.t.sol b/test/unit/PolicyRegistry/policyType.t.sol index 360ecb0..08185bf 100644 --- a/test/unit/PolicyRegistry/policyType.t.sol +++ b/test/unit/PolicyRegistry/policyType.t.sol @@ -3,22 +3,36 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryPolicyTypeTest is PolicyRegistryTest { /// @notice Verifies policyType reverts for an unknown policy id /// @dev Lookup guard for non-existent ids; checks PolicyNotFound() error function test_policyType_revert_policyNotFound(uint64 policyId) public { - // unimplemented + vm.assume(policyId > 1); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + policyRegistry.policyType(policyId); + } + + function test_policyType_success_returnsAlwaysAllowForBuiltinZero() public view { + assertEq(uint8(policyRegistry.policyType(0)), uint8(IPolicyRegistry.PolicyType.ALWAYS_ALLOW)); + } + + function test_policyType_success_returnsAlwaysBlockForBuiltinOne() public view { + assertEq(uint8(policyRegistry.policyType(1)), uint8(IPolicyRegistry.PolicyType.ALWAYS_BLOCK)); } /// @notice Verifies policyType returns ALLOWLIST for an allowlist policy /// @dev Type readback matches the value passed to createPolicy function test_policyType_success_returnsAllowlist() public { - // unimplemented + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + assertEq(uint8(policyRegistry.policyType(policyId)), uint8(IPolicyRegistry.PolicyType.ALLOWLIST)); } /// @notice Verifies policyType returns BLOCKLIST for a blocklist policy /// @dev Type readback matches the value passed to createPolicy function test_policyType_success_returnsBlocklist() public { - // unimplemented + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + assertEq(uint8(policyRegistry.policyType(policyId)), uint8(IPolicyRegistry.PolicyType.BLOCKLIST)); } } diff --git a/test/unit/PolicyRegistry/renounceAdmin.t.sol b/test/unit/PolicyRegistry/renounceAdmin.t.sol index 1a29990..e23d59e 100644 --- a/test/unit/PolicyRegistry/renounceAdmin.t.sol +++ b/test/unit/PolicyRegistry/renounceAdmin.t.sol @@ -3,40 +3,81 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryRenounceAdminTest is PolicyRegistryTest { /// @notice Verifies renounceAdmin reverts when called by any non-admin caller /// @dev Access control: only the current admin may renounce; checks Unauthorized() error function test_renounceAdmin_revert_unauthorized(address caller) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(caller != admin); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(caller); + policyRegistry.renounceAdmin(policyId); } /// @notice Verifies renounceAdmin reverts for an unknown policy id /// @dev Built-ins and unknown ids are not administrable; checks PolicyNotFound() error function test_renounceAdmin_revert_policyNotFound(address caller, uint64 policyId) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(policyId > 1); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(caller); + policyRegistry.renounceAdmin(policyId); } /// @notice Verifies renounceAdmin sets policyAdmin to address(0) /// @dev Admin slot cleared permanently; policy continues to exist function test_renounceAdmin_success_clearsAdmin(address currentAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.renounceAdmin(policyId); + assertEq(policyRegistry.policyAdmin(policyId), address(0)); + assertTrue(policyRegistry.policyExists(policyId)); } /// @notice Verifies renounceAdmin clears any in-flight pending admin /// @dev Side effect: previously-staged pending admin is invalidated function test_renounceAdmin_success_clearsPending(address currentAdmin, address pending) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(pending != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, pending); + vm.prank(currentAdmin); + policyRegistry.renounceAdmin(policyId); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } /// @notice Verifies renounceAdmin freezes membership and admin operations on the policy - /// @dev Post-renounce: stageUpdateAdmin / updateAllowlist / updateBlocklist all revert with Unauthorized + /// @dev Post-renounce: stageUpdateAdmin / updateAllowlist / updateBlocklist all revert Unauthorized function test_renounceAdmin_success_freezesMutation(address currentAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.renounceAdmin(policyId); + + address[] memory accounts = new address[](0); + + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, alice); + + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, true, accounts); } /// @notice Verifies renounceAdmin emits PolicyAdminUpdated with newAdmin = address(0) /// @dev Renouncement variant of PolicyAdminUpdated; canonical event test lives in finalizeUpdateAdmin.t.sol function test_renounceAdmin_success_emitsPolicyAdminUpdatedToZero(address currentAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.PolicyAdminUpdated(policyId, currentAdmin, address(0)); + vm.prank(currentAdmin); + policyRegistry.renounceAdmin(policyId); } } diff --git a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol index d55e5eb..3a3c455 100644 --- a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol +++ b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol @@ -3,23 +3,39 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryStageUpdateAdminTest is PolicyRegistryTest { /// @notice Verifies stageUpdateAdmin reverts when called by any non-admin caller /// @dev Access control: only the current admin may stage a transfer; checks Unauthorized() error function test_stageUpdateAdmin_revert_unauthorized(address caller, address newAdmin) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(caller != admin); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(caller); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); } /// @notice Verifies stageUpdateAdmin reverts for an unknown policy id /// @dev Built-ins and unknown ids are not administrable; checks PolicyNotFound() error function test_stageUpdateAdmin_revert_policyNotFound(address caller, uint64 policyId, address newAdmin) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(policyId > 1); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(caller); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); } /// @notice Verifies stageUpdateAdmin sets pendingPolicyAdmin to the nominated address /// @dev Pending slot updated; current admin unchanged until finalizeUpdateAdmin function test_stageUpdateAdmin_success_setsPending(address currentAdmin, address newAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), newAdmin); + assertEq(policyRegistry.policyAdmin(policyId), currentAdmin); } /// @notice Verifies a second stageUpdateAdmin overwrites a previously-staged candidate @@ -27,18 +43,37 @@ contract PolicyRegistryStageUpdateAdminTest is PolicyRegistryTest { function test_stageUpdateAdmin_success_overwritesPrior(address currentAdmin, address first, address second) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(first != second); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, first); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, second); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), second); } /// @notice Verifies stageUpdateAdmin(address(0)) clears a previously-staged candidate /// @dev Explicit cancel path; pendingPolicyAdmin returns address(0) after function test_stageUpdateAdmin_success_clearsPending(address currentAdmin, address first) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(first != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, first); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, address(0)); + assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } /// @notice Verifies stageUpdateAdmin emits PolicyAdminStaged with the correct args /// @dev Event integrity: policyId, currentAdmin, pendingAdmin match the call function test_stageUpdateAdmin_success_emitsPolicyAdminStaged(address currentAdmin, address newAdmin) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.PolicyAdminStaged(policyId, currentAdmin, newAdmin); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, newAdmin); } } diff --git a/test/unit/PolicyRegistry/updateAllowlist.t.sol b/test/unit/PolicyRegistry/updateAllowlist.t.sol index e74bd08..49f124c 100644 --- a/test/unit/PolicyRegistry/updateAllowlist.t.sol +++ b/test/unit/PolicyRegistry/updateAllowlist.t.sol @@ -3,11 +3,19 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { /// @notice Verifies updateAllowlist reverts when called by any non-admin caller /// @dev Access control: only the current admin may mutate membership; checks Unauthorized() error function test_updateAllowlist_revert_unauthorized(address caller, bool allowed, address[] memory accounts) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(caller != admin); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(caller); + policyRegistry.updateAllowlist(policyId, allowed, accounts); } /// @notice Verifies updateAllowlist reverts when invoked on a BLOCKLIST policy @@ -17,7 +25,12 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { bool allowed, address[] memory accounts ) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, allowed, accounts); } /// @notice Verifies updateAllowlist reverts for an unknown policy id @@ -28,27 +41,56 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { bool allowed, address[] memory accounts ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(policyId > 1); + vm.assume(accounts.length <= 5); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(caller); + policyRegistry.updateAllowlist(policyId, allowed, accounts); } /// @notice Verifies updateAllowlist(allowed = true) adds each account to the membership set /// @dev isAuthorized returns true for each added account afterward function test_updateAllowlist_success_addsAccounts(address currentAdmin, address[] memory accounts) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, true, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + assertTrue(policyRegistry.isAuthorized(policyId, accounts[i])); + } } /// @notice Verifies updateAllowlist(allowed = false) removes each account from the membership set /// @dev isAuthorized returns false for each removed account afterward function test_updateAllowlist_success_removesAccounts(address currentAdmin, address[] memory accounts) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, true, accounts); + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, false, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + assertFalse(policyRegistry.isAuthorized(policyId, accounts[i])); + } } /// @notice Verifies duplicate accounts within a single call are idempotent /// @dev Repeated entries do not change the final membership state - function test_updateAllowlist_success_idempotentDuplicates(address currentAdmin, address[] memory accounts) - public - { - // unimplemented + function test_updateAllowlist_success_idempotentDuplicates(address currentAdmin, address account) public { + vm.assume(currentAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + address[] memory duped = new address[](2); + duped[0] = account; + duped[1] = account; + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, true, duped); + assertTrue(policyRegistry.isAuthorized(policyId, account)); + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, false, duped); + assertFalse(policyRegistry.isAuthorized(policyId, account)); } /// @notice Verifies updateAllowlist emits a single AllowlistUpdated carrying the full batch @@ -58,6 +100,12 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { bool allowed, address[] memory accounts ) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.AllowlistUpdated(policyId, currentAdmin, allowed, accounts); + vm.prank(currentAdmin); + policyRegistry.updateAllowlist(policyId, allowed, accounts); } } diff --git a/test/unit/PolicyRegistry/updateBlocklist.t.sol b/test/unit/PolicyRegistry/updateBlocklist.t.sol index 1bd2372..a2cb2fb 100644 --- a/test/unit/PolicyRegistry/updateBlocklist.t.sol +++ b/test/unit/PolicyRegistry/updateBlocklist.t.sol @@ -3,11 +3,19 @@ pragma solidity ^0.8.20; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; + contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { /// @notice Verifies updateBlocklist reverts when called by any non-admin caller /// @dev Access control: only the current admin may mutate membership; checks Unauthorized() error function test_updateBlocklist_revert_unauthorized(address caller, bool blocked, address[] memory accounts) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(caller != admin); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + vm.expectRevert(IPolicyRegistry.Unauthorized.selector); + vm.prank(caller); + policyRegistry.updateBlocklist(policyId, blocked, accounts); } /// @notice Verifies updateBlocklist reverts when invoked on an ALLOWLIST policy @@ -17,7 +25,12 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { bool blocked, address[] memory accounts ) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); + vm.prank(currentAdmin); + policyRegistry.updateBlocklist(policyId, blocked, accounts); } /// @notice Verifies updateBlocklist reverts for an unknown policy id @@ -28,27 +41,56 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { bool blocked, address[] memory accounts ) public { - // unimplemented + _assumeValidCaller(caller); + vm.assume(policyId > 1); + vm.assume(accounts.length <= 5); + vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); + vm.prank(caller); + policyRegistry.updateBlocklist(policyId, blocked, accounts); } /// @notice Verifies updateBlocklist(blocked = true) adds each account to the membership set /// @dev isAuthorized returns false for each added account afterward function test_updateBlocklist_success_addsAccounts(address currentAdmin, address[] memory accounts) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); + vm.prank(currentAdmin); + policyRegistry.updateBlocklist(policyId, true, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + assertFalse(policyRegistry.isAuthorized(policyId, accounts[i])); + } } /// @notice Verifies updateBlocklist(blocked = false) removes each account from the membership set /// @dev isAuthorized returns true for each removed account afterward function test_updateBlocklist_success_removesAccounts(address currentAdmin, address[] memory accounts) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); + vm.prank(currentAdmin); + policyRegistry.updateBlocklist(policyId, true, accounts); + vm.prank(currentAdmin); + policyRegistry.updateBlocklist(policyId, false, accounts); + for (uint256 i = 0; i < accounts.length; ++i) { + assertTrue(policyRegistry.isAuthorized(policyId, accounts[i])); + } } /// @notice Verifies duplicate accounts within a single call are idempotent /// @dev Repeated entries do not change the final membership state - function test_updateBlocklist_success_idempotentDuplicates(address currentAdmin, address[] memory accounts) - public - { - // unimplemented + function test_updateBlocklist_success_idempotentDuplicates(address currentAdmin, address account) public { + vm.assume(currentAdmin != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); + address[] memory duped = new address[](2); + duped[0] = account; + duped[1] = account; + vm.prank(currentAdmin); + policyRegistry.updateBlocklist(policyId, true, duped); + assertFalse(policyRegistry.isAuthorized(policyId, account)); + vm.prank(currentAdmin); + policyRegistry.updateBlocklist(policyId, false, duped); + assertTrue(policyRegistry.isAuthorized(policyId, account)); } /// @notice Verifies updateBlocklist emits a single BlocklistUpdated carrying the full batch @@ -58,6 +100,12 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { bool blocked, address[] memory accounts ) public { - // unimplemented + vm.assume(currentAdmin != address(0)); + vm.assume(accounts.length <= 5); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); + vm.expectEmit(address(policyRegistry)); + emit IPolicyRegistry.BlocklistUpdated(policyId, currentAdmin, blocked, accounts); + vm.prank(currentAdmin); + policyRegistry.updateBlocklist(policyId, blocked, accounts); } } From 4cdef77f52f16da0eaf856e06fbcc76f32e5afa9 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Wed, 20 May 2026 10:05:51 -0400 Subject: [PATCH 11/17] Fix stale counter docstring; add two missing test cases - MockPolicyRegistry @dev block said counter starts at 2 but it starts at 0 - renounceAdmin freeze test now uses BLOCKLIST + tests updateBlocklist - stageUpdateAdmin adds cancelBlocksFinalize: clearing pending via stage(address(0)) causes subsequent finalizeUpdateAdmin to revert --- test/lib/mocks/MockPolicyRegistry.sol | 6 ++++-- test/unit/PolicyRegistry/renounceAdmin.t.sol | 7 ++++--- test/unit/PolicyRegistry/stageUpdateAdmin.t.sol | 15 +++++++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index 027bc85..0674182 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -31,8 +31,10 @@ import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; /// `_pendingAdmins[policyId]` — address staged by stageUpdateAdmin. /// /// `_nextCounter` — uint56 global counter for the low 56 bits of -/// custom policy IDs. Starts at 2; values 0 and 1 are defensively -/// skipped to avoid any numeric overlap with the built-in IDs. +/// custom policy IDs. Starts at 0. The discriminator byte in bits +/// [63:56] ensures no custom ID can equal built-in 0 or 1 +/// (ALLOWLIST = 0x02, BLOCKLIST = 0x03, minimum custom ID is +/// 0x0200000000000000). /// /// **Policy ID encoding:** /// [63:56] uint8(PolicyType) discriminator diff --git a/test/unit/PolicyRegistry/renounceAdmin.t.sol b/test/unit/PolicyRegistry/renounceAdmin.t.sol index e23d59e..6b0f17c 100644 --- a/test/unit/PolicyRegistry/renounceAdmin.t.sol +++ b/test/unit/PolicyRegistry/renounceAdmin.t.sol @@ -51,11 +51,12 @@ contract PolicyRegistryRenounceAdminTest is PolicyRegistryTest { assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } - /// @notice Verifies renounceAdmin freezes membership and admin operations on the policy + /// @notice Verifies renounceAdmin freezes all mutation on the policy /// @dev Post-renounce: stageUpdateAdmin / updateAllowlist / updateBlocklist all revert Unauthorized function test_renounceAdmin_success_freezesMutation(address currentAdmin) public { vm.assume(currentAdmin != address(0)); - uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + // Use BLOCKLIST so we can test both updateAllowlist (incompatible) and updateBlocklist (frozen) + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); vm.prank(currentAdmin); policyRegistry.renounceAdmin(policyId); @@ -67,7 +68,7 @@ contract PolicyRegistryRenounceAdminTest is PolicyRegistryTest { vm.expectRevert(IPolicyRegistry.Unauthorized.selector); vm.prank(currentAdmin); - policyRegistry.updateAllowlist(policyId, true, accounts); + policyRegistry.updateBlocklist(policyId, true, accounts); } /// @notice Verifies renounceAdmin emits PolicyAdminUpdated with newAdmin = address(0) diff --git a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol index 3a3c455..47c50a5 100644 --- a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol +++ b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol @@ -66,6 +66,21 @@ contract PolicyRegistryStageUpdateAdminTest is PolicyRegistryTest { assertEq(policyRegistry.pendingPolicyAdmin(policyId), address(0)); } + /// @notice Verifies clearing the pending slot (stage address(0)) causes finalizeUpdateAdmin to revert + /// @dev Round-trip: stage a candidate, cancel it, confirm finalize no longer works + function test_stageUpdateAdmin_success_cancelBlocksFinalize(address currentAdmin, address first) public { + vm.assume(currentAdmin != address(0)); + vm.assume(first != address(0)); + uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, first); + vm.prank(currentAdmin); + policyRegistry.stageUpdateAdmin(policyId, address(0)); + vm.expectRevert(IPolicyRegistry.NoPendingAdmin.selector); + vm.prank(first); + policyRegistry.finalizeUpdateAdmin(policyId); + } + /// @notice Verifies stageUpdateAdmin emits PolicyAdminStaged with the correct args /// @dev Event integrity: policyId, currentAdmin, pendingAdmin match the call function test_stageUpdateAdmin_success_emitsPolicyAdminStaged(address currentAdmin, address newAdmin) public { From 2ab89fea94dafff077582f0b975ffe61c7bb4ad5 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Wed, 20 May 2026 10:12:08 -0400 Subject: [PATCH 12/17] PolicyRegistryTest: add harness-style creation helpers Per harness lib-design conventions: extract repeated create sequences into full-parameter + default overloads (_createAllowlist, _createBlocklist) so test bodies only override the variable that matters. --- test/lib/PolicyRegistryTest.sol | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/test/lib/PolicyRegistryTest.sol b/test/lib/PolicyRegistryTest.sol index dfe7928..8436271 100644 --- a/test/lib/PolicyRegistryTest.sol +++ b/test/lib/PolicyRegistryTest.sol @@ -9,10 +9,32 @@ import {StdPrecompiles} from "src/StdPrecompiles.sol"; /// @notice Base test contract for `IPolicyRegistry` unit tests. /// /// Inherits all precompile-mock etch wiring and common actors from -/// `BaseTest`; adds the registry handle. Test bodies that need to set -/// up policies or rotate admins do so inline so the `vm.prank` / call -/// is visible at the test site rather than hidden behind a wrapper. +/// `BaseTest`; adds the registry handle and policy-creation helpers. contract PolicyRegistryTest is BaseTest { // -- Precompile handle -- IPolicyRegistry internal policyRegistry = StdPrecompiles.POLICY_REGISTRY; + + // -- Helpers -- + + /// @notice Create an ALLOWLIST policy with explicit admin and caller. + function _createAllowlist(address caller, address policyAdmin) internal returns (uint64 policyId) { + vm.prank(caller); + policyId = policyRegistry.createPolicy(policyAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); + } + + /// @notice Create an ALLOWLIST policy as the default admin (no prank needed at call site). + function _createAllowlist() internal returns (uint64 policyId) { + policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); + } + + /// @notice Create a BLOCKLIST policy with explicit admin and caller. + function _createBlocklist(address caller, address policyAdmin) internal returns (uint64 policyId) { + vm.prank(caller); + policyId = policyRegistry.createPolicy(policyAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); + } + + /// @notice Create a BLOCKLIST policy as the default admin. + function _createBlocklist() internal returns (uint64 policyId) { + policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); + } } From 49b47bd8f75df8b320feba05dbe71feaeef01a99 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Wed, 20 May 2026 10:17:51 -0400 Subject: [PATCH 13/17] MockPolicyRegistry: store PolicyType in packed slot, drop CREATED_BIT PolicyType (ALLOWLIST=2, BLOCKLIST=3) is stored in bits [7:0] of the packed slot. Both values are non-zero so packed == 0 is a clean existence sentinel even after renounceAdmin zeroes the admin field. No separate CREATED_BIT needed. Packed layout: [167:8] admin address, [7:0] PolicyType. --- test/lib/mocks/MockPolicyRegistry.sol | 58 +++++++++++++-------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index 0674182..ca022b4 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -15,14 +15,13 @@ import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; /// **Storage layout** (Rust impl mirrors these fields in order): /// /// `_policies[id]` — packed uint256: -/// [255:169] unused -/// [168] CREATED_BIT — set at creation, never cleared. -/// Required because a renounced policy has admin = -/// address(0), making bits [167:8] zero; without this -/// sentinel a renounced ALLOWLIST or BLOCKLIST policy -/// would be indistinguishable from an un-created slot. +/// [255:168] unused /// [167:8] admin address (160 bits). Zero after renounceAdmin. -/// [7:0] unused (reserved for future policy flags) +/// [7:0] PolicyType (ALLOWLIST = 2, BLOCKLIST = 3). +/// Since both values are non-zero, `packed == 0` +/// reliably means the policy was never created, even +/// after renounceAdmin zeroes the admin field. No +/// separate existence sentinel is required. /// /// `_members[policyId][account]` — bool: /// ALLOWLIST: true → account IS authorized. @@ -56,12 +55,9 @@ contract MockPolicyRegistry is IPolicyRegistry { // Policy ID encoding: top byte = uint8(PolicyType), low 56 bits = counter. uint256 internal constant TYPE_SHIFT = 56; - // Bit 168 of _policies[id]. Sits one bit above the 160-bit admin field - // at [167:8] so the two fields never overlap regardless of admin value. - uint256 internal constant CREATED_BIT = uint256(1) << 168; - - // Admin address occupies bits [167:8]; bits [7:0] are reserved. + // Admin address occupies bits [167:8]; PolicyType occupies bits [7:0]. uint256 internal constant ADMIN_SHIFT = 8; + uint256 internal constant TYPE_MASK = 0xFF; // ============================================================ // STORAGE @@ -73,8 +69,7 @@ contract MockPolicyRegistry is IPolicyRegistry { // Global monotonic counter for the low 56 bits of custom policy IDs. // Starts at 0. The discriminator byte in bits [63:56] ensures no custom ID - // can equal built-in 0 or 1 regardless of counter value (ALLOWLIST = 0x02, - // BLOCKLIST = 0x03, so the minimum custom ID is 0x0200000000000000). + // can equal built-in 0 or 1 regardless of counter value. uint56 private _nextCounter; // ============================================================ @@ -114,7 +109,7 @@ contract MockPolicyRegistry is IPolicyRegistry { if (pending == address(0)) revert NoPendingAdmin(); if (pending != msg.sender) revert Unauthorized(); address previousAdmin = _decodeAdmin(packed); - _policies[policyId] = _encode(msg.sender); + _policies[policyId] = _encode(_decodeType(packed), msg.sender); delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); } @@ -123,7 +118,7 @@ contract MockPolicyRegistry is IPolicyRegistry { function renounceAdmin(uint64 policyId) external { uint256 packed = _requireCustom(policyId); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _policies[policyId] = _encode(address(0)); + _policies[policyId] = _encode(_decodeType(packed), address(0)); delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, msg.sender, address(0)); } @@ -131,7 +126,7 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function updateAllowlist(uint64 policyId, bool allowed, address[] calldata accounts) external { uint256 packed = _requireCustom(policyId); - if (_typeFromId(policyId) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); + if (_decodeType(packed) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); _batchSetMembers({policyId: policyId, policyType: PolicyType.ALLOWLIST, value: allowed, accounts: accounts}); } @@ -139,7 +134,7 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function updateBlocklist(uint64 policyId, bool blocked, address[] calldata accounts) external { uint256 packed = _requireCustom(policyId); - if (_typeFromId(policyId) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); + if (_decodeType(packed) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); _batchSetMembers({policyId: policyId, policyType: PolicyType.BLOCKLIST, value: blocked, accounts: accounts}); } @@ -155,9 +150,9 @@ contract MockPolicyRegistry is IPolicyRegistry { if (policyId == ALWAYS_ALLOW_ID) return true; if (policyId == ALWAYS_BLOCK_ID) return false; uint256 packed = _policies[policyId]; - if ((packed & CREATED_BIT) == 0) revert PolicyNotFound(); + if (packed == 0) revert PolicyNotFound(); bool member = _members[policyId][account]; - return _typeFromId(policyId) == PolicyType.ALLOWLIST ? member : !member; + return _decodeType(packed) == PolicyType.ALLOWLIST ? member : !member; } // ============================================================ @@ -172,22 +167,23 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function policyExists(uint64 policyId) external view returns (bool) { if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return true; - return (_policies[policyId] & CREATED_BIT) != 0; + return _policies[policyId] != 0; } /// @inheritdoc IPolicyRegistry function policyType(uint64 policyId) external view returns (PolicyType) { if (policyId == ALWAYS_ALLOW_ID) return PolicyType.ALWAYS_ALLOW; if (policyId == ALWAYS_BLOCK_ID) return PolicyType.ALWAYS_BLOCK; - if ((_policies[policyId] & CREATED_BIT) == 0) revert PolicyNotFound(); - return _typeFromId(policyId); + uint256 packed = _policies[policyId]; + if (packed == 0) revert PolicyNotFound(); + return _decodeType(packed); } /// @inheritdoc IPolicyRegistry function policyAdmin(uint64 policyId) external view returns (address) { if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return address(0); uint256 packed = _policies[policyId]; - if ((packed & CREATED_BIT) == 0) revert PolicyNotFound(); + if (packed == 0) revert PolicyNotFound(); return _decodeAdmin(packed); } @@ -211,7 +207,7 @@ contract MockPolicyRegistry is IPolicyRegistry { _nextCounter = counter + 1; } newPolicyId = _makeId(policyType, counter); - _policies[newPolicyId] = _encode(admin); + _policies[newPolicyId] = _encode(policyType, admin); emit PolicyCreated(newPolicyId, msg.sender, policyType); emit PolicyAdminUpdated(newPolicyId, address(0), admin); } @@ -232,20 +228,20 @@ contract MockPolicyRegistry is IPolicyRegistry { function _requireCustom(uint64 policyId) internal view returns (uint256 packed) { packed = _policies[policyId]; - if ((packed & CREATED_BIT) == 0) revert PolicyNotFound(); + if (packed == 0) revert PolicyNotFound(); } function _makeId(PolicyType policyType, uint56 counter) internal pure returns (uint64) { return (uint64(uint8(policyType)) << TYPE_SHIFT) | uint64(counter); } - function _typeFromId(uint64 policyId) internal pure returns (PolicyType) { - // forge-lint: disable-next-line(unsafe-typecast) - return PolicyType(uint8(policyId >> TYPE_SHIFT)); + function _encode(PolicyType policyType, address admin) internal pure returns (uint256) { + return (uint256(uint160(admin)) << ADMIN_SHIFT) | uint256(policyType); } - function _encode(address admin) internal pure returns (uint256) { - return CREATED_BIT | (uint256(uint160(admin)) << ADMIN_SHIFT); + function _decodeType(uint256 packed) internal pure returns (PolicyType) { + // forge-lint: disable-next-line(unsafe-typecast) + return PolicyType(packed & TYPE_MASK); } function _decodeAdmin(uint256 packed) internal pure returns (address) { From 1f6b1d7263bfc83a3cd09c747b197c3c395f212b Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Wed, 20 May 2026 10:20:04 -0400 Subject: [PATCH 14/17] MockPolicyRegistry: remove forge-lint suppression comments _decodeType: cast via uint8 (enum backing type) instead of directly from uint256, making the narrowing explicit and lint-clean. _decodeAdmin: uint160 intermediate already sufficient; suppression comment was unnecessary. Remove unused TYPE_MASK constant. --- test/lib/mocks/MockPolicyRegistry.sol | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index ca022b4..d26ab18 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -57,7 +57,6 @@ contract MockPolicyRegistry is IPolicyRegistry { // Admin address occupies bits [167:8]; PolicyType occupies bits [7:0]. uint256 internal constant ADMIN_SHIFT = 8; - uint256 internal constant TYPE_MASK = 0xFF; // ============================================================ // STORAGE @@ -240,12 +239,10 @@ contract MockPolicyRegistry is IPolicyRegistry { } function _decodeType(uint256 packed) internal pure returns (PolicyType) { - // forge-lint: disable-next-line(unsafe-typecast) - return PolicyType(packed & TYPE_MASK); + return PolicyType(uint8(packed)); } function _decodeAdmin(uint256 packed) internal pure returns (address) { - // forge-lint: disable-next-line(unsafe-typecast) return address(uint160(packed >> ADMIN_SHIFT)); } } From 14169d54418d43d7213066793a6b061266e0ecab Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Wed, 20 May 2026 10:24:34 -0400 Subject: [PATCH 15/17] Apply harness style conventions - Imports: src/ group before test/ group in all 13 test files - bound(): replace vm.assume(accounts.length <= 5) with bound() + assembly mstore in createPolicyWithAccounts, updateAllowlist, updateBlocklist - Named args: use {key: value} form for all _encode and _makeId calls in MockPolicyRegistry --- test/lib/mocks/MockPolicyRegistry.sol | 10 ++++----- test/unit/PolicyRegistry/createPolicy.t.sol | 4 ++-- .../createPolicyWithAccounts.t.sol | 19 ++++++++++------ .../PolicyRegistry/finalizeUpdateAdmin.t.sol | 4 ++-- test/unit/PolicyRegistry/isAuthorized.t.sol | 4 ++-- test/unit/PolicyRegistry/nextPolicyId.t.sol | 4 ++-- .../PolicyRegistry/pendingPolicyAdmin.t.sol | 4 ++-- test/unit/PolicyRegistry/policyAdmin.t.sol | 4 ++-- test/unit/PolicyRegistry/policyExists.t.sol | 4 ++-- test/unit/PolicyRegistry/policyType.t.sol | 4 ++-- test/unit/PolicyRegistry/renounceAdmin.t.sol | 4 ++-- .../PolicyRegistry/stageUpdateAdmin.t.sol | 4 ++-- .../unit/PolicyRegistry/updateAllowlist.t.sol | 22 ++++++++++++------- .../unit/PolicyRegistry/updateBlocklist.t.sol | 22 ++++++++++++------- 14 files changed, 65 insertions(+), 48 deletions(-) diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index d26ab18..d703ea4 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -108,7 +108,7 @@ contract MockPolicyRegistry is IPolicyRegistry { if (pending == address(0)) revert NoPendingAdmin(); if (pending != msg.sender) revert Unauthorized(); address previousAdmin = _decodeAdmin(packed); - _policies[policyId] = _encode(_decodeType(packed), msg.sender); + _policies[policyId] = _encode({policyType: _decodeType(packed), admin: msg.sender}); delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); } @@ -117,7 +117,7 @@ contract MockPolicyRegistry is IPolicyRegistry { function renounceAdmin(uint64 policyId) external { uint256 packed = _requireCustom(policyId); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _policies[policyId] = _encode(_decodeType(packed), address(0)); + _policies[policyId] = _encode({policyType: _decodeType(packed), admin: address(0)}); delete _pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, msg.sender, address(0)); } @@ -160,7 +160,7 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function nextPolicyId(PolicyType policyType) external view returns (uint64) { - return _makeId(policyType, _nextCounter); + return _makeId({policyType: policyType, counter: _nextCounter}); } /// @inheritdoc IPolicyRegistry @@ -205,8 +205,8 @@ contract MockPolicyRegistry is IPolicyRegistry { unchecked { _nextCounter = counter + 1; } - newPolicyId = _makeId(policyType, counter); - _policies[newPolicyId] = _encode(policyType, admin); + newPolicyId = _makeId({policyType: policyType, counter: counter}); + _policies[newPolicyId] = _encode({policyType: policyType, admin: admin}); emit PolicyCreated(newPolicyId, msg.sender, policyType); emit PolicyAdminUpdated(newPolicyId, address(0), admin); } diff --git a/test/unit/PolicyRegistry/createPolicy.t.sol b/test/unit/PolicyRegistry/createPolicy.t.sol index 97b2f07..a06605e 100644 --- a/test/unit/PolicyRegistry/createPolicy.t.sol +++ b/test/unit/PolicyRegistry/createPolicy.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryCreatePolicyTest is PolicyRegistryTest { /// @notice Verifies createPolicy reverts when admin is the zero address /// @dev Required-field guard; checks ZeroAddress() error diff --git a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol index 4c355ee..c7c7442 100644 --- a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol +++ b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { /// @notice Verifies createPolicyWithAccounts reverts when admin is the zero address /// @dev Required-field guard; checks ZeroAddress() error @@ -29,7 +29,8 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { vm.assume(admin_ != address(0)); vm.assume(policyTypeInt != 2 && policyTypeInt != 3); vm.assume(policyTypeInt < 4); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } IPolicyRegistry.PolicyType invalidType = IPolicyRegistry.PolicyType(policyTypeInt); vm.expectRevert(IPolicyRegistry.InvalidPolicyType.selector); vm.prank(caller); @@ -45,7 +46,8 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { ) public { _assumeValidCaller(caller); vm.assume(admin_ != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } vm.prank(caller); uint64 policyId = policyRegistry.createPolicyWithAccounts(admin_, IPolicyRegistry.PolicyType.ALLOWLIST, accounts); @@ -63,7 +65,8 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { ) public { _assumeValidCaller(caller); vm.assume(admin_ != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } vm.prank(caller); uint64 policyId = policyRegistry.createPolicyWithAccounts(admin_, IPolicyRegistry.PolicyType.BLOCKLIST, accounts); @@ -81,7 +84,8 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { ) public { _assumeValidCaller(caller); vm.assume(admin_ != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 expectedId = policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.ALLOWLIST); vm.expectEmit(address(policyRegistry)); emit IPolicyRegistry.AllowlistUpdated(expectedId, caller, true, accounts); @@ -98,7 +102,8 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { ) public { _assumeValidCaller(caller); vm.assume(admin_ != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 expectedId = policyRegistry.nextPolicyId(IPolicyRegistry.PolicyType.BLOCKLIST); vm.expectEmit(address(policyRegistry)); emit IPolicyRegistry.BlocklistUpdated(expectedId, caller, true, accounts); diff --git a/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol b/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol index 7e6d597..a429f0c 100644 --- a/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol +++ b/test/unit/PolicyRegistry/finalizeUpdateAdmin.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryFinalizeUpdateAdminTest is PolicyRegistryTest { /// @notice Verifies finalizeUpdateAdmin reverts when no admin transfer is in flight /// @dev Pending-slot precondition; checks NoPendingAdmin() error diff --git a/test/unit/PolicyRegistry/isAuthorized.t.sol b/test/unit/PolicyRegistry/isAuthorized.t.sol index 46a74d8..a063db6 100644 --- a/test/unit/PolicyRegistry/isAuthorized.t.sol +++ b/test/unit/PolicyRegistry/isAuthorized.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryIsAuthorizedTest is PolicyRegistryTest { /// @notice Verifies isAuthorized reverts for an unknown policy id /// @dev Lookup guard for non-existent ids; checks PolicyNotFound() error diff --git a/test/unit/PolicyRegistry/nextPolicyId.t.sol b/test/unit/PolicyRegistry/nextPolicyId.t.sol index fa20540..15da93c 100644 --- a/test/unit/PolicyRegistry/nextPolicyId.t.sol +++ b/test/unit/PolicyRegistry/nextPolicyId.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryNextPolicyIdTest is PolicyRegistryTest { uint256 private constant TYPE_SHIFT = 56; diff --git a/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol b/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol index fb1e551..c308914 100644 --- a/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol +++ b/test/unit/PolicyRegistry/pendingPolicyAdmin.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryPendingPolicyAdminTest is PolicyRegistryTest { /// @notice Verifies pendingPolicyAdmin returns address(0) before any transfer is staged /// @dev Default state for a freshly-created policy diff --git a/test/unit/PolicyRegistry/policyAdmin.t.sol b/test/unit/PolicyRegistry/policyAdmin.t.sol index 0e85144..bb8f17e 100644 --- a/test/unit/PolicyRegistry/policyAdmin.t.sol +++ b/test/unit/PolicyRegistry/policyAdmin.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryPolicyAdminTest is PolicyRegistryTest { /// @notice Verifies policyAdmin reverts for an unknown policy id /// @dev Lookup guard for non-existent ids; checks PolicyNotFound() error diff --git a/test/unit/PolicyRegistry/policyExists.t.sol b/test/unit/PolicyRegistry/policyExists.t.sol index 22d0a2c..2407288 100644 --- a/test/unit/PolicyRegistry/policyExists.t.sol +++ b/test/unit/PolicyRegistry/policyExists.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryPolicyExistsTest is PolicyRegistryTest { function test_policyExists_success_builtinZero() public view { assertTrue(policyRegistry.policyExists(0)); diff --git a/test/unit/PolicyRegistry/policyType.t.sol b/test/unit/PolicyRegistry/policyType.t.sol index 08185bf..a320589 100644 --- a/test/unit/PolicyRegistry/policyType.t.sol +++ b/test/unit/PolicyRegistry/policyType.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryPolicyTypeTest is PolicyRegistryTest { /// @notice Verifies policyType reverts for an unknown policy id /// @dev Lookup guard for non-existent ids; checks PolicyNotFound() error diff --git a/test/unit/PolicyRegistry/renounceAdmin.t.sol b/test/unit/PolicyRegistry/renounceAdmin.t.sol index 6b0f17c..e7c13d0 100644 --- a/test/unit/PolicyRegistry/renounceAdmin.t.sol +++ b/test/unit/PolicyRegistry/renounceAdmin.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryRenounceAdminTest is PolicyRegistryTest { /// @notice Verifies renounceAdmin reverts when called by any non-admin caller /// @dev Access control: only the current admin may renounce; checks Unauthorized() error diff --git a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol index 47c50a5..a002d80 100644 --- a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol +++ b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryStageUpdateAdminTest is PolicyRegistryTest { /// @notice Verifies stageUpdateAdmin reverts when called by any non-admin caller /// @dev Access control: only the current admin may stage a transfer; checks Unauthorized() error diff --git a/test/unit/PolicyRegistry/updateAllowlist.t.sol b/test/unit/PolicyRegistry/updateAllowlist.t.sol index 49f124c..8e55d3f 100644 --- a/test/unit/PolicyRegistry/updateAllowlist.t.sol +++ b/test/unit/PolicyRegistry/updateAllowlist.t.sol @@ -1,17 +1,18 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { /// @notice Verifies updateAllowlist reverts when called by any non-admin caller /// @dev Access control: only the current admin may mutate membership; checks Unauthorized() error function test_updateAllowlist_revert_unauthorized(address caller, bool allowed, address[] memory accounts) public { _assumeValidCaller(caller); vm.assume(caller != admin); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.ALLOWLIST); vm.expectRevert(IPolicyRegistry.Unauthorized.selector); vm.prank(caller); @@ -26,7 +27,8 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { address[] memory accounts ) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); vm.prank(currentAdmin); @@ -43,7 +45,8 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { ) public { _assumeValidCaller(caller); vm.assume(policyId > 1); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); vm.prank(caller); policyRegistry.updateAllowlist(policyId, allowed, accounts); @@ -53,7 +56,8 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { /// @dev isAuthorized returns true for each added account afterward function test_updateAllowlist_success_addsAccounts(address currentAdmin, address[] memory accounts) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); vm.prank(currentAdmin); policyRegistry.updateAllowlist(policyId, true, accounts); @@ -66,7 +70,8 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { /// @dev isAuthorized returns false for each removed account afterward function test_updateAllowlist_success_removesAccounts(address currentAdmin, address[] memory accounts) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); vm.prank(currentAdmin); policyRegistry.updateAllowlist(policyId, true, accounts); @@ -101,7 +106,8 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { address[] memory accounts ) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); vm.expectEmit(address(policyRegistry)); emit IPolicyRegistry.AllowlistUpdated(policyId, currentAdmin, allowed, accounts); diff --git a/test/unit/PolicyRegistry/updateBlocklist.t.sol b/test/unit/PolicyRegistry/updateBlocklist.t.sol index a2cb2fb..f7995f4 100644 --- a/test/unit/PolicyRegistry/updateBlocklist.t.sol +++ b/test/unit/PolicyRegistry/updateBlocklist.t.sol @@ -1,17 +1,18 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; - import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; + contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { /// @notice Verifies updateBlocklist reverts when called by any non-admin caller /// @dev Access control: only the current admin may mutate membership; checks Unauthorized() error function test_updateBlocklist_revert_unauthorized(address caller, bool blocked, address[] memory accounts) public { _assumeValidCaller(caller); vm.assume(caller != admin); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(admin, IPolicyRegistry.PolicyType.BLOCKLIST); vm.expectRevert(IPolicyRegistry.Unauthorized.selector); vm.prank(caller); @@ -26,7 +27,8 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { address[] memory accounts ) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); vm.expectRevert(IPolicyRegistry.IncompatiblePolicyType.selector); vm.prank(currentAdmin); @@ -43,7 +45,8 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { ) public { _assumeValidCaller(caller); vm.assume(policyId > 1); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } vm.expectRevert(IPolicyRegistry.PolicyNotFound.selector); vm.prank(caller); policyRegistry.updateBlocklist(policyId, blocked, accounts); @@ -53,7 +56,8 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { /// @dev isAuthorized returns false for each added account afterward function test_updateBlocklist_success_addsAccounts(address currentAdmin, address[] memory accounts) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); vm.prank(currentAdmin); policyRegistry.updateBlocklist(policyId, true, accounts); @@ -66,7 +70,8 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { /// @dev isAuthorized returns true for each removed account afterward function test_updateBlocklist_success_removesAccounts(address currentAdmin, address[] memory accounts) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); vm.prank(currentAdmin); policyRegistry.updateBlocklist(policyId, true, accounts); @@ -101,7 +106,8 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { address[] memory accounts ) public { vm.assume(currentAdmin != address(0)); - vm.assume(accounts.length <= 5); + uint256 len = bound(accounts.length, 0, 5); + assembly { mstore(accounts, len) } uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.BLOCKLIST); vm.expectEmit(address(policyRegistry)); emit IPolicyRegistry.BlocklistUpdated(policyId, currentAdmin, blocked, accounts); From ebbbb0de2d0406403f952ea066f63fe08587bdaf Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Wed, 20 May 2026 10:32:14 -0400 Subject: [PATCH 16/17] MockPolicyRegistry: migrate to ERC-7201 namespaced storage Introduces MockPolicyRegistryStorage under namespace base.policy_registry (location 0x00503aeb...4a00), following the MockB20Storage pattern. Struct field order is the slot layout the Rust impl mirrors: POLICIES_OFFSET = 0 (mapping policyId => packed uint256) MEMBERS_OFFSET = 1 (mapping policyId => account => bool) PENDING_ADMINS_OFFSET = 2 (mapping policyId => address) NEXT_COUNTER_OFFSET = 3 (uint56 global counter) MockPolicyRegistry updated to use layout() throughout. Adds a storage-location verification test matching the MockB20Storage.t.sol pattern to catch any drift in the hardcoded STORAGE_LOCATION constant. --- test/lib/mocks/MockPolicyRegistry.sol | 93 +++++++------------ test/lib/mocks/MockPolicyRegistryStorage.sol | 77 +++++++++++++++ .../storage/MockPolicyRegistryStorage.t.sol | 37 ++++++++ 3 files changed, 149 insertions(+), 58 deletions(-) create mode 100644 test/lib/mocks/MockPolicyRegistryStorage.sol create mode 100644 test/unit/storage/MockPolicyRegistryStorage.t.sol diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index d703ea4..cc99a93 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.20; import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; +import {MockPolicyRegistryStorage} from "test/lib/mocks/MockPolicyRegistryStorage.sol"; + /// @title MockPolicyRegistry /// @notice Reference implementation of the `IPolicyRegistry` precompile. /// Etched at the canonical policy-registry address via `vm.etch` @@ -12,33 +14,15 @@ import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; /// with the production Rust precompile is the goal, not gas /// optimisation or Solidity idiom adherence. /// -/// **Storage layout** (Rust impl mirrors these fields in order): -/// -/// `_policies[id]` — packed uint256: -/// [255:168] unused -/// [167:8] admin address (160 bits). Zero after renounceAdmin. -/// [7:0] PolicyType (ALLOWLIST = 2, BLOCKLIST = 3). -/// Since both values are non-zero, `packed == 0` -/// reliably means the policy was never created, even -/// after renounceAdmin zeroes the admin field. No -/// separate existence sentinel is required. -/// -/// `_members[policyId][account]` — bool: -/// ALLOWLIST: true → account IS authorized. -/// BLOCKLIST: true → account IS blocked (NOT authorized). -/// -/// `_pendingAdmins[policyId]` — address staged by stageUpdateAdmin. -/// -/// `_nextCounter` — uint56 global counter for the low 56 bits of -/// custom policy IDs. Starts at 0. The discriminator byte in bits -/// [63:56] ensures no custom ID can equal built-in 0 or 1 -/// (ALLOWLIST = 0x02, BLOCKLIST = 0x03, minimum custom ID is -/// 0x0200000000000000). +/// All mutable state lives in `MockPolicyRegistryStorage.layout()` at +/// a single ERC-7201-namespaced root. The struct field order IS the +/// slot layout the Rust impl mirrors. See `MockPolicyRegistryStorage` +/// for the full layout documentation and per-field slot offsets. /// /// **Policy ID encoding:** /// [63:56] uint8(PolicyType) discriminator -/// [55:0] _nextCounter value at creation time -/// _create rejects ALWAYS_ALLOW and ALWAYS_BLOCK types, so no +/// [55:0] nextCounter value at creation time +/// `_create` rejects ALWAYS_ALLOW and ALWAYS_BLOCK types, so no /// custom ID ever carries discriminator 0x00 or 0x01. /// /// **Built-in IDs** (short-circuited before any storage read): @@ -58,19 +42,6 @@ contract MockPolicyRegistry is IPolicyRegistry { // Admin address occupies bits [167:8]; PolicyType occupies bits [7:0]. uint256 internal constant ADMIN_SHIFT = 8; - // ============================================================ - // STORAGE - // ============================================================ - - mapping(uint64 policyId => uint256 packed) private _policies; - mapping(uint64 policyId => mapping(address account => bool)) private _members; - mapping(uint64 policyId => address pendingAdmin) private _pendingAdmins; - - // Global monotonic counter for the low 56 bits of custom policy IDs. - // Starts at 0. The discriminator byte in bits [63:56] ensures no custom ID - // can equal built-in 0 or 1 regardless of counter value. - uint56 private _nextCounter; - // ============================================================ // POLICY CREATION // ============================================================ @@ -97,28 +68,32 @@ contract MockPolicyRegistry is IPolicyRegistry { function stageUpdateAdmin(uint64 policyId, address newAdmin) external { uint256 packed = _requireCustom(policyId); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _pendingAdmins[policyId] = newAdmin; + MockPolicyRegistryStorage.layout().pendingAdmins[policyId] = newAdmin; emit PolicyAdminStaged(policyId, msg.sender, newAdmin); } /// @inheritdoc IPolicyRegistry function finalizeUpdateAdmin(uint64 policyId) external { - uint256 packed = _requireCustom(policyId); - address pending = _pendingAdmins[policyId]; + MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout(); + uint256 packed = $.policies[policyId]; + if (packed == 0) revert PolicyNotFound(); + address pending = $.pendingAdmins[policyId]; if (pending == address(0)) revert NoPendingAdmin(); if (pending != msg.sender) revert Unauthorized(); address previousAdmin = _decodeAdmin(packed); - _policies[policyId] = _encode({policyType: _decodeType(packed), admin: msg.sender}); - delete _pendingAdmins[policyId]; + $.policies[policyId] = _encode({policyType: _decodeType(packed), admin: msg.sender}); + delete $.pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender); } /// @inheritdoc IPolicyRegistry function renounceAdmin(uint64 policyId) external { - uint256 packed = _requireCustom(policyId); + MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout(); + uint256 packed = $.policies[policyId]; + if (packed == 0) revert PolicyNotFound(); if (_decodeAdmin(packed) != msg.sender) revert Unauthorized(); - _policies[policyId] = _encode({policyType: _decodeType(packed), admin: address(0)}); - delete _pendingAdmins[policyId]; + $.policies[policyId] = _encode({policyType: _decodeType(packed), admin: address(0)}); + delete $.pendingAdmins[policyId]; emit PolicyAdminUpdated(policyId, msg.sender, address(0)); } @@ -145,12 +120,13 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function isAuthorized(uint64 policyId, address account) external view returns (bool) { // Built-in short-circuits MUST precede any storage read: IDs 0 and 1 - // have no entry in _policies and must never reach the storage path. + // have no entry in storage and must never reach the storage path. if (policyId == ALWAYS_ALLOW_ID) return true; if (policyId == ALWAYS_BLOCK_ID) return false; - uint256 packed = _policies[policyId]; + MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout(); + uint256 packed = $.policies[policyId]; if (packed == 0) revert PolicyNotFound(); - bool member = _members[policyId][account]; + bool member = $.members[policyId][account]; return _decodeType(packed) == PolicyType.ALLOWLIST ? member : !member; } @@ -160,20 +136,20 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function nextPolicyId(PolicyType policyType) external view returns (uint64) { - return _makeId({policyType: policyType, counter: _nextCounter}); + return _makeId({policyType: policyType, counter: MockPolicyRegistryStorage.layout().nextCounter}); } /// @inheritdoc IPolicyRegistry function policyExists(uint64 policyId) external view returns (bool) { if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return true; - return _policies[policyId] != 0; + return MockPolicyRegistryStorage.layout().policies[policyId] != 0; } /// @inheritdoc IPolicyRegistry function policyType(uint64 policyId) external view returns (PolicyType) { if (policyId == ALWAYS_ALLOW_ID) return PolicyType.ALWAYS_ALLOW; if (policyId == ALWAYS_BLOCK_ID) return PolicyType.ALWAYS_BLOCK; - uint256 packed = _policies[policyId]; + uint256 packed = MockPolicyRegistryStorage.layout().policies[policyId]; if (packed == 0) revert PolicyNotFound(); return _decodeType(packed); } @@ -181,7 +157,7 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function policyAdmin(uint64 policyId) external view returns (address) { if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return address(0); - uint256 packed = _policies[policyId]; + uint256 packed = MockPolicyRegistryStorage.layout().policies[policyId]; if (packed == 0) revert PolicyNotFound(); return _decodeAdmin(packed); } @@ -189,7 +165,7 @@ contract MockPolicyRegistry is IPolicyRegistry { /// @inheritdoc IPolicyRegistry function pendingPolicyAdmin(uint64 policyId) external view returns (address) { if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return address(0); - return _pendingAdmins[policyId]; + return MockPolicyRegistryStorage.layout().pendingAdmins[policyId]; } // ============================================================ @@ -199,14 +175,15 @@ contract MockPolicyRegistry is IPolicyRegistry { function _create(address admin, PolicyType policyType) internal returns (uint64 newPolicyId) { if (policyType != PolicyType.ALLOWLIST && policyType != PolicyType.BLOCKLIST) revert InvalidPolicyType(); if (admin == address(0)) revert ZeroAddress(); - uint56 counter = _nextCounter; + MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout(); + uint56 counter = $.nextCounter; // No overflow guard: at one policy per 2-second block, exhausting the // 56-bit counter space (~7.2e16 values) takes ~4.6 billion years. unchecked { - _nextCounter = counter + 1; + $.nextCounter = counter + 1; } newPolicyId = _makeId({policyType: policyType, counter: counter}); - _policies[newPolicyId] = _encode({policyType: policyType, admin: admin}); + $.policies[newPolicyId] = _encode({policyType: policyType, admin: admin}); emit PolicyCreated(newPolicyId, msg.sender, policyType); emit PolicyAdminUpdated(newPolicyId, address(0), admin); } @@ -214,7 +191,7 @@ contract MockPolicyRegistry is IPolicyRegistry { function _batchSetMembers(uint64 policyId, PolicyType policyType, bool value, address[] calldata accounts) internal { - mapping(address => bool) storage members = _members[policyId]; + mapping(address => bool) storage members = MockPolicyRegistryStorage.layout().members[policyId]; for (uint256 i = 0; i < accounts.length; ++i) { members[accounts[i]] = value; } @@ -226,7 +203,7 @@ contract MockPolicyRegistry is IPolicyRegistry { } function _requireCustom(uint64 policyId) internal view returns (uint256 packed) { - packed = _policies[policyId]; + packed = MockPolicyRegistryStorage.layout().policies[policyId]; if (packed == 0) revert PolicyNotFound(); } diff --git a/test/lib/mocks/MockPolicyRegistryStorage.sol b/test/lib/mocks/MockPolicyRegistryStorage.sol new file mode 100644 index 0000000..b1f78cc --- /dev/null +++ b/test/lib/mocks/MockPolicyRegistryStorage.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +/// @title MockPolicyRegistryStorage +/// @notice Slot-layout library for the `MockPolicyRegistry` reference implementation. +/// +/// Every piece of mutable registry state lives in this struct at a single +/// ERC-7201-namespaced location, so the Rust precompile implementation +/// has an unambiguous, audit-grep-able source of truth for which slot +/// holds what. +/// +/// @dev **Why ERC-7201 over flat unstructured storage?** +/// The struct field ORDER is the slot layout. There is no separate list +/// of slot constants that can drift out of sync with the fields they +/// describe. The Rust impl reads this struct top-to-bottom and replicates +/// the same ordering. +/// +/// **Namespace:** `base.policy_registry`. The ERC-7201 location is +/// `keccak256(abi.encode(uint256(keccak256("base.policy_registry")) - 1)) & ~bytes32(uint256(0xff))`. +/// +/// **Packed policy slot layout** (field `policies[id]`): +/// [255:168] unused +/// [167:8] admin address (160 bits). Zero after renounceAdmin. +/// [7:0] PolicyType (ALLOWLIST = 2, BLOCKLIST = 3). +/// Both values are non-zero, so `policies[id] == 0` +/// reliably means the policy was never created. +library MockPolicyRegistryStorage { + /// @custom:storage-location erc7201:base.policy_registry + struct Layout { + // Each entry packs admin + PolicyType into a single uint256. + // packed == 0 means the policy was never created (see packed layout above). + mapping(uint64 policyId => uint256 packed) policies; + // ALLOWLIST: true → account IS authorized. + // BLOCKLIST: true → account IS blocked (NOT authorized). + mapping(uint64 policyId => mapping(address account => bool)) members; + // Staged pending admin for in-flight two-step admin transfers. + mapping(uint64 policyId => address pendingAdmin) pendingAdmins; + // Global monotonic counter for the low 56 bits of custom policy IDs. + // Starts at 0 (ERC-7201 default). The discriminator byte in bits [63:56] + // of any custom ID ensures it can never equal built-in IDs 0 or 1. + uint56 nextCounter; + } + + // keccak256(abi.encode(uint256(keccak256("base.policy_registry")) - 1)) & ~bytes32(uint256(0xff)) + // Verified against the computation in derivedLocation() below. + bytes32 internal constant STORAGE_LOCATION = 0x00503aeb06982fa1fe3151dc68f90b3946c55c449dfd447e49dcaece71ba4a00; + + // ============================================================ + // SLOT OFFSETS WITHIN LAYOUT + // ============================================================ + // Solidity allocates struct fields sequentially starting at the struct's + // base slot. These constants name each field's offset from STORAGE_LOCATION + // so the Rust impl can derive member slots via keccak256(key, baseSlot). + // They MUST stay in sync with the field order of Layout above. + + uint256 internal constant POLICIES_OFFSET = 0; + uint256 internal constant MEMBERS_OFFSET = 1; + uint256 internal constant PENDING_ADMINS_OFFSET = 2; + uint256 internal constant NEXT_COUNTER_OFFSET = 3; + + /// @notice Absolute slot for a top-level field of `Layout`. + function slotOf(uint256 offset) internal pure returns (bytes32) { + return bytes32(uint256(STORAGE_LOCATION) + offset); + } + + function layout() internal pure returns (Layout storage $) { + assembly { + $.slot := STORAGE_LOCATION + } + } + + /// @notice Returns the storage location derived per the ERC-7201 formula. + /// Used in tests to assert the hardcoded STORAGE_LOCATION is correct. + function derivedLocation() internal pure returns (bytes32) { + return keccak256(abi.encode(uint256(keccak256("base.policy_registry")) - 1)) & ~bytes32(uint256(0xff)); + } +} diff --git a/test/unit/storage/MockPolicyRegistryStorage.t.sol b/test/unit/storage/MockPolicyRegistryStorage.t.sol new file mode 100644 index 0000000..f84f0b5 --- /dev/null +++ b/test/unit/storage/MockPolicyRegistryStorage.t.sol @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; + +import {MockPolicyRegistryStorage} from "test/lib/mocks/MockPolicyRegistryStorage.sol"; + +/// @notice Asserts the hardcoded `STORAGE_LOCATION` constant on +/// `MockPolicyRegistryStorage` matches the ERC-7201 formula it documents. +/// +/// This constant is the storage-layout contract between the Solidity mock and +/// the Rust precompile impl: both sides hash the same namespace string and +/// arrive at the same root slot. Verifying the constant against the formula +/// in-tree ensures a stale `STORAGE_LOCATION` literal can't drift silently +/// when the namespace changes. +contract MockPolicyRegistryStorageLocationTest is Test { + /// @notice `MockPolicyRegistryStorage.STORAGE_LOCATION` equals + /// keccak256(abi.encode(uint256(keccak256("base.policy_registry")) - 1)) & ~bytes32(uint256(0xff)). + function test_MockPolicyRegistryStorage_storageLocation_matchesFormula() public pure { + assertEq( + MockPolicyRegistryStorage.STORAGE_LOCATION, + MockPolicyRegistryStorage.derivedLocation(), + "MockPolicyRegistryStorage.STORAGE_LOCATION must match its ERC-7201 derivation" + ); + } + + /// @notice The policy-registry namespace must not collide with `base.b20`. + /// @dev Sanity check: different precompiles must have disjoint storage roots. + function test_MockPolicyRegistryStorage_storageLocation_disjointFromB20() public pure { + bytes32 b20Location = + keccak256(abi.encode(uint256(keccak256("base.b20")) - 1)) & ~bytes32(uint256(0xff)); + assertTrue( + MockPolicyRegistryStorage.STORAGE_LOCATION != b20Location, + "policy_registry and b20 namespaces must derive to disjoint roots" + ); + } +} From 3b42374b2f8b5f86247f94480d5d800291f8f91c Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Wed, 20 May 2026 10:43:00 -0400 Subject: [PATCH 17/17] forge fmt --- test/lib/mocks/MockPolicyRegistryStorage.sol | 6 +++--- test/unit/PolicyRegistry/stageUpdateAdmin.t.sol | 4 +--- test/unit/storage/MockPolicyRegistryStorage.t.sol | 3 +-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/test/lib/mocks/MockPolicyRegistryStorage.sol b/test/lib/mocks/MockPolicyRegistryStorage.sol index b1f78cc..3c99c12 100644 --- a/test/lib/mocks/MockPolicyRegistryStorage.sol +++ b/test/lib/mocks/MockPolicyRegistryStorage.sol @@ -53,10 +53,10 @@ library MockPolicyRegistryStorage { // so the Rust impl can derive member slots via keccak256(key, baseSlot). // They MUST stay in sync with the field order of Layout above. - uint256 internal constant POLICIES_OFFSET = 0; - uint256 internal constant MEMBERS_OFFSET = 1; + uint256 internal constant POLICIES_OFFSET = 0; + uint256 internal constant MEMBERS_OFFSET = 1; uint256 internal constant PENDING_ADMINS_OFFSET = 2; - uint256 internal constant NEXT_COUNTER_OFFSET = 3; + uint256 internal constant NEXT_COUNTER_OFFSET = 3; /// @notice Absolute slot for a top-level field of `Layout`. function slotOf(uint256 offset) internal pure returns (bytes32) { diff --git a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol index a002d80..1dcfd27 100644 --- a/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol +++ b/test/unit/PolicyRegistry/stageUpdateAdmin.t.sol @@ -40,9 +40,7 @@ contract PolicyRegistryStageUpdateAdminTest is PolicyRegistryTest { /// @notice Verifies a second stageUpdateAdmin overwrites a previously-staged candidate /// @dev Latest call wins; the prior candidate loses ability to finalize - function test_stageUpdateAdmin_success_overwritesPrior(address currentAdmin, address first, address second) - public - { + function test_stageUpdateAdmin_success_overwritesPrior(address currentAdmin, address first, address second) public { vm.assume(currentAdmin != address(0)); vm.assume(first != second); uint64 policyId = policyRegistry.createPolicy(currentAdmin, IPolicyRegistry.PolicyType.ALLOWLIST); diff --git a/test/unit/storage/MockPolicyRegistryStorage.t.sol b/test/unit/storage/MockPolicyRegistryStorage.t.sol index f84f0b5..79a79b1 100644 --- a/test/unit/storage/MockPolicyRegistryStorage.t.sol +++ b/test/unit/storage/MockPolicyRegistryStorage.t.sol @@ -27,8 +27,7 @@ contract MockPolicyRegistryStorageLocationTest is Test { /// @notice The policy-registry namespace must not collide with `base.b20`. /// @dev Sanity check: different precompiles must have disjoint storage roots. function test_MockPolicyRegistryStorage_storageLocation_disjointFromB20() public pure { - bytes32 b20Location = - keccak256(abi.encode(uint256(keccak256("base.b20")) - 1)) & ~bytes32(uint256(0xff)); + bytes32 b20Location = keccak256(abi.encode(uint256(keccak256("base.b20")) - 1)) & ~bytes32(uint256(0xff)); assertTrue( MockPolicyRegistryStorage.STORAGE_LOCATION != b20Location, "policy_registry and b20 namespaces must derive to disjoint roots"