From c6b6b98c09f858d90eb897d5d3e76f01013760a3 Mon Sep 17 00:00:00 2001 From: Daniel Lima Date: Tue, 23 Apr 2024 16:53:44 -0300 Subject: [PATCH] ON-815: Direct Rentals --- contracts/NftRentalMarketplace.sol | 36 +++ .../libraries/LibNftRentalMarketplace.sol | 87 +++++++ test/NftRentalMarketplace.test.ts | 230 +++++++++++++++++- utils/types.ts | 10 + 4 files changed, 359 insertions(+), 4 deletions(-) diff --git a/contracts/NftRentalMarketplace.sol b/contracts/NftRentalMarketplace.sol index 47585c4..55ad0e5 100644 --- a/contracts/NftRentalMarketplace.sol +++ b/contracts/NftRentalMarketplace.sol @@ -5,6 +5,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'; @@ -242,6 +243,41 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr emit RentalEnded(_offer.lender, _offer.nonce); } + /** + * @notice Withdraws the NFT from roles registry. + * @dev Can only be called by the token owner. + * @param _tokenAddresses The NFT tokenAddresses. + * @param _tokenIds The NFT tokenIds. + */ + function batchWithdraw( + address[] calldata _tokenAddresses, + uint256[] calldata _tokenIds + ) external whenNotPaused { + LibNftRentalMarketplace.batchWithdraw(oriumMarketplaceRoyalties, _tokenAddresses, _tokenIds); + } + + /** + * @notice batchGrantRole grant role in a single transaction. + * @param _params The array of role params. + */ + function batchGrantRole(IERC7432.Role[] calldata _params) external whenNotPaused { + LibNftRentalMarketplace.batchGrantRole(_params, oriumMarketplaceRoyalties); + } + + /** + * @notice batchRevokeRole revokes role in a single transaction. + * @dev owner will be msg.sender and it must approve the marketplace to revoke the roles. + * @param _tokenAddresses The array of tokenAddresses + * @param _tokenIds The array of tokenIds + */ + function batchRevokeRole( + address[] memory _tokenAddresses, + uint256[] memory _tokenIds, + bytes32[] memory _roleIds + ) external whenNotPaused { + LibNftRentalMarketplace.batchRevokeRole(_tokenAddresses, _tokenIds, _roleIds, oriumMarketplaceRoyalties); + } + /** ######### Internals ########### **/ /** diff --git a/contracts/libraries/LibNftRentalMarketplace.sol b/contracts/libraries/LibNftRentalMarketplace.sol index 0117f79..e117084 100644 --- a/contracts/libraries/LibNftRentalMarketplace.sol +++ b/contracts/libraries/LibNftRentalMarketplace.sol @@ -270,4 +270,91 @@ library LibNftRentalMarketplace { IERC7432(_rolesRegsitry).revokeRole(_tokenAddress, _tokenId, _roleIds[i]); } } + + /** + * @notice Withdraws tokens from registry + * @dev Can only be called by the token owner. + * @param _oriumMarketplaceRoyaltiesAddress The address of the OriumMarketplaceRoyalties contract. + * @param _tokenAddresses The NFT tokenAddresses. + * @param _tokenIds The NFT tokenIds. + */ + function batchWithdraw( + address _oriumMarketplaceRoyaltiesAddress, + address[] calldata _tokenAddresses, + uint256[] calldata _tokenIds + ) external { + require(_tokenAddresses.length == _tokenIds.length, 'OriumNftMarketplace: arrays length mismatch'); + for (uint256 i = 0; i < _tokenAddresses.length; i++) { + address _rolesRegistry = IOriumMarketplaceRoyalties(_oriumMarketplaceRoyaltiesAddress).nftRolesRegistryOf( + _tokenAddresses[i] + ); + require( + msg.sender == IERC7432VaultExtension(_rolesRegistry).ownerOf(_tokenAddresses[i], _tokenIds[i]), + "OriumNftMarketplace: sender is not the token's owner" + ); + IERC7432VaultExtension(_rolesRegistry).withdraw(_tokenAddresses[i], _tokenIds[i]); + } + } + + /** + * @notice batchGrantRole grant role in a single transaction. + * @param _params The array of role params. + * @param _oriumMarketplaceRoyalties The Orium marketplace royalties contract address. + */ + function batchGrantRole(IERC7432.Role[] calldata _params, address _oriumMarketplaceRoyalties) external { + for (uint256 i = 0; i < _params.length; i++) { + address _rolesRegistry = IOriumMarketplaceRoyalties(_oriumMarketplaceRoyalties).nftRolesRegistryOf( + _params[i].tokenAddress + ); + require( + msg.sender == + IERC7432VaultExtension(_rolesRegistry).ownerOf(_params[i].tokenAddress, _params[i].tokenId) || + msg.sender == IERC721(_params[i].tokenAddress).ownerOf(_params[i].tokenId), + 'OriumNftMarketplace: sender is not the owner' + ); + + IERC7432(_rolesRegistry).grantRole(_params[i]); + } + } + + /** + * @notice batchRevokeRole revokes role in a single transaction. + * @dev only the owner and recipient can call this function. Be careful as the marketplace have approvals from other users. + * @param _tokenAddresses The array of tokenAddresses + * @param _tokenIds The array of tokenIds + * @param _roleIds The array of roleIds + * @param _oriumMarketplaceRoyalties The Orium marketplace royalties contract address. + */ + function batchRevokeRole( + address[] memory _tokenAddresses, + uint256[] memory _tokenIds, + bytes32[] memory _roleIds, + address _oriumMarketplaceRoyalties + ) external { + require( + _tokenIds.length == _tokenAddresses.length && _tokenIds.length == _roleIds.length, + 'OriumNftMarketplace: arrays length mismatch' + ); + + for (uint256 i = 0; i < _tokenIds.length; i++) { + address _rolesRegistry = IOriumMarketplaceRoyalties(_oriumMarketplaceRoyalties).nftRolesRegistryOf( + _tokenAddresses[i] + ); + require( + IERC7432(_rolesRegistry).isRoleRevocable(_tokenAddresses[i], _tokenIds[i], _roleIds[i]), + 'OriumNftMarketplace: role is not revocable' + ); + require( + IERC7432(_rolesRegistry).roleExpirationDate(_tokenAddresses[i], _tokenIds[i], _roleIds[i]) > + block.timestamp, + 'OriumNftMarketplace: role is expired' + ); + require( + msg.sender == IERC7432(_rolesRegistry).recipientOf(_tokenAddresses[i], _tokenIds[i], _roleIds[i]) || + msg.sender == IERC7432VaultExtension(_rolesRegistry).ownerOf(_tokenAddresses[i], _tokenIds[i]), + "OriumNftMarketplace: sender is not the token's owner or recipient" + ); + IERC7432(_rolesRegistry).revokeRole(_tokenAddresses[i], _tokenIds[i], _roleIds[i]); + } + } } diff --git a/test/NftRentalMarketplace.test.ts b/test/NftRentalMarketplace.test.ts index 1dc8079..570e5d4 100644 --- a/test/NftRentalMarketplace.test.ts +++ b/test/NftRentalMarketplace.test.ts @@ -3,10 +3,10 @@ import { ethers } from 'hardhat' import { loadFixture, time } from '@nomicfoundation/hardhat-network-helpers' import { expect } from 'chai' import { toWei } from '../utils/bignumber' -import { RentalOffer, RoyaltyInfo } from '../utils/types' +import { GrantRoleParams, RentalOffer, RoyaltyInfo } from '../utils/types' import { AddressZero, EMPTY_BYTES, ONE_DAY, ONE_HOUR, THREE_MONTHS } from '../utils/constants' import { randomBytes } from 'crypto' -import { USER_ROLE } from '../utils/roles' +import { UNIQUE_ROLE, USER_ROLE } from '../utils/roles' import { MockERC20, MockERC721, @@ -500,7 +500,7 @@ describe('NftRentalMarketplace', () => { }) it("Should NOT cancel a rental offer after deadline's expiration", async () => { // move forward in time to expire the offer - const blockTimestamp = (await ethers.provider.getBlock('latest'))?.timestamp + const blockTimestamp = await time.latest() const timeToMove = rentalOffer.deadline - Number(blockTimestamp) + 1 await ethers.provider.send('evm_increaseTime', [timeToMove]) @@ -553,7 +553,7 @@ describe('NftRentalMarketplace', () => { }) it('Should NOT end a rental if rental is expired', async () => { // move foward in time to expire the offer - const blockTimestamp = (await ethers.provider.getBlock('latest'))?.timestamp + const blockTimestamp = await time.latest() const timeToMove = rentalOffer.deadline - Number(blockTimestamp) + 1 await ethers.provider.send('evm_increaseTime', [timeToMove]) @@ -586,6 +586,228 @@ describe('NftRentalMarketplace', () => { ) }) }) + + describe('Batch Withdraw Tokens', async () => { + it('Should withdraw tokens after rental is ended and rental offer expired', async () => { + await marketplace.connect(borrower).endRental(rentalOffer) + await time.increase(ONE_DAY) + await expect( + marketplace.connect(lender).batchWithdraw([rentalOffer.tokenAddress], [rentalOffer.tokenId]), + ) + .to.emit(mockERC721, 'Transfer') + .withArgs(await rolesRegistry.getAddress(), rentalOffer.lender, rentalOffer.tokenId) + }) + it('Should NOT withdraw tokens if contract is paused', async () => { + await marketplace.connect(operator).pause() + await expect( + marketplace.connect(lender).batchWithdraw([rentalOffer.tokenAddress], [rentalOffer.tokenId]), + ).to.be.revertedWith('Pausable: paused') + }) + it('Should NOT withdraw tokens if array mismatch', async () => { + await expect( + marketplace.connect(lender).batchWithdraw([rentalOffer.tokenAddress], []), + ).to.be.revertedWith('OriumNftMarketplace: arrays length mismatch') + }) + it('Should NOT withdraw tokens if sender is not the owner', async () => { + await expect( + marketplace.connect(borrower).batchWithdraw([rentalOffer.tokenAddress], [rentalOffer.tokenId]), + ).to.be.revertedWith("OriumNftMarketplace: sender is not the token's owner") + }) + }) + }) + }) + }) + + describe('Direct Rentals', async () => { + describe('When tokens are not deposited', async () => { + describe('batchGrantRole', async () => { + let grantRoleParams: GrantRoleParams[] + beforeEach(async () => { + grantRoleParams = [ + { + tokenAddress: await mockERC721.getAddress(), + tokenId, + roleId: UNIQUE_ROLE, + recipient: borrower.address, + expirationDate: Number(await time.latest()) + ONE_DAY, + revocable: true, + data: EMPTY_BYTES, + }, + ] + await rolesRegistry + .connect(lender) + .setRoleApprovalForAll(await mockERC721.getAddress(), await marketplace.getAddress(), true) + await mockERC721.connect(lender).setApprovalForAll(await rolesRegistry.getAddress(), true) + }) + it('Should deposit tokens and grant role', async () => { + await expect(marketplace.connect(lender).batchGrantRole(grantRoleParams)) + .to.emit(rolesRegistry, 'RoleGranted') + .withArgs( + grantRoleParams[0].tokenAddress, + grantRoleParams[0].tokenId, + grantRoleParams[0].roleId, + lender.address, + grantRoleParams[0].recipient, + grantRoleParams[0].expirationDate, + grantRoleParams[0].revocable, + grantRoleParams[0].data, + ) + }) + it('Should only grant role when a tokenId is passed', async () => { + await expect(marketplace.connect(lender).batchGrantRole(grantRoleParams)) + .to.emit(rolesRegistry, 'RoleGranted') + .withArgs( + grantRoleParams[0].tokenAddress, + grantRoleParams[0].tokenId, + grantRoleParams[0].roleId, + lender.address, + grantRoleParams[0].recipient, + grantRoleParams[0].expirationDate, + grantRoleParams[0].revocable, + grantRoleParams[0].data, + ) + }) + it('Should NOT deposit tokens and grant role if contract is paused', async () => { + await marketplace.connect(operator).pause() + await expect(marketplace.connect(lender).batchGrantRole(grantRoleParams)).to.be.revertedWith( + 'Pausable: paused', + ) + }) + it('Should NOT deposit tokens and grant role if caller is not the owner of the tokenId', async () => { + await expect(marketplace.connect(borrower).batchGrantRole(grantRoleParams)).to.revertedWith( + 'OriumNftMarketplace: sender is not the owner', + ) + }) + }) + }) + + describe('When tokens are deposited', async () => { + let grantRoleParams: GrantRoleParams[] + beforeEach(async () => { + grantRoleParams = [ + { + tokenAddress: await mockERC721.getAddress(), + tokenId, + roleId: UNIQUE_ROLE, + recipient: borrower.address, + expirationDate: Number(await time.latest()) + ONE_DAY, + revocable: true, + data: EMPTY_BYTES, + }, + ] + await rolesRegistry + .connect(lender) + .setRoleApprovalForAll(await mockERC721.getAddress(), await marketplace.getAddress(), true) + await mockERC721.connect(lender).setApprovalForAll(await rolesRegistry.getAddress(), true) + await marketplace.connect(lender).batchGrantRole(grantRoleParams) + }) + describe('batchGrantRole', async () => { + it('Should grant role', async () => { + await time.increase(ONE_DAY) + grantRoleParams[0].expirationDate = Number(await time.latest()) + ONE_DAY + await expect(marketplace.connect(lender).batchGrantRole(grantRoleParams)) + .to.emit(rolesRegistry, 'RoleGranted') + .withArgs( + grantRoleParams[0].tokenAddress, + grantRoleParams[0].tokenId, + grantRoleParams[0].roleId, + lender.address, + grantRoleParams[0].recipient, + grantRoleParams[0].expirationDate, + grantRoleParams[0].revocable, + grantRoleParams[0].data, + ) + }) + }) + describe('batchRevokeRole', async () => { + it('Should batch revoke role', async () => { + await expect( + marketplace + .connect(lender) + .batchRevokeRole( + [grantRoleParams[0].tokenAddress], + [grantRoleParams[0].tokenId], + [grantRoleParams[0].roleId], + ), + ) + .to.emit(rolesRegistry, 'RoleRevoked') + .withArgs(grantRoleParams[0].tokenAddress, grantRoleParams[0].tokenId, grantRoleParams[0].roleId) + }) + it('Should NOT batch revoke role if contract is paused', async () => { + await marketplace.connect(operator).pause() + await expect( + marketplace + .connect(lender) + .batchRevokeRole( + [grantRoleParams[0].tokenAddress], + [grantRoleParams[0].tokenId], + [grantRoleParams[0].roleId], + ), + ).to.be.revertedWith('Pausable: paused') + }) + it("Should NOT batch revoke role if array's length are different", async () => { + await expect( + marketplace + .connect(lender) + .batchRevokeRole( + [grantRoleParams[0].tokenAddress], + [grantRoleParams[0].tokenId], + [grantRoleParams[0].roleId, grantRoleParams[0].roleId], + ), + ).to.be.revertedWith('OriumNftMarketplace: arrays length mismatch') + }) + it("Should batch revoke role if sender is token's recipient", async () => { + await expect( + marketplace + .connect(borrower) + .batchRevokeRole( + [grantRoleParams[0].tokenAddress], + [grantRoleParams[0].tokenId], + [grantRoleParams[0].roleId], + ), + ) + .to.emit(rolesRegistry, 'RoleRevoked') + .withArgs(grantRoleParams[0].tokenAddress, grantRoleParams[0].tokenId, grantRoleParams[0].roleId) + }) + it("Should NOT batch revoke role if sender is not token's owner neither recipient", async () => { + await expect( + marketplace + .connect(notOperator) + .batchRevokeRole( + [grantRoleParams[0].tokenAddress], + [grantRoleParams[0].tokenId], + [grantRoleParams[0].roleId], + ), + ).to.be.revertedWith("OriumNftMarketplace: sender is not the token's owner or recipient") + }) + it('Should NOT batch revoke role if role is not revocable', async () => { + grantRoleParams[0].revocable = false + await rolesRegistry.connect(lender).grantRole(grantRoleParams[0]) + await rolesRegistry + .connect(borrower) + .setRoleApprovalForAll(await mockERC721.getAddress(), await marketplace.getAddress(), true) + await expect( + marketplace + .connect(lender) + .batchRevokeRole( + [grantRoleParams[0].tokenAddress], + [grantRoleParams[0].tokenId], + [grantRoleParams[0].roleId], + ), + ).to.be.revertedWith('OriumNftMarketplace: role is not revocable') + }) + it('Should NOT batch revoke role if role is expired', async () => { + await time.increase(ONE_DAY) + await expect( + marketplace + .connect(lender) + .batchRevokeRole( + [grantRoleParams[0].tokenAddress], + [grantRoleParams[0].tokenId], + [grantRoleParams[0].roleId], + ), + ).to.be.revertedWith('OriumNftMarketplace: role is expired') + }) }) }) }) diff --git a/utils/types.ts b/utils/types.ts index 2e436b9..03b3b90 100644 --- a/utils/types.ts +++ b/utils/types.ts @@ -49,3 +49,13 @@ export interface CommitAndGrantRoleParams { revocable: boolean data: string } + +export interface GrantRoleParams { + roleId: string + tokenAddress: string + tokenId: number + recipient: string + expirationDate: number + revocable: boolean + data: string +}