Skip to content

Publish native price estimates to the event bus - #4688

Draft
data-cowwboy wants to merge 1 commit into
mainfrom
nitish/native-price-events
Draft

Publish native price estimates to the event bus#4688
data-cowwboy wants to merge 1 commit into
mainfrom
nitish/native-price-events

Conversation

@data-cowwboy

Copy link
Copy Markdown

Description

Native prices are the one price flow with no representation on the event bus. The quote path publishes priceEstimate and winningPriceEstimate, but native price competitions publish nothing, so analytics has no view of which estimator produced the prices we cache and price orders with, nor of the tokens whose native price estimation fails and why. auction_prices in the DB only keeps the winning per-auction snapshot, without estimator attribution or failures.

The goal is to get these into ClickHouse through the existing NATS JetStream consumer, which needs one flat event per estimator to map onto a table.

Changes

  • Add nativePriceEstimate: one event per estimator taking part in a competition, emitted as soon as that estimator returns, carrying the token, estimator name, stage timeout, elapsed time and either the price or the error. The malformed-price rejection now happens before the event is built, so the event reports the result the competition actually used (behaviour of the competition itself is unchanged).
  • Add winningNativePriceEstimate: names the estimator whose price won and therefore got cached. Emitted only when there is a winner, i.e. not when every estimator errored.
  • Gate both behind a new [native-price-estimation] publish-events flag (default false), read from the shared native price config the price estimator factory already holds, so no call site changes. CompetitionEstimator::with_event_publishing() does the gating.
  • Regenerate crates/event-bus-dto/schemas/events.json (now 6 events).

Publishing is opt-in because it only makes sense for a component that owns a native price cache. In prod the autopilot's cache serves ~390/s hits against ~2/s misses, so its competitions correspond to actual price refreshes — roughly 4-5 events/s across all chains. The orderbook runs with cache.max-age = 1ms and a Forwarder estimator, resolving every request through the autopilot; publishing there would add ~140/s of pass-through lookups that carry no new information and would evict other events from the size-capped per-chain streams. Intended rollout is publish-events = true on autopilot configs only, staging first.

Notes for consumers of the events:

  • Token addresses use alloy's Display impl, i.e. EIP-55 checksummed mixed case, consistent with the existing quote events. Normalize casing when joining.
  • token is the token that was actually priced. For tokens listed in approximation-tokens that is the approximation token, not the one the caller asked for.
  • Prices are f64 JSON numbers (native token per unit of token), always normal and positive — anything is_price_malformed rejects is reported as an error instead.
  • Background cache refreshes have no inbound request, so the envelope carries no requestId; correlate estimate events with the winner event on token plus a small time window. Lookups arriving through the autopilot's API estimator do carry a requestId.

How to test

Unit tests cover the wire format of both events (event-bus-dto), the new config flag including its default, and that enabling publishing does not change the outcome of a competition.

Manually, against the playground or a staging autopilot with [event-bus] configured and publish-events = true under [native-price-estimation]:

  1. nats stream ls to confirm the <network>-events stream exists, then nats sub 'event.<chain_id>.nativePriceEstimate'.
  2. Watch events arrive as the background native price updater refreshes prices; each envelope body should carry the token, estimator and either a price or an error.
  3. nats sub 'event.<chain_id>.winningNativePriceEstimate' should show one event per successful competition, with an estimator name matching one of the estimate events for the same token.
  4. Restart with the flag unset and confirm no events are published on either subject.

Native prices are the one price flow that has no representation on the
event bus: the quote path publishes `priceEstimate` and
`winningPriceEstimate`, but native price competitions publish nothing, so
analytics has no view of which estimator produced the prices we cache and
price orders with, or of the ones that fail.

Adds two events, mirroring the quote pair:

- `nativePriceEstimate`, one per estimator that took part in a
  competition, carrying the token, the estimator, its stage timeout, how
  long it took and the price or the error it returned.
- `winningNativePriceEstimate`, naming the estimator whose price won and
  therefore got cached.

Publishing is opt-in via `[native-price-estimation] publish-events` and
off by default. It is only meaningful for a component that owns a native
price cache: the autopilot's cache absorbs ~99% of lookups (~390/s hits
vs ~2/s misses in prod), so its competitions correspond to actual price
refreshes. The orderbook runs with its cache effectively disabled and
forwards every request to the autopilot, which would put ~140/s of
pass-through lookups on the bus and evict other events from the capped
per-chain stream.
@github-actions

Copy link
Copy Markdown


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

/// since a component that forwards its lookups elsewhere would publish one
/// event per request rather than per price refresh.
#[serde(default)]
pub publish_events: bool,

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.

For now, we should just publish and filter later, so this should be removed

/// Token the price was estimated for (hex-encoded, including the `0x`
/// prefix). For tokens configured to be approximated by another token this
/// is the approximation token, i.e. the one actually priced.
pub token: String,

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.

Addresses are serialized correctly and take less space than the string, replace the type


#[derive(Serialize, JsonSchema)]
#[serde(untagged)]
pub enum NativePriceResult {

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.

I think the regular Result<f64, Error> would work here

Don't forget to assign this to an estimator 🤔

pub struct WinningNativePriceEstimateEvent {
/// Token the price was estimated for (hex-encoded, including the `0x`
/// prefix).
pub token: String,

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.

ditto on the token/address thing

Comment on lines +102 to +104
// truncating the u128 to u64 is safe.
timeout: timeout.as_millis() as u64,
elapsed: elapsed.as_millis() as u64,

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.

this justification needs a bit more explanation why this is safe

also do we actually need to send the timeut?

Comment on lines +184 to +186
if publish_events {
priority = priority.with_event_publishing();
}

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.

this will be removed with the lack of config, but for reference this isn't the right way of doing this, it should be a constructor parameter

/// Publishing is best-effort and a no-op while the event bus is
/// uninitialized, so it must not affect the result of the competition.
#[tokio::test]
async fn publishing_events_does_not_affect_result() {

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.

this test doesnt seem very useful

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.

2 participants