HydraDX - ZanyBonzy's results

HydraDX Omnipool - An Ocean of Liquidity for Polkadot Trade an abundance of assets in a single pool. The HydraDX Omnipool is efficient, sustainable and trustless.

General Information

Platform: Code4rena

Start Date: 02/02/2024

Pot Size: $100,000 USDC

Total HM: 11

Participants: 27

Period: 28 days

Judge: Lambda

Total Solo HM: 4

Id: 327

League:

HydraDX

Findings Distribution

Researcher Performance

Rank: 13/27

Findings: 3

Award: $293.95

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Awards

152.7401 USDC - $152.74

Labels

bug
2 (Med Risk)
satisfactory
:robot:_15_group
duplicate-93

External Links

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L475-L479 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L995-L1081

Vulnerability details

Impact

The add_liquidity function allows the liquidity providers to provide extra liquidity into the pool, upon which they're given LP shares. The function however doesn't allow the users to define the minimum expected shares that they desire, nor is there a cap on the maximum amount of that can be deposited into the pool. This opens up opportunities for griefing and loss of funds for users, especially those providing lesser liquidity amounts into the pool. This can also be excarcebated as the pool supports adding and removing liquidity in imbalanced proportions. If a user wants to deposit liquidity in an imbalanced ratio (only one token). A attacker can front run the user by doing the same and removing the liquidity directly after the deposit of the user. By doing so the attacker steals a significant percentage of the users funds.

For simplified example -

  1. A pool with 8000 token X and 2000 token Y, as a consequence, creator receives 4000 LP shares in return.
  2. User A sends a transaction to provide liquidity of 80 token X and 20 token Y to the pool, so he expects to receive 40 shares in return. Note that there's no option to enter this amount into the function.
  3. Some seconds before transaction of user A is processed, user B swaps 12000 token X to 1200 token Y. The final balance in the pool is: 20000 token X and 800 token Y.
  4. When transaction of user A is processed, he receives 16 shares in return, instead of 40 shares he was expecting, i.e.: less than 50%.

Proof of Concept

add_liquidity doesn't allow users to enter minimum needed shares.

pub fn add_liquidity( origin: OriginFor<T>, pool_id: T::AssetId, assets: Vec<AssetAmount<T::AssetId>>, ) -> DispatchResult { let who = ensure_signed(origin)?; let shares = Self::do_add_liquidity(&who, pool_id, &assets)?; Self::deposit_event(Event::LiquidityAdded { pool_id, who, shares, assets, }); Ok(()) }

Here, there's only a check to ensure that the share amount is not zero.

fn do_add_liquidity( who: &T::AccountId, pool_id: T::AssetId, assets: &[AssetAmount<T::AssetId>], ) -> Result<Balance, DispatchError> { ... let share_issuance = T::Currency::total_issuance(pool_id); let share_amount = hydra_dx_math::stableswap::calculate_shares::<D_ITERATIONS>( &initial_reserves, &updated_reserves, amplification, share_issuance, pool.fee, ) .ok_or(ArithmeticError::Overflow)?; ensure!(!share_amount.is_zero(), Error::<T>::InvalidAssetAmount); let current_share_balance = T::Currency::free_balance(pool_id, who);

Tools Used

Manual code review

Consider adding a min_shares parameter and making a comparison with the received shares, or adding a max limit to the amounts of each assets that can be deposited.

Assessed type

Other

#0 - c4-pre-sort

2024-03-03T06:38:19Z

0xRobocop marked the issue as duplicate of #158

#1 - c4-judge

2024-03-07T21:13:00Z

OpenCoreCH marked the issue as satisfactory

Awards

22.9928 USDC - $22.99

Labels

bug
grade-b
insufficient quality report
QA (Quality Assurance)
Q-17

External Links


1. Centralization risk from admin points of control

Links to affected code *

https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L337-L342 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L369-L374 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L403-L408 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L447-L454 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1340-L1345 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1384-L1390 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1425-L1426 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1452-L1459 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1552-L1553 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L341-L349 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L384-L386 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L408-L415 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L847

Impact

The admins control a number of important functionalities in the protocol including setting trading limits, fees, which tokens to add or remove, amplifications and so on. These effect can have serious effect on the protocol. For instance, the admin by calling the set_asset_tradable_state function can maliciously freeze asset transactions causing instability and griefing users.

Consider implementing a multisig account and documenting the potential risks.



2. Ensure more sanity checks to prevent user griefing

Links to affected code *

https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1415 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L384 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L408 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L408

Impact

Lack of sanity checks can cause users griefing through unreasonable entries from the admins. For instance, pool fee can be set so high that user swaps become unprofitable, leading to loss of funds for the users.

pub fn update_pool_fee(origin: OriginFor<T>, pool_id: T::AssetId, fee: Permill) -> DispatchResult { T::AuthorityOrigin::ensure_origin(origin)?; Pools::<T>::try_mutate(pool_id, |maybe_pool| -> DispatchResult { let pool = maybe_pool.as_mut().ok_or(Error::<T>::PoolNotFound)?; pool.fee = fee; Self::deposit_event(Event::FeeUpdated { pool_id, fee }); Ok(()) }) }

Consider introduction upper and if needed, lower limits to keep the values at a reasonable range. Consider also including a check for 0 in the update_amplification function.



3. set_asset_tradable_state should also prevent freezing of LRNA token

Links to affected code *

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1337-L1343

Impact

The set_asset_tradable_state allows for setting the tradeability status of the assets including LRNA, the protocol's hub token. For this reason, adding and removing LRNA liquidity is prohibited. However, the token can still be frozen which makes it possible to be removed from the protocol or just destabilize the protocol's swapping functionality.

if asset_id == T::HubAssetId::get() { // Atm omnipool does not allow adding/removing liquidity of hub asset. // Although BUY is not supported yet, we can allow the new state to be set to SELL/BUY. ensure!( !state.contains(Tradability::ADD_LIQUIDITY) && !state.contains(Tradability::REMOVE_LIQUIDITY), Error::<T>::InvalidHubAssetTradableState );

Consider checking also, that it cannot be frozen.



4. sell function asset state check can be refactored

Links to affected code *

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L957-L971

Impact

When selling tokens, the sell function makes two important checks, if any of the tokens to be sold is a hub token and if the tokens to the sold have the SELL tradeability.

Doing this checks first if a token is the hub token, to be bought or sold, after which the the tokens state are finally checked.

if asset_in == T::HubAssetId::get() { // if hub token is to be sold return Self::sell_hub_asset(origin, &who, asset_out, amount, min_buy_amount); // function checks for hub token tradeability } if asset_out == T::HubAssetId::get() { //if hub token is to be bought, Not allowed return Self::sell_asset_for_hub_asset(&who, asset_in, amount, min_buy_amount); // function checks for hub token tradeability } let asset_in_state = Self::load_asset_state(asset_in)?; //gets tokenIn state let asset_out_state = Self::load_asset_state(asset_out)?; //gets tokenOut state ensure!( Self::allow_assets(&asset_in_state, &asset_out_state), Error::<T>::NotAllowed );

The function can be refactored by checking for assets' state before specifically checking for the hub assets. It makes the checks earlier, can revert earlier as the HubAssetId is still a valid asset id and saves gas.

let asset_in_state = Self::load_asset_state(asset_in)?; let asset_out_state = Self::load_asset_state(asset_out)?; ensure!( Self::allow_assets(&asset_in_state, &asset_out_state), //reverts here if hub token is not allowed to be bought/sold Error::<T>::NotAllowed ); if asset_in == T::HubAssetId::get() { return Self::sell_hub_asset(origin, &who, asset_out, amount, min_buy_amount); } if asset_out == T::HubAssetId::get() { //if hub token is to be bought, Not allowed return Self::sell_asset_for_hub_asset(&who, asset_in, amount, min_buy_amount); // function checks for hub token tradeability }


5. Consider checking for asset existence before setting limits

Links to affected code *

https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L369 https://github.com/code-423n4/2024-02-hydradx//blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L337 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L403

Impact

The functions set_add_liquidity_limit set_trade_volume_limit and set_remove_liquidity_limit allow setting limits to asset trading. It however only checks that the asset is not the hub asset.

Consider checking if the assetId is valid to avoid setting limits for non existent assets.



6. Lack of deadline parameter implementation in pools

Links to affected code * https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L577 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L716 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L934 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1140 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L475 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L512 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L551 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L638 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L717 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/stableswap/src/lib.rs#L787

Impact

Interactions with AMMs are vulnerable to sandwich attacks, frontrunning and transaction execution at old slippage values. To fix this, AMMs provide users with an option to limit the execution of pending actions. The most common solution is to include a deadline timestamp as a parameter (for example see Uniswap V2 and Uniswap V3). The absence of a deadline means that the user's trades can be "saved for later", and held for a long time before execution, causing trade executions at bad or slightly unfavourable values, i.e the transaction can be withheld indefinitely at the advantage of the block producers. It also facilitates integration with broader DeFi protocol as protcols that make calls to external AMMs tend to include deadline parameters.

Consider introducing a deadline parameter and a check for dated transactions.



7. Vulnerable packages are being used


name = "shlex" version = "1.2.0"

https://rustsec.org/advisories/RUSTSEC-2024-0006.html

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/Cargo.lock#L12937


name = "snow" version = "0.9.4" https://rustsec.org/advisories/RUSTSEC-2024-0011.html

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/Cargo.lock#L13136


name = "quinn-proto" ?? version = "0.9.6"

https://rustsec.org/advisories/RUSTSEC-2023-0063.html

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/Cargo.lock#L10694


name = "ansi_term" version = "0.12.1"

https://rustsec.org/packages/ansi_term.html

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/Cargo.lock#L141


name = "atty" version = "0.2.14"

https://rustsec.org/packages/atty.html

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/Cargo.lock#L698


name = "h2" version = "0.3.22"

https://rustsec.org/advisories/RUSTSEC-2024-0003.html

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/Cargo.lock#L4212

Consider updating to the latest, safest versions.


#0 - c4-pre-sort

2024-03-03T17:46:40Z

0xRobocop marked the issue as insufficient quality report

#1 - OpenCoreCH

2024-03-08T19:32:36Z

Could have definitely been better formatted and formulated, but the recommendations are reasonable and overlap with what have been reported by several wardens as (now downgraded) separate issues

#2 - c4-judge

2024-03-08T19:32:40Z

OpenCoreCH marked the issue as grade-b

Findings Information

Labels

analysis-advanced
grade-b
sufficient quality report
A-07

Awards

118.2201 USDC - $118.22

External Links

Advanced Analysis Report for <img width= auto src="https://hydradx.io/assets/logo-v2.svg" alt="logo">

Audit approach

Brief Overview

Scope and Architecture Overview

Codebase Overview

Centralization Risks

Systemic Risks

Recommendations

Conclusions

Resources

1. Audit approach

  • Ecosystem analysis: A study of the PolkaDot's ecosystem and its parachains. A brief overview of the rust programming language syntax to provide a base for understanding of the protocol's implementation.

  • Documentation Dive: A comprehensive analysis of provided documentation, threat modelling and invariants was conducted to understand protocol functionality, key points were noted, ambiguities were discussed with the dev, and possible risk areas were mapped.

  • Code Inspection: Manual review of each contract within defined sections was conducted, testing function behavior against expectations, and working out potential attack vectors. Vulnerabilities related to dependencies and inheritances, were assessed.

  • Report Compilation: Identified issues were generated into a comprehensive audit report.


2. Brief Overview

  • HydraDX Omnipool, an innovative polkadot AMM that combines all assets in a single pool to solve the problem fragmented liquidity in existing DeFi protocols makes trading inefficient.
  • It does this by providing unparalleled efficiency through less hopping between pools, lower slippage, and improved capital gains.
  • It eases participation by providing liquidity for any single asset, even in early stages, hence incentivizing growth to bootstrap liquidity.

3. Scope and Architecture Overview

For the audit scope, the contracts are sectioned into three different sections.

3.1. Omnipool pallet & math

  • Omnipool is a type of Automated Market Maker (AMM) where all assets are combined into a single pool. This differs from traditional AMMs which require liquidity providers (LPs) to pair tokens. In Omnipool, LPs deposit any asset and receive pool shares. Traders exchange any token through the pool using the "hub" token LRNA, leading to fragmented liquidity and efficient trades.

  • Key features include use of a single pool where all assets are pooled together, simplifying trades and increasing capital efficiency, provision of a token (LRNA) to facilitate trades and absorbs imbalances to stabilize pool value and generation of LP incentives by granting pool shares to liquidity providers representing their contribution and gain rewards.

Pallet Lib

  • Pool creation and closure - A pool can be created by adding a token into the omnipool. Before this can be accomplished, the token must have been registered in the asset registry and initial liquidity must first be transferred to the pool account upon which the AuthorityOrigin admin calls the add_token function. A collection of NFTs are then created to be minted for future liquidity providers.

  • To close the pool, the token is first frozen and the remaining shares must belong to the protocol. The AuthorityOrigin then calls the remove_token function which transfers the existing protocol shares to the beneficiary and the token is removed.

  • Increasing and decreasing pool liquidity - Users call the add_liquidity and remove_liquidity to increase/decrease their positions in the pools. Adding liquidity mints a position NFT to the user. Important to note that positions cannot be increased per se. A new nft position is minted to the user everytime liquidity is provided. Removing liquidity allows full/partial removals upon which the user's position is decreased, or when 0, the position NFT, burned. The assets to be traded must have the ADD_LIQUIDITY and REMOVE_LIQUIDITY states enabled.

  • Asset trades/swaps - The sell and buy functions allow users to trade an asset for another. The assets to be traded must have the SELL and BUY states enabled.

  • On a side note, the protocol's hub token LRNA, only has the SELL state enabled. Therefore, it cannot be bought, liquidity cannot be added or removed.

  • Position sacrifice and protocol liquidity withdrawal - Users feeling charitable can sacrifice their positions by calling the sacrifice_position function. Doing this is akin to removing liquidity, but transferring it to the protocol instead of themselves. Upon position sacrifice, the NFT positon is burned and the AuthorityOrigin can call the withdraw_protocol_liquidity function to withdraw the protocol's own shares.

To ensure full omnipool functionality, the lib depends on the pallet types and traits contracts to provide the needed data types, perform basic calculations, ensure various prices and holds various omnipool hooks.

Math

  • State changes calculations - When various transactions are performed in the pool, hooks are made from the Omnipool lib contract to the Math contract to rebalance the pool by calculating the delta changes of adding/removing liquidity to the pool. Upon selling assets, the calculate_sell_state_changes or the calculate_sell_hub_state_changes hooks are called, depending on if the assets being sold is the hub token or other assets. The calculate_buy_for_hub_asset_state_changes and calculate_buy_state_changes functions handle rebalancing when the hub token and other assets are called. The calculate_add_liquidity_state_changes and calculate_remove_liquidity_state_changes functions are called when assets are added or removed from the pool.

  • Fee calculations - These functions handle fee calculation upon adding or removing assets from the pools. When assets are being added or removed, the functions call the state function hooks which calculates the fees to be charged to the caller. The calculate_fee_amount_for_buy and calculate_withdrawal_fee functions are responsible for this.

  • Other important calculations made here are the calculate_tvl, calculate_cap_difference and calculate_spot_sprice functions which check the pool tvls, assets cap difference and asset spot prices.

As a support system, for the Math contract, the types contract is used to perform basic arithmetic operations, hold various data types and performing various delta updates.

Omnipool Pallet Lib | |--- Assets<T: Config> | | | |--- StorageMap<_, Blake2_128Concat, T::AssetId, AssetState<Balance>> | |--- HubAssetImbalance<T: Config> | | | |--- StorageValue<_, SimpleImbalance<Balance>, ValueQuery> | |--- HubAssetTradability<T: Config> | | | |--- StorageValue<_, Tradability, ValueQuery, DefaultHubAssetTradability> | |--- Positions<T: Config> | | | |--- StorageMap<_, Blake2_128Concat, T::PositionItemId, Position<Balance, T::AssetId>> | |--- NextPositionId<T: Config> | | | |--- StorageValue<_, T::PositionItemId, ValueQuery> | |--- Events<T: Config> | | | |--- Enum: TokenAdded, TokenRemoved, LiquidityAdded, LiquidityRemoved, ProtocolLiquidityRemoved, SellExecuted, BuyExecuted, PositionCreated, PositionDestroyed, PositionUpdated, TradableStateUpdated, AssetRefunded, AssetWeightCapUpdated | |--- Errors<T> | | | |--- Enum: InsufficientBalance, AssetAlreadyAdded, AssetNotFound, MissingBalance, InvalidInitialAssetPrice, BuyLimitNotReached, SellLimitExceeded, PositionNotFound, InsufficientShares, NotAllowed, Forbidden, AssetWeightCapExceeded, AssetNotRegistered, InsufficientLiquidity, InsufficientTradingAmount, SameAssetTradeNotAllowed, HubAssetUpdateError, PositiveImbalance, InvalidSharesAmount, InvalidHubAssetTradableState, AssetRefundNotAllowed, MaxOutRatioExceeded, MaxInRatioExceeded, PriceDifferenceTooHigh, InvalidOraclePrice, InvalidWithdrawalFee, FeeOverdraft, SharesRemaining, AssetNotFrozen, ZeroAmountOut | |--- Calls<T: Config> | | | |--- add_token | |--- add_liquidity | |--- remove_liquidity | |--- sacrifice_position | |--- sell | |--- buy | |--- set_asset_tradable_state | |--- refund_refused_asset | |--- set_asset_weight_cap | |--- withdraw_protocol_liquidity | |--- remove_token | |--- Implementation<T: Config> | |--- protocol_account |--- load_asset_state |--- set_asset_state |--- create_and_mint_position_instance |--- update_hdx_subpool_hub_asset |--- update_hub_asset_liquidity |--- update_imbalance |--- allow_assets |--- sell_hub_asset |--- buy_asset_for_hub_asset |--- buy_hub_asset |--- sell_asset_for_hub_asset |--- get_hub_asset_balance_of_protocol_account |--- remove_asset |--- set_position |--- add_asset |--- update_asset_state |--- load_position |--- is_hub_asset_allowed |--- exists |--- process_trade_fee |--- process_hub_amount

3.2. StableSwap pallet & math

  • The StableSwap automates trades between stablecoins (like USDC and DAI) similar to Curve Finance, slippage for users. It uses a special algorithm designed for stablecoins, similar to the one used by Curve Finance. Liquidity providers can add and remove assets from pools, represented by share tokens. The system is secure as it limits pool creation and asset types by allowing up to 5 different stablecoins, and only authorized entities can create them. Liquidity providers initially need to add all the pool's stablecoins and get share tokens in return. Later, they can withdraw specific stablecoins from the pool.

Pallet Lib

  • Pool creation - The AuthorityOrigin has the power to create a new stable asset pool with a given list of assets not more than 5. He does this by calling the create_pool function. The assets must have been initially registered in the asset registry, so as to ensure proper tracking. The admin doesn't setup the pool's initial liquid through this function, liquidity has to be added seperately.

  • Liquidity addition and removal - Users call the add_liquidity function to add liquidity to pools of their choice. There is a minimum amount of tokens that can be deposited to prevent the first depositor attack. Users can also make calculations on the desired amount of shares they would like, by calling the add_liquidity_shares, which holds the max_asset_amout parameter to function as slippage. To remove liquidity from the pool, there are 2 main methods. The remove_liquidity_one_asset which allows users to specify the amount of shares they're willing to burn, and the withdraw_asset_amount function which allows users to specify the amount of assets they're willing to receive. The assets being added or removed must have had their tradeability status set to the addliquidity and removeliquidity statuses.

  • Asset swaps - Assets with the sell and buy tradeability status turned on can perform swaps by calling the sell and buy functions. By calling the sell or buy functions, the asset_in is sold to the pool to receive asset out, and to protect from slippage, there are minimum buy amout and maximum sell amount parameters for the user to use.

To handle the various hooks in the StableSwap pool, the lib contract uses the types contract which holds various function types, hook functions, and so on, which can be accessed upon pool activity.

Math

  • Amount conversions calculations - To calculate the amount to be received from the pool when given a certain amount to be sent and vice versa, the calculate_out_given_in and calculate_in_given_out functions can be called. To consider fees in the calculations instead, the calculate_out_given_in_with_fee and calculate_in_given_out_with_fee functions are called instead which convert the needed amounts and call the calculate_fee_amount hook.

  • Shares calculations - When liquidity is provided to a pool, the calculate_shares and calculate_shares_for_amount are used as hooks to provide the total amount of shares a user is entitled to upon adding liquidity into the pools.

  • Price calculations - The calculate_share_prices, calculate_share_price and calculate_spot_price functions are important hooks called to calculate current prices based on the pool balances.

The types contract is used as a dependent library which the math contract uses to hold the asset reserve data types, and perform various checks.

+----------------------------------------------------------------------------------------------------------------+ | StableSwap Pallet lib | +----------------------------------------------------------------------------------------------------------------+ | Storage | | | - Pools | StorageMap<Blake2_128Concat, T::AssetId, PoolInfo> | | - AssetTradability | StorageDoubleMap<Blake2_128Concat, T::AssetId, Tradability, ValueQuery> | +-----------------------------------+----------------------------------------------------------------------------+ | Events | | | - PoolCreated | | | - FeeUpdated | | | - LiquidityAdded | | | - LiquidityRemoved | | | - SellExecuted | | | - BuyExecuted | | | - TradableStateUpdated | | | - AmplificationChanging | | +-----------------------------------+----------------------------------------------------------------------------+ | Errors | | | - IncorrectAssets | | | - MaxAssetsExceeded | | | - PoolNotFound | | | - PoolExists | | | - AssetNotInPool | | | - ShareAssetNotRegistered | | | - ShareAssetInPoolAssets | | | - AssetNotRegistered | | | - InvalidAssetAmount | | | - InsufficientBalance | | | - InsufficientShares | | | - InsufficientLiquidity | | | - InsufficientLiquidityRemaining | | | - InsufficientTradingAmount | | | - BuyLimitNotReached | | | - SellLimitExceeded | | | - InvalidInitialLiquidity | | | - InvalidAmplification | | | - InsufficientShareBalance | | | - NotAllowed | | | - PastBlock | | | - SameAmplification | | | - SlippageLimit | | | - UnknownDecimals | | +-----------------------------------+----------------------------------------------------------------------------+ | Calls | | | - create_pool | origin, share_asset, assets, amplification, fee | | - update_pool_fee | origin, pool_id, fee | | - update_amplification | origin, pool_id, final_amplification, start_block, end_block | | - add_liquidity | origin, pool_id, assets | | - add_liquidity_shares | origin, pool_id, shares, asset_id, max_asset_amount | | - remove_liquidity_one_asset | origin, pool_id, asset_id, share_amount, min_amount_out | | - withdraw_asset_amount | origin, pool_id, asset_id, amount, max_share_amount | | - sell | origin, pool_id, asset_in, asset_out, amount_in, min_buy_amount | | - buy | origin, pool_id, asset_out, asset_in, amount_out, max_sell_amount | | - set_asset_tradable_state | origin, pool_id, asset_id, state | +-----------------------------------+----------------------------------------------------------------------------+ | Hooks | Implemented for BlockNumberFor<T> | +-----------------------------------+----------------------------------------------------------------------------+ | Helper Functions | | | - calculate_out_amount | pool_id, asset_in, asset_out, amount_in | | - calculate_in_amount | pool_id, asset_in, asset_out, amount_out | | - do_create_pool | share_asset, assets, amplification, fee | | - do_add_liquidity | who, pool_id, assets | | - do_add_liquidity_shares | who, pool_id, shares, asset_id, max_asset_amount | | - is_asset_allowed | pool_id, asset_id, operation | | - pool_account | pool_id | | - get_amplification | pool | | - retrieve_decimals | asset_id | +-----------------------------------+----------------------------------------------------------------------------+ | Additional Helpers | | | - calculate_shares | pool_id, assets | | - call_on_liquidity_change_hook | pool_id, initial_reserves, initial_issuance | | - call_on_trade_hook | pool_id, asset_in, asset_out, initial_reserves | | - get_pool_state | pool_id, initial_reserves, initial_issuance | +-----------------------------------+----------------------------------------------------------------------------+

3.3. EMA Oracle pallet & math

  • This pallet offers oracles (data feeds) for price, volume, and liquidity of different asset pairs. These oracles use exponential moving averages (EMA) to smooth out fluctuations and provide stable values over various timeframes. It does this by connecting data feeds from other pallets through callbacks, adding new data points to temporary storage for each block (time period) upon which, the temporary data is merged into permanent storage using the calculations. Oracles are accessed on-demand, and values are "fast-forwarded" to the last block for freshness.
EMA Oracle Pallet | |-- Config | |-- RuntimeEvent | |-- WeightInfo | |-- BlockNumberProvider | |-- SupportedPeriods | |-- MaxUniqueEntries | |-- Error | |-- TooManyUniqueEntries | |-- OnTradeValueZero | |-- Event | |-- Storage | |-- Accumulator | |-- Oracles | |-- GenesisConfig | |-- initial_data | |-- _marker | |-- Hooks | |-- on_initialize | |-- Call | |-- Impl | |-- on_entry | |-- on_trade | |-- on_liquidity_changed | |-- last_block_oracle | |-- update_oracles_from_accumulator | |-- update_oracle | |-- get_updated_entry | |-- OnCreatePoolHandler | |-- on_create_pool | |-- OnTradeHandler | |-- on_trade | |-- on_trade_weight | |-- OnLiquidityChangedHandler | |-- on_liquidity_changed | |-- on_liquidity_changed_weight | |-- Utility Functions | |-- determine_normalized_price | |-- determine_normalized_volume | |-- determine_normalized_liquidity | |-- ordered_pair | |-- OracleError | |-- NotPresent | |-- SameAsset | |-- AggregatedOracle | |-- get_entry | |-- get_entry_weight | |-- AggregatedPriceOracle |-- get_price |-- get_price_weight

3.4. CircuitBreaker pallet

  • This pallet manages and restricts how much of an asset's liquidity can be traded, added, or removed within a single block. It tracks the liquidity trading, adding, and removing limits and resets the these limits to zero after each block.

Pallet lib

Setting limits - The TechnicalOrigin admin can set the trade volume, liquidity that can be added and liquidity that can be removed limits in the protocol. There are sanity checks in place to prevent extreme limits being imposed on the users. These are done by the set_trade_volume_limit, set_add_liquidity_limit, set_remove_liquidity_limit functions.

+-----------------------------------------------------------------------------------------------------------------------------------------------------+ | CircuitBreaker.rs lib | +-----------------------------------------------------------------------------------------------------------------------------------------------------+ | | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | TradeVolumeLimit | | | | +-----------------------+ +------------------------+ +-------------------+ | | | | | volume_in: T::Balance | | volume_out: T::Balance | | limit: T::Balance | | | | | +-----------------------+ +------------------------+ +-------------------+ | | | | | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | | | update_amounts(&mut self, amount_in: T::Balance, amount_out: T::Balance) -> DispatchResult | | | | | check_outflow_limit(&self) -> DispatchResult | | | | | check_influx_limit(&self) -> DispatchResult | | | | | check_limits(&self) -> DispatchResult | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | LiquidityLimit | | | | +-----------------------+ +-------------------+ | | | | | liquidity: T::Balance | | limit: T::Balance | | | | | +-----------------------+ +-------------------+ | | | | | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | | | update_amount(&mut self, liquidity_in: T::Balance) -> DispatchResult | | | | | check_limit(&self) -> DispatchResult | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | Pallet | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | | | set_trade_volume_limit(origin: OriginFor<T>, asset_id: T::AssetId, trade_volume_limit: (u32, u32)) -> DispatchResult | | | | | set_add_liquidity_limit(origin: OriginFor<T>, asset_id: T::AssetId, liquidity_limit: Option<(u32, u32)>) -> DispatchResult | | | | | set_remove_liquidity_limit(origin: OriginFor<T>, asset_id: T::AssetId, liquidity_limit: Option<(u32, u32)>) -> DispatchResult | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | Events | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | | | TradeVolumeLimitChanged { asset_id: T::AssetId, trade_volume_limit: (u32, u32) } | | | | | AddLiquidityLimitChanged { asset_id: T::AssetId, liquidity_limit: Option<(u32, u32)> } | | | | | RemoveLiquidityLimitChanged { asset_id: T::AssetId, liquidity_limit: Option<(u32, u32)> } | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | Errors | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | | | InvalidLimitValue | | | | | LiquidityLimitNotStoredForAsset | | | | | TokenOutflowLimitReached | | | | | TokenInfluxLimitReached | | | | | MaxLiquidityLimitPerBlockReached | | | | | NotAllowed | | | | +---------------------------------------------------------------------------------------------------------------------------------------------+ | | +------------------------------------------------------------------------------------------------------------------------------------------------+ | | | +-----------------------------------------------------------------------------------------------------------------------------------------------------+

3. Codebase Overview

  • Audit Information - For the purpose of the security review, HydraDx comprises thirteen smart contracts totaling over 5273 RLoC. Its core design principle is composition, enabling efficient and flexible integration. The EMA oracle is used to generate token prices.

  • Documentation - The codebase is divided into 4 major sections - Omnipool, StableSwap, EMA oracle, Circuit breaker. Each section has a dedicated README which discusses the major important parts of the contracts. The provided documentation provides brief overview of the omnipool as well as discusses the HydraDX protocol. The contracts are well commented, and explanations where given as to expected functionalities of each function.

  • Protocol Ecosystem - The protocol's ecosystem relies on the polkadot and its parachains, for deployments. The contracts are written in rust, and are to be compiled with cargo.

  • Token Support - The protocol mainly works with ERC20 tokens, including the LRNA and HDX protocol's hub and governance tokens. An ERC721 nft token is also in use which denotes a user's liquidity position in the omnipool.

  • Attack Vectors - Various points of attack exists for a protocol of this size. Pricing manipulations, incorrect calculations in the math contracts, first depositor issues, general issues with AMMs and liquidity pools, etc.


4. Centralization Risks

The protocol employs the actions of two main admins in the scope contracts, the AuthorityOrigin and the TechnicalOrigin While this admin is essentially trusted and assumed to be correctly configured, having such a centralized power puts the protocol at risks, some of which are:

  • Setting various protocol parameters extremely high or low to grief users.
  • Maliciously removing assets and switching assets' tradeablity to grief users.

5. Systemic Risks

  • Low engagement, can cause low liquidity in pools and consequently, affect protocol.
  • External dependencies from compilers, polkachains and so on.
  • Smart contract bugs which at best might be contained to erring contract and at worst could affect entire protocol.
  • Non standard erc20 tokens as assets which can be a source of attack.
  • Slippage and deadline issues due to interactions with pools.

6. Recommendations

  • Setter functions should have two step updates should be implemented including zero address checks. A timelock can also be considered for these functions to give users time to react to protocol changes. Adding these fixes can help protect from admin mistakes and unexepected behaviour.
  • A proper pause function can be implemented to protect users in cases of turbulent market situations and black swan events. It also helps prevent malicious activities or bugs from causing significant damage while the team investigates the potential issues without affecting users' funds.
  • A deadline parameter can also be included to prevent transactions from being executed at an unfavorable time due to network congestion or other factors that could affect the outcome of the trade. By setting a deadline, users specify until what time they are willing to have their transaction included in the blockchain.
  • NFT positon collections are not destroyed upon removal of a token in the omnipool contract, consider checking if this is desired, thus leaving as is or destroying the token collection after removing the token.
  • Multiple fallback oracles should also be implemented and aggregated, with comparisons made with between the various returned prices. This helps ensure more accurate token pricing and protects from bad trades in case the main oracle becomes faulty.
  • Consider upgrading the various cargo packages in use to stay up to date with latest security fixes.

7. Conclusions

  • As an endnote, the codebase was pretty well designed, albeit hard to crack, due its fairly unconventional nautre. All the same, a number of risks were identified and they need to be fixed. Recommended measures should be implemented to protect the protocol from potential attacks. Timely audits and codebase cleanup should be conducted to keep the codebase fresh and up to date with evolving security times.

8. Resources

Time spent:

070 hours

#0 - c4-pre-sort

2024-03-03T18:40:12Z

0xRobocop marked the issue as sufficient quality report

#1 - c4-judge

2024-03-08T19:06:36Z

OpenCoreCH marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter