diff --git a/.prettierrc b/.prettierrc index 937339c8..9b687cc7 100644 --- a/.prettierrc +++ b/.prettierrc @@ -7,6 +7,7 @@ "semi": true, "arrowParens": "avoid", "singleAttributePerLine": true, + "trailingComma": "all", "overrides": [ { "files": "*.sol", diff --git a/contracts/DecentHats_0_1_0.sol b/contracts/DecentHats_0_1_0.sol index a0d9a7bd..a44756ee 100644 --- a/contracts/DecentHats_0_1_0.sol +++ b/contracts/DecentHats_0_1_0.sol @@ -49,7 +49,7 @@ contract DecentHats_0_1_0 { 0x5d0e6ce4fd951366cc55da93f6e79d8b81483109d79676a04bcc2bed6a4b5072; } - function updateKeyValuePairs( + function declareSafeHatTree( address _keyValuePairs, uint256 topHatId ) internal { @@ -195,6 +195,54 @@ contract DecentHats_0_1_0 { } } + /** + * Creates a new role hat and any streams on it. + * + * This contract should be enabled a module on the Safe for which the role is to be created, and disable after. + * In order for the module to be able to create hats on behalf of the Safe, the Safe must first + * transfer its top hat to this contract. This function transfers the top hat back to the Safe after + * creating the role hat. + * + * The function simply calls `createHatAndAccountAndMintAndStreams` and then transfers the top hat back to the Safe. + * + * @dev Role hat creation, minting, smart account creation and stream creation are handled here in order + * to avoid a race condition where not more than one active proposal to create a new role can exist at a time. + * See: https://github.com/decentdao/decent-interface/issues/2402 + */ + function createRoleHat( + IHats hatsProtocol, + uint256 adminHatId, + Hat calldata hat, + uint256 topHatId, + address topHatAccount, + IERC6551Registry registry, + address hatsAccountImplementation, + bytes32 salt + ) public returns (uint256 hatId, address accountAddress) { + (hatId, accountAddress) = createHatAndAccountAndMintAndStreams( + hatsProtocol, + adminHatId, + hat, + topHatAccount, + registry, + hatsAccountImplementation, + salt + ); + + hatsProtocol.transferHat(topHatId, address(this), msg.sender); + } + + /** + * For a safe without any roles previously created on it, this function should be called. It sets up the + * top hat and admin hat, as well as any other hats and their streams that are provided. + * + * This contract should be enabled a module on the Safe for which the role(s) are to be created, and disabled after. + * + * @dev In order for a Safe to seamlessly create roles even if it has never previously created a role and thus has + * no hat tree, we defer the creation of the hat tree and its setup to this contract. This way, in a single tx block, + * the resulting topHatId of the newly created hat can be used to create an admin hat and any other hats needed. + * We also make use of `KeyValuePairs` to associate the topHatId with the Safe. + */ function createAndDeclareTree(CreateTreeParams calldata params) public { bytes32 salt = getSalt(); @@ -207,7 +255,7 @@ contract DecentHats_0_1_0 { salt ); - updateKeyValuePairs(params.keyValuePairs, topHatId); + declareSafeHatTree(params.keyValuePairs, topHatId); (uint256 adminHatId, ) = createHatAndAccountAndMintAndStreams( params.hatsProtocol, diff --git a/contracts/interfaces/hats/IHats.sol b/contracts/interfaces/hats/IHats.sol index c460c46d..e78f6c61 100644 --- a/contracts/interfaces/hats/IHats.sol +++ b/contracts/interfaces/hats/IHats.sol @@ -39,4 +39,9 @@ interface IHats { ) external returns (bool success); function transferHat(uint256 _hatId, address _from, address _to) external; + + function isWearerOfHat( + address _user, + uint256 _hatId + ) external view returns (bool isWearer); } diff --git a/contracts/mocks/MockHats.sol b/contracts/mocks/MockHats.sol index f5ac2ece..0eb011a0 100644 --- a/contracts/mocks/MockHats.sol +++ b/contracts/mocks/MockHats.sol @@ -5,14 +5,16 @@ import {IHats} from "../interfaces/hats/IHats.sol"; contract MockHats is IHats { uint256 public count = 0; + mapping(uint256 => address) hatWearers; function mintTopHat( - address, + address _target, string memory, string memory ) external returns (uint256 topHatId) { topHatId = count; count++; + hatWearers[topHatId] = _target; } function createHat( @@ -28,9 +30,26 @@ contract MockHats is IHats { count++; } - function mintHat(uint256, address) external pure returns (bool success) { + function mintHat( + uint256 hatId, + address wearer + ) external returns (bool success) { success = true; + hatWearers[hatId] = wearer; + } + + function transferHat(uint256 _hatId, address _from, address _to) external { + require( + hatWearers[_hatId] == _from, + "MockHats: Invalid current wearer" + ); + hatWearers[_hatId] = _to; } - function transferHat(uint256, address, address) external {} + function isWearerOfHat( + address _user, + uint256 _hatId + ) external view returns (bool isWearer) { + isWearer = hatWearers[_hatId] == _user; + } } diff --git a/test/DecentHats_0_1_0.test.ts b/test/DecentHats_0_1_0.test.ts index d20a600c..7ffb285e 100644 --- a/test/DecentHats_0_1_0.test.ts +++ b/test/DecentHats_0_1_0.test.ts @@ -506,5 +506,127 @@ describe('DecentHats_0_1_0', () => { expect(stream2.endTime).to.equal(currentBlockTimestamp + 1296000); }); }); + + describe('Creating a new hat on existing Tree', () => { + let createRoleHatPromise: Promise; + const topHatId = 0; + + beforeEach(async () => { + await executeSafeTransaction({ + safe: gnosisSafe, + to: decentHatsAddress, + transactionData: DecentHats_0_1_0__factory.createInterface().encodeFunctionData( + 'createAndDeclareTree', + [ + { + hatsProtocol: mockHatsAddress, + hatsAccountImplementation: mockHatsAccountImplementationAddress, + registry: await erc6551Registry.getAddress(), + keyValuePairs: await keyValuePairs.getAddress(), + topHatDetails: '', + topHatImageURI: '', + adminHat: { + maxSupply: 1, + details: '', + imageURI: '', + isMutable: false, + wearer: ethers.ZeroAddress, + sablierParams: [], + }, + hats: [], + }, + ], + ), + signers: [dao], + }); + + const currentBlockTimestamp = (await hre.ethers.provider.getBlock('latest'))!.timestamp; + + createRoleHatPromise = executeSafeTransaction({ + safe: gnosisSafe, + to: decentHatsAddress, + transactionData: DecentHats_0_1_0__factory.createInterface().encodeFunctionData( + 'createRoleHat', + [ + mockHatsAddress, + 1, + { + maxSupply: 1, + details: '', + imageURI: '', + isMutable: true, + wearer: '0xdce7ca0555101f97451926944f5ae3b7adb2f5ae', + sablierParams: [ + { + sablier: mockSablierAddress, + sender: gnosisSafeAddress, + totalAmount: ethers.parseEther('100'), + asset: mockERC20Address, + cancelable: true, + transferable: false, + timestamps: { + start: currentBlockTimestamp, + cliff: currentBlockTimestamp + 86400, // 1 day cliff + end: currentBlockTimestamp + 2592000, // 30 days from now + }, + broker: { account: ethers.ZeroAddress, fee: 0 }, + }, + ], + }, + 0, + '0xdce7ca0555101f97451926944f5ae3b7adb2f5ae', + await erc6551Registry.getAddress(), + mockHatsAccountImplementationAddress, + '0x5d0e6ce4fd951366cc55da93f6e79d8b81483109d79676a04bcc2bed6a4b5072', + ], + ), + signers: [dao], + }); + }); + + it('Reverts if the top hat is not transferred to the DecentHats module first', async () => { + await expect(createRoleHatPromise).to.be.reverted; + }); + + it('Emits an ExecutionSuccess event', async () => { + // First transfer the top hat to the Safe + await mockHats.transferHat(topHatId, gnosisSafeAddress, decentHatsAddress); + await expect(await createRoleHatPromise).to.emit(gnosisSafe, 'ExecutionSuccess'); + }); + + it('Emits an ExecutionFromModuleSuccess event', async () => { + // First transfer the top hat to the Safe + await mockHats.transferHat(topHatId, gnosisSafeAddress, decentHatsAddress); + await expect(await createRoleHatPromise) + .to.emit(gnosisSafe, 'ExecutionFromModuleSuccess') + .withArgs(decentHatsAddress); + }); + + it('Transfers the top hat back to the Safe', async () => { + // First transfer the top hat to the Safe + await mockHats.transferHat(topHatId, gnosisSafeAddress, decentHatsAddress); + + const isModuleWearerOfTopHat = await mockHats.isWearerOfHat(decentHatsAddress, topHatId); + expect(isModuleWearerOfTopHat).to.equal(true); + + await createRoleHatPromise; + + const isSafeWearerOfTopHat = await mockHats.isWearerOfHat(gnosisSafeAddress, topHatId); + expect(isSafeWearerOfTopHat).to.equal(true); + }); + + it('Actually creates the new hat', async () => { + // First transfer the top hat to the Safe + await mockHats.transferHat(topHatId, gnosisSafeAddress, decentHatsAddress); + + const hatsCountBeforeCreate = await mockHats.count(); + expect(hatsCountBeforeCreate).to.equal(2); // Top hat + admin hat + + await createRoleHatPromise; + + const newHatId = await mockHats.count(); + expect(newHatId).to.equal(3); // + newly created hat + }); + }); }); });