code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

Gas Optimizations #260

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Gas report

[G-01] Caching length on for

Caching the array length is more gas efficient

./contracts/src/PuttyV2Nft.sol:

- L556: for (uint256 i = 0; i < orders.length; i++) {
- L594: for (uint256 i = 0; i < assets.length; i++) {
- L611: for (uint256 i = 0; i < assets.length; i++) {
- L627: for (uint256 i = 0; i < floorTokens.length; i++) {
- L637: for (uint256 i = 0; i < assets.length; i++) {
- L647: for (uint256 i = 0; i < assets.length; i++) {
- L658: for (uint256 i = 0; i < floorTokens.length; i++) {
- L670: for (uint256 i = 0; i < whitelist.length; i++) {
- L728: for (uint256 i = 0; i < arr.length; i++) {
- L742: for (uint256 i = 0; i < arr.length; i++) {

Example:

From:

for (uint256 i; i < array.length; i++) {
  ...

To:

uint256 arrayLength = array.length;

for (uint256 i; i < arrayLength; i++) {
  ...

[G-02] \<VAR>++ or \<VAR> += 1 to ++\<VAR> and use unchecked

When the value of the post-loop increment/decrement is not stored or used in any calculations, the prefix increment/decrement operators (++i/--i) cost less gas PER LOOP than the postfix increment/decrement operators (i++/i--)

Use ++i rather than i++ to save gas Also use unchecked, ++i can't overflow

Instances include:

./contracts/src/PuttyV2Nft.sol:

- L556: for (uint256 i = 0; i < orders.length; i++) {
- L594: for (uint256 i = 0; i < assets.length; i++) {
- L611: for (uint256 i = 0; i < assets.length; i++) {
- L627: for (uint256 i = 0; i < floorTokens.length; i++) {
- L637: for (uint256 i = 0; i < assets.length; i++) {
- L647: for (uint256 i = 0; i < assets.length; i++) {
- L658: for (uint256 i = 0; i < floorTokens.length; i++) {
- L670: for (uint256 i = 0; i < whitelist.length; i++) {
- L728: for (uint256 i = 0; i < arr.length; i++) {
- L742: for (uint256 i = 0; i < arr.length; i++) {

Example:

From:

for (uint256 i; i < array.length; i++) {
  ...
}

To:

for (uint256 i; i < array.length;) {
  ...
  unchecked {
    ++i;
  }
}

[G-03] Public functions to external functions

The following functions could be set external to save gas and improve code quality External call cost is less expensive than of public functions

Instances include:

./contracts/src/PuttyV2Nft.sol:

- L389: function exercise(Order memory order, uint256[] calldata floorAssetTokenIds) public payable {
- L466: function withdraw(Order memory order) public {
- L550: ) public returns (uint256[] memory positionIds) {
- L577: ) public payable returns (uint256 positionId) {

[G-04] Remove unnecesary requires

On PuttyV2.sol#L598 there is no need to check if address is a contract, safeTransferFrom will take care, and if you allow 0 amount order you could also remove next line PuttyV2.sol#L599

[G-05] Function isWhitelisted should be internal

Function isWhitelisted can be declared as internal;

diff --git a/contracts/src/PuttyV2.sol b/contracts/src/PuttyV2.sol
index 04cf2fb..05cb05a 100644
--- a/contracts/src/PuttyV2.sol
+++ b/contracts/src/PuttyV2.sol
@@ -666,7 +666,7 @@ contract PuttyV2 is PuttyV2Nft, EIP712("Putty", "2.0"), ERC721TokenReceiver, Own
         @param target The target address to check.
         @return If it exists in the whitelist or not.
      */
-    function isWhitelisted(address[] memory whitelist, address target) public pure returns (bool) {
+    function isWhitelisted(address[] memory whitelist, address target) internal pure returns (bool) {
         for (uint256 i = 0; i < whitelist.length; i++) {
             if (target == whitelist[i]) return true;
         }
JeeberC4 commented 2 years ago

Warden submitted multiple Gas Optimizations. Will not be judged.