Mintra 1

Reference: #L23

The current contract name MintraDirectListingsLogicStandalone is quite long and can be shortened. In addition, the term Logic could imply the use of a Proxy, which is not the case for the current implementation.

Remediations to Consider

Consider renaming the contract to a shorter name and remove the term Logic, e.g. rename to MintraDirectListings

Reference: #L678

The function setPlatformFeeBps doesn’t emit an event, but it is recommended to emit an event for every state-changing function.

Remediations to Consider

Consider adding an appropriate event for above function.

Reference: #L32, #L40, #L51

Indexed fields allow off-chain components to quicker accessing them. The following events don’t use indexed fields:

• MintraNewSale
• MintraRoyaltyTransfered
• RoyaltyUpdated

Remediations to Consider

Consider adding indexed keywords to above events where needed.

Reference: #L51

The RoyaltyUpdated event is declared as follows:

event RoyaltyUpdated(address assetContract, uint256 royaltyAmount, address royaltyRecipient);   // @audit not royaltyAmount but royaltyBps

The event is lacking a msg.sender value which could be helpful to properly track the address of who updated the royalty. In addition, one of the arguments is named royaltyAmount, where e.g. royaltyBps is more appropriate, as the argument is referring to the bps rather than the actual calculated royalty amount.

Remediations to Consider

Consider refactoring the RoyaltyUpdated event to include above recommendations.

Reference: #L57

It is recommended to declare variables that cannot be changed as constants. In addition, this saves gas as the variable is stored in the contract’s bytecode.

Remediations to Consider

Consider adding the constant keyword to the platformFeeBpsMint variable declaration.

Reference: #L457, #L647, #L679

The following lines use the number 10000 instead of the declared constant MAX_BPS

457:     require(_royaltyInBasisPoints >= 0 && _royaltyInBasisPoints <= **10000**, "Royalty not in range");
647:     royaltyAmount = (_price * royalties[_tokenAddress].basisPoints) / **10000**;

Consider using MAX_BPS instead.

In addition, the following line uses 369 to define the upper limit for the platform fee:

require(_platformFeeBps <= 369, "Fee not in range");

Consider using a more descriptive constant variable for this value.

Reference: #L68, #L102

nativeTokenWrapper is declared and has a value assigned in the constructor, but the value is not used anywhere in the code.

Remediations to Consider

Consider removing all occurrences of nativeTokenWrapper from the code.

Reference: #L118, #L638

When using named return variables as in the following function, values need to be assigned to the return variable but there is no need to explicitly return the variable itself.

function createListing(ListingParameters calldata _params) external returns (uint256 **listingId**) {

Above function declares a return variable listingId and a value is correctly assigned on #L119, thus there is no need to return the variable at the end of the function on #L156.

The same applies to the processRoyalty function.

Remediations to Consider

Consider removing the lines that return values in createListing and processRoyalty functions.

Reference: #L265C2-L302

The function bulkBuyFromListing can be improved as follows:

1. The function performs the following logic for buys with currency specified as NATIVE_TOKEN:

282:   if (_currency[i] == CurrencyTransferLib.NATIVE_TOKEN) {
283:       //calculate total amount for items being sold for PLS
284:       if (_directListingsStorage().currencyPriceForListing[_listingId[i]][_currency[i]] > 0) {
285:           price =
286:               _quantity[i] *
287:               _directListingsStorage().currencyPriceForListing[_listingId[i]][_currency[i]];
288:       } else {
289:           require(_currency[i] == listing.currency, "Paying in invalid currency.");
290:           price = _quantity[i] * listing.pricePerToken;
291:       }
292:
293:       totalAmountPls += price;
294:   }

The above logic can be simplified by removing #283-#291 entirely and replacing the calculated price with _expectedTotalPrice[i] on line 293, leading to:

if (_currency[i] == CurrencyTransferLib.NATIVE_TOKEN) {
   totalAmountPls += _expectedTotalPrice[i];
}

Removing #283-#291 works as the logic is redundant and the same check is done in the internal _buyFromListing function which is called subsequently. In addition, the check on line #L349 ensures that the calculated price must be equal to the _expectedTotalPrice.

Consider refactoring bulkBuyFromListing function as shown above.

1. Line #L301 ensures that the correct amount for msg.value was sent:

require(msg.value == totalAmountPls || (totalAmountPls == 0 && msg.value == 0), "Incorrect PLS amount sent");

However, the right branch of the OR operator is redundant as the case of totalAmountPls == 0 && msg.value == 0 is already covered on the left check.

Consider removing the right branch from the require statement.

Above recommendations increase the code readability and above that, also reduce the gas costs.

Reference: #L638, #L660

Functions:

According to Solidity naming conventions, private and internal functions should be prefixed using an underscore. This is not the case for the processRoyalty and isERC2981 functions.

Remediations to Consider

Consider renaming processRoyalty and isERC2981 functions to _processRoyalty and _isERC2981 respectively.

Reference: #L451, #L678

The following functions can be declared as external instead of public, as they are not called internally by any other function:

• createOrUpdateRoyalty
• setPlatformFeeBps

Reference: #L265

bulkBuyFromListing function uses memory allocation for its function arguments.

Remediations to Consider

Consider using calldata instead to save gas costs.