LI.FI contest - Dravee's results

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/WithdrawFacet.sol#L31

Vulnerability details

This is similar to a previous Code4rena issue: https://github.com/code-423n4/2021-04-meebits-findings/issues/2

POC

See L31:

File: WithdrawFacet.sol
20:     function withdraw(
21:         address _assetAddress,
22:         address _to,
23:         uint256 _amount
24:     ) public {
25:         LibDiamond.enforceIsContractOwner();
26:         address sendTo = (_to == address(0)) ? msg.sender : _to;
27:         uint256 assetBalance;
28:         if (_assetAddress == NATIVE_ASSET) {
29:             address self = address(this); // workaround for a possible solidity bug
30:             assert(_amount <= self.balance); () instead of require()
31:             payable(sendTo).transfer(_amount); //@audit medium risk
32:         } else {
33:             assetBalance = IERC20(_assetAddress).balanceOf(address(this));
34:             assert(_amount <= assetBalance); () instead of require()
35:             IERC20(_assetAddress).safeTransfer(sendTo, _amount);
36:         }
37:         emit LogWithdraw(sendTo, _assetAddress, _amount);
38:     }

For the withdraw logic, this function uses the deprecated transfer() function on an address. This transaction will fail inevitably when:

1. The claimer smart contract does not implement a payable function.
2. The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit.
3. The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, raising the call's gas usage above 2300.

I recommend using call() instead of transfer()

QA Report

1. Avoid floating pragmas: the version should be locked at 0.8.7

Facets/AnyswapFacet.sol:2:pragma solidity ^0.8.7;
Facets/CBridgeFacet.sol:2:pragma solidity ^0.8.7;
Facets/DexManagerFacet.sol:2:pragma solidity ^0.8.7;
Facets/DiamondCutFacet.sol:2:pragma solidity ^0.8.7;
Facets/DiamondLoupeFacet.sol:2:pragma solidity ^0.8.7;
Facets/GenericSwapFacet.sol:2:pragma solidity ^0.8.7;
Facets/HopFacet.sol:2:pragma solidity ^0.8.7;
Facets/NXTPFacet.sol:2:pragma solidity ^0.8.7;
Facets/OwnershipFacet.sol:2:pragma solidity ^0.8.7;
Facets/Swapper.sol:2:pragma solidity ^0.8.7;
Facets/WithdrawFacet.sol:2:pragma solidity ^0.8.7;
Libraries/LibAsset.sol:2:pragma solidity ^0.8.7;
Libraries/LibDiamond.sol:2:pragma solidity ^0.8.7;
Libraries/LibStorage.sol:2:pragma solidity ^0.8.7;
Libraries/LibSwap.sol:2:pragma solidity ^0.8.7;
Libraries/LibUtil.sol:2:pragma solidity ^0.8.7;
LiFiDiamond.sol:2:pragma solidity ^0.8.7;

2. require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking

Properly functioning code should never reach a failing assert statement, unless there is a bug in your contract you should fix. Here, I believe the assert should be a require or a revert:

src/Facets/WithdrawFacet.sol:
  30:             assert(_amount <= self.balance); //@audit low : Usage of assert() instead of require()
  34:             assert(_amount <= assetBalance); //@audit low : Usage of assert() instead of require()

As the Solidity version is > 0.8.0, the remaining gas would still be refunded in case of failure.

3. LibDiamond.sol:setContractOwner() and OwnershipFacet.sol:transferOwnership() should use a 2-step ownership transfer pattern
4. Funds can be locked in LiFiDiamond.sol

There isn't a withdraw mechanism and several payable methods are implemented:

src/LiFiDiamond.sol:
   8:     constructor(address _contractOwner, address _diamondCutFacet) payable {
  26:     fallback() external payable {
  62:     receive() external payable {}

5. Comment says "public" instead of "internal" :

File: CBridgeFacet.sol
172:     /*
173:      * @dev Public view function for the CBridge router address //@audit internal, not public
174:      * @returns the router address
175:      */
176:     function _bridge() internal view returns (address) {

Gas Report

Table of Contents:

• Gas Report
  • Findings
    • Storage
      • Help the optimizer by declaring a storage variable instead of repeatedly fetching a value in storage (for reading and writing)
      • Emitting storage values
    • Variables
      • No need to explicitly initialize variables with default values
      • "constants" expressions are expressions, not constants
    • Comparisons
      • Boolean comparisons
      • > 0 is less efficient than != 0 for unsigned integers (with proof)
    • For-Loops
      • An array's length should be cached to save gas in for-loops
      • ++i costs less gas compared to i++
      • Increments can be unchecked
    • Arithmetics
      • unchecked blocks for arithmetics operations that can't underflow/overflow
    • Visibility
      • Public functions to external
    • Errors
      • Reduce the size of error messages (Long revert Strings)
      • Use Custom Errors instead of Revert Strings to save Gas

Findings

Storage

Help the optimizer by declaring a storage variable instead of repeatedly fetching a value in storage (for reading and writing)

To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the value in a map or an array.

The effect can be quite significant.

Instances include (check the @audit tags):

src/Facets/DexManagerFacet.sol:
  20:         if (s.dexWhitelist[_dex] == true) { //@audit gas: fetch 1 for "s.dexWhitelist[_dexs]" (help the optimizer by storing this in a storage variable)
  24:         s.dexWhitelist[_dex] = true; //@audit gas: fetch 2 for "s.dexWhitelist[_dex]" (help the optimizer by using the previously suggested storage variable for this SSTORE)

  34:             if (s.dexWhitelist[_dexs[i]] == true) { //@audit gas: fetch 1 for "s.dexWhitelist[_dexs[i]]" (help the optimizer by storing this in a storage variable)
  37:             s.dexWhitelist[_dexs[i]] = true; //@audit gas: fetch 2 for "s.dexWhitelist[_dexs[i]]" (help the optimizer by using the previously suggested storage variable for this SSTORE)

  47:         if (s.dexWhitelist[_dex] == false) { //@audit gas: fetch 1 for "s.dexWhitelist[_dexs]" (help the optimizer by storing this in a storage variable)
  51:         s.dexWhitelist[_dex] = false; //@audit gas: fetch 2 for "s.dexWhitelist[_dex]" (help the optimizer by using the previously suggested storage variable for this SSTORE)
  
  66:             if (s.dexWhitelist[_dexs[i]] == false) { //@audit gas: fetch 1 for "s.dexWhitelist[_dexs[i]]" (help the optimizer by storing this in a storage variable)
  69:             s.dexWhitelist[_dexs[i]] = false; //@audit gas: fetch 2 for "s.dexWhitelist[_dexs[i]]" (help the optimizer by using the previously suggested storage variable for this SSTORE)

Emitting storage values

Here, the values emitted shouldn't be read from storage. The existing memory values should be used instead:

src/Facets/CBridgeFacet.sol:
  45          s.cBridge = _cBridge;
  46          s.cBridgeChainId = _chainId;
  47:         emit Inited(s.cBridge, s.cBridgeChainId); //@audit gas: should emit Inited(_cBridge, _chainId)

Variables

No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Instances include:

Facets/WithdrawFacet.sol:10:    address private constant NATIVE_ASSET = address(0);
Libraries/LibAsset.sol:21:    address internal constant NATIVE_ASSETID = address(0);

I suggest removing explicit initializations for default values.

"constants" expressions are expressions, not constants

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.

If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated.

See: ethereum/solidity#9232

Consequences: each usage of a "constant" costs ~100gas more on each access (it is still a little better than storing the result in storage, but not much..). since these are not real constants, they can't be referenced from a real constant environment (e.g. from assembly, or from another library )

Facets/CBridgeFacet.sol:18:    bytes32 internal constant NAMESPACE = keccak256("com.lifi.facets.cbridge2");
Facets/HopFacet.sol:18:    bytes32 internal constant NAMESPACE = keccak256("com.lifi.facets.hop");
Facets/NXTPFacet.sol:18:    bytes32 internal constant NAMESPACE = keccak256("com.lifi.facets.nxtp");
Libraries/LibDiamond.sol:7:    bytes32 internal constant DIAMOND_STORAGE_POSITION = keccak256("diamond.standard.diamond.storage");

Change these expressions from constant to immutable and implement the calculation in the constructor or hardcode these values in the constants and add a comment to say how the value was calculated.

Comparisons

Boolean comparisons

Comparing to a constant (true or false) is a bit more expensive than directly checking the returned boolean value. I suggest using if(directValue) instead of if(directValue == true) and if(!directValue) instead of if(directValue == false) here:

Facets/DexManagerFacet.sol:20:        if (s.dexWhitelist[_dex] == true) {
Facets/DexManagerFacet.sol:34:            if (s.dexWhitelist[_dexs[i]] == true) {
Facets/DexManagerFacet.sol:47:        if (s.dexWhitelist[_dex] == false) {
Facets/DexManagerFacet.sol:66:            if (s.dexWhitelist[_dexs[i]] == false) {
Facets/Swapper.sol:16:                ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,

> 0 is less efficient than != 0 for unsigned integers (with proof)

!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)

Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're in a require statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706

I suggest changing > 0 with != 0 here:

Facets/AnyswapFacet.sol:92:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/AnyswapFacet.sol:105:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/CBridgeFacet.sol:105:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/CBridgeFacet.sol:116:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/HopFacet.sol:109:        require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/NXTPFacet.sol:98:        require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Libraries/LibDiamond.sol:84:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:102:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:121:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:189:            require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
Libraries/LibDiamond.sol:212:        require(contractSize > 0, _errorMessage);

Also, please enable the Optimizer.

For-Loops

An array's length should be cached to save gas in for-loops

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.

Here, I suggest storing the array's length in a variable before the for-loop, and use it instead:

Facets/DexManagerFacet.sol:33:        for (uint256 i; i < _dexs.length; i++) {
Facets/DexManagerFacet.sol:52:        for (uint256 i; i < s.dexs.length; i++) {
Facets/DexManagerFacet.sol:65:        for (uint256 i; i < _dexs.length; i++) {
Facets/DexManagerFacet.sol:70:            for (uint256 j; j < s.dexs.length; j++) {
Facets/HopFacet.sol:48:        for (uint8 i; i < _tokens.length; i++) {
Facets/Swapper.sol:14:        for (uint8 i; i < _swapData.length; i++) {
Libraries/LibDiamond.sol:67:        for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
Libraries/LibDiamond.sol:92:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
Libraries/LibDiamond.sol:110:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
Libraries/LibDiamond.sol:125:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

This is already done here:

Facets/DiamondLoupeFacet.sol:24:        for (uint256 i; i < numFacets; i++) {

++i costs less gas compared to i++

++i costs less gas compared to i++ for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration)

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2

Instances include:

Facets/DexManagerFacet.sol:33:        for (uint256 i; i < _dexs.length; i++) {
Facets/DexManagerFacet.sol:52:        for (uint256 i; i < s.dexs.length; i++) {
Facets/DexManagerFacet.sol:65:        for (uint256 i; i < _dexs.length; i++) {
Facets/DexManagerFacet.sol:70:            for (uint256 j; j < s.dexs.length; j++) {
Facets/DiamondLoupeFacet.sol:24:        for (uint256 i; i < numFacets; i++) {
Facets/HopFacet.sol:48:        for (uint8 i; i < _tokens.length; i++) {
Facets/Swapper.sol:14:        for (uint8 i; i < _swapData.length; i++) {
Libraries/LibDiamond.sol:67:        for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
Libraries/LibDiamond.sol:92:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
Libraries/LibDiamond.sol:97:            selectorPosition++;
Libraries/LibDiamond.sol:110:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
Libraries/LibDiamond.sol:116:            selectorPosition++;
Libraries/LibDiamond.sol:125:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

I suggest using ++i instead of i++ to increment the value of an uint variable.

Increments can be unchecked

In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

ethereum/solidity#10695

Instances include:

Facets/DexManagerFacet.sol:33:        for (uint256 i; i < _dexs.length; i++) {
Facets/DexManagerFacet.sol:52:        for (uint256 i; i < s.dexs.length; i++) {
Facets/DexManagerFacet.sol:65:        for (uint256 i; i < _dexs.length; i++) {
Facets/DexManagerFacet.sol:70:            for (uint256 j; j < s.dexs.length; j++) {
Facets/DiamondLoupeFacet.sol:24:        for (uint256 i; i < numFacets; i++) {
Libraries/LibDiamond.sol:67:        for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
Libraries/LibDiamond.sol:92:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
Libraries/LibDiamond.sol:97:            selectorPosition++;
Libraries/LibDiamond.sol:110:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
Libraries/LibDiamond.sol:116:            selectorPosition++;
Libraries/LibDiamond.sol:125:        for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

The code would go from:

for (uint256 i; i < numIterations; i++) {  
 // ...  
}  

to:

for (uint256 i; i < numIterations;) {  
 // ...  
 unchecked { ++i; }  
}  

I suggest not unchecking the increments at these places as the risk of overflow is existent for uint8 there:

Facets/HopFacet.sol:48:        for (uint8 i; i < _tokens.length; i++) {
Facets/Swapper.sol:14:        for (uint8 i; i < _swapData.length; i++) {

Arithmetics

unchecked blocks for arithmetics operations that can't underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation, or the operation doesn't depend on user input), some gas can be saved by using an unchecked block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

I suggest wrapping with an unchecked block here (see @audit tags for more details):

src/Facets/AnyswapFacet.sol:
   46:                 LibAsset.getOwnBalance(underlyingToken) - _fromTokenBalance == _anyswapData.amount, //@audit gas: should be unchecked (can't underflow)
   90:             uint256 _postSwapBalance = LibAsset.getOwnBalance(underlyingToken) - _fromTokenBalance;  //@audit gas: should be unchecked (can't underflow)
  101:             require(address(this).balance - _fromBalance >= _anyswapData.amount, "ERR_INVALID_AMOUNT");  //@audit gas: should be unchecked (can't underflow)
  103:             uint256 _postSwapBalance = address(this).balance - _fromBalance;  //@audit gas: should be unchecked (can't underflow)

src/Facets/CBridgeFacet.sol:
   64:                 LibAsset.getOwnBalance(_cBridgeData.token) - _fromTokenBalance == _cBridgeData.amount,  //@audit gas: should be unchecked (can't underflow)
  103:             uint256 _postSwapBalance = LibAsset.getOwnBalance(_cBridgeData.token) - _fromTokenBalance;  //@audit gas: should be unchecked (can't underflow)
  114:             uint256 _postSwapBalance = address(this).balance - _fromBalance;  //@audit gas: should be unchecked (can't underflow)

src/Facets/DexManagerFacet.sol:
  85:         s.dexs[index] = s.dexs[s.dexs.length - 1];  //@audit gas: should be unchecked (used in a for-loop that wouldn't call this function if "s.dexs.length == 0")

src/Facets/GenericSwapFacet.sol:
  28:         uint256 postSwapBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId) - receivingAssetIdBalance;  //@audit gas: should be unchecked (can't underflow)

src/Facets/HopFacet.sol:
   69:                 LibAsset.getOwnBalance(sendingAssetId) - _sendingAssetIdBalance == _hopData.amount,  //@audit gas: should be unchecked (can't underflow)
  107:         uint256 _postSwapBalance = LibAsset.getOwnBalance(sendingAssetId) - _sendingAssetIdBalance;  //@audit gas: should be unchecked (can't underflow)

src/Facets/NXTPFacet.sol:
   57:                 LibAsset.getOwnBalance(sendingAssetId) - _sendingAssetIdBalance == _nxtpData.amount,  //@audit gas: should be unchecked (can't underflow)
   96:         uint256 _postSwapBalance = LibAsset.getOwnBalance(sendingAssetId) - _sendingAssetIdBalance;  //@audit gas: should be unchecked (can't underflow)
  165:         if (postSwapBalance > startingBalance) {
  166:             finalBalance = postSwapBalance - startingBalance;  //@audit gas: should be unchecked (see L165)

src/Libraries/LibDiamond.sol:
  159:         uint256 lastSelectorPosition = ds.facetFunctionSelectors[_facetAddress].functionSelectors.length - 1; //@audit gas: should be unchecked (only called inside for-loops of length > 0)
  173:             uint256 lastFacetAddressPosition = ds.facetAddresses.length - 1; //@audit gas: should be unchecked (only called inside for-loops of length > 0)

src/Libraries/LibSwap.sol:
  48:         toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId) - toAmount; //@audit gas: should be unchecked (can't underflow)

src/Libraries/LibUtil.sol:
  11:         if (_res.length < 68) return "Transaction reverted silently";
  12:         bytes memory revertData = _res.slice(4, _res.length - 4); // Remove the selector which is the first 4 bytes //@audit gas: should be unchecked (see L11)

Visibility

Public functions to external

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

 - AnyswapFacet.startBridgeTokensViaAnyswap(ILiFi.LiFiData,AnyswapFacet.AnyswapData) (src/Facets/AnyswapFacet.sol#35-66)
 - CBridgeFacet.startBridgeTokensViaCBridge(ILiFi.LiFiData,CBridgeFacet.CBridgeData) (src/Facets/CBridgeFacet.sol#57-84)
 - GenericSwapFacet.swapTokensGeneric(ILiFi.LiFiData,LibSwap.SwapData[]) (src/Facets/GenericSwapFacet.sol#22-43)
 - HopFacet.startBridgeTokensViaHop(ILiFi.LiFiData,HopFacet.HopData) (src/Facets/HopFacet.sol#61-87)
 - NXTPFacet.startBridgeTokensViaNXTP(ILiFi.LiFiData,ITransactionManager.PrepareArgs) (src/Facets/NXTPFacet.sol#46-76)
 - NXTPFacet.completeBridgeTokensViaNXTP(ILiFi.LiFiData,address,address,uint256) (src/Facets/NXTPFacet.sol#124-140)
 - NXTPFacet.swapAndCompleteBridgeTokensViaNXTP(ILiFi.LiFiData,LibSwap.SwapData[],address,address) (src/Facets/NXTPFacet.sol#150-171)
 - WithdrawFacet.withdraw(address,address,uint256) (src/Facets/WithdrawFacet.sol#20-38)

Errors

Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Revert strings > 32 bytes:

Facets/AnyswapFacet.sol:133:        require(block.chainid != _anyswapData.toChainId, "Cannot bridge to the same network.");
Facets/CBridgeFacet.sol:147:        require(s.cBridgeChainId != _cBridgeData.dstChainId, "Cannot bridge to the same network.");
Facets/HopFacet.sol:146:        require(s.hopChainId != _hopData.chainId, "Cannot bridge to the same network.");
Libraries/LibDiamond.sol:56:        require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner");
Libraries/LibDiamond.sol:76:                revert("LibDiamondCut: Incorrect FacetCutAction");
Libraries/LibDiamond.sol:84:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:86:        require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
Libraries/LibDiamond.sol:95:            require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists");
Libraries/LibDiamond.sol:102:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:104:        require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
Libraries/LibDiamond.sol:113:            require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function");
Libraries/LibDiamond.sol:121:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:124:        require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
Libraries/LibDiamond.sol:133:        enforceHasContractCode(_facetAddress, "LibDiamondCut: New facet has no code");
Libraries/LibDiamond.sol:154:        require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");
Libraries/LibDiamond.sol:156:        require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");
Libraries/LibDiamond.sol:187:            require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty");
Libraries/LibDiamond.sol:189:            require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
Libraries/LibDiamond.sol:200:                    revert("LibDiamondCut: _init function reverted"); 

I suggest shortening the revert strings to fit in 32 bytes, or that using custom errors as described next.

Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Instances include:

Facets/AnyswapFacet.sol:45:            require(
Facets/AnyswapFacet.sol:50:            require(msg.value == _anyswapData.amount, "ERR_INVALID_AMOUNT");
Facets/AnyswapFacet.sol:92:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/AnyswapFacet.sol:101:            require(address(this).balance - _fromBalance >= _anyswapData.amount, "ERR_INVALID_AMOUNT");
Facets/AnyswapFacet.sol:105:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/AnyswapFacet.sol:133:        require(block.chainid != _anyswapData.toChainId, "Cannot bridge to the same network.");
Facets/CBridgeFacet.sol:63:            require(
Facets/CBridgeFacet.sol:68:            require(msg.value >= _cBridgeData.amount, "ERR_INVALID_AMOUNT");
Facets/CBridgeFacet.sol:105:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/CBridgeFacet.sol:116:            require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/CBridgeFacet.sol:147:        require(s.cBridgeChainId != _cBridgeData.dstChainId, "Cannot bridge to the same network.");
Facets/HopFacet.sol:64:        if (sendingAssetId == address(0)) require(msg.value == _hopData.amount, "ERR_INVALID_AMOUNT");
Facets/HopFacet.sol:68:            require(
Facets/HopFacet.sol:109:        require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/HopFacet.sol:146:        require(s.hopChainId != _hopData.chainId, "Cannot bridge to the same network.");
Facets/NXTPFacet.sol:52:        if (sendingAssetId == address(0)) require(msg.value == _nxtpData.amount, "ERR_INVALID_AMOUNT");
Facets/NXTPFacet.sol:56:            require(
Facets/NXTPFacet.sol:98:        require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
Facets/NXTPFacet.sol:131:            require(msg.value == amount, "INVALID_ETH_AMOUNT");
Facets/NXTPFacet.sol:133:            require(msg.value == 0, "ETH_WITH_ERC");
Facets/Swapper.sol:15:            require(
Libraries/LibAsset.sol:50:        require(success, "#TNA:028");
Libraries/LibAsset.sol:114:        require(!isNativeAsset(assetId), "#IA:034");
Libraries/LibAsset.sol:129:        require(!isNativeAsset(assetId), "#DA:034");
Libraries/LibDiamond.sol:56:        require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner");
Libraries/LibDiamond.sol:84:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:86:        require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
Libraries/LibDiamond.sol:95:            require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists");
Libraries/LibDiamond.sol:102:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:104:        require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
Libraries/LibDiamond.sol:113:            require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function");
Libraries/LibDiamond.sol:121:        require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
Libraries/LibDiamond.sol:124:        require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
Libraries/LibDiamond.sol:154:        require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");
Libraries/LibDiamond.sol:156:        require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");
Libraries/LibDiamond.sol:187:            require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty");
Libraries/LibDiamond.sol:189:            require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
Libraries/LibDiamond.sol:212:        require(contractSize > 0, _errorMessage);
LiFiDiamond.sol:38:        require(facet != address(0), "Diamond: Function does not exist");

I suggest replacing revert strings with custom errors.