Skip to content

Conversation

@pkcs8
Copy link

@pkcs8 pkcs8 commented Sep 19, 2025

High Level Overview of Change

Added hashOracle utility hashing function for Oracle objects.

Context of Change

  • Added utility hash function hashOracle
  • Added oracle space key to ledgerSpaces.ts
  • Added a test for hashOracle

Type of Change

  • New feature (non-breaking change which adds functionality)

Did you update HISTORY.md?

  • No, this change does not impact library users

Test Plan

This chance closely follows hashEscrow utility function and implements similar tests for hashOracle function implementation.

Copy link
Collaborator

@achowdhry-ripple achowdhry-ripple left a comment

Choose a reason for hiding this comment

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

LGTM

@achowdhry-ripple achowdhry-ripple enabled auto-merge (squash) October 29, 2025 18:07
@achowdhry-ripple
Copy link
Collaborator

@pkcs8 Looks like the branch is out of date -- can you rebase with main?

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

Walkthrough

This PR adds a new hashOracle utility function to the XRP Ledger hashes module that computes SHA-512Half hashes for OracleSet transactions using an owner's classic address and document ID. The function is integrated into the public API, documented, tested, and follows existing hashing patterns in the codebase.

Changes

Cohort / File(s) Summary
Documentation
packages/xrpl/src/utils/hashes/README.md
Added API documentation for the new hashOracle(address, documentId) function describing its purpose and usage for hashing OracleSet transaction identifiers.
Core Implementation
packages/xrpl/src/utils/hashes/index.ts, packages/xrpl/src/utils/hashes/ledgerSpaces.ts
Implemented hashOracle(address: string, documentId: number): string function that concatenates ledger space hex, address hex, and zero-padded document ID, then applies SHA-512Half hashing. Added oracle: 'R' namespace prefix mapping to ledgerSpaces object.
Public API Export
packages/xrpl/src/utils/index.ts
Imported and re-exported hashOracle from the hashes module to extend the public hashes API surface.
Test Coverage
packages/xrpl/test/utils/hashes.test.ts
Added test case calcOracleEntryHash to verify the hashOracle function computes correct Oracle entry hashes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Straightforward implementation following established patterns from existing hash utility functions
  • New function uses only existing primitives (sha512Half, ledgerSpaceHex, addressToHex)
  • Homogeneous changes across files (consistent additive pattern)
  • Test coverage provided

Areas for attention:

  • Verify the oracle: 'R' namespace prefix value is correct per XRP Ledger specification
  • Confirm the order and concatenation logic of hashing components aligns with protocol requirements
  • Validate test vectors and expected hash outputs

Suggested reviewers

  • Patel-Raj11
  • khancode

Poem

A rabbit hops through ledger space so bright,
Computing oracle hashes, getting them just right!
With SHA-512Half and prefixes galore,
Our fuzzy friend adds functions to explore. 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Add hashOracle utility hash function" is clear, concise, and directly summarizes the main change in the changeset. It accurately describes the new utility function being added and follows good naming conventions without vague terms, emojis, or unnecessary noise. A teammate reviewing the git history would immediately understand that this PR introduces a new hashing utility function for Oracle objects.
Description Check ✅ Passed The pull request description addresses all required sections from the template: High Level Overview of Change is provided with a clear summary of the new hashOracle function, Context of Change explains what was added and notes the implementation follows the existing hashEscrow pattern, Type of Change correctly identifies this as a new feature, HISTORY.md status is marked as not requiring updates, and a Test Plan is included describing the test implementation approach. While the Test Plan section could be more detailed about specific test cases, the description is mostly complete and provides sufficient information about the changes and testing approach.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30a48e3 and 62e9bf3.

📒 Files selected for processing (5)
  • packages/xrpl/src/utils/hashes/README.md (1 hunks)
  • packages/xrpl/src/utils/hashes/index.ts (1 hunks)
  • packages/xrpl/src/utils/hashes/ledgerSpaces.ts (1 hunks)
  • packages/xrpl/src/utils/index.ts (2 hunks)
  • packages/xrpl/test/utils/hashes.test.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/xrpl/src/utils/hashes/index.ts (2)
packages/ripple-binary-codec/src/binary.ts (1)
  • sha512Half (232-232)
packages/ripple-binary-codec/src/hashes.ts (1)
  • sha512Half (75-75)
packages/xrpl/test/utils/hashes.test.ts (1)
packages/xrpl/src/utils/hashes/index.ts (1)
  • hashOracle (174-180)
🔇 Additional comments (7)
packages/xrpl/test/utils/hashes.test.ts (2)

19-19: LGTM!

The import correctly includes hashOracle alongside other hash functions.


142-150: LGTM!

The test case follows the established pattern from hashEscrow and other hash function tests. The test validates the Oracle hash calculation with a valid account address and document ID.

packages/xrpl/src/utils/hashes/README.md (1)

63-66: LGTM!

The documentation clearly describes the hashOracle function and follows the same format as other hash function documentation entries.

packages/xrpl/src/utils/index.ts (2)

48-48: LGTM!

The hashOracle import is correctly positioned after hashEscrow and before hashPaymentChannel, maintaining consistency with the module organization.


182-182: LGTM!

The hashOracle function is correctly added to the exported hashes object, maintaining the same order as the imports.

packages/xrpl/src/utils/hashes/index.ts (1)

166-180: LGTM!

The hashOracle implementation correctly follows the same pattern as hashEscrow (lines 158-164). It properly concatenates the oracle ledger space prefix, address hex, and zero-padded document ID before computing the SHA-512Half hash.

packages/xrpl/src/utils/hashes/ledgerSpaces.ts (1)

20-20: I need to verify the oracle ledger space prefix through an alternative approach since the initial script produced no output. Let me try a different method to access the rippled source.

I can see from the rippled source that the LedgerNameSpace enum defines spaces like spaceAccount ('a'), spaceDirNode ('d'), spaceGenerator ('g'), spaceRipple ('r'), and spaceOffer ('o'). However, the search results don't show an oracle-related entry. Let me search more specifically for oracle namespace support in rippled.

Let me search more specifically for the oracle ledger space prefix in rippled or the XLS-0047 specification.

Based on my web search results, I've found information about the oracle feature being implemented in rippled 2.2.0 per the XLS-0047 spec, but I cannot find explicit documentation confirming that the oracle ledger space prefix should be 'R'.

The most recent rippled LedgerFormats.h source code I found does not include an explicit oracle or spaceOracle entry in the LedgerNameSpace enum. Since the verification cannot be conclusively completed through available web documentation, I'll request manual verification.

Verify the oracle ledger space prefix against current rippled source.

The oracle feature is defined in the XLS-0047 spec, but publicly available rippled source code and documentation do not explicitly confirm the oracle ledger space prefix. Please manually verify that 'R' is the correct prefix by checking the latest rippled LedgerFormats.h or Indexes.cpp against your target rippled version.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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