QuickSwap and StellaSwap contest - ajtra's results

Low issues

1. Outdated compiler version

Description

It's a best practice to use the latest compiler version. The specified minimum compiler version is quite old (0.7.6). Older compilers might be susceptible to some bugs. It's recommended changing the solidity version pragma to the latest version to enforce the use of an up-to-date compiler.

A list of known compiler bugs and their severity can be found here: https://etherscan.io/solcbuginfo

To check the bugfixed and improvements of latest versions see the following link

Mitigation

Update the pragma to 0.8.16

Lines in the code

AlgebraFactory.sol#L2 AlgebraPool.sol#L2 AlgebraPoolDeployer.sol#L2 DataStorageOperator.sol#L2 AdaptiveFee.sol#L2 Constants.sol#L2 DataStorage.sol#L2 PriceMovementMath.sol#L2 TickManager.sol#L2 TickTable.sol#L2 TokenDeltaMath.sol#L2 PoolImmutables.sol#L2 PoolState.sol#L2

2. Missing checks for address(0x0) when assigning values to address state variables

Lines in the code

AlgebraFactory.sol#L54 AlgebraFactory.sol#L55 DataStorageOperator.sol#L33

Non critical

1. File is missing NatSpec

AlgebraFactory.sol AlgebraPool.sol AlgebraPoolDeployer.sol DataStorageOperator.sol AdaptiveFee.sol Constants.sol DataStorage.sol PriceMovementMath.sol TickManager.sol TickTable.sol TokenDeltaMath.sol PoolImmutables.sol PoolState.sol

Index

1. Post-increment/decrement cost more gas then pre-increment/decrement
2. Array length should not be looked up in every loop of a for-loop
3. Initialize variables with default values are not needed
4. != 0 is cheaper than > 0 especially in state variables
5. I = I + (-) X is cheaper in gas cost than I += (-=) X
6. Use custom errors rather than REVERT()/REQUIRE() strings to save deployment gas
7. Using private rather than public for constants, saves gas
8. Use a more recent version of solidity
9. Initialize variables with default values are not needed
10. Using bools for storage incurs overhead
11. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()
12. Multiplication/division by two should use bit shifting
13. Using storage instead of memory for structs/arrays

Details

1. Post-increment/decrement cost more gas then pre-increment/decrement

Description

++I (--I) cost less gas than I++ (I--) especially in a loop.

Proof of concept

contract TestPost {
	function testPost() public {
		uint256 i;
		i++;
	}
}
contract TestPre {
	function testPre() public {
		uint256 i;
		++i;
	}
}

• Transaction cost of testPost is 21333 gas
• Transaction cost of testPre is 21328 gas
• After the test it's possible to save 5 gas per ocurrence with this optimization.

Lines in the code

DataStorage.sol#L307

2. Array length should not be looked up in every loop of a for-loop

Description

Storage array length checks incur an extra Gwarmaccess (100 gas) per loop. Store the array length in a variable and use it in the for loop helps to save gas

Lines in the code

DataStorage.sol#L307

3. Require instead of &&

Description

Split of conditions of an require sentence in different requires sentences can save gas

Lines in the code

AlgebraFactory.sol#L110 AlgebraPool.sol#L739 AlgebraPool.sol#L743 AlgebraPool.sol#L953 AlgebraPool.sol#L968 DataStorageOperator.sol#L46

4. != 0 is cheaper than > 0 especially in state variables

Description

In cases where we used a variable from state assigned to a local variable it's the same gas save.

Proof of concept

contract TestA {
    uint256 _paramA;

    function Test () public returns (bool) {
        return _paramA > 0;
    }

}

contract TestB {
    uint256 _paramA;

    function Test () public returns (bool) {
        return _paramA != 0;
    }

}

contract TestC {
    uint256 _paramA;

    function Test () public returns (bool) {
        uint256 aux = _paramA;
        return aux > 0;
    }

}

contract TestD {
    uint256 _paramA;

    function Test () public returns (bool) {
        uint256 aux = _paramA;
        return aux != 0;
    }

}

• Transaction cost of TestA/C is 23491/23504 gas
• Transaction cost of TestB/D is 23494/23507 gas
• After the test it's possible to save 3 gas per ocurrence with this optimization.

Lines in the code

AlgebraPool.sol#L224 AlgebraPool.sol#L228 AlgebraPool.sol#L237 AlgebraPool.sol#L434 AlgebraPool.sol#L451 AlgebraPool.sol#L452 AlgebraPool.sol#L454 AlgebraPool.sol#L455 AlgebraPool.sol#L469 AlgebraPool.sol#L505 AlgebraPool.sol#L506 AlgebraPool.sol#L617 AlgebraPool.sol#L667 AlgebraPool.sol#L808 AlgebraPool.sol#L814 AlgebraPool.sol#L898 AlgebraPool.sol#L904 AlgebraPool.sol#L911 AlgebraPool.sol#L924 AlgebraPool.sol#L927 AlgebraPool.sol#L938 AlgebraPool.sol#L941 DataStorageOperator.sol#L138 DataStorageOperator.sol#L139 DataStorage.sol#L80 PriceMovementMath.sol#L52 PriceMovementMath.sol#L53

5. I = I + (-) X is cheaper in gas cost than I += (-=) X

Description

This is especially with state variables. In the following example I'm trying to demostrate the save gas in a loop of 10 iterations.

Proof of concept

contract TestA {
    uint256 i;
    function Test () public {
        
        for(;i<10;){
            i += 1;
        }
    }

}

contract TestB {
    uint256 i;
    function Test () public {
        
        for(;i<10;){
            i = i + 1;
        }
    }
}

• Transaction cost of TestA is 48793 gas
• Transaction cost of TestB is 48663 gas
• After the test it's possible to save 130 gas (13 gas per iteration/ocurrence) with this optimization.

Lines in the code

AlgebraPool.sol#L257 AlgebraPool.sol#L258 AlgebraPool.sol#L801 AlgebraPool.sol#L804 AlgebraPool.sol#L810 AlgebraPool.sol#L811 AlgebraPool.sol#L814 AlgebraPool.sol#L922 AlgebraPool.sol#L931 AlgebraPool.sol#L936 AlgebraPool.sol#L945 AdaptiveFee.sol#L84 AdaptiveFee.sol#L88 AdaptiveFee.sol#L92 AdaptiveFee.sol#L96 AdaptiveFee.sol#L100 AdaptiveFee.sol#L104 AdaptiveFee.sol#L107 DataStorage.sol#L79 DataStorage.sol#L80 DataStorage.sol#L81 DataStorage.sol#L83 DataStorage.sol#L119 DataStorage.sol#L251 DataStorage.sol#L252 DataStorage.sol#L255 DataStorage.sol#L256 TickManager.sol#L58 TickManager.sol#L59 TickTable.sol#L92 TickTable.sol#L95 TickTable.sol#L100 TickTable.sol#L112 TickTable.sol#L115

6. Use custom errors rather than REVERT()/REQUIRE() strings to save deployment gas

Description

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas.

Lines in the code

AlgebraFactory.sol#L43 AlgebraFactory.sol#L60 AlgebraFactory.sol#L62 AlgebraFactory.sol#L63 AlgebraFactory.sol#L78 AlgebraFactory.sol#L85 AlgebraFactory.sol#L92 AlgebraFactory.sol#L109 AlgebraFactory.sol#L110 AlgebraPool.sol#L55 AlgebraPool.sol#L60 AlgebraPool.sol#L61 AlgebraPool.sol#L62 AlgebraPool.sol#L122 AlgebraPool.sol#L134 AlgebraPool.sol#L194 AlgebraPool.sol#L224 AlgebraPool.sol#L229 AlgebraPool.sol#L434 AlgebraPool.sol#L454 AlgebraPool.sol#L455 AlgebraPool.sol#L469 AlgebraPool.sol#L474 AlgebraPool.sol#L475 AlgebraPool.sol#L608 AlgebraPool.sol#L614 AlgebraPool.sol#L636 AlgebraPool.sol#L641 AlgebraPool.sol#L645 AlgebraPool.sol#L731 AlgebraPool.sol#L733 AlgebraPool.sol#L739 AlgebraPool.sol#L743 AlgebraPool.sol#L898 AlgebraPool.sol#L921 AlgebraPool.sol#L935 AlgebraPool.sol#L953 AlgebraPool.sol#L960 AlgebraPool.sol#L968 AlgebraPoolDeployer.sol#L22 AlgebraPoolDeployer.sol#L27 AlgebraPoolDeployer.sol#L37 AlgebraPoolDeployer.sol#L38 DataStorageOperator.sol#L27 DataStorageOperator.sol#L43 DataStorageOperator.sol#L45 DataStorageOperator.sol#L46 DataStorage.sol#L238 DataStorage.sol#L369 PriceMovementMath.sol#L52 PriceMovementMath.sol#L53 PriceMovementMath.sol#L70 PriceMovementMath.sol#L71 PriceMovementMath.sol#L87 TickManager.sol#L96 TickTable.sol#L15 TokenDeltaMath.sol#L30 TokenDeltaMath.sol#L51 PoolState.sol#L41

7. Using private rather than public for constants, saves gas

Description

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

Lines in the code

DataStorage.sol#L12

8. Use a more recent version of solidity

Description

Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value. https://github.com/ethereum/solidity/releases

Lines in the code

AlgebraFactory.sol#L2 AlgebraPool.sol#L2 AlgebraPoolDeployer.sol#L2 DataStorageOperator.sol#L2 AdaptiveFee.sol#L2 Constants.sol#L2 DataStorage.sol#L2 PriceMovementMath.sol#L2 TickManager.sol#L2 TickTable.sol#L2 TokenDeltaMath.sol#L2 PoolImmutables.sol#L2 PoolState.sol#L2

9. Initialize variables with default values are not needed

Description

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address,...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Lines in the code

DataStorage.sol#L307

10. Using bools for storage incurs overhead

Description

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from �false� to �true�, after having been �true� in the past

Lines in the code

AlgebraPool.sol#L293 AlgebraPool.sol#L294 AlgebraPool.sol#L680 AlgebraPool.sol#L686 AlgebraPool.sol#L695 AlgebraPool.sol#L728 DataStorage.sol#L15 DataStorage.sol#L161 DataStorage.sol#L166 TickManager.sol#L27 PoolState.sol#L15

11. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()

Lines in the code

AlgebraFactory.sol#L123 AlgebraPoolDeployer.sol#L51

12. Multiplication/division by two should use bit shifting

Description

<x> * 2 is equivalent to <x> << 1 and <x> / 2 is the same as <x> >> 1. The MUL and DIV opcodes cost 5 gas, whereas SHL and SHR only cost 3 gas

Lines in the code

AdaptiveFee.sol#L88

13. Using storage instead of memory for structs/arrays

Description

When retrieving data from a memory location, assigning the data to a memory variable causes all fields of the struct/array to be read from memory, resulting in a Gcoldsload (2100 gas) for each field of the struct/array. When reading fields from new memory variables, they cause an extra MLOAD instead of a cheap stack read. Rather than declaring a variable with the memory keyword, it is much cheaper to declare a variable with the storage keyword and cache all fields that need to be read again in a stack variable, because the fields actually read will only result in a Gcoldsload.

The only case where the entire struct/array is read into a memory variable is when the entire struct/array is returned by a function, passed to a function that needs memory, or when the array/struct is read from another store array/struc

Lines in the code

DataStorage.sol#L218