From 741eec3aee0d9ba0266142e071fb69d4de9e89ce Mon Sep 17 00:00:00 2001 From: horsefacts Date: Tue, 22 Aug 2023 12:03:35 -0400 Subject: [PATCH] feat: add Pausable to KeyRegistry --- .gas-snapshot | 4 +- src/KeyRegistry.sol | 25 +++- test/KeyRegistry/KeyRegistry.t.sol | 194 +++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 6 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 2eee1d44..d1d1f07e 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,6 +1,6 @@ BundleRegistryGasUsageTest:testGasRegisterWithSig() (gas: 834117) -BundleRegistryGasUsageTest:testGasTrustedBatchRegister() (gas: 7068101) -BundleRegistryGasUsageTest:testGasTrustedRegister() (gas: 914283) +BundleRegistryGasUsageTest:testGasTrustedBatchRegister() (gas: 7091701) +BundleRegistryGasUsageTest:testGasTrustedRegister() (gas: 918443) IdRegistryGasUsageTest:testGasRegister() (gas: 736116) IdRegistryGasUsageTest:testGasRegisterForAndRecover() (gas: 1743798) IdRegistryGasUsageTest:testGasRegisterFromTrustedCaller() (gas: 809179) diff --git a/src/KeyRegistry.sol b/src/KeyRegistry.sol index b1580a76..373598fd 100644 --- a/src/KeyRegistry.sol +++ b/src/KeyRegistry.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.21; import {EIP712} from "openzeppelin/contracts/utils/cryptography/EIP712.sol"; import {Nonces} from "openzeppelin-latest/contracts/utils/Nonces.sol"; +import {Pausable} from "openzeppelin/contracts/security/Pausable.sol"; import {IdRegistryLike} from "./interfaces/IdRegistryLike.sol"; import {IMetadataValidator} from "./interfaces/IMetadataValidator.sol"; @@ -16,7 +17,7 @@ import {TrustedCaller} from "./lib/TrustedCaller.sol"; * @notice See ../docs/docs.md for an overview. */ -contract KeyRegistry is TrustedCaller, Signatures, EIP712, Nonces { +contract KeyRegistry is TrustedCaller, Signatures, Pausable, EIP712, Nonces { /** * @notice State enumeration for a key in the registry. During migration, an admin can change * the state of any fids key from NULL to ADDED or ADDED to NULL. After migration, an @@ -492,6 +493,22 @@ contract KeyRegistry is TrustedCaller, Signatures, EIP712, Nonces { idRegistry = IdRegistryLike(_idRegistry); } + /** + * @notice Pause add, remove, and reset.e registration, transfer, and recovery. + * Must be called by the owner. + */ + function pause() external onlyOwner { + _pause(); + } + + /** + * @notice Unpause add, remove, and reset.otice Unpause registration, transfer, and recovery. + * Must be called by the owner. + */ + function unpause() external onlyOwner { + _unpause(); + } + /*////////////////////////////////////////////////////////////// HELPERS //////////////////////////////////////////////////////////////*/ @@ -502,7 +519,7 @@ contract KeyRegistry is TrustedCaller, Signatures, EIP712, Nonces { bytes calldata key, uint8 metadataType, bytes calldata metadata - ) internal { + ) internal whenNotPaused { KeyData storage keyData = keys[fid][key]; if (keyData.state != KeyState.NULL) revert InvalidState(); @@ -518,7 +535,7 @@ contract KeyRegistry is TrustedCaller, Signatures, EIP712, Nonces { emit Add(fid, keyType, key, key, metadataType, metadata); } - function _remove(uint256 fid, bytes calldata key) internal { + function _remove(uint256 fid, bytes calldata key) internal whenNotPaused { KeyData storage keyData = keys[fid][key]; if (keyData.state != KeyState.ADDED) revert InvalidState(); @@ -526,7 +543,7 @@ contract KeyRegistry is TrustedCaller, Signatures, EIP712, Nonces { emit Remove(fid, key, key); } - function _reset(uint256 fid, bytes calldata key) internal { + function _reset(uint256 fid, bytes calldata key) internal whenNotPaused { KeyData storage keyData = keys[fid][key]; if (keyData.state != KeyState.ADDED) revert InvalidState(); diff --git a/test/KeyRegistry/KeyRegistry.t.sol b/test/KeyRegistry/KeyRegistry.t.sol index 3ddf2bad..c9fc502f 100644 --- a/test/KeyRegistry/KeyRegistry.t.sol +++ b/test/KeyRegistry/KeyRegistry.t.sol @@ -201,6 +201,28 @@ contract KeyRegistryTest is KeyRegistryTestSuite { assertRemoved(fid, key, keyType); } + function testFuzzAddRevertsPaused( + address to, + address recovery, + uint32 keyType, + bytes calldata key, + uint8 metadataType, + bytes memory metadata + ) public { + keyType = uint32(bound(keyType, 1, type(uint32).max)); + metadataType = uint8(bound(metadataType, 1, type(uint8).max)); + + _registerFid(to, recovery); + _registerValidator(keyType, metadataType); + + vm.prank(owner); + keyRegistry.pause(); + + vm.prank(to); + vm.expectRevert("Pausable: paused"); + keyRegistry.add(keyType, key, metadataType, metadata); + } + function testFuzzAddFor( address registrar, uint256 ownerPk, @@ -338,6 +360,35 @@ contract KeyRegistryTest is KeyRegistryTestSuite { assertNull(fid, key); } + function testFuzzAddForRevertsPaused( + address registrar, + uint256 fidOwnerPk, + address recovery, + uint32 keyType, + bytes calldata key, + uint8 metadataType, + bytes calldata metadata, + uint40 _deadline + ) public { + keyType = uint32(bound(keyType, 1, type(uint32).max)); + + uint256 deadline = _boundDeadline(_deadline); + fidOwnerPk = _boundPk(fidOwnerPk); + + address fidOwner = vm.addr(fidOwnerPk); + uint256 fid = _registerFid(fidOwner, recovery); + bytes memory sig = _signAdd(fidOwnerPk, fidOwner, keyType, key, metadataType, metadata, deadline); + + vm.prank(owner); + keyRegistry.pause(); + + vm.prank(registrar); + vm.expectRevert("Pausable: paused"); + keyRegistry.addFor(fidOwner, keyType, key, metadataType, metadata, deadline, sig); + + assertNull(fid, key); + } + function testFuzzTrustedAdd( address to, address recovery, @@ -433,6 +484,32 @@ contract KeyRegistryTest is KeyRegistryTestSuite { assertNull(fid, key); } + function testFuzzTrustedAddRevertsPaused( + address to, + address recovery, + uint32 keyType, + bytes calldata key, + uint8 metadataType, + bytes calldata metadata + ) public { + keyType = uint32(bound(keyType, 1, type(uint32).max)); + metadataType = uint8(bound(metadataType, 1, type(uint8).max)); + _registerValidator(keyType, metadataType); + + vm.startPrank(owner); + keyRegistry.setTrustedCaller(trustedCaller); + keyRegistry.pause(); + vm.stopPrank(); + + uint256 fid = _registerFid(to, recovery); + + vm.prank(trustedCaller); + vm.expectRevert("Pausable: paused"); + keyRegistry.trustedAdd(to, keyType, key, metadataType, metadata); + + assertNull(fid, key); + } + function testAddTypeHash() public { assertEq( keyRegistry.addTypehash(), @@ -537,6 +614,35 @@ contract KeyRegistryTest is KeyRegistryTestSuite { assertRemoved(fid, key, keyType); } + function testFuzzRemoveRevertsWhenPaused( + address to, + address recovery, + uint32 keyType, + bytes calldata key, + uint8 metadataType, + bytes memory metadata + ) public { + keyType = uint32(bound(keyType, 1, type(uint32).max)); + metadataType = uint8(bound(metadataType, 1, type(uint8).max)); + + uint256 fid = _registerFid(to, recovery); + _registerValidator(keyType, metadataType); + + vm.startPrank(to); + keyRegistry.add(keyType, key, metadataType, metadata); + keyRegistry.remove(key); + vm.stopPrank(); + + vm.prank(owner); + keyRegistry.pause(); + + vm.prank(to); + vm.expectRevert("Pausable: paused"); + keyRegistry.remove(key); + + assertRemoved(fid, key, keyType); + } + function testFuzzRemoveFor( address registrar, uint256 ownerPk, @@ -691,6 +797,42 @@ contract KeyRegistryTest is KeyRegistryTestSuite { assertAdded(fid, key, keyType); } + function testFuzzRemoveForRevertsWhenPaused( + address registrar, + uint256 fidOwnerPk, + address recovery, + uint32 keyType, + bytes calldata key, + uint8 metadataType, + bytes memory metadata, + uint40 _deadline + ) public { + keyType = uint32(bound(keyType, 1, type(uint32).max)); + metadataType = uint8(bound(metadataType, 1, type(uint8).max)); + + uint256 deadline = _boundDeadline(_deadline); + fidOwnerPk = _boundPk(fidOwnerPk); + _registerValidator(keyType, metadataType); + + address fidOwner = vm.addr(fidOwnerPk); + uint256 fid = _registerFid(fidOwner, recovery); + bytes memory sig = _signRemove(fidOwnerPk, fidOwner, key, deadline); + + vm.prank(fidOwner); + keyRegistry.add(keyType, key, metadataType, metadata); + assertEq(keyRegistry.keyDataOf(fid, key).state, KeyRegistry.KeyState.ADDED); + assertEq(keyRegistry.keyDataOf(fid, key).keyType, keyType); + + vm.prank(owner); + keyRegistry.pause(); + + vm.prank(registrar); + vm.expectRevert("Pausable: paused"); + keyRegistry.removeFor(fidOwner, key, deadline, sig); + + assertAdded(fid, key, keyType); + } + function testRemoveTypeHash() public { assertEq( keyRegistry.removeTypehash(), keccak256("Remove(address owner,bytes key,uint256 nonce,uint256 deadline)") @@ -845,6 +987,20 @@ contract KeyRegistryTest is KeyRegistryTestSuite { vm.stopPrank(); } + function testBulkAddRevertsWhenPaused() public { + _registerValidator(1, 1); + + KeyRegistry.BulkAddData[] memory addItems = + BulkAddDataBuilder.empty().addFid(1).addKey(0, "key1", "metadata1").addFid(2).addKey(1, "key2", "metadata2"); + + vm.startPrank(owner); + keyRegistry.pause(); + + vm.expectRevert("Pausable: paused"); + keyRegistry.bulkAddKeysForMigration(addItems); + vm.stopPrank(); + } + /*////////////////////////////////////////////////////////////// BULK REMOVE //////////////////////////////////////////////////////////////*/ @@ -988,6 +1144,44 @@ contract KeyRegistryTest is KeyRegistryTestSuite { vm.stopPrank(); } + function testFuzzBulkRemoveSignerForMigrationRevertsWhenPaused() public { + KeyRegistry.BulkResetData[] memory items = BulkResetDataBuilder.empty().addFid(1).addKey(0, "key"); + + vm.startPrank(owner); + keyRegistry.pause(); + + vm.expectRevert("Pausable: paused"); + keyRegistry.bulkResetKeysForMigration(items); + + vm.stopPrank(); + } + + /*////////////////////////////////////////////////////////////// + PAUSABILITY + //////////////////////////////////////////////////////////////*/ + + function testFuzzOnlyAdminCanPause(address caller) public { + vm.assume(caller != owner); + + vm.prank(caller); + vm.expectRevert("Ownable: caller is not the owner"); + keyRegistry.pause(); + } + + function testPauseUnpause() public { + assertEq(keyRegistry.paused(), false); + + vm.prank(owner); + keyRegistry.pause(); + + assertEq(keyRegistry.paused(), true); + + vm.prank(owner); + keyRegistry.unpause(); + + assertEq(keyRegistry.paused(), false); + } + /*////////////////////////////////////////////////////////////// SET IDREGISTRY //////////////////////////////////////////////////////////////*/