From 3502b89dc9d52b3c50d9b26b98988fa9446e3be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Sat, 11 Nov 2023 13:04:08 -0500 Subject: [PATCH] included more tests for grantRoleFrom function --- contracts/RolesRegistry/SftRolesRegistry.sol | 62 ++-- test/LinkedLists.spec.ts | 8 +- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 328 +++++++++++++++--- test/SftRolesRegistry/helpers.ts | 61 ++-- 4 files changed, 360 insertions(+), 99 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 39280ae..57479f1 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -28,13 +28,9 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { modifier onlyOwnerOrApprovedWithBalance(address _account, address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount) { require(_tokenAmount > 0, "SftRolesRegistry: tokenAmount must be greater than zero"); require( - _account == msg.sender || IERC1155(_tokenAddress).isApprovedForAll(_account, msg.sender), + _account == msg.sender || isRoleApprovedForAll(_tokenAddress, _account, msg.sender), "SftRolesRegistry: account not approved" ); - require( - IERC1155(_tokenAddress).balanceOf(_account, _tokenId) >= _tokenAmount, - "SftRolesRegistry: account has insufficient balance" - ); _; } @@ -61,8 +57,7 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { ); bytes32 rootKey = _getHeadKey(_roleAssignment.grantee, _roleAssignment.role, _roleAssignment.tokenAddress, _roleAssignment.tokenId); - uint256 headNonce = lists.heads[rootKey]; - LinkedLists.ListItem storage item = lists.items[headNonce]; + LinkedLists.ListItem storage item = lists.items[_roleAssignment.nonce]; if (item.data.expirationDate == 0) { // nonce is not being used @@ -78,29 +73,54 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { // nonce is being used require(item.data.hash == hash, "SftRolesRegistry: nonce exist, but data mismatch"); // validates nonce, role, tokenAddress, tokenId, grantor require(item.data.expirationDate < block.timestamp || item.data.revocable, "SftRolesRegistry: nonce is not expired or is not revocable"); - require(item.data.tokenAmount >= _roleAssignment.tokenAmount, "SftRolesRegistry: insufficient tokenAmount in nonce"); - - // return tokens if any - uint256 tokensToReturn = item.data.tokenAmount - _roleAssignment.tokenAmount; - if (tokensToReturn > 0) { - _transferFrom( - address(this), - _roleAssignment.grantor, - _roleAssignment.tokenAddress, - _roleAssignment.tokenId, - tokensToReturn - ); - } + + // deposit or withdraw tokens + _depositOrWithdrawTokens( + _roleAssignment.tokenAddress, + _roleAssignment.tokenId, + _roleAssignment.grantor, + item.data.tokenAmount, + _roleAssignment.tokenAmount + ); // remove from the list lists.remove(rootKey, _roleAssignment.nonce); - } // insert on the list _insert(hash, rootKey, _roleAssignment); } + function _depositOrWithdrawTokens( + address _tokenAddress, + uint256 _tokenId, + address _account, + uint256 _depositedAmount, + uint256 _amountRequired + ) internal { + if (_depositedAmount > _amountRequired) { + // return leftover tokens + uint256 tokensToReturn = _depositedAmount - _amountRequired; + _transferFrom( + address(this), + _account, + _tokenAddress, + _tokenId, + tokensToReturn + ); + } else if (_amountRequired > _depositedAmount) { + // deposit missing tokens + uint256 tokensToDeposit = _amountRequired - _depositedAmount; + _transferFrom( + _account, + address(this), + _tokenAddress, + _tokenId, + tokensToDeposit + ); + } + } + function _insert(bytes32 _hash, bytes32 _rootKey, RoleAssignment calldata _roleAssignment) internal { RoleData memory data = RoleData( _hash, diff --git a/test/LinkedLists.spec.ts b/test/LinkedLists.spec.ts index ae50553..c46e021 100644 --- a/test/LinkedLists.spec.ts +++ b/test/LinkedLists.spec.ts @@ -6,8 +6,8 @@ import { beforeEach } from 'mocha' import { generateRandomInt, assertListItem, assertList } from './helpers' const { HashZero } = ethers.constants -const HashOne = ethers.utils.formatBytes32String("1") -const HashTwo = ethers.utils.formatBytes32String("2") +const HashOne = ethers.utils.formatBytes32String('1') +const HashTwo = ethers.utils.formatBytes32String('2') describe('LinkedLists', async () => { let LinkedLists: Contract @@ -23,7 +23,6 @@ describe('LinkedLists', async () => { }) describe('Insert Item', async () => { - describe('List with one item', async () => { let FirstItem: { nonce: number; expirationDate: number } @@ -118,7 +117,6 @@ describe('LinkedLists', async () => { }) describe('Remove Item', async () => { - it('when list is empty, should revert', async () => { await expect(LinkedLists.remove(HashZero, 1)).to.revertedWith('LinkedLists: empty list or invalid nonce') }) @@ -220,7 +218,6 @@ describe('LinkedLists', async () => { await assertList(LinkedLists, HashZero, 0) await assertList(LinkedLists, HashOne, 0) await assertList(LinkedLists, HashTwo, 0) - }) it('should insert and remove 1,000 items @skip-on-coverage', async () => { @@ -235,5 +232,4 @@ describe('LinkedLists', async () => { } await assertList(LinkedLists, HashZero, 0) }) - }) diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index a6e17ed..ff6b9e5 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -4,23 +4,17 @@ import { beforeEach } from 'mocha' import { expect } from 'chai' import { loadFixture } from '@nomicfoundation/hardhat-network-helpers' import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { buildRoleAssignment, currentUnixTimestamp, ONE_DAY } from './helpers' - -// grantRoleFrom - // when nonce does not exist - // should revert if grantor does not have enough tokens - // should revert if tokenAmount is zero - // should grant role when grantor is sender and has enough tokens - // should grant role when sender is approved and grantor has enough tokens - // grant role to two different users and have separate balances - // grant role and wait for it to automatically expire - // when nonce exists - // tbd - -describe.only('SftRolesRegistry', async () => { +import { buildRoleAssignment, currentUnixTimestamp, ONE_DAY, generateRoleId } from './helpers' +import { RoleAssignment } from './types' +import { generateRandomInt } from '../helpers' + +const { AddressZero } = ethers.constants + +describe('SftRolesRegistry', async () => { let SftRolesRegistry: Contract let MockToken: Contract let grantor: SignerWithAddress + let anotherUser: SignerWithAddress async function deployContracts() { const SftRolesRegistryFactory = await ethers.getContractFactory('SftRolesRegistry') @@ -29,6 +23,7 @@ describe.only('SftRolesRegistry', async () => { MockToken = await MockTokenFactory.deploy() const signers = await ethers.getSigners() grantor = signers[0] + anotherUser = signers[1] return { SftRolesRegistry, MockToken, signers } } @@ -37,71 +32,318 @@ describe.only('SftRolesRegistry', async () => { }) describe('grantRole', async () => { - it('should revert without a reason if tokenAddress is not an ERC-1155 contract', async () => { - const roleAssignment = buildRoleAssignment() + const roleAssignment = await buildRoleAssignment() await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.reverted }) it('should revert if expirationDate is in the past', async () => { - const roleAssignment = buildRoleAssignment({ + const roleAssignment = await buildRoleAssignment({ expirationDate: currentUnixTimestamp() - ONE_DAY, }) - await expect(SftRolesRegistry.grantRoleFrom(roleAssignment)) - .to.be.revertedWith('SftRolesRegistry: expiration date must be in the future') + await expect(SftRolesRegistry.grantRoleFrom(roleAssignment)).to.be.revertedWith( + 'SftRolesRegistry: expiration date must be in the future', + ) }) it('should revert when sender is not grantor or approved', async () => { - const roleAssignment = buildRoleAssignment({ + const roleAssignment = await buildRoleAssignment({ tokenAddress: MockToken.address, }) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)) - .to.be.revertedWith('SftRolesRegistry: account not approved') + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( + 'SftRolesRegistry: account not approved', + ) }) it('should revert if contract cannot transfer tokens', async () => { - const roleAssignment = buildRoleAssignment({ - tokenAddress: MockToken.address, grantor: grantor.address, + const roleAssignment = await buildRoleAssignment({ + tokenAddress: MockToken.address, + grantor: grantor.address, }) await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)) - .to.be.revertedWith('ERC1155: caller is not token owner or approved') + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( + 'ERC1155: caller is not token owner or approved', + ) }) - - it('should revert if tokenAmount is zero', async () => { - const roleAssignment = buildRoleAssignment({ - tokenAmount: 0 + const roleAssignment = await buildRoleAssignment({ + tokenAmount: 0, }) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)) - .to.be.revertedWith('SftRolesRegistry: tokenAmount must be greater than zero') + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( + 'SftRolesRegistry: tokenAmount must be greater than zero', + ) }) it('should revert when grantor does not have enough tokens', async () => { - const roleAssignment = buildRoleAssignment({ - tokenAddress: MockToken.address, grantor: grantor.address, tokenAmount: 100, + const roleAssignment = await buildRoleAssignment({ + tokenAddress: MockToken.address, + grantor: grantor.address, + tokenAmount: 100, }) await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount - 10) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)) - .to.be.revertedWith('SftRolesRegistry: account has insufficient balance') + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( + 'ERC1155: insufficient balance for transfer', + ) }) - // todo tbd it('should revert if nonce is zero', async () => { - const roleAssignment = buildRoleAssignment({ - nonce: 0, tokenAddress: MockToken.address, grantor: grantor.address, + const roleAssignment = await buildRoleAssignment({ + nonce: 0, + tokenAddress: MockToken.address, + grantor: grantor.address, }) await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)) - .to.be.revertedWith('LinkedLists: invalid nonce') - expect(await MockToken.balanceOf(grantor.address, roleAssignment.tokenId)) - .to.be.equal(roleAssignment.tokenAmount) + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( + 'LinkedLists: invalid nonce', + ) + expect(await MockToken.balanceOf(grantor.address, roleAssignment.tokenId)).to.be.equal(roleAssignment.tokenAmount) }) + describe('when nonce does not exist', async () => { + it('should grant role when grantor is sender and has enough tokens', async () => { + const roleAssignment = await buildRoleAssignment({ + tokenAddress: MockToken.address, + grantor: grantor.address, + }) + await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + roleAssignment.nonce, + roleAssignment.role, + roleAssignment.tokenAddress, + roleAssignment.tokenId, + roleAssignment.tokenAmount, + roleAssignment.grantor, + roleAssignment.grantee, + roleAssignment.expirationDate, + roleAssignment.revocable, + roleAssignment.data, + ) + }) + it('should grant role when sender is approved and grantor has enough tokens', async () => { + const roleAssignment = await buildRoleAssignment({ + tokenAddress: MockToken.address, + grantor: grantor.address, + }) + await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( + roleAssignment.tokenAddress, + anotherUser.address, + true, + ) + await expect(SftRolesRegistry.connect(anotherUser).grantRoleFrom(roleAssignment)) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + roleAssignment.nonce, + roleAssignment.role, + roleAssignment.tokenAddress, + roleAssignment.tokenId, + roleAssignment.tokenAmount, + roleAssignment.grantor, + roleAssignment.grantee, + roleAssignment.expirationDate, + roleAssignment.revocable, + roleAssignment.data, + ) + }) + }) }) + describe('when nonce exists', async () => { + let RoleAssignment: RoleAssignment + + beforeEach(async () => { + RoleAssignment = await buildRoleAssignment({ + tokenAddress: MockToken.address, + grantor: grantor.address, + }) + await MockToken.mint(grantor.address, RoleAssignment.tokenId, RoleAssignment.tokenAmount) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(RoleAssignment)) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + RoleAssignment.nonce, + RoleAssignment.role, + RoleAssignment.tokenAddress, + RoleAssignment.tokenId, + RoleAssignment.tokenAmount, + RoleAssignment.grantor, + RoleAssignment.grantee, + RoleAssignment.expirationDate, + RoleAssignment.revocable, + RoleAssignment.data, + ) + }) + + it('should revert if hash is different', async () => { + // hash validates role, tokenAddress, tokenId, grantor + + // different role + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + role: generateRoleId('Role(uint256 newArg)'), + }), + ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') + + // tokenAddress + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + tokenAddress: AddressZero, + }), + ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') + + // tokenId + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + tokenId: generateRandomInt(), + }), + ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') + + // grantor + await SftRolesRegistry.connect(anotherUser).setRoleApprovalForAll( + RoleAssignment.tokenAddress, + grantor.address, + true, + ) + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + grantor: anotherUser.address, + }), + ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') + }) + + it('should revert if nonce is not expired', async () => { + const revocableRoleAssignment = await buildRoleAssignment({ + tokenAddress: MockToken.address, + grantor: grantor.address, + revocable: false, + }) + + await MockToken.mint(grantor.address, revocableRoleAssignment.tokenId, revocableRoleAssignment.tokenAmount) + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(revocableRoleAssignment)) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + revocableRoleAssignment.nonce, + revocableRoleAssignment.role, + revocableRoleAssignment.tokenAddress, + revocableRoleAssignment.tokenId, + revocableRoleAssignment.tokenAmount, + revocableRoleAssignment.grantor, + revocableRoleAssignment.grantee, + revocableRoleAssignment.expirationDate, + revocableRoleAssignment.revocable, + revocableRoleAssignment.data, + ) + + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(revocableRoleAssignment)).to.be.revertedWith( + 'SftRolesRegistry: nonce is not expired or is not revocable', + ) + }) + + it("should revert if grantor's balance is insufficient", async () => { + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + tokenAmount: RoleAssignment.tokenAmount + 1, + }), + ).to.be.revertedWith('ERC1155: insufficient balance for transfer') + }) + + it('should grant role if tokens deposited are greater than requested', async () => { + const newTokenAmount = RoleAssignment.tokenAmount - 1 + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + tokenAmount: newTokenAmount, + }), + ) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + RoleAssignment.nonce, + RoleAssignment.role, + RoleAssignment.tokenAddress, + RoleAssignment.tokenId, + newTokenAmount, + RoleAssignment.grantor, + RoleAssignment.grantee, + RoleAssignment.expirationDate, + RoleAssignment.revocable, + RoleAssignment.data, + ) + // transfer leftover tokens to grantor + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + SftRolesRegistry.address, + RoleAssignment.grantor, + RoleAssignment.tokenId, + RoleAssignment.tokenAmount - newTokenAmount, + ) + }) + + it('should grant role if tokens deposited are lower than deposited (but grantor deposits more)', async () => { + const additionalAmount = 1 + const newTokenAmount = RoleAssignment.tokenAmount + additionalAmount + await MockToken.mint(grantor.address, RoleAssignment.tokenId, additionalAmount) + + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + tokenAmount: newTokenAmount, + }), + ) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + RoleAssignment.nonce, + RoleAssignment.role, + RoleAssignment.tokenAddress, + RoleAssignment.tokenId, + newTokenAmount, + RoleAssignment.grantor, + RoleAssignment.grantee, + RoleAssignment.expirationDate, + RoleAssignment.revocable, + RoleAssignment.data, + ) + // transfer additional tokens to contract + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + RoleAssignment.grantor, + SftRolesRegistry.address, + RoleAssignment.tokenId, + additionalAmount, + ) + }) + + it('should grant role if tokens deposited are equal to tokens requested', async () => { + await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(RoleAssignment)) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + RoleAssignment.nonce, + RoleAssignment.role, + RoleAssignment.tokenAddress, + RoleAssignment.tokenId, + RoleAssignment.tokenAmount, + RoleAssignment.grantor, + RoleAssignment.grantee, + RoleAssignment.expirationDate, + RoleAssignment.revocable, + RoleAssignment.data, + ) + // should not transfer any tokens + .to.not.emit(MockToken, 'TransferSingle') + }) + }) }) diff --git a/test/SftRolesRegistry/helpers.ts b/test/SftRolesRegistry/helpers.ts index 6472254..7e55c22 100644 --- a/test/SftRolesRegistry/helpers.ts +++ b/test/SftRolesRegistry/helpers.ts @@ -2,51 +2,54 @@ import { solidityKeccak256 } from 'ethers/lib/utils' import { RoleAssignment } from './types' import { generateRandomInt } from '../helpers' import { ethers } from 'hardhat' +import { time } from '@nomicfoundation/hardhat-network-helpers' const { HashZero, AddressZero } = ethers.constants export const ONE_DAY = 60 * 60 * 24 -export function buildRoleAssignment( - { - // default values - nonce = generateRandomInt(), - role = 'Role()', - tokenAddress = AddressZero, - tokenId = generateRandomInt(), - tokenAmount = generateRandomInt(), - grantor = AddressZero, - grantee = AddressZero, - expirationDate = currentUnixTimestamp() + ONE_DAY, - revocable = true, - data = HashZero, - }: { - // types - nonce?: number, - role?: string, - tokenAddress?: string, - tokenId?: number, - tokenAmount?: number, - grantor?: string, - grantee?: string, - expirationDate?: number, - revocable?: boolean, - data?: string, - } = {} -): RoleAssignment { +export async function buildRoleAssignment({ + // default values + nonce = generateRandomInt(), + role = 'Role()', + tokenAddress = AddressZero, + tokenId = generateRandomInt(), + tokenAmount = generateRandomInt(), + grantor = AddressZero, + grantee = AddressZero, + expirationDate = null, + revocable = true, + data = HashZero, +}: { + // types + nonce?: number + role?: string + tokenAddress?: string + tokenId?: number + tokenAmount?: number + grantor?: string + grantee?: string + expirationDate?: number | null + revocable?: boolean + data?: string +} = {}): Promise { return { nonce, - role: solidityKeccak256(['string'], [role]), + role: generateRoleId(role), tokenAddress, tokenId, tokenAmount, grantor, grantee, - expirationDate, + expirationDate: expirationDate ? expirationDate : (await time.latest()) + ONE_DAY, revocable, data, } } +export function generateRoleId(role: string) { + return solidityKeccak256(['string'], [role]) +} + export function currentUnixTimestamp() { return Math.floor(Date.now() / 1000) }