Basin - 0x11singh99's results

Low-Level and Non-Critical issues

Summary

Low Level List

Total 3 Low Level Issues

Non Critical List

Total 5 Non Critical Issues

Low-Level issues:

[L-01] Check receiver address for address(0) before transferring or minting any tokens OR Ethers

3 Instances

Instance#1-2: Make sure recipient is not address(0) before transfer ,here in _swapFrom to fail early

File: /src/Well.sol
195:  amountOut = _swapFrom(fromToken, toToken, amountIn, minAmountOut, recipient);
       ...
212:   amountOut = _swapFrom(fromToken, toToken, amountIn, minAmountOut, recipient);
      ...
304:        toToken.safeTransfer(recipient, amountOut);

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L195 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L212 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L304

[L-02] Check amount for 0 before transferring tokens.

Check before trasferring zero money unnecessary. 6 Instances

Instance#1-6: Make sure amountIn is not 0 before transfer

File: /src/Well.sol
194:  fromToken.safeTransferFrom(msg.sender, address(this), amountIn);
195:  amountOut = _swapFrom(fromToken, toToken, amountIn, minAmountOut, recipient);
     ...
211:   amountIn = _safeTransferFromFeeOnTransfer(fromToken, msg.sender, amountIn);
212:   amountOut = _swapFrom(fromToken, toToken, amountIn, minAmountOut, recipient);
     ...
303:    fromToken.safeTransferFrom(msg.sender, address(this), amountIn);
304:        toToken.safeTransfer(recipient, amountOut);

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L194-L195 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L211-L212 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L303-L304

[L-03] Use Check effect interaction flow to avoid reentrancy

1 Instance

Instance#1: Make sure to update mapping reserves[j] before transferring tokens to outside

File: /src/Well.sol
 371:     tokenOut.safeTransfer(recipient, amountOut);
 372:         reserves[j] -= amountOut;

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L371C5-L372C45

Non Critcal Issues

[NC-01] : Named return is confusing, use explicit returns

Return from function explicitly make code readable

4 Instances

Instance# 1-3 :

File: src/pumps/MultiFlowPump.sol
171:  function readLastReserves(address well) public view returns (uint256[] memory reserves) {
         ...
199:      function _capReserve(
        bytes16 lastReserve,
        bytes16 reserve,
        bytes16 blocksPassed
203:    ) internal view returns (bytes16 cappedReserve) {
         ...
222:  function readLastInstantaneousReserves(address well) public view returns (uint256[] memory reserves) {

https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L171 https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L199C3-L203C54 https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L222

Instance#4 : Doing both explicit return and named return is confusing and ambiguous,do one thing only and explicit return is more better and clear.

File: /src/Aquifer.sol
86:  function wellImplementation(address well) external view returns (address implementation) {
87:       return wellImplementations[well];

https://github.com/code-423n4/2023-07-basin/blob/main/src/Aquifer.sol#L86C4-L87C42

[NC-02] : Verify constructor params specially for immutables to check 0 values

Verify constructor params for 0 values by mistake otherwise they will be in the contract forever Or make contract re-deploying. 1 Instance

Instance#1:

File: src/pumps/MultiFlowPump.sol
 54:  constructor(bytes16 _maxPercentIncrease, bytes16 _maxPercentDecrease, uint256 _blockTime, bytes16 _alpha) {
        LOG_MAX_INCREASE = ABDKMathQuad.ONE.add(_maxPercentIncrease).log_2();
        // _maxPercentDecrease <= 100%
        if (_maxPercentDecrease > ABDKMathQuad.ONE) {
            revert InvalidMaxPercentDecreaseArgument(_maxPercentDecrease);
        }
        LOG_MAX_DECREASE = ABDKMathQuad.ONE.sub(_maxPercentDecrease).log_2();
        BLOCK_TIME = _blockTime;

        // ALPHA <= 1
        if (_alpha > ABDKMathQuad.ONE) {
            revert InvalidAArgument(_alpha);
        }
        ALPHA = _alpha;
 68:   }

https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L54C3-L68C6

[NC-03] Named params of mapping is more readable

1 Instance

Instance#1:

File: /src/Aquifer.sol
25:    mapping(address => address) wellImplementations;

https://github.com/code-423n4/2023-07-basin/blob/main/src/Aquifer.sol#L25

[NC-04] Use ternary operator instead of if else

Using ternary makes code concise and more gas efficient

3 Instance

Instance#1-3:

File: /src/Aquifer.sol

40:   if (immutableData.length > 0) {
            if (salt != bytes32(0)) {
                well = implementation.cloneDeterministic(immutableData, salt);
            } else {
                well = implementation.clone(immutableData);
            }
        } else {
            if (salt != bytes32(0)) {
                well = implementation.cloneDeterministic(salt);
            } else {
                well = implementation.clone();
            }
52:        }

https://github.com/code-423n4/2023-07-basin/blob/main/src/Aquifer.sol#L40C9-L52C10

[NC-05] :Some Contracts are not following proper solidity style guide layout

In some contract variables are declared after function and state changing functions are after view functions which is not proper code layout of solidity. State variable declaration after functions is not proper code layout of solidity. 1 Instance

Instance#1 :

File: src/Well.sol
31:   function init(string memory name, string memory symbol) public initializer {
32:        __ERC20Permit_init(name);
33:        __ERC20_init(name, symbol);

            ...
77:   uint256 constant LOC_AQUIFER_ADDR = 0;
    uint256 constant LOC_TOKENS_COUNT = LOC_AQUIFER_ADDR + PACKED_ADDRESS;
79:    uint256 constant LOC_WELL_FUNCTION_ADDR = LOC_TOKENS_COUNT + ONE_WORD;

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L31-L82

Recommendation: Update using below guide layout of variables Inside each contract, library or interface, use the following order:

Type declarations State variables Events Errors Modifiers Functions

https://docs.soliditylang.org/en/latest/style-guide.html#order-of-layout

Order of Functions Ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.

Functions should be grouped according to their visibility and ordered:

constructor receive function (if exists) fallback function (if exists) external public internal private

https://docs.soliditylang.org/en/latest/style-guide.html#order-of-functions

Gas Optimizations List

Total 7 issues

[G-01] Do not calculate constant variables

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas each time of use.

Total 6 instances - 1 files:

Instance#1-6 : Assign direct simple constant value after calculating off chain

File : /src/Well.sol

29: bytes32 constant RESERVES_STORAGE_SLOT = bytes32(uint256(keccak256("reserves.storage.slot")) - 1);
         ...
78:    uint256 constant LOC_TOKENS_COUNT = LOC_AQUIFER_ADDR + PACKED_ADDRESS;
79:    uint256 constant LOC_WELL_FUNCTION_ADDR = LOC_TOKENS_COUNT + ONE_WORD;
80:    uint256 constant LOC_WELL_FUNCTION_DATA_LENGTH = LOC_WELL_FUNCTION_ADDR + PACKED_ADDRESS;
81:    uint256 constant LOC_PUMPS_COUNT = LOC_WELL_FUNCTION_DATA_LENGTH + ONE_WORD;
82:    uint256 constant LOC_VARIABLE = LOC_PUMPS_COUNT + ONE_WORD;

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L29C5-L30C1 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L78C4-L82C64

[G-02] Check recipient for address(0) to fail early rather than checking inside function call

Total 4 instances - 1 files:

Instance#1 : recipient can be checked for address(0) in starting of external function to fail early.

File : /src/Well.sol

195: amountOut = _swapFrom(fromToken, toToken, amountIn, minAmountOut, recipient);
       ...
212:  amountOut = _swapFrom(fromToken, toToken, amountIn, minAmountOut, recipient);
       ...
398:  lpAmountOut = _addLiquidity(tokenAmountsIn, minLpAmountOut, recipient, false);
     ...
407:  lpAmountOut = _addLiquidity(tokenAmountsIn, minLpAmountOut, recipient, true);

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L195 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L212 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L398 https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L407

[G-03] Write for loops in more gas efficient way

Loop can be written in more gas efficient way, by

1. taking length in stack variable,
2. using unchecked{++i} with pre increment
3. not re-initializing to 0 since it is default
4. Using <= is cheaper than <

6 instances - 1 file:

Instance#1:

File: src/pumps/MultiFlowPump.sol
301:    for (uint256 i; i < cumulativeReserves.length; ++i) {

https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L301

Recommended Changes: Convert above for loop to this for making gas efficient saves more than 130 Gas per iteration

-      for (uint256 i; i < cumulativeReserves.length; ++i) {
        ...

        }

+  uint256 _size = cumulativeReserves.length - 1;
+  for (uint256 i; i <= _size ; ) {

              ...
+            unchecked {
+                ++i;
+            }
        }

Instance#2:

File: src/pumps/MultiFlowPump.sol
322:   for (uint256 i; i < byteCumulativeReserves.length; ++i) {

https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L322 Update like above

Instance#3:

File: /src/Well.sol
36:   for (uint256 i; i < _tokens.length - 1; ++i) {
37:            for (uint256 j = i + 1; j < _tokens.length; ++j) {

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L36C6-L37C63 Update like first instance

Instance#4:

File: /src/Well.sol
101:     for (uint256 i; i < _pumps.length; i++) {

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L101C7-L101C50 Update like first instance

Instance#5:

File: /src/Well.sol
363:      for (uint256 i; i < _tokens.length; ++i) {

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L363C8-L363C51 Update like first instance

Instance#6:

File: /src/Well.sol
382:  for (uint256 i; i < _tokens.length; ++i) {

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L382C8-L382C51 Update like first instance

Note: These types of instances are many more in the Well.sol file so make sure to update them for gas efficiency.

[G‑04] Memory variables should be cached in stack(if possible) variables rather than re-reading them from memory

It saves extra MLOAD opcode and saves gas

5 instances - 2 files:

Instance#1-2: pumpState.lastTimestamp and reserves[i] can be cached

File: src/pumps/MultiFlowPump.sol

72: function update(uint256[] calldata reserves, bytes calldata) external {
        ...
83:    if (pumpState.lastTimestamp == 0){
        ...
109:   uint256 deltaTimestamp = _getDeltaTimestamp(pumpState.lastTimestamp);
        ...
119:    pumpState.lastReserves[i], (reserves[i] > 0 ? reserves[i] : 1).fromUIntToLog2(), blocksPassed
        ...
140:   }

https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L83 https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L109 https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L119

Instance#3: reserves[i] can be cached to stack variable

File: src/pumps/MultiFlowPump.sol

153:   if (reserves[i] == 0) return;
154:         byteReserves[i] = reserves[i].fromUIntToLog2();

https://github.com/code-423n4/2023-07-basin/blob/main/src/pumps/MultiFlowPump.sol#L153C1-L154C60

Instance#4-5: tokenAmountsOut[i] and minTokenAmountsOut[i] can be cached to stack variable

 File: src/Well.sol
 473:    for (uint256 i; i < _tokens.length; ++i) {
            if (tokenAmountsOut[i] < minTokenAmountsOut[i]) {
                revert SlippageOut(tokenAmountsOut[i], minTokenAmountsOut[i]);
            }
            _tokens[i].safeTransfer(recipient, tokenAmountsOut[i]);
            reserves[i] = reserves[i] - tokenAmountsOut[i];
 479:       }

https://github.com/code-423n4/2023-07-basin/blob/main/src/Well.sol#L473C6-L479C10

Note: These types of instances are many more in the Well.sol file so make sure to update them for gas efficiency.

[G-05] Function calls can be cached rather than re calling from same function with same value will save gas

2 instances - 1 file:

Instance#1 : LibContractInfo.getSymbol(address(_tokens[i])) can be cached instead of 2 time reading for each iteration

File : /src/libraries/LibWellConstructor.sol

73:   for (uint256 i = 1; i < _tokens.length; ++i) {
74:           name = string.concat(name, ":", LibContractInfo.getSymbol(address(_tokens[i])));
75:          symbol = string.concat(symbol, LibContractInfo.getSymbol(address(_tokens[i])));
76:        }

https://github.com/code-423n4/2023-07-basin/blob/main/src/libraries/LibWellConstructor.sol#L73C8-L76C10

[G-06] Use unchecked{} whenever underflow/overflow not possible saves 130 gas each time

Because of above check, it can be decided that underflow/overflow not possible.

1 instance - 1 file:

Instance#1 : i-1 can be unchecked due to line 93 for () condition initialization and increment

File: src/libraries/LibLastReserveBytes.sol

 93:         for (uint256 i = 3; i <= n; ++i) {
                // `iByte` is the byte position for the current slot:
                // i        3 4 5 6
                // iByte    1 1 2 2
 97:               iByte = ((i - 1) / 2) * 32;
                ...
          }

https://github.com/code-423n4/2023-07-basin/blob/main/src/libraries/LibLastReserveBytes.sol#L93C9-L97C42

[G-07] Using ternary operator instead of if else saves gas

1 instance - 1 file:

Instance#1 :

File: /src/Aquifer.sol
40:   if (immutableData.length > 0) {
            if (salt != bytes32(0)) {
                well = implementation.cloneDeterministic(immutableData, salt);
            } else {
                well = implementation.clone(immutableData);
            }
        } else {
            if (salt != bytes32(0)) {
                well = implementation.cloneDeterministic(salt);
            } else {
50:                well = implementation.clone();

https://github.com/code-423n4/2023-07-basin/blob/main/src/Aquifer.sol#L40C9-L50C14