Skip to content

refactor(policy): migrate policies[id] slot to Solidity packed struct (bit-identical) #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
amiecorso wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor(policy): migrate policies[id] slot to Solidity packed struct (bit-identical) #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 33 additions & 38 deletions test/lib/mocks/MockPolicyRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,54 +102,55 @@ contract MockPolicyRegistry is IPolicyRegistry {

/// @inheritdoc IPolicyRegistry
function stageUpdateAdmin(uint64 policyId, address newAdmin) external {
uint256 packed = _requireCustom(policyId);
if (_decodeAdmin(packed) != msg.sender) revert Unauthorized();
MockPolicyRegistryStorage.PolicyPacked memory packed = _requireCustom(policyId);
if (packed.admin != msg.sender) revert Unauthorized();
MockPolicyRegistryStorage.layout().pendingAdmins[policyId] = newAdmin;
emit PolicyAdminStaged(policyId, msg.sender, newAdmin);
}

/// @inheritdoc IPolicyRegistry
function finalizeUpdateAdmin(uint64 policyId) external {
MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout();
uint256 packed = $.policies[policyId];
if (packed == 0) revert PolicyNotFound();
MockPolicyRegistryStorage.PolicyPacked memory packed = $.policies[policyId];
if (!MockPolicyRegistryStorage.existsSet(packed)) 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(msg.sender);
// Replace the admin lane in place. Solidity emits a single
// masked SSTORE; the existence byte at bits 248..255 is untouched.
$.policies[policyId].admin = msg.sender;
delete $.pendingAdmins[policyId];
emit PolicyAdminUpdated(policyId, previousAdmin, msg.sender);
emit PolicyAdminUpdated(policyId, packed.admin, msg.sender);
}

/// @inheritdoc IPolicyRegistry
function renounceAdmin(uint64 policyId) external {
MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout();
uint256 packed = $.policies[policyId];
if (packed == 0) revert PolicyNotFound();
if (_decodeAdmin(packed) != msg.sender) revert Unauthorized();
// Admin lane cleared, exists flag (bit 160) survives so the
// policy stays observable via `policyExists` and the existence
// check on subsequent mutating calls still passes (with
// `Unauthorized` taking over as the rejection reason).
$.policies[policyId] = _encode(address(0));
MockPolicyRegistryStorage.PolicyPacked memory packed = $.policies[policyId];
if (!MockPolicyRegistryStorage.existsSet(packed)) revert PolicyNotFound();
if (packed.admin != msg.sender) revert Unauthorized();
// Admin lane cleared, existence byte survives so the policy
// stays observable via `policyExists` and the existence check
// on subsequent mutating calls still passes (with `Unauthorized`
// taking over as the rejection reason).
$.policies[policyId].admin = 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);
MockPolicyRegistryStorage.PolicyPacked memory packed = _requireCustom(policyId);
if (_typeOf(policyId) != PolicyType.ALLOWLIST) revert IncompatiblePolicyType();
if (_decodeAdmin(packed) != msg.sender) revert Unauthorized();
if (packed.admin != 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);
MockPolicyRegistryStorage.PolicyPacked memory packed = _requireCustom(policyId);
if (_typeOf(policyId) != PolicyType.BLOCKLIST) revert IncompatiblePolicyType();
if (_decodeAdmin(packed) != msg.sender) revert Unauthorized();
if (packed.admin != msg.sender) revert Unauthorized();
_batchSetMembers({policyId: policyId, policyType: PolicyType.BLOCKLIST, value: blocked, accounts: accounts});
}

Expand Down Expand Up @@ -181,7 +182,7 @@ contract MockPolicyRegistry is IPolicyRegistry {
function policyExists(uint64 policyId) external view returns (bool) {
if (policyId == ALWAYS_ALLOW_ID || policyId == ALWAYS_BLOCK_ID) return true;
if (!_isWellFormed(policyId)) return false;
return MockPolicyRegistryStorage.layout().policies[policyId] != 0;
return MockPolicyRegistryStorage.existsSet(MockPolicyRegistryStorage.layout().policies[policyId]);
}

/// @inheritdoc IPolicyRegistry
Expand All @@ -190,7 +191,7 @@ contract MockPolicyRegistry is IPolicyRegistry {
// No fast path for built-in IDs needed: lazy init writes them with
// a zero admin, so the normal storage read returns address(0) for
// them just like for renounced policies and uncreated IDs.
return _decodeAdmin(MockPolicyRegistryStorage.layout().policies[policyId]);
return MockPolicyRegistryStorage.layout().policies[policyId].admin;
}

/// @inheritdoc IPolicyRegistry
Expand Down Expand Up @@ -219,15 +220,15 @@ contract MockPolicyRegistry is IPolicyRegistry {
$.nextCounter = counter + 1;
}
newPolicyId = _makeId({policyType: policyType, counter: counter});
$.policies[newPolicyId] = _encode(admin);
$.policies[newPolicyId] = MockPolicyRegistryStorage.newPolicy(admin);
emit PolicyCreated(newPolicyId, msg.sender, policyType);
emit PolicyAdminUpdated(newPolicyId, address(0), admin);
}

/// @dev Writes the two built-in policies into the `policies` mapping and
/// advances `nextCounter` past them so custom policies start at
/// `PolicyRegistryConstants.BUILTIN_POLICY_COUNT`. Both built-ins are
/// written with a renounced (zero) admin, so any later `require_admin`
/// written with a renounced (zero) admin, so any later admin-gated
/// check against them rejects with `Unauthorized`.
///
/// Idempotent: re-entry with `nextCounter >= BUILTIN_POLICY_COUNT` is
Expand All @@ -238,9 +239,9 @@ contract MockPolicyRegistry is IPolicyRegistry {
function _writeBuiltins() internal {
MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout();
if ($.nextCounter >= PolicyRegistryConstants.BUILTIN_POLICY_COUNT) return;
uint256 packed = _encode(address(0));
$.policies[PolicyRegistryConstants.ALWAYS_ALLOW_ID] = packed;
$.policies[PolicyRegistryConstants.ALWAYS_BLOCK_ID] = packed;
MockPolicyRegistryStorage.PolicyPacked memory builtin = MockPolicyRegistryStorage.newPolicy(address(0));
$.policies[PolicyRegistryConstants.ALWAYS_ALLOW_ID] = builtin;
$.policies[PolicyRegistryConstants.ALWAYS_BLOCK_ID] = builtin;
$.nextCounter = PolicyRegistryConstants.BUILTIN_POLICY_COUNT;
}

Expand All @@ -259,25 +260,19 @@ contract MockPolicyRegistry is IPolicyRegistry {
}
}

function _requireCustom(uint64 policyId) internal view returns (uint256 packed) {
function _requireCustom(uint64 policyId)
internal
view
returns (MockPolicyRegistryStorage.PolicyPacked memory packed)
{
packed = MockPolicyRegistryStorage.layout().policies[policyId];
if (packed == 0) revert PolicyNotFound();
if (!MockPolicyRegistryStorage.existsSet(packed)) revert PolicyNotFound();
}

function _makeId(PolicyType policyType, uint56 counter) internal pure returns (uint64) {
return (uint64(uint8(policyType)) << POLICY_ID_TYPE_SHIFT) | uint64(counter);
}

/// @dev Composes a packed slot value. Always sets the exists bit; pass
/// `address(0)` to encode the post-renounce slot.
function _encode(address admin) internal pure returns (uint256) {
return (uint256(1) << MockPolicyRegistryStorage.EXISTS_BIT) | uint256(uint160(admin));
}

function _decodeAdmin(uint256 packed) internal pure returns (address) {
return address(uint160(packed));
}

/// @dev Recovers the `PolicyType` from a well-formed `policyId`'s top byte.
/// Caller MUST ensure `_isWellFormed(policyId)`; otherwise the cast panics.
function _typeOf(uint64 policyId) internal pure returns (PolicyType) {
Expand Down
116 changes: 104 additions & 12 deletions test/lib/mocks/MockPolicyRegistryStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,95 @@ pragma solidity ^0.8.20;
/// `keccak256(abi.encode(uint256(keccak256("base.policy_registry")) - 1)) & ~bytes32(uint256(0xff))`.
///
/// **Packed policy slot layout** (field `policies[id]`):
/// [255] exists flag (set on create, never cleared)
/// [254:160] unused
/// [255] exists flag (high bit of `existsByte`, never cleared)
/// [254:248] unused bits within `existsByte` (always zero)
/// [247:160] reserved (`reservedMiddle`, always zero)
/// [159:0] admin address; zero after renounceAdmin
/// The exists bit survives renunciation, so `policies[id] == 0`
/// Solidity packs the `PolicyPacked` struct LSB-first into a single
/// 256-bit slot, so the struct field declaration order IS the binary
/// layout spec — the Rust impl mirrors the field order and types
/// (`Address` then `uint88` then `uint8`) with no comment-vs-code
/// drift surface. The bit-255 exists location is preserved as a
/// deliberate constraint (storage layout is frozen — see
/// `EXISTS_FLAG_BYTE` for the footgun this creates and the helpers
/// that hide it).
/// The exists bit survives renunciation, so an unset `existsByte`
/// reliably means "never created". PolicyType is NOT stored —
/// it is recovered from `policyId`'s top byte.
///
/// **Polymorphism non-goal.** This struct holds the admin'd
/// ALLOWLIST/BLOCKLIST policy shape. Future composite policy
/// types (e.g. UNION/INTERSECT, immutable + multi-reference)
/// must NOT be overloaded onto this struct: Solidity's lack of
/// sum-types means doing so would force an awkward worst-of-both
/// layout. When composite types land, they get their own struct
/// and a parallel `mapping(uint64 => CompositePolicyPacked)`,
/// with the policy ID's top byte routing lookups between
/// mappings.
library MockPolicyRegistryStorage {
/// @notice Packed storage word for an admin'd policy (ALLOWLIST or
/// BLOCKLIST). Solidity LSB-first packing:
/// bits 0..159 : admin
/// bits 160..247 : reservedMiddle (always zero)
/// bits 248..255 : existsByte — only bit 7 (slot bit 255)
/// carries the existence signal
/// @dev The `existsByte` field is a single byte but only its high
/// bit (= slot bit 255) is meaningful. This is a deliberate
/// consequence of preserving the bit-255 existence-flag
/// location while still expressing the slot as a Solidity
/// packed struct (Solidity has no arbitrary-width sub-byte
/// types, so a single-bit field at byte 31 has to ride along
/// in a `uint8`). **Direct writes to `existsByte` are
/// footgun-prone** — `existsByte = 1` lands the flag at slot
/// bit 248, NOT bit 255. Use `newPolicy()` to construct and
/// `existsSet()` to read; the helpers always write
/// `EXISTS_FLAG_BYTE` and read via inequality with zero.
///
/// The `exists` signal is what lets the registry distinguish
/// "renounced" (admin zero, existsByte != 0) from "never
/// created" (admin zero, existsByte == 0). Without it,
/// BLOCKLIST policies with renounced admins would collide
/// with the zero-default.
struct PolicyPacked {
address admin;
uint88 reservedMiddle;
uint8 existsByte;
}

/// @notice The byte value `existsByte` must hold to encode
/// "exists = true" while preserving the canonical bit-255
/// existence-flag location. `0x80` sets bit 7 of the byte
/// at slot position 31, which equals slot bit 255.
/// @dev The `newPolicy` constructor always writes this value
/// when initializing a struct; reads check for non-zero via
/// `existsSet`. The `newPolicy` / `existsSet` helpers are
/// the only sanctioned API for the existence flag.
uint8 internal constant EXISTS_FLAG_BYTE = 0x80;

/// @notice Constructs a new `PolicyPacked` with the existence flag
/// set in the canonical bit-255 position.
/// @dev Always use this constructor instead of struct-literal
/// initialization — direct `existsByte = ...` writes risk
/// landing the flag at the wrong bit.
function newPolicy(address admin) internal pure returns (PolicyPacked memory) {
return PolicyPacked({admin: admin, reservedMiddle: 0, existsByte: EXISTS_FLAG_BYTE});
}

/// @notice Whether `packed` has its existence flag set.
/// @dev Tolerant of any non-zero `existsByte` value (in case a
/// buggy writer lands a bit anywhere in the byte) — the
/// layout-pin tests catch wrong bit positions, so this
/// helper's job is just to be a correct-by-construction
/// existence check at runtime.
function existsSet(PolicyPacked memory packed) internal pure returns (bool) {
return packed.existsByte != 0;
}

/// @custom:storage-location erc7201:base.policy_registry
struct Layout {
// Packed admin + exists flag; see header for layout.
mapping(uint64 policyId => uint256 packed) policies;
// Packed admin + exists flag via the `PolicyPacked` struct; see
// the struct definition and the header for the bit layout.
mapping(uint64 policyId => PolicyPacked packed) policies;
// ALLOWLIST member: true → authorized. BLOCKLIST member: true → blocked.
mapping(uint64 policyId => mapping(address account => bool)) members;
// Staged pending admin for in-flight two-step admin transfers.
Expand Down Expand Up @@ -100,7 +178,9 @@ library MockPolicyRegistryStorage {
// outer key first to obtain an inner base slot, then hash the inner
// key against that.

/// @notice Slot of `policies[policyId]` (the packed admin+exists uint256).
/// @notice Slot of `policies[policyId]` (the packed `PolicyPacked`
/// struct, a single 256-bit word — admin in bits 0..159, exists
/// flag at bit 255).
function policySlot(uint64 policyId) internal pure returns (bytes32) {
return keccak256(abi.encode(policyId, policiesBaseSlot()));
}
Expand All @@ -119,11 +199,21 @@ library MockPolicyRegistryStorage {
// ============================================================
// PACKED-SLOT CODECS
// ============================================================
// See the library header for the `policies[id]` layout.
// Production code accesses `policies[id]` via the `PolicyPacked`
// struct's named fields — Solidity handles the bit math automatically.
// These pure codecs operate on a raw `uint256` (what `vm.load` returns
// for the slot) and exist for test-side use only: layout-pin tests
// that read the raw slot bytes can use them to extract fields without
// re-deriving the shifts at every callsite.
//
// The roundtrip tests in `MockPolicyRegistrySlotHelpers.t.sol` verify
// that these codecs' bit math matches Solidity's struct packing — so
// a codec drifting away from the canonical struct layout fails CI.

/// @notice Bit position of the existence flag (top bit). Leaves the
/// low 160 bits for the admin lane and reserves bits 161-254
/// for future fields.
/// @notice Bit position of the `exists` flag within the packed slot.
/// Equals the high bit of `existsByte` (bit 7 of byte 31 =
/// slot bit 255). Preserved at this location as a deliberate
/// frozen-layout constraint.
uint256 internal constant EXISTS_BIT = 255;

/// @notice Extracts the policy admin (low 160 bits) from the packed slot.
Expand All @@ -132,13 +222,15 @@ library MockPolicyRegistryStorage {
return address(uint160(packed));
}

/// @notice Reads the existence flag. Lets tests distinguish "renounced"
/// (exists set, admin zero) from "never created" (both zero).
/// @notice Reads the exists flag. Lets tests distinguish "renounced"
/// (exists set, admin zero) from "never created" (slot zero).
function policyExistsFromPacked(uint256 packed) internal pure returns (bool) {
return (packed >> EXISTS_BIT) & 1 != 0;
}

/// @notice Composes a packed slot from an admin (exists bit always set).
/// Matches the binary layout Solidity emits for
/// `PolicyPacked({admin: admin, exists: true})`.
function packPolicy(address admin) internal pure returns (uint256) {
return (uint256(1) << EXISTS_BIT) | uint256(uint160(admin));
}
Expand Down