Token Bridge - Audit

The `callOptionalReturn()` function is being used to transfer tokens, and if the token address is found in the mapping `bugERC20s`, the result of the function call is not checked to determine if the transfer was successful. ```solidity=110 function callOptionalReturn(IERC20 token, bytes memory data, mapping(address => uint8) storage bugERC20s) private { // We need to perform a low level call here, to bypass Solidity's return data size checking mechanism, since // we're implementing it ourselves. // A Solidity high level call has three parts: // 1. The target address is checked to verify it contains contract code // 2. The call itself is made, and success asserted // 3. The return value is decoded, which in turn checks the size of the returned data. // solhint-disable-next-line max-line-length require(address(token).isContract(), "SafeERC20: call to non-contract"); // solhint-disable-next-line avoid-low-level-calls (bool success, bytes memory returndata) = address(token).call(data); require(success, "SafeERC20: low-level call failed"); if (bugERC20s[address(token)] != 0) { return; } if (returndata.length > 0) { // Return data is optional // solhint-disable-next-line max-line-length require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed"); } } ``` This function is called internally in the `createOrSignWithdraw()` function, which means that if the token transfer fails, then the user will not receive the assets. This function is called internally in the `crossOut()` or `crossOutII()` functions, which means that if the tokens transfer fails, then the user may get the assets without having to transfer out the assets.

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee) to the target contract, only 90 tokens actually arrive to the contract.

The variables `_chainid` and `_managers` are initialized in the constructor, but the `_managers` array is not checked to ensure that it does not contain duplicate addresses, it could be better to ensure that duplicate addresses are not added to the _managers array, to prevent any potential issues with managing the array later on. ```solitidy=965 constructor(uint256 _chainid, address[] memory _managers) public{ require(_managers.length <= max_managers, "Exceeded the maximum number of managers"); require(_managers.length >= min_managers, "Not reaching the min number of managers"); owner = msg.sender; managerArray = _managers; for (uint8 i = 0; i < managerArray.length; i++) { managers[managerArray[i]] = 1; seedManagers[managerArray[i]] = 1; seedManagerArray.push(managerArray[i]); } require(managers[owner] == 0, "Contract creator cannot act as manager"); // 设置当前交易的最小签名数量 current_min_signatures = calMinSignatures(managerArray.length); hashSalt = _chainid * 2 + VERSION; } ```

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=971 managers[managerArray[i]] = 1; ``` - `managerArray[i]` is not zero-checked before being used.

State variables that never change should be declared as `constant` to save gas. ```solidity=939 uint public max_managers = 15; ``` - `max_managers` should be declared `constant`. --- ```solidity=941 uint public min_managers = 3; ``` - `min_managers` should be declared `constant`. --- ```solidity=943 uint public rate = 66; ``` - `rate` should be declared `constant`. --- ```solidity=945 uint public signatureLength = 65; ``` - `signatureLength` should be declared `constant`.

Comparisons that are always true or always false may be incorrect or unnecessary. ```solidity=1261 require(balance >= 0, "No enough balance of token"); ```

It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring Ether, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=1007 to.transfer(amount); ``` - `createOrSignWithdraw` uses `transfer()`.