XLS-47d: Price Oracle

[gregtatcam]

Discussions thread can be found here: https://github.com/XRPLF/XRPL-Standards/discussions/129

[gregtatcam]

@aanchal4 and @mDuo13 please review.

[gregtatcam]

Just FYI to the specs reviewers - unless something comes up, the implementation PR is expected to be opened soon - either by the end of this week (10/26) or early next week (10/30).

[gregtatcam]

This is a comment from @thejohnfreeman : https://github.com/XRPLF/rippled/pull/4789#issuecomment-1819874166

I find the description of the OracleID confusing. The spec says this:

We compute the PriceOracle object ID, a.k.a., OracleID, as the SHA-512Half of the following values, concatenated in order:

• The Oracle space key (0x52)
• The Owner Account ID
• The Oracle Sequence. This field must be passed to the transactions and it describes a unique Price Oracle sequence for the given account.

• OracleID is not mentioned in the description of the fields of PriceOracle, but it is included in the JSON of an example PriceOracle. Is it included in the ledger object or not?
• It is implied but not explicitly stated that the OracleID is used as the SHAMap key for a PriceOracle object. Is it? How are these objects supposed to be laid out in the ledger? The aggregation transaction suggests that it is possible to iterate PriceOracle objects for a given account. How?
• Oracle Sequence is not mentioned in the fields of PriceOracle or shown in the example. Is it possible to recover the OracleID for a PriceOracle using only the fields contained within a PriceOracle? If so, how?
• "Sequence" implies to me that it is a number that increments by one with each change, like a Ledger Sequence or an Account Sequence, but OracleID implies a constant identifier. Which is it? Are there any rules on the sequence number?

In the spec, the list following "The transaction fails if" includes:

• The URI field length exceeds 64 bytes.
• The Provider field length exceeds 64 bytes.

But the section describing the PriceOracle on-ledger object fields says:

• Provider identifies an Oracle Provider. It can be URI or any data, for instance chainlink. It is a string of up to 256 ASCII hex encoded characters (0x20-0x7E).
• URI is an optional URI field to reference price data off-chain. It is limited to 256 bytes.

Which is correct? Can we fix it in the spec?

I find this explanation unclear:

If an object with the OracleID Object ID already exists then the new token pairs are added to the Oracle instance PriceDataSeries array. Note that the order of the token pairs in the PriceDataSeries array is not important since the token pair uniquely identifies location in the PriceDataSeries array of the PriceOracle object. Also note that not every token pair price has to be updated. I.e., even though the PriceOracle may define ten token pairs, OracleSet transaction may contain only one token pair price update. In this case the missing token pair will not include AssetPrice and Scale fields. PreviousTxnID can be used to find the last updated Price Data for this token pair.

"If an object with the ID exists, then the new token pairs are added..." I'm assuming "new token pairs" are ones that do not appear in the PriceOracle. If so, then why no mention of "old token pairs"? They should be updating what appears in the PriceOracle, correct?

"In this case the missing token pair..." What missing token pair? The token pairs that are in the object, but not the transaction? If yes, then what is meant by "[they] will not include AssetPrice and Scale fields"? If they are missing, won't they "not include" every field?

I feel like there is an implied behavior for this transaction that is not well described by the existing language. Here is how I would describe that behavior. Is it correct?

An OracleSet transaction uniquely identifies a PriceOracle object with its Account and OracleSequence fields. If such an object does not yet exist in the ledger, it is created. Otherwise, the existing object is updated. The Provider, URI, and AssetClass fields are copied directly from the transaction, if present. Provider and AssetClass must be included in the transaction if the object is being created.

The PriceDataSeries of the transaction is copied to a newly created PriceOracle object, or updates an existing object, like so:

• PriceData objects for (BaseAsset, QuoteAsset) token pairs that appear in the transaction but not the object are copied to the object.
• PriceData objects for token pairs that appear in both the transaction and the object are overwritten in the object.
• PriceData objects for token pairs that appear only in the object are left unchanged.

The order of token pairs in the transaction is not important because the token pair uniquely identifies the location of the PriceData object in the PriceDataSeries array of the PriceOracle object.

LastUpdateTime, PreviousTxnID, and PreviousTxnLgrSeq are set in the same manner as for an AccountSet transaction.

The owner reserve of the account is updated according to the difference in the size of the PriceDataSeries before and after the transaction is applied: 0 for missing, 1 for 1 - 5 objects, 2 for 6 - 10 objects.

AssetClass is a required field on a PriceOracle object and an optional field in an OracleSet transaction, just like Provider, but its field description does not say that it must be included in the transaction when creating a new instance of PriceOracle, unlike Provider. It must be included in the transaction, correct?

A side note on formatting. I think the presentation in the section on PriceOracle fields is best: one table with all the fields, followed by a bulleted list of field descriptions. Not like the transaction sections with separate tables (repeating the same header row) for each field. I think the example should go directly after or before the table, not at the end of the section.

[gregtatcam]

@thejohnfreeman. Thanks for the feedback.

I removed the references to the OracleID. It's not used anywhere. The PriceOracle object is identified by Account and OracleDocumentID. The latter replaces OracleSequence.

I corrected the validation fields for URI and Provider to 32 bytes.

I updated the OracleSet transaction description per your suggestion.

I followed the format of tables/description of other specs like AMM/NFT/DID. I agree that having one table followed by the description is better. I changed the format and moved the json examples for transactions and RPC before the tables.

I'll update the implementation If the updates look right to you.

[gregtatcam]

I don't think the .DS_Store file should be included in this PR (perhaps it needs to be added to .gitignore?

Thanks. I must have selected all Unversioned files in CLion. Will delete.

[mvadari]

Please make sure the PDF has either been updated to match the markdown or remove it.