Zellic - Audit Reports

General coding issues

The codebase contains minor coding mistakes.

One such example is the inclusion of asserts that will always evaluate to true.

For example, amend_order tests the following code that the creator_addr is equal to the signer_addr:

let id = guid::create_id(signer_addr, id_creation_num);
let creator_addr = guid::id_creator_address(&id);
assert!(creator_addr == signer_addr, EINVALID_ORDER_OWNER);

However, the following spec proves this is always true no matter the circumstance.

public fun creator(
    account: &signer,
    id_creation_num: u64
): address {
    let signer_addr = signer::address_of(account);
    let id = guid::create_id(signer_addr, id_creation_num);
    let creator_addr = guid::id_creator_address(&id);
    creator_addr
}

spec creator {
    ensures result == signer::address_of(account);
}

Furthermore, an assert in splay_tree::insert will always evaluate as true, as shown below:

public fun insert<V: store + drop>(tree: &mut SplayTree<V>, key: u64, value: V) {
...
    if (guarded_idx::is_sentinel(maybe_root)) {
...
    } else {
        assert!(!guarded_idx::is_sentinel(maybe_root), ETREE_IS_EMPTY);
...
    }
}

Another such instance is representing two-valued variables with integers when a boolean would be sufficient, such as the side variable in

 fun place_limit_order_internal<Base, Quote>(
        account: &signer,
        book_owner: address,
        side: u8,
        price: u64,
        size: u64,
        time_in_force: u8,
        post_only: bool,
)

In addition, time_in_force, which takes three states - 0, 1, 2 - could be better represented with constants to enhance readability.

So instead of

if (time_in_force == 1) {
    assert!(partially_matchable, EORDER_UNMATCHABLE);
};

if (time_in_force == 2) {
    assert!(fully_matchable, EORDER_UNMATCHABLE);
};

one could use

if (time_in_force == IOC_CONSTANT) {
    assert!(partially_matchable, EORDER_UNMATCHABLE);
};

if (time_in_force == FOK_CONSTANT) {
    assert!(fully_matchable, EORDER_UNMATCHABLE);
};

It should be noted that constants do incur a greater gas cost.

Finally, auditors noticed that the assert in book::is_ask_post_only_valid is incorrect:

assert!(order::get_side(o) == 0, EINVALID_MATCHING_SIDE);

Since this function is for ask orders, it should assert that the side is 1, not 0.

Notably, this function was not called anywhere within the code.