Skip to content

Commit

Permalink
Merge pull request #46 from OriumNetwork/hotfix--ON-815
Browse files Browse the repository at this point in the history
ON-815: Update ERC7432 interface
  • Loading branch information
karacurt authored Apr 24, 2024
2 parents 94e916c + 3457faa commit 9d30bd1
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 85 deletions.
9 changes: 1 addition & 8 deletions contracts/NftRentalMarketplace.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ 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';
import { OwnableUpgradeable } from '@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol';
import { Initializable } from '@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol';
Expand All @@ -19,7 +17,6 @@ import { LibNftRentalMarketplace, RentalOffer, Rental } from './libraries/LibNft
* @author Orium Network Team - [email protected]
*/
contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgradeable {
using ERC165Checker for address;

/** ######### Global Variables ########### **/

Expand Down Expand Up @@ -208,11 +205,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);
}

/**
Expand Down
26 changes: 25 additions & 1 deletion contracts/interfaces/IERC7432.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down
32 changes: 0 additions & 32 deletions contracts/interfaces/IERC7432VaultExtension.sol

This file was deleted.

11 changes: 5 additions & 6 deletions contracts/libraries/LibNftRentalMarketplace.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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'
);
Expand Down Expand Up @@ -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]);
}
}

Expand All @@ -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'
);

Expand Down Expand Up @@ -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"
);
Expand Down
56 changes: 27 additions & 29 deletions contracts/mocks/NftRolesRegistryVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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');
Expand Down Expand Up @@ -76,13 +75,32 @@ 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);
}

/** 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,
Expand Down Expand Up @@ -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 **/
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
Expand Down
10 changes: 1 addition & 9 deletions test/NftRentalMarketplace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 9d30bd1

Please sign in to comment.