Skip to content

Commit

Permalink
ON-547: PR fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Daniel Lima committed Jan 9, 2024
1 parent 3291c4c commit 8873ad2
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 36 deletions.
50 changes: 26 additions & 24 deletions contracts/OriumSftMarketplace.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ contract OriumSftMarketplace is Initializable, OwnableUpgradeable, PausableUpgra
address public defaultRolesRegistry;

/// @dev tokenAddress => rolesRegistry
mapping(address => address) public tokenRolesRegistry;
mapping(address => address) public tokenAddressToRolesRegistry;

/// @dev deadline is set in seconds
uint256 public maxDeadline;

/// @dev tokenAddress => feePercentageInWei
mapping(address => FeeInfo) public feeInfo;

/// @dev tokenAddress => royaltyInfo
mapping(address => RoyaltyInfo) public royaltyInfo;
/// @dev tokenAddress => tokenAddressToRoyaltyInfo
mapping(address => RoyaltyInfo) public tokenAddressToRoyaltyInfo;

/** ######### Structs ########### **/

Expand Down Expand Up @@ -103,8 +103,8 @@ contract OriumSftMarketplace is Initializable, OwnableUpgradeable, PausableUpgra
/** ######### Setters ########### **/

/**
* @notice Sets the roles registry.
* @dev Only owner can set the roles registry.
* @notice Pauses the contract.
* @dev Only owner can pause the contract.
*/
function pause() external onlyOwner {
_pause();
Expand All @@ -130,7 +130,7 @@ contract OriumSftMarketplace is Initializable, OwnableUpgradeable, PausableUpgra
uint256 _feePercentageInWei,
bool _isCustomFee
) external onlyOwner {
uint256 _royaltyPercentage = royaltyInfo[_tokenAddress].royaltyPercentageInWei;
uint256 _royaltyPercentage = tokenAddressToRoyaltyInfo[_tokenAddress].royaltyPercentageInWei;
require(
_royaltyPercentage + _feePercentageInWei < MAX_PERCENTAGE,
"OriumSftMarketplace: Royalty percentage + marketplace fee cannot be greater than 100%"
Expand All @@ -144,26 +144,26 @@ contract OriumSftMarketplace is Initializable, OwnableUpgradeable, PausableUpgra
/**
* @notice Sets the royalty info.
* @dev Only owner can associate a collection with a creator.
* @param _tokenAddress The SFT address.
* @param _creator The address of the creator.
*/
function setCreator(address _tokenAddress, address _creator) external onlyOwner {
_setRoyalty(_creator, _tokenAddress, 0, address(0));
}

/**
* @notice Sets the royalty info.
* @param _tokenAddress The SFT address.
* @param _royaltyPercentageInWei The royalty percentage in wei.
* @param _treasury The address where the fees will be sent. If the treasury is address(0), the fees will be burned.
*/
function setRoyaltyInfo(address _tokenAddress, uint256 _royaltyPercentageInWei, address _treasury) external {
require(
msg.sender == royaltyInfo[_tokenAddress].creator,
"OriumSftMarketplace: Only creator can set royalty info"
);

_setRoyalty(msg.sender, _tokenAddress, _royaltyPercentageInWei, _treasury);
function setRoyaltyInfo(
address _creator,
address _tokenAddress,
uint256 _royaltyPercentageInWei,
address _treasury
) external {
if (msg.sender != owner()) {
require(
msg.sender == tokenAddressToRoyaltyInfo[_tokenAddress].creator,
"OriumSftMarketplace: Only creator or owner can set the royalty info"
);
require(msg.sender == _creator, "OriumSftMarketplace: sender and creator mismatch");
}

_setRoyalty(_creator, _tokenAddress, _royaltyPercentageInWei, _treasury);
}

/**
Expand All @@ -185,7 +185,7 @@ contract OriumSftMarketplace is Initializable, OwnableUpgradeable, PausableUpgra
"OriumSftMarketplace: Royalty percentage + marketplace fee cannot be greater than 100%"
);

royaltyInfo[_tokenAddress] = RoyaltyInfo({
tokenAddressToRoyaltyInfo[_tokenAddress] = RoyaltyInfo({
creator: _creator,
royaltyPercentageInWei: _royaltyPercentageInWei,
treasury: _treasury
Expand All @@ -211,7 +211,7 @@ contract OriumSftMarketplace is Initializable, OwnableUpgradeable, PausableUpgra
* @param _rolesRegistry The roles registry address.
*/
function setRolesRegistry(address _tokenAddress, address _rolesRegistry) external onlyOwner {
tokenRolesRegistry[_tokenAddress] = _rolesRegistry;
tokenAddressToRolesRegistry[_tokenAddress] = _rolesRegistry;
emit RolesRegistrySet(_tokenAddress, _rolesRegistry);
}

Expand Down Expand Up @@ -242,6 +242,8 @@ contract OriumSftMarketplace is Initializable, OwnableUpgradeable, PausableUpgra
*/
function rolesRegistryOf(address _tokenAddress) public view returns (address) {
return
tokenRolesRegistry[_tokenAddress] == address(0) ? defaultRolesRegistry : tokenRolesRegistry[_tokenAddress];
tokenAddressToRolesRegistry[_tokenAddress] == address(0)
? defaultRolesRegistry
: tokenAddressToRolesRegistry[_tokenAddress];
}
}
56 changes: 44 additions & 12 deletions test/OriumSftMarketplace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { deploySftMarketplaceContracts } from './fixtures/OriumSftMarketplaceFix
import { expect } from 'chai'
import { toWei } from '../utils/bignumber'
import { FeeInfo, RoyaltyInfo } from '../utils/types'
import { THREE_MONTHS } from '../utils/constants'
import { AddressZero, THREE_MONTHS } from '../utils/constants'

describe('OriumSftMarketplace', () => {
let marketplace: Contract
Expand Down Expand Up @@ -92,15 +92,17 @@ describe('OriumSftMarketplace', () => {
])
expect(await marketplace.marketplaceFeeOf(mockERC1155.address)).to.be.equal(feeInfo.feePercentageInWei)
})

it('Should NOT set the marketplace fee if caller is not the operator', async () => {
await expect(
marketplace
.connect(notOperator)
.setMarketplaceFeeForCollection(mockERC1155.address, feeInfo.feePercentageInWei, feeInfo.isCustomFee),
).to.be.revertedWith('Ownable: caller is not the owner')
})

it("Should NOT set the marketplace fee if marketplace fee + creator royalty it's greater than 100%", async () => {
await marketplace.connect(operator).setCreator(mockERC1155.address, creator.address)
await marketplace.connect(operator).setRoyaltyInfo(creator.address, mockERC1155.address, 0, AddressZero)

const royaltyInfo: RoyaltyInfo = {
creator: creator.address,
Expand All @@ -110,7 +112,12 @@ describe('OriumSftMarketplace', () => {

await marketplace
.connect(creator)
.setRoyaltyInfo(mockERC1155.address, royaltyInfo.royaltyPercentageInWei, royaltyInfo.treasury)
.setRoyaltyInfo(
creator.address,
mockERC1155.address,
royaltyInfo.royaltyPercentageInWei,
royaltyInfo.treasury,
)

const feeInfo: FeeInfo = {
feePercentageInWei: toWei('95'),
Expand All @@ -132,27 +139,31 @@ describe('OriumSftMarketplace', () => {
treasury: ethers.constants.AddressZero,
}

await expect(marketplace.connect(operator).setCreator(mockERC1155.address, creator.address))
await expect(
marketplace.connect(operator).setRoyaltyInfo(creator.address, mockERC1155.address, 0, AddressZero),
)
.to.emit(marketplace, 'CreatorRoyaltySet')
.withArgs(mockERC1155.address, creator.address, royaltyInfo.royaltyPercentageInWei, royaltyInfo.treasury)

expect(await marketplace.royaltyInfo(mockERC1155.address)).to.have.deep.members([
expect(await marketplace.tokenAddressToRoyaltyInfo(mockERC1155.address)).to.have.deep.members([
royaltyInfo.creator,
royaltyInfo.royaltyPercentageInWei,
royaltyInfo.treasury,
])
})

it('Should NOT set the creator royalties if caller is not the operator', async () => {
await expect(
marketplace.connect(notOperator).setCreator(mockERC1155.address, creator.address),
).to.be.revertedWith('Ownable: caller is not the owner')
marketplace.connect(notOperator).setRoyaltyInfo(creator.address, mockERC1155.address, 0, AddressZero),
).to.be.revertedWith('OriumSftMarketplace: Only creator or owner can set the royalty info')
})
})

describe('Creator', async () => {
beforeEach(async () => {
await marketplace.connect(operator).setCreator(mockERC1155.address, creator.address)
await marketplace.connect(operator).setRoyaltyInfo(creator.address, mockERC1155.address, 0, AddressZero)
})

it("Should update the creator royalties for a collection if it's already set", async () => {
const royaltyInfo: RoyaltyInfo = {
creator: creator.address,
Expand All @@ -163,11 +174,17 @@ describe('OriumSftMarketplace', () => {
await expect(
marketplace
.connect(creator)
.setRoyaltyInfo(mockERC1155.address, royaltyInfo.royaltyPercentageInWei, royaltyInfo.treasury),
.setRoyaltyInfo(
creator.address,
mockERC1155.address,
royaltyInfo.royaltyPercentageInWei,
royaltyInfo.treasury,
),
)
.to.emit(marketplace, 'CreatorRoyaltySet')
.withArgs(mockERC1155.address, creator.address, royaltyInfo.royaltyPercentageInWei, royaltyInfo.treasury)
})

it('Should NOT update the creator royalties for a collection if caller is not the creator', async () => {
const royaltyInfo: RoyaltyInfo = {
creator: creator.address,
Expand All @@ -178,9 +195,15 @@ describe('OriumSftMarketplace', () => {
await expect(
marketplace
.connect(notOperator)
.setRoyaltyInfo(mockERC1155.address, royaltyInfo.royaltyPercentageInWei, royaltyInfo.treasury),
).to.be.revertedWith('OriumSftMarketplace: Only creator can set royalty info')
.setRoyaltyInfo(
creator.address,
mockERC1155.address,
royaltyInfo.royaltyPercentageInWei,
royaltyInfo.treasury,
),
).to.be.revertedWith('OriumSftMarketplace: Only creator or owner can set the royalty info')
})

it("Should NOT update the creator royalties for a collection if creator's royalty percentage + marketplace fee is greater than 100%", async () => {
const royaltyInfo: RoyaltyInfo = {
creator: creator.address,
Expand All @@ -191,7 +214,12 @@ describe('OriumSftMarketplace', () => {
await expect(
marketplace
.connect(creator)
.setRoyaltyInfo(mockERC1155.address, royaltyInfo.royaltyPercentageInWei, royaltyInfo.treasury),
.setRoyaltyInfo(
creator.address,
mockERC1155.address,
royaltyInfo.royaltyPercentageInWei,
royaltyInfo.treasury,
),
).to.be.revertedWith(
'OriumSftMarketplace: Royalty percentage + marketplace fee cannot be greater than 100%',
)
Expand All @@ -203,11 +231,13 @@ describe('OriumSftMarketplace', () => {
await marketplace.connect(operator).setMaxDeadline(maxDeadline)
expect(await marketplace.maxDeadline()).to.be.equal(maxDeadline)
})

it('Should NOT set the max deadline if caller is not the operator', async () => {
await expect(marketplace.connect(notOperator).setMaxDeadline(maxDeadline)).to.be.revertedWith(
'Ownable: caller is not the owner',
)
})

it('Should NOT set the max deadline 0', async () => {
await expect(marketplace.connect(operator).setMaxDeadline(0)).to.be.revertedWith(
'OriumSftMarketplace: Max deadline should be greater than 0',
Expand All @@ -221,6 +251,7 @@ describe('OriumSftMarketplace', () => {
.to.emit(marketplace, 'RolesRegistrySet')
.withArgs(mockERC1155.address, rolesRegistry.address)
})

it('Should NOT set the roles registry if caller is not the operator', async () => {
await expect(
marketplace.connect(notOperator).setRolesRegistry(mockERC1155.address, rolesRegistry.address),
Expand All @@ -232,6 +263,7 @@ describe('OriumSftMarketplace', () => {
it('Should set the default roles registry for a collection', async () => {
await expect(marketplace.connect(operator).setDefaultRolesRegistry(rolesRegistry.address)).to.not.be.reverted
})

it('Should NOT set the default roles registry if caller is not the operator', async () => {
await expect(
marketplace.connect(notOperator).setDefaultRolesRegistry(rolesRegistry.address),
Expand Down

0 comments on commit 8873ad2

Please sign in to comment.