Skip to content

Commit

Permalink
feat: adding reentrancy ERC-721
Browse files Browse the repository at this point in the history
  • Loading branch information
@EduardoMelo00
EduardoMelo00 committed Aug 7, 2024
1 parent 6190a9e commit 938e11b
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 25 deletions.
58 changes: 43 additions & 15 deletions contracts/NftRentalMarketplace.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ import { IOriumMarketplaceRoyalties } from './interfaces/IOriumMarketplaceRoyalt
import { OwnableUpgradeable } from '@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol';
import { Initializable } from '@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol';
import { PausableUpgradeable } from '@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol';
import { ReentrancyGuardUpgradeable } from '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol';
import { LibNftRentalMarketplace, RentalOffer, Rental } from './libraries/LibNftRentalMarketplace.sol';

/**
* @title Orium NFT Marketplace - Marketplace for renting NFTs
* @dev This contract is used to manage NFTs rentals, powered by ERC-7432 Non-Fungible Token Roles
* @author Orium Network Team - [email protected]
*/
contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgradeable {
contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable {
/** ######### Global Variables ########### **/

/// @dev oriumMarketplaceRoyalties stores the collection royalties and fees
Expand Down Expand Up @@ -92,6 +93,7 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr
function initialize(address _owner, address _oriumMarketplaceRoyalties) external initializer {
__Pausable_init();
__Ownable_init();
__ReentrancyGuard_init();

oriumMarketplaceRoyalties = _oriumMarketplaceRoyalties;

Expand Down Expand Up @@ -146,7 +148,10 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr
* @param _offer The rental offer struct. It should be the same as the one used to create the offer.
* @param _duration The duration of the rental.
*/
function acceptRentalOffer(RentalOffer calldata _offer, uint64 _duration) external whenNotPaused {
function acceptRentalOffer(
RentalOffer calldata _offer,
uint64 _duration
) external payable whenNotPaused nonReentrant {
bytes32 _offerHash = LibNftRentalMarketplace.hashRentalOffer(_offer);
uint64 _expirationDate = uint64(block.timestamp + _duration);
LibNftRentalMarketplace.validateAcceptRentalOfferParams(
Expand All @@ -159,15 +164,38 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr
_expirationDate
);

LibNftRentalMarketplace.transferFees(
_offer.feeTokenAddress,
owner(),
_offer.lender,
oriumMarketplaceRoyalties,
_offer.tokenAddress,
_offer.feeAmountPerSecond,
_duration
);
if (_offer.feeTokenAddress == address(0)) {
uint256 totalFeeAmount = _offer.feeAmountPerSecond * _duration;
require(msg.value >= totalFeeAmount, 'NftRentalMarketplace: Incorrect native token amount');

uint256 marketplaceFeeAmount = LibNftRentalMarketplace.getAmountFromPercentage(
totalFeeAmount,
IOriumMarketplaceRoyalties(oriumMarketplaceRoyalties).marketplaceFeeOf(_offer.tokenAddress)
);
IOriumMarketplaceRoyalties.RoyaltyInfo memory royaltyInfo = IOriumMarketplaceRoyalties(
oriumMarketplaceRoyalties
).royaltyInfoOf(_offer.tokenAddress);

uint256 royaltyAmount = LibNftRentalMarketplace.getAmountFromPercentage(
totalFeeAmount,
royaltyInfo.royaltyPercentageInWei
);
uint256 lenderAmount = totalFeeAmount - marketplaceFeeAmount - royaltyAmount;

payable(owner()).transfer(marketplaceFeeAmount);
payable(royaltyInfo.treasury).transfer(royaltyAmount);
payable(_offer.lender).transfer(lenderAmount);
} else {
LibNftRentalMarketplace.transferFees(
_offer.feeTokenAddress,
owner(),
_offer.lender,
oriumMarketplaceRoyalties,
_offer.tokenAddress,
_offer.feeAmountPerSecond,
_duration
);
}

LibNftRentalMarketplace.grantRoles(
oriumMarketplaceRoyalties,
Expand All @@ -180,8 +208,8 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr
);

for (uint256 i = 0; i < _offer.roles.length; i++) {
if(_expirationDate > roleDeadline[_offer.roles[i]][_offer.tokenAddress][_offer.tokenId]) {
roleDeadline[_offer.roles[i]][_offer.tokenAddress][_offer.tokenId] = _expirationDate;
if (_expirationDate > roleDeadline[_offer.roles[i]][_offer.tokenAddress][_offer.tokenId]) {
roleDeadline[_offer.roles[i]][_offer.tokenAddress][_offer.tokenId] = _expirationDate;
}
}

Expand Down Expand Up @@ -233,7 +261,7 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr
_offer.tokenId,
_offer.roles
);

uint64 _offerDeadline = nonceDeadline[_offer.lender][_offer.nonce];
if (_offerDeadline < uint64(block.timestamp)) {
for (uint256 i = 0; i < _offer.roles.length; i++) {
Expand Down Expand Up @@ -268,6 +296,7 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr
* @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
* @param _roleIds The array of roleIds
*/
function batchRevokeRole(
address[] memory _tokenAddresses,
Expand All @@ -294,7 +323,6 @@ contract NftRentalMarketplace is Initializable, OwnableUpgradeable, PausableUpgr

nonceDeadline[msg.sender][_offer.nonce] = uint64(block.timestamp);
for (uint256 i = 0; i < _offer.roles.length; i++) {

if (rentals[_offerHash].expirationDate > uint64(block.timestamp)) {
roleDeadline[_offer.roles[i]][_offer.tokenAddress][_offer.tokenId] = rentals[_offerHash].expirationDate;
} else {
Expand Down
48 changes: 48 additions & 0 deletions test/NftRentalMarketplace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ describe('NftRentalMarketplace', () => {
.to.emit(marketplace, 'RentalStarted')
.withArgs(rentalOffer.lender, rentalOffer.nonce, borrower.address, expirationDate)
})

it('Should accept a rental offer more than once', async () => {
const rentalExpirationDate1 = Number(await time.latest()) + duration + 1

Expand Down Expand Up @@ -451,6 +452,50 @@ describe('NftRentalMarketplace', () => {
marketplace.connect(notOperator).acceptRentalOffer(rentalOffer, duration),
).to.be.revertedWith('NftRentalMarketplace: Sender is not allowed to rent this NFT')
})
it('Should revert when accepting a rental offer with insufficient native tokens', async () => {
await time.increase(ONE_DAY)
rentalOffer.nonce = `0x${randomBytes(32).toString('hex')}`
rentalOffer.borrower = borrower.address
rentalOffer.deadline = Number(await time.latest()) + ONE_DAY
rentalOffer.feeTokenAddress = AddressZero
rentalOffer.feeAmountPerSecond = toWei('0.0000001')
await marketplaceRoyalties
.connect(operator)
.setTrustedFeeTokenForToken([rentalOffer.tokenAddress], [AddressZero], [true])
await marketplace.connect(lender).createRentalOffer(rentalOffer)

const totalFeeAmount = rentalOffer.feeAmountPerSecond * BigInt(duration)

const insufficientAmount = totalFeeAmount - toWei('0.00000001') // slightly less than required
await expect(
marketplace.connect(borrower).acceptRentalOffer(rentalOffer, duration, {
value: insufficientAmount.toString(),
}),
).to.be.revertedWith('NftRentalMarketplace: Incorrect native token amount')
})
it('Should accept a rental offer with native tokens', async () => {
await time.increase(ONE_DAY)
rentalOffer.nonce = `0x${randomBytes(32).toString('hex')}`
rentalOffer.borrower = borrower.address
rentalOffer.deadline = Number(await time.latest()) + ONE_DAY
rentalOffer.feeTokenAddress = AddressZero
rentalOffer.feeAmountPerSecond = toWei('0.0000001')
await marketplaceRoyalties
.connect(operator)
.setTrustedFeeTokenForToken([rentalOffer.tokenAddress], [AddressZero], [true])
await marketplace.connect(lender).createRentalOffer(rentalOffer)

const blockTimestamp = await time.latest()
const expirationDate = blockTimestamp + duration + 1

await expect(
marketplace.connect(borrower).acceptRentalOffer(rentalOffer, duration, {
value: totalFeeAmount.toString(),
}),
)
.to.emit(marketplace, 'RentalStarted')
.withArgs(rentalOffer.lender, rentalOffer.nonce, borrower.address, expirationDate)
})
it('Should NOT accept a rental offer if offer is expired', async () => {
// move foward in time to expire the offer
const blockTimestamp = await time.latest()
Expand All @@ -473,6 +518,7 @@ describe('NftRentalMarketplace', () => {
marketplace.connect(borrower).acceptRentalOffer(rentalOffer, maxDuration),
).to.be.revertedWith('NftRentalMarketplace: expiration date is greater than offer deadline')
})

describe('Fees', async function () {
const feeAmountPerSecond = toWei('1')
const feeAmount = feeAmountPerSecond * BigInt(duration)
Expand All @@ -495,6 +541,7 @@ describe('NftRentalMarketplace', () => {
.withArgs(rentalOffer.lender, rentalOffer.nonce, borrower.address, expirationDate)
.to.emit(mockERC20, 'Transfer')
})

it('Should accept a rental offer if marketplace fee is 0', async () => {
await marketplaceRoyalties
.connect(operator)
Expand All @@ -504,6 +551,7 @@ describe('NftRentalMarketplace', () => {
'RentalStarted',
)
})

it('Should accept a rental offer if royalty fee is 0', async () => {
await marketplaceRoyalties
.connect(creator)
Expand Down
29 changes: 19 additions & 10 deletions test/OriumSftMarketplace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
OriumSftMarketplace,
SftRolesRegistrySingleRole,
SftRolesRegistrySingleRoleLegacy,
ReentrancyAttack,

Check warning on line 19 in test/OriumSftMarketplace.test.ts

View workflow job for this annotation

GitHub Actions / build_test_deploy 

'ReentrancyAttack' is defined but never used
} from '../typechain-types'

describe('OriumSftMarketplace', () => {
Expand Down Expand Up @@ -689,11 +690,6 @@ describe('OriumSftMarketplace', () => {
await marketplace.connect(lender).createRentalOffer({ ...rentalOffer, commitmentId: BigInt(0) })
rentalOffer.commitmentId = BigInt(2)

// Check the borrower's balance before the transaction
const borrowerBalanceBefore = await ethers.provider.getBalance(borrower.address)
console.log('BEFORE')
console.log(borrowerBalanceBefore)

const blockTimestamp = (await ethers.provider.getBlock('latest'))?.timestamp
const expirationDate = Number(blockTimestamp) + duration + 1

Expand All @@ -704,13 +700,26 @@ describe('OriumSftMarketplace', () => {
)
.to.emit(marketplace, 'RentalStarted')
.withArgs(rentalOffer.lender, rentalOffer.nonce, borrower.address, expirationDate)
})

// Check the borrower's balance after the transaction
const borrowerBalanceAfter = await ethers.provider.getBalance(borrower.address)
expect(borrowerBalanceAfter).to.be.lt(borrowerBalanceBefore)
it('Should revert when accepting a rental offer with insufficient native tokens', async function () {
await marketplaceRoyalties
.connect(operator)
.setTrustedFeeTokenForToken([rentalOffer.tokenAddress], [AddressZero], [true])

console.log('AFTER')
console.log(borrowerBalanceAfter)
rentalOffer.feeTokenAddress = AddressZero
rentalOffer.feeAmountPerSecond = toWei('0.0000001')
const totalFeeAmount = rentalOffer.feeAmountPerSecond * BigInt(duration)
rentalOffer.nonce = `0x${randomBytes(32).toString('hex')}`
await marketplace.connect(lender).createRentalOffer({ ...rentalOffer, commitmentId: BigInt(0) })
rentalOffer.commitmentId = BigInt(2)

const insufficientAmount = totalFeeAmount - BigInt(toWei('0.00000001')) // slightly less than required
await expect(
marketplace.connect(borrower).acceptRentalOffer(rentalOffer, BigInt(duration), {
value: insufficientAmount.toString(),
}),
).to.be.revertedWith('OriumSftMarketplace: Insufficient native token amount')
})

describe('Fees', async function () {
Expand Down

0 comments on commit 938e11b

Please sign in to comment.