Prevent partial vote replays (#51)

* Repro the issue

* Fix the bug

* Get code to build

* Expect nonce as the last half word in the params

* Get tests passing

* Handle the nominal signature case

* scopelint fmt

* Add natspec

* Modify based on PR feedback

* Consolidate tests, fuzz over nonces, and take more PR feedback

* Use hex byte representation in assembly for consistency

src/GovernorCountingFractional.sol

@@ -3,6 +3,7 @@
 
 pragma solidity ^0.8.0;
 
+import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import {Governor} from "@openzeppelin/contracts/governance/Governor.sol";
 import {GovernorCompatibilityBravo} from "@openzeppelin/contracts/governance/compatibility/GovernorCompatibilityBravo.sol";
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
@@ -38,6 +39,14 @@ abstract contract GovernorCountingFractional is Governor {
      */
     mapping(uint256 => mapping(address => uint128)) private _proposalVotersWeightCast;
 
+    /**
+     * @dev Mapping from voter address to signature-based vote nonce. The
+     * voter's nonce increments each time a signature-based vote is cast with
+     * fractional voting params and must be included in the `params` as the last
+     * 16 bytes when signing for a fractional vote.
+     */
+    mapping(address => uint128) public fractionalVoteNonce;
+
     /**
      * @dev See {IGovernor-COUNTING_MODE}.
      */
@@ -185,7 +194,7 @@ abstract contract GovernorCountingFractional is Governor {
      * The result of these three calls would be that the account casts 5 votes
      * AGAINST, 2 votes FOR, and 3 votes ABSTAIN on the proposal. Though
      * partial, votes are still final once cast and cannot be changed or
-     * overidden. Subsequent partial votes simply increment existing totals.
+     * overridden. Subsequent partial votes simply increment existing totals.
      *
      * Note that if partial votes are cast, all remaining weight must be cast
      * with _countVoteFractional: _countVoteNominal will revert.
@@ -219,7 +228,7 @@ abstract contract GovernorCountingFractional is Governor {
         _proposalVotes[proposalId] = _proposalVote;
     }
 
-    uint256 constant internal _VOTEMASK = 0xffffffffffffffffffffffffffffffff; // 128 bits of 0's, 128 bits of 1's
+    uint256 constant internal _MASK_HALF_WORD_RIGHT = 0xffffffffffffffffffffffffffffffff; // 128 bits of 0's, 128 bits of 1's
 
     /**
      * @dev Decodes three packed uint128's. Uses assembly because of a Solidity
@@ -237,8 +246,101 @@ abstract contract GovernorCountingFractional is Governor {
     {
         assembly {
             againstVotes := shr(128, mload(add(voteData, 0x20)))
-            forVotes := and(_VOTEMASK, mload(add(voteData, 0x20)))
+            forVotes := and(_MASK_HALF_WORD_RIGHT, mload(add(voteData, 0x20)))
             abstainVotes := shr(128, mload(add(voteData, 0x40)))
         }
     }
+
+    /**
+     * @notice Cast a vote with a reason and additional encoded parameters using
+     * the user's cryptographic signature.
+     *
+     * Emits a {VoteCast} or {VoteCastWithParams} event depending on the length
+     * of params.
+     *
+     * @dev If casting a fractional vote via `params`, the voter's current nonce
+     * must be appended to the `params` as the last 16 bytes and included in the
+     * signature. I.e., the params used when constructing the signature would be:
+     *
+     *   abi.encodePacked(againstVotes, forVotes, abstainVotes, nonce)
+     *
+     * See {fractionalVoteNonce} and {_castVote} for more information.
+     */
+    function castVoteWithReasonAndParamsBySig(
+        uint256 proposalId,
+        uint8 support,
+        string calldata reason,
+        bytes memory params,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual override returns (uint256) {
+        // Signature-based fractional voting requires `params` be two full words
+        // in length:
+        //   16 bytes for againstVotes.
+        //   16 bytes for forVotes.
+        //   16 bytes for abstainVotes.
+        //   16 bytes for the signature nonce.
+        // Signature-based nominal voting requires `params` be 0 bytes.
+        require(
+          params.length == 64 || params.length == 0,
+          "GovernorCountingFractional: invalid params for signature-based vote"
+        );
+
+        address voter = ECDSA.recover(
+            _hashTypedDataV4(
+                keccak256(
+                    abi.encode(
+                        EXTENDED_BALLOT_TYPEHASH,
+                        proposalId,
+                        support,
+                        keccak256(bytes(reason)),
+                        keccak256(params)
+                    )
+                )
+            ),
+            v,
+            r,
+            s
+        );
+
+        // If params are zero-length all of the voter's weight will be cast so
+        // we don't have to worry about checking/incrementing a nonce.
+        if (params.length == 64) {
+          // Get the nonce out of the params. It is the last half-word.
+          uint128 nonce;
+          assembly {
+            nonce := and(
+              // Perform bitwise AND operation on the data in the second word of
+              // `params` with a mask of 128 zeros followed by 128 ones, i.e. take
+              // the last 128 bits of `params`.
+              _MASK_HALF_WORD_RIGHT,
+              // Load the data from memory at the returned address.
+              mload(
+                // Skip the first 64 bytes (0x40):
+                //   32 bytes encoding the length of the bytes array.
+                //   32 bytes for the first word in the params
+                // Return the memory address for the last word in params.
+                add(params, 0x40)
+              )
+            )
+          }
+
+          require(
+            fractionalVoteNonce[voter] == nonce,
+            "GovernorCountingFractional: signature has already been used"
+          );
+
+          fractionalVoteNonce[voter]++;
+
+          // Trim params in place to keep only the first 48 bytes (which are
+          // the voting params) and save gas.
+          assembly {
+            mstore(params, 0x30)
+          }
+        }
+
+        return _castVote(proposalId, voter, support, reason, params);
+    }
+
 }

test/FractionalGovernor.sol

@@ -7,6 +7,20 @@ import "@openzeppelin/contracts/governance/extensions/GovernorVotes.sol";
 contract FractionalGovernor is GovernorVotes, GovernorCountingFractional {
   constructor(string memory name_, IVotes token_) Governor(name_) GovernorVotes(token_) {}
 
+  function castVoteWithReasonAndParamsBySig(
+    uint256 proposalId,
+    uint8 support,
+    string calldata reason,
+    bytes memory params,
+    uint8 v,
+    bytes32 r,
+    bytes32 s
+  ) public virtual override(GovernorCountingFractional, Governor) returns (uint256) {
+    return GovernorCountingFractional.castVoteWithReasonAndParamsBySig(
+      proposalId, support, reason, params, v, r, s
+    );
+  }
+
   function quorum(uint256) public pure override returns (uint256) {
     return 10 ether;
   }
@@ -23,6 +37,10 @@ contract FractionalGovernor is GovernorVotes, GovernorCountingFractional {
     return _quorumReached(_proposalId);
   }
 
+  function exposed_setFractionalVoteNonce(address _voter, uint128 _newNonce) public {
+    fractionalVoteNonce[_voter] = _newNonce;
+  }
+
   function cancel(
     address[] memory targets,
     uint256[] memory values,