From 3d156bf10f7467562b91ee3ca2a6268514010826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Sat, 29 Jun 2024 17:14:02 -0400 Subject: [PATCH 1/5] ON-876: reviewing ERC-7589 interface --- contracts/ERC7589/ERC7589RolesRegistry.sol | 308 +++++++++++ contracts/interfaces/IERC7589.sol | 156 ++++++ ...ERC7589LockTokensAndGrantRoleExtension.sol | 31 ++ contracts/mocks/MockERC1155.sol | 4 - test/ERC7589/ERC7589RolesRegistry.spec.ts | 519 ++++++++++++++++++ test/helpers.ts | 3 +- 6 files changed, 1016 insertions(+), 5 deletions(-) create mode 100644 contracts/ERC7589/ERC7589RolesRegistry.sol create mode 100644 contracts/interfaces/IERC7589.sol create mode 100644 contracts/interfaces/IERC7589LockTokensAndGrantRoleExtension.sol create mode 100644 test/ERC7589/ERC7589RolesRegistry.spec.ts diff --git a/contracts/ERC7589/ERC7589RolesRegistry.sol b/contracts/ERC7589/ERC7589RolesRegistry.sol new file mode 100644 index 0000000..c4e5084 --- /dev/null +++ b/contracts/ERC7589/ERC7589RolesRegistry.sol @@ -0,0 +1,308 @@ +// SPDX-License-Identifier: CC0-1.0 + +pragma solidity 0.8.9; + +import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; +import { IERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol'; +import { ERC1155Holder, ERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol'; +import { IERC7589 } from '../interfaces/IERC7589.sol'; +import { IERC1155 } from '@openzeppelin/contracts/token/ERC1155/IERC1155.sol'; +import { IERC7589RoleBalanceOfExtension } from '../interfaces/IERC7589RoleBalanceOfExtension.sol'; +import { IERC7589LockTokensAndGrantRoleExtension } from '../interfaces/IERC7589LockTokensAndGrantRoleExtension.sol'; +import { Uint64SortedLinkedListLibrary } from '../libraries/Uint64SortedLinkedListLibrary.sol'; +import { IOriumWrapperManager } from '../interfaces/IOriumWrapperManager.sol'; + +contract ERC7589RolesRegistry is IERC7589, ERC1155Holder, IERC7589LockTokensAndGrantRoleExtension { + using Uint64SortedLinkedListLibrary for Uint64SortedLinkedListLibrary.List; + + struct TokenLock { + address owner; + address tokenAddress; + uint256 tokenId; + uint256 tokenAmount; + } + + struct Role { + address recipient; + uint64 expirationDate; + bool revocable; + bytes data; + } + + address public managerAddress; + + address public marketplaceAddress; + + uint256 public lockIdCount; + + // tokenAddress => isAllowed + mapping(address => bool) public isTokenAddressAllowed; + + // lockId => TokenLock + mapping(uint256 => TokenLock) public tokenLocks; + + // lockId => roleId => Role + mapping(uint256 => mapping(bytes32 => Role)) public roles; + + // tokenAddress => tokenId => List + mapping(uint256 => Uint64SortedLinkedListLibrary.List) public tokenLockExpirationDates; + + // ownerAddress => tokenAddress => operator => isApproved + mapping(address => mapping(address => mapping(address => bool))) public roleApprovals; + + // supportedInterfaces => bool + mapping(bytes4 => bool) private supportedInterfaces; + + modifier onlyManager() { + require(msg.sender == managerAddress, 'ERC7589RolesRegistry: sender is not manager'); + _; + } + + modifier onlyAllowedTokenAddress(address _tokenAddress) { + require(isTokenAddressAllowed[_tokenAddress], 'ERC7589RolesRegistry: tokenAddress is not allowed'); + _; + } + + modifier onlyOwnerOrApproved(address _account, address _tokenAddress) { + require( + msg.sender == _account || isRoleApprovedForAll(_tokenAddress, _account, msg.sender), + 'ERC7589RolesRegistry: sender is not owner or approved' + ); + _; + } + + modifier onlyTokenLockOwnerOrApproved(uint256 _lockId) { + TokenLock storage _tokenLock = tokenLocks[_lockId]; + require( + msg.sender == _tokenLock.owner || isRoleApprovedForAll(_tokenLock.tokenAddress, _tokenLock.owner, msg.sender), + 'ERC7589RolesRegistry: sender is not owner or approved' + ); + _; + } + + constructor(address _marketplaceAddress) { + managerAddress = msg.sender; + marketplaceAddress = _marketplaceAddress; + supportedInterfaces[type(IERC7589).interfaceId] = true; + supportedInterfaces[type(IERC1155Receiver).interfaceId] = true; + supportedInterfaces[type(IERC7589LockTokensAndGrantRoleExtension).interfaceId] = true; + } + + /** ERC-7589 External Functions **/ + + function lockTokens( + address _owner, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount + ) external returns (uint256 lockId_) { + lockId_ = _lockTokens(_owner, _tokenAddress, _tokenId, _tokenAmount); + } + + function grantRole( + uint256 _lockId, + bytes32 _roleId, + address _recipient, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external { + _grantERC7589Role(_lockId, _roleId, _recipient, _expirationDate, _revocable, _data); + } + + function revokeRole( + uint256 _lockId, bytes32 _roleId, address _recipient + ) external { + TokenLock storage _tokenLock = tokenLocks[_lockId]; + Role storage _role = roles[_lockId][_roleId]; + require(_role.expirationDate > block.timestamp, 'ERC7589RolesRegistry: role does not exist'); + + // ensure caller is approved + address caller = _findCaller(_tokenLock.owner, _role.recipient, _tokenLock.tokenAddress); + if (_role.expirationDate > block.timestamp && !_role.revocable) { + // if role is not expired and is not revocable, only the grantee can revoke it + require( + caller == _role.recipient, + 'ERC7589RolesRegistry: role is not revocable or caller is not the approved' + ); + } + + if (!_role.revocable) { + tokenLockExpirationDates[_lockId].remove(_role.expirationDate); + } + + delete roles[_lockId][_roleId]; + emit RoleRevoked(_lockId, _roleId, _recipient); + } + + function unlockTokens(uint256 _lockId) external onlyTokenLockOwnerOrApproved(_lockId) { + uint64 _headExpirationDate = tokenLockExpirationDates[_lockId].head; + require(_headExpirationDate < block.timestamp, 'ERC7589RolesRegistry: NFT is locked'); + + address _owner = tokenLocks[_lockId].owner; + address _tokenAddress = tokenLocks[_lockId].tokenAddress; + uint256 _tokenId = tokenLocks[_lockId].tokenId; + uint256 _tokenAmount = tokenLocks[_lockId].tokenAmount; + delete tokenLocks[_lockId]; + emit TokensUnlocked(_lockId); + + _transferFrom( + address(this), _owner, _tokenAddress, _tokenId, _tokenAmount + ); + } + + function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _approved) external { + roleApprovals[msg.sender][_tokenAddress][_operator] = _approved; + } + + /** ERC-7589 Lock Tokens and Grant Role Extension External Functions **/ + + function lockTokensAndGrantRole( + address _owner, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount, + bytes32 _roleId, + address _recipient, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external returns (uint256 lockId_) { + lockId_ = _lockTokens(_owner, _tokenAddress, _tokenId, _tokenAmount); + _grantERC7589Role(lockId_, _roleId, _recipient, _expirationDate, _revocable, _data); + } + + /** Manager External Functions **/ + + function setMarketplaceAddress(address _marketplaceAddress) external onlyManager { + marketplaceAddress = _marketplaceAddress; + } + + function setTokenAddressAllowed(address _tokenAddress, bool _isAllowed) external onlyManager { + isTokenAddressAllowed[_tokenAddress] = _isAllowed; + } + + function setManagerAddress(address _managerAddress) external onlyManager { + managerAddress = _managerAddress; + } + + /** View Functions **/ + + function ownerOf(uint256 _lockId) external view returns (address owner_) { + return tokenLocks[_lockId].owner; + } + + function tokenAddressOf(uint256 _lockId) external view returns (address tokenAddress_) { + return tokenLocks[_lockId].tokenAddress; + } + + function tokenIdOf(uint256 _lockId) external view returns (uint256 tokenId_) { + return tokenLocks[_lockId].tokenId; + } + + function tokenAmountOf(uint256 _lockId) external view returns (uint256 tokenAmount_) { + return tokenLocks[_lockId].tokenAmount; + } + + function roleData(uint256 _lockId, bytes32 _roleId) external view returns (bytes memory data_) { + if (roles[_lockId][_roleId].expirationDate > block.timestamp) { + return roles[_lockId][_roleId].data; + } + return ''; + } + + function roleExpirationDate(uint256 _lockId, bytes32 _roleId) external view returns (uint64 expirationDate_) { + if (roles[_lockId][_roleId].expirationDate > block.timestamp) { + return roles[_lockId][_roleId].expirationDate; + } + return 0; + } + + function isRoleRevocable(uint256 _lockId, bytes32 _roleId) external view returns (bool revocable_) { + if (roles[_lockId][_roleId].expirationDate > block.timestamp) { + return roles[_lockId][_roleId].revocable; + } + return false; + } + + function isRoleApprovedForAll( + address _tokenAddress, + address _owner, + address _operator + ) public view returns (bool isApproved_) { + return _operator == marketplaceAddress || roleApprovals[_owner][_tokenAddress][_operator]; + } + + /** ERC-165 View Functions **/ + + function supportsInterface( + bytes4 interfaceId + ) public view virtual override(IERC165, ERC1155Receiver) returns (bool) { + return supportedInterfaces[interfaceId]; + } + + /** Helper Functions **/ + + function _lockTokens( + address _owner, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount + ) private onlyAllowedTokenAddress(_tokenAddress) onlyOwnerOrApproved(_owner, _tokenAddress) returns (uint256 lockId_) { + require(_tokenAmount > 0, 'ERC7589RolesRegistry: tokenAmount must be greater than zero'); + lockId_ = ++lockIdCount; + tokenLocks[lockId_] = TokenLock(_owner, _tokenAddress, _tokenId, _tokenAmount); + emit TokensLocked(_owner, lockId_, _tokenAddress, _tokenId, _tokenAmount); + _transferFrom(_owner, address(this), _tokenAddress, _tokenId, _tokenAmount); + } + + function _grantERC7589Role( + uint256 _lockId, + bytes32 _roleId, + address _recipient, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) private onlyTokenLockOwnerOrApproved(_lockId) { + require(_expirationDate > block.timestamp, 'ERC7589RolesRegistry: expirationDate must be in the future'); + + // only grant new role if previous role is expired or revocable + Role storage _currentRole = roles[_lockId][_roleId]; + require( + _currentRole.expirationDate < block.timestamp || _currentRole.revocable, + 'ERC7589RolesRegistry: role is not expired nor revocable' + ); + + // if role is not revocable + if (!_revocable) { + // add expiration date to lock list + tokenLockExpirationDates[_lockId].insert(_expirationDate); + } + + roles[_lockId][_roleId] = Role(_recipient, _expirationDate, _revocable, _data); + emit RoleGranted(_lockId, _roleId, _recipient, _expirationDate, _revocable, _data); + } + + function _transferFrom( + address _from, + address _to, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount + ) internal { + IERC1155(_tokenAddress).safeTransferFrom(_from, _to, _tokenId, _tokenAmount, ''); + } + + // careful with the following edge case: + // if both owner and recipient approve the sender, the recipient should be returned + // if owner is returned instead, the recipient won't be able to revoke roles + function _findCaller(address _owner, address _recipient, address _tokenAddress) internal view returns (address) { + if (_recipient == msg.sender || isRoleApprovedForAll(_tokenAddress, _recipient, msg.sender)) { + return _recipient; + } + if (_owner == msg.sender || isRoleApprovedForAll(_tokenAddress, _owner, msg.sender)) { + return _owner; + } + revert('ERC7589RolesRegistry: sender is not approved'); + } +} diff --git a/contracts/interfaces/IERC7589.sol b/contracts/interfaces/IERC7589.sol new file mode 100644 index 0000000..2ce92d6 --- /dev/null +++ b/contracts/interfaces/IERC7589.sol @@ -0,0 +1,156 @@ +// SPDX-License-Identifier: CC0-1.0 + +pragma solidity 0.8.9; + +import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; + +/// @title ERC-7589 Semi-Fungible Token Roles +/// @dev See https://eips.ethereum.org/EIPS/eip-7589 +/// Note: the ERC-165 identifier for this interface is 0x6f831543. +interface IERC7589 is IERC165 { + /** Events **/ + + /// @notice Emitted when tokens are locked (deposited or frozen). + /// @param _owner The owner of the tokens. + /// @param _lockId The identifier of the locked tokens. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + /// @param _tokenAmount The token amount. + event TokensLocked( + address indexed _owner, + uint256 indexed _lockId, + address indexed _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount + ); + + /// @notice Emitted when a role is granted. + /// @param _lockId The identifier of the locked tokens. + /// @param _roleId The role identifier. + /// @param _recipient The recipient the role. + /// @param _expirationDate The expiration date of the role. + /// @param _revocable Whether the role is revocable. + /// @param _data Any additional data about the role. + event RoleGranted( + uint256 indexed _lockId, + bytes32 indexed _roleId, + address indexed _recipient, + uint64 _expirationDate, + bool _revocable, + bytes _data + ); + + /// @notice Emitted when a role is revoked. + /// @param _lockId The identifier of the locked tokens. + /// @param _roleId The role identifier. + /// @param _recipient The recipient of the role revocation. + event RoleRevoked(uint256 indexed _lockId, bytes32 indexed _roleId, address indexed _recipient); + + /// @notice Emitted when tokens are unlocked (withdrawn or unfrozen). + /// @param _lockId The identifier of the locked tokens. + event TokensUnlocked(uint256 indexed _lockId); + + /// @notice Emitted when a user is approved to manage roles on behalf of another user. + /// @param _tokenAddress The token address. + /// @param _operator The user approved to grant and revoke roles. + /// @param _isApproved The approval status. + event RoleApprovalForAll(address indexed _tokenAddress, address indexed _operator, bool _isApproved); + + /** External Functions **/ + + /// @notice Lock tokens (deposits on a contract or freezes balance). + /// @param _owner The owner of the tokens. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + /// @param _tokenAmount The token amount. + /// @return lockId_ The identifier of the locked tokens. + function lockTokens( + address _owner, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount + ) external returns (uint256 lockId_); + + /// @notice Grants a role to a user. + /// @param _lockId The identifier of the locked tokens. + /// @param _roleId The role identifier. + /// @param _recipient The recipient the role. + /// @param _expirationDate The expiration date of the role. + /// @param _revocable Whether the role is revocable. + /// @param _data Any additional data about the role. + function grantRole( + uint256 _lockId, + bytes32 _roleId, + address _recipient, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external; + + /// @notice Revokes a role. + /// @param _lockId The identifier of the locked tokens. + /// @param _roleId The role identifier. + /// @param _recipient The recipient of the role revocation. + function revokeRole(uint256 _lockId, bytes32 _roleId, address _recipient) external; + + /// @notice Unlocks tokens (transfer back to original owner or unfreeze it). + /// @param _lockId The identifier of the locked tokens. + function unlockTokens(uint256 _lockId) external; + + /// @notice Approves operator to grant and revoke roles on behalf of another user. + /// @param _tokenAddress The token address. + /// @param _operator The user approved to grant and revoke roles. + /// @param _approved The approval status. + function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _approved) external; + + /** View Functions **/ + + /// @notice Retrieves the owner of the tokens. + /// @param _lockId The identifier of the locked tokens. + /// @return owner_ The owner of the tokens. + function ownerOf(uint256 _lockId) external view returns (address owner_); + + /// @notice Retrieves the address of the locked tokens. + /// @param _lockId The identifier of the locked tokens. + /// @return tokenAddress_ The token address. + function tokenAddressOf(uint256 _lockId) external view returns (address tokenAddress_); + + /// @notice Retrieves the tokenId of the locked tokens. + /// @param _lockId The identifier of the locked tokens. + /// @return tokenId_ The token identifier. + function tokenIdOf(uint256 _lockId) external view returns (uint256 tokenId_); + + /// @notice Retrieves the amount of tokens locked. + /// @param _lockId The identifier of the locked tokens. + /// @return tokenAmount_ The token amount. + function tokenAmountOf(uint256 _lockId) external view returns (uint256 tokenAmount_); + + /// @notice Retrieves the custom data of a role. + /// @param _lockId The identifier of the locked tokens. + /// @param _roleId The role identifier. + /// @return data_ The custom data. + function roleData(uint256 _lockId, bytes32 _roleId) external view returns (bytes memory data_); + + /// @notice Retrieves the expiration date of a role. + /// @param _lockId The identifier of the locked tokens. + /// @param _roleId The role identifier. + /// @return expirationDate_ The expiration date. + function roleExpirationDate(uint256 _lockId, bytes32 _roleId) external view returns (uint64 expirationDate_); + + /// @notice Retrieves the expiration date of a role. + /// @param _lockId The identifier of the locked tokens. + /// @param _roleId The role identifier. + /// @return revocable_ Whether the role is revocable or not. + function isRoleRevocable(uint256 _lockId, bytes32 _roleId) external view returns (bool revocable_); + + /// @notice Checks if the owner approved the operator for all SFTs. + /// @param _tokenAddress The token address. + /// @param _owner The user that approved the operator. + /// @param _operator The user that can grant and revoke roles. + /// @return isApproved_ Whether the operator is approved or not. + function isRoleApprovedForAll( + address _tokenAddress, + address _owner, + address _operator + ) external view returns (bool isApproved_); +} diff --git a/contracts/interfaces/IERC7589LockTokensAndGrantRoleExtension.sol b/contracts/interfaces/IERC7589LockTokensAndGrantRoleExtension.sol new file mode 100644 index 0000000..8337235 --- /dev/null +++ b/contracts/interfaces/IERC7589LockTokensAndGrantRoleExtension.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: CC0-1.0 + +pragma solidity 0.8.9; + +/// @title ERC-7589 Semi-Fungible Token Roles, optional lock tokens and grant role extension +/// @dev See https://eips.ethereum.org/EIPS/eip-7589 +/// Note: the ERC-165 identifier for this interface is 0x0a644ace. +interface IERC7589LockTokensAndGrantRoleExtension { + /// @notice Lock tokens and grant role in a single transaction. + /// @param _owner The owner of the tokens. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + /// @param _tokenAmount The token amount. + /// @param _roleId The role identifier. + /// @param _recipient The recipient the role. + /// @param _expirationDate The expiration date of the role. + /// @param _revocable Whether the role is revocable. + /// @param _data Any additional data about the role. + /// @return lockId_ The identifier of the locked tokens. + function lockTokensAndGrantRole( + address _owner, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount, + bytes32 _roleId, + address _recipient, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external returns (uint256 lockId_); +} diff --git a/contracts/mocks/MockERC1155.sol b/contracts/mocks/MockERC1155.sol index 170b1ad..84802a0 100644 --- a/contracts/mocks/MockERC1155.sol +++ b/contracts/mocks/MockERC1155.sol @@ -13,8 +13,4 @@ contract MockERC1155 is ERC1155 { function mint(address to, uint256 tokenId, uint256 tokenAmount) external { _mint(to, tokenId, tokenAmount, ''); } - - // function tokenURI(uint256 tokenId) public pure override returns (string memory) { - // return string(abi.encodePacked('https://example.com/', tokenId.toString())); - // } } diff --git a/test/ERC7589/ERC7589RolesRegistry.spec.ts b/test/ERC7589/ERC7589RolesRegistry.spec.ts new file mode 100644 index 0000000..ac9595b --- /dev/null +++ b/test/ERC7589/ERC7589RolesRegistry.spec.ts @@ -0,0 +1,519 @@ +import { ethers } from 'hardhat' +import { Contract, BigNumber } from 'ethers' +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' +import { beforeEach } from 'mocha' +import { loadFixture, time } from '@nomicfoundation/hardhat-network-helpers' +import { expect } from 'chai' +import { + IERC1155Receiver__factory, + IERC7589__factory, + IERC7589LockTokensAndGrantRoleExtension__factory, +} from '../../typechain-types' +import { generateErc165InterfaceId, generateRandomInt, ONE_DAY, ROLE, ZERO } from '../helpers' + +const { AddressZero, HashZero } = ethers.constants + +describe('ERC7589RolesRegistry', async () => { + let ERC7589RolesRegistry: Contract + let MockErc1155Token: Contract + let owner: SignerWithAddress + let recipient: SignerWithAddess + let anotherUser: SignerWithAddress + let tokenId: BigNumber + let tokenAmount: BigNumber + let expirationDate: number + let lockId = BigNumber.from(1) + + async function deployContracts() { + const ERC7589RolesRegistryFactory = await ethers.getContractFactory('ERC7589RolesRegistry') + ERC7589RolesRegistry = await ERC7589RolesRegistryFactory.deploy(AddressZero) + const MockErc1155TokenFactory = await ethers.getContractFactory('MockERC1155') + MockErc1155Token = await MockErc1155TokenFactory.deploy() + await expect(ERC7589RolesRegistry.setTokenAddressAllowed(MockErc1155Token.address, true)).to.not.be.reverted + const signers = await ethers.getSigners() + owner = signers[0] + recipient = signers[1] + anotherUser = signers[2] + expirationDate = (await time.latest()) + ONE_DAY + } + + async function lockTokens(sender: SignerWithAddress) { + await expect( + ERC7589RolesRegistry.connect(sender).lockTokens(owner.address, MockErc1155Token.address, tokenId, tokenAmount), + ) + .to.emit(ERC7589RolesRegistry, 'TokensLocked') + .withArgs(owner.address, 1, MockErc1155Token.address, tokenId, tokenAmount) + .to.emit(MockErc1155Token, 'TransferSingle') + .withArgs(ERC7589RolesRegistry.address, owner.address, ERC7589RolesRegistry.address, tokenId, tokenAmount) + } + + async function grantRole({ sender = owner, revocable = true } = {}) { + await expect( + ERC7589RolesRegistry.connect(sender).grantRole( + lockId, + ROLE, + recipient.address, + expirationDate, + revocable, + HashZero, + ), + ) + .to.emit(ERC7589RolesRegistry, 'RoleGranted') + .withArgs(lockId, ROLE, recipient.address, expirationDate, revocable, HashZero) + } + + beforeEach(async () => { + await loadFixture(deployContracts) + tokenId = BigNumber.from(generateRandomInt()) + tokenAmount = BigNumber.from(generateRandomInt()) + await expect(MockErc1155Token.mint(owner.address, tokenId, tokenAmount)).to.not.be.reverted + }) + + describe('lockTokens', async () => { + it('should revert when sender is not owner or approved', async () => { + await expect( + ERC7589RolesRegistry.connect(anotherUser).lockTokens( + owner.address, + MockErc1155Token.address, + tokenId, + tokenAmount, + ), + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not owner or approved') + }) + + it('should revert when tokenAmount is zero', async () => { + await expect( + ERC7589RolesRegistry.connect(owner).lockTokens(owner.address, MockErc1155Token.address, tokenId, 0), + ).to.be.revertedWith('ERC7589RolesRegistry: tokenAmount must be greater than zero') + }) + + it('should revert without a reason if tokenAddress is not an ERC-1155 contract', async () => { + await expect(ERC7589RolesRegistry.connect(owner).lockTokens(owner.address, AddressZero, tokenId, tokenAmount)).to + .be.reverted + }) + + it('should revert if contract is not approved to transfer tokens', async () => { + await expect( + ERC7589RolesRegistry.connect(owner).lockTokens(owner.address, MockErc1155Token.address, tokenId, tokenAmount), + ).to.be.revertedWith('ERC1155: caller is not token owner or approved') + }) + + it('should revert when owner does not have enough tokens', async () => { + await expect( + ERC7589RolesRegistry.connect(owner).lockTokens( + owner.address, + MockErc1155Token.address, + tokenId, + tokenAmount.add(1), + ), + ).to.be.revertedWith('ERC1155: caller is not token owner or approved') + }) + + it('should lock tokens when sender is owner', async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted + await lockTokens(owner) + }) + + it('should lock tokens when sender is approved', async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted + await expect( + ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ).to.not.be.reverted + await lockTokens(anotherUser) + }) + + it('should lock tokens when sender is marketplace contract', async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted + await expect( + ERC7589RolesRegistry.connect(owner).setMarketplaceAddress(anotherUser.address), + ).to.not.be.reverted + await lockTokens(anotherUser) + }) + }) + + describe('grantRole', async () => { + + beforeEach(async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted + await lockTokens(owner) + }) + + it('should revert when sender is not owner or approved', async () => { + await expect( + ERC7589RolesRegistry.connect(anotherUser).grantRole( + lockId, + ROLE, + recipient.address, + expirationDate, + false, + HashZero, + ), + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not owner or approved') + }) + + it('should revert when expirationDate is in the past', async () => { + const pastExpirationDate = (await time.latest()) - 1 + await expect( + ERC7589RolesRegistry.connect(owner).grantRole( + lockId, + ROLE, + recipient.address, + pastExpirationDate, + false, + HashZero, + ), + ).to.be.revertedWith('ERC7589RolesRegistry: expirationDate must be in the future') + }) + + it('should revert when previous role is not revocable or expired', async () => { + await grantRole({ revocable: false }) + await expect( + ERC7589RolesRegistry.connect(owner).grantRole( + lockId, + ROLE, + recipient.address, + expirationDate, + false, + HashZero, + ), + ).to.be.revertedWith('ERC7589RolesRegistry: role is not expired nor revocable') + }) + + it('should grant role when previous role is not expired but is revocable', async () => { + await grantRole({ revocable: true }) + await grantRole() + }) + + it('should grant role when sender is owner', async () => { + await grantRole() + }) + + it('should grant role when sender is approved', async () => { + await expect( + ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ).to.not.be.reverted + await grantRole({ sender: anotherUser }) + }) + }) + + describe('revokeRole', async () => { + + beforeEach(async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted + await lockTokens(owner) + await grantRole() + }) + + it('should revert when sender is not owner, recipient or approved', async () => { + await expect( + ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address), + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not approved') + }) + + it('should revert when sender is owner but role is not revocable nor expired', async () => { + await grantRole({ revocable: false }) + await expect( + ERC7589RolesRegistry.connect(owner).revokeRole(lockId, ROLE, recipient.address), + ).to.be.revertedWith('ERC7589RolesRegistry: role is not revocable or caller is not the approved') + }) + + it('should revert when role is expired', async () => { + await grantRole() + await time.increase(ONE_DAY) + + await expect( + ERC7589RolesRegistry.revokeRole(lockId, ROLE, recipient.address), + ).to.be.revertedWith('ERC7589RolesRegistry: role does not exist') + }) + + it('should revert when role was already revoked', async () => { + await expect(ERC7589RolesRegistry.revokeRole(lockId, ROLE, recipient.address)) + .to.emit(ERC7589RolesRegistry, 'RoleRevoked') + .withArgs(lockId, ROLE, recipient.address) + + await expect( + ERC7589RolesRegistry.revokeRole(lockId, ROLE, recipient.address), + ).to.be.revertedWith('ERC7589RolesRegistry: role does not exist') + }) + + it('should revoke role when sender is recipient', async () => { + await expect(ERC7589RolesRegistry.connect(recipient).revokeRole(lockId, ROLE, recipient.address)) + .to.emit(ERC7589RolesRegistry, 'RoleRevoked') + .withArgs(lockId, ROLE, recipient.address) + }) + + it('should revoke role when sender is approved by recipient', async () => { + await expect( + ERC7589RolesRegistry.connect(recipient).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ).to.not.be.reverted + await expect( + ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address), + ) + .to.emit(ERC7589RolesRegistry, 'RoleRevoked') + .withArgs(lockId, ROLE, recipient.address) + }) + + it('should revoke role when sender is owner (and role is revocable)', async () => { + await expect(ERC7589RolesRegistry.connect(owner).revokeRole(lockId, ROLE, recipient.address)) + .to.emit(ERC7589RolesRegistry, 'RoleRevoked') + .withArgs(lockId, ROLE, recipient.address) + }) + + it('should revoke role when sender is approved by owner (and role is revocable)', async () => { + await expect( + ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ).to.not.be.reverted + + await expect( + ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address), + ) + .to.emit(ERC7589RolesRegistry, 'RoleRevoked') + .withArgs(lockId, ROLE, recipient.address) + }) + + it('should revoke role when sender is approved both by owner and recipient, and role not revocable', async () => { + await expect( + ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ).to.not.be.reverted + await expect( + ERC7589RolesRegistry.connect(recipient).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ).to.not.be.reverted + + await grantRole({ revocable: false }) + await expect( + ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address), + ) + .to.emit(ERC7589RolesRegistry, 'RoleRevoked') + .withArgs(lockId, ROLE, recipient.address) + }) + + it('should not delete original owner when revoking role', async () => { + expect(await ERC7589RolesRegistry.roleExpirationDate(lockId, ROLE)).to.be.greaterThan(ZERO) + + await expect(ERC7589RolesRegistry.connect(owner).revokeRole(lockId, ROLE, recipient.address)) + .to.emit(ERC7589RolesRegistry, 'RoleRevoked') + .withArgs(lockId, ROLE, recipient.address) + + expect(await ERC7589RolesRegistry.ownerOf(lockId)).to.be.equal(owner.address) + expect(await ERC7589RolesRegistry.roleExpirationDate(lockId, ROLE)).to.be.equal(ZERO) + }) + + }) + + describe('unlockTokens', async () => { + + beforeEach(async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted + await lockTokens(owner) + await grantRole() + }) + + it('should revert if lockId does not exist', async () => { + await expect( + ERC7589RolesRegistry.connect(recipient).unlockTokens(BigNumber.from(2)), + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not owner or approved') + }) + + it('should revert if sender is not owner or approved', async () => { + await expect( + ERC7589RolesRegistry.connect(anotherUser).unlockTokens(lockId), + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not owner or approved') + }) + + it('should revert if NFT is locked', async () => { + await grantRole({ revocable: false }) + + await expect(ERC7589RolesRegistry.unlockTokens(lockId)).to.be.revertedWith( + 'ERC7589RolesRegistry: NFT is locked', + ) + }) + + it('should unlock token if sender is owner and NFT is not locked', async () => { + await expect(ERC7589RolesRegistry.connect(owner).unlockTokens(lockId)) + .to.emit(ERC7589RolesRegistry, 'TokensUnlocked') + .withArgs(lockId) + .to.emit(MockErc1155Token, 'TransferSingle') + .withArgs(ERC7589RolesRegistry.address, ERC7589RolesRegistry.address, owner.address, tokenId, tokenAmount) + }) + + it('should unlock token if sender is approved and NFT is not locked', async () => { + await expect( + ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ).to.not.be.reverted + + await expect(ERC7589RolesRegistry.connect(anotherUser).unlockTokens(lockId)) + .to.emit(ERC7589RolesRegistry, 'TokensUnlocked') + .withArgs(lockId) + .to.emit(MockErc1155Token, 'TransferSingle') + .withArgs(ERC7589RolesRegistry.address, ERC7589RolesRegistry.address, owner.address, tokenId, tokenAmount) + }) + + }) + + describe('setRoleApprovalForAll', async () => { + + it('should approve and revoke role approval for all', async () => { + expect(await ERC7589RolesRegistry.isRoleApprovedForAll(AddressZero, owner.address, anotherUser.address)).to.be.false + expect(await ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(AddressZero, anotherUser.address, true)) + .to.emit(ERC7589RolesRegistry, 'RoleApprovalForAll') + .withArgs(AddressZero, owner.address, anotherUser.address, true) + expect(await ERC7589RolesRegistry.isRoleApprovedForAll(AddressZero, owner.address, anotherUser.address)).to.be.true + }) + + }) + + describe('View Functions', async () => { + + describe('when role is expired or does not exist', async () => { + + it('roleData should return default value', async () => { + expect(await ERC7589RolesRegistry.roleData(lockId, ROLE)).to.be.equal('0x') + }) + + it('roleExpirationDate should return default value', async () => { + expect(await ERC7589RolesRegistry.roleExpirationDate(lockId, ROLE)).to.be.equal(0) + }) + + it('isRoleRevocable should return default value', async () => { + expect(await ERC7589RolesRegistry.isRoleRevocable(lockId, ROLE)).to.be.false + }) + + }) + + describe('when role exists and is not expired', async () => { + + beforeEach(async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted + await lockTokens(owner) + await grantRole() + }) + + it('ownerOf should return owner', async () => { + expect(await ERC7589RolesRegistry.ownerOf(lockId)).to.be.equal(owner.address) + }) + + it('tokenAddressOf should return tokenAddress', async () => { + expect(await ERC7589RolesRegistry.tokenAddressOf(lockId)).to.be.equal(MockErc1155Token.address) + }) + + it('tokenIdOf should return tokenId', async () => { + expect(await ERC7589RolesRegistry.tokenIdOf(lockId)).to.be.equal(tokenId) + }) + + it('tokenAmountOf should return tokenAmount', async () => { + expect(await ERC7589RolesRegistry.tokenAmountOf(lockId)).to.be.equal(tokenAmount) + }) + + it('roleData should return custom data', async () => { + expect(await ERC7589RolesRegistry.roleData(lockId, ROLE)).to.be.equal(HashZero) + }) + + it('roleExpirationDate should return the expiration date', async () => { + expect(await ERC7589RolesRegistry.roleExpirationDate(lockId, ROLE)).to.be.equal(expirationDate) + }) + + it('isRoleRevocable should return whether the role is revocable', async () => { + expect(await ERC7589RolesRegistry.isRoleRevocable(lockId, ROLE)).to.be.true + }) + + }) + + }) + + describe('Optional Extensions', async () => { + + describe('IERC7589LockTokensAndGrantRoleExtension', async () => { + + it('should lock tokens and grant role in a single transaction', async () => { + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)) + .to.not.be.reverted + + await expect( + ERC7589RolesRegistry.connect(owner).lockTokensAndGrantRole( + owner.address, + MockErc1155Token.address, + tokenId, + tokenAmount, + ROLE, + recipient.address, + expirationDate, + true, + HashZero + ) + ) + .to.emit(ERC7589RolesRegistry, 'TokensLocked') + .withArgs(owner.address, 1, MockErc1155Token.address, tokenId, tokenAmount) + .to.emit(MockErc1155Token, 'TransferSingle') + .withArgs(ERC7589RolesRegistry.address, owner.address, ERC7589RolesRegistry.address, tokenId, tokenAmount) + .to.emit(ERC7589RolesRegistry, 'RoleGranted') + .withArgs(lockId, ROLE, recipient.address, expirationDate, true, HashZero) + }) + + }) + + }) + + describe('Manager Functions', async () => { + + it('should revert if not manager', async () => { + await expect( + ERC7589RolesRegistry.connect(anotherUser).setTokenAddressAllowed(MockErc1155Token.address, true) + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not manager') + await expect( + ERC7589RolesRegistry.connect(anotherUser).setManagerAddress(anotherUser.address) + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not manager') + await expect( + ERC7589RolesRegistry.connect(anotherUser).setMarketplaceAddress(anotherUser.address) + ).to.be.revertedWith('ERC7589RolesRegistry: sender is not manager') + }) + + it('should transfer manager role', async () => { + expect(await ERC7589RolesRegistry.managerAddress()).to.be.equal(owner.address) + await expect(ERC7589RolesRegistry.setManagerAddress(anotherUser.address)).to.not.be.reverted + expect(await ERC7589RolesRegistry.managerAddress()).to.be.equal(anotherUser.address) + }) + + it('should set marketplace address', async () => { + expect(await ERC7589RolesRegistry.marketplaceAddress()).to.be.equal(AddressZero) + await expect(ERC7589RolesRegistry.setMarketplaceAddress(anotherUser.address)).to.not.be.reverted + expect(await ERC7589RolesRegistry.marketplaceAddress()).to.be.equal(anotherUser.address) + }) + + it('should set tokenAddress allowed status', async () => { + expect(await ERC7589RolesRegistry.isTokenAddressAllowed(AddressZero)).to.be.false + await expect(ERC7589RolesRegistry.setTokenAddressAllowed(AddressZero, true)).to.not.be.reverted + expect(await ERC7589RolesRegistry.isTokenAddressAllowed(AddressZero)).to.be.true + await expect(ERC7589RolesRegistry.setTokenAddressAllowed(AddressZero, false)).to.not.be.reverted + expect(await ERC7589RolesRegistry.isTokenAddressAllowed(AddressZero)).to.be.false + }) + + }) + + describe('ERC-165 supportsInterface', async () => { + it('should return true when IERC1155Receiver identifier is provided', async () => { + const iface = IERC1155Receiver__factory.createInterface() + const ifaceId = generateErc165InterfaceId(iface) + expect(await ERC7589RolesRegistry.supportsInterface(ifaceId)).to.be.true + }) + + it('should return true when IERC7589 identifier is provided', async () => { + const iface = IERC7589__factory.createInterface() + const ifaceId = generateErc165InterfaceId(iface) + expect(await ERC7589RolesRegistry.supportsInterface(ifaceId)).to.be.true + }) + + it('should return true when IERC7589LockTokensAndGrantRoleExtension identifier is provided', async () => { + const iface = IERC7589LockTokensAndGrantRoleExtension__factory.createInterface() + const ifaceId = generateErc165InterfaceId(iface) + expect(await ERC7589RolesRegistry.supportsInterface(ifaceId)).to.be.true + }) + + }) +}) diff --git a/test/helpers.ts b/test/helpers.ts index 2c07227..726337c 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -1,7 +1,8 @@ -import { Contract, ethers } from 'ethers' +import { BigNumber, Contract, ethers } from 'ethers' import { expect } from 'chai' import { solidityKeccak256 } from 'ethers/lib/utils' +export const ZERO = BigNumber.from(0) export const ONE_DAY = 60 * 60 * 24 export const THREE_MONTHS = ONE_DAY * 30 * 3 export const ROLE = generateRoleId('UNIQUE_ROLE') From 81ad1bba552034966dab99040f18532f2d936e40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Sat, 29 Jun 2024 17:16:54 -0400 Subject: [PATCH 2/5] ON-876: fix lint --- contracts/ERC7589/ERC7589RolesRegistry.sol | 18 ++-- test/ERC7589/ERC7589RolesRegistry.spec.ts | 117 ++++++++------------- 2 files changed, 55 insertions(+), 80 deletions(-) diff --git a/contracts/ERC7589/ERC7589RolesRegistry.sol b/contracts/ERC7589/ERC7589RolesRegistry.sol index c4e5084..65fc9f6 100644 --- a/contracts/ERC7589/ERC7589RolesRegistry.sol +++ b/contracts/ERC7589/ERC7589RolesRegistry.sol @@ -74,7 +74,8 @@ contract ERC7589RolesRegistry is IERC7589, ERC1155Holder, IERC7589LockTokensAndG modifier onlyTokenLockOwnerOrApproved(uint256 _lockId) { TokenLock storage _tokenLock = tokenLocks[_lockId]; require( - msg.sender == _tokenLock.owner || isRoleApprovedForAll(_tokenLock.tokenAddress, _tokenLock.owner, msg.sender), + msg.sender == _tokenLock.owner || + isRoleApprovedForAll(_tokenLock.tokenAddress, _tokenLock.owner, msg.sender), 'ERC7589RolesRegistry: sender is not owner or approved' ); _; @@ -110,9 +111,7 @@ contract ERC7589RolesRegistry is IERC7589, ERC1155Holder, IERC7589LockTokensAndG _grantERC7589Role(_lockId, _roleId, _recipient, _expirationDate, _revocable, _data); } - function revokeRole( - uint256 _lockId, bytes32 _roleId, address _recipient - ) external { + function revokeRole(uint256 _lockId, bytes32 _roleId, address _recipient) external { TokenLock storage _tokenLock = tokenLocks[_lockId]; Role storage _role = roles[_lockId][_roleId]; require(_role.expirationDate > block.timestamp, 'ERC7589RolesRegistry: role does not exist'); @@ -146,9 +145,7 @@ contract ERC7589RolesRegistry is IERC7589, ERC1155Holder, IERC7589LockTokensAndG delete tokenLocks[_lockId]; emit TokensUnlocked(_lockId); - _transferFrom( - address(this), _owner, _tokenAddress, _tokenId, _tokenAmount - ); + _transferFrom(address(this), _owner, _tokenAddress, _tokenId, _tokenAmount); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _approved) external { @@ -248,7 +245,12 @@ contract ERC7589RolesRegistry is IERC7589, ERC1155Holder, IERC7589LockTokensAndG address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount - ) private onlyAllowedTokenAddress(_tokenAddress) onlyOwnerOrApproved(_owner, _tokenAddress) returns (uint256 lockId_) { + ) + private + onlyAllowedTokenAddress(_tokenAddress) + onlyOwnerOrApproved(_owner, _tokenAddress) + returns (uint256 lockId_) + { require(_tokenAmount > 0, 'ERC7589RolesRegistry: tokenAmount must be greater than zero'); lockId_ = ++lockIdCount; tokenLocks[lockId_] = TokenLock(_owner, _tokenAddress, _tokenId, _tokenAmount); diff --git a/test/ERC7589/ERC7589RolesRegistry.spec.ts b/test/ERC7589/ERC7589RolesRegistry.spec.ts index ac9595b..57f61d7 100644 --- a/test/ERC7589/ERC7589RolesRegistry.spec.ts +++ b/test/ERC7589/ERC7589RolesRegistry.spec.ts @@ -22,7 +22,7 @@ describe('ERC7589RolesRegistry', async () => { let tokenId: BigNumber let tokenAmount: BigNumber let expirationDate: number - let lockId = BigNumber.from(1) + const lockId = BigNumber.from(1) async function deployContracts() { const ERC7589RolesRegistryFactory = await ethers.getContractFactory('ERC7589RolesRegistry') @@ -127,15 +127,12 @@ describe('ERC7589RolesRegistry', async () => { it('should lock tokens when sender is marketplace contract', async () => { await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be .reverted - await expect( - ERC7589RolesRegistry.connect(owner).setMarketplaceAddress(anotherUser.address), - ).to.not.be.reverted + await expect(ERC7589RolesRegistry.connect(owner).setMarketplaceAddress(anotherUser.address)).to.not.be.reverted await lockTokens(anotherUser) }) }) describe('grantRole', async () => { - beforeEach(async () => { await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be .reverted @@ -172,14 +169,7 @@ describe('ERC7589RolesRegistry', async () => { it('should revert when previous role is not revocable or expired', async () => { await grantRole({ revocable: false }) await expect( - ERC7589RolesRegistry.connect(owner).grantRole( - lockId, - ROLE, - recipient.address, - expirationDate, - false, - HashZero, - ), + ERC7589RolesRegistry.connect(owner).grantRole(lockId, ROLE, recipient.address, expirationDate, false, HashZero), ).to.be.revertedWith('ERC7589RolesRegistry: role is not expired nor revocable') }) @@ -201,7 +191,6 @@ describe('ERC7589RolesRegistry', async () => { }) describe('revokeRole', async () => { - beforeEach(async () => { await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be .reverted @@ -217,18 +206,18 @@ describe('ERC7589RolesRegistry', async () => { it('should revert when sender is owner but role is not revocable nor expired', async () => { await grantRole({ revocable: false }) - await expect( - ERC7589RolesRegistry.connect(owner).revokeRole(lockId, ROLE, recipient.address), - ).to.be.revertedWith('ERC7589RolesRegistry: role is not revocable or caller is not the approved') + await expect(ERC7589RolesRegistry.connect(owner).revokeRole(lockId, ROLE, recipient.address)).to.be.revertedWith( + 'ERC7589RolesRegistry: role is not revocable or caller is not the approved', + ) }) it('should revert when role is expired', async () => { await grantRole() await time.increase(ONE_DAY) - await expect( - ERC7589RolesRegistry.revokeRole(lockId, ROLE, recipient.address), - ).to.be.revertedWith('ERC7589RolesRegistry: role does not exist') + await expect(ERC7589RolesRegistry.revokeRole(lockId, ROLE, recipient.address)).to.be.revertedWith( + 'ERC7589RolesRegistry: role does not exist', + ) }) it('should revert when role was already revoked', async () => { @@ -236,9 +225,9 @@ describe('ERC7589RolesRegistry', async () => { .to.emit(ERC7589RolesRegistry, 'RoleRevoked') .withArgs(lockId, ROLE, recipient.address) - await expect( - ERC7589RolesRegistry.revokeRole(lockId, ROLE, recipient.address), - ).to.be.revertedWith('ERC7589RolesRegistry: role does not exist') + await expect(ERC7589RolesRegistry.revokeRole(lockId, ROLE, recipient.address)).to.be.revertedWith( + 'ERC7589RolesRegistry: role does not exist', + ) }) it('should revoke role when sender is recipient', async () => { @@ -249,11 +238,13 @@ describe('ERC7589RolesRegistry', async () => { it('should revoke role when sender is approved by recipient', async () => { await expect( - ERC7589RolesRegistry.connect(recipient).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ERC7589RolesRegistry.connect(recipient).setRoleApprovalForAll( + MockErc1155Token.address, + anotherUser.address, + true, + ), ).to.not.be.reverted - await expect( - ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address), - ) + await expect(ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address)) .to.emit(ERC7589RolesRegistry, 'RoleRevoked') .withArgs(lockId, ROLE, recipient.address) }) @@ -269,9 +260,7 @@ describe('ERC7589RolesRegistry', async () => { ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), ).to.not.be.reverted - await expect( - ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address), - ) + await expect(ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address)) .to.emit(ERC7589RolesRegistry, 'RoleRevoked') .withArgs(lockId, ROLE, recipient.address) }) @@ -281,13 +270,15 @@ describe('ERC7589RolesRegistry', async () => { ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), ).to.not.be.reverted await expect( - ERC7589RolesRegistry.connect(recipient).setRoleApprovalForAll(MockErc1155Token.address, anotherUser.address, true), + ERC7589RolesRegistry.connect(recipient).setRoleApprovalForAll( + MockErc1155Token.address, + anotherUser.address, + true, + ), ).to.not.be.reverted await grantRole({ revocable: false }) - await expect( - ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address), - ) + await expect(ERC7589RolesRegistry.connect(anotherUser).revokeRole(lockId, ROLE, recipient.address)) .to.emit(ERC7589RolesRegistry, 'RoleRevoked') .withArgs(lockId, ROLE, recipient.address) }) @@ -302,11 +293,9 @@ describe('ERC7589RolesRegistry', async () => { expect(await ERC7589RolesRegistry.ownerOf(lockId)).to.be.equal(owner.address) expect(await ERC7589RolesRegistry.roleExpirationDate(lockId, ROLE)).to.be.equal(ZERO) }) - }) describe('unlockTokens', async () => { - beforeEach(async () => { await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be .reverted @@ -315,23 +304,21 @@ describe('ERC7589RolesRegistry', async () => { }) it('should revert if lockId does not exist', async () => { - await expect( - ERC7589RolesRegistry.connect(recipient).unlockTokens(BigNumber.from(2)), - ).to.be.revertedWith('ERC7589RolesRegistry: sender is not owner or approved') + await expect(ERC7589RolesRegistry.connect(recipient).unlockTokens(BigNumber.from(2))).to.be.revertedWith( + 'ERC7589RolesRegistry: sender is not owner or approved', + ) }) it('should revert if sender is not owner or approved', async () => { - await expect( - ERC7589RolesRegistry.connect(anotherUser).unlockTokens(lockId), - ).to.be.revertedWith('ERC7589RolesRegistry: sender is not owner or approved') + await expect(ERC7589RolesRegistry.connect(anotherUser).unlockTokens(lockId)).to.be.revertedWith( + 'ERC7589RolesRegistry: sender is not owner or approved', + ) }) it('should revert if NFT is locked', async () => { await grantRole({ revocable: false }) - await expect(ERC7589RolesRegistry.unlockTokens(lockId)).to.be.revertedWith( - 'ERC7589RolesRegistry: NFT is locked', - ) + await expect(ERC7589RolesRegistry.unlockTokens(lockId)).to.be.revertedWith('ERC7589RolesRegistry: NFT is locked') }) it('should unlock token if sender is owner and NFT is not locked', async () => { @@ -353,25 +340,22 @@ describe('ERC7589RolesRegistry', async () => { .to.emit(MockErc1155Token, 'TransferSingle') .withArgs(ERC7589RolesRegistry.address, ERC7589RolesRegistry.address, owner.address, tokenId, tokenAmount) }) - }) describe('setRoleApprovalForAll', async () => { - it('should approve and revoke role approval for all', async () => { - expect(await ERC7589RolesRegistry.isRoleApprovedForAll(AddressZero, owner.address, anotherUser.address)).to.be.false + expect(await ERC7589RolesRegistry.isRoleApprovedForAll(AddressZero, owner.address, anotherUser.address)).to.be + .false expect(await ERC7589RolesRegistry.connect(owner).setRoleApprovalForAll(AddressZero, anotherUser.address, true)) .to.emit(ERC7589RolesRegistry, 'RoleApprovalForAll') .withArgs(AddressZero, owner.address, anotherUser.address, true) - expect(await ERC7589RolesRegistry.isRoleApprovedForAll(AddressZero, owner.address, anotherUser.address)).to.be.true + expect(await ERC7589RolesRegistry.isRoleApprovedForAll(AddressZero, owner.address, anotherUser.address)).to.be + .true }) - }) describe('View Functions', async () => { - describe('when role is expired or does not exist', async () => { - it('roleData should return default value', async () => { expect(await ERC7589RolesRegistry.roleData(lockId, ROLE)).to.be.equal('0x') }) @@ -383,11 +367,9 @@ describe('ERC7589RolesRegistry', async () => { it('isRoleRevocable should return default value', async () => { expect(await ERC7589RolesRegistry.isRoleRevocable(lockId, ROLE)).to.be.false }) - }) describe('when role exists and is not expired', async () => { - beforeEach(async () => { await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be .reverted @@ -422,18 +404,14 @@ describe('ERC7589RolesRegistry', async () => { it('isRoleRevocable should return whether the role is revocable', async () => { expect(await ERC7589RolesRegistry.isRoleRevocable(lockId, ROLE)).to.be.true }) - }) - }) describe('Optional Extensions', async () => { - describe('IERC7589LockTokensAndGrantRoleExtension', async () => { - it('should lock tokens and grant role in a single transaction', async () => { - await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)) - .to.not.be.reverted + await expect(MockErc1155Token.connect(owner).setApprovalForAll(ERC7589RolesRegistry.address, true)).to.not.be + .reverted await expect( ERC7589RolesRegistry.connect(owner).lockTokensAndGrantRole( @@ -445,8 +423,8 @@ describe('ERC7589RolesRegistry', async () => { recipient.address, expirationDate, true, - HashZero - ) + HashZero, + ), ) .to.emit(ERC7589RolesRegistry, 'TokensLocked') .withArgs(owner.address, 1, MockErc1155Token.address, tokenId, tokenAmount) @@ -455,22 +433,19 @@ describe('ERC7589RolesRegistry', async () => { .to.emit(ERC7589RolesRegistry, 'RoleGranted') .withArgs(lockId, ROLE, recipient.address, expirationDate, true, HashZero) }) - }) - }) describe('Manager Functions', async () => { - it('should revert if not manager', async () => { await expect( - ERC7589RolesRegistry.connect(anotherUser).setTokenAddressAllowed(MockErc1155Token.address, true) - ).to.be.revertedWith('ERC7589RolesRegistry: sender is not manager') - await expect( - ERC7589RolesRegistry.connect(anotherUser).setManagerAddress(anotherUser.address) + ERC7589RolesRegistry.connect(anotherUser).setTokenAddressAllowed(MockErc1155Token.address, true), ).to.be.revertedWith('ERC7589RolesRegistry: sender is not manager') + await expect(ERC7589RolesRegistry.connect(anotherUser).setManagerAddress(anotherUser.address)).to.be.revertedWith( + 'ERC7589RolesRegistry: sender is not manager', + ) await expect( - ERC7589RolesRegistry.connect(anotherUser).setMarketplaceAddress(anotherUser.address) + ERC7589RolesRegistry.connect(anotherUser).setMarketplaceAddress(anotherUser.address), ).to.be.revertedWith('ERC7589RolesRegistry: sender is not manager') }) @@ -493,7 +468,6 @@ describe('ERC7589RolesRegistry', async () => { await expect(ERC7589RolesRegistry.setTokenAddressAllowed(AddressZero, false)).to.not.be.reverted expect(await ERC7589RolesRegistry.isTokenAddressAllowed(AddressZero)).to.be.false }) - }) describe('ERC-165 supportsInterface', async () => { @@ -514,6 +488,5 @@ describe('ERC7589RolesRegistry', async () => { const ifaceId = generateErc165InterfaceId(iface) expect(await ERC7589RolesRegistry.supportsInterface(ifaceId)).to.be.true }) - }) }) From 84b8270771ae80b0f9aab76d60bfe458b84718f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Sat, 29 Jun 2024 17:22:15 -0400 Subject: [PATCH 3/5] ON-876: removed unused extension --- contracts/ERC7589/ERC7589RolesRegistry.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/ERC7589/ERC7589RolesRegistry.sol b/contracts/ERC7589/ERC7589RolesRegistry.sol index 65fc9f6..3219996 100644 --- a/contracts/ERC7589/ERC7589RolesRegistry.sol +++ b/contracts/ERC7589/ERC7589RolesRegistry.sol @@ -7,7 +7,6 @@ import { IERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/IERC1155 import { ERC1155Holder, ERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol'; import { IERC7589 } from '../interfaces/IERC7589.sol'; import { IERC1155 } from '@openzeppelin/contracts/token/ERC1155/IERC1155.sol'; -import { IERC7589RoleBalanceOfExtension } from '../interfaces/IERC7589RoleBalanceOfExtension.sol'; import { IERC7589LockTokensAndGrantRoleExtension } from '../interfaces/IERC7589LockTokensAndGrantRoleExtension.sol'; import { Uint64SortedLinkedListLibrary } from '../libraries/Uint64SortedLinkedListLibrary.sol'; import { IOriumWrapperManager } from '../interfaces/IOriumWrapperManager.sol'; From 87cc8269a53f90253f75cbce94d887b9b3f50623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Fri, 5 Jul 2024 11:12:45 -0400 Subject: [PATCH 4/5] ON-876: fixed checks, effects and interactions, also included deploy script --- contracts/ERC7589/ERC7589RolesRegistry.sol | 5 +- .../ERC7589RolesRegistry/01-deploy.ts | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts diff --git a/contracts/ERC7589/ERC7589RolesRegistry.sol b/contracts/ERC7589/ERC7589RolesRegistry.sol index 3219996..b050625 100644 --- a/contracts/ERC7589/ERC7589RolesRegistry.sol +++ b/contracts/ERC7589/ERC7589RolesRegistry.sol @@ -142,9 +142,8 @@ contract ERC7589RolesRegistry is IERC7589, ERC1155Holder, IERC7589LockTokensAndG uint256 _tokenId = tokenLocks[_lockId].tokenId; uint256 _tokenAmount = tokenLocks[_lockId].tokenAmount; delete tokenLocks[_lockId]; - emit TokensUnlocked(_lockId); - _transferFrom(address(this), _owner, _tokenAddress, _tokenId, _tokenAmount); + emit TokensUnlocked(_lockId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _approved) external { @@ -253,8 +252,8 @@ contract ERC7589RolesRegistry is IERC7589, ERC1155Holder, IERC7589LockTokensAndG require(_tokenAmount > 0, 'ERC7589RolesRegistry: tokenAmount must be greater than zero'); lockId_ = ++lockIdCount; tokenLocks[lockId_] = TokenLock(_owner, _tokenAddress, _tokenId, _tokenAmount); - emit TokensLocked(_owner, lockId_, _tokenAddress, _tokenId, _tokenAmount); _transferFrom(_owner, address(this), _tokenAddress, _tokenId, _tokenAmount); + emit TokensLocked(_owner, lockId_, _tokenAddress, _tokenId, _tokenAmount); } function _grantERC7589Role( diff --git a/scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts b/scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts new file mode 100644 index 0000000..5a293b6 --- /dev/null +++ b/scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts @@ -0,0 +1,53 @@ +import hre, { ethers, network } from 'hardhat' +import { AwsKmsSigner } from '@govtechsg/ethers-aws-kms-signer' +import { confirmOrDie, print, colors } from '../../../utils/misc' + +const kmsCredentials = { + accessKeyId: process.env.AWS_ACCESS_KEY_ID || 'AKIAxxxxxxxxxxxxxxxx', // credentials for your IAM user with KMS access + secretAccessKey: process.env.AWS_ACCESS_KEY_SECRET || 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', // credentials for your IAM user with KMS access + region: 'us-east-1', // region of your KMS key + keyId: process.env.AWS_KMS_KEY_ID || 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx', // KMS key id +} + +const NETWORK = network.name +const IMMUTABLE_CONTRACT_NAME = 'ERC7589RolesRegistry' +const MARKETPLACE_CONTRACT = '0xB1D47B09aa6D81d7B00C3A37705a6A157B83C49F' + +const networkConfig: any = network.config +const provider = new ethers.providers.JsonRpcProvider(networkConfig.url || '') + +const deployer = new AwsKmsSigner(kmsCredentials).connect(provider) + +async function main() { + const deployerAddress = await deployer.getAddress() + + /** Deploy ERC7589RolesRegistry **/ + + await confirmOrDie( + `Deploying ${IMMUTABLE_CONTRACT_NAME} contract on: ${NETWORK} network with ${deployerAddress}. Continue?`, + ) + + const ERC7589RolesRegistryFactory = await ethers.getContractFactory(IMMUTABLE_CONTRACT_NAME, { signer: deployer }) + const ERC7589RolesRegistry = await ERC7589RolesRegistryFactory.deploy(MARKETPLACE_CONTRACT, { + gasPrice: ethers.utils.parseUnits('100', 'gwei') + }) + await ERC7589RolesRegistry.deployed() + + console.log(`${IMMUTABLE_CONTRACT_NAME} deployed at: ${ERC7589RolesRegistry.address}`) + + print(colors.highlight, `Verifying contract ${IMMUTABLE_CONTRACT_NAME} on ${NETWORK}...`) + await hre.run('verify:verify', { + address: ERC7589RolesRegistry.address, + constructorArguments: [ MARKETPLACE_CONTRACT ], + }) + print(colors.success, `Contract ${IMMUTABLE_CONTRACT_NAME} verified!`) +} + +main() + .then(async () => { + console.log('All done!') + }) + .catch(error => { + console.error(error) + process.exitCode = 1 + }) From 5520a216a0bbec0e40ef8221550f27c27703ba29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Fri, 5 Jul 2024 11:15:04 -0400 Subject: [PATCH 5/5] ON-876: fixed lint --- scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts b/scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts index 5a293b6..5b31be3 100644 --- a/scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts +++ b/scripts/ERC-7589/ERC7589RolesRegistry/01-deploy.ts @@ -29,7 +29,7 @@ async function main() { const ERC7589RolesRegistryFactory = await ethers.getContractFactory(IMMUTABLE_CONTRACT_NAME, { signer: deployer }) const ERC7589RolesRegistry = await ERC7589RolesRegistryFactory.deploy(MARKETPLACE_CONTRACT, { - gasPrice: ethers.utils.parseUnits('100', 'gwei') + gasPrice: ethers.utils.parseUnits('100', 'gwei'), }) await ERC7589RolesRegistry.deployed() @@ -38,7 +38,7 @@ async function main() { print(colors.highlight, `Verifying contract ${IMMUTABLE_CONTRACT_NAME} on ${NETWORK}...`) await hre.run('verify:verify', { address: ERC7589RolesRegistry.address, - constructorArguments: [ MARKETPLACE_CONTRACT ], + constructorArguments: [MARKETPLACE_CONTRACT], }) print(colors.success, `Contract ${IMMUTABLE_CONTRACT_NAME} verified!`) }