Skip to content

Commit

Permalink
Merge pull request #186 from 1inch/feature/kycnft-eip712
Browse files Browse the repository at this point in the history 
[SC-1389] Use EIP-712 with domain separators for KycNFT owner signature
  • Loading branch information
@zZoMROT
zZoMROT authored Jan 31, 2025
2 parents 95ee348 + f1f8fcf commit 802877f
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 49 deletions.
19 changes: 12 additions & 7 deletions contracts/KycNFT.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,24 @@ pragma solidity 0.8.23;
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import { ERC721Burnable } from "@openzeppelin/contracts/token/ERC721/extensions/ERC721Burnable.sol";
import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
import { ECDSA } from "@1inch/solidity-utils/contracts/libraries/ECDSA.sol";

/**
* @title KycNFT
* @notice ERC721 token that allows only one NFT per address and includes transfer, mint and burn logic restricted to the contract owner.
*/
contract KycNFT is Ownable, ERC721Burnable {
contract KycNFT is Ownable, ERC721Burnable, EIP712 {
/// @notice Thrown when an address attempts to own more than one NFT.
error OnlyOneNFTPerAddress();
/// @notice Thrown when signature is incorrect.
error BadSignature();
/// @notice Thrown when signature is expired.
error SignatureExpired();

bytes32 public constant MINT_TYPEHASH = keccak256("Mint(address to,uint256 nonce,uint256 tokenId,uint256 deadline)");
bytes32 public constant TRANSFER_FROM_TYPEHASH = keccak256("TransferFrom(address to,uint256 nonce,uint256 tokenId,uint256 deadline)");

/// @notice Nonce for each token ID.
mapping(uint256 => uint256) public nonces;

Expand All @@ -27,10 +31,10 @@ contract KycNFT is Ownable, ERC721Burnable {
* @param tokenId The ID of the token.
* @param signature The signature to be verified.
*/
modifier onlyOwnerSignature(address to, uint256 tokenId, uint256 deadline, bytes calldata signature) {
modifier onlyOwnerSignature(address to, uint256 tokenId, uint256 deadline, bytes calldata signature, bytes32 typeHash) {
if (block.timestamp > deadline) revert SignatureExpired();
bytes memory message = abi.encodePacked(address(this), block.chainid, to, nonces[tokenId], tokenId, deadline);
bytes32 hash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n168", message));
bytes32 structHash = keccak256(abi.encode(typeHash, to, nonces[tokenId], tokenId, deadline));
bytes32 hash = _hashTypedDataV4(structHash);
if (owner() != ECDSA.recover(hash, signature)) revert BadSignature();
_;
}
Expand All @@ -41,7 +45,8 @@ contract KycNFT is Ownable, ERC721Burnable {
* @param symbol The symbol of the token.
* @param owner The address of the owner of the contract.
*/
constructor(string memory name, string memory symbol, address owner) ERC721(name, symbol) Ownable(owner) {}
constructor(string memory name, string memory symbol, string memory version, address owner)
ERC721(name, symbol) Ownable(owner) EIP712(name, version) {}

/**
* @notice Transfers a token to a specified address. Only the owner can call this function.
Expand All @@ -61,7 +66,7 @@ contract KycNFT is Ownable, ERC721Burnable {
* @param deadline The deadline for the signature.
* @param signature The signature of the owner permitting the transfer.
*/
function transferFrom(address from, address to, uint256 tokenId, uint256 deadline, bytes calldata signature) public onlyOwnerSignature(to, tokenId, deadline, signature) {
function transferFrom(address from, address to, uint256 tokenId, uint256 deadline, bytes calldata signature) public onlyOwnerSignature(to, tokenId, deadline, signature, TRANSFER_FROM_TYPEHASH) {
_transfer(from, to, tokenId);
}

Expand All @@ -79,7 +84,7 @@ contract KycNFT is Ownable, ERC721Burnable {
* @param deadline The deadline for the signature.
* @param signature The signature of the owner permitting the mint.
*/
function mint(address to, uint256 tokenId, uint256 deadline, bytes calldata signature) external onlyOwnerSignature(to, tokenId, deadline, signature) {
function mint(address to, uint256 tokenId, uint256 deadline, bytes calldata signature) external onlyOwnerSignature(to, tokenId, deadline, signature, MINT_TYPEHASH) {
_mint(to, tokenId);
}

Expand Down
104 changes: 62 additions & 42 deletions test/KycNFT.js
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,34 @@
const { expect, deployContract, constants, trim0x } = require('@1inch/solidity-utils');
const { expect, deployContract, constants } = require('@1inch/solidity-utils');
const { ethers } = require('hardhat');
const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
const { getChainId } = require('./helpers/fixtures');

async function signTokenId(nft, to, tokenId, signer, chainId, deadline) {
const packedData = ethers.solidityPacked(
['address', 'uint256', 'address', 'uint256', 'uint256', 'uint256'],
[await nft.getAddress(), chainId, to, await nft.nonces(tokenId), tokenId, deadline],
);
const message = Buffer.from(trim0x(packedData), 'hex');
return await signer.signMessage(message);
const MINT = {
Mint: [
{ name: 'to', type: 'address' },
{ name: 'nonce', type: 'uint256' },
{ name: 'tokenId', type: 'uint256' },
{ name: 'deadline', type: 'uint256' },
],
};
const TRANSFER_FROM = {
TransferFrom: MINT.Mint,
};

async function signTokenId(types, eip712, nft, to, tokenId, signer, chainId, deadline) {
const domain = {
name: eip712.name,
version: eip712.version,
chainId,
verifyingContract: await nft.getAddress(),
};
const values = {
to,
nonce: await nft.nonces(tokenId),
tokenId,
deadline,
};
return signer.signTypedData(domain, types, values);
}

describe('KycNFT', function () {
Expand All @@ -23,11 +42,12 @@ describe('KycNFT', function () {
another: '0x03',
nonexist: '0xabcdef',
};
const nft = await deployContract('KycNFT', ['KycNFT', 'KYC', owner]);
const eip712 = { name: 'KycNFT', version: '1' };
const nft = await deployContract('KycNFT', [eip712.name, 'KYC', eip712.version, owner]);
await nft.mint(owner, tokenIds.owner);
await nft.mint(bob, tokenIds.bob);
const deadline = '0xffffffff';
return { owner, alice, bob, charlie, nft, tokenIds, chainId, deadline };
return { owner, alice, bob, charlie, nft, tokenIds, chainId, eip712, deadline };
}

describe('transferFrom', function () {
Expand Down Expand Up @@ -62,61 +82,61 @@ describe('KycNFT', function () {

describe('transferFrom with signature', function () {
it('should revert with signature by non-owner', async function () {
const { alice, bob, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.bob, bob, chainId, deadline);
const { alice, bob, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, alice.address, tokenIds.bob, bob, chainId, deadline);
await expect(nft.connect(bob)['transferFrom(address,address,uint256,uint256,bytes)'](bob, alice, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'BadSignature');
});

it('should work with signature by owner', async function () {
const { owner, bob, alice, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const { owner, bob, alice, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const transferToken = tokenIds.bob;
const signature = await signTokenId(nft, alice.address, transferToken, owner, chainId, deadline);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, alice.address, transferToken, owner, chainId, deadline);
await nft.connect(bob)['transferFrom(address,address,uint256,uint256,bytes)'](bob, alice, transferToken, deadline, signature);
expect(await nft.ownerOf(tokenIds.bob)).to.equal(alice.address);
});

it('should revert with signature by owner and transfer someone else\'s token', async function () {
const { owner, bob, alice, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const { owner, bob, alice, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const transferToken = tokenIds.owner;
const signature = await signTokenId(nft, alice.address, transferToken, owner, chainId, deadline);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, alice.address, transferToken, owner, chainId, deadline);
await expect(nft.connect(bob)['transferFrom(address,address,uint256,uint256,bytes)'](bob, alice, transferToken, deadline, signature))
.to.be.revertedWithCustomError(nft, 'ERC721IncorrectOwner');
});

it('should revert when recipient account already has 1 nft', async function () {
const { owner, bob, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, owner.address, tokenIds.bob, owner, chainId, deadline);
const { owner, bob, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, owner.address, tokenIds.bob, owner, chainId, deadline);
await expect(nft.connect(owner)['transferFrom(address,address,uint256,uint256,bytes)'](bob, owner, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'OnlyOneNFTPerAddress');
});

it('should revert when send non-existen token', async function () {
const { owner, alice, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.nonexist, owner, chainId, deadline);
const { owner, alice, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, alice.address, tokenIds.nonexist, owner, chainId, deadline);
await expect(nft.connect(owner)['transferFrom(address,address,uint256,uint256,bytes)'](owner, alice, tokenIds.nonexist, deadline, signature))
.to.be.revertedWithCustomError(nft, 'ERC721NonexistentToken');
});

it('should revert after deadline expired', async function () {
const { alice, bob, nft, tokenIds, chainId } = await loadFixture(initContracts);
const { alice, bob, nft, tokenIds, chainId, eip712 } = await loadFixture(initContracts);
const deadline = '0x01';
const signature = await signTokenId(nft, alice.address, tokenIds.bob, bob, chainId, deadline);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, alice.address, tokenIds.bob, bob, chainId, deadline);
await expect(nft.connect(bob)['transferFrom(address,address,uint256,uint256,bytes)'](bob, alice, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'SignatureExpired');
});

it('should revert after another differrent transfer', async function () {
const { owner, alice, bob, charlie, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.bob, bob, chainId, deadline);
const { owner, alice, bob, charlie, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, alice.address, tokenIds.bob, bob, chainId, deadline);
await nft.connect(owner).transferFrom(bob, charlie, tokenIds.bob);
await expect(nft.connect(bob)['transferFrom(address,address,uint256,uint256,bytes)'](bob, alice, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'BadSignature');
});

it('should revert after burning', async function () {
const { owner, alice, bob, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.bob, bob, chainId, deadline);
const { owner, alice, bob, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(TRANSFER_FROM, eip712, nft, alice.address, tokenIds.bob, bob, chainId, deadline);
await nft.connect(owner).burn(tokenIds.bob);
await expect(nft.connect(bob)['transferFrom(address,address,uint256,uint256,bytes)'](bob, alice, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'BadSignature');
Expand Down Expand Up @@ -148,60 +168,60 @@ describe('KycNFT', function () {

describe('mint with signature', function () {
it('should revert with invalid signature', async function () {
const { owner, alice, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.alice, owner, chainId, deadline);
const { owner, alice, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(MINT, eip712, nft, alice.address, tokenIds.alice, owner, chainId, deadline);
const invalidSignature = signature.substring(-2) + '00';
await expect(nft.connect(owner)['mint(address,uint256,uint256,bytes)'](alice, tokenIds.alice, deadline, invalidSignature))
.to.be.revertedWithCustomError(nft, 'BadSignature');
});

it('should revert with invalid signature when frontrun and change to-address', async function () {
const { owner, bob, alice, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.alice, owner, chainId, deadline);
const { owner, bob, alice, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(MINT, eip712, nft, alice.address, tokenIds.alice, owner, chainId, deadline);
const invalidSignature = signature.substring(-2) + '00';
await expect(nft.connect(owner)['mint(address,uint256,uint256,bytes)'](bob, tokenIds.alice, deadline, invalidSignature))
.to.be.revertedWithCustomError(nft, 'BadSignature');
});

it('should revert with non-owner signature', async function () {
const { alice, bob, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.alice, bob, chainId, deadline);
const { alice, bob, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(MINT, eip712, nft, alice.address, tokenIds.alice, bob, chainId, deadline);
await expect(nft.connect(bob)['mint(address,uint256,uint256,bytes)'](alice, tokenIds.alice, deadline, signature))
.to.be.revertedWithCustomError(nft, 'BadSignature');
});

it('should work with owner signature', async function () {
const { owner, alice, bob, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.alice, owner, chainId, deadline);
const { owner, alice, bob, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(MINT, eip712, nft, alice.address, tokenIds.alice, owner, chainId, deadline);
await nft.connect(bob)['mint(address,uint256,uint256,bytes)'](alice, tokenIds.alice, deadline, signature);
expect(await nft.ownerOf(tokenIds.alice)).to.equal(alice.address);
});

it('should revert when account already has 1 nft', async function () {
const { owner, bob, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, bob.address, tokenIds.another, owner, chainId, deadline);
const { owner, bob, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(MINT, eip712, nft, bob.address, tokenIds.another, owner, chainId, deadline);
await expect(nft.connect(owner)['mint(address,uint256,uint256,bytes)'](bob, tokenIds.another, deadline, signature))
.to.be.revertedWithCustomError(nft, 'OnlyOneNFTPerAddress');
});

it('should revert when tokenId already exist', async function () {
const { owner, alice, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.bob, owner, chainId, deadline);
const { owner, alice, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(MINT, eip712, nft, alice.address, tokenIds.bob, owner, chainId, deadline);
await expect(nft.connect(owner)['mint(address,uint256,uint256,bytes)'](alice, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'ERC721InvalidSender');
});

it('should revert after deadline expired', async function () {
const { owner, alice, nft, tokenIds, chainId } = await loadFixture(initContracts);
const { owner, alice, nft, tokenIds, chainId, eip712 } = await loadFixture(initContracts);
const deadline = '0x01';
const signature = await signTokenId(nft, alice.address, tokenIds.bob, owner, chainId, deadline);
const signature = await signTokenId(MINT, eip712, nft, alice.address, tokenIds.bob, owner, chainId, deadline);
await expect(nft.connect(owner)['mint(address,uint256,uint256,bytes)'](alice, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'SignatureExpired');
});

it('should revert after burning', async function () {
const { owner, alice, nft, tokenIds, chainId, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(nft, alice.address, tokenIds.bob, owner, chainId, deadline);
const { owner, alice, nft, tokenIds, chainId, eip712, deadline } = await loadFixture(initContracts);
const signature = await signTokenId(MINT, eip712, nft, alice.address, tokenIds.bob, owner, chainId, deadline);
await nft.connect(owner).burn(tokenIds.bob);
await expect(nft.connect(owner)['mint(address,uint256,uint256,bytes)'](alice, tokenIds.bob, deadline, signature))
.to.be.revertedWithCustomError(nft, 'BadSignature');
Expand Down

0 comments on commit 802877f

Please sign in to comment.