Skip to content

Commit

Permalink
Merge pull request #14 from Hats-Protocol/salt-nonce
Browse files Browse the repository at this point in the history
add saltNonce to Factory
  • Loading branch information
gershido authored Mar 19, 2024
2 parents 56e8d93 + b0705f6 commit e83bd72
Show file tree
Hide file tree
Showing 8 changed files with 186 additions and 72 deletions.
2 changes: 1 addition & 1 deletion script/HatsEligibilitiesChain.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract DeployImplementation is Script {
address deployer = vm.rememberKey(privKey);
vm.startBroadcast(deployer);

implementation = new HatsEligibilitiesChain{ salt: SALT}(version);
implementation = new HatsEligibilitiesChain{ salt: SALT }(version);

vm.stopBroadcast();

Expand Down
2 changes: 1 addition & 1 deletion script/HatsTogglesChain.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract DeployImplementation is Script {
address deployer = vm.rememberKey(privKey);
vm.startBroadcast(deployer);

implementation = new HatsTogglesChain{ salt: SALT}(version);
implementation = new HatsTogglesChain{ salt: SALT }(version);

vm.stopBroadcast();

Expand Down
56 changes: 35 additions & 21 deletions src/HatsModuleFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ contract HatsModuleFactory {
//////////////////////////////////////////////////////////////*/

/**
* @notice Emitted if attempting to deploy a clone of `implementation` for a given `hatId` and `otherImmutableArgs`
* that already has a HatsModule deployment
* @notice Emitted if attempting to deploy a clone of `implementation` for a given `hatId`, `otherImmutableArgs`, and
* `saltNonce` that already has a HatsModule deployment
*/
error HatsModuleFactory_ModuleAlreadyDeployed(address implementation, uint256 hatId, bytes otherImmutableArgs);
error HatsModuleFactory_ModuleAlreadyDeployed(
address implementation, uint256 hatId, bytes otherImmutableArgs, uint256 saltNonce
);

/// @notice Emitted when array arguments to a batch function have mismatching lengths
error BatchArrayLengthMismatch();
Expand All @@ -24,9 +26,10 @@ contract HatsModuleFactory {
EVENTS
//////////////////////////////////////////////////////////////*/

/// @notice Emitted when a HatsModule for `hatId` and `otherImmutableArgs` is deployed to address `instance`
/// @notice Emitted when a HatsModule for `hatId`, `otherImmutableArgs`, and `saltNonce` is deployed to address
/// `instance`
event HatsModuleFactory_ModuleDeployed(
address implementation, address instance, uint256 hatId, bytes otherImmutableArgs, bytes initData
address implementation, address instance, uint256 hatId, bytes otherImmutableArgs, bytes initData, uint256 saltNonce
);

/*//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -64,21 +67,23 @@ contract HatsModuleFactory {
* @param _otherImmutableArgs Other immutable args to pass to the clone as immutable storage.
* @param _initData The encoded data to pass to the `setUp` function of the new HatsModule instance. Leave empty if no
* {setUp} is required.
* @param _saltNonce The nonce to use when calculating the salt
* @return _instance The address of the deployed HatsModule instance
*/
function createHatsModule(
address _implementation,
uint256 _hatId,
bytes calldata _otherImmutableArgs,
bytes calldata _initData
bytes calldata _initData,
uint256 _saltNonce
) public returns (address _instance) {
// calculate unique params that will be used to check for existing deployments and deploy the clone if none exists
bytes memory args = _encodeArgs(_implementation, _hatId, _otherImmutableArgs);
bytes32 _salt = _calculateSalt(args);
bytes32 _salt = _calculateSalt(args, _saltNonce);

// check if a HatsModule has already been deployed for these parameters
if (_getHatsModuleAddress(_implementation, args, _salt).code.length > 0) {
revert HatsModuleFactory_ModuleAlreadyDeployed(_implementation, _hatId, _otherImmutableArgs);
revert HatsModuleFactory_ModuleAlreadyDeployed(_implementation, _hatId, _otherImmutableArgs, _saltNonce);
}

// deploy the clone to a deterministic address
Expand All @@ -88,7 +93,9 @@ contract HatsModuleFactory {
HatsModule(_instance).setUp(_initData);

// log the deployment
emit HatsModuleFactory_ModuleDeployed(_implementation, address(_instance), _hatId, _otherImmutableArgs, _initData);
emit HatsModuleFactory_ModuleDeployed(
_implementation, address(_instance), _hatId, _otherImmutableArgs, _initData, _saltNonce
);
}

/**
Expand All @@ -101,13 +108,15 @@ contract HatsModuleFactory {
* @param _otherImmutableArgsArray Other immutable args to pass to the clones as immutable storage.
* @param _initDataArray The encoded data to pass to the `setUp` functions of the new HatsModule instances. Leave
* empty if no {setUp} is required.
* @param _saltNonces The nonces to use when calculating the salts
* @return success True if all modules were successfully created
*/
function batchCreateHatsModule(
address[] calldata _implementations,
uint256[] calldata _hatIds,
bytes[] calldata _otherImmutableArgsArray,
bytes[] calldata _initDataArray
bytes[] calldata _initDataArray,
uint256[] calldata _saltNonces
) public returns (bool success) {
uint256 length = _implementations.length;

Expand All @@ -118,7 +127,7 @@ contract HatsModuleFactory {
}

for (uint256 i = 0; i < length;) {
createHatsModule(_implementations[i], _hatIds[i], _otherImmutableArgsArray[i], _initDataArray[i]);
createHatsModule(_implementations[i], _hatIds[i], _otherImmutableArgsArray[i], _initDataArray[i], _saltNonces[i]);

unchecked {
++i;
Expand All @@ -131,16 +140,19 @@ contract HatsModuleFactory {
/**
* @notice Predicts the address of a HatsModule instance for a given hat
* @param _hatId The hat for which to predict the HatsModule instance address
* @param _otherImmutableArgs Other immutable args to pass to the clone as immutable storage.
* @param _saltNonce The nonce to use when calculating the salt
* @return The predicted address of the deployed instance
*/
function getHatsModuleAddress(address _implementation, uint256 _hatId, bytes calldata _otherImmutableArgs)
public
view
returns (address)
{
function getHatsModuleAddress(
address _implementation,
uint256 _hatId,
bytes calldata _otherImmutableArgs,
uint256 _saltNonce
) public view returns (address) {
// prepare the unique inputs
bytes memory args = _encodeArgs(_implementation, _hatId, _otherImmutableArgs);
bytes32 _salt = _calculateSalt(args);
bytes32 _salt = _calculateSalt(args, _saltNonce);
// predict the address
return _getHatsModuleAddress(_implementation, args, _salt);
}
Expand All @@ -149,15 +161,16 @@ contract HatsModuleFactory {
* @notice Checks if a HatsModule instance has already been deployed for a given hat
* @param _hatId The hat for which to check for an existing instance
* @param _otherImmutableArgs Other immutable args to pass to the clone as immutable storage.
* @param _saltNonce The nonce to use when calculating the salt
* @return True if an instance has already been deployed for the given hat
*/
function deployed(address _implementation, uint256 _hatId, bytes calldata _otherImmutableArgs)
function deployed(address _implementation, uint256 _hatId, bytes calldata _otherImmutableArgs, uint256 _saltNonce)
public
view
returns (bool)
{
// check for contract code at the predicted address
return getHatsModuleAddress(_implementation, _hatId, _otherImmutableArgs).code.length > 0;
return getHatsModuleAddress(_implementation, _hatId, _otherImmutableArgs, _saltNonce).code.length > 0;
}

/*//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -203,9 +216,10 @@ contract HatsModuleFactory {
* - The chain ID of the current network, to avoid confusion across networks since the same hat trees
* on different networks may have different wearers/admins
* @param _args The encoded arguments to pass to the clone as immutable storage
* @param _saltNonce The nonce to use when calculating the salt
* @return The salt to use when deploying the clone
*/
function _calculateSalt(bytes memory _args) internal view returns (bytes32) {
return keccak256(abi.encodePacked(_args, block.chainid));
function _calculateSalt(bytes memory _args, uint256 _saltNonce) internal view returns (bytes32) {
return keccak256(abi.encodePacked(_args, block.chainid, _saltNonce));
}
}
7 changes: 4 additions & 3 deletions src/utils/DeployFunctions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@ import { console2 } from "forge-std/Test.sol";
import { HatsModule, HatsModuleFactory, IHats } from "../HatsModuleFactory.sol";

function deployModuleFactory(IHats _hats, bytes32 _salt, string memory _version) returns (HatsModuleFactory _factory) {
_factory = new HatsModuleFactory{ salt: _salt}(_hats, _version);
_factory = new HatsModuleFactory{ salt: _salt }(_hats, _version);
}

function deployModuleInstance(
HatsModuleFactory _factory,
address _implementation,
uint256 _hatId,
bytes memory _otherImmutableArgs,
bytes memory _initData
bytes memory _initData,
uint256 _saltNonce
) returns (address _instance) {
_instance = _factory.createHatsModule(_implementation, _hatId, _otherImmutableArgs, _initData);
_instance = _factory.createHatsModule(_implementation, _hatId, _otherImmutableArgs, _initData, _saltNonce);
}
12 changes: 8 additions & 4 deletions test/HatsEligibilitiesChain.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ contract DeployImplementationTest is DeployImplementation, Test {

address[] expectedModules;

uint256 saltNonce = 1;

function deployInstanceTwoModules(
uint256 targetHat,
uint256 numClauses,
Expand All @@ -43,8 +45,9 @@ contract DeployImplementationTest is DeployImplementation, Test {
) public returns (HatsEligibilitiesChain) {
bytes memory otherImmutableArgs = abi.encodePacked(numClauses, lengths, _module1, _module2);
// deploy the instance
return
HatsEligibilitiesChain(deployModuleInstance(FACTORY, address(implementation), targetHat, otherImmutableArgs, ""));
return HatsEligibilitiesChain(
deployModuleInstance(FACTORY, address(implementation), targetHat, otherImmutableArgs, "", saltNonce)
);
}

function deployInstanceThreeModules(
Expand All @@ -57,8 +60,9 @@ contract DeployImplementationTest is DeployImplementation, Test {
) public returns (HatsEligibilitiesChain) {
bytes memory otherImmutableArgs = abi.encodePacked(numClauses, lengths, _module1, _module2, _module3);
// deploy the instance
return
HatsEligibilitiesChain(deployModuleInstance(FACTORY, address(implementation), targetHat, otherImmutableArgs, ""));
return HatsEligibilitiesChain(
deployModuleInstance(FACTORY, address(implementation), targetHat, otherImmutableArgs, "", saltNonce)
);
}

function setUp() public virtual {
Expand Down
17 changes: 11 additions & 6 deletions test/HatsModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,21 @@ contract DeployInstance is HatsModuleTest {
// expect event emitted with the initData
vm.expectEmit(true, true, true, true);
emit HatsModuleFactory_ModuleDeployed(
address(impl), factory.getHatsModuleAddress(address(impl), hatId, otherArgs), hatId, otherArgs, initData
address(impl),
factory.getHatsModuleAddress(address(impl), hatId, otherArgs, saltNonce),
hatId,
otherArgs,
initData,
saltNonce
);
// deploy an instance of HatsModuleHarness via the factory
// inst = HatsModuleHarness(factory.createHatsModule(address(impl), hatId, otherArgs, initData));
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData));
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData, saltNonce));
}

function test_immutables() public {
// deploy an instance of HatsModuleHarness via the factory
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData));
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData, saltNonce));

assertEq(address(inst.IMPLEMENTATION()), address(impl), "incorrect implementation address");
assertEq(address(inst.HATS()), address(hats), "incorrect hats address");
Expand All @@ -78,14 +83,14 @@ contract DeployInstance is HatsModuleTest {

function test_version() public {
// deploy an instance of HatsModuleHarness via the factory
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData));
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData, saltNonce));

assertEq(inst.version(), MODULE_VERSION, "incorrect module version");
}

function test_setUp_cannotBeCalledTwice() public {
// deploy an instance of HatsModuleHarness via the factory
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData));
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData, saltNonce));

// expect revert if setUp is called again
vm.expectRevert();
Expand All @@ -97,6 +102,6 @@ contract DeployInstance is HatsModuleTest {
vm.expectEmit(true, true, true, true);
emit HatsModuleHarness_SetUp(initData);
// deploy an instance of HatsModuleHarness via the factory
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData));
inst = HatsModuleHarness(deployModuleInstance(factory, address(impl), hatId, otherArgs, initData, saltNonce));
}
}
Loading

0 comments on commit e83bd72

Please sign in to comment.