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.

Sign up for GitHub

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

Jump to bottom

feat(target_chains/ethereum): add twap #2543

Merged
merged 30 commits into from
Apr 11, 2025
Merged

feat(target_chains/ethereum): add twap #2543

merged 30 commits into from
Apr 11, 2025

Conversation

cctdaniel
Copy link
Contributor

@cctdaniel cctdaniel commented Apr 2, 2025 •
edited
Loading

Summary

  • Added comprehensive TWAP (Time-Weighted Average Price) functionality to the Pyth contract system including:
    • New data structures: TwapPriceFeed, TwapPriceInfo, TwapPriceFeedMessage
    • New methods for TWAP calculations and updates
    • Optimized fee structure for TWAP updates
    • Added extensive test coverage including negative test cases
  • Added new error types for TWAP-related validation

Rationale

These changes are necessary to:

  1. Support TWAP price feed calculations which provide more stable price data by averaging over time
  2. Improve UX by optimizing fee structure (charging for single update instead of both updates)
  3. Ensure robust error handling for TWAP-related operations
  4. Keep dependencies up to date for security and compatibility
  5. Maintain code quality through comprehensive test coverage

How has this been tested?

  • Current tests cover my changes
  • Added new tests
  • Manually tested the code

Testing steps:

  1. Added comprehensive positive test case for TWAP calculation verifying:

    • Basic properties (ID, timestamps, exponents)
    • Price calculations
    • Confidence calculations
    • Down slots ratio

  2. Added extensive negative test cases covering:

    • Invalid update data length
    • Mismatched price IDs
    • Invalid time ordering
    • Mismatched exponents
    • Invalid previous publish times
    • Insufficient fees

  3. Manually verified:

    • TWAP calculation accuracy
    • Fee optimization changes
    • Error handling
    • Event emissions
    • Integration with existing price feed functionality

@vercel Vercel
Copy link

vercel bot commented Apr 2, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api-reference ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 10, 2025 2:14pm
5 Skipped Deployments
Name Status Preview Comments Updated (UTC)
component-library ⬜️ Ignored (Inspect) Visit Preview Apr 10, 2025 2:14pm
entropy-debugger ⬜️ Ignored (Inspect) Visit Preview Apr 10, 2025 2:14pm
insights ⬜️ Ignored (Inspect) Visit Preview Apr 10, 2025 2:14pm
proposals ⬜️ Ignored (Inspect) Visit Preview Apr 10, 2025 2:14pm
staking ⬜️ Ignored (Inspect) Visit Preview Apr 10, 2025 2:14pm

@cctdaniel cctdaniel marked this pull request as ready for review April 8, 2025 06:52
@cctdaniel cctdaniel requested a review from a team as a code owner April 8, 2025 06:55
ali-bahjati
ali-bahjati reviewed Apr 8, 2025
View reviewed changes
Copy link
Collaborator

@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. I left some comments that are mostly minor but I like to see them addressed before approving.

@cctdaniel
feat: refactor TWAP price handling by moving TwapPriceInfo struct to …
dac3e31 
…PythStructs and implementing calculateTwap function in PythUtils
ali-bahjati
ali-bahjati approved these changes Apr 10, 2025
View reviewed changes
Copy link
Collaborator

@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.

Nice!

tejasbadadare
tejasbadadare approved these changes Apr 10, 2025
View reviewed changes
Copy link
Contributor

@tejasbadadare tejasbadadare left a comment

Choose a reason for hiding this comment

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

LGTM!

@tejasbadadare tejasbadadare mentioned this pull request Apr 10, 2025
Open
3 tasks
@cctdaniel cctdaniel merged commit 6f0eb47 into main Apr 11, 2025
14 checks passed
@cctdaniel cctdaniel deleted the evm-twap branch April 11, 2025 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tejasbadadare tejasbadadare tejasbadadare approved these changes

@ali-bahjati ali-bahjati ali-bahjati approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cctdaniel @ali-bahjati @tejasbadadare @aditya520