Skip to content

Bug: HSM: execute_arbitrage Profit Measurement Always Returns Zero #1461

Description

@Trevordiex

Description

In execute_arbitrage (pallets/hsm/src/lib.rs:914–930), the pre-trade balance snapshot of ArbitrageProfitReceiver is taken after the EVM flash loan call has already returned, not before. The flash loan callback (execute_arbitrage_with_flash_loan) transfers the arbitrage profit to ArbitrageProfitReceiver synchronously before returning, so by the time the "initial" snapshot is read, the balance already reflects the full settled profit. The "final" snapshot is read immediately after with no intervening state changes, making final - initial = 0 on every execution. The profit field emitted in ArbitrageExecuted is therefore always 0, permanently breaking on-chain profit accounting for the HSM.

Expected Behavior

ArbitrageExecuted.profit should equal the collateral balance gained by ArbitrageProfitReceiver during the arbitrage. For the reference pool configuration (999 000 HOLLAR / 1 000 000 DAI, amp 22, zero fees), the expected profit is 10_875_005_266_593_893 DAI per arbitrage cycle.

Actual Behavior

ArbitrageExecuted.profit is always 0 regardless of how much collateral was earned. The receiver account holds the correct balance (transfers succeed), but the event field is computed as post-trade balance − post-trade balance = 0.

Step Action ArbitrageProfitReceiver DAI balance
Before execute_arbitrage initial state 0
Inside EVM callback profit transferred 10_875_005_266_593_893
Line 916: receiver_balance_initial read already settled 10_875_005_266_593_893
Line 924: receiver_balance_final read no change 10_875_005_266_593_893
Line 928–930: profit computed final − initial 0
Event emitted ArbitrageExecuted { profit: 0 }

The existing test suite does not catch this because pallets/hsm/src/tests/arb.rs:126–127 checks the raw receiver balance (correct) but never asserts on the event's profit field.

Possible Fix

Move receiver_balance_initial to before the T::Evm::call(...) line:

// Snapshot BEFORE the flash loan executes
let receiver_balance_initial = <T as crate::pallet::Config>::Currency::total_balance(
    collateral_asset_id,
    &T::ArbitrageProfitReceiver::get(),
);

let call_result = T::Evm::call(context, data, U256::zero(), T::GasLimit::get());

if call_result.exit_reason != ExitReason::Succeed(ExitSucceed::Returned) {
    log::error!(target: "hsm", "Flash loan Hollar EVM execution failed ...");
    return Err(T::EvmErrorDecoder::convert(call_result));
}

// Snapshot AFTER — delta now reflects actual profit
let receiver_balance_final = <T as crate::pallet::Config>::Currency::total_balance(
    collateral_asset_id,
    &T::ArbitrageProfitReceiver::get(),
);

let profit = receiver_balance_final
    .checked_sub(receiver_balance_initial)
    .ok_or(Error::<T>::NoArbitrageOpportunity)?;

No other changes are required. The test at pallets/hsm/src/tests/poc_profit_snapshot.rs passes once this reordering is applied.

Steps to Reproduce

  1. Clone the repo and check out the affected commit.

  2. Run the PoC test (pallets/hsm/src/tests/poc_profit_snapshot.rs), which is already registered in pallets/hsm/src/tests/mod.rs:

cargo test -p pallet-hsm --locked poc_profit_snapshot
  1. Observe the failure:
thread 'tests::poc_profit_snapshot::execute_arbitrage_should_emit_correct_profit_in_event'
panicked at 'ArbitrageExecuted.profit (0) does not match actual receiver
balance (10875005266593893); snapshot taken after EVM call'

The PoC calls HSM::execute_arbitrage against a pool with a known imbalance, then compares the profit field in the emitted ArbitrageExecuted event against the actual ArbitrageProfitReceiver balance. The receiver holds 10_875_005_266_593_893 DAI; the event reports 0.

Abbreviated test source (pallets/hsm/src/tests/poc_profit_snapshot.rs):

#[test]
fn execute_arbitrage_should_emit_correct_profit_in_event() {
    // pool: 999_000 HOLLAR / 1_000_000 DAI, amp=22, zero fees
    // ... ExtBuilder setup ...
    assert_ok!(HSM::execute_arbitrage(RuntimeOrigin::none(), DAI, opportunity));

    let real_profit = Tokens::free_balance(DAI, &HsmArbProfitReceiver::get());
    assert!(real_profit > 0); // 10_875_005_266_593_893

    let emitted_profit = System::events()
        .into_iter()
        .filter_map(|record| {
            if let RuntimeEvent::HSM(crate::Event::ArbitrageExecuted { profit, .. }) = record.event {
                Some(profit)
            } else {
                None
            }
        })
        .last()
        .expect("ArbitrageExecuted event must have been emitted");

    // FAILS: emitted_profit=0, real_profit=10_875_005_266_593_893
    assert_eq!(
        emitted_profit, real_profit,
        "ArbitrageExecuted.profit ({emitted_profit}) does not match \
         actual receiver balance ({real_profit}); snapshot taken after EVM call"
    );
}

Context

Found during a security audit of the HSM pallet. ArbitrageExecuted.profit is the only on-chain signal for HSM revenue from arbitrage. Off-chain indexers, dashboards, and governance tooling that consume this event will record zero protocol revenue for every arbitrage cycle ever executed. Governance decisions about buyback_rate, max_buy_price_coefficient, and buy_back_fee that rely on observed HSM profitability would be operating on fabricated data. No funds are lost — the profit is transferred correctly — but protocol transparency and parameter governance are fully blind to actual performance.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions