diff --git a/lib/forge-std b/lib/forge-std index 73d44ec7..b93cf4bc 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 73d44ec7d124e3831bc5f832267889ffb6f9bc3f +Subproject commit b93cf4bc34ff214c099dc970b153f85ade8c9f66 diff --git a/src/IdGateway.sol b/src/IdGateway.sol index b2d44daa..e21ee149 100644 --- a/src/IdGateway.sol +++ b/src/IdGateway.sol @@ -9,6 +9,7 @@ import {TransferHelper} from "./libraries/TransferHelper.sol"; import {EIP712} from "./abstract/EIP712.sol"; import {Nonces} from "./abstract/Nonces.sol"; import {Signatures} from "./abstract/Signatures.sol"; +import {Address} from "openzeppelin/contracts/utils/Address.sol"; /** * @title Farcaster IdGateway @@ -19,6 +20,7 @@ import {Signatures} from "./abstract/Signatures.sol"; */ contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces { using TransferHelper for address; + using Address for address; /*////////////////////////////////////////////////////////////// CONSTANTS @@ -90,7 +92,9 @@ contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces { /** * @inheritdoc IIdGateway */ - function price(uint256 extraStorage) external view returns (uint256) { + function price( + uint256 extraStorage + ) external view returns (uint256) { return storageRegistry.price(1 + extraStorage); } @@ -101,7 +105,9 @@ contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces { /** * @inheritdoc IIdGateway */ - function register(address recovery) external payable returns (uint256, uint256) { + function register( + address recovery + ) external payable returns (uint256, uint256) { return register(recovery, 0); } @@ -145,7 +151,12 @@ contract IdGateway is IIdGateway, Guardians, Signatures, EIP712, Nonces { /** * @inheritdoc IIdGateway */ - function setStorageRegistry(address _storageRegistry) external onlyOwner { + function setStorageRegistry( + address _storageRegistry + ) external onlyOwner { + if (_storageRegistry == address(0)) revert Unauthorized(); + if (!_storageRegistry.isContract()) revert Unauthorized(); + emit SetStorageRegistry(address(storageRegistry), _storageRegistry); storageRegistry = IStorageRegistry(_storageRegistry); } diff --git a/test/IdGateway/IdGateway.t.sol b/test/IdGateway/IdGateway.t.sol index 6d241d7c..6256008d 100644 --- a/test/IdGateway/IdGateway.t.sol +++ b/test/IdGateway/IdGateway.t.sol @@ -23,15 +23,15 @@ contract IdGatewayTest is IdGatewayTestSuite { PARAMETERS //////////////////////////////////////////////////////////////*/ - function testVersion() public { + function testVersion() public view { assertEq(idGateway.VERSION(), "2023.11.15"); } - function testIdRegistry() public { + function testIdRegistry() public view { assertEq(address(idGateway.idRegistry()), address(idRegistry)); } - function testStorageRegistry() public { + function testStorageRegistry() public view { assertEq(address(idGateway.storageRegistry()), address(storageRegistry)); } @@ -365,7 +365,7 @@ contract IdGatewayTest is IdGatewayTestSuite { assertEq(idRegistry.recoveryOf(1), address(0)); } - function testRegisterTypehash() public { + function testRegisterTypehash() public view { assertEq( idGateway.REGISTER_TYPEHASH(), keccak256("Register(address to,address recovery,uint256 nonce,uint256 deadline)") @@ -457,7 +457,15 @@ contract IdGatewayTest is IdGatewayTestSuite { SET STORAGE REGISTRY //////////////////////////////////////////////////////////////*/ - function testFuzzSetStorageRegistry(address storageRegistry) public { + function testFuzzSetStorageRegistry( + address storageRegistry + ) public { + vm.assume(storageRegistry != address(0)); + vm.assume(uint160(storageRegistry) > 9); + + // Mocks that the address is a contract + vm.etch(storageRegistry, hex"00"); + address prevStorageRegistry = address(idGateway.storageRegistry()); vm.expectEmit(); diff --git a/test/IdGateway/IdGatewayTestSuite.sol b/test/IdGateway/IdGatewayTestSuite.sol index f111b2db..abbec895 100644 --- a/test/IdGateway/IdGatewayTestSuite.sol +++ b/test/IdGateway/IdGatewayTestSuite.sol @@ -16,11 +16,7 @@ abstract contract IdGatewayTestSuite is StorageRegistryTestSuite, KeyRegistryTes function setUp() public virtual override(StorageRegistryTestSuite, KeyRegistryTestSuite) { super.setUp(); - idGateway = new IdGateway( - address(idRegistry), - address(storageRegistry), - owner - ); + idGateway = new IdGateway(address(idRegistry), address(storageRegistry), owner); vm.startPrank(owner); idRegistry.setIdGateway(address(idGateway)); @@ -29,7 +25,9 @@ abstract contract IdGatewayTestSuite is StorageRegistryTestSuite, KeyRegistryTes addKnownContract(address(idGateway)); } - function _registerTo(address caller) internal returns (uint256 fid) { + function _registerTo( + address caller + ) internal returns (uint256 fid) { fid = _registerWithRecovery(caller, address(0)); } @@ -58,7 +56,7 @@ abstract contract IdGatewayTestSuite is StorageRegistryTestSuite, KeyRegistryTes address to, address recovery, uint256 deadline - ) internal returns (bytes memory signature) { + ) internal view returns (bytes memory signature) { address signer = vm.addr(pk); bytes32 digest = idGateway.hashTypedDataV4( keccak256(abi.encode(idGateway.REGISTER_TYPEHASH(), to, recovery, idGateway.nonces(signer), deadline)) diff --git a/test/IdRegistry/IdRegistryTestSuite.sol b/test/IdRegistry/IdRegistryTestSuite.sol index 073702d6..edaac4ec 100644 --- a/test/IdRegistry/IdRegistryTestSuite.sol +++ b/test/IdRegistry/IdRegistryTestSuite.sol @@ -24,7 +24,9 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup { TEST HELPERS //////////////////////////////////////////////////////////////*/ - function _register(address caller) internal returns (uint256 fid) { + function _register( + address caller + ) internal returns (uint256 fid) { fid = _registerWithRecovery(caller, address(0)); } @@ -44,7 +46,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup { uint256 fid, address to, uint256 deadline - ) internal returns (bytes memory signature) { + ) internal view returns (bytes memory signature) { address signer = vm.addr(pk); bytes32 digest = idRegistry.hashTypedDataV4( keccak256(abi.encode(idRegistry.TRANSFER_TYPEHASH(), fid, to, idRegistry.nonces(signer), deadline)) @@ -60,7 +62,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup { address to, address recovery, uint256 deadline - ) internal returns (bytes memory signature) { + ) internal view returns (bytes memory signature) { address signer = vm.addr(pk); bytes32 digest = idRegistry.hashTypedDataV4( keccak256( @@ -85,7 +87,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup { address from, address to, uint256 deadline - ) internal returns (bytes memory signature) { + ) internal view returns (bytes memory signature) { address signer = vm.addr(pk); bytes32 digest = idRegistry.hashTypedDataV4( keccak256( @@ -99,7 +101,7 @@ abstract contract IdRegistryTestSuite is TestSuiteSetup { assertEq(signature.length, 65); } - function _signDigest(uint256 pk, bytes32 digest) internal returns (bytes memory signature) { + function _signDigest(uint256 pk, bytes32 digest) internal pure returns (bytes memory signature) { (uint8 v, bytes32 r, bytes32 s) = vm.sign(pk, digest); signature = abi.encodePacked(r, s, v); assertEq(signature.length, 65); diff --git a/test/KeyRegistry/KeyRegistryTestSuite.sol b/test/KeyRegistry/KeyRegistryTestSuite.sol index e12fe920..2d54681d 100644 --- a/test/KeyRegistry/KeyRegistryTestSuite.sol +++ b/test/KeyRegistry/KeyRegistryTestSuite.sol @@ -32,7 +32,7 @@ abstract contract KeyRegistryTestSuite is IdRegistryTestSuite { address owner, bytes memory key, uint256 deadline - ) internal returns (bytes memory signature) { + ) internal view returns (bytes memory signature) { bytes32 digest = keyRegistry.hashTypedDataV4( keccak256( abi.encode(keyRegistry.REMOVE_TYPEHASH(), owner, keccak256(key), keyRegistry.nonces(owner), deadline) diff --git a/test/TestSuiteSetup.sol b/test/TestSuiteSetup.sol index 5559d4b7..e83d0acb 100644 --- a/test/TestSuiteSetup.sol +++ b/test/TestSuiteSetup.sol @@ -49,38 +49,44 @@ abstract contract TestSuiteSetup is Test { HELPERS //////////////////////////////////////////////////////////////*/ - function addKnownContract(address contractAddress) public { + function addKnownContract( + address contractAddress + ) public { isKnownContract[contractAddress] = true; } // Ensures that a fuzzed address input does not match a known contract address - function _assumeClean(address a) internal { - assumeNoPrecompiles(a); + function _assumeClean( + address a + ) internal view { + assumeNotPrecompile(a); vm.assume(!isKnownContract[a]); vm.assume(a != ADMIN); vm.assume(a != address(0)); } - function _boundPk(uint256 pk) internal view returns (uint256) { + function _boundPk( + uint256 pk + ) internal pure returns (uint256) { return bound(pk, 1, SECP_256K1_ORDER - 1); } - function _boundDeadline(uint40 deadline) internal view returns (uint256) { + function _boundDeadline( + uint40 deadline + ) internal view returns (uint256) { return block.timestamp + uint256(bound(deadline, 0, type(uint40).max)); } - function _createMockERC1271(address ownerAddress) - internal - returns (ERC1271WalletMock mockWallet, address mockWalletAddress) - { + function _createMockERC1271( + address ownerAddress + ) internal returns (ERC1271WalletMock mockWallet, address mockWalletAddress) { mockWallet = new ERC1271WalletMock(ownerAddress); mockWalletAddress = address(mockWallet); } - function _createMaliciousMockERC1271(address ownerAddress) - internal - returns (ERC1271MaliciousMockForceRevert mockWallet, address mockWalletAddress) - { + function _createMaliciousMockERC1271( + address ownerAddress + ) internal returns (ERC1271MaliciousMockForceRevert mockWallet, address mockWalletAddress) { mockWallet = new ERC1271MaliciousMockForceRevert(ownerAddress); mockWalletAddress = address(mockWallet); }