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: add Price Oracles support #2688

Merged
merged 45 commits into from
May 23, 2024
Merged

feat: add Price Oracles support #2688

merged 45 commits into from
May 23, 2024

Conversation

khancode
Copy link
Collaborator

@khancode khancode commented May 3, 2024

High Level Overview of Change

Add Price Oracles support - XRPLF/XRPL-Standards#129

Context of Change

Adds transactions: OracleSet, OracleDelete

Adds request: get_aggregate_price

Adds ledger entry: Oracle

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Did you update HISTORY.md?

  • Yes
  • No, this change does not impact library users

Test Plan

Adds unit/integration tests for Price Oracle transactions and request.

@khancode khancode changed the title feature: add Price Oracle support feat: add Price Oracle support May 3, 2024
@khancode khancode changed the title feat: add Price Oracle support feat: add Price Oracles support May 6, 2024
@khancode khancode requested a review from mvadari May 17, 2024 19:06
@khancode khancode requested a review from mvadari May 20, 2024 21:54
Copy link
Collaborator

@mvadari mvadari left a comment

Choose a reason for hiding this comment

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

Given the complexity of creating a price data object (with AssetPrice/Scale used to avoid floats), do you think it'd be useful to have a helper function to convert a float to AssetPrice/Scale (and vice versa)?

Copy link
Collaborator

@mvadari mvadari left a comment

Choose a reason for hiding this comment

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

It looks like the ledger_entry filter isn't updated for oracle here.

@khancode
Copy link
Collaborator Author

khancode commented May 22, 2024

It looks like the ledger_entry filter isn't updated for oracle here.

But I included Oracle in the ledger entry filter. What's missing?

@khancode
Copy link
Collaborator Author

Given the complexity of creating a price data object (with AssetPrice/Scale used to avoid floats), do you think it'd be useful to have a helper function to convert a float to AssetPrice/Scale (and vice versa)?

I don't think it's necessary given the docs will explain this.

@khancode khancode requested a review from mvadari May 22, 2024 13:52
@khancode
Copy link
Collaborator Author

@mvadari I'm blocked on figuring out why the browser test fails. Any ideas?

@mvadari
Copy link
Collaborator

mvadari commented May 22, 2024

It looks like the ledger_entry filter isn't updated for oracle here.

But I included Oracle in the ledger entry filter. What's missing?

Missed that, sorry.

@mvadari
Copy link
Collaborator

mvadari commented May 22, 2024

Given the complexity of creating a price data object (with AssetPrice/Scale used to avoid floats), do you think it'd be useful to have a helper function to convert a float to AssetPrice/Scale (and vice versa)?

I don't think it's necessary given the docs will explain this.

I agree that the docs will explain this, but could be nice to have a helper function do the math for you. Not super important though.

Copy link
Collaborator

@pdp2121 pdp2121 left a comment

Choose a reason for hiding this comment

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

LGTM

@khancode khancode requested a review from mvadari May 23, 2024 15:49
@khancode khancode merged commit 23adb49 into main May 23, 2024
15 checks passed
@khancode khancode deleted the price-oracle branch May 23, 2024 16:10
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.

None yet

3 participants