From 797689c653b78f479455640009b960d875e4525b Mon Sep 17 00:00:00 2001 From: Rajath Alex Date: Fri, 8 Dec 2023 18:37:22 -0500 Subject: [PATCH] bySig fixes --- src/token-voting/TokenholderActionCreator.sol | 21 ++- .../ERC20TokenholderActionCreator.t.sol | 125 +++++--------- .../LlamaTokenVotingFactory.t.sol | 8 + .../LlamaTokenVotingTestSetup.sol | 63 +++++-- test/utils/LlamaCoreSigUtils.sol | 155 +----------------- 5 files changed, 117 insertions(+), 255 deletions(-) diff --git a/src/token-voting/TokenholderActionCreator.sol b/src/token-voting/TokenholderActionCreator.sol index 3558d74..79ad3a9 100644 --- a/src/token-voting/TokenholderActionCreator.sol +++ b/src/token-voting/TokenholderActionCreator.sol @@ -27,9 +27,13 @@ abstract contract TokenholderActionCreator is Initializable { /// @dev This role is expected to have the permissions to create appropriate actions. uint8 public role; - /// @dev EIP-712 actionInfo typehash. - bytes32 internal constant ACTION_INFO_TYPEHASH = keccak256( - "ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" + /// @dev EIP-712 base typehash. + bytes32 internal constant EIP712_DOMAIN_TYPEHASH = + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + + /// @dev EIP-712 createAction typehash. + bytes32 internal constant CREATE_ACTION_TYPEHASH = keccak256( + "CreateAction(address tokenHolder,address strategy,address target,uint256 value,bytes data,string description,uint256 nonce)" ); /// @dev EIP-712 cancelAction typehash. @@ -37,15 +41,11 @@ abstract contract TokenholderActionCreator is Initializable { "CancelAction(address tokenHolder,ActionInfo actionInfo,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); - /// @dev EIP-712 createAction typehash. - bytes32 internal constant CREATE_ACTION_TYPEHASH = keccak256( - "CreateAction(address tokenHolder,uint8 role,address strategy,address target,uint256 value,bytes data,string description,uint256 nonce)" + /// @dev EIP-712 actionInfo typehash. + bytes32 internal constant ACTION_INFO_TYPEHASH = keccak256( + "ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); - /// @dev EIP-712 base typehash. - bytes32 internal constant EIP712_DOMAIN_TYPEHASH = - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); - /// @notice The address of the tokenholder that created the action. mapping(uint256 => address) public actionCreators; @@ -283,7 +283,6 @@ abstract contract TokenholderActionCreator is Initializable { abi.encode( CREATE_ACTION_TYPEHASH, tokenHolder, - role, address(strategy), target, value, diff --git a/test/token-voting/ERC20TokenholderActionCreator.t.sol b/test/token-voting/ERC20TokenholderActionCreator.t.sol index 24b25dc..51686d3 100644 --- a/test/token-voting/ERC20TokenholderActionCreator.t.sol +++ b/test/token-voting/ERC20TokenholderActionCreator.t.sol @@ -32,7 +32,6 @@ contract ERC20TokenholderActionCreatorTest is LlamaTokenVotingTestSetup, LlamaCo event ActionThresholdSet(uint256 newThreshold); ERC20TokenholderActionCreator erc20TokenholderActionCreator; - uint8 erc20TokenholderActionCreatorRole; function setUp() public virtual override { LlamaTokenVotingTestSetup.setUp(); @@ -48,7 +47,7 @@ contract ERC20TokenholderActionCreatorTest is LlamaTokenVotingTestSetup, LlamaCo mineBlock(); // Deploy ERC20 Token Voting Module. - (erc20TokenholderActionCreator,) = _deployERC20TokenVotingModule(); + (erc20TokenholderActionCreator,) = _deployERC20TokenVotingModuleAndSetRole(); // Setting ERC20TokenHolderActionCreator's EIP-712 Domain Hash setDomainHash( @@ -61,19 +60,11 @@ contract ERC20TokenholderActionCreatorTest is LlamaTokenVotingTestSetup, LlamaCo ); } - function _initRoleSetRoleHolderSetRolePermissionToTokenholderActionCreator() internal { - // Init role, assign policy, and assign permission for `setRoleHolder` to the TokenholderActionCreator. - vm.startPrank(address(EXECUTOR)); - POLICY.initializeRole(TOKEN_VOTING_ACTION_CREATOR_ROLE_DESC); - erc20TokenholderActionCreatorRole = POLICY.numRoles(); - POLICY.setRoleHolder( - erc20TokenholderActionCreatorRole, - address(erc20TokenholderActionCreator), - DEFAULT_ROLE_QTY, - DEFAULT_ROLE_EXPIRATION - ); + function _setRolePermissionToTokenholderActionCreator() internal { + // Assign permission for `MockProtocol.pause` to the TokenholderActionCreator. + vm.prank(address(EXECUTOR)); POLICY.setRolePermission( - erc20TokenholderActionCreatorRole, + tokenVotingActionCreatorRole, ILlamaPolicy.PermissionData(address(mockProtocol), PAUSE_SELECTOR, address(STRATEGY)), true ); @@ -131,9 +122,7 @@ contract CreateAction is ERC20TokenholderActionCreatorTest { vm.expectRevert(abi.encodeWithSelector(TokenholderActionCreator.InsufficientBalance.selector, 0)); vm.prank(notTokenHolder); - erc20TokenholderActionCreator.createAction( - erc20TokenholderActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" - ); + erc20TokenholderActionCreator.createAction(STRATEGY, address(mockProtocol), 0, data, ""); } function test_RevertsIf_TokenholderActionCreatorDoesNotHavePermission() public { @@ -145,14 +134,12 @@ contract CreateAction is ERC20TokenholderActionCreatorTest { vm.expectRevert(ILlamaCore.PolicyholderDoesNotHavePermission.selector); vm.prank(tokenHolder1); - erc20TokenholderActionCreator.createAction( - erc20TokenholderActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" - ); + erc20TokenholderActionCreator.createAction(STRATEGY, address(mockProtocol), 0, data, ""); } function test_ProperlyCreatesAction() public { - // Assigns Policy to TokenholderActionCreator. - _initRoleSetRoleHolderSetRolePermissionToTokenholderActionCreator(); + // Assigns Permission to TokenholderActionCreator. + _setRolePermissionToTokenholderActionCreator(); // Mint tokens to tokenholder so that they can create action. erc20VotesToken.mint(tokenHolder1, ERC20_CREATION_THRESHOLD); @@ -166,19 +153,10 @@ contract CreateAction is ERC20TokenholderActionCreatorTest { vm.expectEmit(); emit ActionCreated( - actionCount, - address(tokenHolder1), - erc20TokenholderActionCreatorRole, - STRATEGY, - address(mockProtocol), - 0, - data, - "" + actionCount, address(tokenHolder1), tokenVotingActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" ); vm.prank(tokenHolder1); - uint256 actionId = erc20TokenholderActionCreator.createAction( - erc20TokenholderActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" - ); + uint256 actionId = erc20TokenholderActionCreator.createAction(STRATEGY, address(mockProtocol), 0, data, ""); Action memory action = CORE.getAction(actionId); assertEq(actionId, actionCount); @@ -190,8 +168,8 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { function setUp() public virtual override { ERC20TokenholderActionCreatorTest.setUp(); - // Assigns Policy to TokenholderActionCreator. - _initRoleSetRoleHolderSetRolePermissionToTokenholderActionCreator(); + // Assigns Permission to TokenholderActionCreator. + _setRolePermissionToTokenholderActionCreator(); // Mint tokens to tokenholder so that they can create action. erc20VotesToken.mint(tokenHolder1, ERC20_CREATION_THRESHOLD); @@ -211,32 +189,22 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { view returns (uint8 v, bytes32 r, bytes32 s) { - LlamaCoreSigUtils.CreateActionBySig memory _createAction = LlamaCoreSigUtils.CreateActionBySig({ - role: erc20TokenholderActionCreatorRole, + LlamaCoreSigUtils.CreateAction memory _createAction = LlamaCoreSigUtils.CreateAction({ + tokenHolder: tokenHolder1, strategy: address(STRATEGY), target: address(mockProtocol), value: 0, data: abi.encodeCall(mockProtocol.pause, (true)), description: description, - tokenHolder: tokenHolder1, nonce: 0 }); - bytes32 digest = getCreateActionBySigTypedDataHash(_createAction); + bytes32 digest = getCreateActionTypedDataHash(_createAction); (v, r, s) = vm.sign(privateKey, digest); } function createActionBySig(uint8 v, bytes32 r, bytes32 s) internal returns (uint256 actionId) { actionId = erc20TokenholderActionCreator.createActionBySig( - tokenHolder1, - erc20TokenholderActionCreatorRole, - STRATEGY, - address(mockProtocol), - 0, - abi.encodeCall(mockProtocol.pause, (true)), - "", - v, - r, - s + tokenHolder1, STRATEGY, address(mockProtocol), 0, abi.encodeCall(mockProtocol.pause, (true)), "", v, r, s ); } @@ -248,7 +216,7 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { vm.expectEmit(); emit ActionCreated( - actionCount, tokenHolder1, erc20TokenholderActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" + actionCount, tokenHolder1, tokenVotingActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" ); uint256 actionId = createActionBySig(v, r, s); @@ -270,7 +238,7 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { emit ActionCreated( actionCount, tokenHolder1, - erc20TokenholderActionCreatorRole, + tokenVotingActionCreatorRole, STRATEGY, address(mockProtocol), 0, @@ -280,7 +248,6 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { uint256 actionId = erc20TokenholderActionCreator.createActionBySig( tokenHolder1, - erc20TokenholderActionCreatorRole, STRATEGY, address(mockProtocol), 0, @@ -299,9 +266,9 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { function test_CheckNonceIncrements() public { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(tokenHolder1PrivateKey); - assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, ILlamaCore.createActionBySig.selector), 0); + assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, TokenholderActionCreator.createActionBySig.selector), 0); createActionBySig(v, r, s); - assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, ILlamaCore.createActionBySig.selector), 1); + assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, TokenholderActionCreator.createActionBySig.selector), 1); } function test_OperationCannotBeReplayed() public { @@ -309,7 +276,7 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { createActionBySig(v, r, s); // Invalid Signature error since the recovered signer address during the second call is not the same as // policyholder since nonce has increased. - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); createActionBySig(v, r, s); } @@ -318,7 +285,7 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(randomSignerPrivateKey); // Invalid Signature error since the recovered signer address is not the same as the policyholder passed in as // parameter. - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); createActionBySig(v, r, s); } @@ -326,7 +293,7 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(tokenHolder1PrivateKey); // Invalid Signature error since the recovered signer address is zero address due to invalid signature values // (v,r,s). - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); createActionBySig((v + 1), r, s); } @@ -334,11 +301,11 @@ contract CreateActionBySig is ERC20TokenholderActionCreatorTest { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(tokenHolder1PrivateKey); vm.prank(tokenHolder1); - erc20TokenholderActionCreator.incrementNonce(ILlamaCore.createActionBySig.selector); + erc20TokenholderActionCreator.incrementNonce(TokenholderActionCreator.createActionBySig.selector); // Invalid Signature error since the recovered signer address during the call is not the same as policyholder // since nonce has increased. - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); createActionBySig(v, r, s); } } @@ -350,8 +317,8 @@ contract CancelAction is ERC20TokenholderActionCreatorTest { function setUp() public virtual override { ERC20TokenholderActionCreatorTest.setUp(); - // Assigns Policy to TokenholderActionCreator. - _initRoleSetRoleHolderSetRolePermissionToTokenholderActionCreator(); + // Assigns Permission to TokenholderActionCreator. + _setRolePermissionToTokenholderActionCreator(); // Mint tokens to tokenholder so that they can create action. erc20VotesToken.mint(tokenHolder1, ERC20_CREATION_THRESHOLD); @@ -364,14 +331,12 @@ contract CancelAction is ERC20TokenholderActionCreatorTest { bytes memory data = abi.encodeCall(mockProtocol.pause, (true)); vm.prank(tokenHolder1); - actionId = erc20TokenholderActionCreator.createAction( - erc20TokenholderActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" - ); + actionId = erc20TokenholderActionCreator.createAction(STRATEGY, address(mockProtocol), 0, data, ""); actionInfo = ActionInfo( actionId, address(erc20TokenholderActionCreator), - erc20TokenholderActionCreatorRole, + tokenVotingActionCreatorRole, STRATEGY, address(mockProtocol), 0, @@ -401,8 +366,8 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { function setUp() public virtual override { ERC20TokenholderActionCreatorTest.setUp(); - // Assigns Policy to TokenholderActionCreator. - _initRoleSetRoleHolderSetRolePermissionToTokenholderActionCreator(); + // Assigns Permission to TokenholderActionCreator. + _setRolePermissionToTokenholderActionCreator(); // Mint tokens to tokenholder so that they can create action. erc20VotesToken.mint(tokenHolder1, ERC20_CREATION_THRESHOLD); @@ -415,14 +380,12 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { bytes memory data = abi.encodeCall(mockProtocol.pause, (true)); vm.prank(tokenHolder1); - actionId = erc20TokenholderActionCreator.createAction( - erc20TokenholderActionCreatorRole, STRATEGY, address(mockProtocol), 0, data, "" - ); + actionId = erc20TokenholderActionCreator.createAction(STRATEGY, address(mockProtocol), 0, data, ""); actionInfo = ActionInfo( actionId, address(erc20TokenholderActionCreator), - erc20TokenholderActionCreatorRole, + tokenVotingActionCreatorRole, STRATEGY, address(mockProtocol), 0, @@ -435,12 +398,12 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { view returns (uint8 v, bytes32 r, bytes32 s) { - LlamaCoreSigUtils.CancelActionBySig memory cancelAction = LlamaCoreSigUtils.CancelActionBySig({ + LlamaCoreSigUtils.CancelAction memory cancelAction = LlamaCoreSigUtils.CancelAction({ tokenHolder: tokenHolder1, actionInfo: _actionInfo, - nonce: erc20TokenholderActionCreator.nonces(tokenHolder1, ILlamaCore.cancelActionBySig.selector) + nonce: erc20TokenholderActionCreator.nonces(tokenHolder1, TokenholderActionCreator.cancelActionBySig.selector) }); - bytes32 digest = getCancelActionBySigTypedDataHash(cancelAction); + bytes32 digest = getCancelActionTypedDataHash(cancelAction); (v, r, s) = vm.sign(privateKey, digest); } @@ -463,9 +426,9 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { function test_CheckNonceIncrements() public { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(actionInfo, tokenHolder1PrivateKey); - assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, ILlamaCore.cancelActionBySig.selector), 0); + assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, TokenholderActionCreator.cancelActionBySig.selector), 0); cancelActionBySig(actionInfo, v, r, s); - assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, ILlamaCore.cancelActionBySig.selector), 1); + assertEq(erc20TokenholderActionCreator.nonces(tokenHolder1, TokenholderActionCreator.cancelActionBySig.selector), 1); } function test_OperationCannotBeReplayed() public { @@ -473,7 +436,7 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { cancelActionBySig(actionInfo, v, r, s); // Invalid Signature error since the recovered signer address during the second call is not the same as policyholder // since nonce has increased. - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); cancelActionBySig(actionInfo, v, r, s); } @@ -482,7 +445,7 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(actionInfo, randomSignerPrivateKey); // Invalid Signature error since the recovered signer address is not the same as the policyholder passed in as // parameter. - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); cancelActionBySig(actionInfo, v, r, s); } @@ -490,7 +453,7 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(actionInfo, tokenHolder1PrivateKey); // Invalid Signature error since the recovered signer address is zero address due to invalid signature values // (v,r,s). - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); cancelActionBySig(actionInfo, (v + 1), r, s); } @@ -498,11 +461,11 @@ contract CancelActionBySig is ERC20TokenholderActionCreatorTest { (uint8 v, bytes32 r, bytes32 s) = createOffchainSignature(actionInfo, tokenHolder1PrivateKey); vm.prank(tokenHolder1); - erc20TokenholderActionCreator.incrementNonce(ILlamaCore.cancelActionBySig.selector); + erc20TokenholderActionCreator.incrementNonce(TokenholderActionCreator.cancelActionBySig.selector); // Invalid Signature error since the recovered signer address during the call is not the same as policyholder // since nonce has increased. - vm.expectRevert(ILlamaCore.InvalidSignature.selector); + vm.expectRevert(TokenholderActionCreator.InvalidSignature.selector); cancelActionBySig(actionInfo, v, r, s); } } diff --git a/test/token-voting/LlamaTokenVotingFactory.t.sol b/test/token-voting/LlamaTokenVotingFactory.t.sol index 8b8b669..75a6811 100644 --- a/test/token-voting/LlamaTokenVotingFactory.t.sol +++ b/test/token-voting/LlamaTokenVotingFactory.t.sol @@ -91,6 +91,8 @@ contract DeployTokenVotingModule is LlamaTokenVotingFactoryTest { LlamaTokenVotingFactory.deployTokenVotingModule.selector, address(erc20VotesToken), true, + tokenVotingActionCreatorRole, + tokenVotingCasterRole, ERC20_CREATION_THRESHOLD, ERC20_MIN_APPROVAL_PCT, ERC20_MIN_DISAPPROVAL_PCT @@ -126,9 +128,11 @@ contract DeployTokenVotingModule is LlamaTokenVotingFactoryTest { assertEq(address(erc20TokenholderActionCreator.token()), address(erc20VotesToken)); assertEq(address(erc20TokenholderActionCreator.llamaCore()), address(CORE)); + assertEq(erc20TokenholderActionCreator.role(), tokenVotingActionCreatorRole); assertEq(erc20TokenholderActionCreator.creationThreshold(), ERC20_CREATION_THRESHOLD); assertEq(address(erc20TokenholderCaster.token()), address(erc20VotesToken)); assertEq(address(erc20TokenholderCaster.llamaCore()), address(CORE)); + assertEq(erc20TokenholderCaster.role(), tokenVotingCasterRole); assertEq(erc20TokenholderCaster.minApprovalPct(), ERC20_MIN_APPROVAL_PCT); assertEq(erc20TokenholderCaster.minDisapprovalPct(), ERC20_MIN_DISAPPROVAL_PCT); } @@ -139,6 +143,8 @@ contract DeployTokenVotingModule is LlamaTokenVotingFactoryTest { LlamaTokenVotingFactory.deployTokenVotingModule.selector, address(erc721VotesToken), false, + tokenVotingActionCreatorRole, + tokenVotingCasterRole, ERC721_CREATION_THRESHOLD, ERC721_MIN_APPROVAL_PCT, ERC721_MIN_DISAPPROVAL_PCT @@ -174,9 +180,11 @@ contract DeployTokenVotingModule is LlamaTokenVotingFactoryTest { assertEq(address(erc721TokenholderActionCreator.token()), address(erc721VotesToken)); assertEq(address(erc721TokenholderActionCreator.llamaCore()), address(CORE)); + assertEq(erc721TokenholderActionCreator.role(), tokenVotingActionCreatorRole); assertEq(erc721TokenholderActionCreator.creationThreshold(), ERC721_CREATION_THRESHOLD); assertEq(address(erc721TokenholderCaster.token()), address(erc721VotesToken)); assertEq(address(erc721TokenholderCaster.llamaCore()), address(CORE)); + assertEq(erc721TokenholderCaster.role(), tokenVotingCasterRole); assertEq(erc721TokenholderCaster.minApprovalPct(), ERC721_MIN_APPROVAL_PCT); assertEq(erc721TokenholderCaster.minDisapprovalPct(), ERC721_MIN_DISAPPROVAL_PCT); } diff --git a/test/token-voting/LlamaTokenVotingTestSetup.sol b/test/token-voting/LlamaTokenVotingTestSetup.sol index 950e6db..c6aeb9c 100644 --- a/test/token-voting/LlamaTokenVotingTestSetup.sol +++ b/test/token-voting/LlamaTokenVotingTestSetup.sol @@ -32,16 +32,15 @@ contract LlamaTokenVotingTestSetup is LlamaPeripheryTestSetup, DeployLlamaTokenV uint256 public constant ERC721_MIN_APPROVAL_PCT = 1; uint256 public constant ERC721_MIN_DISAPPROVAL_PCT = 1; - // Token Voting Role Descriptions. - RoleDescription public constant TOKEN_VOTING_ACTION_CREATOR_ROLE_DESC = - RoleDescription.wrap("Token Voting Action Creator Role"); - RoleDescription public constant TOKEN_VOTING_CASTER_ROLE_DESC = RoleDescription.wrap("Token Voting Caster Role"); - RoleDescription public constant MADE_UP_ROLE_DESC = RoleDescription.wrap("Made Up Role"); - // Votes Tokens MockERC20Votes public erc20VotesToken; MockERC721Votes public erc721VotesToken; + // Token Voting Roles + uint8 tokenVotingActionCreatorRole; + uint8 tokenVotingCasterRole; + uint8 madeUpRole; + // Token holders. address tokenHolder0; uint256 tokenHolder0PrivateKey; @@ -70,33 +69,71 @@ contract LlamaTokenVotingTestSetup is LlamaPeripheryTestSetup, DeployLlamaTokenV (tokenHolder2, tokenHolder2PrivateKey) = makeAddrAndKey("tokenHolder2"); (tokenHolder3, tokenHolder3PrivateKey) = makeAddrAndKey("tokenHolder3"); (notTokenHolder, notTokenHolderPrivateKey) = makeAddrAndKey("notTokenHolder"); + + // Initialize required roles. + vm.startPrank(address(EXECUTOR)); + POLICY.initializeRole(RoleDescription.wrap("Token Voting Action Creator Role")); + tokenVotingActionCreatorRole = POLICY.numRoles(); + POLICY.initializeRole(RoleDescription.wrap("Token Voting Caster Role")); + tokenVotingCasterRole = POLICY.numRoles(); + POLICY.initializeRole(RoleDescription.wrap("Made Up Role")); + madeUpRole = POLICY.numRoles(); + vm.stopPrank(); } // ========================= // ======== Helpers ======== // ========================= - function _deployERC20TokenVotingModule() internal returns (ERC20TokenholderActionCreator, ERC20TokenholderCaster) { - vm.prank(address(EXECUTOR)); + function _deployERC20TokenVotingModuleAndSetRole() + internal + returns (ERC20TokenholderActionCreator, ERC20TokenholderCaster) + { + vm.startPrank(address(EXECUTOR)); + // Deploy Token Voting Module (address erc20TokenholderActionCreator, address erc20TokenholderCaster) = tokenVotingFactory.deployTokenVotingModule( - address(erc20VotesToken), true, 0, 0, ERC20_CREATION_THRESHOLD, ERC20_MIN_APPROVAL_PCT, ERC20_MIN_DISAPPROVAL_PCT + address(erc20VotesToken), + true, + tokenVotingActionCreatorRole, + tokenVotingCasterRole, + ERC20_CREATION_THRESHOLD, + ERC20_MIN_APPROVAL_PCT, + ERC20_MIN_DISAPPROVAL_PCT + ); + // Assign roles to Token Voting Modules + POLICY.setRoleHolder( + tokenVotingActionCreatorRole, erc20TokenholderActionCreator, DEFAULT_ROLE_QTY, DEFAULT_ROLE_EXPIRATION ); + POLICY.setRoleHolder(tokenVotingCasterRole, erc20TokenholderCaster, DEFAULT_ROLE_QTY, DEFAULT_ROLE_EXPIRATION); + vm.stopPrank(); + return (ERC20TokenholderActionCreator(erc20TokenholderActionCreator), ERC20TokenholderCaster(erc20TokenholderCaster)); } - function _deployERC721TokenVotingModule() internal returns (ERC721TokenholderActionCreator, ERC721TokenholderCaster) { - vm.prank(address(EXECUTOR)); + function _deployERC721TokenVotingModuleAndSetRole() + internal + returns (ERC721TokenholderActionCreator, ERC721TokenholderCaster) + { + vm.startPrank(address(EXECUTOR)); + // Deploy Token Voting Module (address erc721TokenholderActionCreator, address erc721TokenholderCaster) = tokenVotingFactory .deployTokenVotingModule( address(erc721VotesToken), false, - 0, - 0, + tokenVotingActionCreatorRole, + tokenVotingCasterRole, ERC721_CREATION_THRESHOLD, ERC721_MIN_APPROVAL_PCT, ERC721_MIN_DISAPPROVAL_PCT ); + // Assign roles to Token Voting Modules + POLICY.setRoleHolder( + tokenVotingActionCreatorRole, erc721TokenholderActionCreator, DEFAULT_ROLE_QTY, DEFAULT_ROLE_EXPIRATION + ); + POLICY.setRoleHolder(tokenVotingCasterRole, erc721TokenholderCaster, DEFAULT_ROLE_QTY, DEFAULT_ROLE_EXPIRATION); + vm.stopPrank(); + return (ERC721TokenholderActionCreator(erc721TokenholderActionCreator), ERC721TokenholderCaster(erc721TokenholderCaster)); } diff --git a/test/utils/LlamaCoreSigUtils.sol b/test/utils/LlamaCoreSigUtils.sol index 3c752c9..ad44bd7 100644 --- a/test/utils/LlamaCoreSigUtils.sol +++ b/test/utils/LlamaCoreSigUtils.sol @@ -12,19 +12,7 @@ contract LlamaCoreSigUtils { } struct CreateAction { - address policyholder; - uint8 role; - address strategy; - address target; - uint256 value; - bytes data; - string description; - uint256 nonce; - } - - struct CreateActionBySig { address tokenHolder; - uint8 role; address strategy; address target; uint256 value; @@ -34,34 +22,12 @@ contract LlamaCoreSigUtils { } struct CancelAction { - address policyholder; - ActionInfo actionInfo; - uint256 nonce; - } - - struct CancelActionBySig { address tokenHolder; ActionInfo actionInfo; uint256 nonce; } struct CastApproval { - address policyholder; - uint8 role; - ActionInfo actionInfo; - string reason; - uint256 nonce; - } - - struct CastDisapproval { - address policyholder; - uint8 role; - ActionInfo actionInfo; - string reason; - uint256 nonce; - } - - struct CastApprovalBySig { address tokenHolder; uint8 support; ActionInfo actionInfo; @@ -69,7 +35,7 @@ contract LlamaCoreSigUtils { uint256 nonce; } - struct CastDisapprovalBySig { + struct CastDisapproval { address tokenHolder; uint8 support; ActionInfo actionInfo; @@ -83,41 +49,21 @@ contract LlamaCoreSigUtils { /// @notice EIP-712 createAction typehash. bytes32 internal constant CREATE_ACTION_TYPEHASH = keccak256( - "CreateAction(address policyholder,uint8 role,address strategy,address target,uint256 value,bytes data,string description,uint256 nonce)" - ); - - /// @notice EIP-712 createAction typehash. - bytes32 internal constant CREATE_ACTION_BY_SIG_TYPEHASH = keccak256( - "CreateAction(address tokenHolder,uint8 role,address strategy,address target,uint256 value,bytes data,string description,uint256 nonce)" + "CreateAction(address tokenHolder,address strategy,address target,uint256 value,bytes data,string description,uint256 nonce)" ); /// @dev EIP-712 cancelAction typehash. bytes32 internal constant CANCEL_ACTION_TYPEHASH = keccak256( - "CancelAction(address policyholder,ActionInfo actionInfo,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" - ); - - /// @dev EIP-712 cancelAction typehash. - bytes32 internal constant CANCEL_ACTION_BY_SIG_TYPEHASH = keccak256( "CancelAction(address tokenHolder,ActionInfo actionInfo,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); /// @notice EIP-712 castApproval typehash. bytes32 internal constant CAST_APPROVAL_TYPEHASH = keccak256( - "CastApproval(address policyholder,uint8 role,ActionInfo actionInfo,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" - ); - - /// @notice EIP-712 castApproval typehash. - bytes32 internal constant CAST_APPROVAL_BY_SIG_TYPEHASH = keccak256( "CastApproval(address tokenHolder,uint8 support,ActionInfo actionInfo,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); /// @notice EIP-712 castDisapproval typehash. bytes32 internal constant CAST_DISAPPROVAL_TYPEHASH = keccak256( - "CastDisapproval(address policyholder,uint8 role,ActionInfo actionInfo,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" - ); - - /// @notice EIP-712 castDisapproval typehash. - bytes32 internal constant CAST_DISAPPROVAL_BY_SIG_TYPEHASH = keccak256( "CastDisapproval(address tokenHolder,uint8 role,ActionInfo actionInfo,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); @@ -146,25 +92,7 @@ contract LlamaCoreSigUtils { return keccak256( abi.encode( CREATE_ACTION_TYPEHASH, - createAction.policyholder, - createAction.role, - createAction.strategy, - createAction.target, - createAction.value, - keccak256(createAction.data), - keccak256(bytes(createAction.description)), - createAction.nonce - ) - ); - } - - /// @notice Returns the hash of CreateAction. - function getCreateActionBySigHash(CreateActionBySig memory createAction) internal pure returns (bytes32) { - return keccak256( - abi.encode( - CREATE_ACTION_BY_SIG_TYPEHASH, createAction.tokenHolder, - createAction.role, createAction.strategy, createAction.target, createAction.value, @@ -181,32 +109,11 @@ contract LlamaCoreSigUtils { return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCreateActionHash(createAction))); } - /// @notice Returns the hash of the fully encoded EIP-712 message for the CreateAction domain, which can be used to - /// recover the signer. - function getCreateActionBySigTypedDataHash(CreateActionBySig memory createAction) internal view returns (bytes32) { - return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCreateActionBySigHash(createAction))); - } - - /// @notice Returns the hash of CancelAction. - function getCancelActionHash(CancelAction memory cancelAction) internal pure returns (bytes32) { - return keccak256( - abi.encode( - CANCEL_ACTION_TYPEHASH, - cancelAction.policyholder, - getActionInfoHash(cancelAction.actionInfo), - cancelAction.nonce - ) - ); - } - /// @notice Returns the hash of CancelActionBySig. - function getCancelActionBySigHash(CancelActionBySig memory cancelAction) internal pure returns (bytes32) { + function getCancelActionHash(CancelAction memory cancelAction) internal pure returns (bytes32) { return keccak256( abi.encode( - CANCEL_ACTION_BY_SIG_TYPEHASH, - cancelAction.tokenHolder, - getActionInfoHash(cancelAction.actionInfo), - cancelAction.nonce + CANCEL_ACTION_TYPEHASH, cancelAction.tokenHolder, getActionInfoHash(cancelAction.actionInfo), cancelAction.nonce ) ); } @@ -217,31 +124,11 @@ contract LlamaCoreSigUtils { return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCancelActionHash(cancelAction))); } - /// @notice Returns the hash of the fully encoded EIP-712 message for the CancelActionBySig domain, which can be used - /// to - /// recover the signer. - function getCancelActionBySigTypedDataHash(CancelActionBySig memory cancelAction) internal view returns (bytes32) { - return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCancelActionBySigHash(cancelAction))); - } - /// @notice Returns the hash of CastApproval. function getCastApprovalHash(CastApproval memory castApproval) internal pure returns (bytes32) { return keccak256( abi.encode( CAST_APPROVAL_TYPEHASH, - castApproval.policyholder, - castApproval.role, - getActionInfoHash(castApproval.actionInfo), - keccak256(bytes(castApproval.reason)), - castApproval.nonce - ) - ); - } - - function getCastApprovalBySigHash(CastApprovalBySig memory castApproval) internal pure returns (bytes32) { - return keccak256( - abi.encode( - CAST_APPROVAL_BY_SIG_TYPEHASH, castApproval.tokenHolder, castApproval.support, getActionInfoHash(castApproval.actionInfo), @@ -257,32 +144,11 @@ contract LlamaCoreSigUtils { return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCastApprovalHash(castApproval))); } - /// @notice Returns the hash of the fully encoded EIP-712 message for the CastApprovalBySig domain, which can be used - /// to - /// recover the signer. - function getCastApprovalBySigTypedDataHash(CastApprovalBySig memory castApproval) internal view returns (bytes32) { - return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCastApprovalBySigHash(castApproval))); - } - - /// @notice Returns the hash of CastDisapproval. + /// @notice Returns the hash of CastDisapprovalBySig. function getCastDisapprovalHash(CastDisapproval memory castDisapproval) internal pure returns (bytes32) { return keccak256( abi.encode( CAST_DISAPPROVAL_TYPEHASH, - castDisapproval.policyholder, - castDisapproval.role, - getActionInfoHash(castDisapproval.actionInfo), - keccak256(bytes(castDisapproval.reason)), - castDisapproval.nonce - ) - ); - } - - /// @notice Returns the hash of CastDisapprovalBySig. - function getCastDisapprovalBySigHash(CastDisapprovalBySig memory castDisapproval) internal pure returns (bytes32) { - return keccak256( - abi.encode( - CAST_DISAPPROVAL_BY_SIG_TYPEHASH, castDisapproval.tokenHolder, castDisapproval.support, getActionInfoHash(castDisapproval.actionInfo), @@ -298,17 +164,6 @@ contract LlamaCoreSigUtils { return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCastDisapprovalHash(castDisapproval))); } - /// @notice Returns the hash of the fully encoded EIP-712 message for the CastDisapprovalBySig domain, which can be - /// used to - /// recover the signer. - function getCastDisapprovalBySigTypedDataHash(CastDisapprovalBySig memory castDisapproval) - internal - view - returns (bytes32) - { - return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getCastDisapprovalBySigHash(castDisapproval))); - } - /// @notice Returns the hash of ActionInfo. function getActionInfoHash(ActionInfo memory actionInfo) internal pure returns (bytes32) { return keccak256(