Putty contest - delfin454000's results

Typos

---

src/PuttyV2.sol: L751

        @return The domain seperator used when calculating the eip-712 hash.

Change seperator to separator

---

utils/cryptography/draft-EIP712.sol: L94

     *     keccak256("Mail(address to,string contents)"),

Change to,string to to, string

---

utils/Strings.sol: L17

        // Inspired by OraclizeAPI's implementation - MIT licence

Change licence to License

---

Update sensitive terms such as whitelist in both comments and code

Terms incorporating "white," "black," "master" or "slave" are potentially problematic. Substituting more neutral terminology is becoming common practice

---

src/PuttyV2.sol: L78

        address[] whitelist;

Suggestion: Change whitelist to allowlist

---

Similarly for the following instances of 'whitelist' and its variants:

src/PuttyV2.sol: L114

src/PuttyV2.sol: L284

src/PuttyV2.sol: L664

src/PuttyV2.sol: L665

src/PuttyV2.sol: L667

src/PuttyV2.sol: L669

src/PuttyV2.sol: L670

src/PuttyV2.sol: L671

src/PuttyV2.sol: L712

---

Gas optimizations

Require revert string is too long

The revert string below can be shortened to 32 characters or fewer (as shown) to save gas

---

access/Ownable.sol: L63

        require(newOwner != address(0), "Ownable: new owner is the zero address");

Change message to Ownable: new owner is 0 address

---

Use != 0 instead of > 0 in a require statement if variable is an unsigned integer

!= 0 should be used where possible since > 0 costs more gas

---

src/PuttyV2.sol: L293

        require(order.baseAsset.code.length > 0, "baseAsset is not contract");

Change order.baseAsset.code.length > 0 to order.baseAsset.code.length != 0

---

src/PuttyV2.sol: L598

            require(token.code.length > 0, "ERC20: Token is not contract");

Change token.code.length > 0 to token.code.length != 0

---

src/PuttyV2.sol: L599

            require(tokenAmount > 0, "ERC20: Amount too small");

Change tokenAmount > 0 to tokenAmount != 0

---

Variables should not be initialized to their default values

For example, initializing uint variables to their default value of 0 is unnecessary and costs gas

---

src/PuttyV2.sol: L497

            uint256 feeAmount = 0;

Change to uint256 feeAmount;

---

utils/Strings.sol: L46

        uint256 length = 0;

Change to uint256 length;

---

The for loop counter i is initialized unnecessarily to zero in the 10 loops referenced below:

src/PuttyV2.sol: L556-558

src/PuttyV2.sol: L594-602

src/PuttyV2.sol: L611-613

src/PuttyV2.sol: L627-629

src/PuttyV2.sol: L637-639

src/PuttyV2.sol: L647-649

src/PuttyV2.sol: L658-660

src/PuttyV2.sol: L670-672

src/PuttyV2.sol: L728-733

src/PuttyV2.sol: L742-747

Example (src/PuttyV2.sol: L556-558):

        for (uint256 i = 0; i < orders.length; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

Change uint256 i = 0; to uint256 i; in all cases

---

Array length should not be looked up in every iteration of a for loop

Since calculating the array length costs gas, it's best to read the length of the array from memory before executing the loop

---

src/PuttyV2.sol: L556-558

        for (uint256 i = 0; i < orders.length; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

Suggestion:

        uint256 totalOrdersLength = orders.length; 
        for (uint256 i = 0; i < totalOrdersLength; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

---

Similarly for the additional nine for loops referenced below:

src/PuttyV2.sol: L594-602

src/PuttyV2.sol: L611-613

src/PuttyV2.sol: L627-629

src/PuttyV2.sol: L637-639

src/PuttyV2.sol: L647-649

src/PuttyV2.sol: L658-660

src/PuttyV2.sol: L670-672

src/PuttyV2.sol: L728-733

src/PuttyV2.sol: L742-747

---

Note that I will provide an example at the end of this section that combines all the gas-saving recommendations pertaining tofor loops

---

Use i++ instead of i++ to increase count in a for loop

Since use of i++ (or equivalent counter) costs more gas, it is better to use ++i

---

src/PuttyV2.sol: L556-558

        for (uint256 i = 0; i < orders.length; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

Suggestion:

        for (uint256 i = 0; i < orders.length; ++i) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

---

Similarly for the nine additional for loops referenced below:

src/PuttyV2.sol: L594-602

src/PuttyV2.sol: L611-613

src/PuttyV2.sol: L627-629

src/PuttyV2.sol: L637-639

src/PuttyV2.sol: L647-649

src/PuttyV2.sol: L658-660

src/PuttyV2.sol: L670-672

src/PuttyV2.sol: L728-733

src/PuttyV2.sol: L742-747

---

Avoid use of default "checked" behavior in a for loop

Under/overflow checks are made every time i++ (or ++i) is called. Such checks are unnecessary since i is already limited. Therefore, use unchecked{++i}/unchecked{i++} instead

---

src/PuttyV2.sol: L556-558

        for (uint256 i = 0; i < orders.length; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

Suggestion:

        for (uint256 i = 0; i < orders.length;) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
            unchecked{ i++; }
        }

---

Similarly for the nine additional for loops referenced below:

src/PuttyV2.sol: L594-602

src/PuttyV2.sol: L611-613

src/PuttyV2.sol: L627-629

src/PuttyV2.sol: L637-639

src/PuttyV2.sol: L647-649

src/PuttyV2.sol: L658-660

src/PuttyV2.sol: L670-672

src/PuttyV2.sol: L728-733

src/PuttyV2.sol: L742-747

---

For loop gas optimization example

While, for presentation purposes, I have separated the for loop-related gas savings methods, they should be combined. Below I show how all four of the gas saving methods outlined in this submission can be implemented together.

1. Don't initialize the loop counter (i) to zero
2. Read the array length before executing the loop
3. Use ++i rather than i++
4. Use unchecked ++i

src/PuttyV2.sol: L556-558

        for (uint256 i = 0; i < orders.length; i++) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
        }

Recommendation:

        uint256 totalOrdersLength = orders.length; 
        for (uint256 i; i < totalOrdersLength; ++i) {
            positionIds[i] = fillOrder(orders[i], signatures[i], floorAssetTokenIds[i]);
            unchecked{ ++i; }
        }

---

Avoid storage of uintsor ints smaller than 32 bytes, if possible

Storage of uints or ints smaller than 32 bytes incurs overhead. Instead, use size of at least 32, then downcast where needed

---

utils/Strings.sol: L11

    uint8 private constant _ADDRESS_LENGTH = 20;

---