diff --git a/src/chain/ResourceConstraintManager.sol b/src/chain/ResourceConstraintManager.sol index 2913e139..9f2a69f1 100644 --- a/src/chain/ResourceConstraintManager.sol +++ b/src/chain/ResourceConstraintManager.sol @@ -23,6 +23,7 @@ contract ResourceConstraintManager is AccessControlEnumerable { uint64 public constant MAX_PRICING_EXPONENT = 8000; // scaled by 1000 to allow for fractional exponents bytes32 public constant MANAGER_ROLE = keccak256("MANAGER_ROLE"); + uint256 public constant TWO_YEARS_IN_SECONDS = 365 days * 2; uint256 public expiryTimestamp; error TooManyConstraints(); @@ -36,15 +37,23 @@ contract ResourceConstraintManager is AccessControlEnumerable { error PricingExponentTooHigh(uint64 pricingExponent); error NotExpired(); - constructor(address admin, address manager, uint256 _expiryTimestamp) { + constructor(address admin, address manager) { _setupRole(DEFAULT_ADMIN_ROLE, admin); _setupRole(MANAGER_ROLE, manager); - expiryTimestamp = _expiryTimestamp; + } + + /// @notice Sets the expiry timestamp for the current manager contract if it has not been set already. + /// This is called at the beginning of the setGasPricingConstraints and setMultiGasPricingConstraints functions + /// to ensure that this contract can only be used for 2 years from the moment it is first used. + function setExpiryTimestamp() internal { + if (expiryTimestamp == 0) { + expiryTimestamp = block.timestamp + TWO_YEARS_IN_SECONDS; + } } /// @notice Removes the contract from the list of chain owners after the expiry timestamp function revoke() external { - if (block.timestamp < expiryTimestamp) { + if (expiryTimestamp == 0 || block.timestamp < expiryTimestamp) { revert NotExpired(); } ARB_OWNER.removeChainOwner(address(this)); @@ -59,6 +68,9 @@ contract ResourceConstraintManager is AccessControlEnumerable { function setGasPricingConstraints( uint64[3][] calldata constraints ) external onlyRole(MANAGER_ROLE) { + // Set the expiry timestamp in the first call to either `set` function + setExpiryTimestamp(); + // If zero constraints are provided, the chain uses the single-constraint pricing model uint256 nConstraints = constraints.length; if (nConstraints > MAX_SINGLE_GAS_CONSTRAINTS) { @@ -112,6 +124,9 @@ contract ResourceConstraintManager is AccessControlEnumerable { function setMultiGasPricingConstraints( ArbMultiGasConstraintsTypes.ResourceConstraint[] calldata constraints ) external onlyRole(MANAGER_ROLE) { + // Set the expiry timestamp in the first call to either `set` function + setExpiryTimestamp(); + // If zero constraints are provided, the chain uses the single-constraint pricing model // Each constraint adds a small amount of overhead to the gas cost of each transaction and block, so we limit the number of constraints that can be set uint256 nConstraints = constraints.length; diff --git a/test/foundry/ResourceConstraintManager.t.sol b/test/foundry/ResourceConstraintManager.t.sol index 020b4950..cede75c4 100644 --- a/test/foundry/ResourceConstraintManager.t.sol +++ b/test/foundry/ResourceConstraintManager.t.sol @@ -10,26 +10,108 @@ contract ResourceConstraintManagerTest is Test { address constant admin = address(1337); address constant manager = address(7331); - uint256 constant expiryTimestamp = 12345678; constructor() { - resourceConstraintManager = new ResourceConstraintManager(admin, manager, expiryTimestamp); + resourceConstraintManager = new ResourceConstraintManager(admin, manager); vm.etch(address(ARB_OWNER), type(ArbOwnerMock).runtimeCode); } function test_revoke() external { - // Test before expiry - vm.warp(expiryTimestamp - 1); + // Trigger the expiry timestamp via the first set call + uint256 startTime = 1_000_000; + vm.warp(startTime); + uint64[3][] memory constraints = new uint64[3][](1); + constraints[0] = [uint64(10_000_000), uint64(100), uint64(0)]; + vm.prank(manager); + resourceConstraintManager.setGasPricingConstraints(constraints); + + uint256 expiry = startTime + resourceConstraintManager.TWO_YEARS_IN_SECONDS(); + assertEq(resourceConstraintManager.expiryTimestamp(), expiry); + + // One second before expiry should revert + vm.warp(expiry - 1); vm.expectRevert(ResourceConstraintManager.NotExpired.selector); resourceConstraintManager.revoke(); - // Test after expiry - vm.warp(expiryTimestamp); + // Exactly at expiry should succeed + vm.warp(expiry); assertFalse(ARB_OWNER.removeChainOwnerCalled()); resourceConstraintManager.revoke(); assertTrue(ARB_OWNER.removeChainOwnerCalled()); } + function test_revoke_neverUsed() external { + // If no set call has been made, expiryTimestamp is 0 and revoke must revert + assertEq(resourceConstraintManager.expiryTimestamp(), 0); + vm.expectRevert(ResourceConstraintManager.NotExpired.selector); + resourceConstraintManager.revoke(); + + // Warping far forward does not change that — expiry was never initialized + vm.warp(type(uint64).max); + vm.expectRevert(ResourceConstraintManager.NotExpired.selector); + resourceConstraintManager.revoke(); + } + + function test_expiryTimestamp_notUpdatedOnSubsequentCalls() external { + uint256 startTime = 1_000_000; + vm.warp(startTime); + + uint64[3][] memory constraints = new uint64[3][](1); + constraints[0] = [uint64(10_000_000), uint64(100), uint64(0)]; + vm.prank(manager); + resourceConstraintManager.setGasPricingConstraints(constraints); + + uint256 originalExpiry = resourceConstraintManager.expiryTimestamp(); + assertEq(originalExpiry, startTime + resourceConstraintManager.TWO_YEARS_IN_SECONDS()); + + // A later call must not shift the expiry forward + vm.warp(startTime + 1 days); + vm.prank(manager); + resourceConstraintManager.setGasPricingConstraints(constraints); + assertEq(resourceConstraintManager.expiryTimestamp(), originalExpiry); + + // Same guarantee after a much larger gap + vm.warp(startTime + 365 days); + vm.prank(manager); + resourceConstraintManager.setGasPricingConstraints(constraints); + assertEq(resourceConstraintManager.expiryTimestamp(), originalExpiry); + } + + function test_expiryTimestamp_sharedAcrossSetters() external { + uint64[3][] memory singleConstraints = new uint64[3][](1); + singleConstraints[0] = [uint64(10_000_000), uint64(100), uint64(0)]; + ArbMultiGasConstraintsTypes.ResourceConstraint[] memory multiConstraints = + new ArbMultiGasConstraintsTypes.ResourceConstraint[](1); + multiConstraints[0] = _createMultiGasConstraint(10_000_000, 100, 0); + + // Case 1: single-dim first, multi-dim second — multi-dim must not reset expiry + uint256 startTime1 = 1_000_000; + vm.warp(startTime1); + vm.prank(manager); + resourceConstraintManager.setGasPricingConstraints(singleConstraints); + uint256 expiry1 = resourceConstraintManager.expiryTimestamp(); + assertEq(expiry1, startTime1 + resourceConstraintManager.TWO_YEARS_IN_SECONDS()); + + vm.warp(startTime1 + 1 days); + vm.prank(manager); + resourceConstraintManager.setMultiGasPricingConstraints(multiConstraints); + assertEq(resourceConstraintManager.expiryTimestamp(), expiry1); + + // Case 2: multi-dim first, single-dim second — single-dim must not reset expiry + uint256 startTime2 = 2_000_000; + vm.warp(startTime2); + ResourceConstraintManager rcm2 = new ResourceConstraintManager(admin, manager); + vm.prank(manager); + rcm2.setMultiGasPricingConstraints(multiConstraints); + uint256 expiry2 = rcm2.expiryTimestamp(); + assertEq(expiry2, startTime2 + rcm2.TWO_YEARS_IN_SECONDS()); + + vm.warp(startTime2 + 1 days); + vm.prank(manager); + rcm2.setGasPricingConstraints(singleConstraints); + assertEq(rcm2.expiryTimestamp(), expiry2); + } + // // --- setGasPricingConstraints tests --- //