Skip to content

Fix PriceDataSeries order on PriceOracle creation #5485

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

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

tequdev
Copy link
Contributor

@tequdev tequdev commented Jun 10, 2025

High Level Overview of Change

Fixed an issue where the order of PriceDataSeries was out of sync between when PriceOracle was created and when it was updated.

Context of Change

Although they are registered in the canonical order when updated, they are created using the order specified in the transaction, so change so that they are also registered in the canonical order when created.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

API Impact

no

Examples of data with issues

{
   "AffectedNodes" : [
      {
         "ModifiedNode" : {
            "FinalFields" : {
               "Account" : "rwhaYGnJMexktjhxAKzRwoCcQ2g6hvBDWu",
               "Balance" : "999999980",
               "Flags" : 8388608,
               "OwnerCount" : 1,
               "Sequence" : 6
            },
            "LedgerEntryType" : "AccountRoot",
            "LedgerIndex" : "2D6FE275B8B62F3B2C743090152CA05B8A28D5D20BEBED3A4B26745881180462",
            "PreviousFields" : {
               "Balance" : "999999990",
               "Sequence" : 5
            },
            "PreviousTxnID" : "18F8010465F8ABBE5A6DDF05E2AC5E9F28F7BA2B574FD1112B7B57C188E79F87",
            "PreviousTxnLgrSeq" : 4
         }
      },
      {
         "ModifiedNode" : {
            "FinalFields" : {
               "AssetClass" : "63757272656E6379",
               "Flags" : 0,
               "LastUpdateTime" : 946694820,
               "Owner" : "rwhaYGnJMexktjhxAKzRwoCcQ2g6hvBDWu",
               "OwnerNode" : "0",
               "PriceDataSeries" : [
                  {
                     "PriceData" : {
                        "AssetPrice" : "2c7",
                        "BaseAsset" : "XRP",
                        "QuoteAsset" : "EUR",
                        "Scale" : 2
                     }
                  },
                  {
                     "PriceData" : {
                        "AssetPrice" : "2e6",
                        "BaseAsset" : "XRP",
                        "QuoteAsset" : "USD",
                        "Scale" : 2
                     }
                  }
               ],
               "Provider" : "70726F7669646572",
               "URI" : "555249"
            },
            "LedgerEntryType" : "Oracle",
            "LedgerIndex" : "72CD5C8AE42E0A993C1234E95B0F8A3BC7CADBD7097ACB23CC7B6DBFB06AA101",
            "PreviousFields" : {
               "LastUpdateTime" : 946694810,
               "PriceDataSeries" : [
                  {
                     "PriceData" : {
                        "AssetPrice" : "2e6",
                        "BaseAsset" : "XRP",
                        "QuoteAsset" : "USD",
                        "Scale" : 2
                     }
                  },
                  {
                     "PriceData" : {
                        "AssetPrice" : "2c7",
                        "BaseAsset" : "XRP",
                        "QuoteAsset" : "EUR",
                        "Scale" : 2
                     }
                  }
               ]
            },
            "PreviousTxnID" : "18F8010465F8ABBE5A6DDF05E2AC5E9F28F7BA2B574FD1112B7B57C188E79F87",
            "PreviousTxnLgrSeq" : 4
         }
      }
   ],
   "TransactionIndex" : 0,
   "TransactionResult" : "tesSUCCESS"
}

@tequdev tequdev requested a review from a team as a code owner June 10, 2025 08:12
@kennyzlei kennyzlei requested a review from gregtatcam June 10, 2025 08:16
Copy link

codecov bot commented Jun 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.4%. Comparing base (8bfaa7f) to head (1315252).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5485     +/-   ##
=========================================
- Coverage     78.4%   78.4%   -0.0%     
=========================================
  Files          816     816             
  Lines        71710   71727     +17     
  Branches      8572    8578      +6     
=========================================
+ Hits         56248   56258     +10     
- Misses       15462   15469      +7     
Files with missing lines Coverage Δ
src/xrpld/app/tx/detail/SetOracle.cpp 100.0% <100.0%> (ø)

... and 3 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@gregtatcam gregtatcam left a comment

Choose a reason for hiding this comment

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

LGTM

@kennyzlei kennyzlei requested a review from shawnxie999 July 25, 2025 18:21
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.

3 participants