LI.FI contest - PPrieditis's results

Title: Unlocked pragma

Impact

Contracts should be deployed using the same compiler version/flags with which they have been tested. Locking the pragma (for e.g. by not using ^ in pragma solidity 0.5.10) ensures that contracts do not accidentally get deployed using an older compiler version with unfixed bugs. More information can be seen in SWC Registry: https://swcregistry.io/docs/SWC-103

Recommended Mitigation Steps

Use locked pragma for contracts.

---

Title: LiFiDiamond.sol is missing EIP-2535 dependency

Impact

It should be clearly visible in code which part is customization and which part comes from public standard. Documentation states that LI.FI Contract is built using the EIP-2535 (Multi-facet Proxy) standard however in code that is not clearly visible. LiFiDiamond.sol logic is a copy from Diamond.sol. It would be better to use import and inheritance because if there is discovered a bug in EIP-2535 Diamond.sol then it is harder to detect the same bug in LiFiDiamond.sol.

Recommended Mitigation Steps

Use import and inheritance from Diamond.sol https://github.com/mudgen/diamond-3-hardhat/blob/main/contracts/Diamond.sol https://github.com/code-423n4/2022-03-lifinance/blob/main/src/LiFiDiamond.sol#L7

---

Title: Unused imports

Impact

Contracts inheriting from multiple contracts with identical functions should specify the correct inheritance order i.e. more general to more specific to avoid inheriting the incorrect function implementation. https://swcregistry.io/docs/SWC-125

Recommended Mitigation Steps

Remove unnecessary imports. In the following files there are contract imports that aren't used: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L6 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L9

---

Title: In place of a number use type(T).max

Impact

type(T).max should be used for max value and not a number.

Recommended Mitigation Steps

Change: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L8

From: uint256 private constant MAX_INT = 2**256 - 1; To: uint256 private constant MAX_INT = type(uint256).max;

---

Title: Unused inheritance functionality

Impact

Contract should not inherit if functionality of inheritance is not used. Swapper.sol inherits ILiFi however nothing is used from ILiFi.

Recommended Mitigation Steps

Remove: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L4 and ILiFi inheritance at https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L8

---

Title: Use consistently index

Impact

Parameters of certain events are expected to be indexed (e.g. ERC20 Transfer/Approval events) so that they’re included in the block’s bloom filter for faster access. Failure to do so might confuse off-chain tooling looking for such indexed events.

Event AssetSwapped does not use any index: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L19-L27

Recommended Mitigation Steps

Index transactionId for event AssetSwapped because transactionId is indexed also in ILiFi.sol https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Interfaces/ILiFi.sol

Title: Cache array length in for loops can save gas

Impact

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack. Caching the array length in the stack saves around 3 gas per iteration.

Recommended Mitigation Steps

Cache array length: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L14

---

Title: Prefix increaments are cheaper than postfix increaments

Impact

Prefix increaments are cheaper than postfix increaments

Recommended Mitigation Steps

Use ++i in place of i++: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L14

---

Title: Redundant constructs

Impact

Redundant code and comments can be confusing and should be removed (or changed appropriately) after careful evaluation. This will not only reduce gas costs but improve readability and maintainability of the code.

Recommended Mitigation Steps

Change: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L16

From: ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true To: ls.dexWhitelist[_swapData[i].approveTo] && ls.dexWhitelist[_swapData[i].callTo]

---

Title: Functions can be external

Impact

Public functions that are never called by the contract should be declared external to save gas.

Recommended Mitigation Steps

Change below functions to external: AnyswapFacet.startBridgeTokensViaAnyswap() https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L35

AnyswapFacet.swapAndStartBridgeTokensViaAnyswap() https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L74