diff --git a/src/lib/QuorumCheckpoints.sol b/src/lib/QuorumCheckpoints.sol index 1ca3b2f..3e3ab8b 100644 --- a/src/lib/QuorumCheckpoints.sol +++ b/src/lib/QuorumCheckpoints.sol @@ -8,12 +8,12 @@ import {LlamaUtils} from "src/lib/LlamaUtils.sol"; * @dev This library defines the `History` struct, for checkpointing values as they change at different points in * time, and later looking up past values by block timestamp. * - * To create a history of checkpoints define a variable type `PolicyholderCheckpoints.History` in your contract, and store a new + * To create a history of checkpoints define a variable type `QuorumCheckpoints.History` in your contract, and store a new * checkpoint for the current transaction timestamp using the {push} function. * * @dev This was created by modifying then running the OpenZeppelin `Checkpoints.js` script, which generated a version * of this library that uses a 64 bit `timestamp` and 96 bit `quantity` field in the `Checkpoint` struct. The struct - * was then modified to add two uint16 quorum fields. For simplicity, safe cast and math methods were inlined from + * was then modified to work with the below `Checkpoint` struct. For simplicity, safe cast and math methods were inlined from * the OpenZeppelin versions at the same commit. We disable forge-fmt for this file to simplify diffing against the * original OpenZeppelin version: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/d00acef4059807535af0bd0dd0ddf619747a044b/contracts/utils/Checkpoints.sol */ @@ -35,7 +35,7 @@ library QuorumCheckpoints { * timestamp of checkpoints. */ function getAtProbablyRecentTimestamp(History storage self, uint256 timestamp) internal view returns (uint16, uint16) { - require(timestamp < block.timestamp, "PolicyholderCheckpoints: timestamp is not in the past"); + require(timestamp < block.timestamp, "QuorumCheckpoints: timestamp is not in the past"); uint224 _timestamp = LlamaUtils.toUint224(timestamp); uint256 len = self._checkpoints.length; @@ -131,7 +131,7 @@ library QuorumCheckpoints { Checkpoint memory last = _unsafeAccess(self, pos - 1); // Checkpoints timestamps must be increasing. - require(last.timestamp <= timestamp, "Role Checkpoint: invalid timestamp"); + require(last.timestamp <= timestamp, "Quorum Checkpoint: invalid timestamp"); // Update or push new checkpoint if (last.timestamp == timestamp) { diff --git a/src/token-voting/LlamaTokenActionCreator.sol b/src/token-voting/LlamaTokenActionCreator.sol index 48faf5e..8ce6d9d 100644 --- a/src/token-voting/LlamaTokenActionCreator.sol +++ b/src/token-voting/LlamaTokenActionCreator.sol @@ -155,8 +155,8 @@ abstract contract LlamaTokenActionCreator is Initializable { return _createAction(msg.sender, strategy, target, value, data, description); } - /// @notice Creates an action via an off-chain signature. The creator needs to hold a policy with the permission ID - /// of the provided `(target, selector, strategy)`. + /// @notice Creates an action via an off-chain signature. The creator needs to have sufficient token balance that is + /// greater than or equal to the creation threshold. /// @dev Use `""` for `description` if there is no description. /// @param tokenHolder The tokenHolder that signed the message. /// @param strategy The strategy contract that will determine how the action is executed. diff --git a/src/token-voting/LlamaTokenCaster.sol b/src/token-voting/LlamaTokenCaster.sol index b960f09..54fa273 100644 --- a/src/token-voting/LlamaTokenCaster.sol +++ b/src/token-voting/LlamaTokenCaster.sol @@ -139,12 +139,12 @@ abstract contract LlamaTokenCaster is Initializable { /// @notice EIP-712 castVote typehash. bytes32 internal constant CAST_VOTE_TYPEHASH = keccak256( - "CastVote(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)" + "CastVote(address tokenHolder,ActionInfo actionInfo,uint8 support,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); /// @notice EIP-712 castVeto typehash. bytes32 internal constant CAST_VETO_TYPEHASH = keccak256( - "CastVeto(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)" + "CastVeto(address tokenHolder,ActionInfo actionInfo,uint8 support,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); /// @dev EIP-712 actionInfo typehash. @@ -161,11 +161,12 @@ abstract contract LlamaTokenCaster is Initializable { /// @dev Equivalent to 2/3, but in basis points. uint256 internal constant TWO_THIRDS_IN_BPS = 6667; + /// @dev The quorum checkpoints for this token voting module. + QuorumCheckpoints.History internal quorumCheckpoints; + /// @notice The core contract for this Llama instance. ILlamaCore public llamaCore; - QuorumCheckpoints.History internal quorumCheckpoints; - /// @notice The contract that manages the timepoints for this token voting module. ILlamaTokenClockAdapter public clockAdapter; @@ -218,22 +219,37 @@ abstract contract LlamaTokenCaster is Initializable { /// @param support The tokenholder's support of the approval of the action. /// 0 = Against /// 1 = For - /// 2 = Abstain, but this is not currently supported. + /// 2 = Abstain /// @param reason The reason given for the approval by the tokenholder. + /// @return The weight of the cast. function castVote(ActionInfo calldata actionInfo, uint8 support, string calldata reason) external returns (uint96) { return _castVote(msg.sender, actionInfo, support, reason); } + /// @notice How tokenholders add their support of the approval of an action with a reason via an off-chain + /// signature. + /// @dev Use `""` for `reason` if there is no reason. + /// @param caster The tokenholder that signed the message. + /// @param actionInfo Data required to create an action. + /// @param support The tokenholder's support of the approval of the action. + /// 0 = Against + /// 1 = For + /// 2 = Abstain + /// @param reason The reason given for the approval by the tokenholder. + /// @param v ECDSA signature component: Parity of the `y` coordinate of point `R` + /// @param r ECDSA signature component: x-coordinate of `R` + /// @param s ECDSA signature component: `s` value of the signature + /// @return The weight of the cast. function castVoteBySig( address caster, - uint8 support, ActionInfo calldata actionInfo, + uint8 support, string calldata reason, uint8 v, bytes32 r, bytes32 s ) external returns (uint96) { - bytes32 digest = _getCastVoteTypedDataHash(caster, support, actionInfo, reason); + bytes32 digest = _getCastVoteTypedDataHash(caster, actionInfo, support, reason); address signer = ecrecover(digest, v, r, s); if (signer == address(0) || signer != caster) revert InvalidSignature(); return _castVote(signer, actionInfo, support, reason); @@ -245,22 +261,37 @@ abstract contract LlamaTokenCaster is Initializable { /// @param support The tokenholder's support of the approval of the action. /// 0 = Against /// 1 = For - /// 2 = Abstain, but this is not currently supported. + /// 2 = Abstain /// @param reason The reason given for the approval by the tokenholder. + /// @return The weight of the cast. function castVeto(ActionInfo calldata actionInfo, uint8 support, string calldata reason) external returns (uint96) { return _castVeto(msg.sender, actionInfo, support, reason); } + /// @notice How tokenholders add their support of the disapproval of an action with a reason via an off-chain + /// signature. + /// @dev Use `""` for `reason` if there is no reason. + /// @param caster The tokenholder that signed the message. + /// @param actionInfo Data required to create an action. + /// @param support The tokenholder's support of the approval of the action. + /// 0 = Against + /// 1 = For + /// 2 = Abstain + /// @param reason The reason given for the approval by the tokenholder. + /// @param v ECDSA signature component: Parity of the `y` coordinate of point `R` + /// @param r ECDSA signature component: x-coordinate of `R` + /// @param s ECDSA signature component: `s` value of the signature + /// @return The weight of the cast. function castVetoBySig( address caster, - uint8 support, ActionInfo calldata actionInfo, + uint8 support, string calldata reason, uint8 v, bytes32 r, bytes32 s ) external returns (uint96) { - bytes32 digest = _getCastVetoTypedDataHash(caster, support, actionInfo, reason); + bytes32 digest = _getCastVetoTypedDataHash(caster, actionInfo, support, reason); address signer = ecrecover(digest, v, r, s); if (signer == address(0) || signer != caster) revert InvalidSignature(); return _castVeto(signer, actionInfo, support, reason); @@ -500,16 +531,16 @@ abstract contract LlamaTokenCaster is Initializable { /// recover the signer. function _getCastVoteTypedDataHash( address tokenholder, - uint8 support, ActionInfo calldata actionInfo, + uint8 support, string calldata reason ) internal returns (bytes32) { bytes32 castVoteHash = keccak256( abi.encode( CAST_VOTE_TYPEHASH, tokenholder, - support, _getActionInfoHash(actionInfo), + support, keccak256(bytes(reason)), _useNonce(tokenholder, msg.sig) ) @@ -522,16 +553,16 @@ abstract contract LlamaTokenCaster is Initializable { /// recover the signer. function _getCastVetoTypedDataHash( address tokenholder, - uint8 support, ActionInfo calldata actionInfo, + uint8 support, string calldata reason ) internal returns (bytes32) { bytes32 castVetoHash = keccak256( abi.encode( CAST_VETO_TYPEHASH, tokenholder, - support, _getActionInfoHash(actionInfo), + support, keccak256(bytes(reason)), _useNonce(tokenholder, msg.sig) ) diff --git a/test/token-voting/LlamaERC20TokenCaster.t.sol b/test/token-voting/LlamaERC20TokenCaster.t.sol index d35ae8a..2eea5f4 100644 --- a/test/token-voting/LlamaERC20TokenCaster.t.sol +++ b/test/token-voting/LlamaERC20TokenCaster.t.sol @@ -250,7 +250,7 @@ contract CastVoteBySig is LlamaERC20TokenCasterTest { } function castVoteBySig(ActionInfo memory _actionInfo, uint8 support, uint8 v, bytes32 r, bytes32 s) internal { - llamaERC20TokenCaster.castVoteBySig(tokenHolder1, support, _actionInfo, "", v, r, s); + llamaERC20TokenCaster.castVoteBySig(tokenHolder1, _actionInfo, support, "", v, r, s); } function test_CastsVoteBySig() public { @@ -438,7 +438,7 @@ contract CastVetoBySig is LlamaERC20TokenCasterTest { } function castVetoBySig(ActionInfo memory _actionInfo, uint8 v, bytes32 r, bytes32 s) internal { - llamaERC20TokenCaster.castVetoBySig(tokenHolder1, uint8(VoteType.For), _actionInfo, "", v, r, s); + llamaERC20TokenCaster.castVetoBySig(tokenHolder1, _actionInfo, uint8(VoteType.For), "", v, r, s); } function test_CastsVetoBySig() public { diff --git a/test/token-voting/LlamaERC721TokenCaster.t.sol b/test/token-voting/LlamaERC721TokenCaster.t.sol index 1fcb661..071105d 100644 --- a/test/token-voting/LlamaERC721TokenCaster.t.sol +++ b/test/token-voting/LlamaERC721TokenCaster.t.sol @@ -252,7 +252,7 @@ contract CastApprovalBySig is LlamaERC721TokenCasterTest { } function castVoteBySig(ActionInfo memory _actionInfo, uint8 support, uint8 v, bytes32 r, bytes32 s) internal { - llamaERC721TokenCaster.castVoteBySig(tokenHolder1, support, _actionInfo, "", v, r, s); + llamaERC721TokenCaster.castVoteBySig(tokenHolder1, _actionInfo, support, "", v, r, s); } function test_CastsVoteBySig() public { @@ -440,7 +440,7 @@ contract CastVetoBySig is LlamaERC721TokenCasterTest { } function castVetoBySig(ActionInfo memory _actionInfo, uint8 v, bytes32 r, bytes32 s) internal { - llamaERC721TokenCaster.castVetoBySig(tokenHolder1, uint8(VoteType.For), _actionInfo, "", v, r, s); + llamaERC721TokenCaster.castVetoBySig(tokenHolder1, _actionInfo, uint8(VoteType.For), "", v, r, s); } function test_CastsVetoBySig() public { diff --git a/test/utils/LlamaCoreSigUtils.sol b/test/utils/LlamaCoreSigUtils.sol index 1a50bcd..8385b89 100644 --- a/test/utils/LlamaCoreSigUtils.sol +++ b/test/utils/LlamaCoreSigUtils.sol @@ -29,16 +29,16 @@ contract LlamaCoreSigUtils { struct CastVote { address tokenHolder; - uint8 support; ActionInfo actionInfo; + uint8 support; string reason; uint256 nonce; } struct CastVeto { address tokenHolder; - uint8 support; ActionInfo actionInfo; + uint8 support; string reason; uint256 nonce; } @@ -59,12 +59,12 @@ contract LlamaCoreSigUtils { /// @notice EIP-712 castVote typehash. bytes32 internal constant CAST_VOTE_TYPEHASH = keccak256( - "CastVote(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)" + "CastVote(address tokenHolder,ActionInfo actionInfo,uint8 support,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); /// @notice EIP-712 castVeto typehash. bytes32 internal constant CAST_VETO_TYPEHASH = keccak256( - "CastVeto(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)" + "CastVeto(address tokenHolder,ActionInfo actionInfo,uint8 support,string reason,uint256 nonce)ActionInfo(uint256 id,address creator,uint8 creatorRole,address strategy,address target,uint256 value,bytes data)" ); /// @notice EIP-712 actionInfo typehash. @@ -130,8 +130,8 @@ contract LlamaCoreSigUtils { abi.encode( CAST_VOTE_TYPEHASH, castVote.tokenHolder, - castVote.support, getActionInfoHash(castVote.actionInfo), + castVote.support, keccak256(bytes(castVote.reason)), castVote.nonce ) @@ -150,8 +150,8 @@ contract LlamaCoreSigUtils { abi.encode( CAST_VETO_TYPEHASH, castVeto.tokenHolder, - castVeto.support, getActionInfoHash(castVeto.actionInfo), + castVeto.support, keccak256(bytes(castVeto.reason)), castVeto.nonce )