The developers of Orbit DeFi contracted byterocket to conduct a smart contract audit of their token contract suite. Orbit is “a DeFi smart vault Optimizer that automates and rebalances your LP strategies effortlessly, starting with Uniswap V3”.
The team of byterocket reviewed and audited the above smart contracts in the course of this audit. We started on the 6th of May and finished on the 29th of May 2022.
The audit included the following services:
byterocket gained access to the code via a public GitHub repository. We based the audit on the main branch’s state on May 12th, 2022 (commit hash 541460999b7d8fa80c4a4a86383f583392436b67). The updated version was provided to us via multiple new commits to the repository, addressing our findings. The last and most recent commit hash that we audited is 3c6ed8c356be28b2e059a2a7f0171ba31f216bb7.
We conducted a manual multi-pass code review of the smart contracts mentioned in section (1). Three different people went through the smart contract independently and compared their results in multiple concluding discussions.
The manual review and analysis were additionally supported by multiple automated reviewing tools, like Slither, GasGauge, Manticore, and different fuzzing tools.
We are categorizing our findings into four different levels of severity:
On the code level, we found 39 bugs or flaws, with 39 of them being fixed in a subsequent update. Prior to this, there have been 19 non-critical and 6 low, 9 medium and 5 high severity findings. Additionally, we found 9 gas improvements, which have all been implemented.
The contracts are written according to the latest standard used within the Ethereum community and the Solidity community’s best practices. The naming of variables is very logical and understandable, which results in the contract being easy to understand. The code is well documented. The developers provided us with a test suite as well as proper deployment scripts.
Location: PositionManager.sol - Line 105 - 116
Description:
The PositionManager can be re-inititialized countless times, as the init() function as well as the construction function does not implement any way to check whether the contract has been initialized already. This way, ownership can easily be claimed via the init() function.
Recommendation:
Consider adding a flag to ensure that the contracts can only be initialized once. Additionally, we suggest making use of the OpenZeppelin libraries for initialization processes.
Update on the 13th of June 2022:
The developers have update the implementation to make use of OpenZeppelin’s initializer, preventing multiple initializations.
Location: actions/SwapToPositionRatio.sol - Line 34 - 75
Description:
The swapToPositionRatio() function has no checks on authorization, hence it can be called and executed by anyone at any time. There might be cases, where this is not in the best interest of the user, especially during sudden price swings out of the current range.
The same applies to the following functions:
Recommendation:
Consider adding a proper check for the correct authorization of the swapToPosition- Ratio() function.
Update on the 14th of June 2022:
The developers have updated their implementation to include the onlyWhitelisted- Keepers modifier, except for the swapToPositionRatio() function, where it is not necessary to do so. This action function is - as the developers told us - only being called via a fallback function that is properly protected. Subsequently, we consider this finding fixed.
Location: utils/AaveAddressHolder.sol - Line 16 - 18
Description:
The setLendingPoolAddress() function has no limitations on who can call and execute it and thus change the lending pools address.
These are the affected lines of code:
Recommendation:
Consider adding a proper check to ensure that only the right addresses can change the lending pool address.
Update on the 13th of June 2022:
The developers have update the implementation to only allow governance to change these variables.
Location: actions/AaveDeposit.sol - Line 28 - 30
Description:
As the lending pool address of the Aave action can be set by anyone (due to (H.3)), users may deposit into a malicious lending pool or the lending pool of an attacker without knowing so.
These are the affected lines of code:
Recommendation:
Consider addressing (H.3) or finding another way to ensure that the original lending address that a user defined can not be changed without them noticing or them intending to do so.
Update on the 13th of June 2022:
As the developers have properly address finding (H.3), this finding has subsequently been addressed as well.
Location: utils/UniswapAddressHolder.sol - Lines 25, 31, 37
Description:
The variables (nonfungiblePositionManagerAddress, uniswapV3FactoryAddress, and swapRouterAddress) have no limitations on who can set them - thus anyone can do it at any time.
This is one of the affected functions:
Recommendation:
Consider adding a proper check to ensure that only the right addresses can change the variables.
Update on the 13th of June 2022:
The developers have update the implementation to only allow governance to change these variables.
Location: PositionManager.sol - Line 90
Description:
The onlyFactory(_registry) modifier is not working as intended, as its input can be controlled by the caller. The calling address can deploy its own registry with registry.positionManagerFactoryAddress == msg.sender.
Furthermore, the user might be able to deploy their own address this way, but still make use of the official one.
These are the affected lines of code:
Recommendation:
Consider changing the access control to the PositionManager to be safe and not have any user-controlled inputs that can change its authorization.
Update on the 13th of June 2022:
The developers have revamped the constructor-flow as well as the initialization process. The new flow does not contain any user-controller inputs as far as authorization goes, which now leads to the initialization to work as expected.
Location: actions/AaveDeposit.sol - Line 41 - 43
Description:
The deposit function of the Aave action contains a false approval/allowance mechanism. Firstly, it is important to first set the allowance to zero before changing it to a custom value, since some tokens enforce it (see here). Secondly, we would suggest only approving the correct amount (diff) instead of the full amount.
This subsequently also occurs in helpers/ERC20Helper.sol in lines 22 to 25.
These are the affected lines of code:
Recommendation:
Consider changing the approval mechanism to first set the allowance to zero before changing it, as well as only approving the diff instead of the full amount.
Update on the 13th of June 2022:
The developers have update the implementation to first set the allowance to zero before updating it to the required amount.
Location: actions/Swap.sol - Line 50
Description:
During the swap, the call defines the input as well as the expected output. Setting the expected output to zero opens the door to a huge front-running opportunity as the arbitrageur has maximum room for price changes for this trade. Setting the proper expected output with a sensible slippage applied is within the best practice.
This also occurs in the following places:
These are the affected lines of code:
Recommendation:
Consider making use of a proper amount for the expected out tokens instead of setting it to 0.
Update on the 14th of June 2022:
The developers have implemented a new time-weighted average pricing deviation check in the SwapHelper file, which they are using to ensure that the swaps are not carried out when they would incur a loss that is bigger than the accepted deviation. Subsequently, we consider this finding fixed.
Location: actions/ZapOut.sol - Line 111 - 120
Description:
The corresponding section in the _findBestFee() function intended to find the best pool is actually just returning the pool with the most liquidity at the given slot. However, if the impact of the swap leads to the liquidity of the current slot being used up, the rest of the swap is being facilitated in a slot whose liquidity is unknown to the function.
This also occurs in modules/AaveModule.sol in lines 214 to 222.
These are the affected lines of code:
Recommendation:
Consider either documenting this behaviour or simulating the trade in each of the pools to really find out, which of the offers the best circumstances for the corresponding trade.
Update on the 13th of June 2022:
The developers have update the documentation to reflect the described behaviour.
Location: modules/AaveModule.sol - Line 103 - 128
Description:
The amountToAave that is being calculated in the corresponding code section only accounts for the collected fees, not for the liquidity received by “closing” the position.
Recommendation:
Consider including the liquidity that has been received as well, instead of just handling the fees.
Update on the 14th of June 2022:
The developers have responded to our finding, stating that the liquidity is included. It is taken into account by being stored in the tokensOwed variable after a call to decreaseLiquidity(). Afterwards, the liquidity is considered for the collectFees() call. Subsequently, we consider this finding fixed.
Location: modules/IdleLiquidityModule.sol - Line 114 - 118
Description:
The result of the call to the NonfungiblePositionManager returns the lower and upper ticks. These are not checked and can lead to the calculation in line 111 to over- or underflow, without erroring as no SafeMath is used. They could be in the wrong order as well if not properly checked.
These are the affected lines of code:
Recommendation:
Consider verifying whether the ticks are actually ordered correctly as well as making use of SafeMath in this case as well to ensure that no under- or overflow can occur unnoticed.
Update on the 13th of June 2022:
The developers have update the implementation to make use of SafeMath, ensuring that no unnoticed under- or overflows can occur.
Location: modules/IdleLiquidityModule.sol - Line 129
Description:
The cast of a uint24 to int24 is dangerous and should be properly executed and verified, which is not the case here. This can lead to severely wrong values that are being subsequently used.
These are the affected lines of code:
Recommendation:
Consider properly verifying whether the outcome of the unsafe cast is correct.
Update on the 13th of June 2022:
The developers have update the implementation to include a library, which handles the safe casting between int24 and uint24. This library is now being used.
Location: modules/IdleLiquidityModule.sol - Line 131
Description:
As SafeMath is not used, there are several risks (see (L.3)). In this case, there is a severe risk of under- and/or overflows due to the nature of the calculation. Additionally, (in the latter part of the calculation) dividing by tickSpacing and then multiplying by tickSpacing only reduces accuracy without doing anything meaningful, which might not be the desired behavior.
The same occurs in utils/WithdrawRecipes.sol in lines 49 - 50 and in modules/AutoCompoundModule.sol in lines 72 - 73.
These are the affected lines of code:
Recommendation:
Consider making use of SafeMath and removing unnecessary parts of the calculation.
Update on the 13th of June 2022:
The developers have update the implementation to make use of SafeMath.
Location: modules/IdleLiquidityModule.sol - Line 40
Description:
With the modules and actions generally reverting in case of the data being non-existent, this is mostly fine. However, in this case, a definitive rebalance is being facilitated as the values here might still pass the if-checks.
Recommendation:
Consider properly checking whether the module’s data has been correctly set.
Update on the 13th of June 2022:
The developers have update the implementation to ensure that the data of the module is not set to zero.
Location: helpers/ERC20Helper.sol - Line 54
Description:
During the check on whether the balance is greater than the required amount, a less than (<) is used, instead of making use of a less than equal (<=).
These are the affected lines of code:
Recommendation:
Consider changing the comparison to reflect a less than equal (<=) instead of a less than (<) to correct it.
Update on the 13th of June 2022:
The developers have update the implementation to use a less than equal (<=) instead of a less than (<).
Location: Multiple occurrences throughout the project
Description:
Certain parts of the documentation are misleading or wrong:
Recommendation:
Consider correcting the documentation to ensure that they are correct and not misleading.
Update on the 13th of June 2022:
The developers have update the documentation to better represent the actual logic of the code.
Location: Multiple occurrences throughout the project
Description:
In certain locations, calculations are facilitated without SafeMath in a Solidity version that is lower than 0.8:
Recommendation:
Consider using SafeMath functionalities in the corresponding occurrences to ensure their required safety guarantees.
Update on the 13th of June 2022:
The developers have update the implementation to make use of SafeMath in the corresponding occurrences.
Location: Multiple occurrences throughout the project
Description:
As modules currently do not require any data to be set, it is not the best idea to rely on the data being present. In the cases that we found, the call would still revert, but we do not recommend this handling of missing data. This occurs in the following cases:
Recommendation:
Consider adding a proper handling to functions that interact with a module while relying on its data to be present. This can also just be a proper require statement.
Update on the 13th of June 2022:
The developers have update the implementation to ensure that the data of the module is not zero.
Location: utils/DepositRecipes.sol - Line 36
Description:
The use of a uint32 variable does not save gas in this case, as you can see here.
The same also applies to the use of uint8 for the i-variable in for-loops, like in modules/AaveModule.sol in line 213.
These are the affected lines of code:
Recommendation:
Consider reverting the counter back to uint256 to ensure that it can not overflow.
Update on the 13th of June 2022:
The developers have update the implementation to use uint256 variables instead of uint32.
Location: Multiple occurrences throughout the project
Description:
Throughout the project, the unsafe versions of ERC20 transfers are used. With the variety of todays tokens, especially malicious ones, this is never recommended. This occurs in the following cases:
Recommendation:
Consider making use of SafeERC20 mechanics to ensure a safe transfer of ERC20 tokens.
Update on the 13th of June 2022:
The developers have update the implementation to make use of SafeERC20 in all of the corresponding occurrences.
Location: PositionManager.sol - Line 128 - 137
Description:
A non-standard way of “delete from array” pattern is being used here, which we do not usually recommend.
These are the affected lines of code:
Recommendation:
Consider changing the pattern to be more standardized to be safe, like in the example below:
Update on the 13th of June 2022:
The developers have update the implementation to make use of more a standardized pattern to remove an element from the array.
Location: utils/Storage.sol - Line 38
Description:
To ensure a proper usage of the getStorage() function, the key variable should be set to private, so it can not be accessed on accident.
These are the affected lines of code:
Recommendation:
Consider adding the private keyword to the key variable to ensure a proper use of the getStorage() function.
Update on the 13th of June 2022:
The developers have update the variable to be private, enforcing the proper use of their function.
Location: modules/IdleLiquidityModule.sol - Line 70
Description:
There is an undocumented value of 10 being deducted from the swapped token amounts. Furthermore, this value is not being defined as a global (constant) variable for better readability.
These are the affected lines of code:
Recommendation:
Consider documenting the value of 10 and possibly converting it to a global (constant) variable.
Update on the 13th of June 2022:
The developers have update the variable itself as well as its documentation to make sure that it can be properly understood.
Location: PositionManager.sol - Line 257 - 258
Description:
The documentation of the withdrawERC20() function does state that it returns tokens, while in reality it transfers them to the msg.sender.
Recommendation:
Consider correcting the documentation to reflect the actual function.
Update on the 13th of June 2022:
The developers have update the documentation to reflect the actual logic of the function.
Location: PositionManager.sol - Line 260
Description:
The withdrawERC20() function includes an approveToken call, which is (a) not required at all, (b) approving the wrong address, as well as (c) using the wrong amount for the approval.
Additionally, using type(uint256).max is best practice instead of using 2**256 - 1.
These are the affected lines of code:
ERC20Helper._approveToken(tokenAddress, address(this), 2**256 - 1);
uint256 amount = ERC20Helper._withdrawTokens(tokenAddress, msg.sender,
2**256 - 1);
Recommendation:
Consider removing the approve-call and change the uint256 maximum value to reflect the best practice of Solidity. We would suggest a change like the following:
uint amount = ERC20Helper._getBalance(tokenAddress, address(this));
uint got = ERC20Helper._withdrawTokens(tokenAddress, msg.sender, amount);
require(amount == got, "ERC20 Transfer failed");
emit ERC20Withdrawn(tokenAddress, msg.sender, got);
Update on the 13th of June 2022:
The developers have update the implementation, removing the unnecessary approval statement and making use of our suggested change.
Location: utils/WithdrawRecipes.sol - Line 31, 49, 50
Description:
It is best practice to write numerals that are 1,000 or higher with underscores, like so:
This highly increases its readability.
Recommendation:
Consider adding underscores to numerals to that are 1,000 or higher.
Update on the 13th of June 2022:
The developers have update the implementation, adding underscored to the corresponding numerals.
Location: PositionManager.sol - Line 294
Description:
The fallback() function retrieves the storage manually instead of making use of the provided getStorage() function.
These are the affected lines of code:
Recommendation:
Consider making use of the getStorage() function instead of doing it manually.
Update on the 13th of June 2022:
The developers have update the implementation to enforce the proper use of the getStorage() function.
Location: PositionManagerFactory.sol - Line 86 - 88
Description:
The getAllPositionManagers() function merely returns the positionManagers array, which is unnecessary as the array is public anyways. This is increasing the contract size without any benefit.
The same also applies to the moduleKeys array in the Registry, with the getModuleKeys() function in line 102 being implemented as well.
Recommendation:
Consider either removing the getAllPositionManagers() function or changing the array to be private to prevent two exposed functions to exist.
Update on the 14th of June 2022:
The developers have stated that they have a good reason to keep the extra function, which is perfectly fine for us. There is no security implication be keeping it.
Location: PositionManagerFactory.sol - Line 46 - 49
Description:
The changeGovernance() function has two slight problems that we see: it does not validate its input, which - in this case - might be crucial. Additionally, it might emit the event in cases where there has been no change to the state.
The same also applies to the same function in the Registry.sol contract in line 47 - 50.
Recommendation:
Consider validating the input of crucial functions like this one to be valid. Additionally, ensure that events for state-changing functions are only emitted if the state variable has actually been changed.
Update on the 13th of June 2022:
The developers have update the implementation to validate whether the supplied address is not zero.
Location: actions/ClosePosition.sol - Line 46
Description:
The deadline of the swap is being extended by 120 seconds, even though it is being executed by a contract instead of an EOA, in which case it is unnecessary.
The same occurs in the following places:
These are the affected lines of code:
Recommendation:
Consider removing the 120 second addition, as it is unnecessary in this case.
Update on the 13th of June 2022:
The developers have update the implementation, removing the unnecessary 120 second addition to the deadline.
Location: actions/ClosePosition.sol - Line 50 - 52
Description:
The token0Closed and token1Closed variables are of type uint128, as defined in the Uniswap V3 contracts. However, in your implementation, you are using the uint256 type instead, which leads to an implicit conversion.
These are the affected lines of code:
Recommendation:
Consider changing the type of the two variables to be uin128 and then (if required) convert it to uint256 to ensure that no side-effect occur. During our internal tests, this still worked as intended, but we can’t guarantee it in any case.
Update on the 13th of June 2022:
The developers have update the implementation to work with uint128 for the return values of the Uniswap V3 calls.
Location: actions/ClosePosition.sol - Line 57 - 58
Description:
Solidity has a default way to ensure a clean and error-free way of setting the maximum value of a type with type(uint128).max instead of doing a manual calculation. This is not being used here.
The same occurs in the following places:
These are the affected lines of code:
Recommendation:
Consider making use of type(uint128).max instead of 2**128-1.
Update on the 13th of June 2022:
The developers have update the implementation throughout the project to make use of the type(uint).max flow instead of manually calculations.
Location: actions/CollectFees.sol - Line 51 - 64
Description:
The _updateUncollectedFees() function does look like a rather costly way of updating the uncollected fee through an undocumented side-effect. This is usually not within the best practice, but does certainaly not cause any harm here.
Recommendation:
We leave it up to the developers on whether they want to keep using this behaviour. We just wanted to point it out in case this behaviour changes over time.
Update on the 14th of June 2022:
The developers have acknowledged the finding and are open to find a better solution. As there is currently no better solution that is known to either of us, it is perfectly fine to stay like this.
Location: actions/SwapToPositionRatio.sol - Line 74
Description:
It is best practice to only emit an event if the state has actually been changed. This usually does require an additional if-statement to be added, but is generally accepted to be more consistent in terms of the on-chain history.
This also occurs in the following locations:
Recommendation:
Consider adding an if-statement to ensure that the event is only emitted, if something was actually changed.
Update on the 13th of June 2022:
The developers have update the implementation to only emit events in cases where something was actually changed.
Location: actions/ZapIn.sol - Line 45
Description:
The _pullTokensIfNeeded() function does not guarantee that any tokens have actually been pulled. In this case, it does not look like anything would happen, but this is not within the best practice.
Recommendation:
Consider calls to functions like _pullTokensIfNeeded() when writing your logic and keep in mind that they are not required to actually do what they say. We would suggest that an additional require statement may be used to ensure that everything has worked out.
Update on the 13th of June 2022:
The developers have update the implementation to ensure that any call to _pullTokensIfNeeded() only succeeds if it really did its job properly.
Location: helpers/ERC20Helper.sol - Line 77
Description:
The _withdrawTokens() function transfers tokens from itselfv to an address via transferFrom, which is not correct. This can be handled with a regular transfer.
These are the affected lines of code:
IERC20(token).safeTransferFrom(address(this), to, amountOut);
Recommendation:
Consider changing the transferFrom to a regular transfer.
Update on the 13th of June 2022:
The developers have update the implementation to make use of a regular transfer instead of a transferFrom.
Location: Throughout the project
Description:
There are certain occurrences of global variables that should never change, hence should be immutable to ensure that this is the case as well as saving some gas. These occurrences are:
Recommendation:
Consider adding the immutable keyword to the corresponding global variables that qualify.
Update on the 13th of June 2022:
The developers have update the implementation, setting the corresponding unchangeable global variables to be immutable.
Location: Throughout the project
Description:
There are certain occurrences of global variables that are missing a visibility keyword, which leads to them being private, which is often not the desired behaviour. These occurrences are:
Recommendation:
Consider adding the correct visibility keyword to the corresponding global variables that are missing one.
Update on the 13th of June 2022:
The developers have update the implementation, adding a visibility keyword to the corresponding global variables.
Location: modules/AaveModule.sol - Line 173
Description:
The code of the distance check is a near duplicate of the code in the _getTokens() function of the UniswapNFTHelper contract. This code can be exported into a library
Recommendation:
Consider introducing a library to prevent unnecessary code duplication.
Update on the 13th of June 2022:
The developers have moved the logic into the UniswapNFTHelper contract.
Location: Throughout the project
Description:
There are several for-loops that can benefit from caching the length of the used array, which is often the case if it is being accessed outside of the declaration of the loop itself. Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop.
The affected for-loops are:
This is an example of an affected loop:
Recommendation:
Consider moving the length of the used array into a cached variable like in the example below:
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: Throughout the project
Description:
All of the for-loops in the project are using the standard way of declaring them, which is slightly inefficient gas-wise.
This is an example of an unoptimized for-loop:
Recommendation:
Consider optimizing the for-loops in each contract to be more efficient in terms of its gas usage, including:
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: Throughout the project
Description:
Initializing variables with their default values uses more gas than necessary as it involves an unnecessary MSTORE or SSTORE operation.
The affected variables are:
This is an example of an affected variable:
Recommendation:
Consider removing occasions where the default value is being used to initialize a variable to save some gas, as shown in the example below:
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: Throughout the project
Description:
In require statements, it is cheaper to check for != 0 instead of > 0, if the unsigned integer is being validated to not be zero. Changing this will save some gas.
The affected variables are:
This is an example of an affected require statement check:
Recommendation:
Consider changing all of the require statements that involved a zero check for a unsigned integer to be more gas efficient, like in the example below:
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: Throughout the project
Description:
In certain circumstances, it saves gas to cache variables that are read often, especially if they are stored in storage and not in memory.
The affected variables are:
Recommendation:
Consider caching the affected variables and reading from the cached version instead of doing costly read actions.
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: actions/SwapToPositionRatio.sol - Lines 41 - 47
Description:
The pool variable is being obtained in two steps, with the outcome of the first step never being used again. To safe a storage slot and save gas, this can be optimized by directly casting the outcome of the first operation to IUniswapV3Pool.
Recommendation:
Consider directly casting the outcome of poolAddress to IUniswapV3Pool.
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: actions/ZapIn.sol - Lines 56
Description:
During the zapIn() call, the approveToken() function is called twice with the same inputs, hence wasting gas which is not necessary.
These are the affected lines of code:
Recommendation:
Consider removing one of the approval calls to save gas.
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: modules/IdleLiquidityModule.sol - Lines 36 - 72
Description:
The result of the call to the NonfungiblePositionManager is already done in the _checkDistanceFromRange() function. It could be cached to prevent a double calling, saving some gas.
Recommendation:
Consider obtaining the required values from the _checkDistanceFromRange() function, caching them instead of calling the underlying function twice.
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Location: Registry.sol - Lines 11
Description:
The access to the whitelistedKeepers array gets more and more expensive as the array size increases, with the cost of the access being O(n). Making use of a hashmap (mapping) would bring this down to O(1), which is way more efficient and cheaper.
Recommendation:
Consider switching from an array layout to a mapping/hashmap to make the access way more efficient (especially deletions).
Update on the 14th of June 2022:
The developers have updated their implementation, taking our suggestion for gas improvements into consideration.
Part of our audits are also analyses of the protocol and its logic. The byterocket team went through the implementation and documentation of the implemented protocol.
The repository itself contained tests and documentation. We found the provided unit tests that are coming with the repository execute without any issues and cover the most important parts of the protocol.
According to our analysis, the protocol and logic are working as intended, given that any findings with a severity level are fixed. When making use of the Mainnet forking method, we were able to successfully execute the protocol and its modules.
We were not able to discover any additional problems in the protocol implemented in the smart contract.
During our code review (which was done manually and automated), we found 39 bugs or flaws, with 39 of them being fixed in a subsequent update. Prior to this, there have been 19 non-critical and 6 low, 9 medium and 5 high severity findings. Additionally, we found 9 gas improvements, which have all been implemented.
The protocol review and analysis did neither uncover any game-theoretical nature problems nor any other functions prone to abuse besides the ones that have been uncovered in our findings.
In general, there are some improvements that can be made, but we are very happy with the overall quality of the code and its documentation. The developers have been very responsive and were able to answer any questions that we had.
As of the date of publication, the information provided in this report reflects the presently held understanding of the auditor’s knowledge of security patterns as they relate to the client’s contract(s), assuming that blockchain technologies, in particular, will continue to undergo frequent and ongoing development and therefore introduce unknown technical risks and flaws. The scope of the audit presented here is limited to the issues identified in the preliminary section and discussed in more detail in subsequent sections. The audit report does not address or provide opinions on any security aspects of the Solidity compiler, the tools used in the development of the contracts or the blockchain technologies themselves, or any issues not specifically addressed in this audit report.
The audit report makes no statements or warranties about the utility of the code, safety of the code, suitability of the business model, investment advice, endorsement of the platform or its products, the legal framework for the business model, or any other statements about the suitability of the contracts for a particular purpose, or their bug-free status.
To the full extent permissible by applicable law, the auditors disclaim all warranties, express or implied. The information in this report is provided “as is” without warranty, representation, or guarantee of any kind, including the accuracy of the information provided. The auditors hereby disclaim, and each client or user of this audit report hereby waives, releases and holds all auditors harmless from, any and all liability, damage, expense, or harm (actual, threatened, or claimed) from such use.
We store our public audit reports on IPFS; a peer-to-peer network called the "Inter Planetary File System". This allows us to store our reports in a distributed network instead of just a single server, so even if our website is down, every report is still available.
The IPFS Hash, a unique identifier of the report, is signed on-chain by both the client and us to prove that both sides have approved this audit report. This signing mechanism allows users to verify that neither side has faked or tampered with the audit.
