CANISTER SMART CONTRACT AUDIT

ICLighthouse ICDex

October, 2023

Summary

CAYA is pleased to present this audit report outlining our assessment of code, canister smart contracts, and other important audit insights and suggestions for management, developers, and users.
ICDex is a fully on-chain orderbook DEX, with decentralized features and a great user experience like CEX. We had the opportunity to test two canisters belonging to the list of libraries for future use. OrderBook is a module that contains the described basic operations related to buying or selling, matching buyers and sellers in the market. In fact, this is the mechanism used by most electronic exchanges. To correctly convert the parameter into a hashed value, the OrderBook module uses functions from the Binary module.
Each level in the order book consists of a price and a quantity. The book has two sides: asks and bids. Asks consists of orders from other traders offering to sell an asset. Bids are orders from traders offering to buy an asset. As a result, OrderBook contains basic functions to calculate the highest and lowest price when buying or selling, depending on the order level and type.
During the audit process, the CAYA team found several issues. A detailed summary and the current state are displayed in the table below.
Severity of the issue
Total found
Resolved
Unresolved
Critical
0 issues
0 issues
0 issues
High
0 issues
0 issues
0 issues
Medium
1 issue
0 issues
1 issue
Low
10 issues
0 issues
10 issues
Informational
8 issues
0 issues
8 issues
Total
19 issues
0 issues
19 issues
Evaluating the findings in this report, the CAYA auditors can state that the canisters are operational and secure. Under the given circumstances, we set the following risk level:
To set the codebase quality mark, our auditors are evaluating the initial commit given for the scope of the audit and the last commit with the fixes. This approach helps us adequately and sequentially evaluate the quality of the code. Code style, optimization of the canisters, the number of issues, and risk level of the issues are all taken into consideration. The CAYA team has developed a transparent evaluation codebase quality system presented below.
Severity of the issue
Issue severity
Total found
Resolved
Critical
1
10
High
0.8
7
Medium
0.5
5
Low
0.2
0.5
Informational
0
0.1
Please note that the points are deducted out of 100 for each and every issue on the list of findings (according to the current status of the issue). Issues marked as “not valid” are not subject to point deduction.
Score

Based on the given findings, risk level, performance, and code style, CAYA team can grant the following overall score:

Please be aware that this audit does not certify the definitive reliability and security level of the canister smart contract. This document describes all vulnerabilities, typos, performance issues, and security issues found by the CAYA audit team. If the code is still under development, we highly recommend running one more audit once the code is finalized.

Scope of Work

ICLighthouse is a decentralized finance ecosystem on the IC blockchain. ICLighthouse has developed derivatives trading protocols on Ether and have rich experience in financial product model design and blockchain code implementation. It is worth highlighting the following developments: ICP, cycles and tokens wallet; DRC20; ICTC; ICDex; ICSwap; ICHouse, etc.
Within the scope of this audit, two independent auditors thoroughly investigated the given codebase and analyzed the overall security and performance of the canister smart contracts.
The audit was conducted in October, 2023. The outcome is disclosed in this document.
The scope of work for the given audit consists of the following canisters:
Binary.mo;
OrderBook.mo.

Workflow of the auditing process

CAYA audit team uses the most sophisticated and contemporary methods and well-developed techniques to ensure canister smart contracts are free of vulnerabilities and security risks. The overall workflow consists of the following phases:

Phase 1: The research phase

Research
After the Audit kick-off, our security team conducts research on the contract’s logic and expected behavior of the audited canister smart contracts.
Documentation reading
CAYA auditors do a deep dive into your tech documentation with the aim of discovering all the behavior patterns of your codebase and analyzing the potential audit and testing scenarios.
The outcome
At this point, the CAYA auditors are ready to kick off the process. We set the auditing strategies and methods and are prepared to conduct the audit part.

Phase 2: Manual part of the audit

Manual check
During the manual phase of the audit, the CAYA team manually looks through the code in order to find any security issues, typos, or discrepancies with the logic of the contract. The initial commit as stated in the agreement is taken into consideration.
Candid UI check
In addition to the manual part and code review, having deployed canisters locally and using the Candid UI, we also test them in practice from the end user side and see how the canister reacts to various actions.
The outcome
An interim report with the list of issues.

Structure and organization of the findings

For simplicity in reviewing the findings in this report, CAYA auditors classify the findings in accordance with the severity level of the issues (from most critical to least critical).

All issues are marked as “Resolved” or “Unresolved”, depending on if they have been fixed by ICLighthouse or not. The issues with “Not Valid” status are left on the list of findings but are not eligible for the score points deduction.

The CAYA team always provides a detailed description of the issues and recommendations on how to fix them.

Classification of found issues is graded according to 5 levels of severity described below:

Informational
Are classified as every point that increases onboarding time and code reading, as well as the issues which have no impact on the canister’s ability to operate.
Low
The issue doesn’t contain operational or security risks, but are more related to optimization of the codebase.
Medium
The issue slightly impacts the canister’s ability to operate by slightly hindering its intended behavior.
High
The issue significantly affects the ability of the canister smart contract to compile or operate. These are potential security or operational issues.
Critical
The issue affects the canister smart contract in such a way that funds may be lost or allocated incorrectly, or the issue could result in a significant loss.

Manual Report

Unsafe deprecated method

Medium
MM – 01
Unresolved
The arrayAppend() function of OrderBook.mo canister performs operations with arrays and uses a deprecated method toArray(). Since this method is deprecated, it is not safe to use. Instead, there is a similar method in the Buffer module from mo:base library to convert a buffer to an array.
Recommendation:
Сonsider the opportunity to use the correct method from the Buffer module.
For example, instead of
buffer.toArray();
you can do the following ->
Buffer.toArray(buffer);

Useless import

Low
ML – 01
Unresolved
The Result module  from the mo:base library is an unused import in the Binary.mo canister.
Recommendation:
Consider removing useless import to decrease the canister’s bytecode.

The variables that must be declared as constants

Low
ML – 02
Unresolved
The OrderBook module uses the variables declared inside functions, whose values are immutable, but they are not declared as constants.
Recommendation:
Declare the following variables as constants:
_fill() -> item (in all cases);
_popOrderPrice() -> item;
_pushOrderPrice() -> tempItem;
depth() -> depth_;
trade() -> _filled;
inOrderBook() -> ask, bid.

Unnecessary create a new variables

Low
ML – 03
Unresolved
The new local variables are created in the inOrderBook() function of OrderBook.mo canister, which stores the fields of OrderBook type. These variables are used only once, so it is pointless to allocate a memory area if these values are used only once. This is about an useless storage call.
Recommendation:
Remove unneeded additional declarations and use just _ob.ask or _ob.bid where this is needed.

Unused field of the BalanceChange type

Low
ML – 04
Unresolved
The OrderBook.mo canister has the BalanceChange type that saves the possible balance’s states. However, one of the fields is not used in the canister.
Recommendation:
Consider removing the unused NoChange field or adding use case for this value.

Optimization of the output type

Low
ML – 05
Unresolved
In the OrderBook.mo canister, there is a function natHash() that returns a value of type Hash.Hash. In turn, the Hash.Hash type is equal to the Nat32 type. A possible option is to immediately set the Nat32 type output. Also, since the Hash module is only used once in this location, and the use of Nat32 completely replaces Hash.Hash, the Hash module becomes unnecessary and needs to be removed to optimize the codebase.
Recommendation:
Use Nat32 type output instead of Hash.Hash type and remove import of the Hash module.

Separate cases without implementation

Low
ML – 06
Unresolved
The functions level1(), depth(), putK(), putBatch() in the OrderBook.mo have not described the all case's implementations. For example, the cases that perform in other possible variants (_), are without implementations.
Recommendation:
Consider the opportunity to add the implementation for case(_);

Unused input parameter

Low
ML – 07
Unresolved
The function clear() in the OrderBook.mo has the useless input parameter that is not used in the function’s body.
Recommendation:
Сonsider removing the unused parameter _ob.

Condition’s part without implementation

Low
ML – 08
Unresolved
The function putK() in the OrderBook.mo has the described conditions to perform when kbar is new or should be updated. In addition, the code has the construct else without implementation.
Recommendation:
Сonsider the opportunity to add the implementation for else. In case, this construct is useless, remove else.

Unused declared type

Low
ML – 09
Unresolved
The OrderBook.mo canister has a declared public types. However, Amount type is not used.
Recommendation:
Remove unused type.

Type conversion optimization

Low
ML – 10
Unresolved
The OrderBook canister contains the natToFloat() function for correct type conversion. However, there are too many redundant conversions and calls to various functions in this function. This process can be optimized by converting the input parameter to type Int and then calling the Float.fromInt() function on it.
Recommendation:
Сonsider optimizing the function.

The unnecessary comments of code

Informational
MI – 01
Unresolved
Leaving commented-out code in source code is bad practice, as it takes up space, causes confusion, and leads to maintenance issues if it's not removed. The following practice occurs in the OrderBook.mo – 40, 43, 292-301, 403 lines.
Recommendation:
Remove the commented code.

Lack of mo-doc annotations

Informational
MI – 02
Unresolved
Motoko's official documentation provides a number of rules regarding styling decisions for comments. Besides, the commenting methods /// and /** */ refer to generating the technical Motoko’s documentation. Doc comments should be used to provide explanations for functions, classes, types, modules, variables, and more.
Recommendation:
Add the mo-doc annotations for all objects in .mo-files following mo-doc rules and remove unneeded comments or rechange this to mo-doc style.

Naming conventions

Informational
MI – 03
Unresolved
Naming conventions in Motoko are important for creating clear, understandable and maintainable code. In the following canisters, global, local variables and functions are incorrectly named: Binary.mo, OrderBook.mo.
Recommendation:
Use the correct names according to Motoko documentation.

Order of Layout and Functions

Informational
MI – 04
Unresolved
The functions in the canister OrderBook.mo are not grouped according to their visibility and order. Functions should be grouped according to their visibility and ordered in the following way:
public;
private.
Besides, pay attention to logical dependencies in the following way:
accessors;
mutators;
processing.
As for the style of the Layout, it should be styled so that all types are defined next to each other, rather than scattered between functions.
Recommendation:
Layout and functions order in all canisters should be grouped according to Motoko documentation.

Modules without names

Informational
MI – 05
Unresolved
Modules should be named according to their content and it is good practice to set the same file name as the module. No declared module has a name in any canister.
Recommendation:
Consider setting module names to match file names.

Rules of the style code

Informational
MI – 06
Unresolved
In all files, while writing the code, the styling rules regarding spacing are not followed. In addition, after writing each command or the end of a block, it is necessary to highlight it with ‘;’
Recommendation:
Consider using the rules while coding on Motoko from style guideline.

Unclear variables names

Informational
MI – 07
Unresolved
Basically, for easy reading and clear understanding, variable names should be given according to what they are used for and what data they store. The functions _fill(), _popOrderPrice(), _pushOrderPrice(), _put(), depth() of the OrderBook.mo canister have declared variables with unclear names such as item, temp, tempItem or i.
Recommendation:
Consider setting clear variables names in these functions.

Possible inconsistent order status data

Informational
MI – 08
Unresolved
The OrderBook.mo canister contains a clear() function that returns cleared data. The function lacks any checks or warnings before it is called. As a result, the data may have an inconsistent order status.
Recommendation:
Ensure that the main canister with the actor has enough required checks before calling this function.
We are delighted to have a chance to work with the ICLighthouse team and contribute to your company's success by reviewing and certifying the security of your canister smart contracts.
The statements made in this document should be interpreted neither as investment or legal advice, nor should its authors be held accountable for decisions made based on this document.
Our CEO
If you are looking for not execution of your scope only but to have a long-term visionary partner you are in the right place!
Contact US
Contact us today to get a free consultation on security auditing, management, and blockchain development.
Submit
Thank you! Your submission has been received!
Oops! Something went wrong while submitting the form.