From 2db3c18fdee0af70b6bb9e0c3c3d8b3ecd4ce1ad Mon Sep 17 00:00:00 2001 From: horsefacts Date: Mon, 14 Aug 2023 17:17:51 -0400 Subject: [PATCH] feat(StorageRegistry): add explicit pausability --- .gas-snapshot | 16 +-- foundry.toml | 1 + src/StorageRegistry.sol | 42 ++++++- test/StorageRegistry/StorageRegistry.t.sol | 123 +++++++++++++++++++++ 4 files changed, 168 insertions(+), 14 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index ae298cf4..58e66484 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,11 +1,11 @@ -BundleRegistryGasUsageTest:testGasRegisterWithSig() (gas: 827613) -BundleRegistryGasUsageTest:testGasTrustedBatchRegister() (gas: 6642776) -BundleRegistryGasUsageTest:testGasTrustedRegister() (gas: 851985) +BundleRegistryGasUsageTest:testGasRegisterWithSig() (gas: 830963) +BundleRegistryGasUsageTest:testGasTrustedBatchRegister() (gas: 6661976) +BundleRegistryGasUsageTest:testGasTrustedRegister() (gas: 855705) IdRegistryGasUsageTest:testGasRegister() (gas: 735214) IdRegistryGasUsageTest:testGasRegisterForAndRecover() (gas: 1731598) IdRegistryGasUsageTest:testGasRegisterFromTrustedCaller() (gas: 806978) -StorageRegistryGasUsageTest:testGasBatchCredit() (gas: 169333) -StorageRegistryGasUsageTest:testGasBatchRent() (gas: 267520) -StorageRegistryGasUsageTest:testGasContinuousCredit() (gas: 163030) -StorageRegistryGasUsageTest:testGasCredit() (gas: 78170) -StorageRegistryGasUsageTest:testGasRent() (gas: 159750) \ No newline at end of file +StorageRegistryGasUsageTest:testGasBatchCredit() (gas: 173053) +StorageRegistryGasUsageTest:testGasBatchRent() (gas: 270570) +StorageRegistryGasUsageTest:testGasContinuousCredit() (gas: 166530) +StorageRegistryGasUsageTest:testGasCredit() (gas: 81890) +StorageRegistryGasUsageTest:testGasRent() (gas: 163250) \ No newline at end of file diff --git a/foundry.toml b/foundry.toml index 65f7aa34..ddb5edb2 100644 --- a/foundry.toml +++ b/foundry.toml @@ -10,6 +10,7 @@ remappings = [ "chainlink/=lib/chainlink-brownie-contracts/contracts/src/" ] no_match_path = "test/Deploy/*" +libs = ["node_modules", "lib"] [profile.ci] verbosity = 3 diff --git a/src/StorageRegistry.sol b/src/StorageRegistry.sol index a22db9b2..1a705b2e 100644 --- a/src/StorageRegistry.sol +++ b/src/StorageRegistry.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.21; import {AggregatorV3Interface} from "chainlink/v0.8/interfaces/AggregatorV3Interface.sol"; import {AccessControlEnumerable} from "openzeppelin/contracts/access/AccessControlEnumerable.sol"; +import {Pausable} from "openzeppelin/contracts/security/Pausable.sol"; import {FixedPointMathLib} from "solmate/src/utils/FixedPointMathLib.sol"; import {TransferHelper} from "./lib/TransferHelper.sol"; @@ -12,7 +13,7 @@ import {TransferHelper} from "./lib/TransferHelper.sol"; * * @notice See ../docs/docs.md for an overview. */ -contract StorageRegistry is AccessControlEnumerable { +contract StorageRegistry is AccessControlEnumerable, Pausable { using FixedPointMathLib for uint256; using TransferHelper for address; @@ -439,7 +440,10 @@ contract StorageRegistry is AccessControlEnumerable { * @param fid The fid that will receive the storage units. * @param units Number of storage units to rent. */ - function rent(uint256 fid, uint256 units) external payable whenNotDeprecated returns (uint256 overpayment) { + function rent( + uint256 fid, + uint256 units + ) external payable whenNotDeprecated whenNotPaused returns (uint256 overpayment) { // Checks if (units == 0) revert InvalidAmount(); if (rentedUnits + units > maxUnits) revert ExceedsCapacity(); @@ -466,7 +470,10 @@ contract StorageRegistry is AccessControlEnumerable { * @param fids An array of fids. * @param units An array of storage unit quantities. Must be the same length as the fids array. */ - function batchRent(uint256[] calldata fids, uint256[] calldata units) external payable whenNotDeprecated { + function batchRent( + uint256[] calldata fids, + uint256[] calldata units + ) external payable whenNotDeprecated whenNotPaused { // Pre-checks if (fids.length == 0 || units.length == 0) revert InvalidBatchInput(); if (fids.length != units.length) revert InvalidBatchInput(); @@ -660,7 +667,7 @@ contract StorageRegistry is AccessControlEnumerable { * @param fid The fid that will receive the credit. * @param units Number of storage units to credit. */ - function credit(uint256 fid, uint256 units) external onlyOperator whenNotDeprecated { + function credit(uint256 fid, uint256 units) external onlyOperator whenNotDeprecated whenNotPaused { if (units == 0) revert InvalidAmount(); if (rentedUnits + units > maxUnits) revert ExceedsCapacity(); @@ -674,7 +681,10 @@ contract StorageRegistry is AccessControlEnumerable { * @param fids An array of fids. * @param units Number of storage units per fid. */ - function batchCredit(uint256[] calldata fids, uint256 units) external onlyOperator whenNotDeprecated { + function batchCredit( + uint256[] calldata fids, + uint256 units + ) external onlyOperator whenNotDeprecated whenNotPaused { if (units == 0) revert InvalidAmount(); uint256 totalUnits = fids.length * units; if (rentedUnits + totalUnits > maxUnits) revert ExceedsCapacity(); @@ -691,7 +701,11 @@ contract StorageRegistry is AccessControlEnumerable { * @param end Highest fid in sequence (inclusive). * @param units Number of storage units per fid. */ - function continuousCredit(uint256 start, uint256 end, uint256 units) external onlyOperator whenNotDeprecated { + function continuousCredit( + uint256 start, + uint256 end, + uint256 units + ) external onlyOperator whenNotDeprecated whenNotPaused { if (units == 0) revert InvalidAmount(); if (start >= end) revert InvalidRangeInput(); @@ -854,4 +868,20 @@ contract StorageRegistry is AccessControlEnumerable { emit Withdraw(vault, amount); vault.sendNative(amount); } + + /** + * @notice Pause, disabling rentals and credits. + * Only callable by owner. + */ + function pause() external onlyOwner { + _pause(); + } + + /** + * @notice Unpause, enabling rentals and credits. + * Only callable by owner. + */ + function unpause() external onlyOwner { + _unpause(); + } } diff --git a/test/StorageRegistry/StorageRegistry.t.sol b/test/StorageRegistry/StorageRegistry.t.sol index 81beee75..30162e03 100644 --- a/test/StorageRegistry/StorageRegistry.t.sol +++ b/test/StorageRegistry/StorageRegistry.t.sol @@ -434,6 +434,25 @@ contract StorageRegistryTest is StorageRegistryTestSuite { assertEq(storageRegistry.rentedUnits(), maxUnits); } + function testFuzzRentRevertsWhenPaused(address msgSender, uint256 id, uint256 units) public { + uint256 rentedBefore = storageRegistry.rentedUnits(); + + // Pause the contract. + vm.prank(owner); + storageRegistry.pause(); + + // Attempt to buy a fuzzed amount units. + units = bound(units, 1, storageRegistry.maxUnits()); + uint256 price = storageRegistry.unitPrice() * units; + vm.deal(msgSender, price); + + vm.expectRevert("Pausable: paused"); + vm.prank(msgSender); + storageRegistry.rent{value: price}(id, units); + + assertEq(storageRegistry.rentedUnits(), rentedBefore); + } + /*////////////////////////////////////////////////////////////// BATCH RENT //////////////////////////////////////////////////////////////*/ @@ -594,6 +613,46 @@ contract StorageRegistryTest is StorageRegistryTestSuite { assertEq(storageRegistry.rentedUnits(), rentedUnits); } + function testFuzzBatchRentRevertsIfPaused( + address msgSender, + uint256[] calldata _ids, + uint16[] calldata _units + ) public { + vm.prank(owner); + storageRegistry.setMaxUnits(type(uint256).max); + uint256 rentedUnits = storageRegistry.rentedUnits(); + + // Ensure that ids and units are of the same length + uint256 length = _ids.length <= _units.length ? _ids.length : _units.length; + uint256[] memory ids = new uint256[](length); + for (uint256 i; i < length; ++i) { + ids[i] = _ids[i]; + } + uint256[] memory units = new uint256[](length); + for (uint256 i; i < length; ++i) { + units[i] = _units[i]; + } + + // Ensure that the contract is paused + vm.prank(owner); + storageRegistry.pause(); + + // Deal enough funds to complete the transaction + uint256 totalUnits; + for (uint256 i; i < units.length; ++i) { + totalUnits += units[i]; + } + uint256 totalCost = storageRegistry.price(totalUnits); + vm.assume(totalUnits <= storageRegistry.maxUnits() - storageRegistry.rentedUnits()); + vm.deal(msgSender, totalCost); + + vm.prank(msgSender); + vm.expectRevert("Pausable: paused"); + storageRegistry.batchRent{value: totalCost}(ids, units); + + assertEq(storageRegistry.rentedUnits(), rentedUnits); + } + function testFuzzBatchRentRevertsEmptyArray( address msgSender, uint256[] memory ids, @@ -1264,6 +1323,15 @@ contract StorageRegistryTest is StorageRegistryTestSuite { storageRegistry.credit(fid, units); } + function testFuzzCreditRevertsIfPaused(uint256 fid, uint32 units) public { + vm.prank(owner); + storageRegistry.pause(); + + vm.expectRevert("Pausable: paused"); + vm.prank(operator); + storageRegistry.credit(fid, units); + } + function testFuzzCreditRevertsZeroUnits(uint256 fid) public { vm.expectRevert(StorageRegistry.InvalidAmount.selector); vm.prank(operator); @@ -1318,6 +1386,16 @@ contract StorageRegistryTest is StorageRegistryTestSuite { storageRegistry.batchCredit(fids, units); } + function testFuzzBatchCreditRevertsIfPaused(uint256[] calldata fids, uint32 _units) public { + uint256 units = bound(_units, 1, type(uint32).max); + vm.prank(owner); + storageRegistry.pause(); + + vm.expectRevert("Pausable: paused"); + vm.prank(operator); + storageRegistry.batchCredit(fids, units); + } + /*////////////////////////////////////////////////////////////// CONTINUOUS CREDIT //////////////////////////////////////////////////////////////*/ @@ -1397,6 +1475,17 @@ contract StorageRegistryTest is StorageRegistryTestSuite { storageRegistry.continuousCredit(start, end, units); } + function testFuzzContinuousCreditRevertsIfPaused(uint16 start, uint256 n, uint32 _units) public { + uint256 units = bound(_units, 1, type(uint32).max); + uint256 end = uint256(start) + bound(n, 0, 10000); + vm.prank(owner); + storageRegistry.pause(); + + vm.expectRevert("Pausable: paused"); + vm.prank(operator); + storageRegistry.continuousCredit(start, end, units); + } + /*////////////////////////////////////////////////////////////// SET DATA FEEDS //////////////////////////////////////////////////////////////*/ @@ -1772,6 +1861,40 @@ contract StorageRegistryTest is StorageRegistryTestSuite { storageRegistry.setVault(address(0)); } + /*////////////////////////////////////////////////////////////// + PAUSABILITY + //////////////////////////////////////////////////////////////*/ + + function testPauseUnpause() public { + assertEq(storageRegistry.paused(), false); + + vm.prank(owner); + storageRegistry.pause(); + + assertEq(storageRegistry.paused(), true); + + vm.prank(owner); + storageRegistry.unpause(); + + assertEq(storageRegistry.paused(), false); + } + + function testFuzzOnlyOwnerCanPause(address caller) public { + vm.assume(caller != owner); + + vm.prank(caller); + vm.expectRevert(StorageRegistry.NotOwner.selector); + storageRegistry.pause(); + } + + function testFuzzOnlyOwnerCanUnpause(address caller) public { + vm.assume(caller != owner); + + vm.prank(caller); + vm.expectRevert(StorageRegistry.NotOwner.selector); + storageRegistry.unpause(); + } + /*////////////////////////////////////////////////////////////// WITHDRAWAL //////////////////////////////////////////////////////////////*/