diff --git a/contracts/NftRentalMarketplace.sol b/contracts/NftRentalMarketplace.sol index 880551b..fcd1ef8 100644 --- a/contracts/NftRentalMarketplace.sol +++ b/contracts/NftRentalMarketplace.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.9; import { IERC721 } from '@openzeppelin/contracts/token/ERC721/IERC721.sol'; import { IERC20 } from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; -import { IERC7432VaultExtension } from './interfaces/IERC7432VaultExtension.sol'; import { IERC7432 } from './interfaces/IERC7432.sol'; import { ERC165Checker } from '@openzeppelin/contracts/utils/introspection/ERC165Checker.sol'; import { IOriumMarketplaceRoyalties } from './interfaces/IOriumMarketplaceRoyalties.sol'; @@ -208,11 +207,7 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr address _rolesRegistry = IOriumMarketplaceRoyalties(oriumMarketplaceRoyalties).nftRolesRegistryOf( _offer.tokenAddress ); - require( - _rolesRegistry.supportsERC165InterfaceUnchecked(type(IERC7432VaultExtension).interfaceId), - 'NftRentalMarketplace: roles registry does not support IERC7432VaultExtension' - ); - IERC7432VaultExtension(_rolesRegistry).withdraw(_offer.tokenAddress, _offer.tokenId); + IERC7432(_rolesRegistry).unlockToken(_offer.tokenAddress, _offer.tokenId); } /** diff --git a/contracts/interfaces/IERC7432.sol b/contracts/interfaces/IERC7432.sol index d0f103d..beba086 100644 --- a/contracts/interfaces/IERC7432.sol +++ b/contracts/interfaces/IERC7432.sol @@ -6,7 +6,7 @@ import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol /// @title ERC-7432 Non-Fungible Token Roles /// @dev See https://eips.ethereum.org/EIPS/eip-7432 -/// Note: the ERC-165 identifier for this interface is 0xfecc9ed3. +/// Note: the ERC-165 identifier for this interface is 0xd00ca5cf. interface IERC7432 is IERC165 { struct Role { bytes32 roleId; @@ -20,6 +20,12 @@ interface IERC7432 is IERC165 { /** Events **/ + /// @notice Emitted when an NFT is locked (deposited or frozen). + /// @param _owner The owner of the NFT. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + event TokenLocked(address indexed _owner, address indexed _tokenAddress, uint256 _tokenId); + /// @notice Emitted when a role is granted. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. @@ -46,6 +52,12 @@ interface IERC7432 is IERC165 { /// @param _roleId The role identifier. event RoleRevoked(address indexed _tokenAddress, uint256 indexed _tokenId, bytes32 indexed _roleId); + /// @notice Emitted when an NFT is unlocked (withdrawn or unfrozen). + /// @param _owner The original owner of the NFT. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + event TokenUnlocked(address indexed _owner, address indexed _tokenAddress, uint256 indexed _tokenId); + /// @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. @@ -64,6 +76,12 @@ interface IERC7432 is IERC165 { /// @param _roleId The role identifier. function revokeRole(address _tokenAddress, uint256 _tokenId, bytes32 _roleId) external; + /// @notice Unlocks NFT (transfer back to original owner or unfreeze it). + /// @dev Reverts if sender is not approved or the original owner. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + function unlockToken(address _tokenAddress, uint256 _tokenId) 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. @@ -72,6 +90,12 @@ interface IERC7432 is IERC165 { /** View Functions **/ + /// @notice Retrieves the owner of NFT. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + /// @return owner_ The owner of the token. + function ownerOf(address _tokenAddress, uint256 _tokenId) external view returns (address owner_); + /// @notice Retrieves the recipient of an NFT role. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. diff --git a/contracts/interfaces/IERC7432VaultExtension.sol b/contracts/interfaces/IERC7432VaultExtension.sol deleted file mode 100644 index 1e56014..0000000 --- a/contracts/interfaces/IERC7432VaultExtension.sol +++ /dev/null @@ -1,32 +0,0 @@ -// SPDX-License-Identifier: CC0-1.0 - -pragma solidity 0.8.9; - -/// @title ERC-7432 Vault Extension -/// @dev See https://eips.ethereum.org/EIPS/eip-7432 -/// Note: the ERC-165 identifier for this interface is 0xecd7217f. -interface IERC7432VaultExtension { - /** Events **/ - - /// @notice Emitted when an NFT is withdrawn. - /// @param _owner The original owner of the NFT. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - event Withdraw(address indexed _owner, address indexed _tokenAddress, uint256 indexed _tokenId); - - /** External Functions **/ - - /// @notice Withdraw NFT back to original owner. - /// @dev Reverts if sender is not approved or the original owner. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - function withdraw(address _tokenAddress, uint256 _tokenId) external; - - /** View Functions **/ - - /// @notice Retrieves the owner of a deposited NFT. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - /// @return owner_ The owner of the token. - function ownerOf(address _tokenAddress, uint256 _tokenId) external view returns (address owner_); -} diff --git a/contracts/libraries/LibNftRentalMarketplace.sol b/contracts/libraries/LibNftRentalMarketplace.sol index 64df336..418adeb 100644 --- a/contracts/libraries/LibNftRentalMarketplace.sol +++ b/contracts/libraries/LibNftRentalMarketplace.sol @@ -5,7 +5,6 @@ pragma solidity 0.8.9; import { IERC721 } from '@openzeppelin/contracts/token/ERC721/IERC721.sol'; import { IERC20 } from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; import { IERC7432 } from '../interfaces/IERC7432.sol'; -import { IERC7432VaultExtension } from '../interfaces/IERC7432VaultExtension.sol'; import { IOriumMarketplaceRoyalties } from '../interfaces/IOriumMarketplaceRoyalties.sol'; /// @dev Rental offer info. @@ -57,7 +56,7 @@ library LibNftRentalMarketplace { address _nftOwner = IERC721(_offer.tokenAddress).ownerOf(_offer.tokenId); require( msg.sender == _nftOwner || - (msg.sender == IERC7432VaultExtension(_rolesRegistry).ownerOf(_offer.tokenAddress, _offer.tokenId) && + (msg.sender == IERC7432(_rolesRegistry).ownerOf(_offer.tokenAddress, _offer.tokenId) && _rolesRegistry == _nftOwner), 'NftRentalMarketplace: only token owner can call this function' ); @@ -289,10 +288,10 @@ library LibNftRentalMarketplace { _tokenAddresses[i] ); require( - msg.sender == IERC7432VaultExtension(_rolesRegistry).ownerOf(_tokenAddresses[i], _tokenIds[i]), + msg.sender == IERC7432(_rolesRegistry).ownerOf(_tokenAddresses[i], _tokenIds[i]), "OriumNftMarketplace: sender is not the token's owner" ); - IERC7432VaultExtension(_rolesRegistry).withdraw(_tokenAddresses[i], _tokenIds[i]); + IERC7432(_rolesRegistry).unlockToken(_tokenAddresses[i], _tokenIds[i]); } } @@ -309,7 +308,7 @@ library LibNftRentalMarketplace { require( msg.sender == IERC721(_params[i].tokenAddress).ownerOf(_params[i].tokenId) || msg.sender == - IERC7432VaultExtension(_rolesRegistry).ownerOf(_params[i].tokenAddress, _params[i].tokenId), + IERC7432(_rolesRegistry).ownerOf(_params[i].tokenAddress, _params[i].tokenId), 'OriumNftMarketplace: sender is not the owner' ); @@ -350,7 +349,7 @@ library LibNftRentalMarketplace { 'OriumNftMarketplace: role is expired' ); require( - msg.sender == IERC7432VaultExtension(_rolesRegistry).ownerOf(_tokenAddresses[i], _tokenIds[i]) || + msg.sender == IERC7432(_rolesRegistry).ownerOf(_tokenAddresses[i], _tokenIds[i]) || msg.sender == IERC7432(_rolesRegistry).recipientOf(_tokenAddresses[i], _tokenIds[i], _roleIds[i]), "OriumNftMarketplace: sender is not the token's owner or recipient" ); diff --git a/contracts/mocks/NftRolesRegistryVault.sol b/contracts/mocks/NftRolesRegistryVault.sol index 63fa748..9a97b3f 100644 --- a/contracts/mocks/NftRolesRegistryVault.sol +++ b/contracts/mocks/NftRolesRegistryVault.sol @@ -3,10 +3,9 @@ pragma solidity 0.8.9; import { IERC7432 } from '../interfaces/IERC7432.sol'; -import { IERC7432VaultExtension } from '../interfaces/IERC7432VaultExtension.sol'; import { IERC721 } from '@openzeppelin/contracts/token/ERC721/IERC721.sol'; -contract NftRolesRegistryVault is IERC7432, IERC7432VaultExtension { +contract NftRolesRegistryVault is IERC7432 { struct RoleData { address recipient; uint64 expirationDate; @@ -23,7 +22,7 @@ contract NftRolesRegistryVault is IERC7432, IERC7432VaultExtension { // owner => tokenAddress => operator => isApproved mapping(address => mapping(address => mapping(address => bool))) public tokenApprovals; - /** ERC-7432 External Functions **/ + /** External Functions **/ function grantRole(IERC7432.Role calldata _role) external override { require(_role.expirationDate > block.timestamp, 'NftRolesRegistryVault: expiration date must be in the future'); @@ -76,6 +75,21 @@ contract NftRolesRegistryVault is IERC7432, IERC7432VaultExtension { emit RoleRevoked(_tokenAddress, _tokenId, _roleId); } + function unlockToken(address _tokenAddress, uint256 _tokenId) external override { + address originalOwner = originalOwners[_tokenAddress][_tokenId]; + + require(_isLocked(_tokenAddress, _tokenId), 'NftRolesRegistryVault: NFT is locked'); + + require( + originalOwner == msg.sender || isRoleApprovedForAll(_tokenAddress, originalOwner, msg.sender), + 'NftRolesRegistryVault: sender must be owner or approved' + ); + + delete originalOwners[_tokenAddress][_tokenId]; + IERC721(_tokenAddress).transferFrom(address(this), originalOwner, _tokenId); + emit TokenUnlocked(originalOwner, _tokenAddress, _tokenId); + } + function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _approved) external override { tokenApprovals[msg.sender][_tokenAddress][_operator] = _approved; emit RoleApprovalForAll(_tokenAddress, _operator, _approved); @@ -83,6 +97,10 @@ contract NftRolesRegistryVault is IERC7432, IERC7432VaultExtension { /** ERC-7432 View Functions **/ + function ownerOf(address _tokenAddress, uint256 _tokenId) external view returns (address owner_) { + return originalOwners[_tokenAddress][_tokenId]; + } + function recipientOf( address _tokenAddress, uint256 _tokenId, @@ -134,31 +152,10 @@ contract NftRolesRegistryVault is IERC7432, IERC7432VaultExtension { return tokenApprovals[_owner][_tokenAddress][_operator]; } - /** ERC-7432 Vault Extension Functions **/ - - function withdraw(address _tokenAddress, uint256 _tokenId) external override { - address originalOwner = originalOwners[_tokenAddress][_tokenId]; - - require(_isWithdrawable(_tokenAddress, _tokenId), 'NftRolesRegistryVault: NFT is not withdrawable'); - - require( - originalOwner == msg.sender || isRoleApprovedForAll(_tokenAddress, originalOwner, msg.sender), - 'NftRolesRegistryVault: sender must be owner or approved' - ); - - delete originalOwners[_tokenAddress][_tokenId]; - IERC721(_tokenAddress).transferFrom(address(this), originalOwner, _tokenId); - emit Withdraw(originalOwner, _tokenAddress, _tokenId); - } - - function ownerOf(address _tokenAddress, uint256 _tokenId) external view returns (address owner_) { - return originalOwners[_tokenAddress][_tokenId]; - } - /** ERC-165 Functions **/ function supportsInterface(bytes4 interfaceId) external view virtual override returns (bool) { - return interfaceId == type(IERC7432).interfaceId || interfaceId == type(IERC7432VaultExtension).interfaceId; + return interfaceId == type(IERC7432).interfaceId; } /** Internal Functions **/ @@ -186,6 +183,7 @@ contract NftRolesRegistryVault is IERC7432, IERC7432VaultExtension { IERC721(_tokenAddress).transferFrom(_currentOwner, address(this), _tokenId); originalOwners[_tokenAddress][_tokenId] = _currentOwner; originalOwner_ = _currentOwner; + emit TokenLocked(_currentOwner, _tokenAddress, _tokenId); } } @@ -209,12 +207,12 @@ contract NftRolesRegistryVault is IERC7432, IERC7432VaultExtension { revert('NftRolesRegistryVault: role does not exist or sender is not approved'); } - /// @notice Check if an NFT is withdrawable. + /// @notice Checks if an NFT is locked. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. - /// @return True if the NFT is withdrawable. - function _isWithdrawable(address _tokenAddress, uint256 _tokenId) internal view returns (bool) { - // todo needs to implement a way to track expiration dates to make sure NFTs are withdrawable + /// @return True if the NFT is locked. + function _isLocked(address _tokenAddress, uint256 _tokenId) internal view returns (bool) { + // todo needs to implement a way to track expiration dates to make sure NFTs are not locked // mocked result return _isTokenDeposited(_tokenAddress, _tokenId); } diff --git a/test/NftRentalMarketplace.test.ts b/test/NftRentalMarketplace.test.ts index e19a9c8..7d948b8 100644 --- a/test/NftRentalMarketplace.test.ts +++ b/test/NftRentalMarketplace.test.ts @@ -574,17 +574,9 @@ describe('NftRentalMarketplace', () => { await expect(marketplace.connect(lender).cancelRentalOfferAndWithdraw(rentalOffer)) .to.emit(marketplace, 'RentalOfferCancelled') .withArgs(rentalOffer.lender, rentalOffer.nonce) - .to.emit(rolesRegistry, 'Withdraw') + .to.emit(rolesRegistry, 'TokenUnlocked') .withArgs(rentalOffer.lender, rentalOffer.tokenAddress, rentalOffer.tokenId) }) - it('Should NOT cancel a rental offer and withdraw from rolesRegistry, if rolesRegistry does not support IERC7432VaultExtension', async () => { - await marketplaceRoyalties - .connect(operator) - .setRolesRegistry(await mockERC721.getAddress(), AddressZero) - await expect(marketplace.connect(lender).cancelRentalOfferAndWithdraw(rentalOffer)).to.be.revertedWith( - 'NftRentalMarketplace: roles registry does not support IERC7432VaultExtension', - ) - }) it('Should NOT cancel a rental offer and withdraw if contract is paused', async () => { await marketplace.connect(operator).pause() await expect(marketplace.connect(lender).cancelRentalOfferAndWithdraw(rentalOffer)).to.be.revertedWith(