QA Report

[code423n4]

QA Report

Remarks/Recommendations

• the test suite was a joy to work with. I learned a lot about Foundry and will use it on my own projects.
• the documentation was of a very high quality

Low Risk: Don't allow setVaultBeneficiary to be set to address(0)

Yes, getVaultBeneficiary will check for address(0) and return ownerOf[vaultId] but relying on this is a bit dangerous if you change the code in future.

Non-critical: Incorrect error message on check for reserve strike price

There is an incorrect error message in the require at location Cally.sol:169.

It should be:

require(dutchAuctionReserveStrike < strikeOptions[dutchAuctionStartingStrikeIndex], "Reserve strike too large");

In general the error message should always express the opposite of the condition of the require statement.

Non-critical: Several requires with == false in them

The locations are

• Cally.sol:217
• Cally.sol:220
• Cally.sol:328

It is better to use the ! (not) operator. e.g.

require(!vault.isExercised, "Vault already exercised");