From 853c4c47d7c95bd4442d7c8a776d0284bb0b9eee Mon Sep 17 00:00:00 2001 From: donosonaumczuk Date: Mon, 9 May 2022 17:57:32 +0100 Subject: [PATCH 1/4] misc: Remove unused import from mainnet proxy --- contracts/misc/ProfileCreationProxy.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/misc/ProfileCreationProxy.sol b/contracts/misc/ProfileCreationProxy.sol index 33ac11a3..5d54fa4d 100644 --- a/contracts/misc/ProfileCreationProxy.sol +++ b/contracts/misc/ProfileCreationProxy.sol @@ -5,7 +5,6 @@ pragma solidity 0.8.10; import {ILensHub} from '../interfaces/ILensHub.sol'; import {DataTypes} from '../libraries/DataTypes.sol'; import {Errors} from '../libraries/Errors.sol'; -import {Events} from '../libraries/Events.sol'; import {Ownable} from '@openzeppelin/contracts/access/Ownable.sol'; /** From 5b3c665d8163fed80f52b03ab6e5f16fb5302495 Mon Sep 17 00:00:00 2001 From: donosonaumczuk Date: Mon, 9 May 2022 17:58:55 +0100 Subject: [PATCH 2/4] refactor: MockProfileCreationProxy replicates ProfileCreationProxy except for Ownable and suffix --- contracts/mocks/MockProfileCreationProxy.sol | 68 +++++--------------- 1 file changed, 15 insertions(+), 53 deletions(-) diff --git a/contracts/mocks/MockProfileCreationProxy.sol b/contracts/mocks/MockProfileCreationProxy.sol index ae2ff84a..67eddd48 100644 --- a/contracts/mocks/MockProfileCreationProxy.sol +++ b/contracts/mocks/MockProfileCreationProxy.sol @@ -10,69 +10,31 @@ import {Errors} from '../libraries/Errors.sol'; * @title MockProfileCreationProxy * @author Lens Protocol * - * @dev This is a proxy to allow profiles to be created from any address while adding some handle restrictions. + * @notice This is an ownable proxy contract that enforces ".test" handle suffixes at profile creation. */ contract MockProfileCreationProxy { ILensHub immutable LENS_HUB; - address governance; - uint256 requiredMinHandleLengthBeforeSuffix; - string requiredHandleSuffix; - mapping(bytes1 => bool) isCharacterInvalid; - - modifier onlyGov() { - if (msg.sender != governance) revert Errors.NotGovernance(); - _; - } - - constructor( - uint256 minHandleLengthBeforeSuffix, - string memory handleSuffix, - string memory invalidCharacters, - address newGovernance, - address hub - ) { - requiredMinHandleLengthBeforeSuffix = minHandleLengthBeforeSuffix; - requiredHandleSuffix = handleSuffix; - for (uint256 i = 0; i < bytes(invalidCharacters).length; ++i) { - isCharacterInvalid[bytes(invalidCharacters)[i]] = true; - } - governance = newGovernance; - LENS_HUB = ILensHub(hub); + constructor(ILensHub hub) { + LENS_HUB = hub; } function proxyCreateProfile(DataTypes.CreateProfileData memory vars) external { uint256 handleLength = bytes(vars.handle).length; - if (handleLength < requiredMinHandleLengthBeforeSuffix) { - revert Errors.HandleLengthInvalid(); - } - for (uint256 i = 0; i < handleLength; ++i) { - if (isCharacterInvalid[bytes(vars.handle)[i]]) { - revert Errors.HandleContainsInvalidCharacters(); - } - } - if (bytes(requiredHandleSuffix).length > 0) { - vars.handle = string(abi.encodePacked(vars.handle, requiredHandleSuffix)); - } - LENS_HUB.createProfile(vars); - } - - function setRequiredHandleSuffix(string memory handleSuffix) external onlyGov { - requiredHandleSuffix = handleSuffix; - } + if (handleLength < 5) revert Errors.HandleLengthInvalid(); - function setCharacterValidity(bytes1 character, bool isValid) external onlyGov { - isCharacterInvalid[character] = !isValid; - } + bytes1 firstByte = bytes(vars.handle)[0]; + if (firstByte == '-' || firstByte == '_' || firstByte == '.') + revert Errors.HandleFirstCharInvalid(); - function setRequiredMinHandleLengthBeforeSuffix(uint256 minHandleLengthBeforeSuffix) - external - onlyGov - { - requiredMinHandleLengthBeforeSuffix = minHandleLengthBeforeSuffix; - } + for (uint256 i = 1; i < handleLength; ) { + if (bytes(vars.handle)[i] == '.') revert Errors.HandleContainsInvalidCharacters(); + unchecked { + ++i; + } + } - function setGovernance(address newGovernance) external onlyGov { - governance = newGovernance; + vars.handle = string(abi.encodePacked(vars.handle, '.test')); + LENS_HUB.createProfile(vars); } } From 400982f901d53e9bba34ba7a2c862a071f97ef6d Mon Sep 17 00:00:00 2001 From: donosonaumczuk Date: Mon, 9 May 2022 17:59:34 +0100 Subject: [PATCH 3/4] misc: Test and task updated according new MockProfileCreationProxy impl --- tasks/testnet-full-deploy-verify.ts | 15 +- .../other/mock-profile-creation-proxy.spec.ts | 182 +++++------------- 2 files changed, 57 insertions(+), 140 deletions(-) diff --git a/tasks/testnet-full-deploy-verify.ts b/tasks/testnet-full-deploy-verify.ts index 56a6b4f3..32f66af5 100644 --- a/tasks/testnet-full-deploy-verify.ts +++ b/tasks/testnet-full-deploy-verify.ts @@ -272,17 +272,10 @@ task( // Deploy MockProfileCreationProxy console.log('\n\t-- Deploying Profile Creation Proxy --'); const profileCreationProxy = await deployWithVerify( - new MockProfileCreationProxy__factory(deployer).deploy( - 4, - '.test', - '.', - lensHub.address, - governance.address, - { - nonce: deployerNonce++, - } - ), - [4, '.test', '.', lensHub.address], + new MockProfileCreationProxy__factory(deployer).deploy(lensHub.address, { + nonce: deployerNonce++, + }), + [lensHub.address], 'contracts/mocks/MockProfileCreationProxy.sol:MockProfileCreationProxy' ); diff --git a/test/other/mock-profile-creation-proxy.spec.ts b/test/other/mock-profile-creation-proxy.spec.ts index a53683b3..3dc3981f 100644 --- a/test/other/mock-profile-creation-proxy.spec.ts +++ b/test/other/mock-profile-creation-proxy.spec.ts @@ -13,36 +13,33 @@ import { MOCK_PROFILE_URI, user, userAddress, - governanceAddress, + deployerAddress, } from '../__setup.spec'; import { BigNumber } from 'ethers'; import { TokenDataStructOutput } from '../../typechain-types/LensHub'; import { getTimestamp } from '../helpers/utils'; makeSuiteCleanRoom('Mock Profile Creation Proxy', function () { - let mockProfileCreationProxy: MockProfileCreationProxy; - let requiredSuffix = '.lens'; - let invalidChars = '.'; - let requiredMinHandleLengthBeforeSuffix = 4; + const REQUIRED_SUFFIX = '.test'; + const MINIMUM_LENGTH = 5; + + let profileCreationProxy: MockProfileCreationProxy; beforeEach(async function () { - mockProfileCreationProxy = await new MockProfileCreationProxy__factory(deployer).deploy( - requiredMinHandleLengthBeforeSuffix, - requiredSuffix, - invalidChars, - governanceAddress, + profileCreationProxy = await new MockProfileCreationProxy__factory(deployer).deploy( lensHub.address ); await expect( - lensHub.connect(governance).whitelistProfileCreator(mockProfileCreationProxy.address, true) + lensHub.connect(governance).whitelistProfileCreator(profileCreationProxy.address, true) ).to.not.be.reverted; }); context('Negatives', function () { - it('User should fail to create profile if handle length before suffix does not reach minimum length', async function () { + it('Should fail to create profile if handle length before suffix does not reach minimum length', async function () { + const handle = 'a'.repeat(MINIMUM_LENGTH - 1); await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ + profileCreationProxy.proxyCreateProfile({ to: userAddress, - handle: '69', + handle: handle, imageURI: MOCK_PROFILE_URI, followModule: ZERO_ADDRESS, followModuleInitData: [], @@ -51,9 +48,9 @@ makeSuiteCleanRoom('Mock Profile Creation Proxy', function () { ).to.be.revertedWith(ERRORS.INVALID_HANDLE_LENGTH); }); - it('User should fail to create profile if handle contains an invalid character before the suffix', async function () { + it('Should fail to create profile if handle contains an invalid character before the suffix', async function () { await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ + profileCreationProxy.proxyCreateProfile({ to: userAddress, handle: 'dots.are.invalid', imageURI: MOCK_PROFILE_URI, @@ -64,149 +61,76 @@ makeSuiteCleanRoom('Mock Profile Creation Proxy', function () { ).to.be.revertedWith(ERRORS.HANDLE_CONTAINS_INVALID_CHARACTERS); }); - it('User should fail to create profile if handle length before suffix does not reach new minimum length', async function () { + it('Should fail to create profile if handle starts with a dash, underscore or period', async function () { await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ + profileCreationProxy.proxyCreateProfile({ to: userAddress, - handle: 'validhandle', + handle: '.abcdef', imageURI: MOCK_PROFILE_URI, followModule: ZERO_ADDRESS, followModuleInitData: [], followNFTURI: MOCK_FOLLOW_NFT_URI, }) - ).to.not.be.reverted; - await mockProfileCreationProxy.connect(governance).setRequiredMinHandleLengthBeforeSuffix(15); + ).to.be.revertedWith(ERRORS.HANDLE_FIRST_CHARACTER_INVALID); + await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ + profileCreationProxy.proxyCreateProfile({ to: userAddress, - handle: 'validhandletoo', + handle: '-abcdef', imageURI: MOCK_PROFILE_URI, followModule: ZERO_ADDRESS, followModuleInitData: [], followNFTURI: MOCK_FOLLOW_NFT_URI, }) - ).to.be.revertedWith(ERRORS.INVALID_HANDLE_LENGTH); - }); + ).to.be.revertedWith(ERRORS.HANDLE_FIRST_CHARACTER_INVALID); - it('User should fail to create profile if handle contains a new invalid character before the suffix', async function () { await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ + profileCreationProxy.proxyCreateProfile({ to: userAddress, - handle: 'validhandle', + handle: '_abcdef', imageURI: MOCK_PROFILE_URI, followModule: ZERO_ADDRESS, followModuleInitData: [], followNFTURI: MOCK_FOLLOW_NFT_URI, }) - ).to.not.be.reverted; - // Sets 'h' character (0x68 in UTF-8) as invalid - await mockProfileCreationProxy.connect(governance).setCharacterValidity('0x68', false); + ).to.be.revertedWith(ERRORS.HANDLE_FIRST_CHARACTER_INVALID); + }); + }); + + context('Scenarios', function () { + it('Should be able to create a profile using the whitelisted proxy, received NFT should be valid', async function () { + let timestamp: any; + let owner: string; + let totalSupply: BigNumber; + let profileId: BigNumber; + let mintTimestamp: BigNumber; + let tokenData: TokenDataStructOutput; + const validHandleBeforeSuffix = 'v_al-id'; + const expectedHandle = 'v_al-id'.concat(REQUIRED_SUFFIX); + await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ + profileCreationProxy.proxyCreateProfile({ to: userAddress, - handle: 'validhandletoo', + handle: validHandleBeforeSuffix, imageURI: MOCK_PROFILE_URI, followModule: ZERO_ADDRESS, followModuleInitData: [], followNFTURI: MOCK_FOLLOW_NFT_URI, }) - ).to.be.revertedWith(ERRORS.HANDLE_CONTAINS_INVALID_CHARACTERS); - }); - - it('User should fail to change min handle length before suffix if it is not the governance address', async function () { - await expect( - mockProfileCreationProxy.connect(user).setRequiredMinHandleLengthBeforeSuffix(15) - ).to.be.revertedWith(ERRORS.NOT_GOVERNANCE); - }); - - it('User should fail to change suffix if it is not the governance address', async function () { - await expect( - mockProfileCreationProxy.connect(user).setRequiredHandleSuffix('.user') - ).to.be.revertedWith(ERRORS.NOT_GOVERNANCE); - }); - - it('User should fail to change character validity if it is not the governance address', async function () { - await expect( - mockProfileCreationProxy.connect(user).setCharacterValidity('0x68', true) - ).to.be.revertedWith(ERRORS.NOT_GOVERNANCE); - }); + ).to.not.be.reverted; - it('User should fail to change governance if it is not the governance address', async function () { - await expect( - mockProfileCreationProxy.connect(user).setGovernance(userAddress) - ).to.be.revertedWith(ERRORS.NOT_GOVERNANCE); + timestamp = await getTimestamp(); + owner = await lensHub.ownerOf(FIRST_PROFILE_ID); + totalSupply = await lensHub.totalSupply(); + profileId = await lensHub.getProfileIdByHandle(expectedHandle); + mintTimestamp = await lensHub.mintTimestampOf(FIRST_PROFILE_ID); + tokenData = await lensHub.tokenDataOf(FIRST_PROFILE_ID); + expect(owner).to.eq(userAddress); + expect(totalSupply).to.eq(FIRST_PROFILE_ID); + expect(profileId).to.eq(FIRST_PROFILE_ID); + expect(mintTimestamp).to.eq(timestamp); + expect(tokenData.owner).to.eq(userAddress); + expect(tokenData.mintTimestamp).to.eq(timestamp); }); }); - - it('User should be able to create a profile using the whitelisted proxy, received NFT should be valid', async function () { - let timestamp: any; - let owner: string; - let totalSupply: BigNumber; - let profileId: BigNumber; - let mintTimestamp: BigNumber; - let tokenData: TokenDataStructOutput; - const validHandleBeforeSuffix = 'validhandle'; - const expectedHandle = 'validhandle'.concat(requiredSuffix); - - await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ - to: userAddress, - handle: validHandleBeforeSuffix, - imageURI: MOCK_PROFILE_URI, - followModule: ZERO_ADDRESS, - followModuleInitData: [], - followNFTURI: MOCK_FOLLOW_NFT_URI, - }) - ).to.not.be.reverted; - - timestamp = await getTimestamp(); - owner = await lensHub.ownerOf(FIRST_PROFILE_ID); - totalSupply = await lensHub.totalSupply(); - profileId = await lensHub.getProfileIdByHandle(expectedHandle); - mintTimestamp = await lensHub.mintTimestampOf(FIRST_PROFILE_ID); - tokenData = await lensHub.tokenDataOf(FIRST_PROFILE_ID); - expect(owner).to.eq(userAddress); - expect(totalSupply).to.eq(FIRST_PROFILE_ID); - expect(profileId).to.eq(FIRST_PROFILE_ID); - expect(mintTimestamp).to.eq(timestamp); - expect(tokenData.owner).to.eq(userAddress); - expect(tokenData.mintTimestamp).to.eq(timestamp); - }); - - it('User should get the expected handle after governance update the required suffix', async function () { - const handleBeforeSuffix = 'validhandle'; - await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ - to: userAddress, - handle: handleBeforeSuffix, - imageURI: MOCK_PROFILE_URI, - followModule: ZERO_ADDRESS, - followModuleInitData: [], - followNFTURI: MOCK_FOLLOW_NFT_URI, - }) - ).to.not.be.reverted; - expect(await lensHub.getHandle(1)).to.eq(handleBeforeSuffix.concat(requiredSuffix)); - - const newSuffix = '.test'; - await mockProfileCreationProxy.connect(governance).setRequiredHandleSuffix(newSuffix); - - await expect( - mockProfileCreationProxy.connect(user).proxyCreateProfile({ - to: userAddress, - handle: handleBeforeSuffix, - imageURI: MOCK_PROFILE_URI, - followModule: ZERO_ADDRESS, - followModuleInitData: [], - followNFTURI: MOCK_FOLLOW_NFT_URI, - }) - ).to.not.be.reverted; - expect(await lensHub.getHandle(2)).to.eq(handleBeforeSuffix.concat(newSuffix)); - }); - - it('User should succeed making a onlyGov call after setting him as governance address', async function () { - await mockProfileCreationProxy.connect(governance).setGovernance(userAddress); - await expect( - mockProfileCreationProxy.connect(user).setRequiredHandleSuffix('.user') - ).to.not.be.reverted; - }); }); From fd7bfea4d77f470bc9e3b8f2a6e649f97dec826c Mon Sep 17 00:00:00 2001 From: donosonaumczuk Date: Tue, 10 May 2022 11:22:54 +0100 Subject: [PATCH 4/4] misc: MockProfileCreationProxy contract natspec improved --- contracts/mocks/MockProfileCreationProxy.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/mocks/MockProfileCreationProxy.sol b/contracts/mocks/MockProfileCreationProxy.sol index 67eddd48..3026d50a 100644 --- a/contracts/mocks/MockProfileCreationProxy.sol +++ b/contracts/mocks/MockProfileCreationProxy.sol @@ -10,7 +10,7 @@ import {Errors} from '../libraries/Errors.sol'; * @title MockProfileCreationProxy * @author Lens Protocol * - * @notice This is an ownable proxy contract that enforces ".test" handle suffixes at profile creation. + * @notice This is a proxy contract that enforces ".test" handle suffixes and adds char validations at profile creation. */ contract MockProfileCreationProxy { ILensHub immutable LENS_HUB;