MarketHandlerBase Manual Review Findings

MHB-01M: Deprecated Native Asset Transfer

Description:

The transfer member exposed by payable address types has been deprecated as it does not reliably execute and can fail in future updates of the EVM as it forwards a fixed gas stipend which is not compatible with gas cost EIP upgrades such as EIP-2929.

Example:

Copy
221payable(recipient).transfer(royaltyAmount);

Recommendation:

We advise a safe wrapper library to be utilized instead such as the sendValue function of the Address library by OpenZeppelin which is guaranteed to execute under all circumstances.

Alleviation:

The codebase was updated to utilize an internal sendValueOrCreditAccount function that attempts to send the native asset to the recipient address and in case the action fails for any reason, the user is given a redeemable credit of their native asset portion. This change has alleviated this exhibit in full.

MHB-02M: Improper Fee Deduction

Description:

The deductFee function assumes that the split value is equal to feeAmount when multiplied by 2 which is not the case as Solidity suffers from mathematical truncation. This will lead to funds being permanently locked in any MarketHandlerBase contract.

Example:

Copy
320uint256 split = feeAmount / 2;
321address payable staking = marketController.getStaking();
322address payable multisig = marketController.getMultisig();
323staking.transfer(split);
324multisig.transfer(split);
325
326// Return the seller payout amount after fee deduction
327payout = _netAmount - feeAmount;

Recommendation:

We advise the second transfer to instead use feeAmount - split rather than split to ensure truncated funds are accounted for.

Alleviation:

The two values split are now properly calculated via a division and subtraction rather than a single value.

MHB-03M: Non-Standard Application of Escrow Fee

Description:

The deductEscrowAgentFee function does not validate that the total applied fee does not exceed the total available amount, causing "correct" configuration of the contract to lead to a failure.

Example:

Copy
255function deductEscrowAgentFee(Consignment memory _consignment, uint256 _grossSale, uint256 _netAfterRoyalties)
256internal
257returns (uint256 net)
258{
259    // Only pay royalties on secondary market sales
260    uint256 escrowAgentFeeAmount = 0;
261    if (_consignment.market == Market.Secondary) {
262        // Get the MarketController
263        IMarketController marketController = getMarketController();
264        address consignor = marketController.getConsignor(_consignment.id);
265        if(consignor != _consignment.seller) {
266            uint16 escrowAgentBasisPoints = marketController.getEscrowAgentFeeBasisPoints(consignor);
267            if(escrowAgentBasisPoints > 0) {
268                // Determine if consignment is physical
269                address nft = marketController.getNft();
270                if (nft == _consignment.tokenAddress && ISeenHausNFT(nft).isPhysical(_consignment.tokenId)) {
271                    // Consignor is not seller, consigner has a positive escrowAgentBasisPoints value, consignment is of a physical item
272                    // Therefore, pay consignor the escrow agent fees
273                    escrowAgentFeeAmount = getPercentageOf(_grossSale, escrowAgentBasisPoints);
274
275                    // If escrow agent fee is expected...
276                    if (escrowAgentFeeAmount > 0) {
277                        payable(consignor).transfer(escrowAgentFeeAmount);
278                        // Notify listeners of payment
279                        emit EscrowAgentFeeDisbursed(_consignment.id, consignor, escrowAgentFeeAmount);
280                    }
281                }
282            }
283        }
284    }
285
286    // Return the net amount after royalty deduction
287    net = _netAfterRoyalties - escrowAgentFeeAmount;
288}

Recommendation:

We advise the escrow fee to instead be applied on the after-royalty amount to ensure that the statements will always execute properly. Alternatively, we advise the code to gracefully handle the royalty and escrow agent fee exceeding the total amount by introducing a new maximum escrow agent fee to the system.

Alleviation:

A require check has been introduced ensuring that the net amount after royalties can satisfy the escrow agent fee thereby alleviating this exhibit.