QuickSwap and StellaSwap contest - mics's results

Table Of Content

• QA REPORT * Unused success return value * Use safe math for solidity version <8 * SPDX license not provided in source file * Require with empty error message * Loss of precision by using division over possible multiplication * Use safeTransfer() instead transfer() * Array access is out of bounds * Missing two steps verification process * Missing 0 address check at transfer * Magical number should be documented and explained. Use a constant instead * Several functions are declaring named returns but then are using return statements. I suggest choosing only one for readability reasons. * Consider adding constant variables instead of hardcoded strings * Events not emitted for important state changes * Add event to the following functions

QA REPORT

Unused success return value

The following calls ignores the return value of the called function that might indicate the the call failed.

Code Instances:

• TestAlgebraRouter.sol#L63
• AlgebraPoolSwapTest.sol#L40
• AlgebraPoolSwapTest.sol#L38
• TestAlgebraRouter.sol#L65

Use safe math for solidity version <8

You should use safe math for solidity version <8 since there is no default over/under flow check it those versions.'

Code Instances:

• DataStorageEchidnaTest.sol
• AlgebraFactory.sol
• TestAlgebraReentrantCallee.sol
• PriceMovementMathTest.sol

SPDX license not provided in source file

Before publishing, consider adding a comment containing 'SPDX-License-Identifier: MIT' at the beginning of each source file.

Code Instances:

• TestAlgebraReentrantCallee.sol
• AdaptiveFeeTest.sol
• TickTableTest.sol
• FullMathEchidnaTest.sol

Require with empty error message

The following instances of a require statements comes with no popper error message. That means in case of error thrown the user will not know the reason for the error. Consider adding an error message.

Code Instances:

• PriceMovementMathEchidnaTest.sol#L14
• TokenDeltaMathEchidnaTest.sol#L234
• TokenDeltaMathEchidnaTest.sol#L232
• TickTableEchidnaTest.sol#L33

Loss of precision by using division over possible multiplication

In cases of computing a / b < c you could improve precision by doing instead a < c * b.

Code Instances:

• DataStorageOperator.sol#L137
• DataStorageOperator.sol#L138

Use safeTransfer() instead transfer()

Use openzeppelin safeTransfer() method instead of transfer() in the following locations.

Code Instances:

• TestAlgebraCallee.sol#L167
• TestAlgebraCallee.sol#L101
• TestAlgebraCallee.sol#L139
• TestAlgebraSwapPay.sol#L81

Array access is out of bounds

There is no check for the access to be in the array bounds.

Code Instances:

• DataStorageOperator.sol#L70
• DataStorageTest.sol#L100

Missing two steps verification process

The process of transferring ownership is dangerous since typing the wrong address can lead to severe implications. It is better to have to steps verification process with set and claim functions to decrease the chances of human error. Consider changing to two steps verification process of transferring privileges. Human mistakes can happen.

Code Instances:

• AlgebraFactory.sol
• SimulationTimeFactory.sol

Missing 0 address check at transfer

Some contracts does not support 0 transfer, then the transaction will revert with no explanation. We recommend to add a require statement that the amount is not 0.

Code Instances:

• AlgebraPool.sol#L912
• AlgebraPoolSwapTest.sol#L38
• AlgebraPool.sol#L547
• TestAlgebraSwapPay.sol#L82

Magical number should be documented and explained. Use a constant instead

Code Instances:

• DataStorageEchidnaTest.sol#L69
• DataStorageEchidnaTest.sol#L100
• DataStorageEchidnaTest.sol#L136
• TickTableEchidnaTest.sol#L29

Several functions are declaring named returns but then are using return statements. I suggest choosing only one for readability reasons.

Using both named returns and a return statement isn't necessary. Removing one of those can improve code clarity.

Code Instances:

• AlgebraPool.sol#L557
• SafeMathTest.sol#L40
• AdaptiveFeeTest.sol#L21
• PriceMovementMathTest.sol#L22

Consider adding constant variables instead of hardcoded strings

A good practice is to use constant variables instead of hardcoded strings in the code.

Code Instances:

• TestERC20.sol#L25
• AlgebraFactory.sol#L108
• TestERC20.sol#L66
• AlgebraPool.sol#L934

Events not emitted for important state changes

When changing state variables events are not emitted. Emitting events allows monitoring activities with off-chain monitoring tools.

Code Instances:

• TestERC20.sol#L45
• SimulationTimePoolDeployer.sol#L49
• DataStorageTest.sol#L24
• DataStorageTest.sol#L42

Add event to the following functions

Code Instances:

• DataStorageEchidnaTest.sol#L21
• DataStorageTest.sol#L24
• TickOverflowSafetyEchidnaTest.sol#L41
• TestERC20.sol#L45

Table Of Content

• GAS REPORT * Not Efficient Struct Packing * Use assembly opcodes iszero instead of solidity equation to save gas * Caching array size

GAS REPORT

Not Efficient Struct Packing

By reordering the struct variables you can decrease the number of slots in use and therefore reduce the gas cost of using the struct.

For instance, AlgebraPool.sol#L675

Use assembly opcodes iszero instead of solidity equation to save gas

Code Instances:

• TickOverflowSafetyEchidnaTest.sol#L75
• BitMathEchidnaTest.sol#L17
• AlgebraPool.sol#L193
• LowGasSafeMathEchidnaTest.sol#L21

Caching array size

In the following for loops consider caching the array size instead of loading it every iteration.

For instance, DataStorageTest.sol#L55