Arkis
Quantstamp reviewed Arkis on 12 Jan 2024: 17 findings, 0 critical; 13 marked fixed, 4 acknowledged.
Report hosted on certificate.quantstamp.com.
Trustblock does not review code.
Findings17
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 0 |
| High | 0 | 0 | 0 |
| Medium | 0 | 2 | 3 |
| Low | 0 | 2 | 10 |
MediumDebt Entries Can Be OverwrittenFixed
Part of the execution flow of `Fund.forward()` involves calling `MoneyAccount.spendMoney(amount, reward, block.timestamp)`, where `block.timestamp` serves as a nonce. If `spendMoney()` is called more than once in the same block with identical parameters, any child calls to `createDebtIfNecessary()` will overwrite previous debt entries, and not all debt will be accounted for. This is in part because the `block.timestamp` acts as a salt rather than a true nonce.
MediumFaulty Accounting when Claiming Rewards for Mutiple EpochsFixed
Liquidity Providers accumulate rewards across epochs. Rewards are recalculated and distributed when they interact with the Pool. If it has been several epochs since the user last interacted with the pool, the function `calculateUserRewardsForTheWholeNumberOfEpochs()` calculates the rewards for elapsed epochs by applying the average TPS of the epochs across the elapsed time. However, the calculation assumes that each epoch is identical in length. Rewards should be distributed in proportion with the length of the epoch.
MediumUsers Could Lose Rewards Due to Missing Check in `RewardsMath`Fixed
The function `RewardsMath.increaseInterest()` updates the state `Staking.TPSS` by adding to it a factor based on the input `_rewardsAmount`, as seen in the following line: `self.TPSS += uint240((_rewardsAmount * TPSS_DENOMINATOR) / (self.tokenVolume))`. Afterwards, a new epoch is started and a few state variables are updated in the subsequent call to `startNewEpoch()` as seen below: ``` function startNewEpoch(Staking storage self, uint256 passingEpochTPSS) private { self.linkedListTPSS[passingEpochTPSS] = self.TPSS; self.tokenVolume = 0; self.epochStartTime[self.TPSS] = block.timestamp; self.numberOfEpochs++; } ``` If the value of `_rewardsAmount` is zero, then the “new” TPSS will be the same as the old one which leads to two errors: 1. The linked list will be pointing to itself `self.linkedListTPSS[passingEpochTPSS] = self.TPSS` since `passingEpochTPSS` is equal to `self.TPSS`. 2. The epoch start time of `self.TPSS` will be overwritten with a newer timestamp. **Impact** 1. **Users would get zero rewards from the incomplete first epoch**. The function `calculateUserRewardsForIncompleteFirstEpoch()` calculates the rewards based on the multiplicative factor `userFirstEpochTPSS = nextEpochTPSS - depositEpoch.TPSS`. Any user with `depositEpoch.TPSS` equal to the old `self.TPSS` would get a factor zero and therefore zero rewards. 2. **Users would get the wrong rewards from the complete epochs**. The function `calculateUserRewardsForTheWholeNumberOfEpochs()` calculates the rewards based on the multiplicative factor `tpsForTheRemainingEpochs = ((lastRewardTime - ufeEndTime) * (self.TPSS - nextTPSSFromUserDepositEpoch)) / numberOfWholeUserEpochs`. There are three members in this equation: `numberOfWholeUserEpochs` would be the correct since it was updated correctly in `startNewEpoch()`. However: * `A = (lastRewardTime - ufeEndTime)` where `ufeEndTime = self.epochStartTime[nextTPSSFromUserDepositEpoch]`. The timestamp in `ufeEndTime` would be newer (greater) than what it should be which leads to a difference lower that expected. * `B = (self.TPSS - nextTPSSFromUserDepositEpoch)`. If we assume the value of `self.TPSS` cannot be increased to compensate for that repeated update, then the current value of `self.TPSS` and `nextTPSSFromUserDepositEpoch` could more or less compensate each other resulting in the expected value of `B`. However, if the error of updating `TPSS` with the same old value happens multiple times, factor `B` would be lower than expected. On the other hand, if the value of `self.TPSS` can indeed by increased to compensate for that repeated updated, then the value of `B` could be greater than expected and this could compensate to some extent the lower value from `A`. However, it would still result in the wrong values. Any user with `depositEpoch.TPSS` equal to the old `self.TPSS` could get a different reward amount which could potentially reduce the amount of available tokens for other users.
Low'Dead' CodeFixed
Dead code should be avoided since it might consume more resources during development, testing, and compilation without contributing to the software's functionality. It makes the codebase harder to understand and may mislead developers, leading to potential bugs or wasted time. In the case of solidity, it might also increase the bytecode size increasing deployment costs. The following is a non-exhaustive list of dead or unused code instances: 1. The contract `ConvexFiImmutableState` is never imported or used anywhere in the codebase. 2. The variable `borrower` in the `AccountFactory` contract is private and is not used anywhere in the contract. 3. The abstract contract `InternalModifierDS` is never imported or used anywhere in the codebase.
LowChecks-Effects-Interactions Pattern ViolationFixed
The [Checks-Effects-Interactions](https://solidity.readthedocs.io/en/latest/security-considerations.html#use-the-checks-effects-interactions-pattern) coding pattern is meant to mitigate any chance of other contracts manipulating the state of the blockchain in unexpected and possibly malicious ways before control is returned to the original contract. As the name implied, only after checking whether appropriate conditions are met and acting internally on those conditions should any external calls to, or interactions with, other contracts be done. The function `Pool.deposit()` updates the state through `staking.increasePosition()` after doing the external call `token.transferFrom()`.
LowCompliance May Be Bypassed for CurveFixed
To add liquidity to Curve, it is required that the protocol is supported, but to withdraw or exchange it may be either supported or suspended. The curve function `Swaps.exchange_multiple` takes a `_swap_params` argument that determines the swap type. If the argument passed is `7, 8, 9, 10` or `11` then the swap will call `add_liquidity()` and add liquidity to the pool. The compliance check for `exchange_multiple()` on Arkis does not check the `_swap_params` so it may be used to add liquidity to Curve should it ever become suspended.
LowFetch Once and Reuse to Improve Gas ConsumptionFixed
The following are some opportunities for gas savings: 1. `ProtocolsRepository.updateProtocolSupport()` fetches the given protocol twice instead of once and reusing it. The condition in the `else if` branch can just call `prot.status` instead of `protocol(_protocol).status`. 2. `DebtManager.tryRepay()` calls `self.peek()` twice in stead of once and reuse it. The body of the `if` branch could just call `debt.partialRepay()` instead of using `self.peek().partialRepay()`. 3. `RewardsMath.calculateNewUserVolume()` fetches `depositEpoch.timeWhenUserVolumeWasLastUpdated` twice instead of once and reusing it.
LowInconsistent Naming Hinders Code Comprehension & CollaborationFixed
Having two classes or files with the same name should be avoided since it causes ambiguity, confusion, and potential naming conflicts. It also compromises code readability, comprehension, and collaboration. There are two structs called `Debt` in the codebase: the first struct is defined in `interfaces/liquidityManagement/insuranceFund/Debt.sol`, and the second is defined in `liquidityManagement/liquidityPool/libraries/DebtManager.sol`. Additionally, there are two files called `Debt.sol`: the one already mentioned and `liquidityManagement/insuranceFund/libraries/Debt.sol`.
LowLiquidity Providers Can Lose Tokens Due to Unsafe CastingFixed
The `DebtLibrary.createDebt()` function returns a new `Debt` object with the debt amount and rewards. However, these two values are cast from `uint256` to `uint128` without making sure that the values fit in 128 bits. This could potentially lead to the `Fund` forwarding less than it should to its corresponding `Pool` which means that users might end up with fewer tokens than deposited, and also fewer rewards.
LowMissing Input ValidationFixed
It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. The following is a non-exhaustive list of inputs that should be validated: 1. `Account.constructor()`: Validate that the `_marginEngine` is not `address(0)` since it is an immutable variable and would need to be redeployed otherwise. 1. `CurveFiLiquidityPoolsRepository.initializePool()`: Validate that the pool has the specified `numberOfTokens`. 1. `CommandSafeExecutor.constructor()`: Validate that `compliance` is not `address(0)` since it is an immutable variable and would need to be redeployed otherwise. 1. `Fund.initialize()`: Validate that the `debtAdmin` is a non-zero address. 1. `ProtocolsRepository.updateProtocolSupport()`: Validate that `_protocol` is a non-empty string. 1. `ProtocolsRepository.updateOperatorSupport()`: Validate that `_protocol` is a non-empty string. 1. `PoolFactory.createPool()` and `Pool.initialize()`: Neither function validates the borrower and token address against `address(0)`. There is also no validation on `thresholdOnTotalDeposit_` making it possible for it to be zero. All three values are only set during the initialization of the pool and cannot be set later on. A wrong configuration would mean redeploying the pool.
Show 3 more
LowMissing Whitelist EnforcementsFixed
We uncovered a few issues relating to whitelisting/permissions of different protocol interactions. 1. `UniswapV3Evaluator.evaluate(address, ExchangeRequest)` enforces that both the input token and output token are supported. Requiring that the input token be supported (rather than supported or suspended) may prevent favorable trades when liquidating margin accounts. Given that other dexes without this restriction are available, the issue has been marked as Informational severity. 2. The `CurveFiValidatorPlainPool.remove_liquidity()` checks that the pool coins are supported or suspended before creating the `Command` to remove liquidity. However, no token validation is done when adding liquidity. 3. The documentation states the following regarding whitelist enforcements for exchanges: > For exchange: `operator`, `pool` and `input token` MAY be either supported or suspended, but `output token` MUST be supported. However, `CurveFiEvaluator.evaluate(address, ExchangeRequest)` does not enforce that the output token is supported.
LowTrapped Tokens in ConvexFiFixed
If a user is liquidated, their Convex positions are assumed to be staked into the `RewardPool` or `Booster`. However, it is possible that the user has Deposit Tokens that have not been staked. Liquidating these tokens would require a tedious process as they will be omitted by the `DecreasePositionRequest`.
LowUnlocked PragmaFixed
Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.8.*`. The caret (`^`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version _and above_, hence the term "unlocked".
Contracts in scope
No deployed address is recorded for this audit.
The report names a repository or files. Open the report to see its scope.
Repository and files the report names (140)
- contracts/interfaces/accountAbstraction/compliance/curveFi/ICurveFiPool.sol
- contracts/accountAbstraction/interpreter/arkis/MarginAccountEvaluator.sol
- contracts/accountAbstraction/interpreter/JitCompiler.sol
- contracts/accountAbstraction/marginAccount/base/CommandSafeExecutor.sol
- contracts/interfaces/accountAbstraction/marginAccount/IAccountFactory.sol
- contracts/interfaces/accountAbstraction/compliance/lidoFi/IWstEth.sol
- contracts/accountAbstraction/marginAccount/Account.sol
- contracts/liquidityManagement/liquidityPool/base/Suspendable.sol
- contracts/interfaces/accountAbstraction/interpreter/ILiquidityPoolsController.sol
- contracts/libraries/Address.sol
- contracts/liquidityManagement/liquidityPool/libraries/Queue.sol
- contracts/accountAbstraction/interpreter/convexFi/ConvexFiEvaluator.sol
- contracts/accountAbstraction/compliance/curveFi/CurveFiValidator.sol
- contracts/interfaces/accountAbstraction/interpreter/Command.sol
- contracts/accountAbstraction/interpreter/oneinchV5/libraries/UniswapTokens.sol
- contracts/accountAbstraction/compliance/libraries/TokensRepository.sol
- contracts/interfaces/liquidityManagement/liquidityPool/ISuspendable.sol
- contracts/base/CommonErrors.sol
- contracts/interfaces/marginEngine/IDispatcher.sol
- contracts/accountAbstraction/interpreter/uniswapV3/actions/UniswapV3Exchange.sol
- contracts/accountAbstraction/interpreter/arkis/InsuranceFundEvaluator.sol
- contracts/accountAbstraction/interpreter/LiquidityPoolsController.sol
- contracts/accountAbstraction/compliance/approve/ApproveValidator.sol
- contracts/accountAbstraction/interpreter/oneinchV5/base/OneInchV5Hub.sol
- contracts/accountAbstraction/interpreter/lidoFi/LidoFiEvaluator.sol
- contracts/interfaces/accountAbstraction/interpreter/adapters/IExchangeEvaluator.sol
- contracts/libraries/CalldataLibrary.sol
- contracts/libraries/DiamondLib.sol
- contracts/accountAbstraction/interpreter/base/LiquidityPoolsRepository.sol
- contracts/accountAbstraction/compliance/WhitelistingController.sol
- contracts/libraries/SafeCall.sol
- contracts/marginEngine/Dispatcher.sol
- contracts/interfaces/accountAbstraction/interpreter/adapters/index.sol
- contracts/accountAbstraction/interpreter/convexFi/base/ConvexFiImmutableState.sol
- contracts/accountAbstraction/compliance/curveFi/validators/CurveFiValidatorSwapRouter.sol
- contracts/interfaces/accountAbstraction/compliance/convexFi/IBooster.sol
- contracts/liquidityManagement/liquidityPool/MerkleTreeWhitelist.sol
- contracts/interfaces/liquidityManagement/liquidityPool/IMerkleTreeWhitelist.sol
- contracts/liquidityManagement/insuranceFund/FundFactory.sol
- contracts/marginEngine/base/SignatureProcessor.sol
- contracts/liquidityManagement/liquidityPool/libraries/DebtManager.sol
- contracts/interfaces/liquidityManagement/liquidityPool/IPool.sol
- contracts/interfaces/accountAbstraction/compliance/uniswapV3/ISwapRouter.sol
- contracts/libraries/Nonce.sol
- contracts/interfaces/liquidityManagement/insuranceFund/IFund.sol
- contracts/liquidityManagement/insuranceFund/libraries/Debt.sol
- contracts/accountAbstraction/interpreter/convexFi/ConvexFiLiquidityPoolsRepository.sol
- contracts/accountAbstraction/interpreter/uniswapV3/actions/UniswapV3HarvestYield.sol
- contracts/base/auth/AccessControlDS.sol
- contracts/accountAbstraction/interpreter/libraries/Config.sol
- contracts/accountAbstraction/interpreter/curveFi/CurveFiEvaluator.sol
- contracts/accountAbstraction/compliance/curveFi/validators/CurveFiValidatorZapper.sol
- contracts/accountAbstraction/interpreter/convexFi/actions/ConvexFiDecreasePosition.sol
- contracts/liquidityManagement/insuranceFund/libraries/MoneyAccountant.sol
- contracts/accountAbstraction/interpreter/lidoFi/base/LidoFiImmutableState.sol
- contracts/accountAbstraction/compliance/curveFi/validators/CurveFiValidatorPlainPool.sol
- contracts/interfaces/marginEngine/Envelope.sol
- contracts/base/auth/OwnableReadonly.sol
- contracts/accountAbstraction/compliance/libraries/ProtocolsRepository.sol
- contracts/interfaces/accountAbstraction/compliance/curveFi/ICurveFiGauge.sol
- contracts/interfaces/liquidityManagement/insuranceFund/Debt.sol
- contracts/interfaces/accountAbstraction/compliance/convexFi/index.sol
- contracts/accountAbstraction/interpreter/libraries/ScriptCompiler.sol
- contracts/base/proxy/ImmutableBeaconProxy.sol
- contracts/accountAbstraction/interpreter/curveFi/actions/CurveFiExchange.sol
- contracts/interfaces/liquidityManagement/liquidityPool/ILiquidityPool.sol
- contracts/interfaces/accountAbstraction/compliance/oneinchV5/IAggregationRouterV5.sol
- contracts/accountAbstraction/compliance/AbstractValidator.sol
- contracts/interfaces/accountAbstraction/compliance/curveFi/ICurveFiRouter.sol
- contracts/interfaces/accountAbstraction/compliance/oneinchV5/IUniswapPool.sol
- contracts/marginEngine/base/ThresholdsVerifier.sol
- contracts/accountAbstraction/compliance/curveFi/validators/CurveFiValidatorMetapool.sol
- contracts/base/auth/OwnableReadonlyDS.sol
- contracts/interfaces/accountAbstraction/interpreter/adapters/IIncreasePositionEvaluator.sol
- contracts/accountAbstraction/compliance/lidoFi/LidoFiValidator.sol
- contracts/marginEngine/libraries/Envelope.sol
- contracts/marginEngine/libraries/Package.sol
- contracts/liquidityManagement/liquidityPool/libraries/staking/errors.sol
- contracts/base/auth/OwnableAssignable.sol
- contracts/liquidityManagement/liquidityPool/libraries/staking/PositionManager.sol
- contracts/accountAbstraction/interpreter/oneinchV5/base/OneInchV5ImmutableState.sol
- contracts/accountAbstraction/compliance/uniswapV3/UniswapV3Validator.sol
- contracts/interfaces/accountAbstraction/interpreter/IInterpreter.sol
- contracts/interfaces/common/IInitializable.sol
- contracts/interfaces/accountAbstraction/compliance/convexFi/ICrvDepositor.sol
- contracts/base/StateMachine.sol
- contracts/interfaces/liquidityManagement/insuranceFund/IFundInitializer.sol
- contracts/interfaces/accountAbstraction/compliance/lidoFi/IStEth.sol
- contracts/accountAbstraction/interpreter/uniswapV3/UniswapV3LiquidityPoolsRepository.sol
- contracts/base/FixedU256x32.sol
- contracts/interfaces/accountAbstraction/compliance/uniswapV3/index.sol
- contracts/interfaces/accountAbstraction/compliance/oneinchV5/index.sol
- contracts/accountAbstraction/marginAccount/libraries/AccountFactoryRepository.sol
- contracts/accountAbstraction/compliance/convexFi/ConvexFiValidator.sol
- contracts/interfaces/accountAbstraction/compliance/curveFi/index.sol
- contracts/liquidityManagement/insuranceFund/fund/Fund.sol
- contracts/liquidityManagement/liquidityPool/pool/Pool.sol
- contracts/interfaces/accountAbstraction/interpreter/adapters/IDecreasePositionEvaluator.sol
- contracts/interfaces/liquidityManagement/liquidityPool/IPoolFactory.sol
- contracts/accountAbstraction/interpreter/curveFi/actions/CurveFiHarvestYield.sol
- contracts/accountAbstraction/interpreter/transfer/TransferEvaluator.sol
- contracts/liquidityManagement/liquidityPool/PoolFactory.sol
- contracts/libraries/AddressRegistry.sol
- contracts/libraries/Asset.sol
- contracts/interfaces/accountAbstraction/compliance/convexFi/IRewardPool.sol
- contracts/interfaces/accountAbstraction/compliance/convexFi/IClaimZap.sol
- contracts/liquidityManagement/liquidityPool/libraries/MerkleTreeRepository.sol
- contracts/accountAbstraction/interpreter/uniswapV3/libraries/UniswapV3Path.sol
- contracts/base/proxy/BeaconDS.sol
- contracts/accountAbstraction/interpreter/oneinchV5/actions/OneInchV5Exchange.sol
- contracts/interfaces/base/proxy/IBeacon.sol
- contracts/interfaces/accountAbstraction/interpreter/Script.sol
- contracts/interfaces/accountAbstraction/interpreter/adapters/ILiquidityPoolsRepository.sol
- contracts/accountAbstraction/interpreter/lidoFi/LidoFiLiquidityPoolsRepository.sol
- contracts/interfaces/liquidityManagement/liquidityPool/IPoolInitializer.sol
- contracts/interfaces/accountAbstraction/marginAccount/IAccount.sol
- contracts/interfaces/accountAbstraction/compliance/uniswapV3/IUniswapV3Pool.sol
- contracts/accountAbstraction/marginAccount/AccountFactory.sol
- contracts/interfaces/liquidityManagement/insuranceFund/IFundFactory.sol
- contracts/accountAbstraction/interpreter/oneinchV5/OneInchV5Evaluator.sol
- contracts/interfaces/accountAbstraction/compliance/Asset.sol
- contracts/interfaces/accountAbstraction/compliance/curveFi/ICurveFiMinter.sol
- contracts/interfaces/accountAbstraction/interpreter/adapters/PositionDescriptor.sol
- contracts/accountAbstraction/interpreter/arkis/LiquidityPoolEvaluator.sol
- contracts/accountAbstraction/interpreter/curveFi/actions/CurveFiDecreasePosition.sol
- contracts/liquidityManagement/liquidityPool/libraries/staking/index.sol
- contracts/interfaces/accountAbstraction/compliance/IWhitelistingController.sol
- contracts/accountAbstraction/interpreter/uniswapV3/UniswapV3Evaluator.sol
- contracts/libraries/Command.sol
- contracts/interfaces/accountAbstraction/compliance/uniswapV3/INonfungiblePositionManager.sol
- contracts/interfaces/accountAbstraction/interpreter/IJitCompiler.sol
- contracts/accountAbstraction/interpreter/curveFi/CurveFiLiquidityPoolsRepository.sol
- contracts/interfaces/marginEngine/Package.sol
- contracts/accountAbstraction/interpreter/uniswapV3/base/UniswapV3ImmutableState.sol
- contracts/interfaces/accountAbstraction/compliance/curveFi/ICurveFiZapper.sol
- contracts/accountAbstraction/interpreter/curveFi/libraries/CurveFiLib.sol
- contracts/base/auth/InternalModifierDS.sol
- contracts/accountAbstraction/interpreter/libraries/Path.sol
- contracts/liquidityManagement/liquidityPool/libraries/staking/RewardsMath.sol
- contracts/accountAbstraction/interpreter/uniswapV3/actions/UniswapV3DecreasePosition.sol
Part of Arkis’s security record
ArkisSecurity firm
Quantstamp236 projects reviewed · 332 reports · latest Oct 2026
See Quantstamp’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us