Infinity NFT Marketplace contest - sorrynotsorry's results

QA (LOW & NON-CRITICAL)

[L-01] Missing checks for contract existence

OpenZeppelin's safeTransfer and safeTransferFrom functions use functionCall of Address.sol which is a low level call. However, it's stated that the target must be a contract and code existence should be checked prior using. Reference It's advised to check code existence prior calling safeTransfer or safeTransferFrom as below; In InfinityExchange.sol;

729: IERC20(buy.execParams[1]).safeTransferFrom(buy.signer, sell.signer, remainingAmount);

In InfinityStaker.sol;

128:     IERC20(INFINITY_TOKEN).safeTransfer(msg.sender, amount);
141:     IERC20(INFINITY_TOKEN).safeTransfer(msg.sender, totalToUser);
142:     IERC20(INFINITY_TOKEN).safeTransfer(INFINITY_TREASURY, penalty);

[L-02] Missing of zero address checks during construction

There are no address(0) or Zero value check at the constructors of: InfinityExchange.sol, InfinityStaker.sol, InfinityToken.sol

[L-03] Missing indexes in important events

Non of the events in InfinityExchange.sol, InfinityToken.sol has indexed parameter.

[L-04] Redundant statement

The team might consider to remove the line below since the condition is already checked in Signaturechecker.verify function;

523:       order.signer == address(0) ||

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L523

[L-05] Critical address / parameter changes

Critical address / value changes are not done in two steps for the following methods: At InfinityStaker.sol; updateInfinityTreasury(), updateStakeLevelThreshold(), updatePenalties(),

Changing critical addresses in contracts should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one (i.e. claims ownership). This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible. Reference, Reference

[L-06] Signature malleability of EVM's ecrecover in verify()

The contract uses ecrecover() function after many function triggering in InfinityExchange.sol's verifyOrderSig() . EVM's ecrecover is susceptible to signature malleability which allows replay attacks, but using OpenZeppelin’s ECDSA library can be mitigation for this. Reference

[L-07] Zero check for denominator at contract level

At InfinityExchange.sol matchOneToOneOrders() function, the parameter makerOrders1.length should be checked against being zero. No matter it's checked at the interface level, if somehow it's missed and the calldata was input as 0, it will revert.

[N-01] All capitals are used for CONSTANTS

All capital letters in a variable is used for constanst/immutable variables. However, this was not followed for the below;

1161:     uint256 PRECISION = 10**4; // precision for division; similar to bps

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1161