Zellic - Audit Reports

Overall issues with some functions

Some contracts have issues with regards to the functions they do not implement.

Some functions are never called

Throughout the codebase, there are several functions that are never called:

unfrozeBalance in AccountTypeHelper
setBaseMaintenanceMargin, setBaseInitialMargin, and setLiquidationFeeMax in MarketTypeHelper

Event not emitted

The sendMessageWithFee should emit a MessageSent event, similar to how sendMessage does.

function sendMessageWithFee(OrderlyCrossChainMessage.MessageV1 memory data, bytes memory payload)
    public
    payable
    override
    onlyCaller
{
    bytes memory lzPayload = data.encodeMessageV1AndPayload(payload);
    uint16 lzDstChainId = _chainIdMapping[data.dstChainId];
    require(lzDstChainId != 0, "CrossChainRelay: invalid dst chain id");

    uint16 version = 1;
    uint256 gasLimit = _flowGasLimitMapping[data.method];
    if (gasLimit == 0) {
        gasLimit = 3000000;
    }
    bytes memory adapterParams = abi.encodePacked(version, gasLimit);

    _lzSend(lzDstChainId, lzPayload, payable(address(this)), address(0), adapterParams, msg.value);
+   emit MessageSent(data, payload);
}

Remediation

This issue has been acknowledged by Orderly Network, and a fix was implemented in commit f5a34f48↗.

Error status may not be appropriate

The executeWithdrawAction has a type of try-catch if-case statement to handle errors. However, the error status may not be appropriate in the case of multiple errors being thrown.

function executeWithdrawAction(EventTypes.WithdrawData calldata withdraw, uint64 eventId)
    external
    override
    onlyOperatorManager
{
    // avoid stack too deep
    uint128 maxWithdrawFee = vaultManager.getMaxWithdrawFee(tokenHash);
    if (account.lastWithdrawNonce >= withdraw.withdrawNonce) {
        // require withdraw nonce inc
        state = 101;
    } else if (account.balances[tokenHash] < withdraw.tokenAmount) {
        // require balance enough
        state = 1;
    } else if (vaultManager.getBalance(tokenHash, withdraw.chainId) < withdraw.tokenAmount) {
        // require chain has enough balance
        state = 2;
    } else if (!Signature.verifyWithdraw(withdraw.sender, withdraw)) {
        // require signature verify
        state = 4;
    } else if (maxWithdrawFee > 0 && maxWithdrawFee < withdraw.fee) {
        // require fee not exceed maxWithdrawFee
        state = 5;
    }
    // ...
}

The way this is currently implemented, the first error will be saved in state while the rest of the errors will be ignored. This means that should additional errors be throwable, the system will not know about them, at least not until the first error is fixed. We recommend that the system should be able to handle multiple errors, not just the first one.