Skip to content

Add onchain gas cost to trades and orders APIs#4540

Open
jmg-duarte wants to merge 2 commits into
mainfrom
jmgd/order-trade-gas-cost
Open

Add onchain gas cost to trades and orders APIs#4540
jmg-duarte wants to merge 2 commits into
mainfrom
jmgd/order-trade-gas-cost

Conversation

@jmg-duarte

Copy link
Copy Markdown
Contributor

Description

The removal of UCP means that we no longer have fees and gas costs info in a single place, meaning the FE must calculate them; the issue comes when one notices that we don't keep track of gas costs for settlements (meaning trades and orders don't either).

This PR adds gas cost tracking, when settlements get indexed, they will now be indexed with the gas cost.
The PR does not do backfills on purpose, that must be done as a separate process.

Changes

  • DB migrations
  • OpenAPI updates
  • DB queries for trades and orders to include gas costs
  • Update indexing to include gas costs

How to test

Sepolia staging is currently suspended with this code there, please do a trade and review the UI using this link
https://jmgd-ucp.explorer-dev-dxz.pages.dev/

Under costs and fees you should see this:
Screenshot 2026-06-19 at 15 10 58

jmg-duarte and others added 2 commits June 18, 2026 11:13
Persist the actual settlement gas (gas_used, effective_gas_price) read from
the transaction receipt — previously computed by the autopilot but only logged
since the settlement_observations table was dropped in V090 — and expose a
per-trade `gasCost` (native token wei).

Attribution: each trade gets its share of the settlement transaction's gas
cost (gas_used * effective_gas_price), split equally across all trades settled
in the same transaction.

- V111 migration: nullable gas_used + effective_gas_price on settlements
- autopilot save_settlement writes them (going-forward only, no backfill)
- trades query computes per-trade gas_cost in SQL
- model + openapi documentation for the new gasCost field on trades

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Expose a per-order `gasCost` (native token wei): the sum, across the order's
fills, of each trade's share of its settlement transaction's gas cost
(gas_used * effective_gas_price), split equally across all trades settled in
the same transaction. Builds on the settlement gas now persisted for trades.

- order_gas_costs aggregates the per-trade gas attribution per order_uid
- orders wired on the single_order / many_orders read paths
- model + openapi documentation for the new gasCost field on orders

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jmg-duarte jmg-duarte requested a review from a team as a code owner June 19, 2026 14:11
@github-actions

Copy link
Copy Markdown

Reminder: Please update the DB Readme and comment whether migrations are reversible (include rollback scripts if applicable).

  • If creating new tables, update the tables list.
  • When adding a new index, consider using CREATE INDEX CONCURRENTLY for tables involved in the critical execution path.
  • For breaking changes, remember that during rollout k8s starts the new autopilot, runs the Flyway migration, and only then shuts down the old pod. That overlap means the previous version can still be processing requests on the migrated schema, so make it compatible first and ship the breaking DB change in the following release.

Caused by:

@github-actions

Copy link
Copy Markdown

Reminder: Please consider backward compatibility when modifying the API specification.
If breaking changes are unavoidable, ensure:

  • You explicitly pointed out breaking changes.
  • You communicate the changes to affected teams (at least Frontend team and SAFE team).
  • You provide proper versioning and migration mechanisms.

Caused by:

@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Claude finished @jmg-duarte's task in 4m 13s —— View job


PR Review: Add on-chain gas cost to trades and orders APIs

I reviewed the diff against origin/main. The overall approach is solid — sharing the GAS_COST_EXPR between trades and order_gas_costs so they can't drift, and moving the settlement/gas join onto the paginated page CTE (instead of every UNION branch) is a nice performance win. I verified the autopilot stores genuine receipt values (receipt.gas_used, receipt.effective_gas_price), so the "actual on-chain" framing in the OpenAPI is accurate.

I left 3 inline comments:

  1. crates/model/src/order.rs:715 — incorrect doc comment. Says the value is "derived from its quote (gas_amount * gas_price)", but it's actually the on-chain settlement gas cost. The None condition is wrong too.
  2. crates/model/src/trade.rs:34 — same incorrect doc comment, plus the "(e.g. JIT orders)" note is misleading (JIT orders do get a gas cost).
  3. database/sql/V111 — deploy-ordering caveat. The orderbook queries reference the new columns unconditionally; confirm V111 is applied before the new orderbook serves traffic.

The doc-comment mismatches (1 & 2) read like leftovers from an earlier quote-based design — worth fixing since they directly contradict the OpenAPI and the implementation.

Minor (non-blocking)

  • gas_cost uses skip_serializing_if = "Option::is_none", so the field is omitted when absent, while the OpenAPI marks it nullable: true (implying an explicit null). Consistent with other optional fields here, so likely fine — just flagging the spec/impl nuance for the FE.
  • DB tests for the rewritten trades query (cargo nextest run postgres ...) aren't runnable in this environment — worth confirming they pass, since the query restructure is substantial.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements tracking and persistence of actual on-chain gas costs for settlements, trades, and orders, including database migrations, query updates, and OpenAPI documentation. The reviewer feedback highlights that the docstrings for the gas_cost fields in both the OrderMetadata and Trade models are incorrect and misleading, as they describe the value as an estimated cost derived from quotes rather than the actual on-chain gas cost.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/model/src/order.rs
Comment on lines +715 to +720
/// Estimated network gas cost (in native token wei) of executing this
/// order, derived from its quote (`gas_amount * gas_price`). `None` when
/// the order has no stored quote.
#[serde_as(as = "Option<HexOrDecimalU256>")]
#[serde(default, skip_serializing_if = "Option::is_none")]
pub gas_cost: Option<U256>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The docstring for gas_cost is incorrect and misleading. It states that the gas cost is an "Estimated network gas cost... derived from its quote", but the PR actually implements and exposes the actual on-chain gas cost attributed to the order (as documented in the OpenAPI spec and implemented in the database queries). Please update the docstring to accurately reflect that this is the actual on-chain gas cost.

Suggested change
/// Estimated network gas cost (in native token wei) of executing this
/// order, derived from its quote (`gas_amount * gas_price`). `None` when
/// the order has no stored quote.
#[serde_as(as = "Option<HexOrDecimalU256>")]
#[serde(default, skip_serializing_if = "Option::is_none")]
pub gas_cost: Option<U256>,
/// Actual on-chain gas cost (in native token wei) attributed to this
/// order. Computed as the order's share of the gas cost
/// (gas_used * effective_gas_price) of the settlement transaction(s)
/// that executed it, split equally across all trades settled in the same
/// transaction, and summed across the order's fills. None for orders
/// settled before this data started being recorded, or not yet settled.
#[serde_as(as = "Option<HexOrDecimalU256>")]
#[serde(default, skip_serializing_if = "Option::is_none")]
pub gas_cost: Option<U256>,

Comment thread crates/model/src/trade.rs
Comment on lines +34 to +39
/// Estimated network gas cost (in native token wei) of executing this
/// trade, derived from the order's quote (`gas_amount * gas_price`).
/// `None` when the order has no stored quote (e.g. JIT orders).
#[serde_as(as = "Option<HexOrDecimalU256>")]
#[serde(default, skip_serializing_if = "Option::is_none")]
pub gas_cost: Option<U256>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The docstring for gas_cost is incorrect and misleading. It states that the gas cost is an "Estimated network gas cost... derived from the order's quote", but the PR actually implements and exposes the actual on-chain gas cost attributed to the trade. Please update the docstring to accurately reflect that this is the actual on-chain gas cost.

    /// Actual on-chain gas cost (in native token wei) attributed to this
    /// trade. Computed as this trade's share of the settlement transaction's
    /// gas cost (gas_used * effective_gas_price), split equally across all
    /// trades settled in the same transaction. None for trades settled
    /// before this data started being recorded.
    #[serde_as(as = "Option<HexOrDecimalU256>")]
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub gas_cost: Option<U256>,

Comment thread crates/model/src/order.rs
Comment on lines +715 to +717
/// Estimated network gas cost (in native token wei) of executing this
/// order, derived from its quote (`gas_amount * gas_price`). `None` when
/// the order has no stored quote.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc comment is incorrect and contradicts both the implementation and the OpenAPI spec. The value populated into gas_cost comes from database::trades::order_gas_costs, i.e. the actual on-chain settlement gas cost (gas_used * effective_gas_price from the receipt, split across trades) — not the quote (gas_amount * gas_price). The None condition is also wrong: it's None for orders settled before V111 or not yet settled, not "the order has no stored quote".

Suggested change
/// Estimated network gas cost (in native token wei) of executing this
/// order, derived from its quote (`gas_amount * gas_price`). `None` when
/// the order has no stored quote.
/// Actual on-chain gas cost (in native token wei) attributed to this order:
/// its share of the settlement transaction(s)' gas cost
/// (`gas_used * effective_gas_price`), split equally across all trades in
/// each settlement and summed across the order's fills. `None` for orders
/// settled before this data was recorded (see V111), or not yet settled.

Comment thread crates/model/src/trade.rs
Comment on lines +34 to +36
/// Estimated network gas cost (in native token wei) of executing this
/// trade, derived from the order's quote (`gas_amount * gas_price`).
/// `None` when the order has no stored quote (e.g. JIT orders).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as the OrderMetadata::gas_cost comment: this is the actual on-chain gas cost (this trade's share of the settlement's gas_used * effective_gas_price), not a quote-derived estimate. The "None ... (e.g. JIT orders)" claim is also misleading — JIT orders have trades and settlements, so they do get a gas cost here; the value is None only for trades settled before V111.

Suggested change
/// Estimated network gas cost (in native token wei) of executing this
/// trade, derived from the order's quote (`gas_amount * gas_price`).
/// `None` when the order has no stored quote (e.g. JIT orders).
/// Actual on-chain gas cost (in native token wei) attributed to this trade:
/// its share of the settlement transaction's gas cost
/// (`gas_used * effective_gas_price`), split equally across all trades
/// settled in the same transaction. `None` for trades settled before this
/// data was recorded (see V111).

Comment on lines +11 to +13
ALTER TABLE settlements
ADD COLUMN gas_used numeric(78, 0),
ADD COLUMN effective_gas_price numeric(78, 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy-ordering caveat worth confirming: the orderbook trades / order_gas_costs queries now reference s.gas_used and s.effective_gas_price unconditionally. If a new orderbook pod starts serving before this migration has been applied (the migration is run by autopilot's Flyway, a separate deployment), every trades/orders query will fail with column "gas_used" does not exist. Please make sure V111 is applied before (or atomically with) the new orderbook rollout — see the bot's backward-compatibility reminder above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant