NAMI Protocol Rujira Index Product - THORChain

The current fee mechanism in the Nami protocol used for both AUM (Assets Under Management) and transaction fees can be bypassed through rounding behavior during high-frequency or micro-amount operations.

Specifically, both aum_fee and tx_fee functions use .to_uint_floor() to convert Decimal values to integers. When the calculated fee is less than 1 token unit, it is silently rounded down to zero and discarded. Despite this, last_accrual_time is still updated as if the fee had been collected. This allows attackers to repeatedly perform very small interactions (e.g., automated bots calling many micro-withdrawals), causing both AUM and transaction fees to round to zero on every call, effectively avoiding any fee payment.

Over time, this can significantly reduce protocol revenue and break the expected economic model, especially if abused at scale.

Code Location

Code of aum_fee function from packages/nami-rs/src/fee_manager.rs file.

pub fn aum_fee(
    &mut self,
    now: Timestamp,
    total_supply: Uint128,
) -> Result<Uint128, FeeManagerError> {
    let rate = self.rates.management.unwrap_or_default();

    let elapsed = now
        .seconds()
        .saturating_sub(self.last_accrual_time.seconds());

    if elapsed == 0 {
        return Ok(Uint128::zero());
    }

    const SECS_PER_YEAR: u64 = 31_557_600;
    let year_fraction = Decimal::from_ratio(elapsed as u128, SECS_PER_YEAR as u128);
    let effective = rate.checked_mul(year_fraction)?;

    let fee_amount = Decimal::from_ratio(total_supply, Uint128::one())
        .checked_mul(effective)?
        .to_uint_floor();

    self.last_accrual_time = now;
    Ok(fee_amount)
}

Code of tx_fee function from packages/nami-rs/src/fee_manager.rs file.

pub fn tx_fee(&self, amount: Uint128) -> Result<(Uint128, Uint128), FeeManagerError> {
    let rate = self.rates.transaction.unwrap_or_default();

    let fee = Decimal::from_ratio(amount, Uint128::one())
        .checked_mul(rate)?
        .to_uint_floor();
    let net = amount.checked_sub(fee)?;
    Ok((net, fee))

Scenario

This test simulates a scenario where a user makes a deposit into a vault that has both management (AUM) and transaction fees enabled at 1%. The deposit value is intentionally low enough that the resulting fee calculations produce amounts below 1 unit per asset, which are then rounded down to zero.

One year of blockchain time is simulated to pass, allowing AUM fees to accrue. The user then performs a minimal withdrawal of 1 share. The goal is to confirm that the total_fees vector is empty, meaning no fees were collected.

Test

fn test_fee_rounding_to_zero_skips_fee_accrual() {
    
    // Set up a vault with AUM and TX fees
    let balances = vec![
        (
            "user",
            vec![
                coin(10_000_000, "nami"),
                coin(10_000_000, "auto"),
                coin(10_000_000, "lqdy"),
            ],
        ),
        (
            "owner",
            vec![
                coin(100_000_000, "nami"),
                coin(100_000_000, "auto"),
                coin(100_000_000, "lqdy"),
                coin(100_000_000, "eth-usdc"),
            ],
        ),
    ];

    let mut test_env = index::setup(
        balances,
        "eth-usdc".to_string(),
        vec![
            ("nami".to_string(), Uint128::from(100_000u128)),
            ("auto".to_string(), Uint128::from(100_000u128)),
            ("lqdy".to_string(), Uint128::from(100_000u128)),
        ],
        Some(Decimal::percent(1)), // AUM fee
        Some(Decimal::percent(1)), // TX fee
    );

    let rcpt_denom = format!("x/nami-index-{}-rcpt", test_env.index.address);

    // First deposit - small enough so that TX and AUM fee round to 0
    test_env
        .index
        .execute_deposit(
            &mut test_env.app,
            "user",
            vec![
                coin(100_000_0, "nami"),
                coin(100_000_0, "auto"),
                coin(100_000_0, "lqdy"),
            ],
        )
        .unwrap();

    // Move time forward 1 year
    const SECS_PER_YEAR: u64 = 31_557_600;
    test_env.app.update_block(|block| {
        block.height += 1;
        block.time = block.time.plus_seconds(SECS_PER_YEAR);
    });

    // Withdraw 1 share - fee should be 0 due to rounding
    let _ = test_env
        .index
        .execute_withdraw(
            &mut test_env.app,
            "user",
            vec![coin(1, rcpt_denom.clone())],
        )
        .unwrap();

    // Now query vault status
    let status = test_env.index.query_status(&mut test_env.app).unwrap();
    let shares = status.total_shares;
    println!("----> Status:");
    println!("Total shares: {}", shares);
    println!("Allocations:");
    for (denom, weight) in status.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    let config = test_env.index.query_config(&mut test_env.app).unwrap();

    let total_fees = test_env
    .app
    .query_all_balances( config.fee_collector.as_str(), false);


    println!("Total fees collected: {:?}", total_fees);
    assert_eq!(total_fees.len(), 0);

}

Result

The test passes and confirms that both AUM and transaction fees were effectively skipped due to rounding. 

It is recommended to accumulate fractional fees in contract state using a Decimal field (e.g., pending_aum or pending_tx). Only once the accumulated amount surpasses the 1-token threshold should shares be minted. Alternatively, using .to_uint_ceil() would ensure at least 1 share is collected whenever a non-zero fee is due. For further mitigation, the protocol could enforce a minimum fee or implement a cooldown period and minimum transaction size to prevent abuse through micro-interactions.

SOLVED: The Nami team resolved this issue by rounding fractional fees up (using ceiling rounding) and setting a minimum fee of 1 whenever the calculated fee is greater than zero.

In the nami-index-fixed contract, the Withdraw operation triggers a rebalance before calculating and distributing output funds. However, if a Deposit is executed immediately after, the contract still uses the pre-rebalance amount_per_share value to calculate how many shares to mint for the depositor, because the Deposit logic only performs the rebalance after completing the share calculation.

This creates a subtle but important inconsistency: the deposit is evaluated using outdated economic proportions that do not reflect the current vault state. As a result, the user may receive more shares than warranted, especially if the rebalance significantly changed the value of the underlying assets.

One possible solution would be to move the rebalance to the beginning of the Deposit execution. However, this approach may conflict with external systems—such as nami-index-entry-adapter—which rely on pre-rebalance amount_per_share values to proportionally prepare token inputs.

This behaviour introduces opportunities for front-running, where users could time deposits right after large withdrawals to exploit stale accounting data and gain an unfair advantage.

Code Location

Code of execute function from contracts/nami-index-fixed/src/contract.rs file.

match msg {
    ExecuteMsg::Deposit {} => {
        let amount = Vault::deposit(deps.storage, info.funds.clone())?;
        response = response.add_event(event_deposit(
            info.sender.clone(),
            info.funds.clone(),
            amount,
        ));
        vault.rebalance(deps.storage, rcpt.supply(deps.querier)? + aum_fee + amount)?;
        response = response.add_message(rcpt.mint_msg(amount, info.sender));
    }
    ExecuteMsg::Withdraw {} => {
        let amount = must_pay(&info, rcpt.denom().as_str())?;
        let (net, burn_fee) = fee_manager.tx_fee(amount)?;
        vault.rebalance(deps.storage, rcpt.supply(deps.querier)? + aum_fee)?;
        let withdraw_funds = Vault::withdraw(deps.storage, net)?;
        response = response.add_event(event_withdraw(
            info.sender.clone(),
            withdraw_funds.clone(),
            amount,
        ));
        let send_msg = BankMsg::Send {
            to_address: info.sender.to_string(),
            amount: withdraw_funds,
        };
        response = response.add_message(rcpt.burn_msg(amount));
        if burn_fee.gt(&Uint128::zero()) {
            response =
                response.add_message(rcpt.mint_msg(burn_fee, config.fee_collector.clone()));
        }
        response = response.add_message(send_msg);
    }

Scenario

This test demonstrates how a deposit can be incorrectly evaluated using a stale amount_per_share if a rebalance is not executed between a previous withdrawal and the new deposit. This inconsistency leads to an over-minting of shares. To enable manual rebalancing and simulate a correct accounting update, an additional helper function named execute_run was added to the mock_nami_index_fixed.rs file.

A vault is initialized with 3 assets (nami, auto, lqdy), each with equal weights. The user performs an initial deposit, which mints some receipt tokens, establishing the initial amount_per_share for each asset. A full year of simulated block time is advanced to accrue the AUM fee.

Test

#[test]
fn test_deposit_uses_stale_amount_per_share() {
    // Initialize user balances
    let balances = vec![
        (
            "user",
            vec![
                coin(20_000_000_000, "nami"),
                coin(20_000_000_000, "auto"),
                coin(20_000_000_000, "lqdy"),
            ],
        ),
        (
            "owner",
            vec![
                coin(100_000_000_000, "nami"),
                coin(100_000_000_000, "auto"),
                coin(100_000_000_000, "lqdy"),
                coin(100_000_000_000, "eth-usdc"),
            ],
        ),
    ];

    let allocations = vec![
        ("nami".to_string(), Uint128::from(1000u128)),
        ("auto".to_string(), Uint128::from(1000u128)),
        ("lqdy".to_string(), Uint128::from(1000u128)),
    ];

    // ---- Scenario A: without forced Run ----
    let mut test_env_a = index::setup(
        balances.clone(),
        "eth-usdc".to_string(),
        allocations.clone(),
        Some(Decimal::percent(50)),
        Some(Decimal::percent(0)),
    );

    let rcpt_denom_a = format!("x/nami-index-{}-rcpt", test_env_a.index.address);

    // First deposit
    test_env_a
        .index
        .execute_deposit(
            &mut test_env_a.app,
            "user",
            vec![
                coin(10_000u128, "nami"),
                coin(10_000u128, "auto"),
                coin(10_000u128, "lqdy"),
            ],
        )
        .unwrap();


    // move block 1 year
    const SECS_PER_YEAR: u64 = 31_557_600; // ≈365.25 days
    test_env_a.app.update_block(|block| {
        block.height += 1;
        block.time = block.time.plus_seconds(SECS_PER_YEAR);
    });

    let status_a = test_env_a.index.query_status(&mut test_env_a.app).unwrap();
    let shares_a = status_a.total_shares;

    // Check status before withdraw
    println!("");
    println!("----- SCENARIO A -----");
    println!("");
    println!("Check Status: first deposit, before withdraw, without manual Run");
    println!("Total shares: {}", shares_a);
    println!("Allocations:");
    for (denom, weight) in status_a.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    // Withdraw
    test_env_a
        .index
        .execute_withdraw(
            &mut test_env_a.app,
            "user",
            vec![coin(10u128, rcpt_denom_a.clone())],
        )
        .unwrap();


    // Check status after withdraw    
    let status_a = test_env_a.index.query_status(&mut test_env_a.app).unwrap();
    let shares_a = status_a.total_shares;
    println!("Check Status: first deposit, after withdraw, without manual Run");
    println!("Total shares: {}", shares_a);
    println!("Allocations:");
    for (denom, weight) in status_a.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    // Second deposit
    test_env_a
        .index
        .execute_deposit(
            &mut test_env_a.app,
            "user",
            vec![
                coin(1_000_000u128, "nami"),
                coin(1_000_000u128, "auto"),
                coin(1_000_000u128, "lqdy"),
            ],
        )
        .unwrap();

    // Check status after second deposit    
    let status_a = test_env_a.index.query_status(&mut test_env_a.app).unwrap();
    let shares_a = status_a.total_shares;
    println!("Check Status: second deposit, after withdraw, without manual Run");
    println!("Total shares: {}", shares_a);
    println!("Allocations:");
    for (denom, weight) in status_a.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    // ---- Scenario B: with forced Run after withdraw ----
    let mut test_env_b = index::setup(
        balances,
        "eth-usdc".to_string(),
        allocations,
        Some(Decimal::percent(50)),
        Some(Decimal::percent(0)),
    );

    let rcpt_denom_b = format!("x/nami-index-{}-rcpt", test_env_b.index.address);

    // First deposit
    test_env_b
        .index
        .execute_deposit(
            &mut test_env_b.app,
            "user",
            vec![
                coin(10_000u128, "nami"),
                coin(10_000u128, "auto"),
                coin(10_000u128, "lqdy"),
            ],
        )
        .unwrap();

    // move block 1 year
    test_env_b.app.update_block(|block| {
        block.height += 1;
        block.time = block.time.plus_seconds(SECS_PER_YEAR);
    });

    // Check status before withdraw
    let status_b = test_env_b.index.query_status(&mut test_env_b.app).unwrap();
    let shares_b = status_b.total_shares;
    println!("");
    println!("----- SCENARIO B -----");
    println!("");
    println!("Check Status: first deposit, before withdraw, before manual Run");
    println!("Total shares: {}", shares_b);
    println!("Allocations:");
    for (denom, weight) in status_b.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    // Withdraw
    test_env_b
        .index
        .execute_withdraw(
            &mut test_env_b.app,
            "user",
            vec![coin(10u128, rcpt_denom_b.clone())],
        )
        .unwrap();


    // Check status after withdraw    
    let status_b = test_env_b.index.query_status(&mut test_env_b.app).unwrap();
    let shares_b = status_b.total_shares;
    println!("Check Status: first deposit, after withdraw, BEFORE manual Run");
    println!("Total shares: {}", shares_b);
    println!("Allocations:");
    for (denom, weight) in status_b.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    // Call Run manually to update internal state before next deposit
    test_env_b
        .index
        .execute_run(&mut test_env_b.app, "user")
        .unwrap();


    // Check status after Run
    let status_b = test_env_b.index.query_status(&mut test_env_b.app).unwrap();
    let shares_b = status_b.total_shares;
    println!("Check Status: first deposit, after withdraw, AFTER manual Run");
    println!("Total shares: {}", shares_b);
    println!("Allocations:");
    for (denom, weight) in status_b.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    // Second deposit
    test_env_b
        .index
        .execute_deposit(
            &mut test_env_b.app,
            "user",
            vec![
                coin(1_000_000u128, "nami"),
                coin(1_000_000u128, "auto"),
                coin(1_000_000u128, "lqdy"),
            ],
        )
        .unwrap();

    // Check status after second deposit    
    let status_b = test_env_b.index.query_status(&mut test_env_b.app).unwrap();
    let shares_b = status_b.total_shares;
    println!("Check Status: second deposit, after withdraw, after manual Run");
    println!("Total shares: {}", shares_b);
    println!("Allocations:");
    for (denom, weight) in status_b.allocation.iter() {
        println!("- {}: {}", denom, weight);
    }

    // Compare results
    assert!(
        shares_a != shares_b,
        "Expected different share totals with and without Run"
    );
}

Result

The test succeeds in showing a measurable difference in total shares. Without the manual Run, the vault over-mints shares, proving the impact of stale share calculations and demonstrating the vulnerability.

It is recommended to restructure the Withdraw logic so that the rebalance is performed only after the withdrawal has completed. This ensures that the internal amount_per_share values remain stable during the deposit calculation phase.

SOLVED: The Nami team has solved this issue by introducing an extra rebalance of the vault at the end of the withdraw transaction.

In the nami-index-fixed contract, the ExecuteMsg::Callback message can be executed by any sender, without verifying whether the caller is an authorized contract registered in the vault’s allocation map. This lack of sender validation creates a security risk: untrusted or malicious contracts may send Callback messages to trigger sensitive internal operations such as AfterReallocate.

These internal operations are intended to be used exclusively by trusted swap contracts as part of the reallocation flow. Allowing arbitrary contracts to invoke them introduces the risk of unexpected state changes, misuse of vault funds, or denial of service conditions.

Code Location

Code of execute function from contracts/nami-index-fixed/src/contract.rs file.

ExecuteMsg::Callback(cb) => {
    let callback_type: CallbackType = cb.deserialize_callback()?;
    match callback_type {
        CallbackType::AfterReallocate {
            swap_to,
            min_return,
        } => {
            let coin = deps
                .querier
                .query_balance(&env.contract.address, &config.quote_denom)?;
            let msg = WasmMsg::Execute {
                contract_addr: swap_to.to_string(),
                msg: to_json_binary(&fin::ExecuteMsg::Swap(SwapRequest {
                    min_return,
                    to: None,
                    callback: None,
                }))?,
                funds: vec![coin],
            };
            let run_msg = WasmMsg::Execute {
                contract_addr: env.contract.address.to_string(),
                msg: to_json_binary(&ExecuteMsg::Run {})?,
                funds: vec![],
            };
            response = response.add_message(msg).add_message(run_msg);
        }
    }
}

It is recommended to validate that info.sender matches a known swap contract listed in the ALLOCATIONS map before executing any logic under the Callback message. This ensures that only authorized and registered contracts involved in the reallocation process are allowed to invoke internal callbacks.

SOLVED: The Nami team resolved this issue by verifying whether the info.sender address belongs to the set of allowed allocations.

In the nami-index-fixed contract, during a reallocation operation, the vault performs two sequential swaps: the first converts the source asset (from) into quote_denom, and the second swaps quote_denom into the target asset (to). However, the second swap uses the entire quote_denom balance held by the vault, without distinguishing whether those funds originated from the first swap or were already present in the vault.

This design assumes that all quote_denom available at that moment comes from the initial swap. But if the vault holds pre-existing quote_denom due to dust, prior rounding, user donations, or unrelated transfers, then the second swap may consume more than intended. This can lead to:

• Inaccurate amount_per_share calculations after rebalance.

• Systemic misalignment affecting all users and share value accuracy.

The lack of isolation between intended swap input and residual balances introduces uncertainty and undermines the deterministic behavior of the reallocation logic.

Code Location

Code of Callback::AfterReallocate logic from contracts/nami-index-fixed/src/contract.rs file.

ExecuteMsg::Callback(cb) => {
    let callback_type: CallbackType = cb.deserialize_callback()?;
    match callback_type {
        CallbackType::AfterReallocate {
            swap_to,
            min_return,
        } => {
            let coin = deps
                .querier
                .query_balance(&env.contract.address, &config.quote_denom)?;
            let msg = WasmMsg::Execute {
                contract_addr: swap_to.to_string(),
                msg: to_json_binary(&fin::ExecuteMsg::Swap(SwapRequest {
                    min_return,
                    to: None,
                    callback: None,
                }))?,
                funds: vec![coin],
            };
            let run_msg = WasmMsg::Execute {
                contract_addr: env.contract.address.to_string(),
                msg: to_json_binary(&ExecuteMsg::Run {})?,
                funds: vec![],
            };
            response = response.add_message(msg).add_message(run_msg);
        }
    }
}

It is recommended to isolate and track the exact amount of quote_denom received from the first swap, and use only that amount in the second swap step. Alternatively, before initiating the reallocation process, the contract could check for any existing quote_denom balance and abort the operation if such funds are present.

SOLVED: The Nami team resolved this issue by querying the amount swapped during the first operation and passing that exact value to the second swap, instead of using the vault's entire balance.

In the Nami protocol, several functions across different contracts allow the registration of asset-related configurations using a denom string as the map key. These include:

• add_contract in nami-index-entry-adapter

• add_allocation in nami-index-fixed

• save_allocation in nami-index-nav

None of these functions verify whether the key being used (the denom) actually matches the base_denom declared in the configuration of the associated swap contract.

An important nuance is that swap contracts may define the quote_denom in either the base() or quote() field of their internal configuration, depending on the direction of the swap. Therefore, the validation logic must ensure that the registered denom matches the opposite field (i.e., the one that is not equal to the current quote_denom).

This is not just a consistency problem, it also has direct functional consequences. For example, if a mismatched denom is used, subsequent operations that rely on query_balance (such as calculating deposit proportions or rebalancing asset weights) will query for a token that is not actually associated with the contract, likely resulting in zero balances or execution errors.

Code Location

Code of add_contract function from contracts/nami-index-entry-adapter/src/state.rs file.

pub fn add_contract(
    &self,
    storage: &mut dyn Storage,
    querier: &QuerierWrapper,
    key: &'a str,
    value: String,
    quote_denom: String,
) -> Result<(), ContractError> {
    let config: fin::ConfigResponse =
        querier.query_wasm_smart(&value, &fin::QueryMsg::Config {})?;
    ensure_eq!(
        config.denoms.quote(),
        quote_denom,
        ContractError::InvalidQuoteDenom
    );
    self.swap_contracts.save(storage, key, &value)?;
    Ok(())
}

Code of add_allocation function from contracts/nami-index-fixed/src/vault.rs file.

pub fn add_allocation(
    &self,
    storage: &mut dyn Storage,
    quote_denom: &str,
    denom: &str,
    weight: Uint128,
    contract: &str,
) -> Result<(), ContractError> {
    let contract = self.api.addr_validate(contract)?;
    let config: fin::ConfigResponse = self
        .querier
        .query_wasm_smart(&contract, &fin::QueryMsg::Config {})?;
    ensure!(
        config.denoms.quote() == quote_denom || config.denoms.base() == quote_denom,
        ContractError::InvalidQuoteDenom
    );
    ALLOCATIONS.save(storage, denom, &(weight, contract))?;
    Ok(())
}

Code of save_allocation function from contracts/nami-index-nav/src/vault.rs file.

pub fn save_allocation(
    &self,
    storage: &mut dyn Storage,
    allocation: AssetAllocation<OracleConfig>,
) -> Result<(), ContractError> {
    ensure!(
        allocation.swap_contract.is_some() || allocation.denom == self.quote_denom,
        ContractError::InvalidSwapContract
    );
    if let Some(ref swap_addr) = allocation.swap_contract {
        self.api.addr_validate(swap_addr)?;
        let cfg: fin::ConfigResponse = self
            .querier
            .query_wasm_smart(swap_addr, &fin::QueryMsg::Config {})?;
        ensure!(
            cfg.denoms.quote() == self.quote_denom || cfg.denoms.base() == self.quote_denom,
            ContractError::InvalidQuoteDenom
        );
        allocation.oracle.price(*self.querier)?;
    }
    ensure!(
        allocation.slippage.gt(&Decimal::zero()) && allocation.slippage.lt(&Decimal::one()),
        ContractError::SlippageOne
    );
    ALLOCATIONS.save(storage, &allocation.denom, &allocation)?;
    Ok(())
}

It is recommended to retrieve the base_denom value directly from the swap contract’s configuration at the time of registration and validate that it matches the key used in the map. If they differ, the contract should reject the operation to ensure consistent and predictable routing logic.

SOLVED: The Nami team resolved this issue by validating both the denom and the quote before storing the swapping contract.

In the nami-index-fixed contract, the add_allocation function uses ALLOCATIONS.save(...) to insert a new asset allocation entry. However, it does not check if an entry with the same denom already exists in the map. As a result, calling add_allocation with a duplicate denom silently overwrites the previous configuration.

This behavior can lead to accidental or unauthorized modification of index compositions without any indication or rejection. Overwriting existing entries may also interfere with asset tracking, weight management, or rebalancing logic, potentially resulting in misconfigured indices or inconsistent user experience.

Code Location

Code of add_allocation function from contracts/nami-index-fixed/src/vault.rs file.

pub fn add_allocation(
    &self,
    storage: &mut dyn Storage,
    quote_denom: &str,
    denom: &str,
    weight: Uint128,
    contract: &str,
) -> Result<(), ContractError> {
    let contract = self.api.addr_validate(contract)?;
    let config: fin::ConfigResponse = self
        .querier
        .query_wasm_smart(&contract, &fin::QueryMsg::Config {})?;
    ensure!(
        config.denoms.quote() == quote_denom || config.denoms.base() == quote_denom,
        ContractError::InvalidQuoteDenom
    );
    ALLOCATIONS.save(storage, denom, &(weight, contract))?;
    Ok(())
}

It is recommended to call ALLOCATIONS.has(denom) before saving any new allocation. If the denom already exists in the map, the contract should reject the operation to prevent unintended overwrites and preserve index integrity.

SOLVED: The Nami team resolved this issue by adding an ALLOCATIONS.has() check before inserting a new allocation.

In the nami-index-fixed contract, the Vault::reallocate function performs a two-step swap process: first, it converts the source asset (from) into quote_denom, and then swaps quote_denom into the target asset (to). However, the same min_return value is applied across both steps.

This creates a problem because each swap has its own pricing, liquidity, and slippage characteristics. If the first swap fails to meet the threshold, the operation reverts, even if the full two-step swap would have produced an acceptable result overall.

Code Location

Code of reallocate function from contracts/nami-index-fixed/src/vault.rs file.

pub fn reallocate(
    storage: &mut dyn Storage,
    from: &str,
    to: &str,
    weight: Uint128,
    total_shares: Uint128,
    min_return: Option<Uint128>,
) -> Result<CosmosMsg, ContractError> {
    let (curr_weight, swap_from) = ALLOCATIONS.load(storage, from)?;
    let (_, swap_to) = ALLOCATIONS.load(storage, to)?;

    ensure!(curr_weight.gt(&weight), ContractError::InvalidWeight);

    let amount_to_swap = curr_weight.checked_sub(weight)?.checked_mul(total_shares)?;

    let callback = to_json_binary(&CallbackType::AfterReallocate {
        swap_to,
        min_return,
    })?;

    Ok(WasmMsg::Execute {
        contract_addr: swap_from.to_string(),
        msg: to_json_binary(&fin::ExecuteMsg::Swap(SwapRequest {
            min_return,
            to: None,
            callback: Some(callback.into()),
        }))?,
        funds: vec![Coin::new(amount_to_swap, from)],
    }
    .into())
}

It is recommended to use separate min_return values for each swap step—one for from → quote_denom and another for quote_denom → to. Alternatively, compute the final output after both swaps and compare it against a single min_return threshold that reflects the total expected result.

SOLVED: The Nami team resolved this issue by removing the min_amount constraint from the first swap.

In the nami-index-nav contract, the rebalancing logic is only triggered manually via the ExecuteMsg::Run{} message. There is no mechanism to automatically enforce rebalancing after relevant events, such as deposits, withdrawals, or allocation updates.

As a result, the portfolio may remain unbalanced for extended periods, especially if Run is not called regularly. This can cause the vault composition to drift away from the target weights, leading to inaccurate NAV calculations and possible arbitrage opportunities for advanced users.

Since the core purpose of the index is to maintain proportional exposure across assets, the absence of periodic or event-based rebalancing undermines the reliability and predictability of the system.

It is recommended to enforce rebalancing under key conditions —such as after deposits or withdrawals— or to implement a time-based trigger (e.g., after a threshold duration has passed). This ensures that the index remains aligned with its target allocation and prevents long-term divergence in asset distribution.

RISK ACCEPTED: The Nami team accepted the risk associated with this finding and provided the following statement:

We acknowledge the risk of long-term drift without automatic rebalancing. However, triggering Run{} on every deposit or withdrawal could block users during low-liquidity scenarios where Run{} might fail.

To minimize friction, we’ve decided to keep Run{} as a permissioned but publicly callable entry point and rely on off-chain keepers to execute it on a regular schedule. This approach allows the vault to continue accepting deposits and withdrawals without interruption while still gradually converging toward the target allocations.

In the nami-index-nav contract, the sum of allocation weights is enforced to equal exactly 1.0, but only during the initial contract setup. Subsequent updates—such as adding or removing allocations—are performed one at a time via functions like save_allocation and remove_allocation, without revalidating the total weight.

This makes it impossible to enforce the global invariant that allocation weights must always sum to 1. For example, removing one allocation and adding another in separate transactions may temporarily—or permanently—leave the total below or above 1.0. This breaks the core assumptions behind NAV calculations and rebalancing logic, leading to inaccurate share pricing, misaligned portfolio proportions, and potential economic inconsistencies.

It is recommended to refactor the allocation update process to support batched operations, such as submitting a full set of new allocations and removals in a single transaction. This would allow verifying that the total weight remains exactly 1.0 at the end of the update.

SOLVED: The Nami team resolved this issue by refactoring the allocation management logic, enabling the entire batch of allocations to be added or removed through a single function, and verifying that the sum of all weights is equal to 1.

The Nami protocol codebase lacks sufficient inline comments and Rust documentation for public functions, modules, and critical logic. Although the contracts implement complex mechanisms correctly, such as fee computation, NAV calculation, rebalancing, and cross-contract swap orchestration, there is little to no documentation explaining how these parts work or why they are designed this way.

This absence of documentation significantly affects readability, auditability, and long-term maintainability. It makes it harder for external contributors, auditors, and even future maintainers to understand the intended behavior, invariants, and design rationale behind the implementation. It also increases the risk of misinterpretation, regressions, or incorrect modifications during future upgrades.

It is recommended to add Rust documentation comments (///) to all public functions, data structures, and modules. Additionally, inline comments should be used to clarify complex or non-obvious logic, particularly in sections involving fee logic, allocation updates, swap flows, and state transitions. A documented codebase will improve comprehension, reduce onboarding time, and support safer upgrades and audits.

ACKNOWLEDGED: The Nami team acknowledged this finding.

In the nami-index-entry-adapter contract, the deposit logic performs a series of swaps from quote_denom to the target allocation tokens without first simulating how much of each is actually needed to meet the index weights. As a result, it may swap more tokens than necessary, leaving excess amounts unallocated.

These unused tokens are then typically swapped back to quote_denom, adding unnecessary complexity and gas costs to the transaction. This inefficiency becomes more pronounced when the number of target assets is high or when the deposit is unbalanced.

The lack of pre-swap simulation undermines the precision of the deposit flow, wastes resources, and may introduce slippage or rounding errors in the reverse swaps.

It is recommended to simulate each swap beforehand using the Simulate query interface provided by the target swap (FIN) contracts. This allows determining the exact amount needed per token to match the index proportions. Only the required swaps should then be executed, reducing overhead and avoiding the need to reverse unused tokens.

ACKNOWLEDGED: The Nami team has acknowledged this finding and provided the following statement:

We acknowledge the inefficiency risk. However, performing on-chain pre-swap simulations would require the contract to fetch and interpret prices for every asset and execute multiple dry-run loops, introducing higher gas costs than simply reversing excess swaps.

Instead, we’ll rely on the front end to calculate the exact deposit proportions off-chain and pass those values directly to the contract.

In the current withdrawal logic of both nami-index-fixed and nami-index-nav contracts, the contract burns the full number of shares sent by the user (e.g., 100) and then mints the fee portion (e.g., 2 shares) to the fee_collector. Although functionally correct, this implementation introduces unnecessary complexity.

The minting of fee shares gives the impression that new value is being created, when in reality it is simply a redirection of part of the user’s existing shares. This may confuse external observers, complicate accounting, and increase the risk of errors during audits or integrations.

Moreover, using mint unnecessarily increases the cognitive overhead of the fee system. The same result can be achieved more clearly and efficiently by burning only the net shares (e.g., 98) and transferring the remaining fee portion (e.g., 2) directly to the fee_collector.

It is recommended to simplify the fee application logic by using a transfer to move the fee portion of shares to the fee_collector, and then burning only the remaining user shares. This approach better reflects the fact that the fee is not newly created value but a reassignment of ownership, and improves the clarity of the system.

SOLVED: The Nami team resolved this issue by transferring the fees instead of minting them.

Both the nami-index-fixed and nami-index-nav contracts generate receipt tokens with denoms following the pattern nami-index-{contract-address}-rcpt. While the contract address ensures uniqueness on-chain, the format does not indicate the index type (fixed vs. nav), making it difficult for users, integrators, and block explorers to distinguish between them.

This ambiguity can lead to confusion in interfaces that rely on denom patterns, errors in data aggregation or reporting tools, and difficulties in tracing the origin or behavior of receipt tokens. It also complicates off-chain integrations where knowledge of the index logic (e.g., fixed-weighted vs. nav-based) is relevant for display or accounting purposes.

It is recommended to include the index type in the receipt token denom, for example, using nami-fixed-{address}-rcpt and nami-nav-{address}-rcpt. This would improve clarity, reduce integration risks, and make receipt tokens more self-descriptive in logs, UIs, and analysis tools.

SOLVED: The Nami team resolved this issue by including the index type in the denom of the receipt token.

In the nami-index-nav contract, the slippage parameter is interpreted inconsistently across different functions. In the withdraw flow, slippage is used as a percentage of the expected return, meaning that a value of 0.97 implies that the user expects to receive at least 97% of the calculated amount.

In contrast, the rebalance_msg function interprets slippage as a tolerated loss, subtracting it from 1 to compute the minimum return —e.g., a value of 0.03 means that 3% slippage is allowed, and the user should receive at least 97%.

This discrepancy in how slippage is handled across the codebase can confuse developers and integrators, lead to misconfigured transactions.

It is recommended to standardize the slippage semantics across all functions by adopting a unified interpretation. A consistent model, such as treating slippage as the maximum tolerated loss (i.e., min_return = value * (1 - slippage)), should be applied to both withdraw and rebalance flows to ensure predictable and reliable behaviour.

SOLVED: The Nami team resolved this issue by modifying the slippage calculation in the withdraw function to use 1 - slippage.