QA Report

[code423n4]

Floating pragma is used for DiamondInit.sol

All the contracts are using the same fixed solidity version 0.8.14, but DiamondInit.sol is using floating ^0.8.0.

Missing non-zero address checks when setting important addresses

It is a good practice to include 0 address check while updating an important address. I suggest to include a non-zero address check for the functions below.

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L158
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L163
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L168

It would be better to change setAdmin() to a two-step process

setAdmin() is called by the current admin and changes the admin address. If the newAdmin address is entered wrong for some reason, the admin role would be lost.
Therefore, it is a better approach to follow a 2-step process when updating a priviliged adress. First transaction proposes the new admin address, second transaction which can only be called by the proposed address accepts the role.

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L168

Each event can have up to three indexed fields.

Many events do not have indexed fields. Consider using more indexed fields for better log filtering.

Lines of code

There are many lines, some of which are:
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L65-L69
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/AssetFacet.sol#L27
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/AssetFacet.sol#L35
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/AssetFacet.sol#L44

Missing non-zero address checks for token transfers

Tokens would be burned if sent to zero address accidentally. Therefore, it is a good practice to include non-zero address checks for token transfers.

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/AssetLogic.sol#L145
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/SponsorVault.sol#L296
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/SwapUtils.sol#L1059

TODO comments

There are a few TODO comments, meaning there is incomplete work or expected changes. Either the TODO comments should be addressed or deleted.

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/BridgeFacet.sol#L492
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/BridgeFacet.sol#L579
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/BridgeFacet.sol#L1027
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/Executor.sol#L7
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/LibConnextStorage.sol#L303