Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat(observer): Detect artificially noisy stalled publisher prices #81

Merged
merged 7 commits into from
Nov 15, 2024

Conversation

tejasbadadare
Copy link
Contributor

@tejasbadadare tejasbadadare commented Nov 13, 2024

Purpose

  • The PublisherStalledCheck currently detects stalled prices by checking if the updates exactly match over some time frame. However, publishers are evading this by adding small amounts of random noise to their prices (see screenshot below). This PR addresses this by improving the check to detect both exact and noisy price stalls.
  • Redemption rate feeds were excluded from this check since we expect them to stay static for long periods.
  • This check has been disabled for ~6mo because the parameters needed some tuning. We'll enable it with new parameters in the next PR.
  • Updated README to capture some setup gotchas I ran into

Implementation Details

Most of the new stuff is in stall_detection.py. From the docstring:

The detection strategy is based on the intuition that meaningful price movements must exceed some minimum relative threshold. If a price very slightly fluctuates but doesn't exceed the noise_threshold within stall_time_limit, then it's likely that it's just a static price with artificial noise thrown in.

Detection Logic:

  1. Exact Stalls: Prices within stall_time_limit are exactly equal
  2. Noisy Stalls: All price variations stay within a tiny relative noise_threshold (default 0.01% - we may need to tune this)
    for longer than stall_time_limit (and with more updates than min_noise_samples.)

The noise_threshold (default 1e-4 or 0.01%) strategy is chosen because:

  • Real price movements, even for very stable symbols, should exceed this threshold.
  • Hard to circumvent. Avoiding detection would require larger variations, impacting the publisher's
    price accuracy versus the aggregate.
  • The threshold is relative to the base price, making it work across different price scales.
  • It works across different noise patterns (random, sine wave, structured, etc.)

Example:

  • A $100 base price with all variations within ±$0.01 (0.01%) for 2+ minutes is likely stalled
  • Natural price movements would occasionally exceed this tiny threshold
  • Variations this small consistently over time suggest artificial noise

Testing

The test suite has been updated to capture both exact and noisy stalls, normal price movement, and redemption rate feeds. The test file was restructured to enable reuse of setup functions via fixtures.


Ticket: https://www.notion.so/pyth-network/Observer-alert-for-almost-stalled-publishers-e0a12998ac08414c96b6414582280684?pvs=4
We're trying to detect patterns like this:
telegram-cloud-photo-size-4-6046484702413570906-y

@tejasbadadare tejasbadadare changed the title feat(observer): detect artificially noisy stalled publisher prices Feat(observer): Detect artificially noisy stalled publisher prices Nov 13, 2024
Comment on lines +51 to +52
response_json = await response.json()
price_feeds.extend(response_json["parsed"])
Copy link
Contributor Author

@tejasbadadare tejasbadadare Nov 13, 2024

Choose a reason for hiding this comment

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

Not sure how this is working currently in prod, but this wouldn't run locally for me without this change because the json response structure wasn't as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cctdaniel any idea on this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm i can see that the query_string is changed in this PR https://github.com/pyth-network/pyth-observer/pull/79/files#diff-858c896a341166e34351e654992a17790df86095bb42d1b262b9673d43e2ea98R47 it seems like it wasnt tested properly

Copy link
Contributor

@cctdaniel cctdaniel Nov 14, 2024

Choose a reason for hiding this comment

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

prod is still using v0.2.14 which is why its working but observer-pythtest is using v0.2.15 which i assume is probably broken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, thanks -- I'll leave this change in then, and verify it works in testnet before deploying to mainnet

Copy link
Contributor

@ali-bahjati ali-bahjati left a comment

Choose a reason for hiding this comment

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

Very nice! thank you!!


publisher_key = (self.__state.publisher_name, self.__state.symbol)
PUBLISHER_CACHE[publisher_key].append(
PriceUpdate(current_time, self.__state.price)
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the logic in the StalledDetector that you have I think you need to append a price only it's different that the latest stored price, otherwise the exact stalled might never fire.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah you're right, i was considering a few different ways to do this and looks like this slipped between the cracks during refactoring. test suite should have caught this -- let me fix and cover it with a test case

@tejasbadadare tejasbadadare merged commit c90f52a into main Nov 15, 2024
2 checks passed
@tejasbadadare tejasbadadare deleted the tb/observer/improve_publisher_stalled_check branch November 15, 2024 17:43
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.

4 participants