QA Report

[code423n4]

QA Report

Low severity issues

Mistakes in comments

AmountDeriver.sol#66 includes a following comment:

            // Return the current amount (expressed as endAmount internally).

Based on the context, the writer probably meant:

            // Return the current amount (expressed as newAmount internally).

Another comment mistake lies in GettersAndDerivers.sol#354:

            // Hash the relevant region (65 bytes).
            value := keccak256(0, EIP712_DigestPayload_size)

The comment says that 65 bytes are hashed, but this is not true - 66 bytes are hashed.

Issues in comments are causing code to be less readable and imply that there may be mistakes in the code. It is utterly important to avoid such errors.

Non-critical issues

Function _name() doesn't save data permanently

The _name() internal function from Seaport.sol uses memory which is used as scratch space. This function cannot be used in arbitrary functions because they can overwrite the scratch space. This is done wisely as a gas optimization, but it is not recommended to be an internal function. Allocating and returning the name shall be done in an external function so that one can be sure that the usage of scratch space doesn't cause memory corruption.

Special variable isn't used

In GettersAndDerivers.sol#68 there is an offset added to orderParameters pointer:

            // Get the pointer to the offers array.
            let offerArrPtr := mload(add(orderParameters, TwoWords))

The offset that should be used is OrderParameters_offer_head_offset because of the context given. A programmer could change OrderParameters struct and head offset variable. The code wouldn't work then. It is highly recommended to change to the proposed variable.

[GalloDaSballo]

Agree only with the typo / comments, 1 NC

Rest is invalid:

• name function works
• Pointer is a decision to save gas, the codebase has a reference contract to ensure optimizations do not create undefined functionality, if the warden were able to find undefined behaviour they should bring up a higher severity finding

[HardlyDifficult]

The bar for QA reports in this contest is at least 2 valid non-critical findings or at least one valid low risk finding. Per the comments above, this submission is below that bar -- closing as invalid.