BitFluxFi - Stable AMM - CoreDAO


Prepared by:

Halborn Logo

HALBORN

Last Updated 12/09/2024

Date of Engagement: October 22nd, 2024 - November 1st, 2024

Summary

100% of all REPORTED Findings have been addressed

All findings

7

Critical

0

High

0

Medium

1

Low

4

Informational

2


1. Introduction

CoreDAO engaged Halborn to conduct a security assessment on their smart contracts beginning on October 22nd and ending on November 1st, 2024. The security assessment was scoped to the smart contracts provided to the Halborn team.

Commit hashes and further details can be found in the Scope section of this report.

2. Assessment Summary

The team at Halborn assigned a full-time security engineer to assess the security of the smart contracts. The security engineer is a blockchain and smart-contract security expert with advanced penetration testing, smart-contract hacking, and deep knowledge of multiple blockchain protocols.

The purpose of this assessment is to:

    • Ensure that smart contract functions operate as intended.

    • Identify potential security issues with the smart contracts.


In summary, Halborn identified some improvements to reduce the likelihood and impact of risks, which were accepted and acknowledged by the BitFlux team. The main ones were the following: 

    • Replace all instances of transfer and transferFrom with safeTransfer and safeTransferFrom from OpenZeppelin's SafeERC20 library.

    • Replace the use of approve with safeIncreaseAllowance to ensure consistent allowance handling.

    • Prevent unnecessary overwriting and allow users to contribute liquidity according to their preferences.

    • Review the differences between the origin implementation and the current one, and make sure that all deviations are design choices.

    • Check for duplicate token addresses in the input arrays within liquidity operations.

3. Test Approach and Methodology

Halborn performed a combination of manual review of the code and automated security testing to balance efficiency, timeliness, practicality, and accuracy in regard to the scope of the smart contract assessment. While manual testing is recommended to uncover flaws in logic, process, and implementation; automated testing techniques help enhance coverage of smart contracts and can quickly identify items that do not follow security best practices. The following phases and associated tools were used throughout the term of the assessment:

    • Research into the architecture, purpose, and use of the platform.

    • Smart contract manual code review and walkthrough to identify any logic issue.

    • Thorough assessment of safety and usage of critical Solidity variables and functions in scope that could lead to arithmetic related vulnerabilities.

    • Manual testing by custom scripts.

    • Graphing out functionality and contract logic/connectivity/functions (solgraph).

    • Static Analysis of security for scoped contract, and imported functions. (Slither).

    • Local or public testnet deployment (Foundry, Remix IDE).

4. RISK METHODOLOGY

Every vulnerability and issue observed by Halborn is ranked based on two sets of Metrics and a Severity Coefficient. This system is inspired by the industry standard Common Vulnerability Scoring System.
The two Metric sets are: Exploitability and Impact. Exploitability captures the ease and technical means by which vulnerabilities can be exploited and Impact describes the consequences of a successful exploit.
The Severity Coefficients is designed to further refine the accuracy of the ranking with two factors: Reversibility and Scope. These capture the impact of the vulnerability on the environment as well as the number of users and smart contracts affected.
The final score is a value between 0-10 rounded up to 1 decimal place and 10 corresponding to the highest security risk. This provides an objective and accurate rating of the severity of security vulnerabilities in smart contracts.
The system is designed to assist in identifying and prioritizing vulnerabilities based on their level of risk to address the most critical issues in a timely manner.

4.1 EXPLOITABILITY

Attack Origin (AO):
Captures whether the attack requires compromising a specific account.
Attack Cost (AC):
Captures the cost of exploiting the vulnerability incurred by the attacker relative to sending a single transaction on the relevant blockchain. Includes but is not limited to financial and computational cost.
Attack Complexity (AX):
Describes the conditions beyond the attacker’s control that must exist in order to exploit the vulnerability. Includes but is not limited to macro situation, available third-party liquidity and regulatory challenges.
Metrics:
EXPLOITABILITY METRIC (mem_e)METRIC VALUENUMERICAL VALUE
Attack Origin (AO)Arbitrary (AO:A)
Specific (AO:S)
1
0.2
Attack Cost (AC)Low (AC:L)
Medium (AC:M)
High (AC:H)
1
0.67
0.33
Attack Complexity (AX)Low (AX:L)
Medium (AX:M)
High (AX:H)
1
0.67
0.33
Exploitability EE is calculated using the following formula:

E=meE = \prod m_e

4.2 IMPACT

Confidentiality (C):
Measures the impact to the confidentiality of the information resources managed by the contract due to a successfully exploited vulnerability. Confidentiality refers to limiting access to authorized users only.
Integrity (I):
Measures the impact to integrity of a successfully exploited vulnerability. Integrity refers to the trustworthiness and veracity of data stored and/or processed on-chain. Integrity impact directly affecting Deposit or Yield records is excluded.
Availability (A):
Measures the impact to the availability of the impacted component resulting from a successfully exploited vulnerability. This metric refers to smart contract features and functionality, not state. Availability impact directly affecting Deposit or Yield is excluded.
Deposit (D):
Measures the impact to the deposits made to the contract by either users or owners.
Yield (Y):
Measures the impact to the yield generated by the contract for either users or owners.
Metrics:
IMPACT METRIC (mIm_I)METRIC VALUENUMERICAL VALUE
Confidentiality (C)None (I:N)
Low (I:L)
Medium (I:M)
High (I:H)
Critical (I:C)
0
0.25
0.5
0.75
1
Integrity (I)None (I:N)
Low (I:L)
Medium (I:M)
High (I:H)
Critical (I:C)
0
0.25
0.5
0.75
1
Availability (A)None (A:N)
Low (A:L)
Medium (A:M)
High (A:H)
Critical (A:C)
0
0.25
0.5
0.75
1
Deposit (D)None (D:N)
Low (D:L)
Medium (D:M)
High (D:H)
Critical (D:C)
0
0.25
0.5
0.75
1
Yield (Y)None (Y:N)
Low (Y:L)
Medium (Y:M)
High (Y:H)
Critical (Y:C)
0
0.25
0.5
0.75
1
Impact II is calculated using the following formula:

I=max(mI)+mImax(mI)4I = max(m_I) + \frac{\sum{m_I} - max(m_I)}{4}

4.3 SEVERITY COEFFICIENT

Reversibility (R):
Describes the share of the exploited vulnerability effects that can be reversed. For upgradeable contracts, assume the contract private key is available.
Scope (S):
Captures whether a vulnerability in one vulnerable contract impacts resources in other contracts.
Metrics:
SEVERITY COEFFICIENT (CC)COEFFICIENT VALUENUMERICAL VALUE
Reversibility (rr)None (R:N)
Partial (R:P)
Full (R:F)
1
0.5
0.25
Scope (ss)Changed (S:C)
Unchanged (S:U)
1.25
1
Severity Coefficient CC is obtained by the following product:

C=rsC = rs

The Vulnerability Severity Score SS is obtained by:

S=min(10,EIC10)S = min(10, EIC * 10)

The score is rounded up to 1 decimal places.
SeverityScore Value Range
Critical9 - 10
High7 - 8.9
Medium4.5 - 6.9
Low2 - 4.4
Informational0 - 1.9

5. SCOPE

Files and Repository
(a) Repository: contracts
(b) Assessed Commit ID: 3e1cf3e
(c) Items in scope:
  • stable-amm/AmplificationUtils.sol
  • stable-amm/LPToken.sol
  • stable-amm/MathUtils.sol
↓ Expand ↓
Out-of-Scope: Third party dependencies and economic attacks.
Out-of-Scope: New features/implementations after the remediation commit IDs.

6. Assessment Summary & Findings Overview

Critical

0

High

0

Medium

1

Low

4

Informational

2

Security analysisRisk levelRemediation Date
Use of standard transfer methods may lead to undetected failuresMediumRisk Accepted - 11/20/2024
Race condition in allowance updates with approveLowRisk Accepted - 11/20/2024
Potential user restrictions in liquidity contributionsLowRisk Accepted - 11/20/2024
Non compliance with Curve implementationLowNot Applicable
Potential handling errors due to duplicate token entries in liquidity functionsLowRisk Accepted - 11/20/2024
Missing protection against potential reentrancy attacksInformationalAcknowledged - 11/20/2024
Suboptimal gas usage due to post-increment in loopsInformationalAcknowledged - 11/20/2024

7. Findings & Tech Details

7.1 Use of standard transfer methods may lead to undetected failures

//

Medium

Description

The functions convert, addLiquidity, removeLiquidity, and removeBaseLiquidityOneToken in the Router contract use transfer and transferFrom methods for token transfers. Unlike safeTransfer and safeTransferFrom from OpenZeppelin's SafeERC20 library, the standard transfer and transferFrom methods do not handle cases where tokens do not return a value or return false upon failure. This could lead to undetected transaction failures and unexpected behavior if tokens do not adhere strictly to the ERC-20 standard, potentially resulting in failed transfers without reverts. The affected functions are the following:

  • Router.convert

  • Router.addLiquidity

  • Router.removeLiquidity

  • Router.removeBaseLiquidityOneToken

BVSS
Recommendation

Replace all instances of transfer and transferFrom with safeTransfer and safeTransferFrom from OpenZeppelin's SafeERC20 library. This ensures that all token transfers are properly checked and revert in case of failure, improving the security and reliability of the contract.

Remediation

RISK ACCEPTED: The BitFlux team accepted this risk of this finding and stated the following:

Router contract already utilizes OpenZeppelin's SafeERC20 library in several parts of the contract. For example, functions such as swapFromBase, swapToBase, and removeLiquidity already use safeTransferFrom and safeTransfer to ensure secure token transfers. This is to make sure that transfers either succeed or revert, preventing issues with tokens that do not return a boolean on success.

There are specific cases where functions make direct calls to transfer and transferFrom. These are mainly used when interacting with well-known tokens, such as LPs, which strictly follows ERC-20 standards.

References
contracts/stable-amm/Router.sol#L31, L46, L102, L116, L154

7.2 Race condition in allowance updates with approve

//

Low

Description

The removeBaseLiquidityOneToken function in the Router contract uses the approve method to set allowances. Unlike other functions such as removeLiquidity, which use safeIncreaseAllowance, this inconsistency can introduce a potential approval race condition risk. The approve function can be exploited if a spender front-runs the transaction and uses the current allowance before it is updated. This could lead to unintended token transfers, posing a security risk if not handled properly.


Code Location

Use of the approve method to set allowances:

    function removeBaseLiquidityOneToken(
        ISwap pool,
        ISwap basePool,
        uint256 _token_amount,
        uint8 i,
        uint256 _min_amount,
        uint256 deadline
    ) external returns (uint256) {
        IERC20 token = pool.getLpToken();
        IERC20 baseToken = basePool.getLpToken();
        uint8 baseTokenIndex = pool.getTokenIndex(address(baseToken));
        token.transferFrom(msg.sender, address(this), _token_amount);
        token.approve(address(pool), _token_amount);
        pool.removeLiquidityOneToken(_token_amount, baseTokenIndex, 0, deadline);
        uint256 _base_amount = baseToken.balanceOf(address(this));
        baseToken.approve(address(basePool), _base_amount);
        basePool.removeLiquidityOneToken(_base_amount, i, _min_amount, deadline);
        IERC20 coin = basePool.getToken(i);
        uint256 coin_amount = coin.balanceOf(address(this));
        coin.safeTransfer(msg.sender, coin_amount);
        return coin_amount;
    }
BVSS
Recommendation

Replace the use of approve with safeIncreaseAllowance to align with best practices and ensure consistent allowance handling.

Remediation

RISK ACCEPTED: The BitFlux team accepted this risk of this finding and stated the following:

We acknowledge that the approve method can introduce a race condition if a spender front-runs the transaction and uses the current allowance before it is updated. However, in our contract, this risk is mitigated by the specific context in which approve is used. The approve function is only called within tightly controlled internal logic, where the sequence of operations ensures that no external actor can exploit the allowance before it is consumed. Additionally, we use approve only with tokens that strictly follow the ERC-20 standard, which further reduces the likelihood of an issue. 

7.3 Potential user restrictions in liquidity contributions

//

Low

Description

The addLiquidity function in the Router contract has a potential issue where meta_amounts[i] is overwritten by base_lp_received when coin matches base_lp. This leads to two scenarios:


  1. If the initial meta_amounts[i] is lower than base_lp_received, the user is compelled to use the higher base_lp_received value, potentially depositing more tokens than intended.


  2. If the initial meta_amounts[i] is greater than base_lp_received, the user can only deposit up to base_lp_received, restricting the ability to contribute the desired amount.


This behavior reduces flexibility and can result in unexpected outcomes for users who intend to provide a specific amount of liquidity.


Code Location

meta_amounts[i] is overwritten by base_lp_received when coin matches base_lp:

    function addLiquidity(
        ISwap pool,
        ISwap basePool,
        uint256[] memory meta_amounts,
        uint256[] memory base_amounts,
        uint256 minToMint,
        uint256 deadline
    ) external returns (uint256) {
        IERC20 token = IERC20(pool.getLpToken());
        IERC20 base_lp = IERC20(basePool.getLpToken());
        require(base_amounts.length == basePool.getNumberOfTokens(), "invalidBaseAmountsLength");
        require(meta_amounts.length == pool.getNumberOfTokens(), "invalidMetaAmountsLength");
        bool deposit_base = false;
        for (uint8 i = 0; i < base_amounts.length; i++) {
            uint256 amount = base_amounts[i];
            if (amount > 0) {
                deposit_base = true;
                IERC20 coin = basePool.getToken(i);
                uint256 transferred = transferIn(coin, msg.sender, amount);
                coin.safeIncreaseAllowance(address(basePool), transferred);
                base_amounts[i] = transferred;
            }
        }

        uint256 base_lp_received;
        if (deposit_base) {
            base_lp_received = basePool.addLiquidity(base_amounts, 0, deadline);
        }

        for (uint8 i = 0; i < meta_amounts.length; i++) {
            IERC20 coin = pool.getToken(i);

            uint256 transferred;
            if (address(coin) == address(base_lp)) {
                transferred = base_lp_received;
            } else if (meta_amounts[i] > 0) {
                transferred = transferIn(coin, msg.sender, meta_amounts[i]);
            }

            meta_amounts[i] = transferred;
            if (transferred > 0) {
                coin.safeIncreaseAllowance(address(pool), transferred);
            }
        }

        uint256 base_lp_prior = base_lp.balanceOf(address(this));
        pool.addLiquidity(meta_amounts, minToMint, deadline);
        if (deposit_base) {
            require((base_lp.balanceOf(address(this)) + base_lp_received) == base_lp_prior, "invalidBasePool");
        }

        uint256 lpAmount = token.balanceOf(address(this));
        token.transfer(msg.sender, lpAmount);
        return lpAmount;
    }
BVSS
Recommendation

Adjust the logic to ensure that the value of meta_amounts[i] aligns with user input while maintaining proper handling of base_lp tokens. This change should prevent unnecessary overwriting and allow users to contribute liquidity according to their preferences.

Remediation

RISK ACCEPTED: The BitFlux team accepted this risk of this finding and stated the following:

We acknowledge that there is potential for user-specified values in meta_amounts[i] to be overwritten by base_lp_received. However, this behavior is intentional and designed to ensure that liquidity contributions are handled efficiently. In cases where users provide liquidity with both base LP tokens and meta tokens, it is necessary to adjust meta_amounts[i] to reflect the actual amount received from the base pool. This is to make sure accurate accounting and prevents discrepancies between user expectations and actual liquidity added. 

7.4 Non compliance with Curve implementation

//

Low

Description

Context

The stable-amm is an implementation of Curve's stable swap, and the Halborn team verified the proper implementation by inspecting the differences with the SwapTemplateBase.vy contract from the b0bbf77 commit.


Inconsistencies were identified in:

  • calculateTokenAmount from SwapUtils.sol

  • getAdminbalance from Swap.sol

  • stopRampA from AmplificationUtils.sol

  • killMe feature


Calculate token amount

It was found that the calculateTokenAmount of the SwapUtils.sol library returns a positive amount when the total supply of the liquidity pool is zero, while the original Curve implementation simply returns a zero amount. The affected function is only for view purposes so it does not affect the pool state, but could cause unplanned behaviour in external contracts.


  • The Curve implementation in SwapTemplateBase.vy:

def calc_token_amount(_amounts: uint256[N_COINS], _is_deposit: bool) -> uint256:
    """
    @notice Calculate addition or reduction in token supply from a deposit or withdrawal
    @dev This calculation accounts for slippage, but not fees.
         Needed to prevent front-running, not for precise calculations!
    @param _amounts Amount of each coin being deposited
    @param _is_deposit set True for deposits, False for withdrawals
    @return Expected amount of LP tokens received
    """
    amp: uint256 = self._A()
    balances: uint256[N_COINS] = self.balances
    D0: uint256 = self._get_D_mem(balances, amp)
    for i in range(N_COINS):
        if _is_deposit:
            balances[i] += _amounts[i]
        else:
            balances[i] -= _amounts[i]
    D1: uint256 = self._get_D_mem(balances, amp)
    token_amount: uint256 = CurveToken(self.lp_token).totalSupply()
    diff: uint256 = 0
    if _is_deposit:
        diff = D1 - D0
    else:
        diff = D0 - D1
    return diff * token_amount / D0

  • The Bitflux implementation in SwapUtils.sol the new stable pool invariant d1 is returned:

function calculateTokenAmount(
    Swap storage self,
    uint256[] calldata amounts,
    bool deposit
) external view returns (uint256) {
    uint256 a = _getAPrecise(self);
    uint256[] memory balances = self.balances;
    uint256[] memory multipliers = self.tokenPrecisionMultipliers;

    uint256 d0 = getD(_xp(balances, multipliers), a);
    for (uint256 i = 0; i < balances.length; i++) {
        if (deposit) {
            balances[i] = balances[i].add(amounts[i]);
        } else {
            balances[i] = balances[i].sub(
                amounts[i],
                "Cannot withdraw more than available"
            );
        }
    }
    
    uint256 d1 = getD(_xp(balances, multipliers), a);
    uint256 totalSupply = self.lpToken.totalSupply();

    if (totalSupply == 0) {
        return d1; // first depositor take it all
    }

    if (deposit) {
        return d1.sub(d0).mul(totalSupply).div(d0);
    } else {
        return d0.sub(d1).mul(totalSupply).div(d0);
    }
}

Get admin balance

It was found that the getAdminbalance from the Swap.sol contract was not following the same logic than the admin_balances counterpart in the Curve implementation.


  • The Curve implementation in SwapTemplateBase.vy:

def admin_balances(i: uint256) -> uint256:
    return ERC20(self.coins[i]).balanceOf(self) - self.balances[i]

  • The Bitflux implementation in Swap.sol:

function getAdminBalances() external view returns (uint256[] memory adminBalances) {
    uint256 length = swapStorage.pooledTokens.length;
    adminBalances = new uint256[](length);
    for (uint256 i = 0; i < length; i++) {
        adminBalances[i] = swapStorage.getAdminBalance(i);
    }
}

Stop ramp A

It was found that the stopRampA of the AmplificationUtils.sol library included an additional check, in comparison to the original implementation. The stopRampA function stops the transition of the amplifier factor of the stable pool, during its transition. That additional check only makes sure that the function cannot be called outside a transition, which has essentially no impact.


  • The Curve implementation in SwapTemplateBase.vy:

def stop_ramp_A():
    assert msg.sender == self.owner  # dev: only owner

    current_A: uint256 = self._A()
    self.initial_A = current_A
    self.future_A = current_A
    self.initial_A_time = block.timestamp
    self.future_A_time = block.timestamp
    # now (block.timestamp < t1) is always False, so we return saved A

    log StopRampA(current_A, block.timestamp)

  • The Bitflux implementation in AmplificationUtils.sol (note that the onlyOwner modifier is present upstream):

function stopRampA(SwapUtils.Swap storage self) external {
    require(self.futureATime > block.timestamp, "Ramp is already stopped");

    uint256 currentA = _getAPrecise(self);
    self.initialA = currentA;
    self.futureA = currentA;
    self.initialATime = block.timestamp;
    self.futureATime = block.timestamp;

    emit StopRampA(currentA, block.timestamp);
}

Kill feature

The original Curve contract has a killMe feature that allows an owner to shut down a contract, also able to revive it later. It was found that the current implementation does not support this feature. An external contract may assume this feature to be implemented an malfunction as it is not.


  • The kill_me definition in SwapTemplateBase.vy:

is_killed: bool
kill_deadline: uint256
KILL_DEADLINE_DT: constant(uint256) = 2 * 30 * 86400

  • The kill_me toggles in SwapTemplateBase.vy:

@external
def kill_me():
    assert msg.sender == self.owner  # dev: only owner
    assert self.kill_deadline > block.timestamp  # dev: deadline has passed
    self.is_killed = True


@external
def unkill_me():
    assert msg.sender == self.owner  # dev: only owner
    self.is_killed = False

  • Example usage of the kill_me feature in the add_liquidity function of SwapTemplateBase.vy:

@external
@nonreentrant('lock')
def add_liquidity(_amounts: uint256[N_COINS], _min_mint_amount: uint256) -> uint256:
    """
    @notice Deposit coins into the pool
    @param _amounts List of amounts of coins to deposit
    @param _min_mint_amount Minimum amount of LP tokens to mint from the deposit
    @return Amount of LP tokens received by depositing
    """
    assert not self.is_killed  # dev: is killed

    amp: uint256 = self._A()
    old_balances: uint256[N_COINS] = self.balances
BVSS
Recommendation

It is recommended to review the differences between the origin implementation and the current one, and make sure that all deviations are design choices.

Remediation

NOT APPLICABLE: The BitFlux team highlighted that the issue is not applicable and mentioned the following:

We recognize that our implementation differs from Curve’s original design in certain areas, such as calculateTokenAmount, getAdminBalance, and stopRampA. These differences were intentional design choices made to optimize performance and improve usability for our specific use case. For example, returning a positive amount when the total supply is zero in calculateTokenAmount simplifies initial liquidity provision without affecting pool state or security. Similarly, our additional check in stopRampA ensures that transitions are handled more safely without introducing any negative side effects.

7.5 Potential handling errors due to duplicate token entries in liquidity functions

//

Low

Description

In the addLiquidity and removeLiquidity functions of the Router contract, user-passed arrays (meta_amounts, base_amounts, etc.) are used to perform token transfers and allowances. If these arrays contain duplicate token addresses, the functions may perform redundant operations on the same token, leading to potential mismanagement of allowances or incorrect token balances. This could cause inconsistencies in the expected behavior of these operations, making it crucial to ensure that each token is processed only once.

BVSS
Recommendation

Implement a validation mechanism to check for duplicate token addresses in the input arrays within addLiquidity and removeLiquidity functions. This will ensure that operations are executed accurately, with each token being handled only once, maintaining consistency and preventing any potential mismanagement of token balances.

Remediation

RISK ACCEPTED: The BitFlux team accepted this risk of this finding and stated the following:

We acknowledge that allowing duplicate token entries in liquidity functions could theoretically lead to handling errors. However, our contract’s internal logic ensures that duplicate entries are processed correctly without causing unexpected behavior. This is a design decision to handle each token individually, even if duplicates exist in the input array, we make sure that all tokens are accounted for properly during liquidity operations.

7.6 Missing protection against potential reentrancy attacks

//

Informational

Description

The Router contract includes functions such as convert, addLiquidity, removeLiquidity, and removeBaseLiquidityOneToken that handle ERC20 token transfers. Although the use of transfer and transferFrom functions is common, it is important to consider the risk of reentrancy when these functions make multiple external calls. Even though standard ERC20 tokens are assumed to be secure, there could still be potential vulnerabilities if reentrancy protection is not implemented. This observation is informational, as no immediate issues are present given that only valid ERC20 tokens are used. However, it highlights the importance of adopting protective measures as a best practice.

BVSS
Recommendation

It is recommended to implement ReentrancyGuard from OpenZeppelin’s library by adding the nonReentrant modifier to the mentioned function to prevent recursive calls.

Remediation

ACKNOWLEDGED: The BitFlux team acknowledged this issue and stated the following:

Reentrancy protection is indeed crucial for preventing malicious recursive calls. We have already implemented reentrancy protection using OpenZeppelin’s ReentrancyGuard in critical areas where external calls are made. However, for other functions where reentrancy risks are minimal or non-existent (e.g., pure view functions or internal operations), we have opted not to apply reentrancy protection to avoid unnecessary gas costs.

7.7 Suboptimal gas usage due to post-increment in loops

//

Informational

Description

In multiple functions across the codebase, the for loops use i++ (post-increment) for incrementing the loop counter. In Solidity, post-increment (i++) is slightly less efficient than pre-increment (++i) because i++ requires storing the original value of i in a temporary variable before incrementing, which consumes more gas. Although the gas difference is minimal, especially in recent Solidity versions, it becomes noticeable in larger loops or frequently executed functions, leading to inefficiencies in contract execution. The affected functions are the following:

  • Router.convert

  • Router.addLiquidity

  • Router.removeLiquidity

  • Router.calculateConvert

  • Swap.initialize

  • Swap.getAdminBalances

  • SwapUtils.calculateWithdrawOneTokenDY

  • SwapUtils.getYD

  • SwapUtils.getD

  • SwapUtils._xp

  • SwapUtils.getY

  • SwapUtils._calculateRemoveLiquidity

  • SwapUtils.calculateTokenAmount

  • SwapUtils.addLiquidity

  • SwapUtils.removeLiquidity

  • SwapUtils.removeLiquidityImbalance

  • SwapUtils.withdrawAdminFees

BVSS
Recommendation

To optimize gas usage, especially when iterating over large arrays or loops, it is recommended to replace i++ with ++i. Pre-increment (++i) does not require storing the old value of i, making it slightly more efficient in terms of gas consumption.

Remediation

ACKNOWLEDGED: The BitFlux team acknowledged this issue and stated the following:

We acknowledge that using post-increment (i++) instead of pre-increment (++i) can lead to marginally higher gas usage. However, this difference is negligible in practice and does not significantly impact overall gas efficiency. Given that post-increment is more widely used and understood by developers, we have chosen to prioritize readability and consistency across our codebase over micro-optimizations.

References
contracts/stable-amm/Router.sol#L27, L37, L63, L79, L125, L134, L229
contracts/stable-amm/Swap.sol#L130, L307
contracts/stable-amm/SwapUtils.sol#L224, L280, L294, L319, L330, L332, L385, L456, L476, L576, L609, L767, L803, L870, L978, L986, L1006, L1028

Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.

© Halborn 2025. All rights reserved.