Add MockPolicyRegistry implementation#20
Merged
Conversation
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).
…built-in IDs, overflow comment
- 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
eric-ships
force-pushed
the
eric/policy-registry-impl
branch
from
9e51de3 to
6fd0607
Compare
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).
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.
- 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
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.
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.
_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.
- 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
eric-ships
changed the title
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
IPolicyRegistrywas merged to main but had no implementation. This PR adds the full reference implementation asMockPolicyRegistryintest/lib/mocks/, following the project pattern established byMockB20andMockTokenFactoryin PR #26: written as Solidity-as-if-Rust, etched at the precompile address viavm.etch, serving as both the test double and the spec artifact for the Rust precompile.What changed
test/lib/mocks/MockPolicyRegistry.sol: replaces the skeleton with a full implementation of allIPolicyRegistryfunctionstest/lib/PolicyRegistryTest.sol: adds_createAllowlist/_createBlocklisthelpers following harness lib-design conventionstest/lib/BaseTest.sol: updates stale mock-status commenttest/unit/PolicyRegistry/(13 files): implements all 76 test stubsStorage design
Each policy is stored as a single packed
uint256:Since
ALLOWLIST = 2andBLOCKLIST = 3are both non-zero,packed == 0is a clean existence sentinel even afterrenounceAdminzeroes the admin field. No separateCREATED_BITis required.Policy IDs encode the type in the top byte:
(uint8(PolicyType) << 56) | globalCounter. A single global counter (starting at 0) is shared across all types; the discriminator byte ensures no custom ID can equal built-in IDs 0 or 1.How this was tested
76 tests across 13 files — one file per
IPolicyRegistryfunction, fuzz-by-default, following harness unit-test conventions.Coverage on
MockPolicyRegistry.sol:What was tried / considered
CREATED_BITsentinel: Earlier versions used a dedicated bit at position 168 to distinguish a renounced policy (admin = 0) from a never-created slot. Dropped oncePolicyTypevalues were stored in bits [7:0] — since ALLOWLIST = 2 and BLOCKLIST = 3 are non-zero,packed != 0is sufficient without an extra bit.src/impls/PolicyRegistry.sol: The implementation started there but was moved totest/lib/mocks/to align with the project convention where full reference implementations live alongside the test suite as spec artifacts, not insrc/.Per-type vs global counter: The interface encoding doc describes a global counter shared across all types.
nextPolicyId(PolicyType)returns the full encoded ID for the given type using the current counter value; creating a policy of either type advances the counter for both.