QA Report - Githubissues

code423n4 commented 2 years ago

Issue 1 (Low) - Avoid Floating Pragma

Details

Most or all contracts include a floating pragma ^0.8.7. It is recommended that the compiler version be locked to a single version to avoid contracts deployed with different versions that may contain various compiler bugs.

Issue 2 (Low) - No Transfer Ownership Pattern

Link to Code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/OwnershipFacet.sol#L7-L11

Details

It is recommended to employ a transfer/accept ownership transfer pattern. This will avoid accidently transferring ownership to an incorrect address, as the accept function must be called by the pending owner for the transfer to be completed.

Issue 3 (Low) - LifiData can be set arbitrarily, dimenishing useability in analytics

Link to Code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22

Details

The comments specify:

_lifiData data used purely for tracking and analytics

The function uses the _lifiData attributes such as transactionId for logging, but this value can be arbitrary by the msg.sender.

Issue 4 (low) - Unnecessary transfer amount to LibSwap.sol

Link to Code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L33-L35

Details

The swap function contains logic that the fromAmount must be transferred to LibSwap.sol if the contract balance is less than the fromAmount. Given the possiblity that the contract already contains some number of tokens, the entire from amount will still be transferred to the contract, and subsequently swapped, leaving the same remainder tokens as before. Given the functionality that checks if the contract balance is equal to or larger than the from amount in the first place, it would make sense to only send the difference between the fromAmount and the contract balance.

Issue 5 (low) - Use of deprecated safeApprove() function

Link to Code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibAsset.sol#L67-L68