Skip to content

Hyperliquid userFills double-counts position/PnL (snapshot + reconnect replays) #17

Description

@jenish-25

Summary

The Hyperliquid adapter double-counts fills, corrupting strategy position and realized-PnL tracking.

Trade is the framework's only canonical source of position/PnL changes (strategies call InventoryTracker::record_fill from EventHandler<Trade>). The Hyperliquid muxer emits a Trade for every fill in a UserFills message with no guard:

// crates/exchanges/hyperliquid/src/connection.rs
Message::UserFills(f) => {
    for fill in f.data.fills.iter().filter(|f| f.coin == target_coin) {
        if let Some(trade) = convert::fill_to_trade(fill, &client_ids) {
            let _ = trade_tx.send(trade);
        }
    }
}

Hyperliquid's userFills subscription delivers an initial snapshot of recent historical fills (UserFillsData.is_snapshot == Some(true)), then streams live fills. The HL SDK reconnects transparently and re-subscribes, so the snapshot is re-sent on every reconnect. This handler ignores is_snapshot and never dedups by tid.

Impact

  • At startup: strategies seed inventory from REST get_positions() in init() (e.g. avellaneda-stoikov, simple-mm, reference-arb). The userFills snapshot then replays those same historical fills as live Trades on top of the seed → net position roughly doubles, weighted-average entry is wrong, and phantom realized PnL appears.
  • On every reconnect: the snapshot replays recent fills again → further double-counting. Reconnect handling only reconciles open orders (get_open_orders), never re-seeds inventory, so this never self-heals.

For a live bot this produces wrong inventory skew, wrong max_position enforcement, and phantom PnL.

Precedent

The Bullet adapter already guards this exact case. bullet/src/connection.rs keeps a bounded RecentIds set keyed on trade_id and drops replays: "a fill can arrive twice; emitting it twice would double-count the position." events.rs documents Trade.trade_id as existing "for dedup across reconnects." The Hyperliquid adapter never received the same treatment.

Proposed fix

Give the HL fill handler the same canonical-source guarantee, combining two guards (HL needs both because it re-sends an explicit historical snapshot and inventory isn't re-seeded on reconnect):

  1. Record the initial snapshot's tids but don't emit them (already captured by the REST seed).
  2. tid-dedup everything after — drops reconnect replays while still emitting a genuinely new fill that occurred during a disconnect gap.

I have a fix + tests ready and will open a PR referencing this issue. Happy to adjust the approach (e.g. dedup-only, or additionally re-seeding inventory on reconnect) based on your preference.

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