Skip to content

Correctness fixes on top of policy-registry impl #11

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 3 commits into
base: impl/policy-registry
Choose a base branch
from
+250 −57
Draft

Correctness fixes on top of policy-registry impl #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c816cac
Pack constituent type bit in compound slot for 2-SLOAD hot path
amiecorso May 17, 2026
d1bba93
Bound policy ID counter to 61-bit packable range
amiecorso May 17, 2026
91faa39
Two-step admin transfer + one-way freezePolicy
amiecorso May 17, 2026
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
141 changes: 113 additions & 28 deletions src/impls/PolicyRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import {PolicySlot} from "./PolicySlot.sol";
/// This is safe because WHITELIST/BLACKLIST require a non-zero admin (so packed
/// is never zero), and COMPOUND always has type byte = 2 (so packed is never zero).
///
/// Authorization cost: at most 2 SLOADs for any policy. For a compound policy,
/// one SLOAD reads the packed slot (yielding all four constituent IDs), and one SLOAD
/// reads the relevant constituent's member set. Compound constituents cannot themselves be
/// Authorization cost: exactly 2 SLOADs for any custom policy (zero for built-ins).
/// Compound policies pack each constituent's type bit alongside its ID in the
/// compound slot, so a hot-path check reads only (a) the compound slot and
/// (b) the relevant constituent's member set — the constituent's own policy
/// slot does NOT need to be loaded. Compound constituents cannot themselves be
/// COMPOUND (enforced at creation), so evaluation never goes deeper.
contract PolicyRegistry is IPolicyRegistry {
using PolicySlot for uint256;
Expand All @@ -28,6 +30,12 @@ contract PolicyRegistry is IPolicyRegistry {
// BLACKLIST: member == true means the address is restricted.
mapping(uint64 policyId => mapping(address account => bool)) private _members;

// Pending admin per policy. Set by beginPolicyAdminTransfer, cleared on
// accept/cancel/freeze. Kept in its own mapping (rather than packed into the
// policy slot) so the hot-path read on _policyData stays minimal; admin
// rotation is a cold path and the extra SLOAD on accept is acceptable.
mapping(uint64 policyId => address pendingAdmin) private _pendingAdmins;

uint64 private _counter = FIRST_USER_POLICY_ID;

/*//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -73,13 +81,19 @@ contract PolicyRegistry is IPolicyRegistry {
uint64 mintRecipientPolicyId,
uint64 redeemerPolicyId
) external returns (uint64 newPolicyId) {
_requireConstituent(senderPolicyId);
_requireConstituent(recipientPolicyId);
_requireConstituent(mintRecipientPolicyId);
_requireConstituent(redeemerPolicyId);
// Resolve each constituent's type once, at creation, so the hot path can read
// the constituent's type bit directly from the compound slot.
PolicyType senderType = _requireConstituent(senderPolicyId);
PolicyType recipientType = _requireConstituent(recipientPolicyId);
PolicyType mintRecipientType = _requireConstituent(mintRecipientPolicyId);
PolicyType redeemerType = _requireConstituent(redeemerPolicyId);
newPolicyId = _nextPolicyId();
_policyData[newPolicyId] =
PolicySlot.encodeCompound(senderPolicyId, recipientPolicyId, mintRecipientPolicyId, redeemerPolicyId);
_policyData[newPolicyId] = PolicySlot.encodeCompound(
PolicySlot.encodeField(senderPolicyId, senderType),
PolicySlot.encodeField(recipientPolicyId, recipientType),
PolicySlot.encodeField(mintRecipientPolicyId, mintRecipientType),
PolicySlot.encodeField(redeemerPolicyId, redeemerType)
);
emit CompoundPolicyCreated(
newPolicyId, msg.sender, senderPolicyId, recipientPolicyId, mintRecipientPolicyId, redeemerPolicyId
);
Expand All @@ -90,20 +104,67 @@ contract PolicyRegistry is IPolicyRegistry {
//////////////////////////////////////////////////////////////*/

/// @inheritdoc IPolicyRegistry
function setPolicyAdmin(uint64 policyId, address newAdmin) external {
if (newAdmin == address(0)) revert ZeroAddress();
function beginPolicyAdminTransfer(uint64 policyId, address newAdmin) external {
// Permitting newAdmin == address(0) here is intentional: it lets the current
// admin cancel a prior nomination via the same entry point. The pending slot
// is just a nomination; the active admin doesn't change until accept.
uint256 packed = _requireExists(policyId);
PolicyType policyType = packed.decodeType();
if (policyType == PolicyType.COMPOUND) revert IncompatiblePolicyType();
if (packed.decodeFrozen()) revert PolicyFrozen();
if (packed.decodeAdmin() != msg.sender) revert Unauthorized();
_pendingAdmins[policyId] = newAdmin;
emit PolicyAdminTransferBegun(policyId, msg.sender, newAdmin);
}

/// @inheritdoc IPolicyRegistry
function acceptPolicyAdminTransfer(uint64 policyId) external {
uint256 packed = _requireExists(policyId);
PolicyType policyType = packed.decodeType();
if (policyType == PolicyType.COMPOUND) revert IncompatiblePolicyType();
if (packed.decodeFrozen()) revert PolicyFrozen();
address pending = _pendingAdmins[policyId];
if (pending != msg.sender) revert NotPendingAdmin();
// Preserve the frozen bit on rotation. Currently unreachable because the
// freeze check above short-circuits, but coded defensively in case the
// freeze semantics evolve (e.g. allowing rotation while frozen).
_policyData[policyId] = PolicySlot.encodeSimple(policyType, msg.sender)
| (packed.decodeFrozen() ? PolicySlot.FROZEN_BIT : 0);
delete _pendingAdmins[policyId];
emit PolicyAdminUpdated(policyId, msg.sender, msg.sender);
}

/// @inheritdoc IPolicyRegistry
function cancelPolicyAdminTransfer(uint64 policyId) external {
uint256 packed = _requireExists(policyId);
PolicyType policyType = packed.decodeType();
if (policyType == PolicyType.COMPOUND) revert IncompatiblePolicyType();
if (packed.decodeAdmin() != msg.sender) revert Unauthorized();
_policyData[policyId] = PolicySlot.encodeSimple(policyType, newAdmin);
emit PolicyAdminUpdated(policyId, msg.sender, newAdmin);
address pending = _pendingAdmins[policyId];
if (pending == address(0)) revert NoTransferPending();
delete _pendingAdmins[policyId];
emit PolicyAdminTransferCancelled(policyId, msg.sender, pending);
}

/// @inheritdoc IPolicyRegistry
function freezePolicy(uint64 policyId) external {
uint256 packed = _requireExists(policyId);
PolicyType policyType = packed.decodeType();
if (policyType == PolicyType.COMPOUND) revert IncompatiblePolicyType();
if (packed.decodeFrozen()) revert PolicyFrozen();
if (packed.decodeAdmin() != msg.sender) revert Unauthorized();
_policyData[policyId] = packed | PolicySlot.FROZEN_BIT;
// Any in-flight nomination is moot once frozen; clear it so the post-freeze
// state is unambiguous (no zombie pending admin lingering in storage).
if (_pendingAdmins[policyId] != address(0)) delete _pendingAdmins[policyId];
emit PolicyFrozenEvent(policyId, msg.sender);
}

/// @inheritdoc IPolicyRegistry
function modifyPolicyWhitelist(uint64 policyId, address account, bool allowed) external {
uint256 packed = _requireExists(policyId);
if (packed.decodeType() != PolicyType.WHITELIST) revert IncompatiblePolicyType();
if (packed.decodeFrozen()) revert PolicyFrozen();
if (packed.decodeAdmin() != msg.sender) revert Unauthorized();
_members[policyId][account] = allowed;
emit WhitelistUpdated(policyId, msg.sender, account, allowed);
Expand All @@ -113,6 +174,7 @@ contract PolicyRegistry is IPolicyRegistry {
function modifyPolicyBlacklist(uint64 policyId, address account, bool restricted) external {
uint256 packed = _requireExists(policyId);
if (packed.decodeType() != PolicyType.BLACKLIST) revert IncompatiblePolicyType();
if (packed.decodeFrozen()) revert PolicyFrozen();
if (packed.decodeAdmin() != msg.sender) revert Unauthorized();
_members[policyId][account] = restricted;
emit BlacklistUpdated(policyId, msg.sender, account, restricted);
Expand Down Expand Up @@ -172,6 +234,20 @@ contract PolicyRegistry is IPolicyRegistry {
admin = policyType == PolicyType.COMPOUND ? address(0) : packed.decodeAdmin();
}

/// @inheritdoc IPolicyRegistry
function pendingPolicyAdmin(uint64 policyId) external view returns (address) {
return _pendingAdmins[policyId];
}

/// @inheritdoc IPolicyRegistry
function isPolicyFrozen(uint64 policyId) external view returns (bool) {
if (policyId < FIRST_USER_POLICY_ID) return false;
uint256 packed = _policyData[policyId];
if (packed == 0) return false;
if (packed.decodeType() == PolicyType.COMPOUND) return false;
return packed.decodeFrozen();
}

/// @inheritdoc IPolicyRegistry
function compoundPolicyData(uint64 policyId)
external
Expand All @@ -180,10 +256,10 @@ contract PolicyRegistry is IPolicyRegistry {
{
uint256 packed = _requireExists(policyId);
if (packed.decodeType() != PolicyType.COMPOUND) revert IncompatiblePolicyType();
senderPolicyId = packed.decodeIdAt(PolicySlot.SENDER_SHIFT);
recipientPolicyId = packed.decodeIdAt(PolicySlot.RECIPIENT_SHIFT);
mintRecipientPolicyId = packed.decodeIdAt(PolicySlot.MINT_SHIFT);
redeemerPolicyId = packed.decodeIdAt(PolicySlot.REDEEM_SHIFT);
senderPolicyId = packed.decodeId(PolicySlot.SENDER_SHIFT);
recipientPolicyId = packed.decodeId(PolicySlot.RECIPIENT_SHIFT);
mintRecipientPolicyId = packed.decodeId(PolicySlot.MINT_SHIFT);
redeemerPolicyId = packed.decodeId(PolicySlot.REDEEM_SHIFT);
}

/*//////////////////////////////////////////////////////////////
Expand All @@ -192,6 +268,12 @@ contract PolicyRegistry is IPolicyRegistry {

function _nextPolicyId() internal returns (uint64 id) {
id = _counter++;
// The on-chain compound-policy packing reserves 61 bits per constituent ID.
// Bounding the counter here guarantees that every policy ID ever created can
// be round-tripped through the compound slot without silent truncation; the
// packed-field decoders elsewhere in this contract can therefore rely on
// ID <= PolicySlot.ID_MASK without re-checking.
if (id > PolicySlot.ID_MASK) revert PolicyIdOverflow();
}

function _createPolicy(address admin, PolicyType policyType) internal returns (uint64 newPolicyId) {
Expand All @@ -215,21 +297,24 @@ contract PolicyRegistry is IPolicyRegistry {
if (packed == 0) revert PolicyNotFound();
}

// Validates that policyId is a legal compound constituent: must exist and must be
// WHITELIST or BLACKLIST. Built-in IDs 0 and 1 are always valid.
function _requireConstituent(uint64 policyId) internal view {
if (policyId < FIRST_USER_POLICY_ID) return;
// Validates that policyId is a legal compound constituent and returns its type.
// Must exist and must be WHITELIST or BLACKLIST. Built-in IDs (0, 1) are always
// valid; their returned type is WHITELIST as a placeholder (the type bit is
// ignored on the hot path for built-ins because evaluation short-circuits on ID).
function _requireConstituent(uint64 policyId) internal view returns (PolicyType) {
if (policyId < FIRST_USER_POLICY_ID) return PolicyType.WHITELIST;
uint256 packed = _policyData[policyId];
if (packed == 0) revert PolicyNotFound();
PolicyType policyType = packed.decodeType();
if (policyType != PolicyType.WHITELIST && policyType != PolicyType.BLACKLIST) revert ConstituentIsCompound();
return policyType;
}

// Resolves an authorization check for a single role slot. The shift selects
// which 62-bit field to read from a compound policy's packed slot.
// which 62-bit constituent field to read from a compound policy's packed slot.
//
// For a compound policyId: 1 SLOAD (compound slot) + 1 SLOAD (member set) = 2 SLOADs
// For a simple policyId: 1 SLOAD (member set) = 1 SLOAD
// For a simple policyId: 1 SLOAD (policy slot) + 1 SLOAD (member set) = 2 SLOADs
// For a built-in policyId: 0 SLOADs
function _checkRole(uint64 policyId, address user, uint256 shift) internal view returns (bool) {
if (policyId == ALWAYS_REJECT_ID) return false;
Expand All @@ -239,14 +324,14 @@ contract PolicyRegistry is IPolicyRegistry {
PolicyType policyType = packed.decodeType();

if (policyType == PolicyType.COMPOUND) {
uint64 constituentId = packed.decodeIdAt(shift);
(uint64 constituentId, bool isBlacklist) = packed.decodeField(shift);
if (constituentId == ALWAYS_REJECT_ID) return false;
if (constituentId == ALWAYS_ALLOW_ID) return true;
packed = _policyData[constituentId];
policyType = packed.decodeType();
return policyType == PolicyType.WHITELIST ? _members[constituentId][user] : !_members[constituentId][user];
bool member = _members[constituentId][user];
return isBlacklist ? !member : member;
}

return policyType == PolicyType.WHITELIST ? _members[policyId][user] : !_members[policyId][user];
bool simpleMember = _members[policyId][user];
return policyType == PolicyType.WHITELIST ? simpleMember : !simpleMember;
}
}
85 changes: 63 additions & 22 deletions src/impls/PolicySlot.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,47 +10,73 @@ import {IPolicyRegistry} from "../interfaces/IPolicyRegistry.sol";
/// PolicyType discriminator; the remaining bits depend on the type:
///
/// WHITELIST / BLACKLIST:
/// [255:168] unused
/// [255:169] unused
/// [168] frozen flag (1 = policy is permanently immutable)
/// [167:8] admin address (160 bits)
/// [7:0] PolicyType
///
/// COMPOUND:
/// [255:194] redeemerPolicyId (62 bits)
/// [193:132] mintRecipientPolicyId (62 bits)
/// [131:70] recipientPolicyId (62 bits)
/// [69:8] senderPolicyId (62 bits)
/// [255:194] redeemerField (62 bits = 1 type bit + 61 ID bits)
/// [193:132] mintRecipientField (62 bits)
/// [131:70] recipientField (62 bits)
/// [69:8] senderField (62 bits)
/// [7:0] PolicyType = 2
///
/// Constituent policy IDs are stored as 62 bits rather than 64. This lets the type
/// byte plus all four IDs fit in exactly one 256-bit slot (8 + 4*62 = 256).
/// Policy IDs are uint64 in the interface but packed as 62 bits in compound slots.
/// _policyIdCounter would need to reach 2^62 (~4.6e18) for truncation to occur,
/// which is not practically possible.
/// Each constituent field carries both the constituent policy ID (61 bits)
/// and a single type bit (0 = WHITELIST or built-in, 1 = BLACKLIST) so that
/// `isAuthorized*` evaluation needs at most one SLOAD to read the compound
/// slot plus one SLOAD to read the relevant constituent's member set — the
/// constituent's policy slot does NOT need to be loaded on the hot path.
/// The type bit is meaningless for built-in constituents (IDs 0 and 1)
/// because evaluation short-circuits on ID before consulting type.
///
/// Constituent policy IDs are stored as 61 bits. The contract bounds the
/// policy ID counter to never exceed `ID_MASK` so this truncation is
/// structurally impossible.
///
/// All functions are internal so they are inlined at compile time with no
/// runtime overhead.
library PolicySlot {
uint256 internal constant TYPE_MASK = 0xFF;
uint256 internal constant ID_BITS = 62;

// Simple-policy layout.
uint256 internal constant ADMIN_SHIFT = 8;
uint256 internal constant FROZEN_SHIFT = 168;
uint256 internal constant FROZEN_BIT = uint256(1) << FROZEN_SHIFT;

// Compound-policy layout.
uint256 internal constant ID_BITS = 61;
uint256 internal constant ID_MASK = (uint256(1) << ID_BITS) - 1;
uint256 internal constant FIELD_BITS = 62; // 1 type bit + 61 ID bits
uint256 internal constant FIELD_MASK = (uint256(1) << FIELD_BITS) - 1;
uint256 internal constant TYPE_BIT_OFFSET = ID_BITS; // type bit sits above the 61-bit ID
uint256 internal constant TYPE_BIT = uint256(1) << TYPE_BIT_OFFSET;

uint256 internal constant SENDER_SHIFT = 8;
uint256 internal constant RECIPIENT_SHIFT = SENDER_SHIFT + ID_BITS; // 70
uint256 internal constant MINT_SHIFT = RECIPIENT_SHIFT + ID_BITS; // 132
uint256 internal constant REDEEM_SHIFT = MINT_SHIFT + ID_BITS; // 194
uint256 internal constant RECIPIENT_SHIFT = SENDER_SHIFT + FIELD_BITS; // 70
uint256 internal constant MINT_SHIFT = RECIPIENT_SHIFT + FIELD_BITS; // 132
uint256 internal constant REDEEM_SHIFT = MINT_SHIFT + FIELD_BITS; // 194

function encodeSimple(IPolicyRegistry.PolicyType policyType, address policyAdmin) internal pure returns (uint256) {
return uint256(policyType) | (uint256(uint160(policyAdmin)) << 8);
return uint256(policyType) | (uint256(uint160(policyAdmin)) << ADMIN_SHIFT);
}

/// @dev Packs a constituent (policy ID + type bit) into a single 62-bit field.
/// Built-in IDs (0, 1) can be passed with any `constituentType`; the type
/// bit is ignored at decode time for built-ins.
function encodeField(uint64 id, IPolicyRegistry.PolicyType constituentType) internal pure returns (uint256) {
uint256 typeBit = constituentType == IPolicyRegistry.PolicyType.BLACKLIST ? uint256(1) : uint256(0);
return uint256(id) | (typeBit << TYPE_BIT_OFFSET);
}

function encodeCompound(uint64 sender, uint64 recipient, uint64 mintRecipient, uint64 redeemer)
/// @dev Composes a full compound-policy slot from four pre-encoded constituent fields.
function encodeCompound(uint256 senderField, uint256 recipientField, uint256 mintField, uint256 redeemerField)
internal
pure
returns (uint256)
{
return uint256(IPolicyRegistry.PolicyType.COMPOUND) | (uint256(sender) << SENDER_SHIFT)
| (uint256(recipient) << RECIPIENT_SHIFT) | (uint256(mintRecipient) << MINT_SHIFT)
| (uint256(redeemer) << REDEEM_SHIFT);
return uint256(IPolicyRegistry.PolicyType.COMPOUND) | (senderField << SENDER_SHIFT)
| (recipientField << RECIPIENT_SHIFT) | (mintField << MINT_SHIFT) | (redeemerField << REDEEM_SHIFT);
}

function decodeType(uint256 packed) internal pure returns (IPolicyRegistry.PolicyType) {
Expand All @@ -59,12 +85,27 @@ library PolicySlot {

function decodeAdmin(uint256 packed) internal pure returns (address) {
// forge-lint: disable-next-line(unsafe-typecast)
return address(uint160(packed >> 8));
return address(uint160(packed >> ADMIN_SHIFT));
}

// Extracts the constituent policy ID at the given shift offset.
function decodeIdAt(uint256 packed, uint256 shift) internal pure returns (uint64) {
function decodeFrozen(uint256 packed) internal pure returns (bool) {
return (packed & FROZEN_BIT) != 0;
}

/// @dev Extracts the constituent policy ID at the given shift offset.
/// Used by view functions that only care about the ID, not the type.
function decodeId(uint256 packed, uint256 shift) internal pure returns (uint64) {
// forge-lint: disable-next-line(unsafe-typecast)
return uint64((packed >> shift) & ID_MASK);
}

/// @dev Extracts the constituent policy ID and type bit in one operation.
/// Used on the authorization hot path so the constituent's own policy slot
/// does not need to be loaded.
function decodeField(uint256 packed, uint256 shift) internal pure returns (uint64 id, bool isBlacklist) {
uint256 field = (packed >> shift) & FIELD_MASK;
// forge-lint: disable-next-line(unsafe-typecast)
id = uint64(field & ID_MASK);
isBlacklist = (field & TYPE_BIT) != 0;
}
}
Loading