From cc0bda8e596df67b696226a8141604eff0d4af66 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 2 Jun 2026 11:09:09 -0400 Subject: [PATCH 1/2] fix(policy-registry): guard createPolicy against uint56 counter overflow (BOP-248) Add CounterOverflow error to IPolicyRegistry and revert in MockPolicyRegistry._create() when nextCounter reaches type(uint56).max, matching the Rust precompile which reverts at COUNTER_MASK = (1 << 56) - 1. Without the guard, a wrapped counter would collide with the ALWAYS_ALLOW_ID built-in sentinel. --- src/interfaces/IPolicyRegistry.sol | 5 +++++ test/lib/mocks/MockPolicyRegistry.sol | 7 ++----- test/unit/PolicyRegistry/createPolicy.t.sol | 18 ++++++++++++++++++ .../createPolicy_revertOrder.t.sol | 17 ++++++++++++++++- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/interfaces/IPolicyRegistry.sol b/src/interfaces/IPolicyRegistry.sol index 9300ea4..dff6e31 100644 --- a/src/interfaces/IPolicyRegistry.sol +++ b/src/interfaces/IPolicyRegistry.sol @@ -42,6 +42,9 @@ interface IPolicyRegistry { /// @notice `finalizeUpdateAdmin` was called with no pending admin staged. error NoPendingAdmin(); + /// @notice The policy counter has reached the maximum uint56 value and cannot be incremented. + error CounterOverflow(); + /*////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ @@ -69,6 +72,7 @@ interface IPolicyRegistry { /// @notice Creates a new policy with no initial members. Permissionless. /// /// @dev Reverts with `ZeroAddress` when `admin` is `address(0)`. + /// @dev Reverts with `CounterOverflow` when the policy counter has reached its maximum value. /// /// @param admin Initial admin authorized to modify membership and transfer or renounce administration. /// @param policyType BLOCKLIST or ALLOWLIST. @@ -80,6 +84,7 @@ interface IPolicyRegistry { /// /// @dev Reverts with `ZeroAddress` when `admin` is `address(0)`. Takes precedence over `BatchSizeTooLarge`. /// @dev Reverts with `BatchSizeTooLarge` when `accounts.length` exceeds the registry limit. + /// @dev Reverts with `CounterOverflow` when the policy counter has reached its maximum value. /// /// @param admin Initial admin authorized to modify membership and transfer or renounce administration. /// @param policyType BLOCKLIST or ALLOWLIST. diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index bec7f49..9f46564 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -243,11 +243,8 @@ contract MockPolicyRegistry is IPolicyRegistry { _writeBuiltins(); 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; - } + if (counter >= type(uint56).max) revert CounterOverflow(); + $.nextCounter = counter + 1; newPolicyId = _makeId({policyType: policyType, counter: counter}); $.policies[newPolicyId] = _encode(admin); emit PolicyCreated(newPolicyId, msg.sender, policyType); diff --git a/test/unit/PolicyRegistry/createPolicy.t.sol b/test/unit/PolicyRegistry/createPolicy.t.sol index d0afa7c..d14a77c 100644 --- a/test/unit/PolicyRegistry/createPolicy.t.sol +++ b/test/unit/PolicyRegistry/createPolicy.t.sol @@ -125,4 +125,22 @@ contract PolicyRegistryCreatePolicyTest is PolicyRegistryTest { vm.prank(caller); policyRegistry.createPolicy(admin_, pt); } + + /// @notice Verifies createPolicy reverts with CounterOverflow when the counter is at uint56 max. + /// @dev Slot-writes nextCounter to type(uint56).max to avoid iterating 2^56 times. + function test_createPolicy_revert_counterOverflow(address caller, address admin_, uint8 typeIdx) public { + _assumeValidCaller(caller); + vm.assume(admin_ != address(0)); + IPolicyRegistry.PolicyType pt = _creatablePolicyType(typeIdx); + + vm.store( + address(policyRegistry), + MockPolicyRegistryStorage.nextCounterSlot(), + bytes32(uint256(type(uint56).max)) + ); + + vm.expectRevert(IPolicyRegistry.CounterOverflow.selector); + vm.prank(caller); + policyRegistry.createPolicy(admin_, pt); + } } diff --git a/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol b/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol index 7dc9c8a..018bc51 100644 --- a/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol +++ b/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol @@ -4,13 +4,15 @@ pragma solidity ^0.8.20; import {IPolicyRegistry} from "src/interfaces/IPolicyRegistry.sol"; import {PolicyRegistryTest} from "test/lib/PolicyRegistryTest.sol"; +import {MockPolicyRegistryStorage} from "test/lib/mocks/MockPolicyRegistryStorage.sol"; /// @title Sequential revert-order test for `createPolicy`. /// /// @notice **Canonical order:** /// 1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress` +/// 2. COUNTER-OVERFLOW (nextCounter == type(uint56).max) → `CounterOverflow` /// -/// Single revert condition; walks from that condition to success. +/// Walks from the first failing condition to success. contract PolicyRegistryCreatePolicyRevertOrderTest is PolicyRegistryTest { /// @notice Walks through every revert in canonical order, fixing one per step, ending at success. function test_createPolicy_revertOrder(address caller, address admin_, uint8 typeIdx) public { @@ -19,12 +21,25 @@ contract PolicyRegistryCreatePolicyRevertOrderTest is PolicyRegistryTest { IPolicyRegistry.PolicyType pt = _creatablePolicyType(typeIdx); // 1. ZERO-ADMIN: admin == address(0) → ZeroAddress + vm.store( + address(policyRegistry), + MockPolicyRegistryStorage.nextCounterSlot(), + bytes32(uint256(type(uint56).max)) + ); vm.prank(caller); vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); policyRegistry.createPolicy(address(0), pt); // Fix: use a non-zero admin. + // 2. COUNTER-OVERFLOW: nextCounter == type(uint56).max → CounterOverflow + vm.prank(caller); + vm.expectRevert(IPolicyRegistry.CounterOverflow.selector); + policyRegistry.createPolicy(admin_, pt); + + // Fix: reset counter to a valid value. + vm.store(address(policyRegistry), MockPolicyRegistryStorage.nextCounterSlot(), bytes32(uint256(2))); + // Success vm.prank(caller); policyRegistry.createPolicy(admin_, pt); From aaca50253547ae7b6b183de08d632d2af61c166b Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Tue, 2 Jun 2026 11:30:10 -0400 Subject: [PATCH 2/2] fix(policy-registry): align counter overflow to Panic(0x11), not custom error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the CounterOverflow custom error with natural Solidity checked arithmetic overflow, which emits Panic(0x11) — matching the Rust precompile which reverts with BasePrecompileError::Panic(PanicKind::UnderOverflow) at the same boundary. Update tests to expect abi.encodeWithSignature("Panic(uint256)", 0x11). --- src/interfaces/IPolicyRegistry.sol | 7 ++----- test/lib/mocks/MockPolicyRegistry.sol | 3 ++- test/unit/PolicyRegistry/createPolicy.t.sol | 9 ++++----- .../unit/PolicyRegistry/createPolicy_revertOrder.t.sol | 10 ++++------ 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/interfaces/IPolicyRegistry.sol b/src/interfaces/IPolicyRegistry.sol index dff6e31..bc767c3 100644 --- a/src/interfaces/IPolicyRegistry.sol +++ b/src/interfaces/IPolicyRegistry.sol @@ -42,9 +42,6 @@ interface IPolicyRegistry { /// @notice `finalizeUpdateAdmin` was called with no pending admin staged. error NoPendingAdmin(); - /// @notice The policy counter has reached the maximum uint56 value and cannot be incremented. - error CounterOverflow(); - /*////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ @@ -72,7 +69,7 @@ interface IPolicyRegistry { /// @notice Creates a new policy with no initial members. Permissionless. /// /// @dev Reverts with `ZeroAddress` when `admin` is `address(0)`. - /// @dev Reverts with `CounterOverflow` when the policy counter has reached its maximum value. + /// @dev Panics with arithmetic overflow (Panic 0x11) when the policy counter has reached its maximum value. /// /// @param admin Initial admin authorized to modify membership and transfer or renounce administration. /// @param policyType BLOCKLIST or ALLOWLIST. @@ -84,7 +81,7 @@ interface IPolicyRegistry { /// /// @dev Reverts with `ZeroAddress` when `admin` is `address(0)`. Takes precedence over `BatchSizeTooLarge`. /// @dev Reverts with `BatchSizeTooLarge` when `accounts.length` exceeds the registry limit. - /// @dev Reverts with `CounterOverflow` when the policy counter has reached its maximum value. + /// @dev Panics with arithmetic overflow (Panic 0x11) when the policy counter has reached its maximum value. /// /// @param admin Initial admin authorized to modify membership and transfer or renounce administration. /// @param policyType BLOCKLIST or ALLOWLIST. diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index 9f46564..c20462d 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -243,7 +243,8 @@ contract MockPolicyRegistry is IPolicyRegistry { _writeBuiltins(); MockPolicyRegistryStorage.Layout storage $ = MockPolicyRegistryStorage.layout(); uint56 counter = $.nextCounter; - if (counter >= type(uint56).max) revert CounterOverflow(); + // Solidity checked arithmetic panics with Panic(0x11) on uint56 overflow, + // matching the Rust precompile which reverts with Panic(UnderOverflow) at COUNTER_MASK. $.nextCounter = counter + 1; newPolicyId = _makeId({policyType: policyType, counter: counter}); $.policies[newPolicyId] = _encode(admin); diff --git a/test/unit/PolicyRegistry/createPolicy.t.sol b/test/unit/PolicyRegistry/createPolicy.t.sol index d14a77c..97d10c1 100644 --- a/test/unit/PolicyRegistry/createPolicy.t.sol +++ b/test/unit/PolicyRegistry/createPolicy.t.sol @@ -126,20 +126,19 @@ contract PolicyRegistryCreatePolicyTest is PolicyRegistryTest { policyRegistry.createPolicy(admin_, pt); } - /// @notice Verifies createPolicy reverts with CounterOverflow when the counter is at uint56 max. + /// @notice Verifies createPolicy panics with arithmetic overflow when the counter is at uint56 max. /// @dev Slot-writes nextCounter to type(uint56).max to avoid iterating 2^56 times. + /// Matches the Rust precompile which reverts with Panic(UnderOverflow) = Panic(0x11). function test_createPolicy_revert_counterOverflow(address caller, address admin_, uint8 typeIdx) public { _assumeValidCaller(caller); vm.assume(admin_ != address(0)); IPolicyRegistry.PolicyType pt = _creatablePolicyType(typeIdx); vm.store( - address(policyRegistry), - MockPolicyRegistryStorage.nextCounterSlot(), - bytes32(uint256(type(uint56).max)) + address(policyRegistry), MockPolicyRegistryStorage.nextCounterSlot(), bytes32(uint256(type(uint56).max)) ); - vm.expectRevert(IPolicyRegistry.CounterOverflow.selector); + vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11)); vm.prank(caller); policyRegistry.createPolicy(admin_, pt); } diff --git a/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol b/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol index 018bc51..936f490 100644 --- a/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol +++ b/test/unit/PolicyRegistry/createPolicy_revertOrder.t.sol @@ -10,7 +10,7 @@ import {MockPolicyRegistryStorage} from "test/lib/mocks/MockPolicyRegistryStorag /// /// @notice **Canonical order:** /// 1. ZERO-ADMIN (`admin == address(0)`) → `ZeroAddress` -/// 2. COUNTER-OVERFLOW (nextCounter == type(uint56).max) → `CounterOverflow` +/// 2. COUNTER-OVERFLOW (nextCounter == type(uint56).max) → `Panic(0x11)` /// /// Walks from the first failing condition to success. contract PolicyRegistryCreatePolicyRevertOrderTest is PolicyRegistryTest { @@ -22,9 +22,7 @@ contract PolicyRegistryCreatePolicyRevertOrderTest is PolicyRegistryTest { // 1. ZERO-ADMIN: admin == address(0) → ZeroAddress vm.store( - address(policyRegistry), - MockPolicyRegistryStorage.nextCounterSlot(), - bytes32(uint256(type(uint56).max)) + address(policyRegistry), MockPolicyRegistryStorage.nextCounterSlot(), bytes32(uint256(type(uint56).max)) ); vm.prank(caller); vm.expectRevert(IPolicyRegistry.ZeroAddress.selector); @@ -32,9 +30,9 @@ contract PolicyRegistryCreatePolicyRevertOrderTest is PolicyRegistryTest { // Fix: use a non-zero admin. - // 2. COUNTER-OVERFLOW: nextCounter == type(uint56).max → CounterOverflow + // 2. COUNTER-OVERFLOW: nextCounter == type(uint56).max → Panic(0x11) vm.prank(caller); - vm.expectRevert(IPolicyRegistry.CounterOverflow.selector); + vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11)); policyRegistry.createPolicy(admin_, pt); // Fix: reset counter to a valid value.