From 89110056b148c85f5c7f6649f6ed2100789a966b Mon Sep 17 00:00:00 2001 From: neokry Date: Wed, 2 Aug 2023 14:04:33 +0800 Subject: [PATCH 1/3] Add batch delegate by sig ERC1271 --- src/lib/interfaces/IERC1271.sol | 19 ++++++++ src/lib/token/ERC721Votes.sol | 84 ++++++++++++++++++++++++++------- 2 files changed, 85 insertions(+), 18 deletions(-) create mode 100644 src/lib/interfaces/IERC1271.sol diff --git a/src/lib/interfaces/IERC1271.sol b/src/lib/interfaces/IERC1271.sol new file mode 100644 index 0000000..5ec44c7 --- /dev/null +++ b/src/lib/interfaces/IERC1271.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.1 (interfaces/IERC1271.sol) + +pragma solidity ^0.8.0; + +/** + * @dev Interface of the ERC1271 standard signature validation method for + * contracts as defined in https://eips.ethereum.org/EIPS/eip-1271[ERC-1271]. + * + * _Available since v4.1._ + */ +interface IERC1271 { + /** + * @dev Should return whether the signature provided is valid for the provided data + * @param hash Hash of the data to be signed + * @param signature Signature byte array associated with _data + */ + function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4 magicValue); +} diff --git a/src/lib/token/ERC721Votes.sol b/src/lib/token/ERC721Votes.sol index ee89a9e..615a360 100644 --- a/src/lib/token/ERC721Votes.sol +++ b/src/lib/token/ERC721Votes.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.16; import { IERC721Votes } from "../interfaces/IERC721Votes.sol"; +import { IERC1271 } from "../interfaces/IERC1271.sol"; import { ERC721 } from "../token/ERC721.sol"; import { EIP712 } from "../utils/EIP712.sol"; @@ -20,6 +21,9 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { /// @dev The EIP-712 typehash to delegate with a signature bytes32 internal constant DELEGATION_TYPEHASH = keccak256("Delegation(address from,address to,uint256 nonce,uint256 deadline)"); + /// @dev The EIP-712 typehash to batch delegate with a signature + bytes32 internal constant BATCH_DELEGATION_TYPEHASH = keccak256("Delegation(address[] from,address[] to,uint256[] nonce,uint256[] deadline)"); + /// /// /// STORAGE /// /// /// @@ -141,14 +145,7 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { /// @param _v The 129th byte and chain id of the signature /// @param _r The first 64 bytes of the signature /// @param _s Bytes 64-128 of the signature - function delegateBySig( - address _from, - address _to, - uint256 _deadline, - uint8 _v, - bytes32 _r, - bytes32 _s - ) external { + function delegateBySig(address _from, address _to, uint256 _deadline, uint8 _v, bytes32 _r, bytes32 _s) external { // Ensure the signature has not expired if (block.timestamp > _deadline) revert EXPIRED_SIGNATURE(); @@ -173,6 +170,65 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { _delegate(_from, _to); } + function batchDelegateBySigERC1271( + address[] calldata _fromAddresses, + address[] calldata _toAddresses, + uint256[] calldata _deadlines, + bytes memory _signature + ) internal { + uint256 length = _fromAddresses.length; + + // Used to store the digest + bytes32 digest; + + // Cannot realistically overflow + unchecked { + // Store nonces for each from address + uint256[] memory currentNonces = new uint256[](length); + + for (uint256 i = 0; i < length; ++i) { + // Ensure the signature has not expired + if (block.timestamp > _deadlines[i]) revert EXPIRED_SIGNATURE(); + + // Add the addresses current nonce to the list of nonces + currentNonces[i] = nonces[_fromAddresses[i]]++; + } + + // Compute the hash of the domain seperator with the typed delegation data + digest = keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + BATCH_DELEGATION_TYPEHASH, + keccak256(abi.encodePacked(_fromAddresses)), + keccak256(abi.encodePacked(_toAddresses)), + keccak256(abi.encodePacked(currentNonces)), + keccak256(abi.encodePacked(_deadlines)) + ) + ) + ) + ); + + for (uint256 i = 0; i < length; ++i) { + address cachedFromAddress = _fromAddresses[i]; + + // Call the ERC1271 isValidSignature function + (bool success, bytes memory result) = cachedFromAddress.staticcall( + abi.encodeWithSelector(IERC1271.isValidSignature.selector, digest, _signature) + ); + + // Ensure the signature is valid + if (success && result.length >= 32 && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector)) + revert INVALID_SIGNATURE(); + + // Update the delegate + _delegate(cachedFromAddress, _toAddresses[i]); + } + } + } + /// @dev Updates delegate addresses /// @param _from The address delegating votes from /// @param _to The address delegating votes to @@ -196,11 +252,7 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { /// @param _from The address delegating votes from /// @param _to The address delegating votes to /// @param _amount The number of votes delegating - function _moveDelegateVotes( - address _from, - address _to, - uint256 _amount - ) internal { + function _moveDelegateVotes(address _from, address _to, uint256 _amount) internal { unchecked { // If voting weight is being transferred: if (_from != _to && _amount > 0) { @@ -309,11 +361,7 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { /// @param _from The token sender /// @param _to The token recipient /// @param _tokenId The ERC-721 token id - function _afterTokenTransfer( - address _from, - address _to, - uint256 _tokenId - ) internal override { + function _afterTokenTransfer(address _from, address _to, uint256 _tokenId) internal override { // Transfer 1 vote from the sender to the recipient _moveDelegateVotes(delegates(_from), delegates(_to), 1); From acd49fd624c40b10d2b1a1ec348f3ecd53e4825d Mon Sep 17 00:00:00 2001 From: neokry Date: Fri, 4 Aug 2023 15:39:48 +0900 Subject: [PATCH 2/3] Add batch delegate with sig tests --- src/lib/token/ERC721Votes.sol | 51 +++++++- test/Token.t.sol | 200 +++++++++++++++++++++++++++---- test/utils/mocks/MockERC1271.sol | 25 ++++ 3 files changed, 250 insertions(+), 26 deletions(-) create mode 100644 test/utils/mocks/MockERC1271.sol diff --git a/src/lib/token/ERC721Votes.sol b/src/lib/token/ERC721Votes.sol index 615a360..1c34a85 100644 --- a/src/lib/token/ERC721Votes.sol +++ b/src/lib/token/ERC721Votes.sol @@ -57,6 +57,46 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { } } + function getBatchDelegateBySigTypedDataHash( + address[] calldata _fromAddresses, + address[] calldata _toAddresses, + uint256[] calldata _deadlines + ) public view returns (bytes32) { + uint256 length = _fromAddresses.length; + + // Cannot realistically overflow + unchecked { + // Store nonces for each from address + uint256[] memory currentNonces = new uint256[](length); + + for (uint256 i = 0; i < length; ++i) { + // Ensure the signature has not expired + if (block.timestamp > _deadlines[i]) revert EXPIRED_SIGNATURE(); + + // Add the addresses current nonce to the list of nonces + currentNonces[i] = nonces[_fromAddresses[i]]; + } + + // Compute the hash of the domain seperator with the typed delegation data + return + keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + BATCH_DELEGATION_TYPEHASH, + keccak256(abi.encodePacked(_fromAddresses)), + keccak256(abi.encodePacked(_toAddresses)), + keccak256(abi.encodePacked(currentNonces)), + keccak256(abi.encodePacked(_deadlines)) + ) + ) + ) + ); + } + } + /// @notice The number of votes for an account at a past timestamp /// @param _account The account address /// @param _timestamp The past timestamp @@ -175,7 +215,7 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { address[] calldata _toAddresses, uint256[] calldata _deadlines, bytes memory _signature - ) internal { + ) external { uint256 length = _fromAddresses.length; // Used to store the digest @@ -220,11 +260,12 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { ); // Ensure the signature is valid - if (success && result.length >= 32 && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector)) + if (success && result.length >= 32 && abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector)) { + // Update the delegate + _delegate(cachedFromAddress, _toAddresses[i]); + } else { revert INVALID_SIGNATURE(); - - // Update the delegate - _delegate(cachedFromAddress, _toAddresses[i]); + } } } } diff --git a/test/Token.t.sol b/test/Token.t.sol index 2606b24..5da8223 100644 --- a/test/Token.t.sol +++ b/test/Token.t.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.16; import { NounsBuilderTest } from "./utils/NounsBuilderTest.sol"; +import { MockERC1271 } from "./utils/mocks/MockERC1271.sol"; import { IManager, Manager } from "../src/manager/Manager.sol"; import { IToken, Token } from "../src/token/Token.sol"; @@ -11,8 +12,19 @@ import { TokenTypesV2 } from "../src/token/types/TokenTypesV2.sol"; contract TokenTest is NounsBuilderTest, TokenTypesV1 { mapping(address => uint256) public mintedTokens; + address internal delegator; + uint256 internal delegatorPK; + function setUp() public virtual override { super.setUp(); + createDelegator(); + } + + function createDelegator() internal { + delegatorPK = 0xABE; + delegator = vm.addr(delegatorPK); + + vm.deal(delegator, 100 ether); } function test_MockTokenInit() public { @@ -28,11 +40,7 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { } /// Test that the percentages for founders all ends up as expected - function test_FounderShareAllocationFuzz( - uint256 f1Percentage, - uint256 f2Percentage, - uint256 f3Percentage - ) public { + function test_FounderShareAllocationFuzz(uint256 f1Percentage, uint256 f2Percentage, uint256 f3Percentage) public { address f1Wallet = address(0x1); address f2Wallet = address(0x2); address f3Wallet = address(0x3); @@ -426,11 +434,7 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { assertEq(token.ownerOf(tokenId), newMinter); } - function testRevert_OnlyMinterCanMintToRecipient( - address newMinter, - address nonMinter, - address recipient - ) public { + function testRevert_OnlyMinterCanMintToRecipient(address newMinter, address nonMinter, address recipient) public { vm.assume( newMinter != nonMinter && newMinter != founder && newMinter != address(0) && newMinter != address(auction) && recipient != address(0) ); @@ -450,12 +454,7 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { assertEq(token.ownerOf(tokenId), recipient); } - function testRevert_OnlyMinterCanMintBatch( - address newMinter, - address nonMinter, - address recipient, - uint256 amount - ) public { + function testRevert_OnlyMinterCanMintBatch(address newMinter, address nonMinter, address recipient, uint256 amount) public { vm.assume( newMinter != nonMinter && newMinter != founder && @@ -624,11 +623,7 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { assertEq(token.getFounders().length, 1); } - function test_UpdateFounderShareAllocationFuzz( - uint256 f1Percentage, - uint256 f2Percentage, - uint256 f3Percentage - ) public { + function test_UpdateFounderShareAllocationFuzz(uint256 f1Percentage, uint256 f2Percentage, uint256 f3Percentage) public { deployMock(); address f1Wallet = address(0x1); @@ -808,4 +803,167 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { vm.expectRevert(abi.encodeWithSignature("INVALID_OWNER()")); token.ownerOf(tokenId); } + + function test_BatchDelegateWithSig() public { + deployMock(); + + address[] memory fromAddresses = new address[](2); + fromAddresses[0] = address(new MockERC1271(delegator)); + fromAddresses[1] = address(new MockERC1271(delegator)); + + address[] memory toAddresses = new address[](2); + toAddresses[0] = delegator; + toAddresses[1] = delegator; + + uint256[] memory deadlines = new uint256[](2); + deadlines[0] = block.timestamp + 100; + deadlines[1] = block.timestamp + 100; + + bytes32 digest = token.getBatchDelegateBySigTypedDataHash(fromAddresses, toAddresses, deadlines); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(delegatorPK, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.startPrank(address(auction)); + token.mintTo(fromAddresses[0]); + token.mintTo(fromAddresses[1]); + vm.stopPrank(); + + assertEq(token.getVotes(delegator), 0); + + token.batchDelegateBySigERC1271(fromAddresses, toAddresses, deadlines, signature); + + assertEq(token.getVotes(delegator), 2); + } + + function testRevert_BatchDelegateWithSigSignatureExpired() public { + deployMock(); + + address[] memory fromAddresses = new address[](2); + fromAddresses[0] = address(new MockERC1271(delegator)); + fromAddresses[1] = address(new MockERC1271(delegator)); + + address[] memory toAddresses = new address[](2); + toAddresses[0] = delegator; + toAddresses[1] = delegator; + + uint256[] memory deadlines = new uint256[](2); + deadlines[0] = block.timestamp + 100; + deadlines[1] = block.timestamp + 100; + + bytes32 digest = token.getBatchDelegateBySigTypedDataHash(fromAddresses, toAddresses, deadlines); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(delegatorPK, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.startPrank(address(auction)); + token.mintTo(fromAddresses[0]); + token.mintTo(fromAddresses[1]); + vm.stopPrank(); + + vm.warp(block.timestamp + 101); + + assertEq(token.getVotes(delegator), 0); + + vm.expectRevert(abi.encodeWithSignature("EXPIRED_SIGNATURE()")); + token.batchDelegateBySigERC1271(fromAddresses, toAddresses, deadlines, signature); + + assertEq(token.getVotes(delegator), 0); + } + + function test_BatchDelegateWithSigInvalidSignature() public { + deployMock(); + + address[] memory fromAddresses = new address[](2); + fromAddresses[0] = address(new MockERC1271(delegator)); + fromAddresses[1] = address(new MockERC1271(delegator)); + + address[] memory toAddresses = new address[](2); + toAddresses[0] = delegator; + toAddresses[1] = delegator; + + uint256[] memory deadlines = new uint256[](2); + deadlines[0] = block.timestamp + 100; + deadlines[1] = block.timestamp + 100; + + bytes memory signature = new bytes(0); + + vm.startPrank(address(auction)); + token.mintTo(fromAddresses[0]); + token.mintTo(fromAddresses[1]); + vm.stopPrank(); + + assertEq(token.getVotes(delegator), 0); + + vm.expectRevert(abi.encodeWithSignature("INVALID_SIGNATURE()")); + token.batchDelegateBySigERC1271(fromAddresses, toAddresses, deadlines, signature); + + assertEq(token.getVotes(delegator), 0); + } + + function test_BatchDelegateWithSigInvalidSignatureValidation() public { + deployMock(); + + address[] memory fromAddresses = new address[](2); + fromAddresses[0] = address(1); + fromAddresses[1] = address(new MockERC1271(delegator)); + + address[] memory toAddresses = new address[](2); + toAddresses[0] = delegator; + toAddresses[1] = delegator; + + uint256[] memory deadlines = new uint256[](2); + deadlines[0] = block.timestamp + 100; + deadlines[1] = block.timestamp + 100; + + bytes32 digest = token.getBatchDelegateBySigTypedDataHash(fromAddresses, toAddresses, deadlines); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(delegatorPK, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.startPrank(address(auction)); + token.mintTo(fromAddresses[0]); + token.mintTo(fromAddresses[1]); + vm.stopPrank(); + + assertEq(token.getVotes(delegator), 0); + + vm.expectRevert(abi.encodeWithSignature("INVALID_SIGNATURE()")); + token.batchDelegateBySigERC1271(fromAddresses, toAddresses, deadlines, signature); + + assertEq(token.getVotes(delegator), 0); + } + + function test_BatchDelegateWithSigOwnerNotDelegator() public { + deployMock(); + + address[] memory fromAddresses = new address[](2); + fromAddresses[0] = address(new MockERC1271(address(1))); + fromAddresses[1] = address(new MockERC1271(delegator)); + + address[] memory toAddresses = new address[](2); + toAddresses[0] = delegator; + toAddresses[1] = delegator; + + uint256[] memory deadlines = new uint256[](2); + deadlines[0] = block.timestamp + 100; + deadlines[1] = block.timestamp + 100; + + bytes32 digest = token.getBatchDelegateBySigTypedDataHash(fromAddresses, toAddresses, deadlines); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(delegatorPK, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.startPrank(address(auction)); + token.mintTo(fromAddresses[0]); + token.mintTo(fromAddresses[1]); + vm.stopPrank(); + + assertEq(token.getVotes(delegator), 0); + + vm.expectRevert(abi.encodeWithSignature("INVALID_SIGNATURE()")); + token.batchDelegateBySigERC1271(fromAddresses, toAddresses, deadlines, signature); + + assertEq(token.getVotes(delegator), 0); + } } diff --git a/test/utils/mocks/MockERC1271.sol b/test/utils/mocks/MockERC1271.sol new file mode 100644 index 0000000..62feefa --- /dev/null +++ b/test/utils/mocks/MockERC1271.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.16; + +import { IERC1271 } from "../../../src/lib/interfaces/IERC1271.sol"; +import { SignatureChecker } from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; + +contract MockERC1271 is IERC1271 { + address owner; + + constructor(address _owner) { + owner = _owner; + } + + function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4 magicValue) { + bool isValid = SignatureChecker.isValidSignatureNow(owner, hash, signature); + + if (isValid) { + return IERC1271.isValidSignature.selector; + } + + return ""; + } + + receive() external payable {} +} From cf04f56522329ef5843dca17497ce285baa5ee44 Mon Sep 17 00:00:00 2001 From: neokry Date: Mon, 7 Aug 2023 15:58:02 +0900 Subject: [PATCH 3/3] Fix formatting --- src/lib/token/ERC721Votes.sol | 21 ++++++++++++++++++--- test/Token.t.sol | 25 +++++++++++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/lib/token/ERC721Votes.sol b/src/lib/token/ERC721Votes.sol index 1c34a85..7fb7f30 100644 --- a/src/lib/token/ERC721Votes.sol +++ b/src/lib/token/ERC721Votes.sol @@ -185,7 +185,14 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { /// @param _v The 129th byte and chain id of the signature /// @param _r The first 64 bytes of the signature /// @param _s Bytes 64-128 of the signature - function delegateBySig(address _from, address _to, uint256 _deadline, uint8 _v, bytes32 _r, bytes32 _s) external { + function delegateBySig( + address _from, + address _to, + uint256 _deadline, + uint8 _v, + bytes32 _r, + bytes32 _s + ) external { // Ensure the signature has not expired if (block.timestamp > _deadline) revert EXPIRED_SIGNATURE(); @@ -293,7 +300,11 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { /// @param _from The address delegating votes from /// @param _to The address delegating votes to /// @param _amount The number of votes delegating - function _moveDelegateVotes(address _from, address _to, uint256 _amount) internal { + function _moveDelegateVotes( + address _from, + address _to, + uint256 _amount + ) internal { unchecked { // If voting weight is being transferred: if (_from != _to && _amount > 0) { @@ -402,7 +413,11 @@ abstract contract ERC721Votes is IERC721Votes, EIP712, ERC721 { /// @param _from The token sender /// @param _to The token recipient /// @param _tokenId The ERC-721 token id - function _afterTokenTransfer(address _from, address _to, uint256 _tokenId) internal override { + function _afterTokenTransfer( + address _from, + address _to, + uint256 _tokenId + ) internal override { // Transfer 1 vote from the sender to the recipient _moveDelegateVotes(delegates(_from), delegates(_to), 1); diff --git a/test/Token.t.sol b/test/Token.t.sol index 5da8223..a89b156 100644 --- a/test/Token.t.sol +++ b/test/Token.t.sol @@ -40,7 +40,11 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { } /// Test that the percentages for founders all ends up as expected - function test_FounderShareAllocationFuzz(uint256 f1Percentage, uint256 f2Percentage, uint256 f3Percentage) public { + function test_FounderShareAllocationFuzz( + uint256 f1Percentage, + uint256 f2Percentage, + uint256 f3Percentage + ) public { address f1Wallet = address(0x1); address f2Wallet = address(0x2); address f3Wallet = address(0x3); @@ -434,7 +438,11 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { assertEq(token.ownerOf(tokenId), newMinter); } - function testRevert_OnlyMinterCanMintToRecipient(address newMinter, address nonMinter, address recipient) public { + function testRevert_OnlyMinterCanMintToRecipient( + address newMinter, + address nonMinter, + address recipient + ) public { vm.assume( newMinter != nonMinter && newMinter != founder && newMinter != address(0) && newMinter != address(auction) && recipient != address(0) ); @@ -454,7 +462,12 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { assertEq(token.ownerOf(tokenId), recipient); } - function testRevert_OnlyMinterCanMintBatch(address newMinter, address nonMinter, address recipient, uint256 amount) public { + function testRevert_OnlyMinterCanMintBatch( + address newMinter, + address nonMinter, + address recipient, + uint256 amount + ) public { vm.assume( newMinter != nonMinter && newMinter != founder && @@ -623,7 +636,11 @@ contract TokenTest is NounsBuilderTest, TokenTypesV1 { assertEq(token.getFounders().length, 1); } - function test_UpdateFounderShareAllocationFuzz(uint256 f1Percentage, uint256 f2Percentage, uint256 f3Percentage) public { + function test_UpdateFounderShareAllocationFuzz( + uint256 f1Percentage, + uint256 f2Percentage, + uint256 f3Percentage + ) public { deployMock(); address f1Wallet = address(0x1);