From 36f9672dc935614b8a349fbea0fd88d3668fbb01 Mon Sep 17 00:00:00 2001 From: spengrah Date: Sat, 14 Sep 2024 20:06:51 -0500 Subject: [PATCH 1/3] add setUnlock initializer --- script/Deploy.s.sol | 7 ++- src/PublicLockV14Eligibility.sol | 42 ++++++++++++++++-- test/PublicLockEligibility.t.sol | 73 ++++++++++++++++++++++++++++---- 3 files changed, 109 insertions(+), 13 deletions(-) diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol index e024144..d2e1078 100644 --- a/script/Deploy.s.sol +++ b/script/Deploy.s.sol @@ -11,7 +11,7 @@ contract Deploy is Script { // default values bool internal _verbose = true; - string internal _version = "0.1.1"; // increment this with each new deployment + string internal _version = "0.1.2"; // increment this with each new deployment address internal _feeSplitRecipient = 0x58C8854a8E51BdCE9F00726B966905FE2719B4D9; uint256 internal _feeSplitPercentage = 500; // 5% @@ -69,7 +69,10 @@ contract Deploy is Script { * 2. The provided salt, `SALT` */ implementation = - new PublicLockV14Eligibility{ salt: SALT }(_version, getUnlockAddress(), _feeSplitRecipient, _feeSplitPercentage); + new PublicLockV14Eligibility{ salt: SALT }(_version, _feeSplitRecipient, _feeSplitPercentage, deployer()); + + // set the unlock address on the implementation + implementation.setUnlock(getUnlockAddress()); vm.stopBroadcast(); diff --git a/src/PublicLockV14Eligibility.sol b/src/PublicLockV14Eligibility.sol index e7ad199..16dea6b 100644 --- a/src/PublicLockV14Eligibility.sol +++ b/src/PublicLockV14Eligibility.sol @@ -31,6 +31,12 @@ contract PublicLockV14Eligibility is HatsEligibilityModule, ILockKeyPurchaseHook /// @dev Thrown when a non-referrer calls a function only authorized to the referrer error NotReferrer(); + /// @dev Thrown when the implementation contract has already been initialized + error ImplementationAlreadyInitialized(); + + /// @dev Thrown when a non-initializer calls a function that can only be called by the initializer + error NotInitializer(); + /*////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ @@ -93,6 +99,13 @@ contract PublicLockV14Eligibility is HatsEligibilityModule, ILockKeyPurchaseHook /// @notice The Unlock Protocol lock contract that is created along with this module and coupled to the hat IPublicLock public lock; + /// @notice The address authorized to initialize the implementation contract by setting the Unlock Protocol factory + /// contract address + address internal _initializer; + + /// @notice Whether the implementation contract has been initialized + bool internal _implementationInitialized; + /*////////////////////////////////////////////////////////////// CONSTRUCTOR //////////////////////////////////////////////////////////////*/ @@ -101,17 +114,40 @@ contract PublicLockV14Eligibility is HatsEligibilityModule, ILockKeyPurchaseHook /// @param _version The version of the implementation contract /// @param _referrer The referrer address, which will receive a portion of the fees /// @param __referrerFeePercentage The percentage of fees to go to the referrer, in basis points (10000 = 100%) + /// @param __initializer The address authorized to initialize the implementation contract by setting the Unlock + /// Protocol factory contract address /// @dev This is only used to deploy the implementation contract, and should not be used to deploy clones - constructor(string memory _version, IUnlock _unlock, address _referrer, uint256 __referrerFeePercentage) + constructor(string memory _version, address _referrer, uint256 __referrerFeePercentage, address __initializer) HatsModule(_version) { - unlock_ = _unlock; REFERRER = _referrer; implementationReferrerFeePercentage = __referrerFeePercentage; + _initializer = __initializer; + } + + /*////////////////////////////////////////////////////////////// + IMPLEMENTATION INITIALIZER + //////////////////////////////////////////////////////////////*/ + + /// @notice Sets the Unlock Protocol factory contract. This function must be called before instances can be created. + /// @dev This function can only be called once by the initializer + /// @param _unlock The Unlock Protocol factory contract + function setUnlock(IUnlock _unlock) external { + // caller must be the initializer + if (msg.sender != _initializer) revert NotInitializer(); + + // the implementationcontract must not be initialized + if (_implementationInitialized) revert ImplementationAlreadyInitialized(); + + // prevent re-initialization + _implementationInitialized = true; + + // set the unlock contract + unlock_ = _unlock; } /*////////////////////////////////////////////////////////////// - INITIALIZER + INSTANCE INITIALIZER //////////////////////////////////////////////////////////////*/ /// @inheritdoc HatsModule diff --git a/test/PublicLockEligibility.t.sol b/test/PublicLockEligibility.t.sol index 7c265b9..032007e 100644 --- a/test/PublicLockEligibility.t.sol +++ b/test/PublicLockEligibility.t.sol @@ -17,7 +17,7 @@ contract PublicLockV14EligibilityTest is Deploy, Test { uint256 public fork; uint256 public BLOCK_NUMBER = 19_467_227; // deployment block for HatsModuleFactory v0.7.0 - string public NETWORK = "mainnet"; + IHats public HATS = IHats(0x3bc1A0Ad72417f2d411118085256fC53CBdDd137); // v1.hatsprotocol.eth HatsModuleFactory public factory; PublicLockV14Eligibility public instance; @@ -55,8 +55,11 @@ contract PublicLockV14EligibilityTest is Deploy, Test { string public MODULE_VERSION; function setUp() public virtual { - // create and activate a fork, at BLOCK_NUMBER - fork = _createForkForNetwork(NETWORK); + // create and activate a fork, unless we're already on a fork + // 31337 is the chain id for the default local network + if (block.chainid == 31_337) { + fork = _createForkForNetwork("mainnet"); + } // deploy implementation via the script prepare(false, MODULE_VERSION, referrer, referrerFeePercentage); @@ -97,10 +100,6 @@ contract PublicLockV14EligibilityTest is Deploy, Test { function _createForkForNetwork(string memory _network) internal returns (uint256) { return vm.createSelectFork(vm.rpcUrl(_network), _getForkBlockForNetwork(_network)); } - - function test_mainnet() public { - _createForkForNetwork("mainnet"); - } } contract WithInstanceTest is PublicLockV14EligibilityTest { @@ -111,7 +110,6 @@ contract WithInstanceTest is PublicLockV14EligibilityTest { saltNonce = 1; // set lock init data - expirationDuration_ = 1 days; // 1 day tokenAddress_ = address(0); // ETH keyPrice_ = 1 ether; // 1 ETH @@ -243,6 +241,51 @@ contract Deployment is WithInstanceTest { // lock version assertEq(lock.publicLockVersion(), lockVersion); } + + function test_revert_setUnlock() public { + address unlockTry = makeAddr("unlockTry"); + + vm.prank(deployer()); + vm.expectRevert(PublicLockV14Eligibility.ImplementationAlreadyInitialized.selector); + implementation.setUnlock(IUnlock(unlockTry)); + } +} + +contract SetUnlock is PublicLockV14EligibilityTest { + PublicLockV14Eligibility public newImplementation; + bytes32 public newSalt = keccak256(abi.encode(1)); + address unlockTry = makeAddr("unlockTry"); + + function setUp() public override { + super.setUp(); + + // deploy a new implementation contract + newImplementation = + new PublicLockV14Eligibility{ salt: newSalt }(_version, _feeSplitRecipient, _feeSplitPercentage, deployer()); + } + + function test_happy() public { + // set the new implementation contract's unlock address + vm.prank(deployer()); + newImplementation.setUnlock(IUnlock(unlockTry)); + } + + function test_revert_notInitializer() public { + // try to set the unlock address from an arbitrary address, expect a revert + vm.expectRevert(PublicLockV14Eligibility.NotInitializer.selector); + newImplementation.setUnlock(IUnlock(unlockTry)); + } + + function test_revert_alreadyInitialized() public { + // // set the new implementation contract's unlock address + vm.prank(deployer()); + newImplementation.setUnlock(IUnlock(unlockTry)); + + // try to set the unlock address from an authorized address, expect a revert now that its already initialized + vm.prank(deployer()); + vm.expectRevert(PublicLockV14Eligibility.ImplementationAlreadyInitialized.selector); + newImplementation.setUnlock(IUnlock(unlockTry)); + } } contract DeploymentArbitrum is Deployment { @@ -255,6 +298,8 @@ contract DeploymentArbitrum is Deployment { function test_arbitrum() public { test_createLock(); + console2.log("chainid", block.chainid); + console2.log("implementation", address(implementation)); } } @@ -268,6 +313,8 @@ contract DeploymentBase is Deployment { function test_base() public { test_createLock(); + console2.log("chainid", block.chainid); + console2.log("implementation", address(implementation)); } } @@ -281,6 +328,8 @@ contract DeploymentCelo is Deployment { function test_celo() public { test_createLock(); + console2.log("chainid", block.chainid); + console2.log("implementation", address(implementation)); } } @@ -294,6 +343,8 @@ contract DeploymentGnosis is Deployment { function test_gnosis() public { test_createLock(); + console2.log("chainid", block.chainid); + console2.log("implementation", address(implementation)); } } @@ -307,6 +358,8 @@ contract DeploymentOptimism is Deployment { function test_optimism() public { test_createLock(); + console2.log("chainid", block.chainid); + console2.log("implementation", address(implementation)); } } @@ -320,6 +373,8 @@ contract DeploymentPolygon is Deployment { function test_polygon() public { test_createLock(); + console2.log("chainid", block.chainid); + console2.log("implementation", address(implementation)); } } @@ -333,6 +388,8 @@ contract DeploymentSepolia is Deployment { function test_sepolia() public { test_createLock(); + console2.log("chainid", block.chainid); + console2.log("implementation", address(implementation)); } } From 926be6f33a10bb5f4e8c9a0fc7555ef9e8386a64 Mon Sep 17 00:00:00 2001 From: spengrah Date: Sat, 14 Sep 2024 20:07:14 -0500 Subject: [PATCH 2/3] deal with gnosis public rpc limited history --- script/Deployments.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/Deployments.json b/script/Deployments.json index a192240..23451ba 100644 --- a/script/Deployments.json +++ b/script/Deployments.json @@ -40,7 +40,7 @@ "unlockDeploymentBlock": 13994123 }, "100": { - "hatsModuleFactoryDeploymentBlock": 34772144, + "hatsModuleFactoryDeploymentBlock": 36005519, "unlock": "0x1bc53f4303c711cc693F6Ec3477B83703DcB317f", "unlockDeploymentBlock": 19338710 }, From 71acd0766d478bbfcc0d0ecd0859faeeab40b791 Mon Sep 17 00:00:00 2001 From: spengrah Date: Sun, 15 Sep 2024 10:11:25 -0500 Subject: [PATCH 3/3] update readme --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 74a3cc3..c9d87b2 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,12 @@ When users purchase a key, they are also automatically minted the hat. To remain The module must serve as both a Hats eligibility module and a hatter contract. To mint the target hat when a user purchases a new key, it must be an amin of hte target hat — i.e. wear one of the target hat's admin hats — which makes it a hatter contract. To control eligibility for the target hat, it must also be set as the eligibility module for the target hat. +## Implementation Deployment + +In order to deploy a new implementation — eg to a new network — you must not only call the constructor but also the `setUnlock()` initializer function. This function sets the address of the Unlock contract that instances created from the new implementation will use. This is separate from the constructor to enable deployment to use the same initCode and therefore achieve the same address across multiple chains, even though the Unlock address differs by chain. + +The full flow is included in the `script/Deploy.s.sol` script. + ## Development This repo uses Foundry for development and testing. To get started: