Putty contest - 0xf15ers's results

1. Use local variable for expression re-used multiple times

• uint256(orderHash) is used in 3 places inside fillOrder(), it's casted value can be saved into a local variable to reuse variable instead of multiple casting

• PuttyV2.sol#L303

• PuttyV2.sol#L312

• PuttyV2.sol#L316

// recommendation example 
uint tempOrderHash = uint256(orderHash)

2. 10_000 days can be saved in a constant varible

• use contants for numbers
• PuttyV2.sol#L287

3. Use bytes.concat instead of `abi.encodePacked

• ref: https://github.com/ethereum/solidity/issues/10903

4. No method to rescue stucked tokens

• Anyone accidentally sending any tokens into the contract have no way to rescue it.
• Create a skim() like method to withdraw stucked funds.

5 .Unused imported library

• Openzeppelin's string library is imported in PuttyV2Nft.sol is not used.
• PuttyV2Nft.sol#L5

1. Cache array length in loops instead of computing length in every iteration

While looping through array, the array length can be cached to save gas instead of computing length in each array iteration.

for eg.

uint256 len = assets.length;
for(uint256 i = 0; i < len; i++) {
  ...
}

in:

./contracts/src/PuttyV2.sol:556:        for (uint256 i = 0; i < orders.length; i++) {
./contracts/src/PuttyV2.sol:594:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:611:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:627:        for (uint256 i = 0; i < floorTokens.length; i++) {
./contracts/src/PuttyV2.sol:637:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:647:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:658:        for (uint256 i = 0; i < floorTokens.length; i++) {
./contracts/src/PuttyV2.sol:670:        for (uint256 i = 0; i < whitelist.length; i++) {
./contracts/src/PuttyV2.sol:728:        for (uint256 i = 0; i < arr.length; i++) {
./contracts/src/PuttyV2.sol:742:        for (uint256 i = 0; i < arr.length; i++) {   

2. Prefix increment (++i) is cheaper than postfix increment (i++)

Using prefix increment is saves small amount of gas than postfix increment because it returns the updated value hence doesn't requires to store intermediate value. This can be more significant in loops where this operation is done multiple times.

for eg.

// before
for(uint256 i = 0; i < len; i++) {
  ...
}

// Replace with
for(uint256 i = 0; i < len; ++i) {
  ...
}

In:

./contracts/src/PuttyV2.sol:556:        for (uint256 i = 0; i < orders.length; i++) {
./contracts/src/PuttyV2.sol:594:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:611:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:627:        for (uint256 i = 0; i < floorTokens.length; i++) {
./contracts/src/PuttyV2.sol:637:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:647:        for (uint256 i = 0; i < assets.length; i++) {
./contracts/src/PuttyV2.sol:658:        for (uint256 i = 0; i < floorTokens.length; i++) {
./contracts/src/PuttyV2.sol:670:        for (uint256 i = 0; i < whitelist.length; i++) {
./contracts/src/PuttyV2.sol:728:        for (uint256 i = 0; i < arr.length; i++) {
./contracts/src/PuttyV2.sol:742:        for (uint256 i = 0; i < arr.length; i++) {   

3. Use solidity custom errors to save gas

solidity 0.8.4 introduces custom errors which are cheaper than using revert strings in terms of gas. This can be used to save gas.

for eg.

  // Before
  require(condition, "Revert strings");

  // After
  error CustomError();
  if (!condition) {
    revert CustomError();
  }

more details can be found here

4. !=0 is more gas efficient than >0 for uints

For uint comparision, != 0 can be used instead of > 0 in order to save gas In:

./contracts/src/PuttyV2.sol:293:        require(order.baseAsset.code.length > 0, "baseAsset is not contract");
./contracts/src/PuttyV2.sol:327:            if (weth == order.baseAsset && msg.value > 0) {
./contracts/src/PuttyV2.sol:351:            if (weth == order.baseAsset && msg.value > 0) {
./contracts/src/PuttyV2.sol:427:            if (weth == order.baseAsset && msg.value > 0) {
./contracts/src/PuttyV2.sol:498:            if (fee > 0) {
./contracts/src/PuttyV2.sol:598:            require(token.code.length > 0, "ERC20: Token is not contract");
./contracts/src/PuttyV2.sol:599:            require(tokenAmount > 0, "ERC20: Amount too small");   

5. constant expressions are not actually constants

Constant expressions such as const a = keccak256(x) is not constant value but constant expression. Hence it will need to be evaluated every time it is referenced. Making the constant expression immutable will save gas, as it will not need to be evaluated every time.

• https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L89-L122

6. Make public functions external

There are several functions which are public, but never called from within the contract. Those functions can be changed to external as external functions are cheaper than public.

in PuttyV2.sol, these functions can be made external

exercise()
withdraw()
batchFillOrder()
acceptCounterOffer()
domainSeparatorV4()

7. Checking for default conditions in if statement can be avoided

if the condition inside if statement will always be true, it can be avoided to save gas.

In PuttyV2.sol#L374 The condition checked will always be true, so it doesn't needs to be checked.

8. Use unchecked math for conditions which will never overflow

Several operations such as the loop counter increment will never overflow since it starts from zero and only increases. In such cases unchecked math can be used to save gas.

for eg.

  // Change
  for (uint256 i = 0; i < orders.length; i++) {

  }

  // To
  for (uint256 i = 0; i < orders.length; ) {

    unchecked {
      i++;
    }
  }

9. Functions doesn't uses specified named return

The function fillOrder has a named return positionId but all cases of return is explicit. This causes more gas on deployment.

10. Constants can be changed to private from public

Having public constants also makes their getter functions. This will increase code size and hence deployment gas. They can be changed to private to save gas.

• https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L89-L122