From e34c76ce875eaf9d552fa73cc7f4f818adf4122b Mon Sep 17 00:00:00 2001 From: PacificYield <173040337+PacificYield@users.noreply.github.com> Date: Tue, 12 Nov 2024 11:55:43 +0100 Subject: [PATCH 1/5] feat: Add EIP712 and SignatureChecker --- contracts/governance/Comp.sol | 69 +++++++++++++++++--------- contracts/test/governance/TestComp.sol | 7 ++- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/contracts/governance/Comp.sol b/contracts/governance/Comp.sol index 49499f4..0f19439 100644 --- a/contracts/governance/Comp.sol +++ b/contracts/governance/Comp.sol @@ -3,12 +3,14 @@ pragma solidity ^0.8.24; import "fhevm/lib/TFHE.sol"; import { Ownable2Step, Ownable } from "@openzeppelin/contracts/access/Ownable2Step.sol"; +import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; +import { SignatureChecker } from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; import { EncryptedERC20 } from "../token/ERC20/EncryptedERC20.sol"; import { IComp } from "./IComp.sol"; /** * @title Comp - * @notice This contract inherits EncryptedERC20 and Ownable2Step. + * @notice This contract inherits EncryptedERC20, EIP712, and Ownable2Step. * This is based on the Comp.sol contract written by Compound Labs. * see: compound-finance/compound-protocol/blob/master/contracts/Governance/Comp.sol * It is a governance token used to delegate votes, which can be used by contracts such as @@ -17,7 +19,7 @@ import { IComp } from "./IComp.sol"; * with an account's balance. * @dev The delegation of votes leaks information about the account's encrypted balance to the `delegatee`. */ -abstract contract Comp is IComp, EncryptedERC20, Ownable2Step { +abstract contract Comp is IComp, EncryptedERC20, EIP712, Ownable2Step { /// @notice Returned if the `blockNumber` is higher or equal to the (current) `block.number`. /// @dev It is returned in requests to access votes. error BlockNumberEqualOrHigherThanCurrentBlock(); @@ -25,6 +27,16 @@ abstract contract Comp is IComp, EncryptedERC20, Ownable2Step { /// @notice Returned if the `msg.sender` is not the `governor` contract. error GovernorInvalid(); + /// @notice Returned if the signature has expired. + error SignatureExpired(); + + /// @notice Returned if the signature's nonce is invalid. + error SignatureNonceInvalid(); + + /// @notice Returned if the signature's verification has failed. + /// @dev See {SignatureChecker} for potential reasons. + error SignatureVerificationFail(); + /// @notice Emitted when an `account` (i.e. `delegator`) changes its delegate. event DelegateChanged(address indexed delegator, address indexed fromDelegate, address indexed toDelegate); @@ -46,10 +58,6 @@ abstract contract Comp is IComp, EncryptedERC20, Ownable2Step { euint64 votes; } - /// @notice The EIP-712 typehash for the contract's domain. - bytes32 public constant DOMAIN_TYPEHASH = - keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); - /// @notice The EIP-712 typehash for the `Delegation` struct. bytes32 public constant DELEGATION_TYPEHASH = keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)"); @@ -77,10 +85,18 @@ abstract contract Comp is IComp, EncryptedERC20, Ownable2Step { euint64 private _EUINT64_ZERO; /** - * @param owner Owner address + * @param owner_ Owner address. + * @param name_ Token name. + * @param symbol_ Token symbol. + * @param version_ Version (e.g. "0.1", "1.0"). */ - constructor(address owner) EncryptedERC20("Compound", "COMP") Ownable(owner) { - _unsafeMint(owner, TFHE.asEuint64(10000000e6)); /// 10 million Comp + constructor( + address owner_, + string memory name_, + string memory symbol_, + string memory version_ + ) EncryptedERC20(name_, symbol_) EIP712(name_, version_) Ownable(owner_) { + _unsafeMint(owner_, TFHE.asEuint64(10000000e6)); /// 10 million Comp _totalSupply = 10000000e6; /// @dev Define the constant in the storage. @@ -98,32 +114,37 @@ abstract contract Comp is IComp, EncryptedERC20, Ownable2Step { /** * @notice Delegate votes from signatory to `delegatee`. + * @param delegator The account that delegates its votes. It must be the signer. * @param delegatee The address to delegate votes to. * @param nonce The contract state required to match the signature. * @param expiry The time at which to expire the signature. - * @param v The recovery byte of the signature. - * @param r Half of the ECDSA signature pair. - * @param s Half of the ECDSA signature pair. + * @param signature The signature. + * @dev Signature can be either 64-byte or 65-byte long if it is from an EOA. + * Else, it must adhere to ERC1271. See {https://eips.ethereum.org/EIPS/eip-1271} */ function delegateBySig( + address delegator, address delegatee, uint256 nonce, uint256 expiry, - uint8 v, - bytes32 r, - bytes32 s + bytes memory signature ) public virtual { - bytes32 domainSeparator = keccak256( - abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name())), block.chainid, address(this)) - ); bytes32 structHash = keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry)); - bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); - address signatory = ecrecover(digest, v, r, s); + bytes32 digest = keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), structHash)); + + if (!SignatureChecker.isValidSignatureNow(delegator, digest, signature)) { + revert SignatureVerificationFail(); + } + + if (nonce != nonces[delegator]++) { + revert SignatureNonceInvalid(); + } + + if (block.timestamp > expiry) { + revert SignatureExpired(); + } - require(signatory != address(0), "Comp::delegateBySig: invalid signature"); - require(nonce == nonces[signatory]++, "Comp::delegateBySig: invalid nonce"); - require(block.timestamp <= expiry, "Comp::delegateBySig: signature expired"); - return _delegate(signatory, delegatee); + return _delegate(delegator, delegatee); } /** diff --git a/contracts/test/governance/TestComp.sol b/contracts/test/governance/TestComp.sol index a4c4f11..1b5290a 100644 --- a/contracts/test/governance/TestComp.sol +++ b/contracts/test/governance/TestComp.sol @@ -5,7 +5,12 @@ import { Comp } from "../../governance/Comp.sol"; import { DefaultFHEVMConfig } from "../DefaultFHEVMConfig.sol"; contract TestComp is DefaultFHEVMConfig, Comp { - constructor(address owner_) Comp(owner_) { + constructor( + address owner_, + string memory name_, + string memory symbol_, + string memory version_ + ) Comp(owner_, name_, symbol_, version_) { // } } From 01a8bef541425ee3b043e899ab6bab0452a432b8 Mon Sep 17 00:00:00 2001 From: PacificYield <173040337+PacificYield@users.noreply.github.com> Date: Tue, 12 Nov 2024 11:55:51 +0100 Subject: [PATCH 2/5] test: Adjustments --- test/governance/Comp.fixture.ts | 4 +++- test/governance/Comp.test.ts | 38 ++++++++++++++++++++------------ test/governance/DelegateBySig.ts | 26 ++++++++++++++++------ 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/test/governance/Comp.fixture.ts b/test/governance/Comp.fixture.ts index ef6d4c7..62b0af8 100644 --- a/test/governance/Comp.fixture.ts +++ b/test/governance/Comp.fixture.ts @@ -7,7 +7,9 @@ import { FhevmInstances } from "../types"; export async function deployCompFixture(signers: Signers): Promise { const contractFactory = await ethers.getContractFactory("TestComp"); - const contract = await contractFactory.connect(signers.alice).deploy(signers.alice.address); + const contract = await contractFactory + .connect(signers.alice) + .deploy(signers.alice.address, "CompoundZama", "COMP", "1.0"); await contract.waitForDeployment(); return contract; } diff --git a/test/governance/Comp.test.ts b/test/governance/Comp.test.ts index 9f046a0..85b07ae 100644 --- a/test/governance/Comp.test.ts +++ b/test/governance/Comp.test.ts @@ -67,14 +67,17 @@ describe("Comp", function () { }); it("can delegate votes via delegateBySig if signature is valid", async function () { + const delegator = this.signers.alice; const delegatee = this.signers.bob; const nonce = 0; let latestBlockNumber = await ethers.provider.getBlockNumber(); const block = await ethers.provider.getBlock(latestBlockNumber); const expiry = block!.timestamp + 100; - const [v, r, s] = await delegateBySig(this.signers.alice, delegatee.address, this.comp, nonce, expiry); + const signature = await delegateBySig(delegator, delegatee.address, this.comp, nonce, expiry, false); - const tx = await this.comp.connect(this.signers.alice).delegateBySig(delegatee, nonce, expiry, v, r, s); + const tx = await this.comp + .connect(this.signers.alice) + .delegateBySig(delegator, delegatee, nonce, expiry, signature); await tx.wait(); latestBlockNumber = await ethers.provider.getBlockNumber(); @@ -125,49 +128,56 @@ describe("Comp", function () { }); it("cannot delegate votes if nonce is invalid", async function () { + const delegator = this.signers.alice; const delegatee = this.signers.bob; const nonce = 0; let latestBlockNumber = await ethers.provider.getBlockNumber(); const block = await ethers.provider.getBlock(latestBlockNumber); const expiry = block!.timestamp + 100; - const [v, r, s] = await delegateBySig(this.signers.alice, delegatee.address, this.comp, nonce, expiry); + const signature = await delegateBySig(delegator, delegatee.address, this.comp, nonce, expiry); - const tx = await this.comp.connect(this.signers.alice).delegateBySig(delegatee, nonce, expiry, v, r, s); + const tx = await this.comp + .connect(this.signers.alice) + .delegateBySig(delegator, delegatee, nonce, expiry, signature); await tx.wait(); // Cannot reuse same nonce when delegating by sig - await expect(this.comp.delegateBySig(delegatee, nonce, expiry, v, r, s)).to.be.revertedWith( - "Comp::delegateBySig: invalid nonce", + await expect(this.comp.delegateBySig(delegator, delegatee, nonce, expiry, signature)).to.be.revertedWithCustomError( + this.comp, + "SignatureNonceInvalid", ); }); it("cannot delegate votes if signer is invalid", async function () { + const delegator = this.signers.alice; const delegatee = this.signers.bob; const nonce = 0; let latestBlockNumber = await ethers.provider.getBlockNumber(); const block = await ethers.provider.getBlock(latestBlockNumber); const expiry = block!.timestamp + 100; - const [v, r, s] = await delegateBySig(this.signers.alice, delegatee.address, this.comp, nonce, expiry); - // Cannot use invalid signature when delegating by sig - await expect(this.comp.delegateBySig(delegatee, nonce, expiry, 30, r, s)).to.be.revertedWith( - "Comp::delegateBySig: invalid signature", + // Signer is not the delegator + const signature = await delegateBySig(this.signers.carol, delegatee.address, this.comp, nonce, expiry); + await expect(this.comp.delegateBySig(delegator, delegatee, nonce, expiry, signature)).to.be.revertedWithCustomError( + this.comp, + "SignatureVerificationFail", ); }); it("cannot delegate votes if signature has expired", async function () { + const delegator = this.signers.alice; const delegatee = this.signers.bob; const nonce = 0; let latestBlockNumber = await ethers.provider.getBlockNumber(); const block = await ethers.provider.getBlock(latestBlockNumber); const expiry = block!.timestamp + 100; - const [v, r, s] = await delegateBySig(this.signers.alice, delegatee.address, this.comp, nonce, expiry); + const signature = await delegateBySig(delegator, delegatee.address, this.comp, nonce, expiry); ethers.provider.send("evm_increaseTime", ["0xffff"]); - await expect(this.comp.connect(delegatee).delegateBySig(delegatee, nonce, expiry, v, r, s)).to.be.revertedWith( - "Comp::delegateBySig: signature expired", - ); + await expect( + this.comp.connect(delegatee).delegateBySig(delegator, delegatee, nonce, expiry, signature), + ).to.be.revertedWithCustomError(this.comp, "SignatureExpired"); }); it("cannot request votes if blocktime is equal to current blocktime", async function () { diff --git a/test/governance/DelegateBySig.ts b/test/governance/DelegateBySig.ts index bc0fe7c..08f6cff 100644 --- a/test/governance/DelegateBySig.ts +++ b/test/governance/DelegateBySig.ts @@ -4,26 +4,42 @@ import { Address } from "hardhat-deploy/types"; import type { Comp } from "../../types"; +/** + * + * @param _signer Signer from ethers. + * @param _delegatee Delegatee address. + * @param _comp Comp token. + * @param _nonce Nonce to sign. + * @param _expiry Expiry timestamp. + * @param _is64Bytes Whether the signature must be 64 bytes. If false, it has 65 bytes. + * Default is false. + * @dev See: https://eips.ethereum.org/EIPS/eip-2098 + * @returns The signature. + */ export const delegateBySig = async ( _signer: HardhatEthersSigner, _delegatee: Address, _comp: Comp, _nonce: number, _expiry: number, -): Promise<[BigInt, string, string]> => { + _is64Bytes: boolean = false, +): Promise => { const compAddress_ = await _comp.getAddress(); const delegatee_ = _delegatee; const nonce_ = _nonce; const expiry_ = _expiry; - const network = await ethers.provider.getNetwork(); const chainId = network.chainId; + const domain = { name: await _comp.name(), + version: "1.0", chainId: chainId, verifyingContract: compAddress_, }; + // Delegation(address delegatee,uint256 nonce,uint256 expiry) + const types = { Delegation: [ { @@ -48,9 +64,5 @@ export const delegateBySig = async ( }; const signature = await _signer.signTypedData(domain, types, message); - const sigRSV = ethers.Signature.from(signature); - const v = 27 + sigRSV.yParity; - const r = sigRSV.r; - const s = sigRSV.s; - return [BigInt(v), r, s]; + return signature; }; From ff6782884898682c44eb797dd996fd2a11150313 Mon Sep 17 00:00:00 2001 From: PacificYield <173040337+PacificYield@users.noreply.github.com> Date: Tue, 12 Nov 2024 12:05:48 +0100 Subject: [PATCH 3/5] refactor: Add totalSupply in constructor --- contracts/governance/Comp.sol | 16 +++++++++------- contracts/test/governance/TestComp.sol | 5 +++-- test/governance/Comp.fixture.ts | 3 ++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/contracts/governance/Comp.sol b/contracts/governance/Comp.sol index 0f19439..5c71b8d 100644 --- a/contracts/governance/Comp.sol +++ b/contracts/governance/Comp.sol @@ -85,19 +85,21 @@ abstract contract Comp is IComp, EncryptedERC20, EIP712, Ownable2Step { euint64 private _EUINT64_ZERO; /** - * @param owner_ Owner address. - * @param name_ Token name. - * @param symbol_ Token symbol. - * @param version_ Version (e.g. "0.1", "1.0"). + * @param owner_ Owner address. + * @param name_ Token name. + * @param symbol_ Token symbol. + * @param version_ Version (e.g. "0.1", "1.0"). + * @param totalSupply_ Total supply to mint. */ constructor( address owner_, string memory name_, string memory symbol_, - string memory version_ + string memory version_, + uint64 totalSupply_ ) EncryptedERC20(name_, symbol_) EIP712(name_, version_) Ownable(owner_) { - _unsafeMint(owner_, TFHE.asEuint64(10000000e6)); /// 10 million Comp - _totalSupply = 10000000e6; + _unsafeMint(owner_, TFHE.asEuint64(totalSupply_)); + _totalSupply = totalSupply_; /// @dev Define the constant in the storage. _EUINT64_ZERO = TFHE.asEuint64(0); diff --git a/contracts/test/governance/TestComp.sol b/contracts/test/governance/TestComp.sol index 1b5290a..200faf2 100644 --- a/contracts/test/governance/TestComp.sol +++ b/contracts/test/governance/TestComp.sol @@ -9,8 +9,9 @@ contract TestComp is DefaultFHEVMConfig, Comp { address owner_, string memory name_, string memory symbol_, - string memory version_ - ) Comp(owner_, name_, symbol_, version_) { + string memory version_, + uint64 totalSupply_ + ) Comp(owner_, name_, symbol_, version_, totalSupply_) { // } } diff --git a/test/governance/Comp.fixture.ts b/test/governance/Comp.fixture.ts index 62b0af8..889bb98 100644 --- a/test/governance/Comp.fixture.ts +++ b/test/governance/Comp.fixture.ts @@ -1,3 +1,4 @@ +import { parseUnits } from "ethers"; import { ethers } from "hardhat"; import type { TestComp } from "../../types"; @@ -9,7 +10,7 @@ export async function deployCompFixture(signers: Signers): Promise { const contractFactory = await ethers.getContractFactory("TestComp"); const contract = await contractFactory .connect(signers.alice) - .deploy(signers.alice.address, "CompoundZama", "COMP", "1.0"); + .deploy(signers.alice.address, "CompoundZama", "COMP", "1.0", parseUnits("10000000", 6)); await contract.waitForDeployment(); return contract; } From 54eef6573bee39cfc3538929a24546b796316076 Mon Sep 17 00:00:00 2001 From: PacificYield <173040337+PacificYield@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:33:06 +0100 Subject: [PATCH 4/5] fix: Unused param --- test/governance/DelegateBySig.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/governance/DelegateBySig.ts b/test/governance/DelegateBySig.ts index 08f6cff..5ec02b5 100644 --- a/test/governance/DelegateBySig.ts +++ b/test/governance/DelegateBySig.ts @@ -11,9 +11,6 @@ import type { Comp } from "../../types"; * @param _comp Comp token. * @param _nonce Nonce to sign. * @param _expiry Expiry timestamp. - * @param _is64Bytes Whether the signature must be 64 bytes. If false, it has 65 bytes. - * Default is false. - * @dev See: https://eips.ethereum.org/EIPS/eip-2098 * @returns The signature. */ export const delegateBySig = async ( @@ -22,7 +19,6 @@ export const delegateBySig = async ( _comp: Comp, _nonce: number, _expiry: number, - _is64Bytes: boolean = false, ): Promise => { const compAddress_ = await _comp.getAddress(); const delegatee_ = _delegatee; From ae95ed251a213ce86cd8dbce9945446853de3786 Mon Sep 17 00:00:00 2001 From: PacificYield <173040337+PacificYield@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:44:37 +0100 Subject: [PATCH 5/5] feat: Add incrementNonce --- contracts/governance/Comp.sol | 14 ++++++++++++++ test/governance/Comp.test.ts | 23 +++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/contracts/governance/Comp.sol b/contracts/governance/Comp.sol index 5c71b8d..0573045 100644 --- a/contracts/governance/Comp.sol +++ b/contracts/governance/Comp.sol @@ -47,6 +47,9 @@ abstract contract Comp is IComp, EncryptedERC20, EIP712, Ownable2Step { /// @dev WARNING: it can be set to a malicious contract, which could reencrypt all user votes. event NewGovernor(address indexed governor); + /// @notice Emitted when the account cancels a signature. + event NonceIncremented(address account, uint256 newNonce); + /// @notice A checkpoint for marking number of votes from a given block. /// @param fromBlock Block from where the checkpoint applies. /// @param votes Total number of votes for the account power. @@ -149,6 +152,17 @@ abstract contract Comp is IComp, EncryptedERC20, EIP712, Ownable2Step { return _delegate(delegator, delegatee); } + /** + * @notice Increment the nonce. + * @dev This function enables the sender to cancel a signature. + */ + function incrementNonce() public virtual { + uint256 currentNonce = nonces[msg.sender]; + nonces[msg.sender] = ++currentNonce; + + emit NonceIncremented(msg.sender, currentNonce); + } + /** * @notice See {IComp-getPriorVotesForGovernor}. */ diff --git a/test/governance/Comp.test.ts b/test/governance/Comp.test.ts index 85b07ae..884af62 100644 --- a/test/governance/Comp.test.ts +++ b/test/governance/Comp.test.ts @@ -148,6 +148,25 @@ describe("Comp", function () { ); }); + it("cannot delegate votes if nonce is invalid due to the delegator incrementing her nonce", async function () { + const delegator = this.signers.alice; + const delegatee = this.signers.bob; + const nonce = 0; + let latestBlockNumber = await ethers.provider.getBlockNumber(); + const block = await ethers.provider.getBlock(latestBlockNumber); + const expiry = block!.timestamp + 100; + const signature = await delegateBySig(delegator, delegatee.address, this.comp, nonce, expiry); + + const tx = await this.comp.connect(delegator).incrementNonce(); + await tx.wait(); + + // Cannot reuse same nonce when delegating by sig + await expect(this.comp.delegateBySig(delegator, delegatee, nonce, expiry, signature)).to.be.revertedWithCustomError( + this.comp, + "SignatureNonceInvalid", + ); + }); + it("cannot delegate votes if signer is invalid", async function () { const delegator = this.signers.alice; const delegatee = this.signers.bob; @@ -353,7 +372,7 @@ describe("Comp", function () { await reencryptPriorVotes( this.signers, this.instances, - "alice", + "carol", blockNumbers[i], this.comp, this.compAddress, @@ -364,7 +383,7 @@ describe("Comp", function () { await reencryptPriorVotes( this.signers, this.instances, - "carol", + "alice", blockNumbers[i], this.comp, this.compAddress,